Skip to content

Commit

Permalink
apply suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
RahulGautamSingh committed Nov 21, 2024
1 parent 9f49864 commit 8d06529
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 39 deletions.
18 changes: 9 additions & 9 deletions lib/workers/global/limits.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,8 +218,8 @@ describe('workers/global/limits', () => {

describe('isLimitReached', () => {
it('returns false based on concurrent limits', () => {
setCount('PullRequests', 1);
setCount('HourlyPullRequests', 1);
setCount('ConcurrentPRs', 1);
setCount('HourlyPRs', 1);
incCountValue('Branches'); // using incCountValue so it gets test coverage
const upgrades = partial<BranchUpgradeConfig>([
{
Expand All @@ -242,14 +242,14 @@ describe('workers/global/limits', () => {
isLimitReached('Branches', partial<BranchConfig>({ upgrades })),
).toBe(false);
expect(
isLimitReached('PullRequests', partial<BranchConfig>({ upgrades })),
isLimitReached('ConcurrentPRs', partial<BranchConfig>({ upgrades })),
).toBe(false);
});

it('returns true when hourly limit is reached', () => {
setCount('Branches', 2);
setCount('PullRequests', 2);
setCount('HourlyPullRequests', 2);
setCount('ConcurrentPRs', 2);
setCount('HourlyPRs', 2);
const upgrades = partial<BranchUpgradeConfig>([
{
prHourlyLimit: 10,
Expand All @@ -271,14 +271,14 @@ describe('workers/global/limits', () => {
isLimitReached('Branches', partial<BranchConfig>({ upgrades })),
).toBe(true);
expect(
isLimitReached('PullRequests', partial<BranchConfig>({ upgrades })),
isLimitReached('ConcurrentPRs', partial<BranchConfig>({ upgrades })),
).toBe(true);
});

it('returns true when concurrent limit is reached', () => {
setCount('Branches', 3);
setCount('PullRequests', 3);
setCount('HourlyPullRequests', 4);
setCount('ConcurrentPRs', 3);
setCount('HourlyPRs', 4);
const upgrades = partial<BranchUpgradeConfig>([
{
prHourlyLimit: 10,
Expand All @@ -300,7 +300,7 @@ describe('workers/global/limits', () => {
isLimitReached('Branches', partial<BranchConfig>({ upgrades })),
).toBe(true);
expect(
isLimitReached('PullRequests', partial<BranchConfig>({ upgrades })),
isLimitReached('ConcurrentPRs', partial<BranchConfig>({ upgrades })),
).toBe(true);
});
});
Expand Down
37 changes: 16 additions & 21 deletions lib/workers/global/limits.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ export function incLimitedValue(key: Limit, incBy = 1): void {
});
}

function handleCommitsLimit(key: Limit): boolean {
const limit = limits.get(key);
function handleCommitsLimit(): boolean {
const limit = limits.get('Commits');
// TODO: fix me?
// eslint-disable-next-line @typescript-eslint/prefer-optional-chain
if (!limit || limit.max === null) {
Expand All @@ -39,33 +39,28 @@ function handleCommitsLimit(key: Limit): boolean {
return max - current <= 0;
}

export type CountName = 'PullRequests' | 'HourlyPullRequests' | 'Branches';
interface CountValue {
current: number;
}
export type CountName = 'ConcurrentPRs' | 'HourlyPRs' | 'Branches';

type BranchLimitName =
| 'prHourlyLimit'
| 'branchConcurrentLimit'
| 'prConcurrentLimit';
| 'prConcurrentLimit'
| 'prHourlyLimit';

export const counts = new Map<CountName, CountValue>();
export const counts = new Map<CountName, number>();

export function setCount(key: CountName, val: number): void {
const count = val;
counts.set(key, { current: count });
counts.set(key, count);
logger.debug(`${key} count = ${count}`);
}

export function incCountValue(key: CountName, incBy = 1): void {
const count = counts.get(key) ?? { current: 0 };
counts.set(key, {
...count,
current: count.current + incBy,
});
const count = counts.get(key) ?? 0;
counts.set(key, count + incBy);
}

function handleOtherLimits(
key: Exclude<CountName, 'HourlyPullRequests'>,
key: Exclude<CountName, 'HourlyPRs'>,
config: BranchConfig,
): boolean {
const limitKey =
Expand All @@ -76,11 +71,11 @@ function handleOtherLimits(
const limitValue = calcLimit(config.upgrades, limitKey);

const hourlyPrCount =
counts.get('HourlyPullRequests')?.current ??
counts.get('HourlyPRs') ??
// istanbul ignore next: should not happen
0;
const currentCount =
counts.get(key)?.current ??
counts.get(key) ??
// istanbul ignore next: should not happen
0;

Expand Down Expand Up @@ -179,15 +174,15 @@ export function hasMultipleLimits(

export function isLimitReached(limit: 'Commits'): boolean;
export function isLimitReached(
limit: 'Branches' | 'PullRequests',
limit: 'Branches' | 'ConcurrentPRs',
config: BranchConfig,
): boolean;
export function isLimitReached(
limit: 'Commits' | 'Branches' | 'PullRequests',
limit: 'Commits' | 'Branches' | 'ConcurrentPRs',
config?: BranchConfig,
): boolean {
if (limit === 'Commits') {
return handleCommitsLimit(limit);
return handleCommitsLimit();
}

if (config) {
Expand Down
2 changes: 1 addition & 1 deletion lib/workers/repository/process/write.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ describe('workers/repository/process/write', () => {
GlobalConfig.set({ dryRun: 'full' });
config.baseBranches = ['main', 'dev'];
await writeUpdates(config, branches);
expect(counts.get('Branches')?.current).toBe(1);
expect(counts.get('Branches')).toBe(1);
expect(addMeta).toHaveBeenCalledWith({
baseBranch: 'main',
branch: branchName,
Expand Down
4 changes: 2 additions & 2 deletions lib/workers/repository/process/write.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,13 +133,13 @@ export async function writeUpdates(
);

const concurrentPrsCount = await getConcurrentPrsCount(config, branches);
setCount('PullRequests', concurrentPrsCount);
setCount('ConcurrentPRs', concurrentPrsCount);

const concurrentBranchesCount = await getConcurrentBranchesCount(branches);
setCount('Branches', concurrentBranchesCount);

const prsThisHourCount = await getPrHourlyCount(config);
setCount('HourlyPullRequests', prsThisHourCount);
setCount('HourlyPRs', prsThisHourCount);

for (const branch of branches) {
const { baseBranch, branchName } = branch;
Expand Down
2 changes: 1 addition & 1 deletion lib/workers/repository/update/branch/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ export async function processBranch(
}

logger.debug(
`Open PR Count: ${counts.get('PullRequests')?.current}, Existing Branch Count: ${counts.get('Branches')?.current}, Hourly PR Count: ${counts.get('HourlyPullRequests')?.current}`,
`Open PR Count: ${counts.get('ConcurrentPRs')}, Existing Branch Count: ${counts.get('Branches')}, Hourly PR Count: ${counts.get('HourlyPRs')}`,
);

if (
Expand Down
4 changes: 2 additions & 2 deletions lib/workers/repository/update/pr/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ describe('workers/repository/update/pr/index', () => {

expect(res).toEqual({ type: 'with-pr', pr });
expect(limits.incCountValue).toHaveBeenCalledTimes(2);
expect(limits.incCountValue).toHaveBeenCalledWith('PullRequests');
expect(limits.incCountValue).toHaveBeenCalledWith('HourlyPullRequests');
expect(limits.incCountValue).toHaveBeenCalledWith('ConcurrentPRs');
expect(limits.incCountValue).toHaveBeenCalledWith('HourlyPRs');
expect(logger.logger.info).toHaveBeenCalledWith(
{ pr: pr.number, prTitle },
'PR created',
Expand Down
6 changes: 3 additions & 3 deletions lib/workers/repository/update/pr/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ export async function ensurePr(
try {
if (
!dependencyDashboardCheck &&
isLimitReached('PullRequests', prConfig) &&
isLimitReached('ConcurrentPRs', prConfig) &&
!config.isVulnerabilityAlert
) {
logger.debug('Skipping PR - limit reached');
Expand All @@ -499,8 +499,8 @@ export async function ensurePr(
milestone: config.milestone,
});

incCountValue('PullRequests');
incCountValue('HourlyPullRequests');
incCountValue('ConcurrentPRs');
incCountValue('HourlyPRs');
logger.info({ pr: pr?.number, prTitle }, 'PR created');
} catch (err) {
logger.debug({ err }, 'Pull request creation error');
Expand Down

0 comments on commit 8d06529

Please sign in to comment.