diff --git a/lib/api/github.js b/lib/api/github.js
index 93b1fa398efbb90daada523241b36ebc51107e3d..7c371b95df9cb951477a42dca99dc0b3531222ff 100644
--- a/lib/api/github.js
+++ b/lib/api/github.js
@@ -31,7 +31,6 @@ module.exports = {
addLabels,
// PR
findPr,
- checkForClosedPr,
createPr,
getPr,
getAllPrs,
@@ -463,6 +462,8 @@ async function addLabels(issueNo, labels) {
});
}
+// Pull Request
+
async function findPr(branchName, prTitle, state = 'all') {
logger.debug(`findPr(${branchName}, ${state})`);
const urlString = `repos/${config.repoName}/pulls?head=${config.owner}:${branchName}&state=${state}`;
@@ -481,25 +482,6 @@ async function findPr(branchName, prTitle, state = 'all') {
return pr;
}
-// Pull Request
-async function checkForClosedPr(branchName, prTitle) {
- logger.debug(`checkForClosedPr(${branchName}, ${prTitle})`);
- const url = `repos/${config.repoName}/pulls?state=closed&head=${config.owner}:${branchName}`;
- try {
- const res = await get(url);
- // Return true if any of the titles match exactly
- return res.body.some(
- pr =>
- pr.title === prTitle &&
- pr.head.label === `${config.owner}:${branchName}`
- );
- } catch (err) {
- // Sometimes this fails with a 401 for no apparently reason, so we catch and reeturn false
- logger.info({ err }, 'Error checking for closed PR. Continuing..');
- return false;
- }
-}
-
// Creates PR and returns PR number
async function createPr(branchName, title, body, useDefaultBranch) {
const base = useDefaultBranch ? config.defaultBranch : config.baseBranch;
diff --git a/lib/api/gitlab.js b/lib/api/gitlab.js
index afec808201f4646d2a131e568e82834565dfab8e..f2b043370752c0131eb46d4711c04bd67b03bd85 100644
--- a/lib/api/gitlab.js
+++ b/lib/api/gitlab.js
@@ -25,7 +25,6 @@ module.exports = {
addLabels,
// PR
findPr,
- checkForClosedPr,
createPr,
getPr,
updatePr,
@@ -366,13 +365,6 @@ async function findPr(branchName, prTitle, state = 'all') {
}
// Pull Request
-async function checkForClosedPr(branchName, prTitle) {
- const pr = await findPr(branchName, prTitle, 'closed');
- if (pr) {
- return true;
- }
- return false;
-}
async function createPr(branchName, title, body, useDefaultBranch) {
const targetBranch = useDefaultBranch
diff --git a/lib/workers/branch/check-existing.js b/lib/workers/branch/check-existing.js
index 3ab995ef649c7fc4fd31714d1a05ee9f23b9eb6b..6fdd4621d3618ecb77201f73c70a0c92d01ae9d2 100644
--- a/lib/workers/branch/check-existing.js
+++ b/lib/workers/branch/check-existing.js
@@ -14,29 +14,20 @@ async function prAlreadyExisted(config) {
logger.debug('recreateClosed is false');
// Return if same PR already existed
// Check for current PR title format
- if (await config.api.checkForClosedPr(config.branchName, config.prTitle)) {
+ const pr = config.api.findPr(config.branchName, config.prTitle, 'closed');
+ if (pr) {
logger.debug('Found closed PR with current title');
// this code exists to ignore mistakenly closed PRs which occurred due to a bug
// TODO: Remove this by end of October 2017 or in v10
- const pr = await config.api.findPr(
- config.branchName,
- config.prTitle,
- 'closed'
- );
- if (pr) {
- const closedAt = moment(pr.closed_at);
- const problemStart = moment('2017-10-15T20:00:00Z');
- const problemStopped = moment('2017-10-16T06:00:00Z');
- if (
- problemStart.isBefore(closedAt) &&
- closedAt.isBefore(problemStopped)
- ) {
- logger.info(
- { closedAt, problemStart, problemStopped },
- 'Ignoring mistakenly closed PR'
- );
- return false;
- }
+ const closedAt = moment(pr.closed_at);
+ const problemStart = moment('2017-10-15T20:00:00Z');
+ const problemStopped = moment('2017-10-16T06:00:00Z');
+ if (problemStart.isBefore(closedAt) && closedAt.isBefore(problemStopped)) {
+ logger.info(
+ { closedAt, problemStart, problemStopped },
+ 'Ignoring mistakenly closed PR'
+ );
+ return false;
}
return true;
}
@@ -45,7 +36,7 @@ async function prAlreadyExisted(config) {
const legacyPrTitle = config.prTitle
.replace(/to v(\d+)$/, 'to version $1.x') // Major
.replace(/to v(\d+)/, 'to version $1'); // Non-major
- if (await config.api.checkForClosedPr(config.branchName, legacyPrTitle)) {
+ if (await config.api.findPr(config.branchName, legacyPrTitle, 'closed')) {
logger.debug('Found closed PR with legacy title');
return true;
}
diff --git a/test/api/github.spec.js b/test/api/github.spec.js
index 6cd0e455765e2cb589344650661c6059a3cc6c6d..9f4fbae38644e0263b7971943c445064ca28d3e2 100644
--- a/test/api/github.spec.js
+++ b/test/api/github.spec.js
@@ -987,34 +987,6 @@ describe('api/github', () => {
expect(pr).toMatchSnapshot();
});
});
- describe('checkForClosedPr(branchName, prTitle)', () => {
- [
- ['some-branch', 'foo', true],
- ['some-branch', 'bar', false],
- ['some-branch', 'bop', false],
- ].forEach(([branch, title, expected], i) => {
- it(`should return true if a closed PR is found - ${i}`, async () => {
- await initRepo('some/repo', 'token');
- get.mockImplementationOnce(() => ({
- body: [
- { title: 'foo', head: { label: 'theowner:some-branch' } },
- { title: 'bar', head: { label: 'theowner:some-other-branch' } },
- { title: 'baz', head: { label: 'theowner:some-branch' } },
- ],
- }));
- const res = await github.checkForClosedPr(branch, title);
- expect(res).toBe(expected);
- });
- });
- it(`should return false if error thrown`, async () => {
- await initRepo('some/repo', 'token');
- get.mockImplementationOnce(() => {
- throw new Error('fail');
- });
- const res = await github.checkForClosedPr('some-branch', 'some-title');
- expect(res).toBe(false);
- });
- });
describe('createPr(branchName, title, body)', () => {
it('should create and return a PR object', async () => {
await initRepo('some/repo', 'token');
diff --git a/test/api/gitlab.spec.js b/test/api/gitlab.spec.js
index 6d02a9832474e1a58ac1333ec929183ca780ff21..bd8c631127a441eb4ccaa2f6de0b32f755cf29ab 100644
--- a/test/api/gitlab.spec.js
+++ b/test/api/gitlab.spec.js
@@ -541,31 +541,6 @@ describe('api/gitlab', () => {
expect(pr.number).toBe(2);
});
});
- describe('checkForClosedPr(branchName, prTitle)', () => {
- it('returns true if pr exists', async () => {
- get.mockReturnValueOnce({
- body: [
- {
- source_branch: 'some-branch',
- id: 1,
- },
- {
- source_branch: 'some-branch',
- id: 2,
- },
- ],
- });
- const res = await gitlab.checkForClosedPr('some-branch');
- expect(res).toBe(true);
- });
- it('returns false if pr does not exist', async () => {
- get.mockReturnValueOnce({
- body: [],
- });
- const res = await gitlab.checkForClosedPr('some-branch');
- expect(res).toBe(false);
- });
- });
describe('createPr(branchName, title, body)', () => {
it('returns the PR', async () => {
get.post.mockReturnValueOnce({
diff --git a/test/workers/branch/check-existing.spec.js b/test/workers/branch/check-existing.spec.js
index d3acc030605eaaba5f1fbcfacd596c6b9c870c48..2bf417c0b5864e8aa701b2c026f1469de01079ab 100644
--- a/test/workers/branch/check-existing.spec.js
+++ b/test/workers/branch/check-existing.spec.js
@@ -10,7 +10,7 @@ describe('workers/branch/check-existing', () => {
beforeEach(() => {
config = {
...defaultConfig,
- api: { checkForClosedPr: jest.fn(), findPr: jest.fn() },
+ api: { findPr: jest.fn() },
logger,
branchName: 'some-branch',
prTitle: 'some-title',
@@ -19,26 +19,25 @@ describe('workers/branch/check-existing', () => {
it('returns false if recreating closed PRs', async () => {
config.recreateClosed = true;
expect(await prAlreadyExisted(config)).toBe(false);
- expect(config.api.checkForClosedPr.mock.calls.length).toBe(0);
+ expect(config.api.findPr.mock.calls.length).toBe(0);
});
it('returns false if both checks miss', async () => {
config.recreatedClosed = true;
expect(await prAlreadyExisted(config)).toBe(false);
- expect(config.api.checkForClosedPr.mock.calls.length).toBe(2);
+ expect(config.api.findPr.mock.calls.length).toBe(2);
});
it('returns true if first check hits', async () => {
- config.api.checkForClosedPr.mockReturnValueOnce(true);
+ config.api.findPr.mockReturnValueOnce({});
expect(await prAlreadyExisted(config)).toBe(true);
- expect(config.api.checkForClosedPr.mock.calls.length).toBe(1);
+ expect(config.api.findPr.mock.calls.length).toBe(1);
});
it('returns true if second check hits', async () => {
- config.api.checkForClosedPr.mockReturnValueOnce(false);
- config.api.checkForClosedPr.mockReturnValueOnce(true);
+ config.api.findPr.mockReturnValueOnce(null);
+ config.api.findPr.mockReturnValueOnce({});
expect(await prAlreadyExisted(config)).toBe(true);
- expect(config.api.checkForClosedPr.mock.calls.length).toBe(2);
+ expect(config.api.findPr.mock.calls.length).toBe(2);
});
it('returns false if mistaken', async () => {
- config.api.checkForClosedPr.mockReturnValueOnce(true);
config.api.findPr.mockReturnValueOnce({
closed_at: '2017-10-15T21:28:07.000Z',
});