honeymoose
/
angular-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(dev-infra): detect multiple target labels as invalid (#40156) 

When multiple target labels are applied to a PR, it should be considered
invalid as our tooling does not support a single PR targetting multiple
trains/versions.

PR Close #40156
This commit is contained in:
Joey Perrott 2020-12-16 10:02:48 -08:00 committed by atscott
parent 3acbec8532
commit 3d7e207a25
6 changed files with 69 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

46
dev-infra/ng-dev.js
View File

@ -2782,21 +2782,21 @@ var InvalidTargetLabelError = /** @class */ (function () {
/** Gets the target label from the specified pull request labels. */
function getTargetLabelFromPullRequest(config, labels) {
var e_1, _a;
/** List of discovered target labels for the PR. */
var matches = [];
var _loop_1 = function (label) {
var match = config.labels.find(function (_a) {
var pattern = _a.pattern;
return matchesPattern(label, pattern);
});
if (match !== undefined) {
return { value: match };
matches.push(match);
}
};
try {
for (var labels_1 = tslib.__values(labels), labels_1_1 = labels_1.next(); !labels_1_1.done; labels_1_1 = labels_1.next()) {
var label = labels_1_1.value;
var state_1 = _loop_1(label);
if (typeof state_1 === "object")
return state_1.value;
_loop_1(label);
}
}
catch (e_1_1) { e_1 = { error: e_1_1 }; }
@ -2806,7 +2806,13 @@ function getTargetLabelFromPullRequest(config, labels) {
}
finally { if (e_1) throw e_1.error; }
}
return null;
if (matches.length === 1) {
return matches[0];
}
if (matches.length === 0) {
throw new InvalidTargetLabelError('Unable to determine target for the PR as it has no target label.');
}
throw new InvalidTargetLabelError('Unable to determine target for the PR as it has multiple target labels.');
}
/**
* Gets the branches from the specified target label.
@ -2862,11 +2868,17 @@ function getTargetBranchesForPr(prNumber) {
/** The branch targetted via the Github UI. */
const githubTargetBranch = prData.base.ref;
/** The active label which is being used for targetting the PR. */
const targetLabel = getTargetLabelFromPullRequest(mergeConfig, labels);
if (targetLabel === null) {
error(red(`No target label was found on pr #${prNumber}`));
process.exitCode = 1;
return;
let targetLabel;
try {
targetLabel = getTargetLabelFromPullRequest(mergeConfig, labels);
}
catch (e) {
if (e instanceof InvalidTargetLabelError) {
error(red(e.failureMessage));
process.exitCode = 1;
return;
}
throw e;
}
/** The target branches based on the target label and branch targetted in the Github UI. */
return yield getBranchesFromTargetLabel(targetLabel, githubTargetBranch);
@ -3333,9 +3345,6 @@ var PullRequestFailure = /** @class */ (function () {
PullRequestFailure.notMergeReady = function () {
return new this("Not marked as merge ready.");
};
PullRequestFailure.noTargetLabel = function () {
return new this("No target branch could be determined. Please ensure a target label is set.");
};
PullRequestFailure.mismatchingTargetBranch = function (allowedBranches) {
return new this("Pull request is set to wrong base branch. Please update the PR in the Github UI " +
("to one of the following branches: " + allowedBranches.join(', ') + "."));
@ -3413,9 +3422,14 @@ function loadAndValidatePullRequest(_a, prNumber, ignoreNonFatalFailures) {
if (!labels.some(function (name) { return matchesPattern(name, config.claSignedLabel); })) {
return [2 /*return*/, PullRequestFailure.claUnsigned()];
}
targetLabel = getTargetLabelFromPullRequest(config, labels);
if (targetLabel === null) {
return [2 /*return*/, PullRequestFailure.noTargetLabel()];
try {
targetLabel = getTargetLabelFromPullRequest(config, labels);
}
catch (error) {
if (error instanceof InvalidTargetLabelError) {
return [2 /*return*/, new PullRequestFailure(error.failureMessage)];
}
throw error;
}
return [4 /*yield*/, git.github.repos.getCombinedStatusForRef(tslib.__assign(tslib.__assign({}, git.remoteParams), { ref: prData.head.sha }))];
case 2:

20
dev-infra/pr/check-target-branches/check-target-branches.ts
View File

@ -9,8 +9,8 @@
import {getConfig} from '../../utils/config';
import {error, info, red} from '../../utils/console';
import {GitClient} from '../../utils/git/index';
import {loadAndValidateConfig} from '../merge/config';
import {getBranchesFromTargetLabel, getTargetLabelFromPullRequest} from '../merge/target-label';
import {loadAndValidateConfig, TargetLabel} from '../merge/config';
import {getBranchesFromTargetLabel, getTargetLabelFromPullRequest, InvalidTargetLabelError} from '../merge/target-label';
export async function getTargetBranchesForPr(prNumber: number) {
/** The ng-dev configuration. */
@ -31,11 +31,17 @@ export async function getTargetBranchesForPr(prNumber: number) {
/** The branch targetted via the Github UI. */
const githubTargetBranch = prData.base.ref;
/** The active label which is being used for targetting the PR. */
const targetLabel = getTargetLabelFromPullRequest(mergeConfig!, labels);
if (targetLabel === null) {
error(red(`No target label was found on pr #${prNumber}`));
process.exitCode = 1;
return;
let targetLabel: TargetLabel;
try {
targetLabel = getTargetLabelFromPullRequest(mergeConfig!, labels);
} catch (e) {
if (e instanceof InvalidTargetLabelError) {
error(red(e.failureMessage));
process.exitCode = 1;
return;
}
throw e;
}
/** The target branches based on the target label and branch targetted in the Github UI. */
return await getBranchesFromTargetLabel(targetLabel, githubTargetBranch);

6
dev-infra/pr/merge/defaults/integration.spec.ts
View File

@ -87,8 +87,10 @@ describe('default target labels', () => {
if (labels === undefined) {
labels = await computeTargetLabels();
}
const label = getTargetLabelFromPullRequest({labels}, [name]);
if (label === null) {
let label: TargetLabel;
try {
label = getTargetLabelFromPullRequest({labels}, [name]);
} catch (error) {
return null;
}
return await getBranchesFromTargetLabel(label, githubTargetBranch);

4
dev-infra/pr/merge/failures.ts
View File

@ -34,10 +34,6 @@ export class PullRequestFailure {
return new this(`Not marked as merge ready.`);
}
static noTargetLabel() {
return new this(`No target branch could be determined. Please ensure a target label is set.`);
}
static mismatchingTargetBranch(allowedBranches: string[]) {
return new this(
`Pull request is set to wrong base branch. Please update the PR in the Github UI ` +

12
dev-infra/pr/merge/pull-request.ts
View File

@ -9,6 +9,7 @@
import * as Octokit from '@octokit/rest';
import {GitClient} from '../../utils/git/index';
import {TargetLabel} from './config';
import {PullRequestFailure} from './failures';
import {matchesPattern} from './string-pattern';
@ -61,9 +62,14 @@ export async function loadAndValidatePullRequest(
return PullRequestFailure.claUnsigned();
}
const targetLabel = getTargetLabelFromPullRequest(config, labels);
if (targetLabel === null) {
return PullRequestFailure.noTargetLabel();
let targetLabel: TargetLabel;
try {
targetLabel = getTargetLabelFromPullRequest(config, labels);
} catch (error) {
if (error instanceof InvalidTargetLabelError) {
return new PullRequestFailure(error.failureMessage);
}
throw error;
}
const {data: {state}} =

16
dev-infra/pr/merge/target-label.ts
View File

@ -27,14 +27,24 @@ export class InvalidTargetLabelError {
/** Gets the target label from the specified pull request labels. */
export function getTargetLabelFromPullRequest(
config: Pick<MergeConfig, 'labels'>, labels: string[]): TargetLabel|null {
config: Pick<MergeConfig, 'labels'>, labels: string[]): TargetLabel {
/** List of discovered target labels for the PR. */
const matches = [];
for (const label of labels) {
const match = config.labels.find(({pattern}) => matchesPattern(label, pattern));
if (match !== undefined) {
return match;
matches.push(match);
}
}
return null;
if (matches.length === 1) {
return matches[0];
}
if (matches.length === 0) {
throw new InvalidTargetLabelError(
'Unable to determine target for the PR as it has no target label.');
}
throw new InvalidTargetLabelError(
'Unable to determine target for the PR as it has multiple target labels.');
}
/**