From b2fde316934c79c4d75875cbed6c2fcce4473bca Mon Sep 17 00:00:00 2001 From: Sergio Zharinov <zharinov@users.noreply.github.com> Date: Wed, 7 Oct 2020 14:07:25 +0400 Subject: [PATCH] fix(limits): Count concurrent PR with platform API instead of branches (#7421) Co-authored-by: Rhys Arkins <rhys@arkins.net> Co-authored-by: Michael Kriese <michael.kriese@visualon.de> --- lib/workers/repository/process/limits.spec.ts | 33 +++++++------- lib/workers/repository/process/limits.ts | 45 +++++++++++++------ lib/workers/repository/process/write.ts | 20 +++++++++ 3 files changed, 68 insertions(+), 30 deletions(-) diff --git a/lib/workers/repository/process/limits.spec.ts b/lib/workers/repository/process/limits.spec.ts index 8c2fc901ec..ae31e9a46d 100644 --- a/lib/workers/repository/process/limits.spec.ts +++ b/lib/workers/repository/process/limits.spec.ts @@ -1,15 +1,9 @@ import moment from 'moment'; -import { - RenovateConfig, - getConfig, - git, - platform, -} from '../../../../test/util'; +import { RenovateConfig, getConfig, platform } from '../../../../test/util'; +import { PrState } from '../../../types'; import { BranchConfig } from '../../common'; import * as limits from './limits'; -jest.mock('../../../util/git'); - let config: RenovateConfig; beforeEach(() => { jest.resetAllMocks(); @@ -39,18 +33,25 @@ describe('workers/repository/process/limits', () => { }); }); describe('getConcurrentPrsRemaining()', () => { - it('calculates concurrent limit remaining', () => { + it('calculates concurrent limit remaining', async () => { config.prConcurrentLimit = 20; - git.branchExists.mockReturnValueOnce(true); + platform.getBranchPr.mockImplementation((branchName) => + branchName + ? Promise.resolve({ + sourceBranch: branchName, + state: PrState.Open, + } as never) + : Promise.reject('some error') + ); const branches: BranchConfig[] = [ - { branchName: 'test', upgrades: [] }, - { branchName: undefined, upgrades: [] }, - ]; - const res = limits.getConcurrentPrsRemaining(config, branches); + { branchName: 'test' }, + { branchName: null }, + ] as never; + const res = await limits.getConcurrentPrsRemaining(config, branches); expect(res).toEqual(19); }); - it('returns 99 if no concurrent limit', () => { - const res = limits.getConcurrentPrsRemaining(config, []); + it('returns 99 if no concurrent limit', async () => { + const res = await limits.getConcurrentPrsRemaining(config, []); expect(res).toEqual(99); }); }); diff --git a/lib/workers/repository/process/limits.ts b/lib/workers/repository/process/limits.ts index d26b5391cf..088c84752d 100644 --- a/lib/workers/repository/process/limits.ts +++ b/lib/workers/repository/process/limits.ts @@ -1,8 +1,8 @@ import moment from 'moment'; import { RenovateConfig } from '../../../config'; import { logger } from '../../../logger'; -import { platform } from '../../../platform'; -import { branchExists } from '../../../util/git'; +import { Pr, platform } from '../../../platform'; +import { PrState } from '../../../types'; import { BranchConfig } from '../../common'; export async function getPrHourlyRemaining( @@ -40,22 +40,39 @@ export async function getPrHourlyRemaining( return 99; } -export function getConcurrentPrsRemaining( +export async function getConcurrentPrsRemaining( config: RenovateConfig, branches: BranchConfig[] -): number { +): Promise<number> { if (config.prConcurrentLimit) { - logger.debug(`Enforcing prConcurrentLimit (${config.prConcurrentLimit})`); - let currentlyOpen = 0; - for (const branch of branches) { - if (branchExists(branch.branchName)) { - currentlyOpen += 1; + logger.debug(`Calculating prConcurrentLimit (${config.prConcurrentLimit})`); + try { + const openPrs: Pr[] = []; + for (const { branchName } of branches) { + try { + const pr = await platform.getBranchPr(branchName); + if ( + pr && + pr.sourceBranch !== config.onboardingBranch && + pr.state === PrState.Open + ) { + openPrs.push(pr); + } + } catch (err) { + // no-op + } } + logger.debug(`${openPrs.length} PRs are currently open`); + const concurrentRemaining = Math.max( + 0, + config.prConcurrentLimit - openPrs.length + ); + logger.debug(`PR concurrent limit remaining: ${concurrentRemaining}`); + return concurrentRemaining; + } catch (err) /* istanbul ignore next */ { + logger.error({ err }, 'Error checking concurrent PRs'); + return config.prConcurrentLimit; } - logger.debug(`${currentlyOpen} PRs are currently open`); - const concurrentRemaining = config.prConcurrentLimit - currentlyOpen; - logger.debug(`PR concurrent limit remaining: ${concurrentRemaining}`); - return concurrentRemaining; } return 99; } @@ -65,7 +82,7 @@ export async function getPrsRemaining( branches: BranchConfig[] ): Promise<number> { const hourlyRemaining = await getPrHourlyRemaining(config); - const concurrentRemaining = getConcurrentPrsRemaining(config, branches); + const concurrentRemaining = await getConcurrentPrsRemaining(config, branches); return hourlyRemaining < concurrentRemaining ? hourlyRemaining : concurrentRemaining; diff --git a/lib/workers/repository/process/write.ts b/lib/workers/repository/process/write.ts index 62752e0b15..7af253ec84 100644 --- a/lib/workers/repository/process/write.ts +++ b/lib/workers/repository/process/write.ts @@ -1,5 +1,7 @@ import { RenovateConfig } from '../../../config'; import { addMeta, logger, removeMeta } from '../../../logger'; +import { platform } from '../../../platform'; +import { PrState } from '../../../types'; import { branchExists } from '../../../util/git'; import { processBranch } from '../../branch'; import { BranchConfig, ProcessBranchResult } from '../../common'; @@ -8,6 +10,15 @@ import { getPrsRemaining } from './limits'; export type WriteUpdateResult = 'done' | 'automerged'; +async function prExists(branchName: string): Promise<boolean> { + try { + const pr = await platform.getBranchPr(branchName); + return pr?.state === PrState.Open; + } catch (err) /* istanbul ignore next */ { + return false; + } +} + export async function writeUpdates( config: RenovateConfig, allBranches: BranchConfig[] @@ -35,6 +46,7 @@ export async function writeUpdates( 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); branch.res = res; if ( @@ -64,6 +76,14 @@ export async function writeUpdates( ) { deductPrRemainingCount = 1; } + // istanbul ignore if + if ( + deductPrRemainingCount === 0 && + !prExisted && + (await prExists(branch.branchName)) + ) { + deductPrRemainingCount = 1; + } prsRemaining -= deductPrRemainingCount; } removeMeta(['branch']); -- GitLab