refactor(workers): Use limiting API for PRs instead of in-place count…

…ers (#8031)

lib/workers/branch/index.spec.ts

@@ -9,6 +9,7 @@ import { PrState } from '../../types';
 import * as _exec from '../../util/exec';
 import { File, StatusResult } from '../../util/git';
 import { BranchConfig, PrResult, ProcessBranchResult } from '../common';
+import * as _limits from '../global/limits';
 import * as _prWorker from '../pr';
 import * as _automerge from './automerge';
 import * as _checkExisting from './check-existing';
@@ -29,6 +30,7 @@ jest.mock('../pr');
 jest.mock('../../util/exec');
 jest.mock('../../util/git');
 jest.mock('fs-extra');
+jest.mock('../global/limits');
 
 const getUpdated = mocked(_getUpdated);
 const schedule = mocked(_schedule);
@@ -40,6 +42,7 @@ const commit = mocked(_commit);
 const prWorker = mocked(_prWorker);
 const exec = mocked(_exec);
 const fs = mocked(_fs);
+const limits = mocked(_limits);
 
 describe('workers/branch', () => {
   describe('processBranch', () => {
@@ -215,8 +218,9 @@ describe('workers/branch', () => {
         artifactErrors: [],
         updatedArtifacts: [],
       });
-      git.branchExists.mockReturnValue(false);
-      expect(await branchWorker.processBranch(config, true)).toEqual(
+      limits.isLimitReached.mockReturnValueOnce(true);
+      limits.isLimitReached.mockReturnValueOnce(false);
+      expect(await branchWorker.processBranch(config)).toEqual(
         ProcessBranchResult.PrLimitReached
       );
     });
@@ -232,7 +236,8 @@ describe('workers/branch', () => {
       prWorker.ensurePr.mockResolvedValueOnce({
         prResult: PrResult.LimitReached,
       });
-      expect(await branchWorker.processBranch(config, true)).toEqual(
+      limits.isLimitReached.mockReturnValue(false);
+      expect(await branchWorker.processBranch(config)).toEqual(
         ProcessBranchResult.PrLimitReached
       );
     });
@@ -245,7 +250,9 @@ describe('workers/branch', () => {
         updatedArtifacts: [],
       });
       git.branchExists.mockReturnValue(false);
-      expect(await branchWorker.processBranch(config, false, true)).toEqual(
+      limits.isLimitReached.mockReturnValueOnce(false);
+      limits.isLimitReached.mockReturnValueOnce(true);
+      expect(await branchWorker.processBranch(config)).toEqual(
         ProcessBranchResult.CommitLimitReached
       );
     });

lib/workers/branch/index.ts

@@ -30,6 +30,7 @@ import {
 import { regEx } from '../../util/regex';
 import * as template from '../../util/template';
 import { BranchConfig, PrResult, ProcessBranchResult } from '../common';
+import { Limit, isLimitReached } from '../global/limits';
 import { checkAutoMerge, ensurePr, getPlatformPrOptions } from '../pr';
 import { tryBranchAutomerge } from './automerge';
 import { prAlreadyExisted } from './check-existing';
@@ -60,9 +61,7 @@ async function deleteBranchSilently(branchName: string): Promise<void> {
 }
 
 export async function processBranch(
-  branchConfig: BranchConfig,
-  prLimitReached?: boolean,
-  commitLimitReached?: boolean
+  branchConfig: BranchConfig
 ): Promise<ProcessBranchResult> {
   const config: BranchConfig = { ...branchConfig };
   const dependencies = config.upgrades
@@ -153,15 +152,15 @@ export async function processBranch(
     }
     if (
       !branchExists &&
-      prLimitReached &&
+      isLimitReached(Limit.PullRequests) &&
       !dependencyDashboardCheck &&
       !config.vulnerabilityAlert
     ) {
       logger.debug('Reached PR limit - skipping branch creation');
       return ProcessBranchResult.PrLimitReached;
     }
     if (
-      commitLimitReached &&
+      isLimitReached(Limit.Commits) &&
       !dependencyDashboardCheck &&
       !config.vulnerabilityAlert
     ) {
@@ -583,7 +582,7 @@ export async function processBranch(
     logger.debug(
       `There are ${config.errors.length} errors and ${config.warnings.length} warnings`
     );
-    const { prResult: result, pr } = await ensurePr(config, prLimitReached);
+    const { prResult: result, pr } = await ensurePr(config);
     if (result === PrResult.LimitReached) {
       logger.debug('Reached PR limit - skipping PR creation');
       return ProcessBranchResult.PrLimitReached;

lib/workers/global/limits.ts

@@ -2,6 +2,7 @@ import { logger } from '../../logger';
 
 export enum Limit {
   Commits = 'Commits',
+  PullRequests = 'PullRequests',
 }
 
 interface LimitValue {

lib/workers/pr/index.spec.ts

@@ -4,6 +4,7 @@ import { PLATFORM_TYPE_GITLAB } from '../../constants/platforms';
 import { Pr, platform as _platform } from '../../platform';
 import { BranchStatus } from '../../types';
 import { BranchConfig, PrResult } from '../common';
+import * as _limits from '../global/limits';
 import * as _changelogHelper from './changelog';
 import { getChangeLogJSON } from './changelog';
 import * as codeOwners from './code-owners';
@@ -14,10 +15,12 @@ const changelogHelper = mocked(_changelogHelper);
 const gitlabChangelogHelper = mocked(_changelogHelper);
 const platform = mocked(_platform);
 const defaultConfig = getConfig();
+const limits = mocked(_limits);
 
 jest.mock('../../util/git');
 jest.mock('./changelog');
 jest.mock('./code-owners');
+jest.mock('../global/limits');
 
 function setupChangelogMock() {
   changelogHelper.getChangeLogJSON = jest.fn();
@@ -268,7 +271,8 @@ describe('workers/pr', () => {
       config.prCreation = 'status-success';
       config.automerge = true;
       config.schedule = ['before 5am'];
-      const { prResult } = await prWorker.ensurePr(config, true);
+      limits.isLimitReached.mockReturnValueOnce(true);
+      const { prResult } = await prWorker.ensurePr(config);
       expect(prResult).toEqual(PrResult.LimitReached);
       expect(platform.createPr.mock.calls).toBeEmpty();
     });
@@ -278,13 +282,11 @@ describe('workers/pr', () => {
       config.prCreation = 'status-success';
       config.automerge = true;
       config.schedule = ['before 5am'];
-      const { prResult } = await prWorker.ensurePr(
-        {
-          ...config,
-          dependencyDashboardChecks: { 'renovate/dummy-1.x': 'true' },
-        },
-        true
-      );
+      limits.isLimitReached.mockReturnValueOnce(true);
+      const { prResult } = await prWorker.ensurePr({
+        ...config,
+        dependencyDashboardChecks: { 'renovate/dummy-1.x': 'true' },
+      });
       expect(prResult).toEqual(PrResult.Created);
       expect(platform.createPr).toHaveBeenCalled();
     });

lib/workers/pr/index.ts

@@ -15,6 +15,7 @@ import {
   isBranchModified,
 } from '../../util/git';
 import { BranchConfig, PrResult } from '../common';
+import { Limit, isLimitReached } from '../global/limits';
 import { getPrBody } from './body';
 import { ChangeLogError } from './changelog';
 import { codeOwnersForPr } from './code-owners';
@@ -114,8 +115,7 @@ export function getPlatformPrOptions(
 
 // Ensures that PR exists with matching title/body
 export async function ensurePr(
-  prConfig: BranchConfig,
-  prLimitReached?: boolean
+  prConfig: BranchConfig
 ): Promise<{
   prResult: PrResult;
   pr?: Pr;
@@ -381,7 +381,7 @@ export async function ensurePr(
         logger.info('DRY-RUN: Would create PR: ' + prTitle);
         pr = { number: 0, displayNumber: 'Dry run PR' } as never;
       } else {
-        if (!dependencyDashboardCheck && prLimitReached) {
+        if (!dependencyDashboardCheck && isLimitReached(Limit.PullRequests)) {
           return { prResult: PrResult.LimitReached };
         }
         pr = await platform.createPr({

lib/workers/repository/process/write.spec.ts

@@ -1,6 +1,7 @@
 import { RenovateConfig, getConfig, git, mocked } from '../../../../test/util';
 import * as _branchWorker from '../../branch';
 import { BranchConfig, ProcessBranchResult } from '../../common';
+import { Limit, isLimitReached } from '../../global/limits';
 import * as _limits from './limits';
 import { writeUpdates } from './write';
 
@@ -57,28 +58,17 @@ describe('workers/repository/write', () => {
       expect(res).toEqual('automerged');
       expect(branchWorker.processBranch).toHaveBeenCalledTimes(4);
     });
-    it('counts created branches', async () => {
-      const branches: BranchConfig[] = [
-        { res: ProcessBranchResult.Pending, expect: false },
-        { res: ProcessBranchResult.PrLimitReached, expect: true },
-      ] as never;
-      limits.getPrsRemaining.mockResolvedValueOnce(1);
-
-      branches.forEach(({ res, expect: limitReached }) => {
-        branchWorker.processBranch.mockResolvedValueOnce(res);
-        git.branchExists.mockReturnValueOnce(false);
-        git.branchExists.mockReturnValueOnce(!limitReached as never);
-      });
-
-      await writeUpdates(config, branches);
-
-      branches.forEach((branch) =>
-        expect(branchWorker.processBranch).toHaveBeenCalledWith(
-          branch,
-          branch.expect,
-          false
-        )
+    it('increments branch counter', async () => {
+      const branches: BranchConfig[] = [{}] as never;
+      branchWorker.processBranch.mockResolvedValueOnce(
+        ProcessBranchResult.Pending
       );
+      git.branchExists.mockReturnValueOnce(false);
+      git.branchExists.mockReturnValueOnce(true);
+      limits.getPrsRemaining.mockResolvedValueOnce(1);
+      expect(isLimitReached(Limit.PullRequests)).toBeFalse();
+      await writeUpdates({ config }, branches);
+      expect(isLimitReached(Limit.PullRequests)).toBeTrue();
     });
   });
 });

lib/workers/repository/process/write.ts

@@ -5,7 +5,7 @@ import { PrState } from '../../../types';
 import { branchExists } from '../../../util/git';
 import { processBranch } from '../../branch';
 import { BranchConfig, ProcessBranchResult } from '../../common';
-import { Limit, isLimitReached } from '../../global/limits';
+import { Limit, incLimitedValue, setMaxLimit } from '../../global/limits';
 import { getPrsRemaining } from './limits';
 
 export type WriteUpdateResult = 'done' | 'automerged';
@@ -39,15 +39,14 @@ export async function writeUpdates(
     }
     return true;
   });
-  let prsRemaining = await getPrsRemaining(config, branches);
+  const prsRemaining = await getPrsRemaining(config, branches);
   logger.debug({ prsRemaining }, 'Calculated maximum PRs remaining this run');
+  setMaxLimit(Limit.PullRequests, prsRemaining);
   for (const branch of branches) {
     addMeta({ branch: branch.branchName });
-    const prLimitReached = prsRemaining <= 0;
-    const commitLimitReached = isLimitReached(Limit.Commits);
     const branchExisted = branchExists(branch.branchName);
     const prExisted = await prExists(branch.branchName);
-    const res = await processBranch(branch, prLimitReached, commitLimitReached);
+    const res = await processBranch(branch);
     branch.res = res;
     if (
       res === ProcessBranchResult.Automerged &&
@@ -56,34 +55,27 @@ export async function writeUpdates(
       // Stop processing other branches because base branch has been changed
       return 'automerged';
     }
-    let deductPrRemainingCount = 0;
     if (res === ProcessBranchResult.PrCreated) {
-      deductPrRemainingCount = 1;
+      incLimitedValue(Limit.PullRequests);
     }
     // istanbul ignore if
-    if (
+    else if (
       res === ProcessBranchResult.Automerged &&
       branch.automergeType === 'pr-comment' &&
       branch.requiredStatusChecks === null
     ) {
-      deductPrRemainingCount = 1;
-    }
-    if (
+      incLimitedValue(Limit.PullRequests);
+    } else if (
       res === ProcessBranchResult.Pending &&
       !branchExisted &&
       branchExists(branch.branchName)
     ) {
-      deductPrRemainingCount = 1;
+      incLimitedValue(Limit.PullRequests);
     }
     // istanbul ignore if
-    if (
-      deductPrRemainingCount === 0 &&
-      !prExisted &&
-      (await prExists(branch.branchName))
-    ) {
-      deductPrRemainingCount = 1;
+    else if (!prExisted && (await prExists(branch.branchName))) {
+      incLimitedValue(Limit.PullRequests);
     }
-    prsRemaining -= deductPrRemainingCount;
   }
   removeMeta(['branch']);
   return 'done';