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

@@ -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';
     }

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) {

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';

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: [{}],

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([

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);
     });
   });
 });