diff --git a/ng-dev/pr/common/validation/assert-passing-ci.ts b/ng-dev/pr/common/validation/assert-passing-ci.ts index f78fa02857..159171c119 100644 --- a/ng-dev/pr/common/validation/assert-passing-ci.ts +++ b/ng-dev/pr/common/validation/assert-passing-ci.ts @@ -31,10 +31,11 @@ class Validation extends PullRequestValidation { if (statuses.find((status) => status.name === 'lint') === undefined) { throw this._createError( 'Pull request is missing expected status checks. Check the pull request for pending workflows', + false, ); } if (combinedStatus === PullRequestStatus.PENDING) { - throw this._createError('Pull request has pending status checks.'); + throw this._createError('Pull request has pending status checks.', false); } if (combinedStatus === PullRequestStatus.FAILING) { throw this._createError('Pull request has failing status checks.'); diff --git a/ng-dev/pr/common/validation/validate-pull-request.ts b/ng-dev/pr/common/validation/validate-pull-request.ts index f633be1788..62ee5f075b 100644 --- a/ng-dev/pr/common/validation/validate-pull-request.ts +++ b/ng-dev/pr/common/validation/validate-pull-request.ts @@ -10,7 +10,9 @@ import {parseCommitMessage} from '../../../commit-message/parse.js'; import {ActiveReleaseTrains} from '../../../release/versioning/active-release-trains.js'; import {NgDevConfig, GithubConfig} from '../../../utils/config.js'; import {PullRequestConfig, PullRequestValidationConfig} from '../../config/index.js'; -import {PullRequestFromGithub} from '../fetch-pull-request.js'; +import {fetchPullRequestFromGithub, PullRequestFromGithub} from '../fetch-pull-request.js'; +import {Log} from '../../../utils/logging.js'; +import {Spinner} from '../../../utils/spinner.js'; import {PullRequestTarget} from '../targeting/target-label.js'; import {changesAllowForTargetLabelValidation} from './assert-allowed-target-label.js'; import {breakingChangeInfoValidation} from './assert-breaking-change-info.js'; @@ -33,7 +35,7 @@ import {AuthenticatedGitClient} from '../../../utils/git/authenticated-git-clien * Active release trains may be available for additional checks or not. */ export async function assertValidPullRequest( - pullRequest: PullRequestFromGithub, + originalPullRequest: PullRequestFromGithub, validationConfig: PullRequestValidationConfig, ngDevConfig: NgDevConfig<{ pullRequest: PullRequestConfig; @@ -43,48 +45,98 @@ export async function assertValidPullRequest( target: PullRequestTarget, gitClient: AuthenticatedGitClient, ): Promise { - const labels = pullRequest.labels.nodes.map((l) => l.name); - const commitsInPr = pullRequest.commits.nodes.map((n) => { - return parseCommitMessage(n.commit.message); - }); + let pullRequest = originalPullRequest; + let spinner: Spinner | undefined; + let attempts = 0; + const maxAttempts = 60; - const validationResults = [ - minimumReviewsValidation.run(validationConfig, pullRequest), - completedReviewsValidation.run(validationConfig, pullRequest), - mergeReadyValidation.run(validationConfig, pullRequest), - signedClaValidation.run(validationConfig, pullRequest), - pendingStateValidation.run(validationConfig, pullRequest), - breakingChangeInfoValidation.run(validationConfig, commitsInPr, labels), - passingCiValidation.run(validationConfig, pullRequest), - enforcedStatusesValidation.run(validationConfig, pullRequest, ngDevConfig.pullRequest), - isolatedSeparateFilesValidation.run( - validationConfig, - ngDevConfig, - pullRequest.number, - gitClient, - ), - enforceTestedValidation.run(validationConfig, pullRequest, gitClient), - ]; + while (true) { + const labels = pullRequest.labels.nodes.map((l) => l.name); + const commitsInPr = pullRequest.commits.nodes.map((n) => { + return parseCommitMessage(n.commit.message); + }); - if (activeReleaseTrains !== null) { - validationResults.push( - changesAllowForTargetLabelValidation.run( + const validationResults = [ + minimumReviewsValidation.run(validationConfig, pullRequest), + completedReviewsValidation.run(validationConfig, pullRequest), + mergeReadyValidation.run(validationConfig, pullRequest), + signedClaValidation.run(validationConfig, pullRequest), + pendingStateValidation.run(validationConfig, pullRequest), + breakingChangeInfoValidation.run(validationConfig, commitsInPr, labels), + passingCiValidation.run(validationConfig, pullRequest), + enforcedStatusesValidation.run(validationConfig, pullRequest, ngDevConfig.pullRequest), + isolatedSeparateFilesValidation.run( validationConfig, - commitsInPr, - target.label, - ngDevConfig.pullRequest, - activeReleaseTrains, - labels, - pullRequest, + ngDevConfig, + pullRequest.number, + gitClient, ), - ); - } + enforceTestedValidation.run(validationConfig, pullRequest, gitClient), + ]; + + if (activeReleaseTrains !== null) { + validationResults.push( + changesAllowForTargetLabelValidation.run( + validationConfig, + commitsInPr, + target.label, + ngDevConfig.pullRequest, + activeReleaseTrains, + labels, + pullRequest, + ), + ); + } - return await Promise.all(validationResults).then((results) => { - return results.filter( + const results = await Promise.all(validationResults); + const failures = results.filter( <(result: null | PullRequestValidationFailure) => result is PullRequestValidationFailure>( ((result) => result !== null) ), ); - }); + + const finalFailures = failures.filter((f) => f.isFinal); + const nonFinalFailures = failures.filter((f) => !f.isFinal); + + if ( + nonFinalFailures.length > 0 && + finalFailures.length === 0 && + validationConfig.waitIfPending + ) { + if (attempts >= maxAttempts) { + if (spinner) { + spinner.complete(); + } + Log.error( + `Timed out waiting for non-final validations to complete after ${maxAttempts} minutes.`, + ); + return failures; + } + attempts++; + + const names = nonFinalFailures.map((f) => f.validationName).join(', '); + const verb = nonFinalFailures.length === 1 ? 'is' : 'are'; + const spinnerText = `[${names}] ${verb} not final. Waiting for completion (attempt ${attempts}/${maxAttempts})...`; + + if (!spinner) { + spinner = new Spinner(spinnerText); + } else { + spinner.update(spinnerText); + } + + await new Promise((resolve) => setTimeout(resolve, 60000)); // Wait 1 minute + const freshPr = await fetchPullRequestFromGithub(gitClient, originalPullRequest.number); + if (!freshPr) { + throw new Error('Failed to re-fetch pull request data'); + } + pullRequest = freshPr; + continue; + } + + if (spinner) { + spinner.complete(); + } + + return failures; + } } diff --git a/ng-dev/pr/common/validation/validation-config.ts b/ng-dev/pr/common/validation/validation-config.ts index 11de3a00b0..63309a9178 100644 --- a/ng-dev/pr/common/validation/validation-config.ts +++ b/ng-dev/pr/common/validation/validation-config.ts @@ -33,7 +33,10 @@ export function createPullRequestValidationConfig( } /** Type describing a helper function for validations to create a validation failure. */ -export type PullRequestValidationErrorCreateFn = (message: string) => PullRequestValidationFailure; +export type PullRequestValidationErrorCreateFn = ( + message: string, + isFinal?: boolean, +) => PullRequestValidationFailure; /** * Base class for pull request validations, providing helpers for the validation errors, @@ -62,7 +65,8 @@ export function createPullRequestValidation( if (validationConfig[name]) { const validation = new (getValidationCtor())( name, - (message) => new PullRequestValidationFailure(message, name, canBeForceIgnored), + (message, isFinal = true) => + new PullRequestValidationFailure(message, name, canBeForceIgnored, isFinal), ); try { await validation.assert(...args); diff --git a/ng-dev/pr/common/validation/validation-failure.ts b/ng-dev/pr/common/validation/validation-failure.ts index 33b273eeac..4c40009756 100644 --- a/ng-dev/pr/common/validation/validation-failure.ts +++ b/ng-dev/pr/common/validation/validation-failure.ts @@ -17,5 +17,7 @@ export class PullRequestValidationFailure { public readonly validationName: keyof PullRequestValidationConfig, /** Validation config name for the failure. */ public readonly canBeForceIgnored: boolean, + /** Whether the failure is final and will not change without action from a human. */ + public readonly isFinal: boolean = true, ) {} } diff --git a/ng-dev/pr/config/index.ts b/ng-dev/pr/config/index.ts index c2dae51fec..b2b678a3f2 100644 --- a/ng-dev/pr/config/index.ts +++ b/ng-dev/pr/config/index.ts @@ -90,4 +90,5 @@ export function assertValidPullRequestConfig( export interface PullRequestValidationConfig { [key: `assert${string}`]: boolean; + waitIfPending?: boolean; } diff --git a/ng-dev/pr/merge/cli.ts b/ng-dev/pr/merge/cli.ts index 8aff5658f7..bf827977f3 100644 --- a/ng-dev/pr/merge/cli.ts +++ b/ng-dev/pr/merge/cli.ts @@ -18,6 +18,7 @@ export interface MergeCommandOptions { forceManualBranches: boolean; dryRun: boolean; ignorePendingReviews: boolean; + waitForValidations: boolean; } /** Builds the command. */ @@ -45,6 +46,11 @@ async function builder(argv: Argv) { type: 'boolean', default: false, description: 'Bypass the check for pending reviews on the pull request', + }) + .option('wait-for-validations' as 'waitForValidations', { + type: 'boolean', + default: false, + description: 'Wait for pending validations to complete before merging.', }); } @@ -55,8 +61,15 @@ async function handler({ forceManualBranches, dryRun, ignorePendingReviews, + waitForValidations, }: Arguments) { - await mergePullRequest(pr, {branchPrompt, forceManualBranches, dryRun, ignorePendingReviews}); + await mergePullRequest(pr, { + branchPrompt, + forceManualBranches, + dryRun, + ignorePendingReviews, + waitForValidations, + }); } /** yargs command module describing the command. */ diff --git a/ng-dev/pr/merge/merge-tool.ts b/ng-dev/pr/merge/merge-tool.ts index 754a592342..6b8108ea38 100644 --- a/ng-dev/pr/merge/merge-tool.ts +++ b/ng-dev/pr/merge/merge-tool.ts @@ -39,6 +39,7 @@ export interface PullRequestMergeFlags { forceManualBranches: boolean; dryRun: boolean; ignorePendingReviews: boolean; + waitForValidations: boolean; } const defaultPullRequestMergeFlags: PullRequestMergeFlags = { @@ -46,6 +47,7 @@ const defaultPullRequestMergeFlags: PullRequestMergeFlags = { forceManualBranches: false, dryRun: false, ignorePendingReviews: false, + waitForValidations: false, }; /** @@ -82,6 +84,7 @@ export class MergeTool { const validationConfig = createPullRequestValidationConfig({ ...this.config.pullRequest.validators, ...partialValidationConfig, + waitIfPending: this.flags.waitForValidations, }); if (this.git.hasUncommittedChanges()) {