From c8587325bf059b8f7e346b0b5c2426283092a16f Mon Sep 17 00:00:00 2001 From: Joey Perrott Date: Fri, 24 Sep 2021 14:43:38 -0700 Subject: [PATCH] refactor(ng-dev/pr): move overall CI pending/failing check to common valiations Move the check of the overall CI pending or failing check to the common validations. --- ng-dev/pr/common/validation/validations.ts | 13 +++++++++++++ ng-dev/pr/merge/pull-request.ts | 20 ++++++-------------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/ng-dev/pr/common/validation/validations.ts b/ng-dev/pr/common/validation/validations.ts index bfe2807127..f3e5108abc 100644 --- a/ng-dev/pr/common/validation/validations.ts +++ b/ng-dev/pr/common/validation/validations.ts @@ -134,6 +134,19 @@ export function assertMergeReady(pullRequest: PullRequestFromGithub, config: Pul throw PullRequestFailure.notMergeReady(); } +/** + * Assert the pull request has been marked ready for merge by the author. + * @throws {PullRequestFailure} if the pull request is missing the merge ready label. + */ +export function assertPassingCi(pullRequest: PullRequestFromGithub) { + if (getStatusesForPullRequest(pullRequest).combinedStatus === PullRequestStatus.FAILING) { + throw PullRequestFailure.pendingCiJobs(); + } + if (getStatusesForPullRequest(pullRequest).combinedStatus === PullRequestStatus.FAILING) { + throw PullRequestFailure.failingCiJobs(); + } +} + // TODO: Remove need to export this pattern matching utility. /** Checks whether the specified value matches the given pattern. */ export function matchesPattern(value: string, pattern: RegExp | string): boolean { diff --git a/ng-dev/pr/merge/pull-request.ts b/ng-dev/pr/merge/pull-request.ts index 938e0991d9..217cf0b233 100644 --- a/ng-dev/pr/merge/pull-request.ts +++ b/ng-dev/pr/merge/pull-request.ts @@ -16,12 +16,9 @@ import { matchesPattern, assertPendingState, assertSignedCla, + assertPassingCi, } from '../common/validation/validations'; -import { - fetchPullRequestFromGithub, - getStatusesForPullRequest, - PullRequestStatus, -} from '../common/fetch-pull-request'; +import {fetchPullRequestFromGithub} from '../common/fetch-pull-request'; /** Interface that describes a pull request. */ export interface PullRequest { @@ -81,6 +78,10 @@ export async function loadAndValidatePullRequest( assertSignedCla(prData); assertPendingState(prData); assertCorrectBreakingChangeLabeling(commitsInPr, labels); + + if (!ignoreNonFatalFailures) { + assertPassingCi(prData); + } } catch (error) { // If the error is a pull request failure, we pass it through gracefully // as the tool expects such failures to be returned from the function. @@ -90,15 +91,6 @@ export async function loadAndValidatePullRequest( throw error; } - /** The combined status of the latest commit in the pull request. */ - const {combinedStatus} = getStatusesForPullRequest(prData); - if (combinedStatus === PullRequestStatus.FAILING && !ignoreNonFatalFailures) { - return PullRequestFailure.failingCiJobs(); - } - if (combinedStatus === PullRequestStatus.PENDING && !ignoreNonFatalFailures) { - return PullRequestFailure.pendingCiJobs(); - } - const requiredBaseSha = config.pullRequest.requiredBaseCommits && config.pullRequest.requiredBaseCommits[githubTargetBranch];