feat: auto-detect the value of the no-squash option (#118)

The auto-no-squash option is added to:

* backport all the commits when the pull/merge request has been merged
* backport the squashed commit otherwise

It is equivalent to dynamically adjust the value of the no-squash
option, depending on the context.

The no-squash option is kept for backward compatibility for a single
use case: backporting the merged commit instead of backporting the
commits of the pull/merge request request.

Detecting if a pull/merge request was squashed or not depends on the
underlying forge:

* Forgejo / GitHub: use the API to count the number of parents
* GitLab: if the squash_commit_sha is set, the merge request was
  squashed

If the pull/merge request is open, always backport all the commits it
contains.

Fixes: https://github.com/kiegroup/git-backporting/issues/113

Co-authored-by: Andrea Lamparelli <a.lamparelli95@gmail.com>
This commit is contained in:
Earl Warren 2024-04-08 18:51:13 +02:00 committed by GitHub
parent fc5dba6703
commit 6042bcc40b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
23 changed files with 324 additions and 54 deletions

View file

@ -57,7 +57,7 @@ describe("github pull request config parser", () => {
const configs: Configs = await configParser.parseAndValidate(args);
expect(GitHubClient.prototype.getPullRequest).toBeCalledTimes(1);
expect(GitHubClient.prototype.getPullRequest).toBeCalledWith("owner", "reponame", 2368, true);
expect(GitHubClient.prototype.getPullRequest).toBeCalledWith("owner", "reponame", 2368, undefined);
expect(GitHubMapper.prototype.mapPullRequest).toBeCalledTimes(1);
expect(GitHubMapper.prototype.mapPullRequest).toBeCalledWith(expect.anything(), []);
@ -129,7 +129,7 @@ describe("github pull request config parser", () => {
const configs: Configs = await configParser.parseAndValidate(args);
expect(GitHubClient.prototype.getPullRequest).toBeCalledTimes(1);
expect(GitHubClient.prototype.getPullRequest).toBeCalledWith("owner", "reponame", 2368, true);
expect(GitHubClient.prototype.getPullRequest).toBeCalledWith("owner", "reponame", 2368, undefined);
expect(GitHubMapper.prototype.mapPullRequest).toBeCalledTimes(1);
expect(GitHubMapper.prototype.mapPullRequest).toBeCalledWith(expect.anything(), []);
@ -202,7 +202,7 @@ describe("github pull request config parser", () => {
const configs: Configs = await configParser.parseAndValidate(args);
expect(GitHubClient.prototype.getPullRequest).toBeCalledTimes(1);
expect(GitHubClient.prototype.getPullRequest).toBeCalledWith("owner", "reponame", 2368, true);
expect(GitHubClient.prototype.getPullRequest).toBeCalledWith("owner", "reponame", 2368, undefined);
expect(GitHubMapper.prototype.mapPullRequest).toBeCalledTimes(1);
expect(GitHubMapper.prototype.mapPullRequest).toBeCalledWith(expect.anything(), []);
@ -275,7 +275,7 @@ describe("github pull request config parser", () => {
const configs: Configs = await configParser.parseAndValidate(args);
expect(GitHubClient.prototype.getPullRequest).toBeCalledTimes(1);
expect(GitHubClient.prototype.getPullRequest).toBeCalledWith("owner", "reponame", 2368, true);
expect(GitHubClient.prototype.getPullRequest).toBeCalledWith("owner", "reponame", 2368, undefined);
expect(GitHubMapper.prototype.mapPullRequest).toBeCalledTimes(1);
expect(GitHubMapper.prototype.mapPullRequest).toBeCalledWith(expect.anything(), []);

View file

@ -89,7 +89,7 @@ describe("github pull request config parser", () => {
const configs: Configs = await configParser.parseAndValidate(args);
expect(GitHubClient.prototype.getPullRequest).toBeCalledTimes(1);
expect(GitHubClient.prototype.getPullRequest).toBeCalledWith("owner", "reponame", 2368, true);
expect(GitHubClient.prototype.getPullRequest).toBeCalledWith("owner", "reponame", 2368, undefined);
expect(GitHubMapper.prototype.mapPullRequest).toBeCalledTimes(1);
expect(GitHubMapper.prototype.mapPullRequest).toBeCalledWith(expect.anything(), []);
@ -182,9 +182,9 @@ describe("github pull request config parser", () => {
const configs: Configs = await configParser.parseAndValidate(args);
expect(GitHubClient.prototype.getPullRequest).toBeCalledTimes(1);
expect(GitHubClient.prototype.getPullRequest).toBeCalledWith("owner", "reponame", 4444, true);
expect(GitHubClient.prototype.getPullRequest).toBeCalledWith("owner", "reponame", 4444, undefined);
expect(GitHubMapper.prototype.mapPullRequest).toBeCalledTimes(1);
expect(GitHubMapper.prototype.mapPullRequest).toBeCalledWith(expect.anything(), []);
expect(GitHubMapper.prototype.mapPullRequest).toBeCalledWith(expect.anything(), ["0404fb922ab75c3a8aecad5c97d9af388df04695", "11da4e38aa3e577ffde6d546f1c52e53b04d3151"]);
expect(configs.dryRun).toEqual(true);
expect(configs.auth).toEqual("whatever");
@ -217,8 +217,7 @@ describe("github pull request config parser", () => {
},
bpBranchName: undefined,
nCommits: 2,
// taken from head.sha
commits: ["91748965051fae1330ad58d15cf694e103267c87"]
commits: ["0404fb922ab75c3a8aecad5c97d9af388df04695", "11da4e38aa3e577ffde6d546f1c52e53b04d3151"],
});
});
@ -258,7 +257,7 @@ describe("github pull request config parser", () => {
const configs: Configs = await configParser.parseAndValidate(args);
expect(GitHubClient.prototype.getPullRequest).toBeCalledTimes(1);
expect(GitHubClient.prototype.getPullRequest).toBeCalledWith("owner", "reponame", 2368, true);
expect(GitHubClient.prototype.getPullRequest).toBeCalledWith("owner", "reponame", 2368, undefined);
expect(GitHubMapper.prototype.mapPullRequest).toBeCalledTimes(1);
expect(GitHubMapper.prototype.mapPullRequest).toBeCalledWith(expect.anything(), []);
@ -331,7 +330,7 @@ describe("github pull request config parser", () => {
const configs: Configs = await configParser.parseAndValidate(args);
expect(GitHubClient.prototype.getPullRequest).toBeCalledTimes(1);
expect(GitHubClient.prototype.getPullRequest).toBeCalledWith("owner", "reponame", 2368, true);
expect(GitHubClient.prototype.getPullRequest).toBeCalledWith("owner", "reponame", 2368, undefined);
expect(GitHubMapper.prototype.mapPullRequest).toBeCalledTimes(1);
expect(GitHubMapper.prototype.mapPullRequest).toBeCalledWith(expect.anything(), []);
@ -372,7 +371,7 @@ describe("github pull request config parser", () => {
const configs: Configs = await configParser.parseAndValidate(args);
expect(GitHubClient.prototype.getPullRequest).toBeCalledTimes(1);
expect(GitHubClient.prototype.getPullRequest).toBeCalledWith("owner", "reponame", 2368, true);
expect(GitHubClient.prototype.getPullRequest).toBeCalledWith("owner", "reponame", 2368, undefined);
expect(GitHubMapper.prototype.mapPullRequest).toBeCalledTimes(1);
expect(GitHubMapper.prototype.mapPullRequest).toBeCalledWith(expect.anything(), []);
@ -444,7 +443,7 @@ describe("github pull request config parser", () => {
const configs: Configs = await configParser.parseAndValidate(args);
expect(GitHubClient.prototype.getPullRequest).toBeCalledTimes(1);
expect(GitHubClient.prototype.getPullRequest).toBeCalledWith("owner", "reponame", 2368, true);
expect(GitHubClient.prototype.getPullRequest).toBeCalledWith("owner", "reponame", 2368, undefined);
expect(GitHubMapper.prototype.mapPullRequest).toBeCalledTimes(1);
expect(GitHubMapper.prototype.mapPullRequest).toBeCalledWith(expect.anything(), []);
@ -518,7 +517,7 @@ describe("github pull request config parser", () => {
const configs: Configs = await configParser.parseAndValidate(args);
expect(GitHubClient.prototype.getPullRequest).toBeCalledTimes(1);
expect(GitHubClient.prototype.getPullRequest).toBeCalledWith("owner", "reponame", 2368, true);
expect(GitHubClient.prototype.getPullRequest).toBeCalledWith("owner", "reponame", 2368, undefined);
expect(GitHubMapper.prototype.mapPullRequest).toBeCalledTimes(1);
expect(GitHubMapper.prototype.mapPullRequest).toBeCalledWith(expect.anything(), []);
@ -791,7 +790,7 @@ describe("github pull request config parser", () => {
const configs: Configs = await configParser.parseAndValidate(args);
expect(GitHubClient.prototype.getPullRequest).toBeCalledTimes(1);
expect(GitHubClient.prototype.getPullRequest).toBeCalledWith("owner", "reponame", 2368, true);
expect(GitHubClient.prototype.getPullRequest).toBeCalledWith("owner", "reponame", 2368, undefined);
expect(GitHubMapper.prototype.mapPullRequest).toBeCalledTimes(1);
expect(GitHubMapper.prototype.mapPullRequest).toBeCalledWith(expect.anything(), []);
@ -888,7 +887,7 @@ describe("github pull request config parser", () => {
const configs: Configs = await configParser.parseAndValidate(args);
expect(GitHubClient.prototype.getPullRequest).toBeCalledTimes(1);
expect(GitHubClient.prototype.getPullRequest).toBeCalledWith("owner", "reponame", 2368, true);
expect(GitHubClient.prototype.getPullRequest).toBeCalledWith("owner", "reponame", 2368, undefined);
expect(GitHubMapper.prototype.mapPullRequest).toBeCalledTimes(1);
expect(GitHubMapper.prototype.mapPullRequest).toBeCalledWith(expect.anything(), []);

View file

@ -51,6 +51,7 @@ describe("gitlab merge request config parser", () => {
labels: [],
inheritLabels: false,
comments: [],
squash: true,
};
const configs: Configs = await configParser.parseAndValidate(args);
@ -123,6 +124,7 @@ describe("gitlab merge request config parser", () => {
labels: [],
inheritLabels: false,
comments: [],
squash: true,
};
const configs: Configs = await configParser.parseAndValidate(args);
@ -195,7 +197,8 @@ describe("gitlab merge request config parser", () => {
labels: [],
inheritLabels: false,
comments: [],
bpBranchName: "custom-branch"
bpBranchName: "custom-branch",
squash: true,
};
const configs: Configs = await configParser.parseAndValidate(args);
@ -268,7 +271,8 @@ describe("gitlab merge request config parser", () => {
labels: [],
inheritLabels: false,
comments: [],
bpBranchName: "custom1, custom2, custom3"
bpBranchName: "custom1, custom2, custom3",
squash: true,
};
const configs: Configs = await configParser.parseAndValidate(args);

View file

@ -88,6 +88,7 @@ describe("gitlab merge request config parser", () => {
reviewers: [],
assignees: [],
inheritReviewers: true,
squash: true,
};
const configs: Configs = await configParser.parseAndValidate(args);
@ -158,6 +159,7 @@ describe("gitlab merge request config parser", () => {
reviewers: [],
assignees: [],
inheritReviewers: true,
squash: true,
};
const configs: Configs = await configParser.parseAndValidate(args);
@ -187,6 +189,7 @@ describe("gitlab merge request config parser", () => {
reviewers: [],
assignees: [],
inheritReviewers: true,
squash: true,
};
const configs: Configs = await configParser.parseAndValidate(args);
@ -243,6 +246,7 @@ describe("gitlab merge request config parser", () => {
reviewers: [],
assignees: [],
inheritReviewers: true,
squash: true,
};
await expect(() => configParser.parseAndValidate(args)).rejects.toThrow("Provided pull request is closed and not merged");
@ -262,6 +266,7 @@ describe("gitlab merge request config parser", () => {
reviewers: [],
assignees: [],
inheritReviewers: true,
squash: true,
};
const configs: Configs = await configParser.parseAndValidate(args);
@ -333,6 +338,7 @@ describe("gitlab merge request config parser", () => {
reviewers: ["user1", "user2"],
assignees: ["user3", "user4"],
inheritReviewers: true, // not taken into account
squash: true,
};
const configs: Configs = await configParser.parseAndValidate(args);
@ -404,6 +410,7 @@ describe("gitlab merge request config parser", () => {
reviewers: [],
assignees: ["user3", "user4"],
inheritReviewers: false,
squash: true,
};
const configs: Configs = await configParser.parseAndValidate(args);
@ -477,6 +484,7 @@ describe("gitlab merge request config parser", () => {
inheritReviewers: false,
labels: ["custom-label", "backport-prod"], // also include the one inherited
inheritLabels: true,
squash: true,
};
const configs: Configs = await configParser.parseAndValidate(args);
@ -742,6 +750,7 @@ describe("gitlab merge request config parser", () => {
labels: [],
inheritLabels: false,
comments: ["First comment", "Second comment"],
squash: true,
};
const configs: Configs = await configParser.parseAndValidate(args);
@ -816,6 +825,7 @@ describe("gitlab merge request config parser", () => {
labels: [],
inheritLabels: false,
comments: ["First comment", "Second comment"],
squash: true,
};
const configs: Configs = await configParser.parseAndValidate(args);

View file

@ -1,4 +1,4 @@
import { inferGitApiUrl, inferGitClient } from "@bp/service/git/git-util";
import { inferGitApiUrl, inferGitClient, inferSquash } from "@bp/service/git/git-util";
import { GitClientType } from "@bp/service/git/git.types";
describe("check git utilities", () => {
@ -54,4 +54,10 @@ describe("check git utilities", () => {
test("check infer codeberg client", ()=> {
expect(inferGitClient("https://codeberg.org/lampajr/backporting-example/pulls/1")).toStrictEqual(GitClientType.CODEBERG);
});
});
test("check inferSquash", ()=> {
expect(inferSquash(true, undefined)).toStrictEqual(false);
expect(inferSquash(false, "SHA")).toStrictEqual(true);
expect(inferSquash(false, undefined)).toStrictEqual(false);
});
});

View file

@ -22,7 +22,7 @@ describe("github service", () => {
});
test("get pull request: success", async () => {
const res: GitPullRequest = await gitClient.getPullRequest(TARGET_OWNER, REPO, MERGED_PR_FIXTURE.number);
const res: GitPullRequest = await gitClient.getPullRequest(TARGET_OWNER, REPO, MERGED_PR_FIXTURE.number, true);
expect(res.sourceRepo).toEqual({
owner: "fork",
project: "reponame",

View file

@ -31,7 +31,7 @@ describe("github service", () => {
});
test("get merged pull request", async () => {
const res: GitPullRequest = await gitClient.getPullRequest("superuser", "backporting-example", 1);
const res: GitPullRequest = await gitClient.getPullRequest("superuser", "backporting-example", 1, true);
// check content
expect(res.sourceRepo).toEqual({
@ -56,7 +56,7 @@ describe("github service", () => {
});
test("get open pull request", async () => {
const res: GitPullRequest = await gitClient.getPullRequest("superuser", "backporting-example", 2);
const res: GitPullRequest = await gitClient.getPullRequest("superuser", "backporting-example", 2, true);
expect(res.sourceRepo).toEqual({
owner: "superuser",
project: "backporting-example",
@ -325,7 +325,7 @@ describe("github service", () => {
});
test("get pull request for nested namespaces", async () => {
const res: GitPullRequest = await gitClient.getPullRequestFromUrl("https://my.gitlab.host.com/mysuperorg/6/mysuperproduct/mysuperunit/backporting-example/-/merge_requests/4");
const res: GitPullRequest = await gitClient.getPullRequestFromUrl("https://my.gitlab.host.com/mysuperorg/6/mysuperproduct/mysuperunit/backporting-example/-/merge_requests/4", true);
// check content
expect(res.sourceRepo).toEqual({

View file

@ -300,7 +300,7 @@ describe("cli runner", () => {
await expect(() => runner.execute()).rejects.toThrow("Provided pull request is closed and not merged");
});
test("open pull request", async () => {
test("open pull request simple", async () => {
addProcessArgs([
"-tb",
"target",
@ -347,6 +347,55 @@ describe("cli runner", () => {
expect(GitHubClient.prototype.createPullRequest).toReturnTimes(1);
});
test("open pull request with --auto-no-squash", async () => {
addProcessArgs([
"-tb",
"target",
"-pr",
"https://github.com/owner/reponame/pull/4444",
"--auto-no-squash",
]);
await runner.execute();
const cwd = process.cwd() + "/bp";
expect(GitClientFactory.getOrCreate).toBeCalledTimes(1);
expect(GitClientFactory.getOrCreate).toBeCalledWith(GitClientType.GITHUB, undefined, "https://api.github.com");
expect(GitCLIService.prototype.clone).toBeCalledTimes(1);
expect(GitCLIService.prototype.clone).toBeCalledWith("https://github.com/owner/reponame.git", cwd, "target");
expect(GitCLIService.prototype.createLocalBranch).toBeCalledTimes(1);
expect(GitCLIService.prototype.createLocalBranch).toBeCalledWith(cwd, "bp-target-0404fb9-11da4e3");
expect(GitCLIService.prototype.fetch).toBeCalledTimes(1);
expect(GitCLIService.prototype.fetch).toBeCalledWith(cwd, "pull/4444/head:pr/4444");
expect(GitCLIService.prototype.cherryPick).toBeCalledTimes(2);
expect(GitCLIService.prototype.cherryPick).toHaveBeenLastCalledWith(cwd, "0404fb922ab75c3a8aecad5c97d9af388df04695", undefined, undefined, undefined);
expect(GitCLIService.prototype.cherryPick).toBeCalledWith(cwd, "11da4e38aa3e577ffde6d546f1c52e53b04d3151", undefined, undefined, undefined);
expect(GitCLIService.prototype.push).toBeCalledTimes(1);
expect(GitCLIService.prototype.push).toBeCalledWith(cwd, "bp-target-0404fb9-11da4e3");
expect(GitHubClient.prototype.createPullRequest).toBeCalledTimes(1);
expect(GitHubClient.prototype.createPullRequest).toBeCalledWith({
owner: "owner",
repo: "reponame",
head: "bp-target-0404fb9-11da4e3",
base: "target",
title: "[target] PR Title",
body: "**Backport:** https://github.com/owner/reponame/pull/4444\r\n\r\nPlease review and merge",
reviewers: ["gh-user"],
assignees: [],
labels: [],
comments: [],
}
);
expect(GitHubClient.prototype.createPullRequest).toReturnTimes(1);
});
test("override backporting pr data", async () => {
addProcessArgs([
"-tb",

View file

@ -1,6 +1,6 @@
import LoggerServiceFactory from "@bp/service/logger/logger-service-factory";
import { Moctokit } from "@kie/mock-github";
import { TARGET_OWNER, REPO, MERGED_PR_FIXTURE, OPEN_PR_FIXTURE, NOT_MERGED_PR_FIXTURE, NOT_FOUND_PR_NUMBER, MULT_COMMITS_PR_FIXTURE, MULT_COMMITS_PR_COMMITS, NEW_PR_URL, NEW_PR_NUMBER } from "./github-data";
import { TARGET_OWNER, REPO, MERGED_PR_FIXTURE, OPEN_PR_FIXTURE, NOT_MERGED_PR_FIXTURE, NOT_FOUND_PR_NUMBER, MULT_COMMITS_PR_FIXTURE, MULT_COMMITS_PR_COMMITS, NEW_PR_URL, NEW_PR_NUMBER, GITHUB_GET_COMMIT } from "./github-data";
import { CLOSED_NOT_MERGED_MR, MERGED_SQUASHED_MR, NESTED_NAMESPACE_MR, OPEN_MR, OPEN_PR_COMMITS, PROJECT_EXAMPLE, NESTED_PROJECT_EXAMPLE, SUPERUSER, MERGED_SQUASHED_MR_COMMITS } from "./gitlab-data";
// high number, for each test we are not expecting
@ -157,6 +157,17 @@ export const mockGitHubClient = (apiUrl = "https://api.github.com"): Moctokit =>
data: MULT_COMMITS_PR_COMMITS
});
mock.rest.pulls
.listCommits({
owner: TARGET_OWNER,
repo: REPO,
pull_number: OPEN_PR_FIXTURE.number
})
.reply({
status: 200,
data: MULT_COMMITS_PR_COMMITS
});
mock.rest.pulls
.create()
.reply({
@ -200,6 +211,17 @@ export const mockGitHubClient = (apiUrl = "https://api.github.com"): Moctokit =>
data: {}
});
mock.rest.git
.getCommit({
owner: TARGET_OWNER,
repo: REPO,
commit_sha: "28f63db774185f4ec4b57cd9aaeb12dbfb4c9ecc",
})
.reply({
status: 200,
data: GITHUB_GET_COMMIT,
});
// invalid requests
mock.rest.pulls
.get({

View file

@ -1832,6 +1832,14 @@ export const MULT_COMMITS_PR_FIXTURE = {
"changed_files": 2
};
export const GITHUB_GET_COMMIT = {
"parents": [
{
"sha": "SHA"
}
]
};
export const MULT_COMMITS_PR_COMMITS = [
{
"sha": "0404fb922ab75c3a8aecad5c97d9af388df04695",