refactor: remove checkForClosedPr

replace with findPr

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;

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

lib/workers/branch/check-existing.js

@@ -14,30 +14,21 @@ 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)
-      ) {
+    if (problemStart.isBefore(closedAt) && closedAt.isBefore(problemStopped)) {
       logger.info(
         { closedAt, problemStart, problemStopped },
         'Ignoring mistakenly closed PR'
       );
       return false;
     }
-    }
     return true;
   }
   // Check for legacy PR title format
@@ -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;
   }

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');

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({

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',
       });