From 8fc61e35bcec54a22bc0c93450d47819e5246b79 Mon Sep 17 00:00:00 2001
From: Rhys Arkins <rhys@keylocation.sg>
Date: Fri, 13 Oct 2017 10:56:18 +0200
Subject: [PATCH] feat: add assignees and reviewers whenever status checks fail
(#928)
This feature means that you can configure branches/PRs to automerge, but if status checks fail (preventing automerge) then you can still get assigneed the PR to take action. Previously such PRs remained unassigned because we do not assign automerging PRs by default, to reduce noise.
Closes #722
---
lib/workers/pr/index.js | 67 +++++++++++--------
.../pr/__snapshots__/index.spec.js.snap | 2 +
test/workers/pr/index.spec.js | 23 +++++++
3 files changed, 63 insertions(+), 29 deletions(-)
diff --git a/lib/workers/pr/index.js b/lib/workers/pr/index.js
index 6d9db42e51..3f9df72d49 100644
--- a/lib/workers/pr/index.js
+++ b/lib/workers/pr/index.js
@@ -143,6 +143,10 @@ async function ensurePr(prConfig) {
// Check if existing PR exists
const existingPr = await config.api.getBranchPr(branchName);
if (existingPr) {
+ if (config.automerge && branchStatus === 'failure') {
+ logger.debug(`Setting assignees and reviewers as status checks failed`);
+ await addAssigneesReviewers(config, existingPr);
+ }
// Check if existing PR needs updating
if (existingPr.title === prTitle && existingPr.body === prBody) {
logger.info(`${existingPr.displayNumber} does not need updating`);
@@ -166,39 +170,16 @@ async function ensurePr(prConfig) {
await config.api.addLabels(pr.number, config.labels);
}
// Skip assign and review if automerging PR
- if (config.automerge && config.automergeType === 'pr') {
+ if (
+ config.automerge &&
+ config.automergeType === 'pr' &&
+ branchStatus !== 'failure'
+ ) {
logger.debug(
`Skipping assignees and reviewers as automerge=${config.automerge}`
);
} else {
- if (config.assignees.length > 0) {
- logger.debug({ assignees: config.assignees }, 'Adding assignees');
- try {
- const assignees = config.assignees.map(
- assignee =>
- assignee.length && assignee[0] === '@'
- ? assignee.slice(1)
- : assignee
- );
- await config.api.addAssignees(pr.number, assignees);
- } catch (err) {
- logger.warn({ err }, 'Failed to add assignees');
- }
- }
- if (config.reviewers.length > 0) {
- logger.debug({ reviewers: config.reviewers }, 'Adding reviewers');
- try {
- const reviewers = config.reviewers.map(
- reviewer =>
- reviewer.length && reviewer[0] === '@'
- ? reviewer.slice(1)
- : reviewer
- );
- await config.api.addReviewers(pr.number, reviewers);
- } catch (err) {
- logger.warn({ err }, 'Failed to add reviewers');
- }
- }
+ await addAssigneesReviewers(config, pr);
}
logger.info(`Created ${pr.displayNumber}`);
return pr;
@@ -208,6 +189,34 @@ async function ensurePr(prConfig) {
return null;
}
+async function addAssigneesReviewers(config, pr) {
+ const { logger } = config;
+ if (config.assignees.length > 0) {
+ logger.debug({ assignees: config.assignees }, 'Adding assignees');
+ try {
+ const assignees = config.assignees.map(
+ assignee =>
+ assignee.length && assignee[0] === '@' ? assignee.slice(1) : assignee
+ );
+ await config.api.addAssignees(pr.number, assignees);
+ } catch (err) {
+ logger.warn({ err }, 'Failed to add assignees');
+ }
+ }
+ if (config.reviewers.length > 0) {
+ logger.debug({ reviewers: config.reviewers }, 'Adding reviewers');
+ try {
+ const reviewers = config.reviewers.map(
+ reviewer =>
+ reviewer.length && reviewer[0] === '@' ? reviewer.slice(1) : reviewer
+ );
+ await config.api.addReviewers(pr.number, reviewers);
+ } catch (err) {
+ logger.warn({ err }, 'Failed to add reviewers');
+ }
+ }
+}
+
async function checkAutoMerge(pr, config) {
const { logger } = config;
logger.trace({ config }, 'checkAutoMerge');
diff --git a/test/workers/pr/__snapshots__/index.spec.js.snap b/test/workers/pr/__snapshots__/index.spec.js.snap
index 619d8f2a1d..f4400485f0 100644
--- a/test/workers/pr/__snapshots__/index.spec.js.snap
+++ b/test/workers/pr/__snapshots__/index.spec.js.snap
@@ -1,5 +1,7 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
+exports[`workers/pr ensurePr should add assignees and reviewers to existing PR 1`] = `Array []`;
+
exports[`workers/pr ensurePr should add assignees and reviewers to new PR 1`] = `
Array [
Array [
diff --git a/test/workers/pr/index.spec.js b/test/workers/pr/index.spec.js
index 4c88bd3588..564bb8b522 100644
--- a/test/workers/pr/index.spec.js
+++ b/test/workers/pr/index.spec.js
@@ -95,6 +95,8 @@ describe('workers/pr', () => {
config = {
...defaultConfig,
api: {
+ addAssignees: jest.fn(),
+ addReviewers: jest.fn(),
createPr: jest.fn(() => ({ displayNumber: 'New Pull Request' })),
getBranchStatus: jest.fn(),
},
@@ -266,6 +268,27 @@ describe('workers/pr', () => {
expect(config.api.addAssignees.mock.calls.length).toBe(0);
expect(config.api.addReviewers.mock.calls.length).toBe(0);
});
+ it('should add assignees and reviewers to existing PR', async () => {
+ config.depName = 'dummy';
+ config.automerge = true;
+ config.assignees = ['bar'];
+ config.reviewers = ['baz'];
+ config.isGitHub = true;
+ config.privateRepo = true;
+ config.currentVersion = '1.0.0';
+ config.newVersion = '1.1.0';
+ config.repositoryUrl = 'https://github.com/renovateapp/dummy';
+ config.api.getBranchPr = jest.fn(() => existingPr);
+ config.api.getBranchStatus.mockReturnValueOnce('failure');
+ config.api.updatePr = jest.fn();
+ config.semanticPrefix = '';
+ const pr = await prWorker.ensurePr(config);
+ expect(config.api.updatePr.mock.calls).toMatchSnapshot();
+ expect(config.api.updatePr.mock.calls.length).toBe(0);
+ expect(config.api.addAssignees.mock.calls.length).toBe(1);
+ expect(config.api.addReviewers.mock.calls.length).toBe(1);
+ expect(pr).toMatchObject(existingPr);
+ });
it('should return unmodified existing PR', async () => {
config.depName = 'dummy';
config.isGitHub = true;
--
GitLab