From 913d0cf20842abb89e631f7043e360d70f2135e6 Mon Sep 17 00:00:00 2001 From: Rhys Arkins <rhys@keylocation.sg> Date: Thu, 5 Oct 2017 09:31:10 +0200 Subject: [PATCH] feat: create PRs if branch automerge failed (#888) --- lib/workers/branch/automerge.js | 7 ++++--- lib/workers/branch/index.js | 6 ++++-- lib/workers/pr/index.js | 2 +- test/workers/branch/automerge.spec.js | 10 +++++----- test/workers/branch/index.spec.js | 4 ++-- test/workers/pr/index.spec.js | 11 ++++++++++- 6 files changed, 26 insertions(+), 14 deletions(-) diff --git a/lib/workers/branch/automerge.js b/lib/workers/branch/automerge.js index c0acb0460e..ab43531349 100644 --- a/lib/workers/branch/automerge.js +++ b/lib/workers/branch/automerge.js @@ -6,7 +6,7 @@ async function tryBranchAutomerge(config) { const { logger } = config; logger.debug('Checking if we can automerge branch'); if (!config.automerge || config.automergeType === 'pr') { - return false; + return 'no automerge'; } const branchStatus = await config.api.getBranchStatus( config.branchName, @@ -16,12 +16,13 @@ async function tryBranchAutomerge(config) { logger.info(`Automerging branch`); try { await config.api.mergeBranch(config.branchName, config.automergeType); - return true; // Branch no longer exists + return 'automerged'; // Branch no longer exists } catch (err) { logger.info({ err }, `Failed to automerge branch`); + return 'failed'; } } else { logger.debug(`Branch status is "${branchStatus}" - skipping automerge`); } - return false; + return 'no automerge'; } diff --git a/lib/workers/branch/index.js b/lib/workers/branch/index.js index ef803acd49..1ea8aa0bf4 100644 --- a/lib/workers/branch/index.js +++ b/lib/workers/branch/index.js @@ -75,10 +75,12 @@ async function processBranch(branchConfig) { // Try to automerge branch and finish if successful logger.debug('Checking if we should automerge the branch'); - const branchMerged = await tryBranchAutomerge(config); - if (branchMerged) { + const mergeStatus = await tryBranchAutomerge(config); + if (mergeStatus === 'automerged') { logger.debug('Branch is automerged - returning'); return 'automerged'; + } else if (mergeStatus === 'failed') { + config.forcePr = true; } } catch (err) { logger.error({ err }, `Error updating branch: ${err.message}`); diff --git a/lib/workers/pr/index.js b/lib/workers/pr/index.js index 082e6edf49..6d9db42e51 100644 --- a/lib/workers/pr/index.js +++ b/lib/workers/pr/index.js @@ -28,7 +28,7 @@ async function ensurePr(prConfig) { logger.debug( `Branch is configured for branch automerge, branchStatus is: ${branchStatus}` ); - if (branchStatus === 'failure') { + if (config.forcePr || branchStatus === 'failure') { logger.debug(`Branch tests failed, so will create PR`); } else { return null; diff --git a/test/workers/branch/automerge.spec.js b/test/workers/branch/automerge.spec.js index f957ea1131..114a96f1f6 100644 --- a/test/workers/branch/automerge.spec.js +++ b/test/workers/branch/automerge.spec.js @@ -14,18 +14,18 @@ describe('workers/branch/automerge', () => { }); it('returns false if not configured for automerge', async () => { config.automerge = false; - expect(await tryBranchAutomerge(config)).toBe(false); + expect(await tryBranchAutomerge(config)).toBe('no automerge'); }); it('returns false if automergType is pr', async () => { config.automerge = true; config.automergeType = 'pr'; - expect(await tryBranchAutomerge(config)).toBe(false); + expect(await tryBranchAutomerge(config)).toBe('no automerge'); }); it('returns false if branch status is not success', async () => { config.automerge = true; config.automergeType = 'branch-push'; config.api.getBranchStatus.mockReturnValueOnce('pending'); - expect(await tryBranchAutomerge(config)).toBe(false); + expect(await tryBranchAutomerge(config)).toBe('no automerge'); }); it('returns false if automerge fails', async () => { config.automerge = true; @@ -34,13 +34,13 @@ describe('workers/branch/automerge', () => { config.api.mergeBranch.mockImplementationOnce(() => { throw new Error('merge error'); }); - expect(await tryBranchAutomerge(config)).toBe(false); + expect(await tryBranchAutomerge(config)).toBe('failed'); }); it('returns true if automerge succeeds', async () => { config.automerge = true; config.automergeType = 'branch-push'; config.api.getBranchStatus.mockReturnValueOnce('success'); - expect(await tryBranchAutomerge(config)).toBe(true); + expect(await tryBranchAutomerge(config)).toBe('automerged'); }); }); }); diff --git a/test/workers/branch/index.spec.js b/test/workers/branch/index.spec.js index b1d8d10bc1..eda5c4ed22 100644 --- a/test/workers/branch/index.spec.js +++ b/test/workers/branch/index.spec.js @@ -64,7 +64,7 @@ describe('workers/branch', () => { updatedLockFiles: [{}], }); config.api.branchExists.mockReturnValueOnce(true); - automerge.tryBranchAutomerge.mockReturnValueOnce(true); + automerge.tryBranchAutomerge.mockReturnValueOnce('automerged'); await branchWorker.processBranch(config); expect(statusChecks.setUnpublishable.mock.calls).toHaveLength(1); expect(automerge.tryBranchAutomerge.mock.calls).toHaveLength(1); @@ -77,7 +77,7 @@ describe('workers/branch', () => { updatedLockFiles: [{}], }); config.api.branchExists.mockReturnValueOnce(true); - automerge.tryBranchAutomerge.mockReturnValueOnce(false); + automerge.tryBranchAutomerge.mockReturnValueOnce('failed'); prWorker.ensurePr.mockReturnValueOnce({}); prWorker.checkAutoMerge.mockReturnValueOnce(true); await branchWorker.processBranch(config); diff --git a/test/workers/pr/index.spec.js b/test/workers/pr/index.spec.js index 0d6b9671fe..4c88bd3588 100644 --- a/test/workers/pr/index.spec.js +++ b/test/workers/pr/index.spec.js @@ -291,7 +291,7 @@ describe('workers/pr', () => { const pr = await prWorker.ensurePr(config); expect(pr).toMatchSnapshot(); }); - it('should create PR if branch automerging failed', async () => { + it('should create PR if branch tests failed', async () => { config.automerge = true; config.automergeType = 'branch-push'; config.api.getBranchStatus.mockReturnValueOnce('failure'); @@ -299,6 +299,15 @@ describe('workers/pr', () => { const pr = await prWorker.ensurePr(config); expect(pr).toMatchObject({ displayNumber: 'New Pull Request' }); }); + it('should create PR if branch automerging failed', async () => { + config.automerge = true; + config.automergeType = 'branch-push'; + config.api.getBranchStatus.mockReturnValueOnce('success'); + config.forcePr = true; + config.api.getBranchPr = jest.fn(); + const pr = await prWorker.ensurePr(config); + expect(pr).toMatchObject({ displayNumber: 'New Pull Request' }); + }); it('should return null if branch automerging not failed', async () => { config.automerge = true; config.automergeType = 'branch-push'; -- GitLab