From fa2bda45fe3b88a4f7c8e66a13e136c0471c5680 Mon Sep 17 00:00:00 2001 From: Rhys Arkins Date: Fri, 16 Mar 2018 10:40:07 +0100 Subject: [PATCH] fix: delete lockFileMaintenance branch if no longer necessary If a lockFileMaintenance branch returns no updated lockfiles then we should delete it. Closes #1655 --- lib/workers/branch/index.js | 6 ++++++ lib/workers/repository/cleanup.js | 18 ++++++++++++++---- lib/workers/repository/write.js | 9 +++++++++ test/workers/branch/index.spec.js | 12 ++++++++++++ test/workers/repository/cleanup.spec.js | 12 ++++++++++++ test/workers/repository/write.spec.js | 5 +++-- 6 files changed, 56 insertions(+), 6 deletions(-) diff --git a/lib/workers/branch/index.js b/lib/workers/branch/index.js index 16ff277b07..2a6d29bc36 100644 --- a/lib/workers/branch/index.js +++ b/lib/workers/branch/index.js @@ -141,6 +141,12 @@ async function processBranch(branchConfig) { } const committedFiles = await commitFilesToBranch(config); + if (config.type === 'lockFileMaintenance' && !config.committedFiles) { + logger.debug( + 'Deleting lock file maintenance branch as master lock file no longer needs updating' + ); + return 'delete'; + } if (!(committedFiles || branchExists)) { return 'no-work'; } diff --git a/lib/workers/repository/cleanup.js b/lib/workers/repository/cleanup.js index 58ee07637a..e3adeeb3b1 100644 --- a/lib/workers/repository/cleanup.js +++ b/lib/workers/repository/cleanup.js @@ -4,15 +4,22 @@ module.exports = { async function pruneStaleBranches(config) { // TODO: try/catch - const { branchList } = config; + let { branchList } = config; + const deletedBranches = config.deletedBranches || []; logger.debug('Removing any stale branches'); logger.trace({ config }, `pruneStaleBranches`); logger.debug(`config.repoIsOnboarded=${config.repoIsOnboarded}`); - if (!config.branchList) { + if (!branchList) { logger.debug('No branchList'); return; } - logger.debug({ branchList }, 'branchList'); + logger.debug( + { branchList, deletedBranches }, + 'branchList and deletedBranches' + ); + branchList = branchList.filter( + branchName => !deletedBranches.includes(branchName) + ); let renovateBranches = await platform.getAllRenovateBranches( config.branchPrefix ); @@ -22,7 +29,10 @@ async function pruneStaleBranches(config) { } logger.debug({ branchList, renovateBranches }); const lockFileBranch = `${config.branchPrefix}lock-file-maintenance`; - if (renovateBranches.includes(lockFileBranch)) { + if ( + renovateBranches.includes(lockFileBranch) && + !deletedBranches.includes(lockFileBranch) + ) { logger.debug('Checking lock file branch'); const pr = await platform.getBranchPr(lockFileBranch); if (pr && pr.isUnmergeable) { diff --git a/lib/workers/repository/write.js b/lib/workers/repository/write.js index 109c7272a8..0fef225a26 100644 --- a/lib/workers/repository/write.js +++ b/lib/workers/repository/write.js @@ -49,6 +49,8 @@ async function writeUpdates(config) { prsRemaining < concurrentRemaining ? prsRemaining : concurrentRemaining; } try { + // eslint-disable-next-line no-param-reassign + config.deletedBranches = []; for (const branch of branches) { const res = await branchWorker.processBranch({ ...branch, @@ -59,6 +61,13 @@ async function writeUpdates(config) { // Stop procesing other branches because base branch has been changed return res; } + if (res === 'delete') { + logger.debug( + { branch: branch.branchName }, + 'Adding branch to deletedBranches list' + ); + config.deletedBranches.push(branch.branchName); + } prsRemaining -= res === 'pr-created' ? 1 : 0; } return 'done'; diff --git a/test/workers/branch/index.spec.js b/test/workers/branch/index.spec.js index 5a50498345..6d684b2ab7 100644 --- a/test/workers/branch/index.spec.js +++ b/test/workers/branch/index.spec.js @@ -142,6 +142,18 @@ describe('workers/branch', () => { platform.branchExists.mockReturnValueOnce(false); expect(await branchWorker.processBranch(config)).toEqual('no-work'); }); + it('returns delete if existing lock file maintenace is pointless', async () => { + manager.getUpdatedPackageFiles.mockReturnValueOnce({ + updatedPackageFiles: [], + }); + lockFiles.getUpdatedLockFiles.mockReturnValueOnce({ + lockFileError: false, + updatedLockFiles: [], + }); + config.type = 'lockFileMaintenance'; + platform.branchExists.mockReturnValueOnce(false); + expect(await branchWorker.processBranch(config)).toEqual('delete'); + }); it('returns if branch automerged', async () => { manager.getUpdatedPackageFiles.mockReturnValueOnce({ updatedPackageFiles: [{}], diff --git a/test/workers/repository/cleanup.spec.js b/test/workers/repository/cleanup.spec.js index 4f2e132797..1d77b4ee84 100644 --- a/test/workers/repository/cleanup.spec.js +++ b/test/workers/repository/cleanup.spec.js @@ -51,6 +51,18 @@ describe('workers/repository/cleanup', () => { expect(platform.getAllRenovateBranches.mock.calls).toHaveLength(1); expect(platform.deleteBranch.mock.calls).toHaveLength(1); }); + it('deletes lock file maintenance if should be deleted', async () => { + config.branchList = ['renovate/lock-file-maintenance']; + config.deletedBranches = ['renovate/lock-file-maintenance']; + platform.getAllRenovateBranches.mockReturnValueOnce([ + 'renovate/lock-file-maintenance', + ]); + await cleanup.pruneStaleBranches(config, [ + 'renovate/lock-file-maintenance', + ]); + expect(platform.getAllRenovateBranches.mock.calls).toHaveLength(1); + expect(platform.deleteBranch.mock.calls).toHaveLength(1); + }); it('calls delete only once', async () => { config.branchList = ['renovate/lock-file-maintenance']; platform.getAllRenovateBranches.mockReturnValueOnce([ diff --git a/test/workers/repository/write.spec.js b/test/workers/repository/write.spec.js index f695164caa..00f8b604c0 100644 --- a/test/workers/repository/write.spec.js +++ b/test/workers/repository/write.spec.js @@ -45,12 +45,13 @@ describe('workers/repository/write', () => { expect(branchWorker.processBranch.mock.calls).toHaveLength(1); }); it('stops after automerge', async () => { - config.branches = [{}, {}, {}]; + config.branches = [{}, {}, {}, {}]; branchWorker.processBranch.mockReturnValueOnce('created'); + branchWorker.processBranch.mockReturnValueOnce('delete'); branchWorker.processBranch.mockReturnValueOnce('automerged'); const res = await writeUpdates(config); expect(res).toEqual('automerged'); - expect(branchWorker.processBranch.mock.calls).toHaveLength(2); + expect(branchWorker.processBranch.mock.calls).toHaveLength(3); }); }); });