Skip to content

Commit

Permalink
[ISSUE 461] Fix issue when getting the fork name for the cases where …
Browse files Browse the repository at this point in the history 
…the fork has a different name (#462)

* [ISSUE 461] Fix issue when getting the fork name for the cases where the fork has a different name

* fix e2e-regression tests

* fix e2e test

* add unitary test for fork repository with different name

* Apply suggestions from code review

Co-authored-by: Enrique Mingorance Cano <ginxaco@gmail.com>

* fix typo in e2e test

* fix typo in debug message

---------

Co-authored-by: Enrique Mingorance Cano <ginxaco@gmail.com>
  • Loading branch information
@rgdoliveira @Ginxo
rgdoliveira and Ginxo committed Oct 5, 2023
1 parent 52ecca8 commit da21001
Show file tree
Hide file tree
Showing 9 changed files with 72 additions and 44 deletions.
4 changes: 2 additions & 2 deletions package-lock.json
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@kie/build-chain-action",
"version": "3.5.4",
"version": "3.5.5",
"description": "Library to execute commands based on github projects dependencies.",
"main": "dist/index.js",
"author": "",
Expand Down
17 changes: 7 additions & 10 deletions src/service/git/git-api-service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,15 +125,18 @@ export class GitAPIService {
): Promise<string> {
try {
// check whether there is a fork with the same name as repo name
const repoName = await this.checkIfRepositoryExists(targetOwner, repo);
this.logger.info(`Checking if ${targetOwner}/${repo} is forked to ${sourceOwner}/${repo}`);
const repoName = await this.checkIfRepositoryExists(sourceOwner, repo);

if (repoName) {
this.logger.info(`Fork ${sourceOwner}/${repo} found.`);
return repoName;
} else if (targetOwner !== sourceOwner) {
/**
* find repo from fork list. we reach this case only if we are in the edge case where the forked repo's name is different
* from the original one
*/
this.logger.info(`Fork ${sourceOwner}/${repo} does not exist. Trying to find a fork with a different name in ${sourceOwner}`);
const forkName = (
await this.client
.rest(targetOwner, repo)
Expand All @@ -144,17 +147,13 @@ export class GitAPIService {
})
).data;
if (forkName) {
this.logger.info(`Fork ${sourceOwner}/${forkName} found from ${targetOwner}/${repo}`);
return forkName;
}
}
throw new NotFoundError();
} catch (err) {
this.logger.error(
this.getErrorMessage(
err,
`Error getting fork name for ${targetOwner}/${repo} where owner is ${sourceOwner}`
)
);
this.logger.info(`Fork for ${targetOwner}/${repo} not found where owner is ${sourceOwner}`);
throw err;
}
}
Expand Down Expand Up @@ -202,9 +201,7 @@ export class GitAPIService {
});
return repo;
} catch (err) {
this.logger.error(
this.getErrorMessage(err, `Failed to get ${owner}/${repo}.`)
);
this.logger.debug(`Failed to get ${owner}/${repo}, it is not necessarily an error`);
return undefined;
}
}
Expand Down
6 changes: 3 additions & 3 deletions test/e2e-regression/cli/tests.json
View file
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
[
{
"name": "issue-372",
"cmd": "build-chain build cross_pr -f 'https://raw.githubusercontent.com/kiegroup/droolsjbpm-build-bootstrap/${BRANCH:main}/.ci/compilation-config.yaml' -o bc -u https://github.com/kiegroup/drools/pull/4978 --skipExecution",
"cmd": "build-chain build cross_pr -f 'https://raw.githubusercontent.com/kiegroup/droolsjbpm-build-bootstrap/${BRANCH:main}/.ci/compilation-config.yaml' -o bc -u https://github.com/kiegroup/appformer/pull/1394 --skipExecution",
"description": "To test that definition file url placeholders fallback to using default value. Checking whether build succeeded is enough"
},
{
"name": "issue-401",
"cmd": "build-chain build cross_pr -f 'https://raw.githubusercontent.com/kiegroup/kogito-pipelines/%{process.env.GITHUB_BASE_REF.replace(/(\\d*)\\.(.*)\\.(.*)/g, (m, n1, n2, n3) => `${+n1-7}.${n2}.${n3}`)}/.ci/pull-request-config.yaml' -o bc -u https://github.com/kiegroup/optaplanner/pull/2634 -p kiegroup/optaplanner --skipExecution",
"cmd": "build-chain build cross_pr -f 'https://raw.githubusercontent.com/apache/incubator-kie-kogito-pipelines/%{process.env.GITHUB_BASE_REF.replace(/(\\d*)\\.(.*)\\.(.*)/g, (m, n1, n2, n3) => `${+n1-7}.${n2}.${n3}`)}/.ci/pull-request-config.yaml' -o bc -u https://github.com/apache/incubator-kie-optaplanner/pull/2634 -p kiegroup/optaplanner --skipExecution",
"description": "To test that definition file url placeholders as well as expressions work when defined together. Checking whether build succeeded is enough"
},
{
"name": "issue-386",
"cmd": "build-chain build cross_pr -f 'https://raw.githubusercontent.com/kiegroup/kogito-pipelines/1.34.x/.ci/pull-request-config.yaml' -o bc -u https://github.com/kiegroup/kogito-examples/pull/1570 -p kiegroup/kogito-examples --skipExecution",
"cmd": "build-chain build cross_pr -f 'https://raw.githubusercontent.com/apache/incubator-kie-kogito-pipelines/1.34.x/.ci/pull-request-config.yaml' -o bc -u https://github.com/apache/incubator-kie-kogito-examples/pull/1570 -p kiegroup/kogito-examples --skipExecution",
"description": "To test if GITHUB_BASE_REF is set during CLI execution. Checking if 8.34.x is checked out of drools should verify this",
"matchOutput": [
"Project taken from kiegroup/drools:8.34.x",
Expand Down
8 changes: 4 additions & 4 deletions test/e2e-regression/github-action/tests.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,18 @@
"name": "issue-372",
"definition-file": "https://raw.githubusercontent.com/kiegroup/droolsjbpm-build-bootstrap/${BRANCH:main}/.ci/compilation-config.yaml",
"flow-type": "cross_pr",
"starting-project": "kiegroup/drools",
"starting-project": "kiegroup/appformer",
"skip-execution": "true",
"event": "https://github.com/kiegroup/drools/pull/4978",
"event": "https://github.com/kiegroup/appformer/pull/1394",
"description": "To test that definition file url placeholders fallback to using default value. Checking whether build succeeded is enough"
},
{
"name": "issue-401",
"definition-file": "https://raw.githubusercontent.com/kiegroup/kogito-pipelines/%{process.env.GITHUB_BASE_REF.replace(/(\\d*)\\.(.*)\\.(.*)/g, (m, n1, n2, n3) => `${+n1-7}.${n2}.${n3}`)}/.ci/pull-request-config.yaml",
"definition-file": "https://raw.githubusercontent.com/apache/incubator-kie-kogito-pipelines/%{process.env.GITHUB_BASE_REF.replace(/(\\d*)\\.(.*)\\.(.*)/g, (m, n1, n2, n3) => `${+n1-7}.${n2}.${n3}`)}/.ci/pull-request-config.yaml",
"starting-project": "kiegroup/optaplanner",
"flow-type": "cross_pr",
"skip-execution": "true",
"event": "https://github.com/kiegroup/optaplanner/pull/2634",
"event": "https://github.com/apache/incubator-kie-optaplanner/pull/2634",
"description": "To test that definition file url placeholders as well as expressions work when defined together. Checking whether build succeeded is enough"
},
{
Expand Down
19 changes: 14 additions & 5 deletions test/e2e/cross-pr/cross-pr.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,13 +158,22 @@ test("PR from owner1/target:branchA to owner2/target:branchB while using mapping
mockApi: [
moctokit.rest.repos
.get({
owner: "owner1",
repo: /project(1|2|4)/,
owner: "owner2",
repo: /project(1|2)/,
})
.setResponse({
status: 404,
data: {},
repeat: 3
repeat: 2
}),
moctokit.rest.repos
.get({
owner: "owner2",
repo: "project4"
})
.setResponse({
status: 200,
data: {},
}),
moctokit.rest.repos
.listForks({
Expand All @@ -184,12 +193,12 @@ test("PR from owner1/target:branchA to owner2/target:branchB while using mapping
}),
moctokit.rest.repos
.get({
owner: "owner1",
owner: "owner2",
repo: "project3",
})
.setResponse({
status: 200,
data: { name: "project3", owner: { login: "owner1" } },
data: { name: "project3", owner: { login: "owner2" } },
}),
moctokit.rest.repos
.listForks({
Expand Down
17 changes: 13 additions & 4 deletions test/e2e/full-downstream/full-downstream.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,13 +187,22 @@ test("PR from owner1/target:branchA to owner2/target:branchB while using mapping
mockApi: [
moctokit.rest.repos
.get({
owner: "owner1",
repo: /project(1|2|3|4)/,
owner: "owner2",
repo: "project2",
})
.setResponse({
status: 404,
data: {},
repeat: 4
}),
moctokit.rest.repos
.get({
owner: "owner2",
repo: /project(1|3|4)/,
})
.setResponse({
status: 200,
data: {},
repeat: 3
}),
moctokit.rest.repos
.listForks({
Expand Down Expand Up @@ -313,7 +322,7 @@ test("PR from owner1/target:branchA to owner2/target:branchB while using mapping
);
expect(group4.output).toEqual(
expect.stringContaining(
"Merged owner1/project4:branchA into branch 8.x"
"Merged owner2/project4:branchA into branch 8.x"
)
);

Expand Down
6 changes: 3 additions & 3 deletions test/e2e/single-pr/single-pr.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -262,15 +262,15 @@ test("PR from owner2/target:branchA to owner1/target:branchB", async () => {
mockApi: [
moctokit.rest.repos
.get({
owner: "owner1",
owner: "owner2",
repo: "project3",
})
.setResponse({
status: 200,
data: {
name: "project3",
owner: {
login: "owner1",
login: "owner2",
},
},
}),
Expand Down Expand Up @@ -391,7 +391,7 @@ test("PR from owner2/target:branchA to owner1/target-different-name:branchB", as
mockApi: [
moctokit.rest.repos
.get({
owner: "owner1",
owner: "owner2",
repo: "project1",
})
.setResponse({
Expand Down
37 changes: 25 additions & 12 deletions test/unitary/service/git/git-api.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,49 +119,62 @@ test.each([
[
"success: same source and target owner",
true,
["target1", "target1", "repoA"],
["target1", "repoA", "target1", "repoA"],
],
[
"failure: same source and target owner",
false,
["target2", "target2", "repoA"],
["target2", "repoA", "target2", "repoA"],
],
[
"success: different source and target owner",
true,
["target3", "source", "repoA"],
["target3", "repoA", "source", "repoA"],
],
[
"failure: different source and target owner",
false,
["target4", "source", "repoA"],
["target4", "repoA", "source", "repoA"],
],
[
"success: different source and target owner / different repo name",
true,
["target5", "repoA", "source", "repoA_fork"],
],
[
"failure: different source and target owner / different repo name",
false,
["target6", "repoA", "source", "no_fork"],
],
])(
"getForkName %p",
async (_title: string, testForSuccess: boolean, args: string[]) => {
const moctokit = new Moctokit();
if (testForSuccess) {
moctokit.rest.repos
.get({ owner: args[1], repo: args[2] })
.get({ owner: args[0], repo: args[1] })
.reply({ status: 200, data: {} });
moctokit.rest.repos
.get({ owner: args[2], repo: args[3] })
.reply({ status: 200, data: {} });
moctokit.rest.repos.listForks({ owner: args[0], repo: args[2] }).reply({
moctokit.rest.repos.listForks({ owner: args[0], repo: args[1] }).reply({
status: 200,
data: [{ name: args[2], owner: { login: args[1] } }],
data: [{ name: args[3], owner: { login: args[2] } }],
});

await expect(git.getForkName(args[0], args[1], args[2])).resolves.toBe(
args[2]
await expect(git.getForkName(args[0], args[2], args[1])).resolves.toBe(
args[3]
);
} else {
moctokit.rest.repos
.get({ owner: args[1], repo: args[2] })
.get({ owner: args[0], repo: args[1] })
.reply({ status: 404, data: {} });
moctokit.rest.repos
.listForks({ owner: args[0], repo: args[2] })
.listForks({ owner: args[2], repo: args[3] })
.reply({ status: 200, data: [] });

await expect(
git.getForkName(args[0], args[1], args[2])
git.getForkName(args[0], args[2], args[1])
).rejects.toThrowError();
}
}
Expand Down

0 comments on commit da21001

Please sign in to comment.