8000 fix(dev-infra): detect multiple target labels as invalid by josephperrott · Pull Request #40156 · angular/angular · GitHub
[go: up one dir, main page]
Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
26ff23a
fix(dev-infra): detect multiple target labels as invalid
josephperrott Dec 16, 2020
4b89baf
fixup! fix(dev-infra): detect multiple target labels as invalid
josephperrott Jan 8, 2021
fe502d2
fixup! fix(dev-infra): detect multiple target labels as invalid
josephperrott Jan 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 30 additions & 16 deletions dev-infra/ng-dev.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 }; }
Expand All @@ -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.
Expand Down Expand Up @@ -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 = getTarg 8000 etLabelFromPullRequest(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);
Expand Down Expand Up @@ -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(', ') + "."));
Expand Down Expand Up @@ -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:
Expand Down
20 changes: 13 additions & 7 deletions dev-infra/pr/check-target-branches/check-target-branches.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand All @@ -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);
Expand Down
6 changes: 4 additions & 2 deletions dev-infra/pr/merge/defaults/integration.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,10 @@ describe('default target labels', () => {
if (labels === undefined) {
labels = await computeTargetLabels();
}
const label = getTargetLabelFromPullRequest({labels}, [n 8000 ame]);
if (label === null) {
let label: TargetLabel;
try {
label = getTargetLabelFromPullRequest({labels}, [name]);
} catch (error) {
return null;
}
return await getBranchesFromTargetLabel(label, githubTargetBranch);
Expand Down
4 changes: 0 additions & 4 deletions dev-infra/pr/merge/failures.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 ` +
Expand Down
12 changes: 9 additions & 3 deletions dev-infra/pr/merge/pull-request.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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}} =
Expand Down
16 changes: 13 additions & 3 deletions dev-infra/pr/merge/target-label.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.');
}

/**
Expand Down
0