From 34ef699b64b5de1c6f9fe70eafbb99422b839081 Mon Sep 17 00:00:00 2001
From: Rhys Arkins <rhys@arkins.net>
Date: Sat, 5 Jun 2021 13:19:20 +0200
Subject: [PATCH] fix: prResult -> prBlockedBy (#10213)
---
.../branch/__snapshots__/index.spec.ts.snap | 54 ++++++
lib/workers/branch/index.spec.ts | 67 ++++++--
lib/workers/branch/index.ts | 154 ++++++++++++++----
lib/workers/pr/index.spec.ts | 126 ++++++--------
lib/workers/pr/index.ts | 46 ++++--
.../master-issue_with_2_PR_edited.txt | 2 +-
.../master-issue_with_3_PR_in_progress.txt | 2 +-
.../repository/dependency-dashboard.spec.ts | 63 ++-----
.../repository/dependency-dashboard.ts | 47 ++++--
lib/workers/repository/process/write.ts | 2 +
lib/workers/types.ts | 20 +--
11 files changed, 361 insertions(+), 222 deletions(-)
diff --git a/lib/workers/branch/__snapshots__/index.spec.ts.snap b/lib/workers/branch/__snapshots__/index.spec.ts.snap
index 596958e7a2..945f5c3f24 100644
--- a/lib/workers/branch/__snapshots__/index.spec.ts.snap
+++ b/lib/workers/branch/__snapshots__/index.spec.ts.snap
@@ -3,6 +3,7 @@
exports[`workers/branch/index processBranch branch pr no rebase (dry run) 1`] = `
Object {
"branchExists": true,
+ "prNo": undefined,
"result": "pr-edited",
}
`;
@@ -10,6 +11,7 @@ Object {
exports[`workers/branch/index processBranch branch pr no schedule (dry run) 1`] = `
Object {
"branchExists": true,
+ "prNo": undefined,
"result": "done",
}
`;
@@ -17,6 +19,7 @@ Object {
exports[`workers/branch/index processBranch branch pr no schedule 1`] = `
Object {
"branchExists": true,
+ "prNo": undefined,
"result": "done",
}
`;
@@ -24,6 +27,7 @@ Object {
exports[`workers/branch/index processBranch branch pr no schedule lockfile (dry run) 1`] = `
Object {
"branchExists": true,
+ "prNo": undefined,
"result": "done",
}
`;
@@ -31,6 +35,7 @@ Object {
exports[`workers/branch/index processBranch closed pr (dry run) 1`] = `
Object {
"branchExists": false,
+ "prNo": undefined,
"result": "already-existed",
}
`;
@@ -38,6 +43,7 @@ Object {
exports[`workers/branch/index processBranch continues branch if branch edited and but PR found 1`] = `
Object {
"branchExists": true,
+ "prNo": undefined,
"result": "error",
}
`;
@@ -45,6 +51,7 @@ Object {
exports[`workers/branch/index processBranch does not skip branch if edited PR found with rebaseLabel 1`] = `
Object {
"branchExists": true,
+ "prNo": undefined,
"result": "error",
}
`;
@@ -52,6 +59,7 @@ Object {
exports[`workers/branch/index processBranch executes post-upgrade tasks if trust is high 1`] = `
Object {
"branchExists": true,
+ "prNo": undefined,
"result": "done",
}
`;
@@ -59,6 +67,7 @@ Object {
exports[`workers/branch/index processBranch executes post-upgrade tasks once when set to branch mode 1`] = `
Object {
"branchExists": true,
+ "prNo": undefined,
"result": "done",
}
`;
@@ -66,6 +75,7 @@ Object {
exports[`workers/branch/index processBranch executes post-upgrade tasks with disabled post-upgrade command templating 1`] = `
Object {
"branchExists": true,
+ "prNo": undefined,
"result": "done",
}
`;
@@ -73,6 +83,31 @@ Object {
exports[`workers/branch/index processBranch executes post-upgrade tasks with multiple dependecy in one branch 1`] = `
Object {
"branchExists": true,
+ "prNo": undefined,
+ "result": "done",
+}
+`;
+
+exports[`workers/branch/index processBranch handles unknown PrBlockedBy 1`] = `
+Object {
+ "branchExists": true,
+ "prBlockedBy": "whoops",
+ "result": "error",
+}
+`;
+
+exports[`workers/branch/index processBranch returns if PR creation failed 1`] = `
+Object {
+ "branchExists": true,
+ "prBlockedBy": "Error",
+ "result": "error",
+}
+`;
+
+exports[`workers/branch/index processBranch returns if branch automerge is pending 1`] = `
+Object {
+ "branchExists": true,
+ "prBlockedBy": "BranchAutomerge",
"result": "done",
}
`;
@@ -80,6 +115,7 @@ Object {
exports[`workers/branch/index processBranch returns if branch creation limit exceeded 1`] = `
Object {
"branchExists": false,
+ "prNo": undefined,
"result": "branch-limit-reached",
}
`;
@@ -87,6 +123,7 @@ Object {
exports[`workers/branch/index processBranch returns if branch exists and prCreation set to approval 1`] = `
Object {
"branchExists": true,
+ "prBlockedBy": "NeedsApproval",
"result": "needs-pr-approval",
}
`;
@@ -94,6 +131,7 @@ Object {
exports[`workers/branch/index processBranch returns if branch exists but pending 1`] = `
Object {
"branchExists": true,
+ "prBlockedBy": "AwaitingTests",
"result": "pending",
}
`;
@@ -101,6 +139,7 @@ Object {
exports[`workers/branch/index processBranch returns if branch exists but updated 1`] = `
Object {
"branchExists": true,
+ "prNo": undefined,
"result": "pending",
}
`;
@@ -108,6 +147,7 @@ Object {
exports[`workers/branch/index processBranch returns if commit limit exceeded 1`] = `
Object {
"branchExists": false,
+ "prNo": undefined,
"result": "commit-limit-reached",
}
`;
@@ -115,6 +155,7 @@ Object {
exports[`workers/branch/index processBranch returns if no work 1`] = `
Object {
"branchExists": false,
+ "prNo": undefined,
"result": "no-work",
}
`;
@@ -122,6 +163,7 @@ Object {
exports[`workers/branch/index processBranch returns if pending checks 1`] = `
Object {
"branchExists": false,
+ "prNo": undefined,
"result": "pending",
}
`;
@@ -129,6 +171,7 @@ Object {
exports[`workers/branch/index processBranch returns if pr creation limit exceeded and branch exists 1`] = `
Object {
"branchExists": true,
+ "prBlockedBy": "RateLimited",
"result": "pr-limit-reached",
}
`;
@@ -136,6 +179,7 @@ Object {
exports[`workers/branch/index processBranch skips branch for fresh release with stabilityDays 1`] = `
Object {
"branchExists": false,
+ "prNo": undefined,
"result": "pending",
}
`;
@@ -143,6 +187,7 @@ Object {
exports[`workers/branch/index processBranch skips branch if branch edited and and PR found with sha mismatch 1`] = `
Object {
"branchExists": true,
+ "prNo": undefined,
"result": "pr-edited",
}
`;
@@ -150,6 +195,7 @@ Object {
exports[`workers/branch/index processBranch skips branch if branch edited and no PR found 1`] = `
Object {
"branchExists": true,
+ "prNo": undefined,
"result": "pr-edited",
}
`;
@@ -157,6 +203,7 @@ Object {
exports[`workers/branch/index processBranch skips branch if edited PR found 1`] = `
Object {
"branchExists": true,
+ "prNo": undefined,
"result": "pr-edited",
}
`;
@@ -164,6 +211,7 @@ Object {
exports[`workers/branch/index processBranch skips branch if not scheduled and branch does not exist 1`] = `
Object {
"branchExists": false,
+ "prNo": undefined,
"result": "not-scheduled",
}
`;
@@ -171,6 +219,7 @@ Object {
exports[`workers/branch/index processBranch skips branch if not scheduled and not updating out of schedule 1`] = `
Object {
"branchExists": true,
+ "prNo": undefined,
"result": "not-scheduled",
}
`;
@@ -178,6 +227,7 @@ Object {
exports[`workers/branch/index processBranch skips branch if not stabilityDays not met 1`] = `
Object {
"branchExists": false,
+ "prNo": undefined,
"result": "pending",
}
`;
@@ -185,6 +235,7 @@ Object {
exports[`workers/branch/index processBranch skips branch if target branch changed 1`] = `
Object {
"branchExists": true,
+ "prNo": undefined,
"result": "pr-edited",
}
`;
@@ -192,6 +243,7 @@ Object {
exports[`workers/branch/index processBranch swallows branch errors 1`] = `
Object {
"branchExists": false,
+ "prNo": undefined,
"result": "error",
}
`;
@@ -199,6 +251,7 @@ Object {
exports[`workers/branch/index processBranch swallows pr errors 1`] = `
Object {
"branchExists": true,
+ "prNo": undefined,
"result": "done",
}
`;
@@ -206,6 +259,7 @@ Object {
exports[`workers/branch/index processBranch throws and swallows branch errors 1`] = `
Object {
"branchExists": true,
+ "prNo": undefined,
"result": "pr-created",
}
`;
diff --git a/lib/workers/branch/index.spec.ts b/lib/workers/branch/index.spec.ts
index 7a71e27a17..f284452a1a 100644
--- a/lib/workers/branch/index.spec.ts
+++ b/lib/workers/branch/index.spec.ts
@@ -24,7 +24,6 @@ import type { EnsurePrResult } from '../pr';
import * as _prAutomerge from '../pr/automerge';
import type { Pr } from '../repository/onboarding/branch/check';
import type { BranchConfig, BranchUpgradeConfig } from '../types';
-import { PrResult } from '../types';
import * as _automerge from './automerge';
import * as _checkExisting from './check-existing';
import * as _commit from './commit';
@@ -89,7 +88,6 @@ describe(getName(), () => {
platform.massageMarkdown.mockImplementation((prBody) => prBody);
prWorker.ensurePr.mockResolvedValue({
- prResult: PrResult.Created,
pr: {
title: '',
sourceBranch: '',
@@ -297,7 +295,7 @@ describe(getName(), () => {
});
git.branchExists.mockReturnValue(true);
prWorker.ensurePr.mockResolvedValueOnce({
- prResult: PrResult.LimitReached,
+ prBlockedBy: 'RateLimited',
});
limits.isLimitReached.mockReturnValue(false);
expect(await branchWorker.processBranch(config)).toMatchSnapshot();
@@ -403,7 +401,7 @@ describe(getName(), () => {
commit.commitFilesToBranch.mockResolvedValueOnce(null);
automerge.tryBranchAutomerge.mockResolvedValueOnce('failed');
prWorker.ensurePr.mockResolvedValueOnce({
- prResult: PrResult.AwaitingApproval,
+ prBlockedBy: 'NeedsApproval',
});
expect(await branchWorker.processBranch(config)).toMatchSnapshot();
});
@@ -420,7 +418,58 @@ describe(getName(), () => {
commit.commitFilesToBranch.mockResolvedValueOnce(null);
automerge.tryBranchAutomerge.mockResolvedValueOnce('failed');
prWorker.ensurePr.mockResolvedValueOnce({
- prResult: PrResult.AwaitingNotPending,
+ prBlockedBy: 'AwaitingTests',
+ });
+ expect(await branchWorker.processBranch(config)).toMatchSnapshot();
+ });
+ it('returns if branch automerge is pending', async () => {
+ expect.assertions(1);
+ getUpdated.getUpdatedPackageFiles.mockResolvedValueOnce({
+ updatedPackageFiles: [{}],
+ } as PackageFilesResult);
+ npmPostExtract.getAdditionalFiles.mockResolvedValueOnce({
+ artifactErrors: [],
+ updatedArtifacts: [{}],
+ } as WriteExistingFilesResult);
+ git.branchExists.mockReturnValue(true);
+ commit.commitFilesToBranch.mockResolvedValueOnce(null);
+ automerge.tryBranchAutomerge.mockResolvedValueOnce('no automerge');
+ prWorker.ensurePr.mockResolvedValueOnce({
+ prBlockedBy: 'BranchAutomerge',
+ });
+ expect(await branchWorker.processBranch(config)).toMatchSnapshot();
+ });
+ it('returns if PR creation failed', async () => {
+ expect.assertions(1);
+ getUpdated.getUpdatedPackageFiles.mockResolvedValueOnce({
+ updatedPackageFiles: [{}],
+ } as PackageFilesResult);
+ npmPostExtract.getAdditionalFiles.mockResolvedValueOnce({
+ artifactErrors: [],
+ updatedArtifacts: [{}],
+ } as WriteExistingFilesResult);
+ git.branchExists.mockReturnValue(true);
+ commit.commitFilesToBranch.mockResolvedValueOnce(null);
+ automerge.tryBranchAutomerge.mockResolvedValueOnce('failed');
+ prWorker.ensurePr.mockResolvedValueOnce({
+ prBlockedBy: 'Error',
+ });
+ expect(await branchWorker.processBranch(config)).toMatchSnapshot();
+ });
+ it('handles unknown PrBlockedBy', async () => {
+ expect.assertions(1);
+ getUpdated.getUpdatedPackageFiles.mockResolvedValueOnce({
+ updatedPackageFiles: [{}],
+ } as PackageFilesResult);
+ npmPostExtract.getAdditionalFiles.mockResolvedValueOnce({
+ artifactErrors: [],
+ updatedArtifacts: [{}],
+ } as WriteExistingFilesResult);
+ git.branchExists.mockReturnValue(true);
+ commit.commitFilesToBranch.mockResolvedValueOnce(null);
+ automerge.tryBranchAutomerge.mockResolvedValueOnce('failed');
+ prWorker.ensurePr.mockResolvedValueOnce({
+ prBlockedBy: 'whoops' as any,
});
expect(await branchWorker.processBranch(config)).toMatchSnapshot();
});
@@ -455,7 +504,6 @@ describe(getName(), () => {
git.branchExists.mockReturnValue(true);
automerge.tryBranchAutomerge.mockResolvedValueOnce('failed');
prWorker.ensurePr.mockResolvedValueOnce({
- prResult: PrResult.Created,
pr: {},
} as EnsurePrResult);
prAutomerge.checkAutoMerge.mockResolvedValueOnce({ automerged: true });
@@ -476,7 +524,6 @@ describe(getName(), () => {
git.branchExists.mockReturnValue(true);
automerge.tryBranchAutomerge.mockResolvedValueOnce('stale');
prWorker.ensurePr.mockResolvedValueOnce({
- prResult: PrResult.Created,
pr: {},
} as EnsurePrResult);
prAutomerge.checkAutoMerge.mockResolvedValueOnce({ automerged: false });
@@ -501,7 +548,6 @@ describe(getName(), () => {
git.branchExists.mockReturnValue(true);
automerge.tryBranchAutomerge.mockResolvedValueOnce('failed');
prWorker.ensurePr.mockResolvedValueOnce({
- prResult: PrResult.Created,
pr: {},
} as EnsurePrResult);
prAutomerge.checkAutoMerge.mockResolvedValueOnce({ automerged: true });
@@ -522,7 +568,6 @@ describe(getName(), () => {
git.branchExists.mockReturnValue(true);
automerge.tryBranchAutomerge.mockResolvedValueOnce('failed');
prWorker.ensurePr.mockResolvedValueOnce({
- prResult: PrResult.Created,
pr: {},
} as EnsurePrResult);
prAutomerge.checkAutoMerge.mockResolvedValueOnce({ automerged: true });
@@ -544,7 +589,6 @@ describe(getName(), () => {
git.branchExists.mockReturnValue(true);
automerge.tryBranchAutomerge.mockResolvedValueOnce('failed');
prWorker.ensurePr.mockResolvedValueOnce({
- prResult: PrResult.Created,
pr: {},
} as EnsurePrResult);
prAutomerge.checkAutoMerge.mockResolvedValueOnce({ automerged: true });
@@ -566,7 +610,6 @@ describe(getName(), () => {
git.branchExists.mockReturnValue(false);
automerge.tryBranchAutomerge.mockResolvedValueOnce('failed');
prWorker.ensurePr.mockResolvedValueOnce({
- prResult: PrResult.Created,
pr: {},
} as EnsurePrResult);
prAutomerge.checkAutoMerge.mockResolvedValueOnce({ automerged: true });
@@ -587,7 +630,6 @@ describe(getName(), () => {
git.branchExists.mockReturnValue(true);
automerge.tryBranchAutomerge.mockResolvedValueOnce('failed');
prWorker.ensurePr.mockResolvedValueOnce({
- prResult: PrResult.Created,
pr: {},
} as EnsurePrResult);
prAutomerge.checkAutoMerge.mockResolvedValueOnce({ automerged: true });
@@ -698,7 +740,6 @@ describe(getName(), () => {
git.isBranchModified.mockResolvedValueOnce(true);
schedule.isScheduledNow.mockReturnValueOnce(false);
prWorker.ensurePr.mockResolvedValueOnce({
- prResult: PrResult.Created,
pr: {},
} as EnsurePrResult);
commit.commitFilesToBranch.mockResolvedValueOnce(null);
diff --git a/lib/workers/branch/index.ts b/lib/workers/branch/index.ts
index 36b8199c2f..bb71ec0f49 100644
--- a/lib/workers/branch/index.ts
+++ b/lib/workers/branch/index.ts
@@ -30,7 +30,7 @@ import {
import { Limit, isLimitReached } from '../global/limits';
import { ensurePr, getPlatformPrOptions } from '../pr';
import { checkAutoMerge } from '../pr/automerge';
-import { BranchConfig, BranchResult, PrResult } from '../types';
+import { BranchConfig, BranchResult, PrBlockedBy } from '../types';
import { tryBranchAutomerge } from './automerge';
import { prAlreadyExisted } from './check-existing';
import { commitFilesToBranch } from './commit';
@@ -63,6 +63,8 @@ async function deleteBranchSilently(branchName: string): Promise<void> {
export interface ProcessBranchResult {
branchExists: boolean;
+ prBlockedBy?: PrBlockedBy;
+ prNo?: number;
result: BranchResult;
}
@@ -73,7 +75,7 @@ export async function processBranch(
logger.trace({ config }, 'processBranch()');
await checkoutBranch(config.baseBranch);
const branchExists = gitBranchExists(config.branchName);
- const branchPr = await platform.getBranchPr(config.branchName);
+ let branchPr = await platform.getBranchPr(config.branchName);
logger.debug(`branchExists=${branchExists}`);
const dependencyDashboardCheck =
config.dependencyDashboardChecks?.[config.branchName];
@@ -92,7 +94,11 @@ export async function processBranch(
'Closed PR already exists. Skipping branch.'
);
await handlepr(config, existingPr);
- return { branchExists: false, result: BranchResult.AlreadyExisted };
+ return {
+ branchExists: false,
+ prNo: existingPr.number,
+ result: BranchResult.AlreadyExisted,
+ };
}
// istanbul ignore if
if (!branchExists && config.dependencyDashboardApproval) {
@@ -100,7 +106,11 @@ export async function processBranch(
logger.debug(`Branch ${config.branchName} is approved for creation`);
} else {
logger.debug(`Branch ${config.branchName} needs approval`);
- return { branchExists, result: BranchResult.NeedsApproval };
+ return {
+ branchExists,
+ prNo: branchPr?.number,
+ result: BranchResult.NeedsApproval,
+ };
}
}
if (
@@ -110,7 +120,11 @@ export async function processBranch(
!config.isVulnerabilityAlert
) {
logger.debug('Reached branch limit - skipping branch creation');
- return { branchExists, result: BranchResult.BranchLimitReached };
+ return {
+ branchExists,
+ prNo: branchPr?.number,
+ result: BranchResult.BranchLimitReached,
+ };
}
if (
isLimitReached(Limit.Commits) &&
@@ -118,14 +132,22 @@ export async function processBranch(
!config.isVulnerabilityAlert
) {
logger.debug('Reached commits limit - skipping branch');
- return { branchExists, result: BranchResult.CommitLimitReached };
+ return {
+ branchExists,
+ prNo: branchPr?.number,
+ result: BranchResult.CommitLimitReached,
+ };
}
if (
!branchExists &&
branchConfig.pendingChecks &&
!dependencyDashboardCheck
) {
- return { branchExists: false, result: BranchResult.Pending };
+ return {
+ branchExists: false,
+ prNo: branchPr?.number,
+ result: BranchResult.Pending,
+ };
}
if (branchExists) {
logger.debug('Checking if PR has been edited');
@@ -162,7 +184,11 @@ export async function processBranch(
platformOptions: getPlatformPrOptions(config),
});
}
- return { branchExists, result: BranchResult.PrEdited };
+ return {
+ branchExists,
+ prNo: branchPr.number,
+ result: BranchResult.PrEdited,
+ };
}
}
} else if (branchIsModified) {
@@ -172,7 +198,11 @@ export async function processBranch(
});
if (!oldPr) {
logger.debug('Branch has been edited but found no PR - skipping');
- return { branchExists, result: BranchResult.PrEdited };
+ return {
+ branchExists,
+ prNo: branchPr?.number,
+ result: BranchResult.PrEdited,
+ };
}
const branchSha = getBranchCommit(config.branchName);
const oldPrSha = oldPr?.sha;
@@ -186,7 +216,11 @@ export async function processBranch(
{ oldPrNumber: oldPr.number, oldPrSha, branchSha },
'Found old PR but the SHA is different'
);
- return { branchExists, result: BranchResult.PrEdited };
+ return {
+ branchExists,
+ prNo: branchPr?.number,
+ result: BranchResult.PrEdited,
+ };
}
}
}
@@ -196,16 +230,28 @@ export async function processBranch(
if (!config.isScheduledNow && !dependencyDashboardCheck) {
if (!branchExists) {
logger.debug('Skipping branch creation as not within schedule');
- return { branchExists, result: BranchResult.NotScheduled };
+ return {
+ branchExists,
+ prNo: branchPr?.number,
+ result: BranchResult.NotScheduled,
+ };
}
if (config.updateNotScheduled === false && !config.rebaseRequested) {
logger.debug('Skipping branch update as not within schedule');
- return { branchExists, result: BranchResult.NotScheduled };
+ return {
+ branchExists,
+ prNo: branchPr?.number,
+ result: BranchResult.NotScheduled,
+ };
}
// istanbul ignore if
if (!branchPr) {
logger.debug('Skipping PR creation out of schedule');
- return { branchExists, result: BranchResult.NotScheduled };
+ return {
+ branchExists,
+ prNo: branchPr?.number,
+ result: BranchResult.NotScheduled,
+ };
}
logger.debug(
'Branch + PR exists but is not scheduled -- will update if necessary'
@@ -245,7 +291,11 @@ export async function processBranch(
['not-pending', 'status-success'].includes(config.prCreation)
) {
logger.debug('Skipping branch creation due to stability days not met');
- return { branchExists, result: BranchResult.Pending };
+ return {
+ branchExists,
+ prNo: branchPr?.number,
+ result: BranchResult.Pending,
+ };
}
}
@@ -351,7 +401,11 @@ export async function processBranch(
await platform.refreshPr(branchPr.number);
}
if (!commitSha && !branchExists) {
- return { branchExists, result: BranchResult.NoWork };
+ return {
+ branchExists,
+ prNo: branchPr?.number,
+ result: BranchResult.NoWork,
+ };
}
if (commitSha) {
const action = branchExists ? 'updated' : 'created';
@@ -368,7 +422,11 @@ export async function processBranch(
(config.requiredStatusChecks?.length || config.prCreation !== 'immediate')
) {
logger.debug({ commitSha }, `Branch status pending`);
- return { branchExists: true, result: BranchResult.Pending };
+ return {
+ branchExists: true,
+ prNo: branchPr?.number,
+ result: BranchResult.Pending,
+ };
}
// Try to automerge branch and finish if successful, but only if branch already existed before this run
@@ -456,7 +514,11 @@ export async function processBranch(
logger.warn('Error updating branch: update failure');
} else if (err.message.startsWith('bundler-')) {
// we have already warned inside the bundler artifacts error handling, so just return
- return { branchExists: true, result: BranchResult.Error };
+ return {
+ branchExists: true,
+ prNo: branchPr?.number,
+ result: BranchResult.Error,
+ };
} else if (
err.messagee &&
err.message.includes('fatal: Authentication failed')
@@ -475,27 +537,47 @@ export async function processBranch(
logger.warn({ err }, `Error updating branch`);
}
// Don't throw here - we don't want to stop the other renovations
- return { branchExists, result: BranchResult.Error };
+ return { branchExists, prNo: branchPr?.number, result: BranchResult.Error };
}
try {
logger.debug('Ensuring PR');
logger.debug(
`There are ${config.errors.length} errors and ${config.warnings.length} warnings`
);
- const { prResult: result, pr } = await ensurePr(config);
- if (result === PrResult.LimitReached && !config.isVulnerabilityAlert) {
- logger.debug('Reached PR limit - skipping PR creation');
- return { branchExists, result: BranchResult.PrLimitReached };
- }
- // TODO: ensurePr should check for automerge itself (#9719)
- if (result === PrResult.AwaitingApproval) {
- return { branchExists, result: BranchResult.NeedsPrApproval };
- }
- if (
- result === PrResult.AwaitingGreenBranch ||
- result === PrResult.AwaitingNotPending
- ) {
- return { branchExists, result: BranchResult.Pending };
+ const { prBlockedBy, pr } = await ensurePr(config);
+ branchPr = pr;
+ if (prBlockedBy) {
+ if (prBlockedBy === 'RateLimited' && !config.isVulnerabilityAlert) {
+ logger.debug('Reached PR limit - skipping PR creation');
+ return {
+ branchExists,
+ prBlockedBy,
+ result: BranchResult.PrLimitReached,
+ };
+ }
+ // TODO: ensurePr should check for automerge itself (#9719)
+ if (prBlockedBy === 'NeedsApproval') {
+ return {
+ branchExists,
+ prBlockedBy,
+ result: BranchResult.NeedsPrApproval,
+ };
+ }
+ if (prBlockedBy === 'AwaitingTests') {
+ return { branchExists, prBlockedBy, result: BranchResult.Pending };
+ }
+ if (prBlockedBy === 'BranchAutomerge') {
+ return {
+ branchExists,
+ prBlockedBy,
+ result: BranchResult.Done,
+ };
+ }
+ if (prBlockedBy === 'Error') {
+ return { branchExists, prBlockedBy, result: BranchResult.Error };
+ }
+ logger.warn({ prBlockedBy }, 'Unknown PrBlockedBy result');
+ return { branchExists, prBlockedBy, result: BranchResult.Error };
}
if (pr) {
if (config.artifactErrors?.length) {
@@ -587,7 +669,11 @@ export async function processBranch(
logger.error({ err }, `Error ensuring PR: ${String(err.message)}`);
}
if (!branchExists) {
- return { branchExists: true, result: BranchResult.PrCreated };
+ return {
+ branchExists: true,
+ prNo: branchPr?.number,
+ result: BranchResult.PrCreated,
+ };
}
- return { branchExists, result: BranchResult.Done };
+ return { branchExists, prNo: branchPr?.number, result: BranchResult.Done };
}
diff --git a/lib/workers/pr/index.spec.ts b/lib/workers/pr/index.spec.ts
index edd068b3dc..4d0e5f62ea 100644
--- a/lib/workers/pr/index.spec.ts
+++ b/lib/workers/pr/index.spec.ts
@@ -4,7 +4,7 @@ import { PLATFORM_TYPE_GITLAB } from '../../constants/platforms';
import { Pr, platform as _platform } from '../../platform';
import { BranchStatus } from '../../types';
import * as _limits from '../global/limits';
-import { BranchConfig, PrResult } from '../types';
+import type { BranchConfig } from '../types';
import * as prAutomerge from './automerge';
import * as _changelogHelper from './changelog';
import { getChangeLogJSON } from './changelog';
@@ -206,28 +206,27 @@ describe(getName(), () => {
afterEach(() => {
jest.clearAllMocks();
});
- it('should return null if check fails', async () => {
+ it('should return PR if update fails', async () => {
platform.updatePr.mockImplementationOnce(() => {
throw new Error('oops');
});
config.newValue = '1.2.0';
platform.getBranchPr.mockResolvedValueOnce(existingPr);
- const { prResult, pr } = await prWorker.ensurePr(config);
- expect(prResult).toEqual(PrResult.Error);
- expect(pr).toBeUndefined();
+ const { pr } = await prWorker.ensurePr(config);
+ expect(pr).toBeDefined();
});
it('should return null if waiting for success', async () => {
platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.red);
config.prCreation = 'status-success';
- const { prResult, pr } = await prWorker.ensurePr(config);
- expect(prResult).toEqual(PrResult.AwaitingGreenBranch);
+ const { prBlockedBy, pr } = await prWorker.ensurePr(config);
+ expect(prBlockedBy).toEqual('AwaitingTests');
expect(pr).toBeUndefined();
});
it('should return needs-approval if prCreation set to approval', async () => {
platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.green);
config.prCreation = 'approval';
- const { prResult, pr } = await prWorker.ensurePr(config);
- expect(prResult).toEqual(PrResult.AwaitingApproval);
+ const { prBlockedBy, pr } = await prWorker.ensurePr(config);
+ expect(prBlockedBy).toEqual('NeedsApproval');
expect(pr).toBeUndefined();
});
it('should create PR if success for gitlab deps', async () => {
@@ -241,8 +240,7 @@ describe(getName(), () => {
config.prCreation = 'status-success';
config.automerge = true;
config.schedule = ['before 5am'];
- const { prResult, pr } = await prWorker.ensurePr(config);
- expect(prResult).toEqual(PrResult.Created);
+ const { pr } = await prWorker.ensurePr(config);
expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
expect(platform.createPr.mock.calls[0]).toMatchSnapshot();
existingPr.body = platform.createPr.mock.calls[0][0].prBody;
@@ -257,8 +255,7 @@ describe(getName(), () => {
config.prCreation = 'status-success';
config.automerge = true;
config.schedule = ['before 5am'];
- const { prResult, pr } = await prWorker.ensurePr(config);
- expect(prResult).toEqual(PrResult.Created);
+ const { pr } = await prWorker.ensurePr(config);
expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
expect(platform.createPr.mock.calls[0]).toMatchSnapshot();
existingPr.body = platform.createPr.mock.calls[0][0].prBody;
@@ -270,8 +267,8 @@ describe(getName(), () => {
config.automerge = true;
config.schedule = ['before 5am'];
limits.isLimitReached.mockReturnValueOnce(true);
- const { prResult } = await prWorker.ensurePr(config);
- expect(prResult).toEqual(PrResult.LimitReached);
+ const { prBlockedBy } = await prWorker.ensurePr(config);
+ expect(prBlockedBy).toEqual('RateLimited');
expect(platform.createPr.mock.calls).toBeEmpty();
});
it('should create PR if limit is reached but dashboard checked', async () => {
@@ -281,11 +278,10 @@ describe(getName(), () => {
config.automerge = true;
config.schedule = ['before 5am'];
limits.isLimitReached.mockReturnValueOnce(true);
- const { prResult } = await prWorker.ensurePr({
+ await prWorker.ensurePr({
...config,
dependencyDashboardChecks: { 'renovate/dummy-1.x': 'true' },
});
- expect(prResult).toEqual(PrResult.Created);
expect(platform.createPr).toHaveBeenCalled();
});
it('should create group PR', async () => {
@@ -319,8 +315,7 @@ describe(getName(), () => {
for (const upgrade of config.upgrades) {
upgrade.logJSON = await getChangeLogJSON(upgrade);
}
- const { prResult, pr } = await prWorker.ensurePr(config);
- expect(prResult).toEqual(PrResult.Created);
+ const { pr } = await prWorker.ensurePr(config);
expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
expect(platform.createPr.mock.calls[0]).toMatchSnapshot();
});
@@ -333,8 +328,7 @@ describe(getName(), () => {
config.timezone = 'some timezone';
config.rebaseWhen = 'behind-base-branch';
config.logJSON = await getChangeLogJSON(config);
- const { prResult, pr } = await prWorker.ensurePr(config);
- expect(prResult).toEqual(PrResult.Created);
+ const { pr } = await prWorker.ensurePr(config);
expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
expect(platform.createPr.mock.calls[0]).toMatchSnapshot();
expect(platform.createPr.mock.calls[0][0].prBody).toContain(
@@ -348,8 +342,8 @@ describe(getName(), () => {
throw new Error('Validation Failed (422)');
});
config.prCreation = 'status-success';
- const { prResult, pr } = await prWorker.ensurePr(config);
- expect(prResult).toEqual(PrResult.Error);
+ const { prBlockedBy, pr } = await prWorker.ensurePr(config);
+ expect(prBlockedBy).toEqual('Error');
expect(pr).toBeUndefined();
});
it('should return null if waiting for not pending', async () => {
@@ -358,8 +352,8 @@ describe(getName(), () => {
Promise.resolve(new Date())
);
config.prCreation = 'not-pending';
- const { prResult, pr } = await prWorker.ensurePr(config);
- expect(prResult).toEqual(PrResult.AwaitingNotPending);
+ const { prBlockedBy, pr } = await prWorker.ensurePr(config);
+ expect(prBlockedBy).toEqual('AwaitingTests');
expect(pr).toBeUndefined();
});
it('should not create PR if waiting for not pending with stabilityStatus yellow', async () => {
@@ -369,8 +363,8 @@ describe(getName(), () => {
);
config.prCreation = 'not-pending';
config.stabilityStatus = BranchStatus.yellow;
- const { prResult, pr } = await prWorker.ensurePr(config);
- expect(prResult).toEqual(PrResult.AwaitingNotPending);
+ const { prBlockedBy, pr } = await prWorker.ensurePr(config);
+ expect(prBlockedBy).toEqual('AwaitingTests');
expect(pr).toBeUndefined();
});
it('should create PR if pending timeout hit', async () => {
@@ -380,27 +374,23 @@ describe(getName(), () => {
);
config.prCreation = 'not-pending';
config.stabilityStatus = BranchStatus.yellow;
- const { prResult, pr } = await prWorker.ensurePr(config);
- expect(prResult).toEqual(PrResult.Created);
+ const { pr } = await prWorker.ensurePr(config);
expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
});
it('should create PR if no longer pending', async () => {
platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.red);
config.prCreation = 'not-pending';
- const { prResult, pr } = await prWorker.ensurePr(config);
- expect(prResult).toEqual(PrResult.Created);
+ const { pr } = await prWorker.ensurePr(config);
expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
});
it('should create new branch if none exists', async () => {
- const { prResult, pr } = await prWorker.ensurePr(config);
- expect(prResult).toEqual(PrResult.Created);
+ const { pr } = await prWorker.ensurePr(config);
expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
});
it('should add assignees and reviewers to new PR', async () => {
config.assignees = ['@foo', 'bar'];
config.reviewers = ['baz', '@boo'];
- const { prResult, pr } = await prWorker.ensurePr(config);
- expect(prResult).toEqual(PrResult.Created);
+ const { pr } = await prWorker.ensurePr(config);
expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
expect(platform.addAssignees).toHaveBeenCalledTimes(1);
expect(platform.addAssignees.mock.calls).toMatchSnapshot();
@@ -446,8 +436,7 @@ describe(getName(), () => {
});
config.assignees = ['@foo', 'bar'];
config.reviewers = ['baz', '@boo'];
- const { prResult, pr } = await prWorker.ensurePr(config);
- expect(prResult).toEqual(PrResult.Created);
+ const { pr } = await prWorker.ensurePr(config);
expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
expect(platform.addAssignees).toHaveBeenCalledTimes(1);
expect(platform.addReviewers).toHaveBeenCalledTimes(1);
@@ -458,8 +447,7 @@ describe(getName(), () => {
});
config.assignees = ['@foo', 'bar'];
config.reviewers = ['baz', '@boo'];
- const { prResult, pr } = await prWorker.ensurePr(config);
- expect(prResult).toEqual(PrResult.Created);
+ const { pr } = await prWorker.ensurePr(config);
expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
expect(platform.addAssignees).toHaveBeenCalledTimes(1);
expect(platform.addReviewers).toHaveBeenCalledTimes(1);
@@ -468,8 +456,7 @@ describe(getName(), () => {
config.assignees = ['bar'];
config.reviewers = ['baz'];
config.automerge = true;
- const { prResult, pr } = await prWorker.ensurePr(config);
- expect(prResult).toEqual(PrResult.Created);
+ const { pr } = await prWorker.ensurePr(config);
expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
expect(platform.addAssignees).toHaveBeenCalledTimes(0);
expect(platform.addReviewers).toHaveBeenCalledTimes(0);
@@ -479,8 +466,7 @@ describe(getName(), () => {
config.reviewers = ['baz'];
config.automerge = true;
config.assignAutomerge = true;
- const { prResult, pr } = await prWorker.ensurePr(config);
- expect(prResult).toEqual(PrResult.Created);
+ const { pr } = await prWorker.ensurePr(config);
expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
expect(platform.addAssignees).toHaveBeenCalledTimes(1);
expect(platform.addReviewers).toHaveBeenCalledTimes(1);
@@ -490,8 +476,7 @@ describe(getName(), () => {
config.assigneesSampleSize = 2;
config.reviewers = ['baz', 'boo', 'bor'];
config.reviewersSampleSize = 2;
- const { prResult, pr } = await prWorker.ensurePr(config);
- expect(prResult).toEqual(PrResult.Created);
+ const { pr } = await prWorker.ensurePr(config);
expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
expect(platform.addAssignees).toHaveBeenCalledTimes(1);
const assignees = platform.addAssignees.mock.calls[0][1];
@@ -508,8 +493,7 @@ describe(getName(), () => {
config.assigneesSampleSize = 0;
config.reviewers = ['baz', 'boo', 'bor'];
config.reviewersSampleSize = 0;
- const { prResult, pr } = await prWorker.ensurePr(config);
- expect(prResult).toEqual(PrResult.Created);
+ const { pr } = await prWorker.ensurePr(config);
expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
expect(platform.addAssignees).toHaveBeenCalledTimes(0);
expect(platform.addReviewers).toHaveBeenCalledTimes(0);
@@ -517,8 +501,7 @@ describe(getName(), () => {
it('should add and deduplicate additionalReviewers on new PR', async () => {
config.reviewers = ['@foo', 'bar'];
config.additionalReviewers = ['bar', 'baz', '@boo'];
- const { prResult, pr } = await prWorker.ensurePr(config);
- expect(prResult).toEqual(PrResult.Created);
+ const { pr } = await prWorker.ensurePr(config);
expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
expect(platform.addReviewers).toHaveBeenCalledTimes(1);
expect(platform.addReviewers.mock.calls).toMatchSnapshot();
@@ -526,8 +509,7 @@ describe(getName(), () => {
it('should add and deduplicate additionalReviewers to empty reviewers on new PR', async () => {
config.reviewers = [];
config.additionalReviewers = ['bar', 'baz', '@boo', '@foo', 'bar'];
- const { prResult, pr } = await prWorker.ensurePr(config);
- expect(prResult).toEqual(PrResult.Created);
+ const { pr } = await prWorker.ensurePr(config);
expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
expect(platform.addReviewers).toHaveBeenCalledTimes(1);
expect(platform.addReviewers.mock.calls).toMatchSnapshot();
@@ -538,8 +520,7 @@ describe(getName(), () => {
config.automerge = true;
config.schedule = ['before 5am'];
config.logJSON = await getChangeLogJSON(config);
- const { prResult, pr } = await prWorker.ensurePr(config);
- expect(prResult).toEqual(PrResult.NotUpdated);
+ const { pr } = await prWorker.ensurePr(config);
expect(platform.updatePr.mock.calls).toMatchSnapshot();
expect(platform.updatePr).toHaveBeenCalledTimes(0);
expect(pr).toMatchObject(existingPr);
@@ -553,8 +534,7 @@ describe(getName(), () => {
config.automerge = true;
config.schedule = ['before 5am'];
config.logJSON = await getChangeLogJSON(config);
- const { prResult, pr } = await prWorker.ensurePr(config);
- expect(prResult).toEqual(PrResult.NotUpdated);
+ const { pr } = await prWorker.ensurePr(config);
expect(platform.updatePr).toHaveBeenCalledTimes(0);
expect(pr).toMatchObject(modifiedPr);
});
@@ -564,8 +544,7 @@ describe(getName(), () => {
config.schedule = ['before 5am'];
config.logJSON = await getChangeLogJSON(config);
platform.getBranchPr.mockResolvedValueOnce(existingPr);
- const { prResult, pr } = await prWorker.ensurePr(config);
- expect(prResult).toEqual(PrResult.NotUpdated);
+ const { pr } = await prWorker.ensurePr(config);
expect(pr).toMatchSnapshot();
});
it('should return modified existing PR title', async () => {
@@ -574,8 +553,7 @@ describe(getName(), () => {
...existingPr,
title: 'wrong',
});
- const { prResult, pr } = await prWorker.ensurePr(config);
- expect(prResult).toEqual(PrResult.Updated);
+ const { pr } = await prWorker.ensurePr(config);
expect(pr).toMatchSnapshot();
});
it('should create PR if branch tests failed', async () => {
@@ -583,8 +561,7 @@ describe(getName(), () => {
config.automergeType = 'branch';
config.branchAutomergeFailureMessage = 'branch status error';
platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.red);
- const { prResult, pr } = await prWorker.ensurePr(config);
- expect(prResult).toEqual(PrResult.Created);
+ const { pr } = await prWorker.ensurePr(config);
expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
});
it('should create PR if branch automerging failed', async () => {
@@ -592,8 +569,7 @@ describe(getName(), () => {
config.automergeType = 'branch';
platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.green);
config.forcePr = true;
- const { prResult, pr } = await prWorker.ensurePr(config);
- expect(prResult).toEqual(PrResult.Created);
+ const { pr } = await prWorker.ensurePr(config);
expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
});
it('should return no PR if branch automerging not failed', async () => {
@@ -601,8 +577,8 @@ describe(getName(), () => {
config.automergeType = 'branch';
platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.yellow);
git.getBranchLastCommitTime.mockResolvedValueOnce(new Date());
- const { prResult, pr } = await prWorker.ensurePr(config);
- expect(prResult).toEqual(PrResult.BlockedByBranchAutomerge);
+ const { prBlockedBy, pr } = await prWorker.ensurePr(config);
+ expect(prBlockedBy).toEqual('BranchAutomerge');
expect(pr).toBeUndefined();
});
it('should return PR if branch automerging taking too long', async () => {
@@ -610,8 +586,7 @@ describe(getName(), () => {
config.automergeType = 'branch';
platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.yellow);
git.getBranchLastCommitTime.mockResolvedValueOnce(new Date('2018-01-01'));
- const { prResult, pr } = await prWorker.ensurePr(config);
- expect(prResult).toEqual(PrResult.Created);
+ const { pr } = await prWorker.ensurePr(config);
expect(pr).toBeDefined();
});
it('should return no PR if stabilityStatus yellow', async () => {
@@ -620,14 +595,13 @@ describe(getName(), () => {
config.stabilityStatus = BranchStatus.yellow;
platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.yellow);
git.getBranchLastCommitTime.mockResolvedValueOnce(new Date('2018-01-01'));
- const { prResult, pr } = await prWorker.ensurePr(config);
- expect(prResult).toEqual(PrResult.BlockedByBranchAutomerge);
+ const { prBlockedBy, pr } = await prWorker.ensurePr(config);
+ expect(prBlockedBy).toEqual('BranchAutomerge');
expect(pr).toBeUndefined();
});
it('handles duplicate upgrades', async () => {
config.upgrades.push(config.upgrades[0]);
- const { prResult, pr } = await prWorker.ensurePr(config);
- expect(prResult).toEqual(PrResult.Created);
+ const { pr } = await prWorker.ensurePr(config);
expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
});
it('should create privateRepo PR if success', async () => {
@@ -637,8 +611,7 @@ describe(getName(), () => {
config.logJSON = await getChangeLogJSON(config);
config.logJSON.project.gitlab = 'someproject';
delete config.logJSON.project.github;
- const { prResult, pr } = await prWorker.ensurePr(config);
- expect(prResult).toEqual(PrResult.Created);
+ const { pr } = await prWorker.ensurePr(config);
expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
expect(platform.createPr.mock.calls[0]).toMatchSnapshot();
existingPr.body = platform.createPr.mock.calls[0][0].prBody;
@@ -649,8 +622,7 @@ describe(getName(), () => {
config.prCreation = 'not-pending';
config.artifactErrors = [{}];
config.platform = PLATFORM_TYPE_GITLAB;
- const { prResult, pr } = await prWorker.ensurePr(config);
- expect(prResult).toEqual(PrResult.Created);
+ const { pr } = await prWorker.ensurePr(config);
expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
});
@@ -667,8 +639,8 @@ describe(getName(), () => {
it('should create a PR with set of labels and mergeable addLabels', async () => {
config.labels = ['deps', 'renovate'];
config.addLabels = ['deps', 'js'];
- const { prResult } = await prWorker.ensurePr(config);
- expect(prResult).toEqual(PrResult.Created);
+ const { pr } = await prWorker.ensurePr(config);
+ expect(pr).toBeDefined();
expect(platform.createPr.mock.calls[0][0]).toMatchObject({
labels: ['deps', 'renovate', 'js'],
});
diff --git a/lib/workers/pr/index.ts b/lib/workers/pr/index.ts
index d5a92eb901..b4bd2b75b9 100644
--- a/lib/workers/pr/index.ts
+++ b/lib/workers/pr/index.ts
@@ -14,7 +14,7 @@ import { stripEmojis } from '../../util/emoji';
import { deleteBranch, getBranchLastCommitTime } from '../../util/git';
import * as template from '../../util/template';
import { Limit, incLimitedValue, isLimitReached } from '../global/limits';
-import { BranchConfig, PrResult } from '../types';
+import type { BranchConfig, PrBlockedBy } from '../types';
import { getPrBody } from './body';
import { ChangeLogError } from './changelog/types';
import { codeOwnersForPr } from './code-owners';
@@ -126,11 +126,19 @@ export function getPlatformPrOptions(
config.gitLabAutomerge,
};
}
-export type EnsurePrResult = {
- prResult: PrResult;
- pr?: Pr;
+
+export type ResultWithPr = {
+ pr: Pr;
+ prBlockedBy?: never;
+};
+
+export type ResultWithoutPr = {
+ pr?: never;
+ prBlockedBy: PrBlockedBy;
};
+export type EnsurePrResult = ResultWithPr | ResultWithoutPr;
+
// Ensures that PR exists with matching title/body
export async function ensurePr(
prConfig: BranchConfig
@@ -196,7 +204,7 @@ export async function ensurePr(
logger.debug(`Branch tests failed, so will create PR`);
} else {
// Branch should be automerged, so we don't want to create a PR
- return { prResult: PrResult.BlockedByBranchAutomerge };
+ return { prBlockedBy: 'BranchAutomerge' };
}
}
if (config.prCreation === 'status-success') {
@@ -205,7 +213,7 @@ export async function ensurePr(
logger.debug(
`Branch status is "${await getBranchStatus()}" - not creating PR`
);
- return { prResult: PrResult.AwaitingGreenBranch };
+ return { prBlockedBy: 'AwaitingTests' };
}
logger.debug('Branch status success');
} else if (
@@ -213,7 +221,7 @@ export async function ensurePr(
!existingPr &&
dependencyDashboardCheck !== 'approvePr'
) {
- return { prResult: PrResult.AwaitingApproval };
+ return { prBlockedBy: 'NeedsApproval' };
} else if (
config.prCreation === 'not-pending' &&
!existingPr &&
@@ -239,7 +247,9 @@ export async function ensurePr(
logger.debug(
`Branch is ${elapsedHours} hours old - skipping PR creation`
);
- return { prResult: PrResult.AwaitingNotPending };
+ return {
+ prBlockedBy: 'AwaitingTests',
+ };
}
const prNotPendingHours = String(config.prNotPendingHours);
logger.debug(
@@ -351,7 +361,7 @@ export async function ensurePr(
noWhitespaceOrHeadings(stripEmojis(prBody))
) {
logger.debug(`${existingPr.displayNumber} does not need updating`);
- return { prResult: PrResult.NotUpdated, pr: existingPr };
+ return { pr: existingPr };
}
// PR must need updating
if (existingPrTitle !== newPrTitle) {
@@ -383,7 +393,9 @@ export async function ensurePr(
});
logger.info({ pr: existingPr.number, prTitle }, `PR updated`);
}
- return { prResult: PrResult.Updated, pr: existingPr };
+ return {
+ pr: existingPr,
+ };
}
logger.debug({ branch: branchName, prTitle }, `Creating PR`);
// istanbul ignore if
@@ -403,7 +415,7 @@ export async function ensurePr(
!config.isVulnerabilityAlert
) {
logger.debug('Skipping PR - limit reached');
- return { prResult: PrResult.LimitReached };
+ return { prBlockedBy: 'RateLimited' };
}
pr = await platform.createPr({
sourceBranch: branchName,
@@ -429,7 +441,7 @@ export async function ensurePr(
)
) {
logger.warn('A pull requests already exists');
- return { prResult: PrResult.ErrorAlreadyExists };
+ return { prBlockedBy: 'Error' };
}
if (err.statusCode === 502) {
logger.warn(
@@ -442,7 +454,7 @@ export async function ensurePr(
await deleteBranch(branchName);
}
}
- return { prResult: PrResult.Error };
+ return { prBlockedBy: 'Error' };
}
if (
config.branchAutomergeFailureMessage &&
@@ -480,7 +492,7 @@ export async function ensurePr(
await addAssigneesReviewers(config, pr);
}
logger.debug(`Created ${pr.displayNumber}`);
- return { prResult: PrResult.Created, pr };
+ return { pr };
} catch (err) {
// istanbul ignore if
if (
@@ -494,5 +506,9 @@ export async function ensurePr(
}
logger.error({ err }, 'Failed to ensure PR: ' + prTitle);
}
- return { prResult: PrResult.Error };
+ if (existingPr) {
+ return { pr: existingPr };
+ }
+ // istanbul ignore next
+ return { prBlockedBy: 'Error' };
}
diff --git a/lib/workers/repository/__fixtures__/master-issue_with_2_PR_edited.txt b/lib/workers/repository/__fixtures__/master-issue_with_2_PR_edited.txt
index a3c51068a9..ba9398e96e 100644
--- a/lib/workers/repository/__fixtures__/master-issue_with_2_PR_edited.txt
+++ b/lib/workers/repository/__fixtures__/master-issue_with_2_PR_edited.txt
@@ -5,5 +5,5 @@ This issue contains a list of Renovate updates and their statuses.
These updates have been manually edited so Renovate will no longer make changes. To discard all commits and start over, check the box below.
- [ ] <!-- rebase-branch=branchName1 -->[pr1](../pull/1)
- - [ ] <!-- rebase-branch=branchName2 -->pr2 (`dep2`, `dep3`)
+ - [ ] <!-- rebase-branch=branchName2 -->[pr2](../pull/2) (`dep2`, `dep3`)
diff --git a/lib/workers/repository/__fixtures__/master-issue_with_3_PR_in_progress.txt b/lib/workers/repository/__fixtures__/master-issue_with_3_PR_in_progress.txt
index 37d2fdc5b0..1f009661c7 100644
--- a/lib/workers/repository/__fixtures__/master-issue_with_3_PR_in_progress.txt
+++ b/lib/workers/repository/__fixtures__/master-issue_with_3_PR_in_progress.txt
@@ -5,7 +5,7 @@ This issue contains a list of Renovate updates and their statuses.
These updates have all been created already. Click a checkbox below to force a retry/rebase of any.
- [ ] <!-- rebase-branch=branchName1 -->[pr1](../pull/1)
- - [ ] <!-- rebase-branch=branchName2 -->pr2 (`dep2`, `dep3`)
+ - [ ] <!-- rebase-branch=branchName2 -->[pr2](../pull/2) (`dep2`, `dep3`)
- [ ] <!-- rebase-branch=branchName3 -->[pr3](../pull/3)
- [ ] <!-- rebase-all-open-prs -->**Check this option to rebase all the above open PRs at once**
diff --git a/lib/workers/repository/dependency-dashboard.spec.ts b/lib/workers/repository/dependency-dashboard.spec.ts
index 3eba9b79b3..a92c3e32c2 100644
--- a/lib/workers/repository/dependency-dashboard.spec.ts
+++ b/lib/workers/repository/dependency-dashboard.spec.ts
@@ -10,8 +10,7 @@ import {
} from '../../../test/util';
import { setAdminConfig } from '../../config/admin';
import { PLATFORM_TYPE_GITHUB } from '../../constants/platforms';
-import { Platform, Pr } from '../../platform';
-import { PrState } from '../../types';
+import type { Platform } from '../../platform';
import { BranchConfig, BranchResult, BranchUpgradeConfig } from '../types';
import * as dependencyDashboard from './dependency-dashboard';
@@ -31,9 +30,7 @@ async function dryRun(
// eslint-disable-next-line @typescript-eslint/no-shadow
platform: jest.Mocked<Platform>,
ensureIssueClosingCalls = 0,
- ensureIssueCalls = 0,
- getBranchPrCalls = 0,
- findPrCalls = 0
+ ensureIssueCalls = 0
) {
jest.clearAllMocks();
setAdminConfig({ dryRun: true });
@@ -42,8 +39,6 @@ async function dryRun(
ensureIssueClosingCalls
);
expect(platform.ensureIssue).toHaveBeenCalledTimes(ensureIssueCalls);
- expect(platform.getBranchPr).toHaveBeenCalledTimes(getBranchPrCalls);
- expect(platform.findPr).toHaveBeenCalledTimes(findPrCalls);
}
describe(getName(), () => {
@@ -56,8 +51,6 @@ describe(getName(), () => {
await dependencyDashboard.ensureDependencyDashboard(config, branches);
expect(platform.ensureIssueClosing).toHaveBeenCalledTimes(0);
expect(platform.ensureIssue).toHaveBeenCalledTimes(0);
- expect(platform.getBranchPr).toHaveBeenCalledTimes(0);
- expect(platform.findPr).toHaveBeenCalledTimes(0);
// same with dry run
await dryRun(branches, platform);
@@ -78,8 +71,6 @@ describe(getName(), () => {
await dependencyDashboard.ensureDependencyDashboard(config, branches);
expect(platform.ensureIssueClosing).toHaveBeenCalledTimes(0);
expect(platform.ensureIssue).toHaveBeenCalledTimes(0);
- expect(platform.getBranchPr).toHaveBeenCalledTimes(0);
- expect(platform.findPr).toHaveBeenCalledTimes(0);
// same with dry run
await dryRun(branches, platform);
@@ -95,8 +86,6 @@ describe(getName(), () => {
config.dependencyDashboardTitle
);
expect(platform.ensureIssue).toHaveBeenCalledTimes(0);
- expect(platform.getBranchPr).toHaveBeenCalledTimes(0);
- expect(platform.findPr).toHaveBeenCalledTimes(0);
// same with dry run
await dryRun(branches, platform);
@@ -124,8 +113,6 @@ describe(getName(), () => {
config.dependencyDashboardTitle
);
expect(platform.ensureIssue).toHaveBeenCalledTimes(0);
- expect(platform.getBranchPr).toHaveBeenCalledTimes(0);
- expect(platform.findPr).toHaveBeenCalledTimes(0);
// same with dry run
await dryRun(branches, platform);
@@ -142,8 +129,6 @@ describe(getName(), () => {
config.dependencyDashboardTitle
);
expect(platform.ensureIssue.mock.calls[0][0].body).toMatchSnapshot();
- expect(platform.getBranchPr).toHaveBeenCalledTimes(0);
- expect(platform.findPr).toHaveBeenCalledTimes(0);
// same with dry run
await dryRun(branches, platform);
@@ -165,8 +150,6 @@ describe(getName(), () => {
config.dependencyDashboardTitle
);
expect(platform.ensureIssue.mock.calls[0][0].body).toMatchSnapshot();
- expect(platform.getBranchPr).toHaveBeenCalledTimes(0);
- expect(platform.findPr).toHaveBeenCalledTimes(0);
// same with dry run
await dryRun(branches, platform);
@@ -241,8 +224,6 @@ describe(getName(), () => {
expect(platform.ensureIssue.mock.calls[0][0].body).toBe(
loadFixture('master-issue_with_8_PR.txt')
);
- expect(platform.getBranchPr).toHaveBeenCalledTimes(0);
- expect(platform.findPr).toHaveBeenCalledTimes(0);
// same with dry run
await dryRun(branches, platform);
@@ -252,6 +233,7 @@ describe(getName(), () => {
const branches: BranchConfig[] = [
{
...mock<BranchConfig>(),
+ prNo: 1,
prTitle: 'pr1',
upgrades: [{ ...mock<PrUpgrade>(), depName: 'dep1' }],
result: BranchResult.PrEdited,
@@ -259,6 +241,7 @@ describe(getName(), () => {
},
{
...mock<BranchConfig>(),
+ prNo: 2,
prTitle: 'pr2',
upgrades: [
{ ...mock<PrUpgrade>(), depName: 'dep2' },
@@ -269,9 +252,6 @@ describe(getName(), () => {
},
];
config.dependencyDashboard = true;
- platform.getBranchPr
- .mockResolvedValueOnce({ ...mock<Pr>(), number: 1 })
- .mockResolvedValueOnce(undefined);
await dependencyDashboard.ensureDependencyDashboard(config, branches);
expect(platform.ensureIssueClosing).toHaveBeenCalledTimes(0);
expect(platform.ensureIssue).toHaveBeenCalledTimes(1);
@@ -281,13 +261,9 @@ describe(getName(), () => {
expect(platform.ensureIssue.mock.calls[0][0].body).toBe(
loadFixture('master-issue_with_2_PR_edited.txt')
);
- expect(platform.getBranchPr).toHaveBeenCalledTimes(2);
- expect(platform.getBranchPr.mock.calls[0][0]).toBe('branchName1');
- expect(platform.getBranchPr.mock.calls[1][0]).toBe('branchName2');
- expect(platform.findPr).toHaveBeenCalledTimes(0);
// same with dry run
- await dryRun(branches, platform, 0, 0, 2, 0);
+ await dryRun(branches, platform, 0, 0);
});
it('checks an issue with 3 PR in progress and rebase all option', async () => {
@@ -297,11 +273,13 @@ describe(getName(), () => {
prTitle: 'pr1',
upgrades: [{ ...mock<PrUpgrade>(), depName: 'dep1' }],
result: BranchResult.Rebase,
+ prNo: 1,
branchName: 'branchName1',
},
{
...mock<BranchConfig>(),
prTitle: 'pr2',
+ prNo: 2,
upgrades: [
{ ...mock<PrUpgrade>(), depName: 'dep2' },
{ ...mock<PrUpgrade>(), depName: 'dep3' },
@@ -312,16 +290,13 @@ describe(getName(), () => {
{
...mock<BranchConfig>(),
prTitle: 'pr3',
+ prNo: 3,
upgrades: [{ ...mock<PrUpgrade>(), depName: 'dep3' }],
result: BranchResult.Rebase,
branchName: 'branchName3',
},
];
config.dependencyDashboard = true;
- platform.getBranchPr
- .mockResolvedValueOnce({ ...mock<Pr>(), number: 1 })
- .mockResolvedValueOnce(undefined)
- .mockResolvedValueOnce({ ...mock<Pr>(), number: 3 });
await dependencyDashboard.ensureDependencyDashboard(config, branches);
expect(platform.ensureIssueClosing).toHaveBeenCalledTimes(0);
expect(platform.ensureIssue).toHaveBeenCalledTimes(1);
@@ -331,14 +306,9 @@ describe(getName(), () => {
expect(platform.ensureIssue.mock.calls[0][0].body).toBe(
loadFixture('master-issue_with_3_PR_in_progress.txt')
);
- expect(platform.getBranchPr).toHaveBeenCalledTimes(3);
- expect(platform.getBranchPr.mock.calls[0][0]).toBe('branchName1');
- expect(platform.getBranchPr.mock.calls[1][0]).toBe('branchName2');
- expect(platform.getBranchPr.mock.calls[2][0]).toBe('branchName3');
- expect(platform.findPr).toHaveBeenCalledTimes(0);
// same with dry run
- await dryRun(branches, platform, 0, 0, 3, 0);
+ await dryRun(branches, platform, 0, 0);
});
it('checks an issue with 2 PR closed / ignored', async () => {
@@ -362,10 +332,6 @@ describe(getName(), () => {
},
];
config.dependencyDashboard = true;
- platform.getBranchPr
- .mockResolvedValueOnce({ ...mock<Pr>(), number: 1 })
- .mockResolvedValueOnce(undefined)
- .mockResolvedValueOnce({ ...mock<Pr>(), number: 3 });
await dependencyDashboard.ensureDependencyDashboard(config, branches);
expect(platform.ensureIssueClosing).toHaveBeenCalledTimes(0);
expect(platform.ensureIssue).toHaveBeenCalledTimes(1);
@@ -375,17 +341,9 @@ describe(getName(), () => {
expect(platform.ensureIssue.mock.calls[0][0].body).toBe(
loadFixture('master-issue_with_2_PR_closed_ignored.txt')
);
- expect(platform.getBranchPr).toHaveBeenCalledTimes(0);
- expect(platform.findPr).toHaveBeenCalledTimes(2);
- expect(platform.findPr.mock.calls[0][0].branchName).toBe('branchName1');
- expect(platform.findPr.mock.calls[0][0].prTitle).toBe('pr1');
- expect(platform.findPr.mock.calls[0][0].state).toBe(PrState.NotOpen);
- expect(platform.findPr.mock.calls[1][0].branchName).toBe('branchName2');
- expect(platform.findPr.mock.calls[1][0].prTitle).toBe('pr2');
- expect(platform.findPr.mock.calls[1][0].state).toBe(PrState.NotOpen);
// same with dry run
- await dryRun(branches, platform, 0, 0, 0, 2);
+ await dryRun(branches, platform, 0, 0);
});
it('checks an issue with 3 PR in approval', async () => {
@@ -433,7 +391,6 @@ describe(getName(), () => {
expect(platform.ensureIssue.mock.calls[0][0].body).toBe(
loadFixture('master-issue_with_3_PR_in_approval.txt')
);
- expect(platform.findPr).toHaveBeenCalledTimes(0);
// same with dry run
await dryRun(branches, platform);
diff --git a/lib/workers/repository/dependency-dashboard.ts b/lib/workers/repository/dependency-dashboard.ts
index 74abd1e887..f4ddb1435f 100644
--- a/lib/workers/repository/dependency-dashboard.ts
+++ b/lib/workers/repository/dependency-dashboard.ts
@@ -3,15 +3,14 @@ import { nameFromLevel } from 'bunyan';
import { getAdminConfig } from '../../config/admin';
import type { RenovateConfig } from '../../config/types';
import { getProblems, logger } from '../../logger';
-import { Pr, platform } from '../../platform';
-import { PrState } from '../../types';
+import { platform } from '../../platform';
import { BranchConfig, BranchResult } from '../types';
-function getListItem(branch: BranchConfig, type: string, pr?: Pr): string {
+function getListItem(branch: BranchConfig, type: string): string {
let item = ' - [ ] ';
item += `<!-- ${type}-branch=${branch.branchName} -->`;
- if (pr) {
- item += `[${branch.prTitle}](../pull/${pr.number})`;
+ if (branch.prNo) {
+ item += `[${branch.prTitle}](../pull/${branch.prNo})`;
} else {
item += branch.prTitle;
}
@@ -166,8 +165,7 @@ export async function ensureDependencyDashboard(
issueBody += '## Edited/Blocked\n\n';
issueBody += `These updates have been manually edited so Renovate will no longer make changes. To discard all commits and start over, check the box below.\n\n`;
for (const branch of prEdited) {
- const pr = await platform.getBranchPr(branch.branchName);
- issueBody += getListItem(branch, 'rebase', pr);
+ issueBody += getListItem(branch, 'rebase');
}
issueBody += '\n';
}
@@ -195,16 +193,38 @@ export async function ensureDependencyDashboard(
BranchResult.Automerged,
BranchResult.PrEdited,
];
- const inProgress = branches.filter(
+ let inProgress = branches.filter(
(branch) => !otherRes.includes(branch.result)
);
+ const otherBranches = inProgress.filter(
+ (branch) => branch.prBlockedBy || !branch.prNo
+ );
+ // istanbul ignore if
+ if (otherBranches.length) {
+ issueBody += '## Other Branches\n\n';
+ issueBody += `These updates are pending. To force PRs open, check the box below.\n\n`;
+ for (const branch of otherBranches) {
+ logger.info(
+ {
+ prBlockedBy: branch.prBlockedBy,
+ prNo: branch.prNo,
+ result: branch.result,
+ },
+ 'Blocked PR'
+ );
+ issueBody += getListItem(branch, 'other');
+ }
+ issueBody += '\n';
+ }
+ inProgress = inProgress.filter(
+ (branch) => branch.prNo && !branch.prBlockedBy
+ );
if (inProgress.length) {
issueBody += '## Open\n\n';
issueBody +=
'These updates have all been created already. Click a checkbox below to force a retry/rebase of any.\n\n';
for (const branch of inProgress) {
- const pr = await platform.getBranchPr(branch.branchName);
- issueBody += getListItem(branch, 'rebase', pr);
+ issueBody += getListItem(branch, 'rebase');
}
if (inProgress.length > 2) {
issueBody += ' - [ ] ';
@@ -223,12 +243,7 @@ export async function ensureDependencyDashboard(
issueBody +=
'These are blocked by an existing closed PR and will not be recreated unless you click a checkbox below.\n\n';
for (const branch of alreadyExisted) {
- const pr = await platform.findPr({
- branchName: branch.branchName,
- prTitle: branch.prTitle,
- state: PrState.NotOpen,
- });
- issueBody += getListItem(branch, 'recreate', pr);
+ issueBody += getListItem(branch, 'recreate');
}
issueBody += '\n';
}
diff --git a/lib/workers/repository/process/write.ts b/lib/workers/repository/process/write.ts
index 1fa41c3c69..2dfac7b13e 100644
--- a/lib/workers/repository/process/write.ts
+++ b/lib/workers/repository/process/write.ts
@@ -36,6 +36,8 @@ export async function writeUpdates(
addMeta({ branch: branch.branchName });
const branchExisted = branchExists(branch.branchName);
const res = await processBranch(branch);
+ branch.prBlockedBy = res?.prBlockedBy;
+ branch.prNo = res?.prNo;
branch.result = res?.result;
if (
branch.result === BranchResult.Automerged &&
diff --git a/lib/workers/types.ts b/lib/workers/types.ts
index 5e9fca00d4..d9a19e7fdf 100644
--- a/lib/workers/types.ts
+++ b/lib/workers/types.ts
@@ -66,18 +66,12 @@ export interface BranchUpgradeConfig
sourceUrl?: string;
}
-export enum PrResult {
- AwaitingApproval = 'AwaitingApproval',
- AwaitingGreenBranch = 'AwaitingGreenBranch',
- AwaitingNotPending = 'AwaitingNotPending',
- BlockedByBranchAutomerge = 'BlockedByBranchAutomerge',
- Created = 'Created',
- Error = 'Error',
- ErrorAlreadyExists = 'ErrorAlreadyExists',
- NotUpdated = 'NotUpdated',
- Updated = 'Updated',
- LimitReached = 'LimitReached',
-}
+export type PrBlockedBy =
+ | 'BranchAutomerge'
+ | 'NeedsApproval'
+ | 'AwaitingTests'
+ | 'RateLimited'
+ | 'Error';
export enum BranchResult {
AlreadyExisted = 'already-existed',
@@ -113,4 +107,6 @@ export interface BranchConfig
result?: BranchResult;
upgrades: BranchUpgradeConfig[];
packageFiles?: Record<string, PackageFile[]>;
+ prBlockedBy?: PrBlockedBy;
+ prNo?: number;
}
--
GitLab