From 93547f12dfa8c1a5b1d4aaf640a4b8cf66638eac Mon Sep 17 00:00:00 2001
From: Rhys Arkins <rhys@arkins.net>
Date: Sun, 6 May 2018 12:26:21 +0200
Subject: [PATCH] refactor: simplify stale lock file maintenance deletion

---
 lib/workers/branch/index.js             |  4 +++-
 lib/workers/repository/cleanup.js       | 16 +++-------------
 lib/workers/repository/write.js         |  8 --------
 test/workers/branch/index.spec.js       | 12 ------------
 test/workers/repository/cleanup.spec.js | 22 ----------------------
 5 files changed, 6 insertions(+), 56 deletions(-)

diff --git a/lib/workers/branch/index.js b/lib/workers/branch/index.js
index 329be5a6ec..9563c77a6f 100644
--- a/lib/workers/branch/index.js
+++ b/lib/workers/branch/index.js
@@ -153,6 +153,7 @@ async function processBranch(branchConfig) {
     }
 
     const committedFiles = await commitFilesToBranch(config);
+    // istanbul ignore if
     if (
       config.type === 'lockFileMaintenance' &&
       !committedFiles &&
@@ -162,7 +163,8 @@ async function processBranch(branchConfig) {
       logger.info(
         'Deleting lock file maintenance branch as master lock file no longer needs updating'
       );
-      return 'delete';
+      await platform.deleteBranch(config.branchName);
+      return 'done';
     }
     if (!(committedFiles || branchExists)) {
       return 'no-work';
diff --git a/lib/workers/repository/cleanup.js b/lib/workers/repository/cleanup.js
index e3adeeb3b1..213f7612f7 100644
--- a/lib/workers/repository/cleanup.js
+++ b/lib/workers/repository/cleanup.js
@@ -4,8 +4,7 @@ module.exports = {
 
 async function pruneStaleBranches(config) {
   // TODO: try/catch
-  let { branchList } = config;
-  const deletedBranches = config.deletedBranches || [];
+  const { branchList } = config;
   logger.debug('Removing any stale branches');
   logger.trace({ config }, `pruneStaleBranches`);
   logger.debug(`config.repoIsOnboarded=${config.repoIsOnboarded}`);
@@ -13,13 +12,7 @@ async function pruneStaleBranches(config) {
     logger.debug('No branchList');
     return;
   }
-  logger.debug(
-    { branchList, deletedBranches },
-    'branchList and deletedBranches'
-  );
-  branchList = branchList.filter(
-    branchName => !deletedBranches.includes(branchName)
-  );
+  logger.debug({ branchList }, 'branchList');
   let renovateBranches = await platform.getAllRenovateBranches(
     config.branchPrefix
   );
@@ -29,10 +22,7 @@ async function pruneStaleBranches(config) {
   }
   logger.debug({ branchList, renovateBranches });
   const lockFileBranch = `${config.branchPrefix}lock-file-maintenance`;
-  if (
-    renovateBranches.includes(lockFileBranch) &&
-    !deletedBranches.includes(lockFileBranch)
-  ) {
+  if (renovateBranches.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 e548008416..edd05a6d27 100644
--- a/lib/workers/repository/write.js
+++ b/lib/workers/repository/write.js
@@ -50,7 +50,6 @@ async function writeUpdates(config) {
   }
   try {
     // eslint-disable-next-line no-param-reassign
-    config.deletedBranches = [];
     for (const branch of branches) {
       const res = await branchWorker.processBranch({
         ...branch,
@@ -61,13 +60,6 @@ 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 c044913e8f..2826715f4a 100644
--- a/test/workers/branch/index.spec.js
+++ b/test/workers/branch/index.spec.js
@@ -150,18 +150,6 @@ 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(true);
-      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 1d77b4ee84..8ca8169d03 100644
--- a/test/workers/repository/cleanup.spec.js
+++ b/test/workers/repository/cleanup.spec.js
@@ -51,27 +51,5 @@ 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([
-        'renovate/lock-file-maintenance',
-      ]);
-      platform.getBranchPr = jest.fn(() => ({ isUnmergeable: true }));
-      await cleanup.pruneStaleBranches(config, []);
-      expect(platform.getAllRenovateBranches.mock.calls).toHaveLength(1);
-      expect(platform.deleteBranch.mock.calls).toHaveLength(1);
-    });
   });
 });
-- 
GitLab