From b08439a6edabc116fa706ec344b854ac9f41b8d7 Mon Sep 17 00:00:00 2001
From: Jamie Magee <jamie.magee@gmail.com>
Date: Tue, 1 Mar 2022 21:09:06 -0800
Subject: [PATCH] fix: union types for ensurePR (#14293)
---
lib/workers/branch/index.spec.ts | 33 +++--
lib/workers/branch/index.ts | 11 +-
lib/workers/pr/index.spec.ts | 202 +++++++++++++++++++------------
lib/workers/pr/index.ts | 29 +++--
4 files changed, 168 insertions(+), 107 deletions(-)
diff --git a/lib/workers/branch/index.spec.ts b/lib/workers/branch/index.spec.ts
index 2286cb6a58..17b33e734f 100644
--- a/lib/workers/branch/index.spec.ts
+++ b/lib/workers/branch/index.spec.ts
@@ -21,7 +21,7 @@ import * as _mergeConfidence from '../../util/merge-confidence';
import * as _sanitize from '../../util/sanitize';
import * as _limits from '../global/limits';
import * as _prWorker from '../pr';
-import type { EnsurePrResult } from '../pr';
+import type { ResultWithPr } from '../pr';
import * as _prAutomerge from '../pr/automerge';
import type { Pr } from '../repository/onboarding/branch/check';
import type { BranchConfig, BranchUpgradeConfig } from '../types';
@@ -99,6 +99,7 @@ describe('workers/branch/index', () => {
platform.massageMarkdown.mockImplementation((prBody) => prBody);
prWorker.ensurePr.mockResolvedValue({
+ type: 'with-pr',
pr: partial<Pr>({
title: '',
sourceBranch: '',
@@ -381,6 +382,7 @@ describe('workers/branch/index', () => {
});
git.branchExists.mockReturnValue(true);
prWorker.ensurePr.mockResolvedValueOnce({
+ type: 'without-pr',
prBlockedBy: 'RateLimited',
});
limits.isLimitReached.mockReturnValue(false);
@@ -503,6 +505,7 @@ describe('workers/branch/index', () => {
commit.commitFilesToBranch.mockResolvedValueOnce(null);
automerge.tryBranchAutomerge.mockResolvedValueOnce('failed');
prWorker.ensurePr.mockResolvedValueOnce({
+ type: 'without-pr',
prBlockedBy: 'NeedsApproval',
});
expect(await branchWorker.processBranch(config)).toEqual({
@@ -524,6 +527,7 @@ describe('workers/branch/index', () => {
commit.commitFilesToBranch.mockResolvedValueOnce(null);
automerge.tryBranchAutomerge.mockResolvedValueOnce('failed');
prWorker.ensurePr.mockResolvedValueOnce({
+ type: 'without-pr',
prBlockedBy: 'AwaitingTests',
});
expect(await branchWorker.processBranch(config)).toEqual({
@@ -545,6 +549,7 @@ describe('workers/branch/index', () => {
commit.commitFilesToBranch.mockResolvedValueOnce(null);
automerge.tryBranchAutomerge.mockResolvedValueOnce('no automerge');
prWorker.ensurePr.mockResolvedValueOnce({
+ type: 'without-pr',
prBlockedBy: 'BranchAutomerge',
});
expect(await branchWorker.processBranch(config)).toEqual({
@@ -566,6 +571,7 @@ describe('workers/branch/index', () => {
commit.commitFilesToBranch.mockResolvedValueOnce(null);
automerge.tryBranchAutomerge.mockResolvedValueOnce('failed');
prWorker.ensurePr.mockResolvedValueOnce({
+ type: 'without-pr',
prBlockedBy: 'Error',
});
expect(await branchWorker.processBranch(config)).toEqual({
@@ -587,6 +593,7 @@ describe('workers/branch/index', () => {
commit.commitFilesToBranch.mockResolvedValueOnce(null);
automerge.tryBranchAutomerge.mockResolvedValueOnce('failed');
prWorker.ensurePr.mockResolvedValueOnce({
+ type: 'without-pr',
prBlockedBy: 'whoops' as any,
});
expect(await branchWorker.processBranch(config)).toEqual({
@@ -630,8 +637,9 @@ describe('workers/branch/index', () => {
git.branchExists.mockReturnValue(true);
automerge.tryBranchAutomerge.mockResolvedValueOnce('failed');
prWorker.ensurePr.mockResolvedValueOnce({
+ type: 'with-pr',
pr: {},
- } as EnsurePrResult);
+ } as ResultWithPr);
prAutomerge.checkAutoMerge.mockResolvedValueOnce({ automerged: true });
commit.commitFilesToBranch.mockResolvedValueOnce(null);
await branchWorker.processBranch({ ...config, automerge: true });
@@ -650,8 +658,9 @@ describe('workers/branch/index', () => {
git.branchExists.mockReturnValue(true);
automerge.tryBranchAutomerge.mockResolvedValueOnce('stale');
prWorker.ensurePr.mockResolvedValueOnce({
+ type: 'with-pr',
pr: {},
- } as EnsurePrResult);
+ } as ResultWithPr);
prAutomerge.checkAutoMerge.mockResolvedValueOnce({ automerged: false });
commit.commitFilesToBranch.mockResolvedValueOnce(null);
await branchWorker.processBranch({
@@ -674,8 +683,9 @@ describe('workers/branch/index', () => {
git.branchExists.mockReturnValue(true);
automerge.tryBranchAutomerge.mockResolvedValueOnce('failed');
prWorker.ensurePr.mockResolvedValueOnce({
+ type: 'with-pr',
pr: {},
- } as EnsurePrResult);
+ } as ResultWithPr);
prAutomerge.checkAutoMerge.mockResolvedValueOnce({ automerged: true });
commit.commitFilesToBranch.mockResolvedValueOnce(null);
await branchWorker.processBranch(config);
@@ -694,8 +704,9 @@ describe('workers/branch/index', () => {
git.branchExists.mockReturnValue(true);
automerge.tryBranchAutomerge.mockResolvedValueOnce('failed');
prWorker.ensurePr.mockResolvedValueOnce({
+ type: 'with-pr',
pr: {},
- } as EnsurePrResult);
+ } as ResultWithPr);
prAutomerge.checkAutoMerge.mockResolvedValueOnce({ automerged: true });
config.releaseTimestamp = '2018-04-26T05:15:51.877Z';
commit.commitFilesToBranch.mockResolvedValueOnce(null);
@@ -715,8 +726,9 @@ describe('workers/branch/index', () => {
git.branchExists.mockReturnValue(true);
automerge.tryBranchAutomerge.mockResolvedValueOnce('failed');
prWorker.ensurePr.mockResolvedValueOnce({
+ type: 'with-pr',
pr: {},
- } as EnsurePrResult);
+ } as ResultWithPr);
prAutomerge.checkAutoMerge.mockResolvedValueOnce({ automerged: true });
config.releaseTimestamp = new Date().toISOString();
commit.commitFilesToBranch.mockResolvedValueOnce(null);
@@ -736,8 +748,9 @@ describe('workers/branch/index', () => {
git.branchExists.mockReturnValue(false);
automerge.tryBranchAutomerge.mockResolvedValueOnce('failed');
prWorker.ensurePr.mockResolvedValueOnce({
+ type: 'with-pr',
pr: {},
- } as EnsurePrResult);
+ } as ResultWithPr);
prAutomerge.checkAutoMerge.mockResolvedValueOnce({ automerged: true });
config.releaseTimestamp = new Date().toISOString();
await expect(branchWorker.processBranch(config)).rejects.toThrow(
@@ -756,8 +769,9 @@ describe('workers/branch/index', () => {
git.branchExists.mockReturnValue(true);
automerge.tryBranchAutomerge.mockResolvedValueOnce('failed');
prWorker.ensurePr.mockResolvedValueOnce({
+ type: 'with-pr',
pr: {},
- } as EnsurePrResult);
+ } as ResultWithPr);
prAutomerge.checkAutoMerge.mockResolvedValueOnce({ automerged: true });
commit.commitFilesToBranch.mockResolvedValueOnce(null);
await branchWorker.processBranch(config);
@@ -890,8 +904,9 @@ describe('workers/branch/index', () => {
git.isBranchModified.mockResolvedValueOnce(true);
schedule.isScheduledNow.mockReturnValueOnce(false);
prWorker.ensurePr.mockResolvedValueOnce({
+ type: 'with-pr',
pr: {},
- } as EnsurePrResult);
+ } as ResultWithPr);
commit.commitFilesToBranch.mockResolvedValueOnce(null);
GlobalConfig.set({ ...adminConfig, dryRun: true });
expect(
diff --git a/lib/workers/branch/index.ts b/lib/workers/branch/index.ts
index 5c3377dff2..bebe650458 100644
--- a/lib/workers/branch/index.ts
+++ b/lib/workers/branch/index.ts
@@ -619,9 +619,10 @@ export async function processBranch(
logger.debug(
`There are ${config.errors.length} errors and ${config.warnings.length} warnings`
);
- const { prBlockedBy, pr } = await ensurePr(config);
- branchPr = pr;
- if (prBlockedBy) {
+ const ensurePrResult = await ensurePr(config);
+ if (ensurePrResult.type === 'without-pr') {
+ const { prBlockedBy } = ensurePrResult;
+ branchPr = null;
if (prBlockedBy === 'RateLimited' && !config.isVulnerabilityAlert) {
logger.debug('Reached PR limit - skipping PR creation');
return {
@@ -654,7 +655,9 @@ export async function processBranch(
logger.warn({ prBlockedBy }, 'Unknown PrBlockedBy result');
return { branchExists, prBlockedBy, result: BranchResult.Error };
}
- if (pr) {
+ if (ensurePrResult.type === 'with-pr') {
+ const { pr } = ensurePrResult;
+ branchPr = pr;
if (config.artifactErrors?.length) {
logger.warn(
{ artifactErrors: config.artifactErrors },
diff --git a/lib/workers/pr/index.spec.ts b/lib/workers/pr/index.spec.ts
index 25bbf2595d..d48c662774 100644
--- a/lib/workers/pr/index.spec.ts
+++ b/lib/workers/pr/index.spec.ts
@@ -9,6 +9,7 @@ import * as _changelogHelper from './changelog';
import type { ChangeLogResult } from './changelog';
import * as codeOwners from './code-owners';
import * as prWorker from '.';
+import type { EnsurePrResult, ResultWithPr, ResultWithoutPr } from '.';
const codeOwnersMock = mocked(codeOwners);
const changelogHelper = mocked(_changelogHelper);
@@ -94,6 +95,20 @@ function setupGitlabChangelogMock() {
gitlabChangelogHelper.getChangeLogJSON.mockResolvedValue(resultValue);
}
+function isResultWithPr(value: EnsurePrResult): asserts value is ResultWithPr {
+ if (value.type !== 'with-pr') {
+ throw new TypeError();
+ }
+}
+
+function isResultWithoutPr(
+ value: EnsurePrResult
+): asserts value is ResultWithoutPr {
+ if (value.type !== 'without-pr') {
+ throw new TypeError();
+ }
+}
+
describe('workers/pr/index', () => {
describe('checkAutoMerge(pr, config)', () => {
let config: BranchConfig;
@@ -205,22 +220,23 @@ describe('workers/pr/index', () => {
});
config.newValue = '1.2.0';
platform.getBranchPr.mockResolvedValueOnce(existingPr);
- const { pr } = await prWorker.ensurePr(config);
- expect(pr).toBeDefined();
+ const result = await prWorker.ensurePr(config);
+ isResultWithPr(result);
+ expect(result.pr).toBeDefined();
});
it('should return null if waiting for success', async () => {
platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.red);
config.prCreation = 'status-success';
- const { prBlockedBy, pr } = await prWorker.ensurePr(config);
- expect(prBlockedBy).toBe('AwaitingTests');
- expect(pr).toBeUndefined();
+ const result = await prWorker.ensurePr(config);
+ isResultWithoutPr(result);
+ expect(result.prBlockedBy).toBe('AwaitingTests');
});
it('should return needs-approval if prCreation set to approval', async () => {
platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.green);
config.prCreation = 'approval';
- const { prBlockedBy, pr } = await prWorker.ensurePr(config);
- expect(prBlockedBy).toBe('NeedsApproval');
- expect(pr).toBeUndefined();
+ const result = await prWorker.ensurePr(config);
+ isResultWithoutPr(result);
+ expect(result.prBlockedBy).toBe('NeedsApproval');
});
it('should create PR if success for gitlab deps', async () => {
setupGitlabChangelogMock();
@@ -233,8 +249,9 @@ describe('workers/pr/index', () => {
config.prCreation = 'status-success';
config.automerge = true;
config.schedule = ['before 5am'];
- const { pr } = await prWorker.ensurePr(config);
- expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
+ const result = await prWorker.ensurePr(config);
+ isResultWithPr(result);
+ expect(result.pr).toMatchObject({ displayNumber: 'New Pull Request' });
expect(platform.createPr.mock.calls[0]).toMatchSnapshot([
{
prTitle: 'Update dependency dummy to v1.1.0',
@@ -253,8 +270,9 @@ describe('workers/pr/index', () => {
config.prCreation = 'status-success';
config.automerge = true;
config.schedule = ['before 5am'];
- const { pr } = await prWorker.ensurePr(config);
- expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
+ const result = await prWorker.ensurePr(config);
+ isResultWithPr(result);
+ expect(result.pr).toMatchObject({ displayNumber: 'New Pull Request' });
expect(platform.createPr.mock.calls[0]).toMatchSnapshot([
{
prTitle: 'Update dependency dummy to v1.1.0',
@@ -270,8 +288,9 @@ describe('workers/pr/index', () => {
config.automerge = true;
config.schedule = ['before 5am'];
limits.isLimitReached.mockReturnValueOnce(true);
- const { prBlockedBy } = await prWorker.ensurePr(config);
- expect(prBlockedBy).toBe('RateLimited');
+ const result = await prWorker.ensurePr(config);
+ isResultWithoutPr(result);
+ expect(result.prBlockedBy).toBe('RateLimited');
expect(platform.createPr.mock.calls).toBeEmpty();
});
it('should create PR if limit is reached but dashboard checked', async () => {
@@ -420,8 +439,9 @@ describe('workers/pr/index', () => {
};
}
}
- const { pr } = await prWorker.ensurePr(config);
- expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
+ const result = await prWorker.ensurePr(config);
+ isResultWithPr(result);
+ expect(result.pr).toMatchObject({ displayNumber: 'New Pull Request' });
expect(platform.createPr.mock.calls[0]).toMatchSnapshot([
{
prTitle: 'Update dependency dummy to v1.1.0',
@@ -438,8 +458,9 @@ describe('workers/pr/index', () => {
config.timezone = 'some timezone';
config.rebaseWhen = 'behind-base-branch';
config.logJSON = await changelogHelper.getChangeLogJSON(config);
- const { pr } = await prWorker.ensurePr(config);
- expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
+ const result = await prWorker.ensurePr(config);
+ isResultWithPr(result);
+ expect(result.pr).toMatchObject({ displayNumber: 'New Pull Request' });
expect(platform.createPr.mock.calls[0]).toMatchSnapshot([
{
prTitle: 'Update dependency dummy to v1.1.0',
@@ -456,9 +477,9 @@ describe('workers/pr/index', () => {
throw new Error('Validation Failed (422)');
});
config.prCreation = 'status-success';
- const { prBlockedBy, pr } = await prWorker.ensurePr(config);
- expect(prBlockedBy).toBe('Error');
- expect(pr).toBeUndefined();
+ const result = await prWorker.ensurePr(config);
+ isResultWithoutPr(result);
+ expect(result.prBlockedBy).toBe('Error');
});
it('should return null if waiting for not pending', async () => {
platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.yellow);
@@ -466,9 +487,9 @@ describe('workers/pr/index', () => {
Promise.resolve(new Date())
);
config.prCreation = 'not-pending';
- const { prBlockedBy, pr } = await prWorker.ensurePr(config);
- expect(prBlockedBy).toBe('AwaitingTests');
- expect(pr).toBeUndefined();
+ const result = await prWorker.ensurePr(config);
+ isResultWithoutPr(result);
+ expect(result.prBlockedBy).toBe('AwaitingTests');
});
it('should not create PR if waiting for not pending with stabilityStatus yellow', async () => {
platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.yellow);
@@ -477,9 +498,9 @@ describe('workers/pr/index', () => {
);
config.prCreation = 'not-pending';
config.stabilityStatus = BranchStatus.yellow;
- const { prBlockedBy, pr } = await prWorker.ensurePr(config);
- expect(prBlockedBy).toBe('AwaitingTests');
- expect(pr).toBeUndefined();
+ const result = await prWorker.ensurePr(config);
+ isResultWithoutPr(result);
+ expect(result.prBlockedBy).toBe('AwaitingTests');
});
it('should create PR if pending timeout hit', async () => {
platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.yellow);
@@ -488,24 +509,28 @@ describe('workers/pr/index', () => {
);
config.prCreation = 'not-pending';
config.stabilityStatus = BranchStatus.yellow;
- const { pr } = await prWorker.ensurePr(config);
- expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
+ const result = await prWorker.ensurePr(config);
+ isResultWithPr(result);
+ expect(result.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 { pr } = await prWorker.ensurePr(config);
- expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
+ const result = await prWorker.ensurePr(config);
+ isResultWithPr(result);
+ expect(result.pr).toMatchObject({ displayNumber: 'New Pull Request' });
});
it('should create new branch if none exists', async () => {
- const { pr } = await prWorker.ensurePr(config);
- expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
+ const result = await prWorker.ensurePr(config);
+ isResultWithPr(result);
+ expect(result.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 { pr } = await prWorker.ensurePr(config);
- expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
+ const result = await prWorker.ensurePr(config);
+ isResultWithPr(result);
+ expect(result.pr).toMatchObject({ displayNumber: 'New Pull Request' });
expect(platform.addAssignees).toHaveBeenCalledTimes(1);
expect(platform.addAssignees.mock.calls).toMatchSnapshot();
expect(platform.addReviewers).toHaveBeenCalledTimes(1);
@@ -551,8 +576,9 @@ describe('workers/pr/index', () => {
});
config.assignees = ['@foo', 'bar'];
config.reviewers = ['baz', '@boo'];
- const { pr } = await prWorker.ensurePr(config);
- expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
+ const result = await prWorker.ensurePr(config);
+ isResultWithPr(result);
+ expect(result.pr).toMatchObject({ displayNumber: 'New Pull Request' });
expect(platform.addAssignees).toHaveBeenCalledTimes(1);
expect(platform.addReviewers).toHaveBeenCalledTimes(1);
});
@@ -562,8 +588,9 @@ describe('workers/pr/index', () => {
});
config.assignees = ['@foo', 'bar'];
config.reviewers = ['baz', '@boo'];
- const { pr } = await prWorker.ensurePr(config);
- expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
+ const result = await prWorker.ensurePr(config);
+ isResultWithPr(result);
+ expect(result.pr).toMatchObject({ displayNumber: 'New Pull Request' });
expect(platform.addAssignees).toHaveBeenCalledTimes(1);
expect(platform.addReviewers).toHaveBeenCalledTimes(1);
});
@@ -571,8 +598,9 @@ describe('workers/pr/index', () => {
config.assignees = ['bar'];
config.reviewers = ['baz'];
config.automerge = true;
- const { pr } = await prWorker.ensurePr(config);
- expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
+ const result = await prWorker.ensurePr(config);
+ isResultWithPr(result);
+ expect(result.pr).toMatchObject({ displayNumber: 'New Pull Request' });
expect(platform.addAssignees).toHaveBeenCalledTimes(0);
expect(platform.addReviewers).toHaveBeenCalledTimes(0);
});
@@ -581,8 +609,9 @@ describe('workers/pr/index', () => {
config.reviewers = ['baz'];
config.automerge = true;
config.assignAutomerge = true;
- const { pr } = await prWorker.ensurePr(config);
- expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
+ const result = await prWorker.ensurePr(config);
+ isResultWithPr(result);
+ expect(result.pr).toMatchObject({ displayNumber: 'New Pull Request' });
expect(platform.addAssignees).toHaveBeenCalledTimes(1);
expect(platform.addReviewers).toHaveBeenCalledTimes(1);
});
@@ -591,8 +620,9 @@ describe('workers/pr/index', () => {
config.assigneesSampleSize = 2;
config.reviewers = ['baz', 'boo', 'bor'];
config.reviewersSampleSize = 2;
- const { pr } = await prWorker.ensurePr(config);
- expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
+ const result = await prWorker.ensurePr(config);
+ isResultWithPr(result);
+ expect(result.pr).toMatchObject({ displayNumber: 'New Pull Request' });
expect(platform.addAssignees).toHaveBeenCalledTimes(1);
const assignees = platform.addAssignees.mock.calls[0][1];
expect(assignees).toHaveLength(2);
@@ -608,24 +638,27 @@ describe('workers/pr/index', () => {
config.assigneesSampleSize = 0;
config.reviewers = ['baz', 'boo', 'bor'];
config.reviewersSampleSize = 0;
- const { pr } = await prWorker.ensurePr(config);
- expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
+ const result = await prWorker.ensurePr(config);
+ isResultWithPr(result);
+ expect(result.pr).toMatchObject({ displayNumber: 'New Pull Request' });
expect(platform.addAssignees).toHaveBeenCalledTimes(0);
expect(platform.addReviewers).toHaveBeenCalledTimes(0);
});
it('should add and deduplicate additionalReviewers on new PR', async () => {
config.reviewers = ['@foo', 'bar'];
config.additionalReviewers = ['bar', 'baz', '@boo'];
- const { pr } = await prWorker.ensurePr(config);
- expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
+ const result = await prWorker.ensurePr(config);
+ isResultWithPr(result);
+ expect(result.pr).toMatchObject({ displayNumber: 'New Pull Request' });
expect(platform.addReviewers).toHaveBeenCalledTimes(1);
expect(platform.addReviewers.mock.calls).toMatchSnapshot();
});
it('should add and deduplicate additionalReviewers to empty reviewers on new PR', async () => {
config.reviewers = [];
config.additionalReviewers = ['bar', 'baz', '@boo', '@foo', 'bar'];
- const { pr } = await prWorker.ensurePr(config);
- expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
+ const result = await prWorker.ensurePr(config);
+ isResultWithPr(result);
+ expect(result.pr).toMatchObject({ displayNumber: 'New Pull Request' });
expect(platform.addReviewers).toHaveBeenCalledTimes(1);
expect(platform.addReviewers.mock.calls).toMatchSnapshot();
});
@@ -635,10 +668,11 @@ describe('workers/pr/index', () => {
config.automerge = true;
config.schedule = ['before 5am'];
config.logJSON = await changelogHelper.getChangeLogJSON(config);
- const { pr } = await prWorker.ensurePr(config);
+ const result = await prWorker.ensurePr(config);
+ isResultWithPr(result);
expect(platform.updatePr.mock.calls).toMatchSnapshot();
expect(platform.updatePr).toHaveBeenCalledTimes(0);
- expect(pr).toMatchObject(existingPr);
+ expect(result.pr).toMatchObject(existingPr);
});
it('should return unmodified existing PR if only whitespace changes', async () => {
const modifiedPr = JSON.parse(
@@ -649,9 +683,10 @@ describe('workers/pr/index', () => {
config.automerge = true;
config.schedule = ['before 5am'];
config.logJSON = await changelogHelper.getChangeLogJSON(config);
- const { pr } = await prWorker.ensurePr(config);
+ const result = await prWorker.ensurePr(config);
+ isResultWithPr(result);
expect(platform.updatePr).toHaveBeenCalledTimes(0);
- expect(pr).toMatchObject(modifiedPr);
+ expect(result.pr).toMatchObject(modifiedPr);
});
it('should return modified existing PR', async () => {
config.newValue = '1.2.0';
@@ -659,8 +694,9 @@ describe('workers/pr/index', () => {
config.schedule = ['before 5am'];
config.logJSON = await changelogHelper.getChangeLogJSON(config);
platform.getBranchPr.mockResolvedValueOnce(existingPr);
- const { pr } = await prWorker.ensurePr(config);
- expect(pr).toMatchSnapshot({
+ const result = await prWorker.ensurePr(config);
+ isResultWithPr(result);
+ expect(result.pr).toMatchSnapshot({
displayNumber: 'Existing PR',
title: 'Update dependency dummy to v1.1.0',
});
@@ -671,8 +707,9 @@ describe('workers/pr/index', () => {
...existingPr,
title: 'wrong',
});
- const { pr } = await prWorker.ensurePr(config);
- expect(pr).toMatchSnapshot({
+ const result = await prWorker.ensurePr(config);
+ isResultWithPr(result);
+ expect(result.pr).toMatchSnapshot({
displayNumber: 'Existing PR',
title: 'wrong',
});
@@ -682,33 +719,36 @@ describe('workers/pr/index', () => {
config.automergeType = 'branch';
config.branchAutomergeFailureMessage = 'branch status error';
platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.red);
- const { pr } = await prWorker.ensurePr(config);
- expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
+ const result = await prWorker.ensurePr(config);
+ isResultWithPr(result);
+ expect(result.pr).toMatchObject({ displayNumber: 'New Pull Request' });
});
it('should create PR if branch automerging failed', async () => {
config.automerge = true;
config.automergeType = 'branch';
platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.green);
config.forcePr = true;
- const { pr } = await prWorker.ensurePr(config);
- expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
+ const result = await prWorker.ensurePr(config);
+ isResultWithPr(result);
+ expect(result.pr).toMatchObject({ displayNumber: 'New Pull Request' });
});
it('should return no PR if branch automerging not failed', async () => {
config.automerge = true;
config.automergeType = 'branch';
platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.yellow);
git.getBranchLastCommitTime.mockResolvedValueOnce(new Date());
- const { prBlockedBy, pr } = await prWorker.ensurePr(config);
- expect(prBlockedBy).toBe('BranchAutomerge');
- expect(pr).toBeUndefined();
+ const result = await prWorker.ensurePr(config);
+ isResultWithoutPr(result);
+ expect(result.prBlockedBy).toBe('BranchAutomerge');
});
it('should return PR if branch automerging taking too long', async () => {
config.automerge = true;
config.automergeType = 'branch';
platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.yellow);
git.getBranchLastCommitTime.mockResolvedValueOnce(new Date('2018-01-01'));
- const { pr } = await prWorker.ensurePr(config);
- expect(pr).toBeDefined();
+ const result = await prWorker.ensurePr(config);
+ isResultWithPr(result);
+ expect(result.pr).toBeDefined();
});
it('should return no PR if stabilityStatus yellow', async () => {
config.automerge = true;
@@ -716,14 +756,15 @@ describe('workers/pr/index', () => {
config.stabilityStatus = BranchStatus.yellow;
platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.yellow);
git.getBranchLastCommitTime.mockResolvedValueOnce(new Date('2018-01-01'));
- const { prBlockedBy, pr } = await prWorker.ensurePr(config);
- expect(prBlockedBy).toBe('BranchAutomerge');
- expect(pr).toBeUndefined();
+ const result = await prWorker.ensurePr(config);
+ isResultWithoutPr(result);
+ expect(result.prBlockedBy).toBe('BranchAutomerge');
});
it('handles duplicate upgrades', async () => {
config.upgrades.push(config.upgrades[0]);
- const { pr } = await prWorker.ensurePr(config);
- expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
+ const result = await prWorker.ensurePr(config);
+ isResultWithPr(result);
+ expect(result.pr).toMatchObject({ displayNumber: 'New Pull Request' });
});
it('should create privateRepo PR if success', async () => {
platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.green);
@@ -731,8 +772,9 @@ describe('workers/pr/index', () => {
config.privateRepo = false;
config.logJSON = await changelogHelper.getChangeLogJSON(config);
config.logJSON.project.repository = 'someproject';
- const { pr } = await prWorker.ensurePr(config);
- expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
+ const result = await prWorker.ensurePr(config);
+ isResultWithPr(result);
+ expect(result.pr).toMatchObject({ displayNumber: 'New Pull Request' });
expect(platform.createPr.mock.calls[0]).toMatchSnapshot();
existingPr.body = platform.createPr.mock.calls[0][0].prBody;
});
@@ -742,8 +784,9 @@ describe('workers/pr/index', () => {
config.prCreation = 'not-pending';
config.artifactErrors = [{}];
config.platform = PlatformId.Gitlab;
- const { pr } = await prWorker.ensurePr(config);
- expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
+ const result = await prWorker.ensurePr(config);
+ isResultWithPr(result);
+ expect(result.pr).toMatchObject({ displayNumber: 'New Pull Request' });
});
it('should trigger GitLab automerge when configured', async () => {
@@ -761,8 +804,9 @@ describe('workers/pr/index', () => {
it('should create a PR with set of labels and mergeable addLabels', async () => {
config.labels = ['deps', 'renovate'];
config.addLabels = ['deps', 'js'];
- const { pr } = await prWorker.ensurePr(config);
- expect(pr).toBeDefined();
+ const result = await prWorker.ensurePr(config);
+ isResultWithPr(result);
+ expect(result.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 383aa0ead3..504fe36f2e 100644
--- a/lib/workers/pr/index.ts
+++ b/lib/workers/pr/index.ts
@@ -145,12 +145,12 @@ export function getPlatformPrOptions(
}
export type ResultWithPr = {
+ type: 'with-pr';
pr: Pr;
- prBlockedBy?: never;
};
export type ResultWithoutPr = {
- pr?: never;
+ type: 'without-pr';
prBlockedBy: PrBlockedBy;
};
@@ -217,14 +217,14 @@ 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 { prBlockedBy: 'BranchAutomerge' };
+ return { type: 'without-pr', prBlockedBy: 'BranchAutomerge' };
}
}
if (config.prCreation === 'status-success') {
logger.debug('Checking branch combined status');
if ((await getBranchStatus()) !== BranchStatus.green) {
logger.debug(`Branch status isn't green - not creating PR`);
- return { prBlockedBy: 'AwaitingTests' };
+ return { type: 'without-pr', prBlockedBy: 'AwaitingTests' };
}
logger.debug('Branch status success');
} else if (
@@ -232,7 +232,7 @@ export async function ensurePr(
!existingPr &&
dependencyDashboardCheck !== 'approvePr'
) {
- return { prBlockedBy: 'NeedsApproval' };
+ return { type: 'without-pr', prBlockedBy: 'NeedsApproval' };
} else if (
config.prCreation === 'not-pending' &&
!existingPr &&
@@ -257,6 +257,7 @@ export async function ensurePr(
`Branch is ${elapsedHours} hours old - skipping PR creation`
);
return {
+ type: 'without-pr',
prBlockedBy: 'AwaitingTests',
};
}
@@ -385,7 +386,7 @@ export async function ensurePr(
noWhitespaceOrHeadings(stripEmojis(prBody))
) {
logger.debug(`${existingPr.displayNumber} does not need updating`);
- return { pr: existingPr };
+ return { type: 'with-pr', pr: existingPr };
}
// PR must need updating
if (existingPrTitle !== newPrTitle) {
@@ -417,9 +418,7 @@ export async function ensurePr(
});
logger.info({ pr: existingPr.number, prTitle }, `PR updated`);
}
- return {
- pr: existingPr,
- };
+ return { type: 'with-pr', pr: existingPr };
}
logger.debug({ branch: branchName, prTitle }, `Creating PR`);
// istanbul ignore if
@@ -439,7 +438,7 @@ export async function ensurePr(
!config.isVulnerabilityAlert
) {
logger.debug('Skipping PR - limit reached');
- return { prBlockedBy: 'RateLimited' };
+ return { type: 'without-pr', prBlockedBy: 'RateLimited' };
}
pr = await platform.createPr({
sourceBranch: branchName,
@@ -463,7 +462,7 @@ export async function ensurePr(
)
) {
logger.warn('A pull requests already exists');
- return { prBlockedBy: 'Error' };
+ return { type: 'without-pr', prBlockedBy: 'Error' };
}
if (err.statusCode === 502) {
logger.warn(
@@ -476,7 +475,7 @@ export async function ensurePr(
await deleteBranch(branchName);
}
}
- return { prBlockedBy: 'Error' };
+ return { type: 'without-pr', prBlockedBy: 'Error' };
}
if (
config.branchAutomergeFailureMessage &&
@@ -514,7 +513,7 @@ export async function ensurePr(
await addAssigneesReviewers(config, pr);
}
logger.debug(`Created ${pr.displayNumber}`);
- return { pr };
+ return { type: 'with-pr', pr };
} catch (err) {
// istanbul ignore if
if (
@@ -529,8 +528,8 @@ export async function ensurePr(
logger.error({ err }, 'Failed to ensure PR: ' + prTitle);
}
if (existingPr) {
- return { pr: existingPr };
+ return { type: 'with-pr', pr: existingPr };
}
// istanbul ignore next
- return { prBlockedBy: 'Error' };
+ return { type: 'without-pr', prBlockedBy: 'Error' };
}
--
GitLab