diff --git a/lib/workers/branch/index.spec.ts b/lib/workers/branch/index.spec.ts index 28fa8400d3397ff1a980cb1221bce873e161ec23..d24742e00e8170d84013b31e28eaa03ecdfe1e7e 100644 --- a/lib/workers/branch/index.spec.ts +++ b/lib/workers/branch/index.spec.ts @@ -451,7 +451,7 @@ describe(getName(__filename), () => { prResult: PrResult.Created, pr: {}, } as EnsurePrResult); - prAutomerge.checkAutoMerge.mockResolvedValueOnce(true); + prAutomerge.checkAutoMerge.mockResolvedValueOnce({ automerged: true }); commit.commitFilesToBranch.mockResolvedValueOnce(null); await branchWorker.processBranch({ ...config, automerge: true }); expect(prWorker.ensurePr).toHaveBeenCalledTimes(1); @@ -472,7 +472,7 @@ describe(getName(__filename), () => { prResult: PrResult.Created, pr: {}, } as EnsurePrResult); - prAutomerge.checkAutoMerge.mockResolvedValueOnce(true); + prAutomerge.checkAutoMerge.mockResolvedValueOnce({ automerged: true }); commit.commitFilesToBranch.mockResolvedValueOnce(null); await branchWorker.processBranch(config); expect(platform.ensureComment).toHaveBeenCalledTimes(1); @@ -493,7 +493,7 @@ describe(getName(__filename), () => { prResult: PrResult.Created, pr: {}, } as EnsurePrResult); - prAutomerge.checkAutoMerge.mockResolvedValueOnce(true); + prAutomerge.checkAutoMerge.mockResolvedValueOnce({ automerged: true }); config.releaseTimestamp = '2018-04-26T05:15:51.877Z'; commit.commitFilesToBranch.mockResolvedValueOnce(null); await branchWorker.processBranch(config); @@ -515,7 +515,7 @@ describe(getName(__filename), () => { prResult: PrResult.Created, pr: {}, } as EnsurePrResult); - prAutomerge.checkAutoMerge.mockResolvedValueOnce(true); + prAutomerge.checkAutoMerge.mockResolvedValueOnce({ automerged: true }); config.releaseTimestamp = new Date().toISOString(); commit.commitFilesToBranch.mockResolvedValueOnce(null); await branchWorker.processBranch(config); @@ -537,7 +537,7 @@ describe(getName(__filename), () => { prResult: PrResult.Created, pr: {}, } as EnsurePrResult); - prAutomerge.checkAutoMerge.mockResolvedValueOnce(true); + prAutomerge.checkAutoMerge.mockResolvedValueOnce({ automerged: true }); config.releaseTimestamp = new Date().toISOString(); await expect(branchWorker.processBranch(config)).rejects.toThrow( Error(MANAGER_LOCKFILE_ERROR) @@ -558,7 +558,7 @@ describe(getName(__filename), () => { prResult: PrResult.Created, pr: {}, } as EnsurePrResult); - prAutomerge.checkAutoMerge.mockResolvedValueOnce(true); + prAutomerge.checkAutoMerge.mockResolvedValueOnce({ automerged: true }); commit.commitFilesToBranch.mockResolvedValueOnce(null); await branchWorker.processBranch(config); expect(platform.ensureComment).toHaveBeenCalledTimes(1); diff --git a/lib/workers/branch/index.ts b/lib/workers/branch/index.ts index e84774dcbd88750f5dfe223d3c9ef8eef23976b5..d2b2576b366896f28ebd2ded77042240b4f5111e 100644 --- a/lib/workers/branch/index.ts +++ b/lib/workers/branch/index.ts @@ -601,8 +601,8 @@ export async function processBranch( } } else if (config.automerge) { logger.debug('PR is configured for automerge'); - const prAutomerged = await checkAutoMerge(pr, config); - if (prAutomerged) { + const prAutomergeResult = await checkAutoMerge(pr, config); + if (prAutomergeResult?.automerged) { return ProcessBranchResult.Automerged; } } else { diff --git a/lib/workers/pr/__snapshots__/automerge.spec.ts.snap b/lib/workers/pr/__snapshots__/automerge.spec.ts.snap index 424eda4b0896ee07a42f56d42f11abd3e25d5e51..76721cbc3840a36b7392111d754de709a29d5207 100644 --- a/lib/workers/pr/__snapshots__/automerge.spec.ts.snap +++ b/lib/workers/pr/__snapshots__/automerge.spec.ts.snap @@ -1,15 +1,57 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`workers/pr/automerge checkAutoMerge(pr, config) should automerge comment 1`] = `true`; +exports[`workers/pr/automerge checkAutoMerge(pr, config) should automerge comment 1`] = ` +Object { + "automerged": true, + "branchRemoved": false, +} +`; -exports[`workers/pr/automerge checkAutoMerge(pr, config) should automerge if enabled and pr is mergeable 1`] = `true`; +exports[`workers/pr/automerge checkAutoMerge(pr, config) should automerge if enabled and pr is mergeable 1`] = ` +Object { + "automerged": true, + "branchRemoved": true, +} +`; -exports[`workers/pr/automerge checkAutoMerge(pr, config) should not automerge if enabled and pr is mergeable but branch status is not success 1`] = `false`; +exports[`workers/pr/automerge checkAutoMerge(pr, config) should indicate if automerge failed 1`] = ` +Object { + "automerged": false, + "prAutomergeBlockReason": "PlatformRejection", +} +`; -exports[`workers/pr/automerge checkAutoMerge(pr, config) should not automerge if enabled and pr is mergeable but cannot rebase 1`] = `false`; +exports[`workers/pr/automerge checkAutoMerge(pr, config) should not automerge if enabled and pr is mergeable but branch status is not success 1`] = ` +Object { + "automerged": false, + "prAutomergeBlockReason": "BranchNotGreen", +} +`; -exports[`workers/pr/automerge checkAutoMerge(pr, config) should not automerge if enabled and pr is mergeable but unstable 1`] = `false`; +exports[`workers/pr/automerge checkAutoMerge(pr, config) should not automerge if enabled and pr is mergeable but cannot rebase 1`] = ` +Object { + "automerged": false, + "prAutomergeBlockReason": "BranchModified", +} +`; -exports[`workers/pr/automerge checkAutoMerge(pr, config) should not automerge if enabled and pr is unmergeable 1`] = `false`; +exports[`workers/pr/automerge checkAutoMerge(pr, config) should not automerge if enabled and pr is mergeable but unstable 1`] = ` +Object { + "automerged": false, + "prAutomergeBlockReason": "PlatformNotReady", +} +`; -exports[`workers/pr/automerge checkAutoMerge(pr, config) should remove previous automerge comment when rebasing 1`] = `true`; +exports[`workers/pr/automerge checkAutoMerge(pr, config) should not automerge if enabled and pr is unmergeable 1`] = ` +Object { + "automerged": false, + "prAutomergeBlockReason": "Conflicted", +} +`; + +exports[`workers/pr/automerge checkAutoMerge(pr, config) should remove previous automerge comment when rebasing 1`] = ` +Object { + "automerged": true, + "branchRemoved": false, +} +`; diff --git a/lib/workers/pr/automerge.spec.ts b/lib/workers/pr/automerge.spec.ts index 3e4f2d72b1edfb3adc828505ac109490f9b5f9d5..1343997e39c9289bc3ee1f779535d5ca1bd1a226 100644 --- a/lib/workers/pr/automerge.spec.ts +++ b/lib/workers/pr/automerge.spec.ts @@ -32,6 +32,14 @@ describe(getName(__filename), () => { expect(res).toMatchSnapshot(); expect(platform.mergePr).toHaveBeenCalledTimes(1); }); + it('should indicate if automerge failed', async () => { + config.automerge = true; + platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.green); + platform.mergePr.mockResolvedValueOnce(false); + const res = await prAutomerge.checkAutoMerge(pr, config); + expect(res).toMatchSnapshot(); + expect(platform.mergePr).toHaveBeenCalledTimes(1); + }); it('should automerge comment', async () => { config.automerge = true; config.automergeType = 'pr-comment'; diff --git a/lib/workers/pr/automerge.ts b/lib/workers/pr/automerge.ts index 0dabe65dfe326abdd42163d6e93dd1b31c1e3052..d3d86a4ef0f3cc121d5412aebcd292892e6349b4 100644 --- a/lib/workers/pr/automerge.ts +++ b/lib/workers/pr/automerge.ts @@ -5,10 +5,25 @@ import { BranchStatus } from '../../types'; import { deleteBranch, isBranchModified } from '../../util/git'; import { BranchConfig } from '../types'; +export enum PrAutomergeBlockReason { + BranchModified = 'BranchModified', + BranchNotGreen = 'BranchNotGreen', + Conflicted = 'Conflicted', + DryRun = 'DryRun', + PlatformNotReady = 'PlatformNotReady', + PlatformRejection = 'PlatformRejection', +} + +export type AutomergePrResult = { + automerged: boolean; + branchRemoved?: boolean; + prAutomergeBlockReason?: PrAutomergeBlockReason; +}; + export async function checkAutoMerge( pr: Pr, config: BranchConfig -): Promise<boolean> { +): Promise<AutomergePrResult> { logger.trace({ config }, 'checkAutoMerge'); const { branchName, @@ -20,15 +35,20 @@ export async function checkAutoMerge( // Return if PR not ready for automerge if (pr.isConflicted) { logger.debug('PR is conflicted'); - logger.debug({ pr }); - return false; + return { + automerged: false, + prAutomergeBlockReason: PrAutomergeBlockReason.Conflicted, + }; } if (requiredStatusChecks && pr.canMerge !== true) { logger.debug( { canMergeReason: pr.canMergeReason }, 'PR is not ready for merge' ); - return false; + return { + automerged: false, + prAutomergeBlockReason: PrAutomergeBlockReason.PlatformNotReady, + }; } const branchStatus = await platform.getBranchStatus( branchName, @@ -38,12 +58,18 @@ export async function checkAutoMerge( logger.debug( `PR is not ready for merge (branch status is ${branchStatus})` ); - return false; + return { + automerged: false, + prAutomergeBlockReason: PrAutomergeBlockReason.BranchNotGreen, + }; } // Check if it's been touched if (await isBranchModified(branchName)) { logger.debug('PR is ready for automerge but has been modified'); - return false; + return { + automerged: false, + prAutomergeBlockReason: PrAutomergeBlockReason.BranchModified, + }; } if (automergeType === 'pr-comment') { logger.debug(`Applying automerge comment: ${automergeComment}`); @@ -52,7 +78,10 @@ export async function checkAutoMerge( logger.info( `DRY-RUN: Would add PR automerge comment to PR #${pr.number}` ); - return false; + return { + automerged: false, + prAutomergeBlockReason: PrAutomergeBlockReason.DryRun, + }; } if (rebaseRequested) { await platform.ensureCommentRemoval({ @@ -60,27 +89,37 @@ export async function checkAutoMerge( content: automergeComment, }); } - return platform.ensureComment({ + await platform.ensureComment({ number: pr.number, topic: null, content: automergeComment, }); + return { automerged: true, branchRemoved: false }; } // Let's merge this - logger.debug(`Automerging #${pr.number}`); // istanbul ignore if if (getAdminConfig().dryRun) { logger.info(`DRY-RUN: Would merge PR #${pr.number}`); - return false; + return { + automerged: false, + prAutomergeBlockReason: PrAutomergeBlockReason.DryRun, + }; } + logger.debug(`Automerging #${pr.number}`); const res = await platform.mergePr(pr.number, branchName); if (res) { logger.info({ pr: pr.number, prTitle: pr.title }, 'PR automerged'); + let branchRemoved = false; try { await deleteBranch(branchName); + branchRemoved = true; } catch (err) /* istanbul ignore next */ { logger.warn({ branchName, err }, 'Branch auto-remove failed'); } + return { automerged: true, branchRemoved }; } - return res; + return { + automerged: false, + prAutomergeBlockReason: PrAutomergeBlockReason.PlatformRejection, + }; }