From 0d369dd6126b42f1b92ce40e5471ae1d5d31f96b Mon Sep 17 00:00:00 2001 From: Andrea Lamparelli Date: Thu, 22 Jun 2023 17:35:24 +0200 Subject: [PATCH] feat(issue-32): override reviewers and assignees --- README.md | 3 + action.yml | 10 + dist/cli/index.js | 44 +++- dist/gha/index.js | 49 ++++- src/service/args/args.types.ts | 3 + src/service/args/cli/cli-args-parser.ts | 13 +- src/service/args/gha/gha-args-parser.ts | 20 +- .../configs/pullrequest/pr-configs-parser.ts | 12 +- src/service/git/git.types.ts | 2 + src/service/git/github/github-mapper.ts | 1 + src/service/git/github/github-service.ts | 15 +- src/service/runner/runner.ts | 3 +- test/service/args/cli/cli-args-parser.test.ts | 19 +- test/service/args/gha/gha-args-parser.test.ts | 26 ++- .../pullrequest/pr-configs-parser.test.ts | 188 +++++++++++++++++- test/service/runner/cli-runner.test.ts | 68 ++++++- test/service/runner/gha-runner.test.ts | 57 +++++- test/support/utils.ts | 10 + 18 files changed, 499 insertions(+), 44 deletions(-) diff --git a/README.md b/README.md index c1ec758..539d70c 100644 --- a/README.md +++ b/README.md @@ -62,6 +62,9 @@ This toold comes with some inputs that allow users to override the default behav | Title | --title | N | Backporting pull request title | "{original-pr-title}" | | Body | --body | N | Backporting pull request body | "{original-pr-body}" | | Body Prefix | --body-prefix | N | Prefix to the backporting pull request body | "Backport: {original-pr-link}" | +| Reviewers | --reviewers | N | Backporting pull request comma-separated reviewers list | [] | +| Assignees | --assignes | N | Backporting pull request comma-separated assignees list | [] | +| No Reviewers Inheritance | --no-inherit-reviewers | N | Considered only if reviewers is empty, if true keep reviewers as empty list, otherwise inherit from original pull request | false | | Backport Branch Name | --bp-branch-name | N | Name of the backporting pull request branch | bp-{target-branch}-{sha} | | Dry Run | -d, --dry-run | N | If enabled the tool does not push nor create anything remotely, use this to skip PR creation | false | diff --git a/action.yml b/action.yml index dff598c..aa6327e 100644 --- a/action.yml +++ b/action.yml @@ -35,6 +35,16 @@ inputs: bp-branch-name: description: "Backporting PR branch name. Default is auto-generated from commit." required: false + reviewers: + description: "Comma separated list of reviewers for the backporting pull request." + required: false + assignees: + description: "Comma separated list of reviewers for the backporting pull request." + required: false + inherith-reviewers: + description: "If enabled and reviewers option is empty then inherit them from original pull request." + required: false + default: "true" runs: using: node16 diff --git a/dist/cli/index.js b/dist/cli/index.js index e97b0f5..a0bfc53 100755 --- a/dist/cli/index.js +++ b/dist/cli/index.js @@ -30,6 +30,11 @@ runner.run(); Object.defineProperty(exports, "__esModule", ({ value: true })); const commander_1 = __nccwpck_require__(4379); const package_json_1 = __nccwpck_require__(6625); +function commaSeparatedList(value, _prev) { + // remove all whitespaces + const cleanedValue = value.trim(); + return cleanedValue !== "" ? cleanedValue.replace(/\s/g, "").split(",") : []; +} class CLIArgsParser { getCommand() { return new commander_1.Command(package_json_1.name) @@ -45,7 +50,10 @@ class CLIArgsParser { .option("--title ", "backport pr title, default original pr title prefixed by target branch.", undefined) .option("--body ", "backport pr title, default original pr body prefixed by bodyPrefix.", undefined) .option("--body-prefix ", "backport pr body prefix, default `backport `.", undefined) - .option("--bp-branch-name ", "backport pr branch name, default auto-generated by the commit.", undefined); + .option("--bp-branch-name ", "backport pr branch name, default auto-generated by the commit.", undefined) + .option("--reviewers ", "comma separated list of reviewers for the backporting pull request.", commaSeparatedList, []) + .option("--assignees ", "comma separated list of assignees for the backporting pull request.", commaSeparatedList, []) + .option("--no-inherit-reviewers", "if provided and reviewers option is empty then inherit them from original pull request", true); } parse() { const opts = this.getCommand() @@ -63,6 +71,9 @@ class CLIArgsParser { body: opts.body, bodyPrefix: opts.bodyPrefix, bpBranchName: opts.bpBranchName, + reviewers: opts.reviewers, + assignees: opts.assignees, + inheritReviewers: opts.inheritReviewers, }; } } @@ -151,10 +162,13 @@ class PullRequestConfigsParser extends configs_parser_1.default { * @returns {GitPullRequest} */ getDefaultBackportPullRequest(originalPullRequest, args) { - const reviewers = []; - reviewers.push(originalPullRequest.author); - if (originalPullRequest.mergedBy) { - reviewers.push(originalPullRequest.mergedBy); + const reviewers = args.reviewers ?? []; + if (reviewers.length == 0 && args.inheritReviewers) { + // inherit only if args.reviewers is empty and args.inheritReviewers set to true + reviewers.push(originalPullRequest.author); + if (originalPullRequest.mergedBy) { + reviewers.push(originalPullRequest.mergedBy); + } } const bodyPrefix = args.bodyPrefix ?? `**Backport:** ${originalPullRequest.htmlUrl}\r\n\r\n`; const body = args.body ?? `${originalPullRequest.body}\r\n\r\nPowered by [BPer](https://github.com/lampajr/backporting).`; @@ -163,6 +177,7 @@ class PullRequestConfigsParser extends configs_parser_1.default { title: args.title ?? `[${args.targetBranch}] ${originalPullRequest.title}`, body: `${bodyPrefix}${body}`, reviewers: [...new Set(reviewers)], + assignees: [...new Set(args.assignees)], targetRepo: originalPullRequest.targetRepo, sourceRepo: originalPullRequest.targetRepo, branchName: args.bpBranchName, @@ -382,6 +397,7 @@ class GitHubMapper { merged: pr.merged ?? false, mergedBy: pr.merged_by?.login, reviewers: pr.requested_reviewers.filter(r => "login" in r).map((r => r?.login)), + assignees: pr.assignees.filter(r => "login" in r).map(r => r.login), sourceRepo: { owner: pr.head.repo.full_name.split("/")[0], project: pr.head.repo.full_name.split("/")[1], @@ -453,13 +469,26 @@ class GitHubService { owner: backport.owner, repo: backport.repo, pull_number: data.number, - reviewers: backport.reviewers + reviewers: backport.reviewers, }); } catch (error) { this.logger.error(`Error requesting reviewers: ${error}`); } } + if (backport.assignees.length > 0) { + try { + await this.octokit.issues.addAssignees({ + owner: backport.owner, + repo: backport.repo, + issue_number: data.number, + assignees: backport.assignees, + }); + } + catch (error) { + this.logger.error(`Error setting assignees: ${error}`); + } + } } // UTILS /** @@ -686,7 +715,8 @@ class Runner { base: configs.targetBranch, title: backportPR.title, body: backportPR.body, - reviewers: backportPR.reviewers + reviewers: backportPR.reviewers, + assignees: backportPR.assignees, }; if (!configs.dryRun) { // 8. push the new branch to origin diff --git a/dist/gha/index.js b/dist/gha/index.js index b7c7682..e200d08 100755 --- a/dist/gha/index.js +++ b/dist/gha/index.js @@ -41,12 +41,21 @@ class GHAArgsParser { } getOrDefault(key, defaultValue) { const value = (0, core_1.getInput)(key); - return (value !== undefined && value !== "") ? value : defaultValue; + return value !== "" ? value : defaultValue; + } + getAsCommaSeparatedList(key) { + // trim the value + const value = ((0, core_1.getInput)(key) ?? "").trim(); + return value !== "" ? value.replace(/\s/g, "").split(",") : []; + } + getAsBooleanOrDefault(key, defaultValue) { + const value = (0, core_1.getInput)(key).trim(); + return value !== "" ? value.toLowerCase() === "true" : defaultValue; } parse() { return { - dryRun: (0, core_1.getInput)("dry-run") === "true", - auth: (0, core_1.getInput)("auth") ? (0, core_1.getInput)("auth") : "", + dryRun: this.getAsBooleanOrDefault("dry-run", false), + auth: (0, core_1.getInput)("auth"), pullRequest: (0, core_1.getInput)("pull-request"), targetBranch: (0, core_1.getInput)("target-branch"), folder: this.getOrUndefined("folder"), @@ -56,6 +65,9 @@ class GHAArgsParser { body: this.getOrUndefined("body"), bodyPrefix: this.getOrUndefined("body-prefix"), bpBranchName: this.getOrUndefined("bp-branch-name"), + reviewers: this.getAsCommaSeparatedList("reviewers"), + assignees: this.getAsCommaSeparatedList("assignees"), + inheritReviewers: !this.getAsBooleanOrDefault("no-inherit-reviewers", false), }; } } @@ -144,10 +156,13 @@ class PullRequestConfigsParser extends configs_parser_1.default { * @returns {GitPullRequest} */ getDefaultBackportPullRequest(originalPullRequest, args) { - const reviewers = []; - reviewers.push(originalPullRequest.author); - if (originalPullRequest.mergedBy) { - reviewers.push(originalPullRequest.mergedBy); + const reviewers = args.reviewers ?? []; + if (reviewers.length == 0 && args.inheritReviewers) { + // inherit only if args.reviewers is empty and args.inheritReviewers set to true + reviewers.push(originalPullRequest.author); + if (originalPullRequest.mergedBy) { + reviewers.push(originalPullRequest.mergedBy); + } } const bodyPrefix = args.bodyPrefix ?? `**Backport:** ${originalPullRequest.htmlUrl}\r\n\r\n`; const body = args.body ?? `${originalPullRequest.body}\r\n\r\nPowered by [BPer](https://github.com/lampajr/backporting).`; @@ -156,6 +171,7 @@ class PullRequestConfigsParser extends configs_parser_1.default { title: args.title ?? `[${args.targetBranch}] ${originalPullRequest.title}`, body: `${bodyPrefix}${body}`, reviewers: [...new Set(reviewers)], + assignees: [...new Set(args.assignees)], targetRepo: originalPullRequest.targetRepo, sourceRepo: originalPullRequest.targetRepo, branchName: args.bpBranchName, @@ -375,6 +391,7 @@ class GitHubMapper { merged: pr.merged ?? false, mergedBy: pr.merged_by?.login, reviewers: pr.requested_reviewers.filter(r => "login" in r).map((r => r?.login)), + assignees: pr.assignees.filter(r => "login" in r).map(r => r.login), sourceRepo: { owner: pr.head.repo.full_name.split("/")[0], project: pr.head.repo.full_name.split("/")[1], @@ -446,13 +463,26 @@ class GitHubService { owner: backport.owner, repo: backport.repo, pull_number: data.number, - reviewers: backport.reviewers + reviewers: backport.reviewers, }); } catch (error) { this.logger.error(`Error requesting reviewers: ${error}`); } } + if (backport.assignees.length > 0) { + try { + await this.octokit.issues.addAssignees({ + owner: backport.owner, + repo: backport.repo, + issue_number: data.number, + assignees: backport.assignees, + }); + } + catch (error) { + this.logger.error(`Error setting assignees: ${error}`); + } + } } // UTILS /** @@ -679,7 +709,8 @@ class Runner { base: configs.targetBranch, title: backportPR.title, body: backportPR.body, - reviewers: backportPR.reviewers + reviewers: backportPR.reviewers, + assignees: backportPR.assignees, }; if (!configs.dryRun) { // 8. push the new branch to origin diff --git a/src/service/args/args.types.ts b/src/service/args/args.types.ts index d82aa2a..9751ee6 100644 --- a/src/service/args/args.types.ts +++ b/src/service/args/args.types.ts @@ -13,4 +13,7 @@ export interface Args { body?: string, // backport pr title, default original pr body prefixed by bodyPrefix bodyPrefix?: string, // backport pr body prefix, default `backport ` bpBranchName?: string, // backport pr branch name, default computed from commit + reviewers?: string[], // backport pr reviewers + assignees?: string[], // backport pr assignees + inheritReviewers: boolean, // if true and reviewers == [] then inherit reviewers from original pr } \ No newline at end of file diff --git a/src/service/args/cli/cli-args-parser.ts b/src/service/args/cli/cli-args-parser.ts index 4a01a0b..cb20d9a 100644 --- a/src/service/args/cli/cli-args-parser.ts +++ b/src/service/args/cli/cli-args-parser.ts @@ -3,6 +3,11 @@ import { Args } from "@bp/service/args/args.types"; import { Command } from "commander"; import { name, version, description } from "@bp/../package.json"; +function commaSeparatedList(value: string, _prev: unknown): string[] { + // remove all whitespaces + const cleanedValue: string = value.trim(); + return cleanedValue !== "" ? cleanedValue.replace(/\s/g, "").split(",") : []; +} export default class CLIArgsParser implements ArgsParser { @@ -20,7 +25,10 @@ export default class CLIArgsParser implements ArgsParser { .option("--title ", "backport pr title, default original pr title prefixed by target branch.", undefined) .option("--body ", "backport pr title, default original pr body prefixed by bodyPrefix.", undefined) .option("--body-prefix ", "backport pr body prefix, default `backport `.", undefined) - .option("--bp-branch-name ", "backport pr branch name, default auto-generated by the commit.", undefined); + .option("--bp-branch-name ", "backport pr branch name, default auto-generated by the commit.", undefined) + .option("--reviewers ", "comma separated list of reviewers for the backporting pull request.", commaSeparatedList, []) + .option("--assignees ", "comma separated list of assignees for the backporting pull request.", commaSeparatedList, []) + .option("--no-inherit-reviewers", "if provided and reviewers option is empty then inherit them from original pull request", true); } parse(): Args { @@ -40,6 +48,9 @@ export default class CLIArgsParser implements ArgsParser { body: opts.body, bodyPrefix: opts.bodyPrefix, bpBranchName: opts.bpBranchName, + reviewers: opts.reviewers, + assignees: opts.assignees, + inheritReviewers: opts.inheritReviewers, }; } diff --git a/src/service/args/gha/gha-args-parser.ts b/src/service/args/gha/gha-args-parser.ts index e82d6e5..0ab2ee9 100644 --- a/src/service/args/gha/gha-args-parser.ts +++ b/src/service/args/gha/gha-args-parser.ts @@ -16,13 +16,24 @@ export default class GHAArgsParser implements ArgsParser { public getOrDefault(key: string, defaultValue: string): string { const value = getInput(key); - return (value !== undefined && value !== "") ? value : defaultValue; + return value !== "" ? value : defaultValue; + } + + public getAsCommaSeparatedList(key: string): string[] { + // trim the value + const value: string = (getInput(key) ?? "").trim(); + return value !== "" ? value.replace(/\s/g, "").split(",") : []; + } + + public getAsBooleanOrDefault(key: string, defaultValue: boolean): boolean { + const value = getInput(key).trim(); + return value !== "" ? value.toLowerCase() === "true" : defaultValue; } parse(): Args { return { - dryRun: getInput("dry-run") === "true", - auth: getInput("auth") ? getInput("auth") : "", + dryRun: this.getAsBooleanOrDefault("dry-run", false), + auth: getInput("auth"), pullRequest: getInput("pull-request"), targetBranch: getInput("target-branch"), folder: this.getOrUndefined("folder"), @@ -32,6 +43,9 @@ export default class GHAArgsParser implements ArgsParser { body: this.getOrUndefined("body"), bodyPrefix: this.getOrUndefined("body-prefix"), bpBranchName: this.getOrUndefined("bp-branch-name"), + reviewers: this.getAsCommaSeparatedList("reviewers"), + assignees: this.getAsCommaSeparatedList("assignees"), + inheritReviewers: !this.getAsBooleanOrDefault("no-inherit-reviewers", false), }; } diff --git a/src/service/configs/pullrequest/pr-configs-parser.ts b/src/service/configs/pullrequest/pr-configs-parser.ts index 745caf9..4cba755 100644 --- a/src/service/configs/pullrequest/pr-configs-parser.ts +++ b/src/service/configs/pullrequest/pr-configs-parser.ts @@ -44,10 +44,13 @@ export default class PullRequestConfigsParser extends ConfigsParser { * @returns {GitPullRequest} */ private getDefaultBackportPullRequest(originalPullRequest: GitPullRequest, args: Args): GitPullRequest { - const reviewers = []; - reviewers.push(originalPullRequest.author); - if (originalPullRequest.mergedBy) { - reviewers.push(originalPullRequest.mergedBy); + const reviewers = args.reviewers ?? []; + if (reviewers.length == 0 && args.inheritReviewers) { + // inherit only if args.reviewers is empty and args.inheritReviewers set to true + reviewers.push(originalPullRequest.author); + if (originalPullRequest.mergedBy) { + reviewers.push(originalPullRequest.mergedBy); + } } const bodyPrefix = args.bodyPrefix ?? `**Backport:** ${originalPullRequest.htmlUrl}\r\n\r\n`; @@ -58,6 +61,7 @@ export default class PullRequestConfigsParser extends ConfigsParser { title: args.title ?? `[${args.targetBranch}] ${originalPullRequest.title}`, body: `${bodyPrefix}${body}`, reviewers: [...new Set(reviewers)], + assignees: [...new Set(args.assignees)], targetRepo: originalPullRequest.targetRepo, sourceRepo: originalPullRequest.targetRepo, branchName: args.bpBranchName, diff --git a/src/service/git/git.types.ts b/src/service/git/git.types.ts index ba90f6d..05db4a0 100644 --- a/src/service/git/git.types.ts +++ b/src/service/git/git.types.ts @@ -9,6 +9,7 @@ export interface GitPullRequest { title: string, body: string, reviewers: string[], + assignees: string[], targetRepo: GitRepository, sourceRepo: GitRepository, nCommits: number, // number of commits in the pr @@ -30,6 +31,7 @@ export interface BackportPullRequest { title: string, // pr title body: string, // pr body reviewers: string[], // pr list of reviewers + assignees: string[], // pr list of assignees branchName?: string, } diff --git a/src/service/git/github/github-mapper.ts b/src/service/git/github/github-mapper.ts index 8b519aa..e21d95c 100644 --- a/src/service/git/github/github-mapper.ts +++ b/src/service/git/github/github-mapper.ts @@ -15,6 +15,7 @@ export default class GitHubMapper { merged: pr.merged ?? false, mergedBy: pr.merged_by?.login, reviewers: pr.requested_reviewers.filter(r => "login" in r).map((r => (r as User)?.login)), + assignees: pr.assignees.filter(r => "login" in r).map(r => r.login), sourceRepo: { owner: pr.head.repo.full_name.split("/")[0], project: pr.head.repo.full_name.split("/")[1], diff --git a/src/service/git/github/github-service.ts b/src/service/git/github/github-service.ts index 3b016be..073fd1b 100644 --- a/src/service/git/github/github-service.ts +++ b/src/service/git/github/github-service.ts @@ -58,12 +58,25 @@ export default class GitHubService implements GitService { owner: backport.owner, repo: backport.repo, pull_number: (data as PullRequest).number, - reviewers: backport.reviewers + reviewers: backport.reviewers, }); } catch (error) { this.logger.error(`Error requesting reviewers: ${error}`); } } + + if (backport.assignees.length > 0) { + try { + await this.octokit.issues.addAssignees({ + owner: backport.owner, + repo: backport.repo, + issue_number: (data as PullRequest).number, + assignees: backport.assignees, + }); + } catch (error) { + this.logger.error(`Error setting assignees: ${error}`); + } + } } // UTILS diff --git a/src/service/runner/runner.ts b/src/service/runner/runner.ts index ff90e8e..c604093 100644 --- a/src/service/runner/runner.ts +++ b/src/service/runner/runner.ts @@ -107,7 +107,8 @@ export default class Runner { base: configs.targetBranch, title: backportPR.title, body: backportPR.body, - reviewers: backportPR.reviewers + reviewers: backportPR.reviewers, + assignees: backportPR.assignees, }; if (!configs.dryRun) { diff --git a/test/service/args/cli/cli-args-parser.test.ts b/test/service/args/cli/cli-args-parser.test.ts index fa8810a..ba2f59c 100644 --- a/test/service/args/cli/cli-args-parser.test.ts +++ b/test/service/args/cli/cli-args-parser.test.ts @@ -1,6 +1,6 @@ import { Args } from "@bp/service/args/args.types"; import CLIArgsParser from "@bp/service/args/cli/cli-args-parser"; -import { addProcessArgs, resetProcessArgs } from "../../../support/utils"; +import { addProcessArgs, resetProcessArgs, expectArrayEqual } from "../../../support/utils"; describe("cli args parser", () => { let parser: CLIArgsParser; @@ -33,6 +33,9 @@ describe("cli args parser", () => { expect(args.body).toEqual(undefined); expect(args.bodyPrefix).toEqual(undefined); expect(args.bpBranchName).toEqual(undefined); + expect(args.reviewers).toEqual([]); + expect(args.assignees).toEqual([]); + expect(args.inheritReviewers).toEqual(true); }); test("valid execution [default, long]", () => { @@ -55,6 +58,9 @@ describe("cli args parser", () => { expect(args.body).toEqual(undefined); expect(args.bodyPrefix).toEqual(undefined); expect(args.bpBranchName).toEqual(undefined); + expect(args.reviewers).toEqual([]); + expect(args.assignees).toEqual([]); + expect(args.inheritReviewers).toEqual(true); }); test("valid execution [override, short]", () => { @@ -84,6 +90,9 @@ describe("cli args parser", () => { expect(args.body).toEqual(undefined); expect(args.bodyPrefix).toEqual(undefined); expect(args.bpBranchName).toEqual(undefined); + expect(args.reviewers).toEqual([]); + expect(args.assignees).toEqual([]); + expect(args.inheritReviewers).toEqual(true); }); test("valid execution [override, long]", () => { @@ -107,6 +116,11 @@ describe("cli args parser", () => { "New Body Prefix", "--bp-branch-name", "bp_branch_name", + "--reviewers", + "al , john, jack", + "--assignees", + " pippo,pluto, paperino", + "--no-inherit-reviewers", ]); const args: Args = parser.parse(); @@ -121,6 +135,9 @@ describe("cli args parser", () => { expect(args.body).toEqual("New Body"); expect(args.bodyPrefix).toEqual("New Body Prefix"); expect(args.bpBranchName).toEqual("bp_branch_name"); + expectArrayEqual(["al", "john", "jack"], args.reviewers!); + expectArrayEqual(["pippo", "pluto", "paperino"], args.assignees!); + expect(args.inheritReviewers).toEqual(false); }); }); \ No newline at end of file diff --git a/test/service/args/gha/gha-args-parser.test.ts b/test/service/args/gha/gha-args-parser.test.ts index 35fca19..701ac03 100644 --- a/test/service/args/gha/gha-args-parser.test.ts +++ b/test/service/args/gha/gha-args-parser.test.ts @@ -1,6 +1,6 @@ import { Args } from "@bp/service/args/args.types"; import GHAArgsParser from "@bp/service/args/gha/gha-args-parser"; -import { spyGetInput } from "../../../support/utils"; +import { spyGetInput, expectArrayEqual } from "../../../support/utils"; describe("gha args parser", () => { let parser: GHAArgsParser; @@ -25,12 +25,23 @@ describe("gha args parser", () => { test("getOrUndefined", () => { spyGetInput({ "present": "value", - "empty": "" + "empty": "", }); expect(parser.getOrUndefined("empty")).toStrictEqual(undefined); expect(parser.getOrUndefined("present")).toStrictEqual("value"); }); + test("getAsCommaSeparatedList", () => { + spyGetInput({ + "present": "value1, value2 , value3", + "empty": "", + "blank": " ", + }); + expectArrayEqual(parser.getAsCommaSeparatedList("present")!, ["value1", "value2", "value3"]); + expect(parser.getAsCommaSeparatedList("empty")).toStrictEqual([]); + expect(parser.getAsCommaSeparatedList("blank")).toStrictEqual([]); + }); + test("valid execution [default]", () => { spyGetInput({ "target-branch": "target", @@ -47,8 +58,9 @@ describe("gha args parser", () => { expect(args.pullRequest).toEqual("https://localhost/whatever/pulls/1"); expect(args.title).toEqual(undefined); expect(args.body).toEqual(undefined); - expect(args.bodyPrefix).toEqual(undefined); - expect(args.bpBranchName).toEqual(undefined); + expect(args.reviewers).toEqual([]); + expect(args.assignees).toEqual([]); + expect(args.inheritReviewers).toEqual(true); }); test("valid execution [override]", () => { @@ -63,6 +75,9 @@ describe("gha args parser", () => { "body": "New Body", "body-prefix": "New Body Prefix", "bp-branch-name": "bp_branch_name", + "reviewers": "al , john, jack", + "assignees": " pippo,pluto, paperino", + "no-inherit-reviewers": "true", }); const args: Args = parser.parse(); @@ -77,6 +92,9 @@ describe("gha args parser", () => { expect(args.body).toEqual("New Body"); expect(args.bodyPrefix).toEqual("New Body Prefix"); expect(args.bpBranchName).toEqual("bp_branch_name"); + expectArrayEqual(["al", "john", "jack"], args.reviewers!); + expectArrayEqual(["pippo", "pluto", "paperino"], args.assignees!); + expect(args.inheritReviewers).toEqual(false); }); }); \ No newline at end of file diff --git a/test/service/configs/pullrequest/pr-configs-parser.test.ts b/test/service/configs/pullrequest/pr-configs-parser.test.ts index afdac3d..52dab30 100644 --- a/test/service/configs/pullrequest/pr-configs-parser.test.ts +++ b/test/service/configs/pullrequest/pr-configs-parser.test.ts @@ -35,7 +35,10 @@ describe("pull request config parser", () => { pullRequest: mergedPRUrl, targetBranch: "prod", gitUser: "GitHub", - gitEmail: "noreply@github.com" + gitEmail: "noreply@github.com", + reviewers: [], + assignees: [], + inheritReviewers: true, }; const configs: Configs = await parser.parseAndValidate(args); @@ -59,6 +62,7 @@ describe("pull request config parser", () => { title: "PR Title", body: "Please review and merge", reviewers: ["requested-gh-user", "gh-user"], + assignees: [], targetRepo: { owner: "owner", project: "reponame", @@ -79,6 +83,7 @@ describe("pull request config parser", () => { title: "[prod] PR Title", body: "**Backport:** https://github.com/owner/reponame/pull/2368\r\n\r\nPlease review and merge\r\n\r\nPowered by [BPer](https://github.com/lampajr/backporting).", reviewers: ["gh-user", "that-s-a-user"], + assignees: [], targetRepo: { owner: "owner", project: "reponame", @@ -103,7 +108,10 @@ describe("pull request config parser", () => { targetBranch: "prod", folder: "/tmp/test", gitUser: "GitHub", - gitEmail: "noreply@github.com" + gitEmail: "noreply@github.com", + reviewers: [], + assignees: [], + inheritReviewers: true, }; const configs: Configs = await parser.parseAndValidate(args); @@ -125,7 +133,10 @@ describe("pull request config parser", () => { pullRequest: mergedPRUrl, targetBranch: "prod", gitUser: "GitHub", - gitEmail: "noreply@github.com" + gitEmail: "noreply@github.com", + reviewers: [], + assignees: [], + inheritReviewers: true, }; const configs: Configs = await parser.parseAndValidate(args); @@ -146,7 +157,10 @@ describe("pull request config parser", () => { pullRequest: openPRUrl, targetBranch: "prod", gitUser: "GitHub", - gitEmail: "noreply@github.com" + gitEmail: "noreply@github.com", + reviewers: [], + assignees: [], + inheritReviewers: true, }; const configs: Configs = await parser.parseAndValidate(args); @@ -169,6 +183,7 @@ describe("pull request config parser", () => { title: "PR Title", body: "Please review and merge", reviewers: ["gh-user"], + assignees: [], targetRepo: { owner: "owner", project: "reponame", @@ -193,13 +208,17 @@ describe("pull request config parser", () => { pullRequest: notMergedPRUrl, targetBranch: "prod", gitUser: "GitHub", - gitEmail: "noreply@github.com" + gitEmail: "noreply@github.com", + reviewers: [], + assignees: [], + inheritReviewers: true, }; expect(async () => await parser.parseAndValidate(args)).rejects.toThrow("Provided pull request is closed and not merged!"); }); - test("override backport pr data", async () => { + + test("override backport pr data inherting reviewers", async () => { const args: Args = { dryRun: false, auth: "", @@ -210,6 +229,9 @@ describe("pull request config parser", () => { title: "New Title", body: "New Body", bodyPrefix: "New Body Prefix -", + reviewers: [], + assignees: [], + inheritReviewers: true, }; const configs: Configs = await parser.parseAndValidate(args); @@ -233,6 +255,7 @@ describe("pull request config parser", () => { title: "PR Title", body: "Please review and merge", reviewers: ["requested-gh-user", "gh-user"], + assignees: [], targetRepo: { owner: "owner", project: "reponame", @@ -254,6 +277,159 @@ describe("pull request config parser", () => { title: "New Title", body: "New Body Prefix -New Body", reviewers: ["gh-user", "that-s-a-user"], + assignees: [], + targetRepo: { + owner: "owner", + project: "reponame", + cloneUrl: "https://github.com/owner/reponame.git" + }, + sourceRepo: { + owner: "owner", + project: "reponame", + cloneUrl: "https://github.com/owner/reponame.git" + }, + bpBranchName: undefined, + nCommits: 0, + commits: [] + }); + }); + + test("override backport pr reviewers and assignees", async () => { + const args: Args = { + dryRun: false, + auth: "", + pullRequest: mergedPRUrl, + targetBranch: "prod", + gitUser: "Me", + gitEmail: "me@email.com", + title: "New Title", + body: "New Body", + bodyPrefix: "New Body Prefix -", + reviewers: ["user1", "user2"], + assignees: ["user3", "user4"], + inheritReviewers: true, // not taken into account + }; + + const configs: Configs = await parser.parseAndValidate(args); + + expect(configs.dryRun).toEqual(false); + expect(configs.git).toEqual({ + user: "Me", + email: "me@email.com" + }); + expect(configs.auth).toEqual(""); + expect(configs.targetBranch).toEqual("prod"); + expect(configs.folder).toEqual(process.cwd() + "/bp"); + expect(configs.originalPullRequest).toEqual({ + number: 2368, + author: "gh-user", + url: "https://api.github.com/repos/owner/reponame/pulls/2368", + htmlUrl: "https://github.com/owner/reponame/pull/2368", + state: "closed", + merged: true, + mergedBy: "that-s-a-user", + title: "PR Title", + body: "Please review and merge", + reviewers: ["requested-gh-user", "gh-user"], + assignees: [], + targetRepo: { + owner: "owner", + project: "reponame", + cloneUrl: "https://github.com/owner/reponame.git" + }, + sourceRepo: { + owner: "fork", + project: "reponame", + cloneUrl: "https://github.com/fork/reponame.git" + }, + bpBranchName: undefined, + nCommits: 2, + commits: ["28f63db774185f4ec4b57cd9aaeb12dbfb4c9ecc"], + }); + expect(configs.backportPullRequest).toEqual({ + author: "Me", + url: undefined, + htmlUrl: undefined, + title: "New Title", + body: "New Body Prefix -New Body", + reviewers: ["user1", "user2"], + assignees: ["user3", "user4"], + targetRepo: { + owner: "owner", + project: "reponame", + cloneUrl: "https://github.com/owner/reponame.git" + }, + sourceRepo: { + owner: "owner", + project: "reponame", + cloneUrl: "https://github.com/owner/reponame.git" + }, + bpBranchName: undefined, + nCommits: 0, + commits: [] + }); + }); + + test("override backport pr empty reviewers", async () => { + const args: Args = { + dryRun: false, + auth: "", + pullRequest: mergedPRUrl, + targetBranch: "prod", + gitUser: "Me", + gitEmail: "me@email.com", + title: "New Title", + body: "New Body", + bodyPrefix: "New Body Prefix -", + reviewers: [], + assignees: ["user3", "user4"], + inheritReviewers: false, + }; + + const configs: Configs = await parser.parseAndValidate(args); + + expect(configs.dryRun).toEqual(false); + expect(configs.git).toEqual({ + user: "Me", + email: "me@email.com" + }); + expect(configs.auth).toEqual(""); + expect(configs.targetBranch).toEqual("prod"); + expect(configs.folder).toEqual(process.cwd() + "/bp"); + expect(configs.originalPullRequest).toEqual({ + number: 2368, + author: "gh-user", + url: "https://api.github.com/repos/owner/reponame/pulls/2368", + htmlUrl: "https://github.com/owner/reponame/pull/2368", + state: "closed", + merged: true, + mergedBy: "that-s-a-user", + title: "PR Title", + body: "Please review and merge", + reviewers: ["requested-gh-user", "gh-user"], + assignees: [], + targetRepo: { + owner: "owner", + project: "reponame", + cloneUrl: "https://github.com/owner/reponame.git" + }, + sourceRepo: { + owner: "fork", + project: "reponame", + cloneUrl: "https://github.com/fork/reponame.git" + }, + bpBranchName: undefined, + nCommits: 2, + commits: ["28f63db774185f4ec4b57cd9aaeb12dbfb4c9ecc"], + }); + expect(configs.backportPullRequest).toEqual({ + author: "Me", + url: undefined, + htmlUrl: undefined, + title: "New Title", + body: "New Body Prefix -New Body", + reviewers: [], + assignees: ["user3", "user4"], targetRepo: { owner: "owner", project: "reponame", diff --git a/test/service/runner/cli-runner.test.ts b/test/service/runner/cli-runner.test.ts index f8c232d..bb758b5 100644 --- a/test/service/runner/cli-runner.test.ts +++ b/test/service/runner/cli-runner.test.ts @@ -188,7 +188,8 @@ describe("cli runner", () => { base: "target", title: "[target] PR Title", body: expect.stringContaining("**Backport:** https://github.com/owner/reponame/pull/2368"), - reviewers: ["gh-user", "that-s-a-user"] + reviewers: ["gh-user", "that-s-a-user"], + assignees: [], } ); }); @@ -227,7 +228,8 @@ describe("cli runner", () => { base: "target", title: "[target] PR Title", body: expect.stringContaining("**Backport:** https://github.com/owner/reponame/pull/8632"), - reviewers: ["gh-user", "that-s-a-user"] + reviewers: ["gh-user", "that-s-a-user"], + assignees: [], } ); }); @@ -278,7 +280,8 @@ describe("cli runner", () => { base: "target", title: "[target] PR Title", body: expect.stringContaining("**Backport:** https://github.com/owner/reponame/pull/4444"), - reviewers: ["gh-user", "that-s-a-user"] + reviewers: ["gh-user", "that-s-a-user"], + assignees: [], } ); }); @@ -297,6 +300,10 @@ describe("cli runner", () => { "New Body Prefix - ", "--bp-branch-name", "bp_branch_name", + "--reviewers", + "user1,user2", + "--assignees", + "user3,user4" ]); await runner.execute(); @@ -326,7 +333,60 @@ describe("cli runner", () => { base: "target", title: "New Title", body: "New Body Prefix - New Body", - reviewers: ["gh-user", "that-s-a-user"] + reviewers: ["user1", "user2"], + assignees: ["user3", "user4"], + } + ); + }); + + test("set empty reviewers", async () => { + addProcessArgs([ + "-tb", + "target", + "-pr", + "https://github.com/owner/reponame/pull/2368", + "--title", + "New Title", + "--body", + "New Body", + "--body-prefix", + "New Body Prefix - ", + "--bp-branch-name", + "bp_branch_name", + "--no-inherit-reviewers", + "--assignees", + "user3,user4", + ]); + + await runner.execute(); + + const cwd = process.cwd() + "/bp"; + + 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_branch_name"); + + expect(GitCLIService.prototype.fetch).toBeCalledTimes(1); + expect(GitCLIService.prototype.fetch).toBeCalledWith(cwd, "pull/2368/head:pr/2368"); + + expect(GitCLIService.prototype.cherryPick).toBeCalledTimes(1); + expect(GitCLIService.prototype.cherryPick).toBeCalledWith(cwd, "28f63db774185f4ec4b57cd9aaeb12dbfb4c9ecc"); + + expect(GitCLIService.prototype.push).toBeCalledTimes(1); + expect(GitCLIService.prototype.push).toBeCalledWith(cwd, "bp_branch_name"); + + expect(GitHubService.prototype.createPullRequest).toBeCalledTimes(1); + expect(GitHubService.prototype.createPullRequest).toBeCalledWith({ + owner: "owner", + repo: "reponame", + head: "bp_branch_name", + base: "target", + title: "New Title", + body: "New Body Prefix - New Body", + reviewers: [], + assignees: ["user3", "user4"], } ); }); diff --git a/test/service/runner/gha-runner.test.ts b/test/service/runner/gha-runner.test.ts index e1abfd0..f374527 100644 --- a/test/service/runner/gha-runner.test.ts +++ b/test/service/runner/gha-runner.test.ts @@ -87,7 +87,8 @@ describe("gha runner", () => { base: "target", title: "[target] PR Title", body: expect.stringContaining("**Backport:** https://github.com/owner/reponame/pull/2368"), - reviewers: ["gh-user", "that-s-a-user"] + reviewers: ["gh-user", "that-s-a-user"], + assignees: [], } ); }); @@ -134,7 +135,8 @@ describe("gha runner", () => { base: "target", title: "[target] PR Title", body: expect.stringContaining("**Backport:** https://github.com/owner/reponame/pull/4444"), - reviewers: ["gh-user", "that-s-a-user"] + reviewers: ["gh-user", "that-s-a-user"], + assignees: [], } ); }); @@ -147,6 +149,8 @@ describe("gha runner", () => { "body": "New Body", "body-prefix": "New Body Prefix - ", "bp-branch-name": "bp_branch_name", + "reviewers": "user1, user2", + "assignees": "user3, user4", }); await runner.execute(); @@ -176,7 +180,54 @@ describe("gha runner", () => { base: "target", title: "New Title", body: "New Body Prefix - New Body", - reviewers: ["gh-user", "that-s-a-user"] + reviewers: ["user1", "user2"], + assignees: ["user3", "user4"], + } + ); + }); + + test("set empty reviewers", async () => { + spyGetInput({ + "target-branch": "target", + "pull-request": "https://github.com/owner/reponame/pull/2368", + "title": "New Title", + "body": "New Body", + "body-prefix": "New Body Prefix - ", + "bp-branch-name": "bp_branch_name", + "reviewers": "", + "assignees": "user3, user4", + "no-inherit-reviewers": "true", + }); + + await runner.execute(); + + const cwd = process.cwd() + "/bp"; + + 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_branch_name"); + + expect(GitCLIService.prototype.fetch).toBeCalledTimes(1); + expect(GitCLIService.prototype.fetch).toBeCalledWith(cwd, "pull/2368/head:pr/2368"); + + expect(GitCLIService.prototype.cherryPick).toBeCalledTimes(1); + expect(GitCLIService.prototype.cherryPick).toBeCalledWith(cwd, "28f63db774185f4ec4b57cd9aaeb12dbfb4c9ecc"); + + expect(GitCLIService.prototype.push).toBeCalledTimes(1); + expect(GitCLIService.prototype.push).toBeCalledWith(cwd, "bp_branch_name"); + + expect(GitHubService.prototype.createPullRequest).toBeCalledTimes(1); + expect(GitHubService.prototype.createPullRequest).toBeCalledWith({ + owner: "owner", + repo: "reponame", + head: "bp_branch_name", + base: "target", + title: "New Title", + body: "New Body Prefix - New Body", + reviewers: [], + assignees: ["user3", "user4"], } ); }); diff --git a/test/support/utils.ts b/test/support/utils.ts index beffea9..8be9212 100644 --- a/test/support/utils.ts +++ b/test/support/utils.ts @@ -14,4 +14,14 @@ export const spyGetInput = (obj: any) => { mock.mockImplementation((name: string) : string => { return obj[name] ?? ""; }); +}; + +/** + * Check array equality performing sort on both sides. + * DO NOT USE this if ordering matters + * @param actual + * @param expected + */ +export const expectArrayEqual = (actual: unknown[], expected: unknown[]) => { + expect(actual.sort()).toEqual(expected.sort()); }; \ No newline at end of file