diff --git a/lib/workers/branch/index.spec.ts b/lib/workers/branch/index.spec.ts index fd6d88eb208250d124e93b401bc82801f6d010ba..1c755c898c10b448d1759c47d29248b591553e2f 100644 --- a/lib/workers/branch/index.spec.ts +++ b/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 ); }); diff --git a/lib/workers/branch/index.ts b/lib/workers/branch/index.ts index 8650a2d5854a79fc150bce2a41bbdec4b2e4ecbf..e5ce7680eb512f33399f9a66c1d49d110775ca25 100644 --- a/lib/workers/branch/index.ts +++ b/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,7 +152,7 @@ export async function processBranch( } if ( !branchExists && - prLimitReached && + isLimitReached(Limit.PullRequests) && !dependencyDashboardCheck && !config.vulnerabilityAlert ) { @@ -161,7 +160,7 @@ export async function processBranch( 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; diff --git a/lib/workers/global/limits.ts b/lib/workers/global/limits.ts index 04d194c72d22f7d4c4ffdf9368a96c5e02f300d2..c5476f8922fb81b7c78843675fc278233142bc88 100644 --- a/lib/workers/global/limits.ts +++ b/lib/workers/global/limits.ts @@ -2,6 +2,7 @@ import { logger } from '../../logger'; export enum Limit { Commits = 'Commits', + PullRequests = 'PullRequests', } interface LimitValue { diff --git a/lib/workers/pr/index.spec.ts b/lib/workers/pr/index.spec.ts index 59ee12f457315767cd9be5edbe43749b413818e9..cd5ec032bfd08ea85b5fcc61ca8376a39dbea363 100644 --- a/lib/workers/pr/index.spec.ts +++ b/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(); }); diff --git a/lib/workers/pr/index.ts b/lib/workers/pr/index.ts index 068874f390664d3f92ac4c2e1ea322871ffd7cf5..7d991b3367d6a3665e0ea628f29084f45095048d 100644 --- a/lib/workers/pr/index.ts +++ b/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({ diff --git a/lib/workers/repository/process/write.spec.ts b/lib/workers/repository/process/write.spec.ts index db7620c79b290e64ca7937004a9bc9c0255f692b..378544c3bb9f6f390d1d993f6fa8a2f26d4dfee1 100644 --- a/lib/workers/repository/process/write.spec.ts +++ b/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(); }); }); }); diff --git a/lib/workers/repository/process/write.ts b/lib/workers/repository/process/write.ts index 74a86a76868c9943354b6afd0e14acd9de45b08f..19c2bb98a7644ce9be22a7f3ac19bd9a23ca0f09 100644 --- a/lib/workers/repository/process/write.ts +++ b/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';