From 77806dc1053cf78fdb5329f8ad16be5a3deff14e Mon Sep 17 00:00:00 2001 From: Niels Bauman Date: Mon, 24 Mar 2025 13:04:58 +0100 Subject: [PATCH 1/2] Allow configuring RegEx patterns for `copySourcePRLabels` Currently, we only allow copying over either all or no labels. With this commit, we also allow specifying a list of RegEx patterns that will be used to conditionally copy over labels. This is useful when you don't want to copy over all labels, but you also don't want to manually specify every time which labels you want to be copied over. --- docs/config-file-options.md | 8 ++++-- .../copySourcePullRequestLabels.ts | 27 +++++++++++++++---- src/options/ConfigOptions.ts | 2 +- 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/docs/config-file-options.md b/docs/config-file-options.md index a38fc00e..11204497 100644 --- a/docs/config-file-options.md +++ b/docs/config-file-options.md @@ -490,11 +490,15 @@ Assign the same reviewers to the target pull request that were assigned to the o #### `copySourcePRLabels` -Copies all labels from the original (source) pull request to the backport (target) pull request. +Copies labels from the original (source) pull request to the backport (target) pull request. +Can be either a boolean or an array of strings. When set to `true`, _all_ labels from the source PR are copied to the target PR. +When an array of strings is configured, each string is used as a RegEx pattern and will be compared to each label of the source PR. +If the label on the source PR matches at least one configured string, it will be copied to the target PR. +Default: `false` ```json { - "copySourcePRLabels": false + "copySourcePRLabels": ["^Team:.*", "^:.*", "^>.*"] } ``` diff --git a/src/lib/cherrypickAndCreateTargetPullRequest/copySourcePullRequestLabels.ts b/src/lib/cherrypickAndCreateTargetPullRequest/copySourcePullRequestLabels.ts index a6c22cbb..84319fa4 100644 --- a/src/lib/cherrypickAndCreateTargetPullRequest/copySourcePullRequestLabels.ts +++ b/src/lib/cherrypickAndCreateTargetPullRequest/copySourcePullRequestLabels.ts @@ -7,22 +7,39 @@ export async function copySourcePullRequestLabelsToTargetPullRequest( commits: Commit[], pullNumber: number, ) { - const labels = getNonBackportLabels(commits); + const labels = getNonBackportLabels(commits, options); if (labels.length > 0) { await addLabelsToPullRequest({ ...options, pullNumber, labels }); } } -function getNonBackportLabels(commits: Commit[]) { +function getNonBackportLabels(commits: Commit[], options: ValidConfigOptions) { return commits.flatMap((commit) => { if (!commit.sourcePullRequest) { return []; } const backportLabels = commit.targetPullRequestStates.map((pr) => pr.label); - const labels = commit.sourcePullRequest.labels.filter( - (label) => !backportLabels.includes(label), - ); + const labels = commit.sourcePullRequest.labels.filter((label) => { + // If `copySourcePRLabels` is a boolean, it must be true to reach this method. + // Therefore, we simply copy all labels from the source PR that aren't already on the target PR. + const copySourcePRLabels = options.copySourcePRLabels; + if (typeof copySourcePRLabels === 'boolean') { + return !backportLabels.includes(label); + } + // Otherwise, it's an array of regex patterns. + if ( + typeof copySourcePRLabels === 'object' && + copySourcePRLabels.constructor === Array + ) { + return copySourcePRLabels.some((sourceLabel) => + label.match(new RegExp(sourceLabel)), + ); + } + throw new Error( + 'Unexpected type of copySourcePRLabels, must be boolean or array', + ); + }); return labels; }); diff --git a/src/options/ConfigOptions.ts b/src/options/ConfigOptions.ts index 1271cb58..c29fd499 100644 --- a/src/options/ConfigOptions.ts +++ b/src/options/ConfigOptions.ts @@ -31,7 +31,7 @@ type Options = Partial<{ cherrypickRef: boolean; commitConflicts: boolean; commitPaths: string[]; - copySourcePRLabels: boolean; + copySourcePRLabels: boolean | string[]; copySourcePRReviewers: boolean; details: boolean; dir: string; From 9114dad16f4275ad7664ff9993fc4bec5aac66d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Louv-Jansen?= Date: Wed, 16 Apr 2025 22:13:38 +0200 Subject: [PATCH 2/2] Add unit tests --- .../copySourcePullRequestLabels.test.ts | 72 +++++++++++++++++++ .../copySourcePullRequestLabels.ts | 21 +++--- src/options/cliArgs.test.ts | 30 ++++++++ src/options/cliArgs.ts | 8 ++- .../e2e/cli/entrypoint.cli.private.test.ts | 2 +- 5 files changed, 122 insertions(+), 11 deletions(-) create mode 100644 src/lib/cherrypickAndCreateTargetPullRequest/copySourcePullRequestLabels.test.ts diff --git a/src/lib/cherrypickAndCreateTargetPullRequest/copySourcePullRequestLabels.test.ts b/src/lib/cherrypickAndCreateTargetPullRequest/copySourcePullRequestLabels.test.ts new file mode 100644 index 00000000..67d648e0 --- /dev/null +++ b/src/lib/cherrypickAndCreateTargetPullRequest/copySourcePullRequestLabels.test.ts @@ -0,0 +1,72 @@ +import { Commit } from '../../entrypoint.api'; +import { ValidConfigOptions } from '../../options/options'; +import { getLabelsToCopy } from './copySourcePullRequestLabels'; + +describe('getLabelsToCopy', () => { + it('should return an empty array when no commits have sourcePullRequest', () => { + const commits = [{ sourcePullRequest: null }] as unknown as Commit[]; + const options = { copySourcePRLabels: true } as ValidConfigOptions; + const result = getLabelsToCopy(commits, options); + expect(result).toEqual([]); + }); + + it('copies all labels except backport labels when copySourcePRLabels is boolean', () => { + const commits = [ + { + sourcePullRequest: { + title: 'My pr title', + labels: ['a', 'b', 'my-backport-label'], + number: 1, + }, + targetPullRequestStates: [{ label: 'my-backport-label' } as any], + } as unknown as Commit, + ]; + + const options = { + copySourcePRLabels: true, + } as unknown as ValidConfigOptions; + + const result = getLabelsToCopy(commits, options); + expect(result).toEqual(['a', 'b']); + }); + + it('copies labels using regex patterns when copySourcePRLabels is string array', () => { + const commits = [ + { + sourcePullRequest: { + title: 'PR', + labels: ['feat:new', 'chore', 'important-bug'], + number: 2, + }, + targetPullRequestStates: [], + } as unknown as Commit, + ]; + + const options = { + copySourcePRLabels: ['^feat', 'bug$'], + } as unknown as ValidConfigOptions; + + const result = getLabelsToCopy(commits, options); + expect(result).toEqual(['feat:new', 'important-bug']); + }); + + it('handles multiple commits and flattens results', () => { + const commits = [ + { + sourcePullRequest: { title: 'PR1', labels: ['x'], number: 3 }, + targetPullRequestStates: [], + } as unknown as Commit, + { + sourcePullRequest: { title: 'PR2', labels: ['y'], number: 4 }, + targetPullRequestStates: [], + } as unknown as Commit, + ]; + + const options = { + copySourcePRLabels: true, + } as unknown as ValidConfigOptions; + + const result = getLabelsToCopy(commits, options); + expect(result).toEqual(['x', 'y']); + }); +}); diff --git a/src/lib/cherrypickAndCreateTargetPullRequest/copySourcePullRequestLabels.ts b/src/lib/cherrypickAndCreateTargetPullRequest/copySourcePullRequestLabels.ts index 84319fa4..5e5409e2 100644 --- a/src/lib/cherrypickAndCreateTargetPullRequest/copySourcePullRequestLabels.ts +++ b/src/lib/cherrypickAndCreateTargetPullRequest/copySourcePullRequestLabels.ts @@ -1,3 +1,4 @@ +import { isArray } from 'lodash'; import { ValidConfigOptions } from '../../options/options'; import { addLabelsToPullRequest } from '../github/v3/addLabelsToPullRequest'; import { Commit } from '../sourceCommit/parseSourceCommit'; @@ -7,13 +8,16 @@ export async function copySourcePullRequestLabelsToTargetPullRequest( commits: Commit[], pullNumber: number, ) { - const labels = getNonBackportLabels(commits, options); + const labels = getLabelsToCopy(commits, options); if (labels.length > 0) { await addLabelsToPullRequest({ ...options, pullNumber, labels }); } } -function getNonBackportLabels(commits: Commit[], options: ValidConfigOptions) { +export function getLabelsToCopy( + commits: Commit[], + options: ValidConfigOptions, +) { return commits.flatMap((commit) => { if (!commit.sourcePullRequest) { return []; @@ -24,20 +28,19 @@ function getNonBackportLabels(commits: Commit[], options: ValidConfigOptions) { // If `copySourcePRLabels` is a boolean, it must be true to reach this method. // Therefore, we simply copy all labels from the source PR that aren't already on the target PR. const copySourcePRLabels = options.copySourcePRLabels; - if (typeof copySourcePRLabels === 'boolean') { - return !backportLabels.includes(label); + if (copySourcePRLabels === true) { + const isBackportLabel = backportLabels.includes(label); + return !isBackportLabel; } + // Otherwise, it's an array of regex patterns. - if ( - typeof copySourcePRLabels === 'object' && - copySourcePRLabels.constructor === Array - ) { + if (isArray(copySourcePRLabels)) { return copySourcePRLabels.some((sourceLabel) => label.match(new RegExp(sourceLabel)), ); } throw new Error( - 'Unexpected type of copySourcePRLabels, must be boolean or array', + `Unexpected type of copySourcePRLabels: ${JSON.stringify(copySourcePRLabels)}, must be boolean or array`, ); }); diff --git a/src/options/cliArgs.test.ts b/src/options/cliArgs.test.ts index 0f4f0b49..c8696c86 100644 --- a/src/options/cliArgs.test.ts +++ b/src/options/cliArgs.test.ts @@ -261,6 +261,36 @@ describe('getOptionsFromCliArgs', () => { }); }); + describe('copySourcePRReviewers', () => { + it('should be undefined when not provided', () => { + const res = getOptionsFromCliArgs([]); + expect(res.copySourcePRReviewers).toBe(undefined); + }); + + it('should be true when option is provided with no value given', () => { + const res = getOptionsFromCliArgs(['--copy-source-pr-reviewers']); + expect(res.copySourcePRReviewers).toBe(true); + }); + + it('should set to false', () => { + const argv = ['--copy-source-pr-reviewers', 'false']; + const res = getOptionsFromCliArgs(argv); + expect(res.copySourcePRReviewers).toBe(false); + }); + + it('should set to true', () => { + const argv = ['--copy-source-pr-reviewers', 'true']; + const res = getOptionsFromCliArgs(argv); + expect(res.copySourcePRReviewers).toBe(true); + }); + + it('should accept a list of regexes', () => { + const argv = ['--copy-source-pr-reviewers', '^foo, bar$, baz*']; + const res = getOptionsFromCliArgs(argv); + expect(res.copySourcePRReviewers).toEqual(['^foo', 'bar$', 'baz*']); + }); + }); + describe('repo', () => { it('splits into repoOwner and repoName', () => { const argv = ['--repo', 'elastic/kibana']; diff --git a/src/options/cliArgs.ts b/src/options/cliArgs.ts index 1ce27098..2248c977 100644 --- a/src/options/cliArgs.ts +++ b/src/options/cliArgs.ts @@ -392,7 +392,13 @@ export function getOptionsFromCliArgs(processArgs: readonly string[]) { .option('copySourcePRReviewers', { description: 'Copy reviewers from the source PR to the target PR', alias: ['copySourcePrReviewers', 'addOriginalReviewers'], - type: 'boolean', + type: 'string', + coerce: (val: string) => { + if (val === 'true' || val === '') return true; + if (val === 'false') return false; + + return val.split(',').map((s) => s.trim()); + }, }) .option('targetBranch', { diff --git a/src/test/e2e/cli/entrypoint.cli.private.test.ts b/src/test/e2e/cli/entrypoint.cli.private.test.ts index e4efada2..9fa5ebdd 100644 --- a/src/test/e2e/cli/entrypoint.cli.private.test.ts +++ b/src/test/e2e/cli/entrypoint.cli.private.test.ts @@ -120,7 +120,7 @@ Options: [boolean] --copySourcePRReviewers, Copy reviewers from the source PR to the --copySourcePrReviewers, target PR - --addOriginalReviewers [boolean] + --addOriginalReviewers [string] -b, --targetBranch, --branch Branch(es) to backport to [array] --targetBranchChoice List branches to backport to [array] -l, --targetPRLabel, --label Add labels to the target (backport) PR [array]