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>
This commit is contained in:
Sergio Zharinov 2020-10-07 14:07:25 +04:00 committed by GitHub
parent 46cc3c5305
commit b2fde31693
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 68 additions and 30 deletions

View file

@ -1,15 +1,9 @@
import moment from 'moment'; import moment from 'moment';
import { import { RenovateConfig, getConfig, platform } from '../../../../test/util';
RenovateConfig, import { PrState } from '../../../types';
getConfig,
git,
platform,
} from '../../../../test/util';
import { BranchConfig } from '../../common'; import { BranchConfig } from '../../common';
import * as limits from './limits'; import * as limits from './limits';
jest.mock('../../../util/git');
let config: RenovateConfig; let config: RenovateConfig;
beforeEach(() => { beforeEach(() => {
jest.resetAllMocks(); jest.resetAllMocks();
@ -39,18 +33,25 @@ describe('workers/repository/process/limits', () => {
}); });
}); });
describe('getConcurrentPrsRemaining()', () => { describe('getConcurrentPrsRemaining()', () => {
it('calculates concurrent limit remaining', () => { it('calculates concurrent limit remaining', async () => {
config.prConcurrentLimit = 20; 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[] = [ const branches: BranchConfig[] = [
{ branchName: 'test', upgrades: [] }, { branchName: 'test' },
{ branchName: undefined, upgrades: [] }, { branchName: null },
]; ] as never;
const res = limits.getConcurrentPrsRemaining(config, branches); const res = await limits.getConcurrentPrsRemaining(config, branches);
expect(res).toEqual(19); expect(res).toEqual(19);
}); });
it('returns 99 if no concurrent limit', () => { it('returns 99 if no concurrent limit', async () => {
const res = limits.getConcurrentPrsRemaining(config, []); const res = await limits.getConcurrentPrsRemaining(config, []);
expect(res).toEqual(99); expect(res).toEqual(99);
}); });
}); });

View file

@ -1,8 +1,8 @@
import moment from 'moment'; import moment from 'moment';
import { RenovateConfig } from '../../../config'; import { RenovateConfig } from '../../../config';
import { logger } from '../../../logger'; import { logger } from '../../../logger';
import { platform } from '../../../platform'; import { Pr, platform } from '../../../platform';
import { branchExists } from '../../../util/git'; import { PrState } from '../../../types';
import { BranchConfig } from '../../common'; import { BranchConfig } from '../../common';
export async function getPrHourlyRemaining( export async function getPrHourlyRemaining(
@ -40,22 +40,39 @@ export async function getPrHourlyRemaining(
return 99; return 99;
} }
export function getConcurrentPrsRemaining( export async function getConcurrentPrsRemaining(
config: RenovateConfig, config: RenovateConfig,
branches: BranchConfig[] branches: BranchConfig[]
): number { ): Promise<number> {
if (config.prConcurrentLimit) { if (config.prConcurrentLimit) {
logger.debug(`Enforcing prConcurrentLimit (${config.prConcurrentLimit})`); logger.debug(`Calculating prConcurrentLimit (${config.prConcurrentLimit})`);
let currentlyOpen = 0; try {
for (const branch of branches) { const openPrs: Pr[] = [];
if (branchExists(branch.branchName)) { for (const { branchName } of branches) {
currentlyOpen += 1; 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; return 99;
} }
@ -65,7 +82,7 @@ export async function getPrsRemaining(
branches: BranchConfig[] branches: BranchConfig[]
): Promise<number> { ): Promise<number> {
const hourlyRemaining = await getPrHourlyRemaining(config); const hourlyRemaining = await getPrHourlyRemaining(config);
const concurrentRemaining = getConcurrentPrsRemaining(config, branches); const concurrentRemaining = await getConcurrentPrsRemaining(config, branches);
return hourlyRemaining < concurrentRemaining return hourlyRemaining < concurrentRemaining
? hourlyRemaining ? hourlyRemaining
: concurrentRemaining; : concurrentRemaining;

View file

@ -1,5 +1,7 @@
import { RenovateConfig } from '../../../config'; import { RenovateConfig } from '../../../config';
import { addMeta, logger, removeMeta } from '../../../logger'; import { addMeta, logger, removeMeta } from '../../../logger';
import { platform } from '../../../platform';
import { PrState } from '../../../types';
import { branchExists } from '../../../util/git'; import { branchExists } from '../../../util/git';
import { processBranch } from '../../branch'; import { processBranch } from '../../branch';
import { BranchConfig, ProcessBranchResult } from '../../common'; import { BranchConfig, ProcessBranchResult } from '../../common';
@ -8,6 +10,15 @@ import { getPrsRemaining } from './limits';
export type WriteUpdateResult = 'done' | 'automerged'; 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( export async function writeUpdates(
config: RenovateConfig, config: RenovateConfig,
allBranches: BranchConfig[] allBranches: BranchConfig[]
@ -35,6 +46,7 @@ export async function writeUpdates(
const prLimitReached = prsRemaining <= 0; const prLimitReached = prsRemaining <= 0;
const commitLimitReached = isLimitReached(Limit.Commits); const commitLimitReached = isLimitReached(Limit.Commits);
const branchExisted = branchExists(branch.branchName); const branchExisted = branchExists(branch.branchName);
const prExisted = await prExists(branch.branchName);
const res = await processBranch(branch, prLimitReached, commitLimitReached); const res = await processBranch(branch, prLimitReached, commitLimitReached);
branch.res = res; branch.res = res;
if ( if (
@ -64,6 +76,14 @@ export async function writeUpdates(
) { ) {
deductPrRemainingCount = 1; deductPrRemainingCount = 1;
} }
// istanbul ignore if
if (
deductPrRemainingCount === 0 &&
!prExisted &&
(await prExists(branch.branchName))
) {
deductPrRemainingCount = 1;
}
prsRemaining -= deductPrRemainingCount; prsRemaining -= deductPrRemainingCount;
} }
removeMeta(['branch']); removeMeta(['branch']);