diff --git a/ng-dev/release/publish/actions.ts b/ng-dev/release/publish/actions.ts index c087e5770..6ff982397 100644 --- a/ng-dev/release/publish/actions.ts +++ b/ng-dev/release/publish/actions.ts @@ -744,16 +744,24 @@ export abstract class ReleaseAction { branch: string, version: semver.SemVer, previousSha: string, - isRetry = false, ): Promise { - try { + let latestSha: string | null = null; + + // Support re-checking as many times as needed. Often times the GitHub API + // is not up-to-date and we don't want to exit the release script then. + while (latestSha === null) { const commit = await this.getLatestCommitOfBranch(branch); + // Ensure the latest commit in the publish branch is the bump commit. if (!commit.commit.message.startsWith(getCommitMessageForRelease(version))) { /** The shortened sha of the commit for usage in the error message. */ const sha = commit.sha.slice(0, 8); Log.error(` ✘ Latest commit (${sha}) in "${branch}" branch is not a staging commit.`); Log.error(' Please make sure the staging pull request has been merged.'); + + if (await Prompt.confirm({message: `Do you want to re-try?`, default: true})) { + continue; + } throw new FatalReleaseActionError(); } @@ -762,16 +770,17 @@ export abstract class ReleaseAction { if (commit.parents[0].sha !== previousSha) { Log.error(` ✘ Unexpected additional commits have landed while staging the release.`); Log.error(' Please revert the bump commit and retry, or cut a new version on top.'); + + if (await Prompt.confirm({message: `Do you want to re-try?`, default: true})) { + continue; + } throw new FatalReleaseActionError(); } - return commit.sha; - } catch (e: unknown) { - if (isRetry) { - throw e; - } - return this._getAndValidateLatestCommitForPublishing(branch, version, previousSha, true); + latestSha = commit.sha; } + + return latestSha; } // TODO: Remove this check and run it as part of common release validation. diff --git a/ng-dev/release/publish/test/common.spec.ts b/ng-dev/release/publish/test/common.spec.ts index f4d86faa0..f1517c3c1 100644 --- a/ng-dev/release/publish/test/common.spec.ts +++ b/ng-dev/release/publish/test/common.spec.ts @@ -358,6 +358,61 @@ describe('common release action logic', () => { ); }); + it('should allow for re-trying when GitHub API is not reflecting latest commit', async () => { + const {repo, instance, builtPackagesWithInfo, promptConfirmSpy} = + setupReleaseActionForTesting(DelegateTestAction, baseReleaseTrains); + const {version, branchName} = baseReleaseTrains.latest; + const tagName = version.format(); + + repo + .expectBranchRequest(branchName, { + sha: 'THE_ONE_BEFORE_STAGING_SHA', + parents: [{sha: 'bla'}], + commit: {message: `build: some other change`}, + }) + .expectTagToBeCreated(tagName, 'STAGING_SHA') + .expectReleaseToBeCreated(version.toString(), tagName, true); + + spyOn(Log, 'error'); + + const promptPending = withResolverPromise(); + const promptResolveFns: Array<(val: boolean) => void> = []; + promptConfirmSpy.and.callFake(() => { + promptPending.resolve(); + return new Promise((resolve) => promptResolveFns.push(resolve)); + }); + + const publishProcess = instance.testPublish( + builtPackagesWithInfo, + version, + branchName, + 'BEFORE_STAGING_SHA', + 'latest', + ); + + // Wait for re-try prompt. + await promptPending.promise; + + // Re-mock the branch request, with the correct info now. + repo.expectBranchRequest(branchName, { + sha: 'STAGING_SHA', + parents: [{sha: 'BEFORE_STAGING_SHA'}], + commit: {message: `release: cut the v${version} release`}, + }); + + // Reset logs to see if we pass gracefully then. + (Log.error as unknown as jasmine.Spy).calls.reset(); + + // Kick off the re-try. + promptResolveFns.shift()!(true); + + // See if it completes now, without errors. + expect(Log.error).toHaveBeenCalledTimes(0); + + // Ensure publish finishes completely and gracefully. + await publishProcess; + }); + it('should ensure that no new changes have landed after release staging has started', async () => { const {repo, instance, builtPackagesWithInfo} = setupReleaseActionForTesting( DelegateTestAction, @@ -546,3 +601,19 @@ class MockReleaseNotes extends ReleaseNotes { super(ngDevConfig, version, commits, git); } } + +// Polyfill for `Promise.withResolvers`. +function withResolverPromise(): { + promise: Promise; + resolve: (v: T) => void; + reject: (v: unknown) => void; +} { + let resolve!: (v: T) => void; + let reject!: (v: unknown) => void; + + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return {promise, resolve, reject}; +} diff --git a/ng-dev/utils/prompt.ts b/ng-dev/utils/prompt.ts index b81ab2a1a..69167ec6d 100644 --- a/ng-dev/utils/prompt.ts +++ b/ng-dev/utils/prompt.ts @@ -13,11 +13,11 @@ import {confirm, input, checkbox, select, editor} from '@inquirer/prompts'; * class to allow easier mocking management in test environments. */ export class Prompt { - static confirm: typeof confirm = ( + static confirm = ( // These are extractions of the PromptConfig from the inquirer types. _config: Parameters[0], - _context: Parameters[1], - ) => { + _context?: Parameters[1], + ): Promise => { /** Config to use when creating a confirm prompt, changes the default to `false` instead of `true`. */ const config = { default: false,