diff --git a/lib/workers/branch/__snapshots__/index.spec.ts.snap b/lib/workers/branch/__snapshots__/index.spec.ts.snap
index 596958e7a2ec4aff9422410fec6da2465a5a3308..945f5c3f247d143c0626683b686c8714bd662eb6 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 7a71e27a1757a159130fbbb4512c5c9d4b0c69de..f284452a1add44a7e83534ee600cec53949c42cc 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 36b8199c2f71571e73757fc27a33aff2398977b4..bb71ec0f49d51776440808068e480530da30f4a0 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 edd068b3dcf57ea7c873ce2836e27515f962eb18..4d0e5f62ea85a005015dad2d77dab0f29593db79 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 d5a92eb901e08ab88a3ee71c112619c1374cb726..b4bd2b75b9a236892336fb0fccc43d04696d7ee3 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 a3c51068a96acd0492c1803cd1f3c2803fca1c4c..ba9398e96e0a7a835708c4aaf6d8b458ef56f8dc 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 37d2fdc5b0d067fbfcf19fb4604bb4cf1ccd21e9..1f009661c7fe443e00ec6360b45483f80df84a46 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 3eba9b79b32fe2ea5d5c69f4fd32115d9c09df4d..a92c3e32c28db78c4c6d243c6475757f3f3e4341 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 74abd1e8870bb659b2c601f46511ab2764318e93..f4ddb1435f44d1d37f23e4c7315751c010d540b2 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 1fa41c3c698bbe28804c8a3fa07225efd558ba15..2dfac7b13eea064110061426b8de7f05caad8569 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 5e9fca00d498b1fae5a3df839af22f20e0f1db21..d9a19e7fdfeb294f7e2ed5ce15cd5cddc9469415 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;
}