From b4d0481c5649115367f1ae0724d12d55b6b86e10 Mon Sep 17 00:00:00 2001 From: Ratchanan Srirattanamet Date: Mon, 7 Oct 2024 19:40:31 +0700 Subject: [PATCH] fix: auto-no-squash inference for GitLab (#140) When a GitLab MR is not squashed, `squash_commit_sha` will be `null`, not `undefined`. Update `inferSquash()` to account for this. --- dist/cli/index.js | 4 +- dist/gha/index.js | 4 +- src/service/git/git-util.ts | 6 +- test/service/git/git-util.test.ts | 1 + test/service/runner/cli-gitlab-runner.test.ts | 44 +++++ test/service/runner/gha-gitlab-runner.test.ts | 42 +++++ test/support/mock/git-client-mock-support.ts | 6 +- test/support/mock/gitlab-data.ts | 158 ++++++++++++++++++ 8 files changed, 257 insertions(+), 8 deletions(-) diff --git a/dist/cli/index.js b/dist/cli/index.js index 677bc1f..cf88f1e 100755 --- a/dist/cli/index.js +++ b/dist/cli/index.js @@ -733,7 +733,7 @@ exports.inferGitApiUrl = inferGitApiUrl; /** * Infer the value of the squash option * @param open true if the pull/merge request is still open - * @param squash_commit undefined if the pull/merge request was merged, the sha of the squashed commit if it was squashed + * @param squash_commit undefined or null if the pull/merge request was merged, the sha of the squashed commit if it was squashed * @returns true if a single commit must be cherry-picked, false if all merged commits must be cherry-picked */ const inferSquash = (open, squash_commit) => { @@ -743,7 +743,7 @@ const inferSquash = (open, squash_commit) => { return false; } else { - if (squash_commit !== undefined) { + if (squash_commit) { logger.debug(`cherry-pick the squashed commit ${squash_commit}`); return true; } diff --git a/dist/gha/index.js b/dist/gha/index.js index aaa35f5..5798d22 100755 --- a/dist/gha/index.js +++ b/dist/gha/index.js @@ -698,7 +698,7 @@ exports.inferGitApiUrl = inferGitApiUrl; /** * Infer the value of the squash option * @param open true if the pull/merge request is still open - * @param squash_commit undefined if the pull/merge request was merged, the sha of the squashed commit if it was squashed + * @param squash_commit undefined or null if the pull/merge request was merged, the sha of the squashed commit if it was squashed * @returns true if a single commit must be cherry-picked, false if all merged commits must be cherry-picked */ const inferSquash = (open, squash_commit) => { @@ -708,7 +708,7 @@ const inferSquash = (open, squash_commit) => { return false; } else { - if (squash_commit !== undefined) { + if (squash_commit) { logger.debug(`cherry-pick the squashed commit ${squash_commit}`); return true; } diff --git a/src/service/git/git-util.ts b/src/service/git/git-util.ts index 1a70a1d..4e649b5 100644 --- a/src/service/git/git-util.ts +++ b/src/service/git/git-util.ts @@ -45,17 +45,17 @@ export const inferGitApiUrl = (prUrl: string, apiVersion = "v4"): string => { /** * Infer the value of the squash option * @param open true if the pull/merge request is still open - * @param squash_commit undefined if the pull/merge request was merged, the sha of the squashed commit if it was squashed + * @param squash_commit undefined or null if the pull/merge request was merged, the sha of the squashed commit if it was squashed * @returns true if a single commit must be cherry-picked, false if all merged commits must be cherry-picked */ -export const inferSquash = (open: boolean, squash_commit: string | undefined): boolean => { +export const inferSquash = (open: boolean, squash_commit: string | undefined | null): boolean => { const logger = LoggerServiceFactory.getLogger(); if (open) { logger.debug("cherry-pick all commits because they have not been merged (or squashed) in the base branch yet"); return false; } else { - if (squash_commit !== undefined) { + if (squash_commit) { logger.debug(`cherry-pick the squashed commit ${squash_commit}`); return true; } else { diff --git a/test/service/git/git-util.test.ts b/test/service/git/git-util.test.ts index 0a73d75..9a90936 100644 --- a/test/service/git/git-util.test.ts +++ b/test/service/git/git-util.test.ts @@ -59,5 +59,6 @@ describe("check git utilities", () => { expect(inferSquash(true, undefined)).toStrictEqual(false); expect(inferSquash(false, "SHA")).toStrictEqual(true); expect(inferSquash(false, undefined)).toStrictEqual(false); + expect(inferSquash(false, null)).toStrictEqual(false); }); }); diff --git a/test/service/runner/cli-gitlab-runner.test.ts b/test/service/runner/cli-gitlab-runner.test.ts index ae26d36..e066fe8 100644 --- a/test/service/runner/cli-gitlab-runner.test.ts +++ b/test/service/runner/cli-gitlab-runner.test.ts @@ -604,6 +604,50 @@ describe("cli runner", () => { ); }); + test("merged MR with --auto-no-squash", async () => { + addProcessArgs([ + "-tb", + "target", + "-pr", + "https://my.gitlab.host.com/superuser/backporting-example/-/merge_requests/5", + "--auto-no-squash", + ]); + + await runner.execute(); + + const cwd = process.cwd() + "/bp"; + + expect(GitClientFactory.getOrCreate).toBeCalledTimes(1); + expect(GitClientFactory.getOrCreate).toBeCalledWith(GitClientType.GITLAB, undefined, "https://my.gitlab.host.com/api/v4"); + + expect(GitCLIService.prototype.clone).toBeCalledTimes(1); + expect(GitCLIService.prototype.clone).toBeCalledWith("https://my.gitlab.host.com/superuser/backporting-example.git", cwd, "target"); + + expect(GitCLIService.prototype.createLocalBranch).toBeCalledTimes(1); + expect(GitCLIService.prototype.createLocalBranch).toBeCalledWith(cwd, "bp-target-e4dd336"); + + expect(GitCLIService.prototype.cherryPick).toBeCalledTimes(1); + expect(GitCLIService.prototype.cherryPick).toBeCalledWith(cwd, "e4dd336a4a20f394df6665994df382fb1d193a11", undefined, undefined, undefined); + + expect(GitCLIService.prototype.push).toBeCalledTimes(1); + expect(GitCLIService.prototype.push).toBeCalledWith(cwd, "bp-target-e4dd336"); + + expect(GitLabClient.prototype.createPullRequest).toBeCalledTimes(1); + expect(GitLabClient.prototype.createPullRequest).toBeCalledWith({ + owner: "superuser", + repo: "backporting-example", + head: "bp-target-e4dd336", + base: "target", + title: "[target] Update test.txt", + body: expect.stringContaining("**Backport:** https://my.gitlab.host.com/superuser/backporting-example/-/merge_requests/5"), + reviewers: ["superuser"], + assignees: [], + labels: [], + comments: [], + } + ); + }); + test("auth using GITLAB_TOKEN takes precedence over GIT_TOKEN env variable", async () => { process.env[AuthTokenId.GIT_TOKEN] = "mygittoken"; process.env[AuthTokenId.GITLAB_TOKEN] = "mygitlabtoken"; diff --git a/test/service/runner/gha-gitlab-runner.test.ts b/test/service/runner/gha-gitlab-runner.test.ts index 472df31..f0f41f6 100644 --- a/test/service/runner/gha-gitlab-runner.test.ts +++ b/test/service/runner/gha-gitlab-runner.test.ts @@ -519,4 +519,46 @@ describe("gha runner", () => { } ); }); + + test("merged MR with auto-no-squash", async () => { + spyGetInput({ + "target-branch": "target", + "pull-request": "https://my.gitlab.host.com/superuser/backporting-example/-/merge_requests/5", + "auto-no-squash": "true", + }); + + await runner.execute(); + + const cwd = process.cwd() + "/bp"; + + expect(GitClientFactory.getOrCreate).toBeCalledTimes(1); + expect(GitClientFactory.getOrCreate).toBeCalledWith(GitClientType.GITLAB, undefined, "https://my.gitlab.host.com/api/v4"); + + expect(GitCLIService.prototype.clone).toBeCalledTimes(1); + expect(GitCLIService.prototype.clone).toBeCalledWith("https://my.gitlab.host.com/superuser/backporting-example.git", cwd, "target"); + + expect(GitCLIService.prototype.createLocalBranch).toBeCalledTimes(1); + expect(GitCLIService.prototype.createLocalBranch).toBeCalledWith(cwd, "bp-target-e4dd336"); + + expect(GitCLIService.prototype.cherryPick).toBeCalledTimes(1); + expect(GitCLIService.prototype.cherryPick).toBeCalledWith(cwd, "e4dd336a4a20f394df6665994df382fb1d193a11", undefined, undefined, undefined); + + expect(GitCLIService.prototype.push).toBeCalledTimes(1); + expect(GitCLIService.prototype.push).toBeCalledWith(cwd, "bp-target-e4dd336"); + + expect(GitLabClient.prototype.createPullRequest).toBeCalledTimes(1); + expect(GitLabClient.prototype.createPullRequest).toBeCalledWith({ + owner: "superuser", + repo: "backporting-example", + head: "bp-target-e4dd336", + base: "target", + title: "[target] Update test.txt", + body: expect.stringContaining("**Backport:** https://my.gitlab.host.com/superuser/backporting-example/-/merge_requests/5"), + reviewers: ["superuser"], + assignees: [], + labels: [], + comments: [], + } + ); + }); }); \ No newline at end of file diff --git a/test/support/mock/git-client-mock-support.ts b/test/support/mock/git-client-mock-support.ts index 6c81f91..1a38493 100644 --- a/test/support/mock/git-client-mock-support.ts +++ b/test/support/mock/git-client-mock-support.ts @@ -1,7 +1,7 @@ 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, 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"; +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, MERGED_NOT_SQUASHED_MR, MERGED_NOT_SQUASHED_MR_COMMITS } from "./gitlab-data"; import { CB_TARGET_OWNER, CB_REPO, CB_MERGED_PR_FIXTURE, CB_OPEN_PR_FIXTURE, CB_NOT_MERGED_PR_FIXTURE, CB_NOT_FOUND_PR_NUMBER, CB_MULT_COMMITS_PR_FIXTURE, CB_MULT_COMMITS_PR_COMMITS, CB_NEW_PR_URL, CB_NEW_PR_NUMBER, CODEBERG_GET_COMMIT } from "./codeberg-data"; // high number, for each test we are not expecting @@ -25,6 +25,8 @@ export const getAxiosMocked = (url: string) => { data = CLOSED_NOT_MERGED_MR; } else if (url.endsWith("merge_requests/4")) { data = NESTED_NAMESPACE_MR; + } else if (url.endsWith("merge_requests/5")) { + data = MERGED_NOT_SQUASHED_MR; } else if (url.endsWith("projects/76316")) { data = PROJECT_EXAMPLE; } else if (url.endsWith("projects/1645")) { @@ -35,6 +37,8 @@ export const getAxiosMocked = (url: string) => { data = MERGED_SQUASHED_MR_COMMITS; } else if (url.endsWith("merge_requests/2/commits")) { data = OPEN_PR_COMMITS; + } else if (url.endsWith("merge_requests/5/commits")) { + data = MERGED_NOT_SQUASHED_MR_COMMITS; } return { diff --git a/test/support/mock/gitlab-data.ts b/test/support/mock/gitlab-data.ts index 5202925..7adcf52 100644 --- a/test/support/mock/gitlab-data.ts +++ b/test/support/mock/gitlab-data.ts @@ -755,6 +755,29 @@ export const OPEN_PR_COMMITS = [ } ]; +export const MERGED_NOT_SQUASHED_MR_COMMITS = [ + { + "id":"e4dd336a4a20f394df6665994df382fb1d193a11", + "short_id":"e4dd336a", + "created_at":"2023-06-29T09:59:10.000Z", + "parent_ids":[ + + ], + "title":"Add new file", + "message":"Add new file", + "author_name":"Super User", + "author_email":"superuser@email.com", + "authored_date":"2023-06-29T09:59:10.000Z", + "committer_name":"Super User", + "committer_email":"superuser@email.com", + "committed_date":"2023-06-29T09:59:10.000Z", + "trailers":{ + + }, + "web_url":"https://gitlab.com/superuser/backporting-example/-/commit/e4dd336a4a20f394df6665994df382fb1d193a11" + }, +]; + export const SUPERUSER = { "id":14041, "username":"superuser", @@ -898,3 +921,138 @@ export const NESTED_NAMESPACE_MR = { "can_merge":true } }; + +export const MERGED_NOT_SQUASHED_MR = { + "id":807106, + "iid":1, + "project_id":76316, + "title":"Update test.txt", + "description":"This is the body", + "state":"merged", + "created_at":"2023-06-28T14:32:40.943Z", + "updated_at":"2023-06-28T14:37:12.108Z", + "merged_by":{ + "id":14041, + "username":"superuser", + "name":"Super User", + "state":"active", + "avatar_url":"https://my.gitlab.host.com/uploads/-/system/user/avatar/14041/avatar.png", + "web_url":"https://my.gitlab.host.com/superuser" + }, + "merge_user":{ + "id":14041, + "username":"superuser", + "name":"Super User", + "state":"active", + "avatar_url":"https://my.gitlab.host.com/uploads/-/system/user/avatar/14041/avatar.png", + "web_url":"https://my.gitlab.host.com/superuser" + }, + "merged_at":"2023-06-28T14:37:11.667Z", + "closed_by":null, + "closed_at":null, + "target_branch":"main", + "source_branch":"feature", + "user_notes_count":0, + "upvotes":0, + "downvotes":0, + "author":{ + "id":14041, + "username":"superuser", + "name":"Super User", + "state":"active", + "avatar_url":"https://my.gitlab.host.com/uploads/-/system/user/avatar/14041/avatar.png", + "web_url":"https://my.gitlab.host.com/superuser" + }, + "assignees":[ + { + "id":14041, + "username":"superuser", + "name":"Super User", + "state":"active", + "avatar_url":"https://my.gitlab.host.com/uploads/-/system/user/avatar/14041/avatar.png", + "web_url":"https://my.gitlab.host.com/superuser" + } + ], + "assignee":{ + "id":14041, + "username":"superuser", + "name":"Super User", + "state":"active", + "avatar_url":"https://my.gitlab.host.com/uploads/-/system/user/avatar/14041/avatar.png", + "web_url":"https://my.gitlab.host.com/superuser" + }, + "reviewers":[ + { + "id":1404188, + "username":"superuser1", + "name":"Super User", + "state":"active", + "avatar_url":"https://my.gitlab.host.com/uploads/-/system/user/avatar/14041/avatar.png", + "web_url":"https://my.gitlab.host.com/superuser" + }, + { + "id":1404199, + "username":"superuser2", + "name":"Super User", + "state":"active", + "avatar_url":"https://my.gitlab.host.com/uploads/-/system/user/avatar/14041/avatar.png", + "web_url":"https://my.gitlab.host.com/superuser" + } + ], + "source_project_id":76316, + "target_project_id":76316, + "labels":[ + "backport-prod" + ], + "draft":false, + "work_in_progress":false, + "milestone":null, + "merge_when_pipeline_succeeds":false, + "merge_status":"can_be_merged", + "detailed_merge_status":"not_open", + "sha":"9e15674ebd48e05c6e428a1fa31dbb60a778d644", + "merge_commit_sha":"4d369c3e9a8d1d5b7e56c892a8ab2a7666583ac3", + "squash_commit_sha":null, + "discussion_locked":null, + "should_remove_source_branch":true, + "force_remove_source_branch":true, + "reference":"!5", + "references":{ + "short":"!5", + "relative":"!5", + "full":"superuser/backporting-example!5" + }, + "web_url":"https://my.gitlab.host.com/superuser/backporting-example/-/merge_requests/5", + "time_stats":{ + "time_estimate":0, + "total_time_spent":0, + "human_time_estimate":null, + "human_total_time_spent":null + }, + "squash":false, + "squash_on_merge":false, + "task_completion_status":{ + "count":0, + "completed_count":0 + }, + "has_conflicts":false, + "blocking_discussions_resolved":true, + "approvals_before_merge":null, + "subscribed":true, + "changes_count":"1", + "latest_build_started_at":null, + "latest_build_finished_at":null, + "first_deployed_to_production_at":null, + "pipeline":null, + "head_pipeline":null, + "diff_refs":{ + "base_sha":"2c553a0c4c133a51806badce5fa4842b7253cb3b", + "head_sha":"9e15674ebd48e05c6e428a1fa31dbb60a778d644", + "start_sha":"2c553a0c4c133a51806badce5fa4842b7253cb3b" + }, + "merge_error":null, + "first_contribution":false, + "user":{ + "can_merge":true + } +}; \ No newline at end of file