diff --git a/src/lib/cherrypickAndCreateTargetPullRequest/autoMergeNowOrLater.ts b/src/lib/cherrypickAndCreateTargetPullRequest/autoMergeNowOrLater.ts index f3c4df44..11398a60 100644 --- a/src/lib/cherrypickAndCreateTargetPullRequest/autoMergeNowOrLater.ts +++ b/src/lib/cherrypickAndCreateTargetPullRequest/autoMergeNowOrLater.ts @@ -1,7 +1,10 @@ import { ValidConfigOptions } from '../../options/options'; import { mergePullRequest } from '../github/v3/mergePullRequest'; import { GithubV4Exception } from '../github/v4/apiRequestV4'; -import { enablePullRequestAutoMerge } from '../github/v4/enablePullRequestAutoMerge'; +import { + enablePullRequestAutoMerge, + parseGithubError, +} from '../github/v4/enablePullRequestAutoMerge'; import { logger } from '../logger'; import { ora } from '../ora'; @@ -28,38 +31,18 @@ export async function autoMergeNowOrLater( } logger.info( - `Auto merge: Could not enable auto merge for PR "#${pullNumber}" due to ${e.message}` + `Auto merge: Failed to enable auto merge for PR "#${pullNumber}" due to ${e.message}` ); - const isMissingStatusChecks = e.githubResponse.data.errors?.some( - (e) => - e.type === 'UNPROCESSABLE' && - e.message.includes( - 'Branch does not have required protected branch rules' - ) - ); - - // if auto merge cannot be enabled due to missing status checks, the PR should be merged immediately + const { isMissingStatusChecks } = parseGithubError(e); if (!isMissingStatusChecks) { throw e; } - try { - await mergePullRequest(options, pullNumber); - spinner.text = - 'Auto-merge: Pull request was merged without waiting for status checks'; - } catch (e) { - if (!(e instanceof Error)) { - throw new Error(`Unknown error: ${e}`); - } - - logger.error( - `Auto merge: Could not merge PR "#${pullNumber}" immediately due to ${e.message}`, - e - ); - - throw e; - } + // if auto merge cannot be enabled due to missing status checks, the PR should be merged immediately + logger.info('Auto merge: Attempting to merge immediately'); + await mergePullRequest(options, pullNumber); + spinner.text = 'Auto-merge: Pull request was merged immediately'; } spinner.succeed(); diff --git a/src/lib/github/v4/enablePullRequestAutoMerge.mutation.test.ts b/src/lib/github/v4/enablePullRequestAutoMerge.mutation.test.ts index a43ed222..d9b5d96d 100644 --- a/src/lib/github/v4/enablePullRequestAutoMerge.mutation.test.ts +++ b/src/lib/github/v4/enablePullRequestAutoMerge.mutation.test.ts @@ -3,8 +3,12 @@ import { Octokit } from '@octokit/rest'; import { ValidConfigOptions } from '../../../options/options'; import { getDevAccessToken } from '../../../test/private/getDevAccessToken'; import { createPullRequest, PullRequestPayload } from '../v3/createPullRequest'; +import { GithubV4Exception } from './apiRequestV4'; import { disablePullRequestAutoMerge } from './disablePullRequestAutoMerge'; -import { enablePullRequestAutoMerge } from './enablePullRequestAutoMerge'; +import { + enablePullRequestAutoMerge, + parseGithubError, +} from './enablePullRequestAutoMerge'; import { fetchPullRequestAutoMergeMethod } from './fetchPullRequestAutoMergeMethod'; // The test repo requires auto-merge being enabled in options, as well as all merge types enabled (merge, squash, rebase) @@ -29,7 +33,13 @@ function resetReference(octokit: Octokit) { }); } -async function closePr(octokit: Octokit, pullNumber: number) { +async function closePr({ + octokit, + pullNumber, +}: { + octokit: Octokit; + pullNumber: number; +}) { await octokit.pulls.update({ owner: TEST_REPO_OWNER, repo: TEST_REPO_NAME, @@ -38,14 +48,18 @@ async function closePr(octokit: Octokit, pullNumber: number) { }); } -async function createPr( - options: ValidConfigOptions, - branchName: string, - baseBranch: string -) { +async function createPr({ + options, + headBranch, + baseBranch, +}: { + options: ValidConfigOptions; + headBranch: string; + baseBranch: string; +}) { const prPayload: PullRequestPayload = { base: baseBranch, - head: branchName, + head: headBranch, body: 'testing...', owner: TEST_REPO_OWNER, repo: TEST_REPO_NAME, @@ -56,7 +70,13 @@ async function createPr( return number; } -async function deleteBranch(octokit: Octokit, branchName: string) { +async function deleteBranch({ + octokit, + branchName, +}: { + octokit: Octokit; + branchName: string; +}) { await octokit.git.deleteRef({ owner: TEST_REPO_OWNER, repo: TEST_REPO_NAME, @@ -64,7 +84,15 @@ async function deleteBranch(octokit: Octokit, branchName: string) { }); } -async function createBranch(octokit: Octokit, branchName: string, sha: string) { +async function createBranch({ + octokit, + branchName, + sha, +}: { + octokit: Octokit; + branchName: string; + sha: string; +}) { await octokit.git.createRef({ owner: TEST_REPO_OWNER, repo: TEST_REPO_NAME, @@ -94,7 +122,7 @@ async function addCommit(octokit: Octokit) { } describe('enablePullRequestAutoMerge', () => { - describe('create a PR for 7.x which has status checks', () => { + describe('create a PR and enable auto-merge against "approvals-required-branch"', () => { let pullNumber: number; let branchName: string; let octokit: Octokit; @@ -114,14 +142,18 @@ describe('enablePullRequestAutoMerge', () => { await resetReference(octokit); const sha = await addCommit(octokit); - await createBranch(octokit, branchName, sha); - pullNumber = await createPr(options, branchName, '7.x'); + await createBranch({ octokit, branchName, sha }); + pullNumber = await createPr({ + options, + headBranch: branchName, + baseBranch: 'approvals-required-branch', + }); }); // cleanup afterAll(async () => { - await closePr(octokit, pullNumber); - await deleteBranch(octokit, branchName); + await closePr({ octokit, pullNumber }); + await deleteBranch({ octokit, branchName }); await resetReference(octokit); }); @@ -154,14 +186,24 @@ describe('enablePullRequestAutoMerge', () => { expect(autoMergeMethod).toBe('MERGE'); }); - it('should not enable auto-merge via rebase because it is disallowed', async () => { - await expect( - async () => - await enablePullRequestAutoMerge( - { ...options, autoMergeMethod: 'rebase' }, - pullNumber - ) - ).rejects.toThrowErrorMatchingInlineSnapshot( + it('should fail when enabling auto-merge via rebase because it is disallowed', async () => { + let errorMessage; + let isMissingStatusChecks; + + try { + await enablePullRequestAutoMerge( + { ...options, autoMergeMethod: 'rebase' }, + pullNumber + ); + } catch (e) { + const err = e as GithubV4Exception; + const res = parseGithubError(err); + errorMessage = err.message; + isMissingStatusChecks = res.isMissingStatusChecks; + } + + expect(isMissingStatusChecks).toBe(false); + expect(errorMessage).toMatchInlineSnapshot( `"[\\"Merge method rebase merging is not allowed on this repository\\"] (Github API v4)"` ); @@ -192,7 +234,64 @@ describe('enablePullRequestAutoMerge', () => { }); }); - describe('create a PR for 6.x which does not have status checks', () => { + describe('when creating a PR against "no-checks-required-branch"', () => { + let pullNumber: number; + let branchName: string; + let octokit: Octokit; + let options: ValidConfigOptions; + + beforeAll(async () => { + const randomString = crypto.randomBytes(4).toString('hex'); + branchName = `test-${randomString}`; + + options = { + accessToken, + repoOwner: TEST_REPO_OWNER, + repoName: TEST_REPO_NAME, + } as ValidConfigOptions; + + octokit = new Octokit({ auth: accessToken }); + await resetReference(octokit); + + const sha = await addCommit(octokit); + await createBranch({ octokit, branchName, sha }); + pullNumber = await createPr({ + options, + headBranch: branchName, + baseBranch: 'no-checks-required-branch', + }); + }); + + // cleanup + afterAll(async () => { + await closePr({ octokit, pullNumber }); + await deleteBranch({ octokit, branchName }); + await resetReference(octokit); + }); + + it('should not be possible to enable auto-merge', async () => { + let isMissingStatusChecks; + let errorMessage; + try { + await enablePullRequestAutoMerge( + { ...options, autoMergeMethod: 'merge' }, + pullNumber + ); + } catch (e) { + const err = e as GithubV4Exception; + const res = parseGithubError(err); + errorMessage = err.message; + isMissingStatusChecks = res.isMissingStatusChecks; + } + + expect(errorMessage).toMatchInlineSnapshot( + `"[\\"Pull request Pull request is in clean status\\"] (Github API v4)"` + ); + expect(isMissingStatusChecks).toBe(true); + }); + }); + + describe('when createing a PR against "status-checks-required-branch"', () => { let pullNumber: number; let branchName: string; let octokit: Octokit; @@ -212,27 +311,41 @@ describe('enablePullRequestAutoMerge', () => { await resetReference(octokit); const sha = await addCommit(octokit); - await createBranch(octokit, branchName, sha); - pullNumber = await createPr(options, branchName, '6.x'); + await createBranch({ octokit, branchName, sha }); + pullNumber = await createPr({ + options, + headBranch: branchName, + baseBranch: 'status-checks-required-branch', + }); }); // cleanup afterAll(async () => { - await closePr(octokit, pullNumber); - await deleteBranch(octokit, branchName); + await closePr({ octokit, pullNumber }); + await deleteBranch({ octokit, branchName }); await resetReference(octokit); }); - it('should not be able to enable auto-merge via merge', async () => { - await expect( - async () => - await enablePullRequestAutoMerge( - { ...options, autoMergeMethod: 'merge' }, - pullNumber - ) - ).rejects.toThrowErrorMatchingInlineSnapshot( + it('should not be possible to enable auto-merge', async () => { + let isMissingStatusChecks; + let errorMessage; + + try { + await enablePullRequestAutoMerge( + { ...options, autoMergeMethod: 'merge' }, + pullNumber + ); + } catch (e) { + const err = e as GithubV4Exception; + const res = parseGithubError(err); + errorMessage = err.message; + isMissingStatusChecks = res.isMissingStatusChecks; + } + + expect(errorMessage).toMatchInlineSnapshot( `"[\\"Pull request Pull request is in clean status\\"] (Github API v4)"` ); + expect(isMissingStatusChecks).toBe(true); }); }); }); diff --git a/src/lib/github/v4/enablePullRequestAutoMerge.ts b/src/lib/github/v4/enablePullRequestAutoMerge.ts index 8665a01c..269da456 100644 --- a/src/lib/github/v4/enablePullRequestAutoMerge.ts +++ b/src/lib/github/v4/enablePullRequestAutoMerge.ts @@ -1,7 +1,7 @@ import gql from 'graphql-tag'; import { ValidConfigOptions } from '../../../options/options'; import { fetchPullRequestId } from './FetchPullRequestId'; -import { apiRequestV4 } from './apiRequestV4'; +import { apiRequestV4, GithubV4Exception } from './apiRequestV4'; interface Response { enablePullRequestAutoMerge: { pullRequest?: { number: number } }; @@ -49,3 +49,16 @@ export async function enablePullRequestAutoMerge( return res.enablePullRequestAutoMerge.pullRequest?.number; } + +export function parseGithubError(e: GithubV4Exception) { + const isMissingStatusChecks = e.githubResponse.data.errors?.some( + (e) => + e.type === 'UNPROCESSABLE' && + (e.message.includes( + 'Branch does not have required protected branch rules' + ) || + e.message.includes('Pull request is in clean status')) + ); + + return { isMissingStatusChecks }; +}