refactor: separate automerge function in pr

lib/workers/branch/index.spec.ts

@@ -20,6 +20,7 @@ import * as _sanitize from '../../util/sanitize';
 import * as _limits from '../global/limits';
 import * as _prWorker from '../pr';
 import type { EnsurePrResult } from '../pr';
+import * as _prAutomerge from '../pr/automerge';
 import type { Pr } from '../repository/onboarding/branch/check';
 import type { BranchConfig, BranchUpgradeConfig } from '../types';
 import { PrResult, ProcessBranchResult } from '../types';
@@ -40,6 +41,7 @@ jest.mock('../../manager/npm/post-update');
 jest.mock('./automerge');
 jest.mock('./commit');
 jest.mock('../pr');
+jest.mock('../pr/automerge');
 jest.mock('../../util/exec');
 jest.mock('../../util/sanitize');
 jest.mock('../../util/git');
@@ -53,6 +55,7 @@ const reuse = mocked(_reuse);
 const npmPostExtract = mocked(_npmPostExtract);
 const automerge = mocked(_automerge);
 const commit = mocked(_commit);
+const prAutomerge = mocked(_prAutomerge);
 const prWorker = mocked(_prWorker);
 const exec = mocked(_exec);
 const sanitize = mocked(_sanitize);
@@ -69,7 +72,7 @@ describe(getName(__filename), () => {
     let config: BranchConfig;
     beforeEach(() => {
       prWorker.ensurePr = jest.fn();
-      prWorker.checkAutoMerge = jest.fn();
+      prAutomerge.checkAutoMerge = jest.fn();
       config = {
         ...defaultConfig,
         branchName: 'renovate/some-branch',
@@ -448,12 +451,12 @@ describe(getName(__filename), () => {
         prResult: PrResult.Created,
         pr: {},
       } as EnsurePrResult);
-      prWorker.checkAutoMerge.mockResolvedValueOnce(true);
+      prAutomerge.checkAutoMerge.mockResolvedValueOnce(true);
       commit.commitFilesToBranch.mockResolvedValueOnce(null);
       await branchWorker.processBranch(config);
       expect(prWorker.ensurePr).toHaveBeenCalledTimes(1);
       expect(platform.ensureCommentRemoval).toHaveBeenCalledTimes(0);
-      expect(prWorker.checkAutoMerge).toHaveBeenCalledTimes(1);
+      expect(prAutomerge.checkAutoMerge).toHaveBeenCalledTimes(1);
     });
     it('ensures PR and adds lock file error comment if no releaseTimestamp', async () => {
       getUpdated.getUpdatedPackageFiles.mockResolvedValueOnce({
@@ -469,12 +472,12 @@ describe(getName(__filename), () => {
         prResult: PrResult.Created,
         pr: {},
       } as EnsurePrResult);
-      prWorker.checkAutoMerge.mockResolvedValueOnce(true);
+      prAutomerge.checkAutoMerge.mockResolvedValueOnce(true);
       commit.commitFilesToBranch.mockResolvedValueOnce(null);
       await branchWorker.processBranch(config);
       expect(platform.ensureComment).toHaveBeenCalledTimes(1);
       expect(prWorker.ensurePr).toHaveBeenCalledTimes(1);
-      expect(prWorker.checkAutoMerge).toHaveBeenCalledTimes(0);
+      expect(prAutomerge.checkAutoMerge).toHaveBeenCalledTimes(0);
     });
     it('ensures PR and adds lock file error comment if old releaseTimestamp', async () => {
       getUpdated.getUpdatedPackageFiles.mockResolvedValueOnce({
@@ -490,13 +493,13 @@ describe(getName(__filename), () => {
         prResult: PrResult.Created,
         pr: {},
       } as EnsurePrResult);
-      prWorker.checkAutoMerge.mockResolvedValueOnce(true);
+      prAutomerge.checkAutoMerge.mockResolvedValueOnce(true);
       config.releaseTimestamp = '2018-04-26T05:15:51.877Z';
       commit.commitFilesToBranch.mockResolvedValueOnce(null);
       await branchWorker.processBranch(config);
       expect(platform.ensureComment).toHaveBeenCalledTimes(1);
       expect(prWorker.ensurePr).toHaveBeenCalledTimes(1);
-      expect(prWorker.checkAutoMerge).toHaveBeenCalledTimes(0);
+      expect(prAutomerge.checkAutoMerge).toHaveBeenCalledTimes(0);
     });
     it('ensures PR and adds lock file error comment if new releaseTimestamp and branch exists', async () => {
       getUpdated.getUpdatedPackageFiles.mockResolvedValueOnce({
@@ -512,13 +515,13 @@ describe(getName(__filename), () => {
         prResult: PrResult.Created,
         pr: {},
       } as EnsurePrResult);
-      prWorker.checkAutoMerge.mockResolvedValueOnce(true);
+      prAutomerge.checkAutoMerge.mockResolvedValueOnce(true);
       config.releaseTimestamp = new Date().toISOString();
       commit.commitFilesToBranch.mockResolvedValueOnce(null);
       await branchWorker.processBranch(config);
       expect(platform.ensureComment).toHaveBeenCalledTimes(1);
       expect(prWorker.ensurePr).toHaveBeenCalledTimes(1);
-      expect(prWorker.checkAutoMerge).toHaveBeenCalledTimes(0);
+      expect(prAutomerge.checkAutoMerge).toHaveBeenCalledTimes(0);
     });
     it('throws error if lock file errors and new releaseTimestamp', async () => {
       getUpdated.getUpdatedPackageFiles.mockResolvedValueOnce({
@@ -534,7 +537,7 @@ describe(getName(__filename), () => {
         prResult: PrResult.Created,
         pr: {},
       } as EnsurePrResult);
-      prWorker.checkAutoMerge.mockResolvedValueOnce(true);
+      prAutomerge.checkAutoMerge.mockResolvedValueOnce(true);
       config.releaseTimestamp = new Date().toISOString();
       await expect(branchWorker.processBranch(config)).rejects.toThrow(
         Error(MANAGER_LOCKFILE_ERROR)
@@ -555,12 +558,12 @@ describe(getName(__filename), () => {
         prResult: PrResult.Created,
         pr: {},
       } as EnsurePrResult);
-      prWorker.checkAutoMerge.mockResolvedValueOnce(true);
+      prAutomerge.checkAutoMerge.mockResolvedValueOnce(true);
       commit.commitFilesToBranch.mockResolvedValueOnce(null);
       await branchWorker.processBranch(config);
       expect(platform.ensureComment).toHaveBeenCalledTimes(1);
       expect(prWorker.ensurePr).toHaveBeenCalledTimes(1);
-      expect(prWorker.checkAutoMerge).toHaveBeenCalledTimes(0);
+      expect(prAutomerge.checkAutoMerge).toHaveBeenCalledTimes(0);
     });
     it('swallows branch errors', async () => {
       getUpdated.getUpdatedPackageFiles.mockImplementationOnce(() => {

lib/workers/branch/index.ts

@@ -27,7 +27,8 @@ import {
   isBranchModified,
 } from '../../util/git';
 import { Limit, isLimitReached } from '../global/limits';
-import { checkAutoMerge, ensurePr, getPlatformPrOptions } from '../pr';
+import { ensurePr, getPlatformPrOptions } from '../pr';
+import { checkAutoMerge } from '../pr/automerge';
 import { BranchConfig, PrResult, ProcessBranchResult } from '../types';
 import { tryBranchAutomerge } from './automerge';
 import { prAlreadyExisted } from './check-existing';

lib/workers/pr/automerge.spec.ts

@@ -0,0 +1,83 @@
+import { getConfig, getName, git, mocked, partial } from '../../../test/util';
+import { Pr, platform as _platform } from '../../platform';
+import { BranchStatus } from '../../types';
+import { BranchConfig } from '../types';
+import * as prAutomerge from './automerge';
+
+jest.mock('../../util/git');
+
+const platform = mocked(_platform);
+const defaultConfig = getConfig();
+
+describe(getName(__filename), () => {
+  describe('checkAutoMerge(pr, config)', () => {
+    let config: BranchConfig;
+    let pr: Pr;
+    beforeEach(() => {
+      config = partial<BranchConfig>({
+        ...defaultConfig,
+      });
+      pr = partial<Pr>({
+        canMerge: true,
+      });
+    });
+    afterEach(() => {
+      jest.clearAllMocks();
+    });
+    it('should not automerge if not configured', async () => {
+      await prAutomerge.checkAutoMerge(pr, config);
+      expect(platform.mergePr).toHaveBeenCalledTimes(0);
+    });
+    it('should automerge if enabled and pr is mergeable', async () => {
+      config.automerge = true;
+      platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.green);
+      platform.mergePr.mockResolvedValueOnce(true);
+      await prAutomerge.checkAutoMerge(pr, config);
+      expect(platform.mergePr).toHaveBeenCalledTimes(1);
+    });
+    it('should automerge comment', async () => {
+      config.automerge = true;
+      config.automergeType = 'pr-comment';
+      config.automergeComment = '!merge';
+      platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.green);
+      await prAutomerge.checkAutoMerge(pr, config);
+      expect(platform.ensureCommentRemoval).toHaveBeenCalledTimes(0);
+      expect(platform.ensureComment).toHaveBeenCalledTimes(1);
+    });
+    it('should remove previous automerge comment when rebasing', async () => {
+      config.automerge = true;
+      config.automergeType = 'pr-comment';
+      config.automergeComment = '!merge';
+      config.rebaseRequested = true;
+      platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.green);
+      await prAutomerge.checkAutoMerge(pr, config);
+      expect(platform.ensureCommentRemoval).toHaveBeenCalledTimes(1);
+      expect(platform.ensureComment).toHaveBeenCalledTimes(1);
+    });
+    it('should not automerge if enabled and pr is mergeable but cannot rebase', async () => {
+      config.automerge = true;
+      platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.green);
+      git.isBranchModified.mockResolvedValueOnce(true);
+      await prAutomerge.checkAutoMerge(pr, config);
+      expect(platform.mergePr).toHaveBeenCalledTimes(0);
+    });
+    it('should not automerge if enabled and pr is mergeable but branch status is not success', async () => {
+      config.automerge = true;
+      platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.yellow);
+      await prAutomerge.checkAutoMerge(pr, config);
+      expect(platform.mergePr).toHaveBeenCalledTimes(0);
+    });
+    it('should not automerge if enabled and pr is mergeable but unstable', async () => {
+      config.automerge = true;
+      pr.canMerge = undefined;
+      await prAutomerge.checkAutoMerge(pr, config);
+      expect(platform.mergePr).toHaveBeenCalledTimes(0);
+    });
+    it('should not automerge if enabled and pr is unmergeable', async () => {
+      config.automerge = true;
+      pr.isConflicted = true;
+      await prAutomerge.checkAutoMerge(pr, config);
+      expect(platform.mergePr).toHaveBeenCalledTimes(0);
+    });
+  });
+});

lib/workers/pr/automerge.ts

@@ -0,0 +1,91 @@
+import { getAdminConfig } from '../../config/admin';
+import { logger } from '../../logger';
+import { Pr, platform } from '../../platform';
+import { BranchStatus } from '../../types';
+import { isBranchModified } from '../../util/git';
+import { BranchConfig } from '../types';
+
+export async function checkAutoMerge(
+  pr: Pr,
+  config: BranchConfig
+): Promise<boolean> {
+  logger.trace({ config }, 'checkAutoMerge');
+  const {
+    branchName,
+    automerge,
+    automergeType,
+    automergeComment,
+    requiredStatusChecks,
+    rebaseRequested,
+  } = config;
+  logger.debug(
+    { automerge, automergeType, automergeComment },
+    `Checking #${pr.number} for automerge`
+  );
+  if (automerge) {
+    logger.debug('PR is configured for automerge');
+    // Return if PR not ready for automerge
+    if (pr.isConflicted) {
+      logger.debug('PR is conflicted');
+      logger.debug({ pr });
+      return false;
+    }
+    if (requiredStatusChecks && pr.canMerge !== true) {
+      logger.debug(
+        { canMergeReason: pr.canMergeReason },
+        'PR is not ready for merge'
+      );
+      return false;
+    }
+    const branchStatus = await platform.getBranchStatus(
+      branchName,
+      requiredStatusChecks
+    );
+    if (branchStatus !== BranchStatus.green) {
+      logger.debug(
+        `PR is not ready for merge (branch status is ${branchStatus})`
+      );
+      return false;
+    }
+    // Check if it's been touched
+    if (await isBranchModified(branchName)) {
+      logger.debug('PR is ready for automerge but has been modified');
+      return false;
+    }
+    if (automergeType === 'pr-comment') {
+      logger.debug(`Applying automerge comment: ${automergeComment}`);
+      // istanbul ignore if
+      if (getAdminConfig().dryRun) {
+        logger.info(
+          `DRY-RUN: Would add PR automerge comment to PR #${pr.number}`
+        );
+        return false;
+      }
+      if (rebaseRequested) {
+        await platform.ensureCommentRemoval({
+          number: pr.number,
+          content: automergeComment,
+        });
+      }
+      return platform.ensureComment({
+        number: pr.number,
+        topic: null,
+        content: automergeComment,
+      });
+    }
+    // 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;
+    }
+    const res = await platform.mergePr(pr.number, branchName);
+    if (res) {
+      logger.info({ pr: pr.number, prTitle: pr.title }, 'PR automerged');
+    }
+    return res;
+  }
+  logger.debug('No automerge');
+  return false;
+}

lib/workers/pr/index.spec.ts

@@ -5,6 +5,7 @@ import { Pr, platform as _platform } from '../../platform';
 import { BranchStatus } from '../../types';
 import * as _limits from '../global/limits';
 import { BranchConfig, PrResult } from '../types';
+import * as prAutomerge from './automerge';
 import * as _changelogHelper from './changelog';
 import { getChangeLogJSON } from './changelog';
 import * as codeOwners from './code-owners';
@@ -114,14 +115,14 @@ describe(getName(__filename), () => {
       jest.clearAllMocks();
     });
     it('should not automerge if not configured', async () => {
-      await prWorker.checkAutoMerge(pr, config);
+      await prAutomerge.checkAutoMerge(pr, config);
       expect(platform.mergePr).toHaveBeenCalledTimes(0);
     });
     it('should automerge if enabled and pr is mergeable', async () => {
       config.automerge = true;
       platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.green);
       platform.mergePr.mockResolvedValueOnce(true);
-      await prWorker.checkAutoMerge(pr, config);
+      await prAutomerge.checkAutoMerge(pr, config);
       expect(platform.mergePr).toHaveBeenCalledTimes(1);
     });
     it('should automerge comment', async () => {
@@ -129,7 +130,7 @@ describe(getName(__filename), () => {
       config.automergeType = 'pr-comment';
       config.automergeComment = '!merge';
       platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.green);
-      await prWorker.checkAutoMerge(pr, config);
+      await prAutomerge.checkAutoMerge(pr, config);
       expect(platform.ensureCommentRemoval).toHaveBeenCalledTimes(0);
       expect(platform.ensureComment).toHaveBeenCalledTimes(1);
     });
@@ -139,7 +140,7 @@ describe(getName(__filename), () => {
       config.automergeComment = '!merge';
       config.rebaseRequested = true;
       platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.green);
-      await prWorker.checkAutoMerge(pr, config);
+      await prAutomerge.checkAutoMerge(pr, config);
       expect(platform.ensureCommentRemoval).toHaveBeenCalledTimes(1);
       expect(platform.ensureComment).toHaveBeenCalledTimes(1);
     });
@@ -147,25 +148,25 @@ describe(getName(__filename), () => {
       config.automerge = true;
       platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.green);
       git.isBranchModified.mockResolvedValueOnce(true);
-      await prWorker.checkAutoMerge(pr, config);
+      await prAutomerge.checkAutoMerge(pr, config);
       expect(platform.mergePr).toHaveBeenCalledTimes(0);
     });
     it('should not automerge if enabled and pr is mergeable but branch status is not success', async () => {
       config.automerge = true;
       platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.yellow);
-      await prWorker.checkAutoMerge(pr, config);
+      await prAutomerge.checkAutoMerge(pr, config);
       expect(platform.mergePr).toHaveBeenCalledTimes(0);
     });
     it('should not automerge if enabled and pr is mergeable but unstable', async () => {
       config.automerge = true;
       pr.canMerge = undefined;
-      await prWorker.checkAutoMerge(pr, config);
+      await prAutomerge.checkAutoMerge(pr, config);
       expect(platform.mergePr).toHaveBeenCalledTimes(0);
     });
     it('should not automerge if enabled and pr is unmergeable', async () => {
       config.automerge = true;
       pr.isConflicted = true;
-      await prWorker.checkAutoMerge(pr, config);
+      await prAutomerge.checkAutoMerge(pr, config);
       expect(platform.mergePr).toHaveBeenCalledTimes(0);
     });
   });

lib/workers/pr/index.ts

@@ -10,11 +10,7 @@ import { PlatformPrOptions, Pr, platform } from '../../platform';
 import { BranchStatus } from '../../types';
 import { ExternalHostError } from '../../types/errors/external-host-error';
 import { sampleSize } from '../../util';
-import {
-  deleteBranch,
-  getBranchLastCommitTime,
-  isBranchModified,
-} from '../../util/git';
+import { deleteBranch, getBranchLastCommitTime } from '../../util/git';
 import * as template from '../../util/template';
 import { Limit, incLimitedValue, isLimitReached } from '../global/limits';
 import { BranchConfig, PrResult } from '../types';
@@ -479,88 +475,3 @@ export async function ensurePr(
   }
   return { prResult: PrResult.Error };
 }
-
-export async function checkAutoMerge(
-  pr: Pr,
-  config: BranchConfig
-): Promise<boolean> {
-  logger.trace({ config }, 'checkAutoMerge');
-  const {
-    branchName,
-    automerge,
-    automergeType,
-    automergeComment,
-    requiredStatusChecks,
-    rebaseRequested,
-  } = config;
-  logger.debug(
-    { automerge, automergeType, automergeComment },
-    `Checking #${pr.number} for automerge`
-  );
-  if (automerge) {
-    logger.debug('PR is configured for automerge');
-    // Return if PR not ready for automerge
-    if (pr.isConflicted) {
-      logger.debug('PR is conflicted');
-      logger.debug({ pr });
-      return false;
-    }
-    if (requiredStatusChecks && pr.canMerge !== true) {
-      logger.debug(
-        { canMergeReason: pr.canMergeReason },
-        'PR is not ready for merge'
-      );
-      return false;
-    }
-    const branchStatus = await platform.getBranchStatus(
-      branchName,
-      requiredStatusChecks
-    );
-    if (branchStatus !== BranchStatus.green) {
-      logger.debug(
-        `PR is not ready for merge (branch status is ${branchStatus})`
-      );
-      return false;
-    }
-    // Check if it's been touched
-    if (await isBranchModified(branchName)) {
-      logger.debug('PR is ready for automerge but has been modified');
-      return false;
-    }
-    if (automergeType === 'pr-comment') {
-      logger.debug(`Applying automerge comment: ${automergeComment}`);
-      // istanbul ignore if
-      if (getAdminConfig().dryRun) {
-        logger.info(
-          `DRY-RUN: Would add PR automerge comment to PR #${pr.number}`
-        );
-        return false;
-      }
-      if (rebaseRequested) {
-        await platform.ensureCommentRemoval({
-          number: pr.number,
-          content: automergeComment,
-        });
-      }
-      return platform.ensureComment({
-        number: pr.number,
-        topic: null,
-        content: automergeComment,
-      });
-    }
-    // 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;
-    }
-    const res = await platform.mergePr(pr.number, branchName);
-    if (res) {
-      logger.info({ pr: pr.number, prTitle: pr.title }, 'PR automerged');
-    }
-    return res;
-  }
-  logger.debug('No automerge');
-  return false;
-}