feat: support timeout for pr creation = not-pending (#748)

We should not leave the PR unopened forever if the branch remains in not-pending state too long. Some status checks may leave the status as “pending” instead of “failed”. Defaults to 12 hours but is configurable.

Closes #747

docs/configuration.md

@@ -98,6 +98,7 @@ $ node renovate --help
     --rebase-stale-prs [boolean]         Rebase stale PRs (GitHub only)
     --unpublish-safe [boolean]           Set a status check for unpublish-safe upgrades
     --pr-creation <string>               When to create the PR for a branch. Values: immediate, not-pending, status-success.
+    --pr-not-pending-hours <integer>     Timeout in hours for when prCreation=not-pending
     --automerge [boolean]                Whether to automerge branches/PRs automatically, without human intervention
     --automerge-type <string>            How to automerge - "branch-merge-commit", "branch-push" or "pr". Branch support is GitHub-only
     --lazy-grouping [boolean]            Use group names only when multiple dependencies upgraded
@@ -515,6 +516,14 @@ Obviously, you can't set repository or package file location with this method.
   <td>`RENOVATE_PR_CREATION`</td>
   <td>`--pr-creation`<td>
 </tr>
+<tr>
+  <td>`prNotPendingHours`</td>
+  <td>Timeout in hours for when prCreation=not-pending</td>
+  <td>integer</td>
+  <td><pre>12</pre></td>
+  <td>`RENOVATE_PR_NOT_PENDING_HOURS`</td>
+  <td>`--pr-not-pending-hours`<td>
+</tr>
 <tr>
   <td>`automerge`</td>
   <td>Whether to automerge branches/PRs automatically, without human intervention</td>

lib/api/github.js

@@ -24,6 +24,7 @@ module.exports = {
   setBranchStatus,
   deleteBranch,
   mergeBranch,
+  getBranchLastCommitTime,
   // issue
   addAssignees,
   addReviewers,
@@ -44,6 +45,8 @@ module.exports = {
   getFileJson,
   // Commits
   getCommitMessages,
+  getBranchCommit,
+  getCommitDetails,
 };
 
 // Get all installations for a GitHub app
@@ -411,6 +414,18 @@ async function mergeBranch(branchName, mergeType) {
   await deleteBranch(branchName);
 }
 
+async function getBranchLastCommitTime(branchName) {
+  try {
+    const res = await ghGotRetry(
+      `repos/${config.repoName}/commits?sha=${branchName}`
+    );
+    return new Date(res.body[0].commit.committer.date);
+  } catch (err) {
+    logger.error({ err }, `getBranchLastCommitTime error`);
+    return new Date();
+  }
+}
+
 // Issue
 
 async function addAssignees(issueNo, assignees) {
@@ -741,9 +756,10 @@ async function createBlob(fileContents) {
 
 // Return the commit SHA for a branch
 async function getBranchCommit(branchName) {
-  return (await ghGotRetry(
+  const res = await ghGotRetry(
     `repos/${config.repoName}/git/refs/heads/${branchName}`
-  )).body.object.sha;
+  );
+  return res.body.object.sha;
 }
 
 async function getCommitDetails(commit) {

lib/api/gitlab.js

@@ -18,6 +18,7 @@ module.exports = {
   getBranchStatusCheck,
   setBranchStatus,
   deleteBranch,
+  getBranchLastCommitTime,
   // issue
   addAssignees,
   addReviewers,
@@ -270,6 +271,18 @@ async function deleteBranch(branchName) {
   );
 }
 
+async function getBranchLastCommitTime(branchName) {
+  try {
+    const res = await glGot(
+      `projects/${config.repoName}/repository/commits?ref_name=${branchName}`
+    );
+    return new Date(res.body[0].committed_date);
+  } catch (err) {
+    logger.error({ err }, `getBranchLastCommitTime error`);
+    return new Date();
+  }
+}
+
 // Issue
 
 async function addAssignees(prNo, assignees) {

lib/config/definitions.js

@@ -376,6 +376,12 @@ const options = [
     type: 'string',
     default: 'immediate',
   },
+  {
+    name: 'prNotPendingHours',
+    description: 'Timeout in hours for when prCreation=not-pending',
+    type: 'integer',
+    default: 12,
+  },
   // Automatic merging
   {
     name: 'automerge',

lib/workers/pr/index.js

@@ -45,8 +45,24 @@ async function ensurePr(prConfig) {
   } else if (config.prCreation === 'not-pending') {
     logger.debug('Checking branch combined status');
     if (branchStatus === 'pending' || branchStatus === 'running') {
-      logger.debug(`Branch status is "${branchStatus}" - not creating PR`);
+      logger.debug(`Branch status is "${branchStatus}" - checking timeout`);
-      return null;
+      const lastCommitTime = await config.api.getBranchLastCommitTime(
+        branchName
+      );
+      const currentTime = new Date();
+      const millisecondsPerHour = 1000 * 60 * 60;
+      const elapsedHours = Math.round(
+        (currentTime.getTime() - lastCommitTime.getTime()) / millisecondsPerHour
+      );
+      if (elapsedHours < config.prNotPendingHours) {
+        logger.debug(
+          `Branch is ${elapsedHours} hours old - skipping PR creation`
+        );
+        return null;
+      }
+      logger.debug(
+        `prNotPendingHours=${config.prNotPendingHours} threshold hit - creating PR`
+      );
     }
     logger.debug('Branch status success');
   }

test/api/__snapshots__/github.spec.js.snap

@@ -689,6 +689,8 @@ Array [
 ]
 `;
 
+exports[`api/github getBranchLastCommitTime should return a Date 1`] = `2011-04-14T16:00:49.000Z`;
+
 exports[`api/github getBranchPr(branchName) should return null if no PR exists 1`] = `
 Array [
   Array [

test/api/__snapshots__/gitlab.spec.js.snap

@@ -195,6 +195,8 @@ Array [
 
 exports[`api/gitlab getBranch returns a branch 1`] = `"foo"`;
 
+exports[`api/gitlab getBranchLastCommitTime should return a Date 1`] = `2012-09-20T08:50:22.000Z`;
+
 exports[`api/gitlab getBranchPr(branchName) should return null if no PR exists 1`] = `
 Array [
   Array [

test/api/github.spec.js

@@ -1044,6 +1044,32 @@ describe('api/github', () => {
       expect(ghGot.delete.mock.calls).toMatchSnapshot();
     });
   });
+  describe('getBranchLastCommitTime', () => {
+    it('should return a Date', async () => {
+      await initRepo('some/repo', 'token');
+      ghGot.mockReturnValueOnce({
+        body: [
+          {
+            commit: {
+              committer: {
+                date: '2011-04-14T16:00:49Z',
+              },
+            },
+          },
+        ],
+      });
+      const res = await github.getBranchLastCommitTime('some-branch');
+      expect(res).toMatchSnapshot();
+    });
+    it('handles error', async () => {
+      await initRepo('some/repo', 'token');
+      ghGot.mockReturnValueOnce({
+        body: [],
+      });
+      const res = await github.getBranchLastCommitTime('some-branch');
+      expect(res).toBeDefined();
+    });
+  });
   describe('addAssignees(issueNo, assignees)', () => {
     it('should add the given assignees to the issue', async () => {
       await initRepo('some/repo', 'token');

test/api/gitlab.spec.js

@@ -399,6 +399,51 @@ describe('api/gitlab', () => {
       expect(glGot.delete.mock.calls.length).toBe(1);
     });
   });
+  describe('getBranchLastCommitTime', () => {
+    it('should return a Date', async () => {
+      await initRepo('some/repo', 'token');
+      glGot.mockReturnValueOnce({
+        body: [
+          {
+            id: 'ed899a2f4b50b4370feeea94676502b42383c746',
+            short_id: 'ed899a2f4b5',
+            title: 'Replace sanitize with escape once',
+            author_name: 'Dmitriy Zaporozhets',
+            author_email: 'dzaporozhets@sphereconsultinginc.com',
+            authored_date: '2012-09-20T11:50:22+03:00',
+            committer_name: 'Administrator',
+            committer_email: 'admin@example.com',
+            committed_date: '2012-09-20T11:50:22+03:00',
+            created_at: '2012-09-20T11:50:22+03:00',
+            message: 'Replace sanitize with escape once',
+            parent_ids: ['6104942438c14ec7bd21c6cd5bd995272b3faff6'],
+          },
+          {
+            id: '6104942438c14ec7bd21c6cd5bd995272b3faff6',
+            short_id: '6104942438c',
+            title: 'Sanitize for network graph',
+            author_name: 'randx',
+            author_email: 'dmitriy.zaporozhets@gmail.com',
+            committer_name: 'Dmitriy',
+            committer_email: 'dmitriy.zaporozhets@gmail.com',
+            created_at: '2012-09-20T09:06:12+03:00',
+            message: 'Sanitize for network graph',
+            parent_ids: ['ae1d9fb46aa2b07ee9836d49862ec4e2c46fbbba'],
+          },
+        ],
+      });
+      const res = await gitlab.getBranchLastCommitTime('some-branch');
+      expect(res).toMatchSnapshot();
+    });
+    it('handles error', async () => {
+      await initRepo('some/repo', 'token');
+      glGot.mockReturnValueOnce({
+        body: [],
+      });
+      const res = await gitlab.getBranchLastCommitTime('some-branch');
+      expect(res).toBeDefined();
+    });
+  });
   describe('addAssignees(issueNo, assignees)', () => {
     it('should add the given assignees to the issue', async () => {
       await initRepo('some/repo', 'token');

test/workers/package/__snapshots__/index.spec.js.snap

@@ -13,6 +13,7 @@ Array [
   "rebaseStalePrs",
   "unpublishSafe",
   "prCreation",
+  "prNotPendingHours",
   "automerge",
   "automergeType",
   "requiredStatusChecks",
@@ -47,6 +48,7 @@ Array [
   "rebaseStalePrs",
   "unpublishSafe",
   "prCreation",
+  "prNotPendingHours",
   "automerge",
   "automergeType",
   "requiredStatusChecks",
@@ -204,6 +206,7 @@ Please make sure the following warnings are safe to ignore:
 
 This {{#if isGitHub}}PR{{else}}MR{{/if}} has been generated by [Renovate Bot](https://renovateapp.com).",
     "prCreation": "immediate",
+    "prNotPendingHours": 12,
     "prTitle": "{{#if isPin}}Pin{{else}}{{#if isRollback}}Roll back{{else}}Update{{/if}}{{/if}} dependency {{depName}} to {{#if isRange}}{{newVersion}}{{else}}{{#if isMajor}}v{{newVersionMajor}}{{else}}v{{newVersion}}{{/if}}{{/if}}",
     "rebaseStalePrs": false,
     "recreateClosed": false,
@@ -357,6 +360,7 @@ Please make sure the following warnings are safe to ignore:
 
 This {{#if isGitHub}}PR{{else}}MR{{/if}} has been generated by [Renovate Bot](https://renovateapp.com).",
     "prCreation": "immediate",
+    "prNotPendingHours": 12,
     "prTitle": "{{#if isPin}}Pin{{else}}{{#if isRollback}}Roll back{{else}}Update{{/if}}{{/if}} dependency {{depName}} to {{#if isRange}}{{newVersion}}{{else}}{{#if isMajor}}v{{newVersionMajor}}{{else}}v{{newVersion}}{{/if}}{{/if}}",
     "rebaseStalePrs": false,
     "recreateClosed": false,

test/workers/pr/index.spec.js

@@ -139,10 +139,21 @@ describe('workers/pr', () => {
     });
     it('should return null if waiting for not pending', async () => {
       config.api.getBranchStatus = jest.fn(() => 'pending');
+      config.api.getBranchLastCommitTime = jest.fn(() => new Date());
       config.prCreation = 'not-pending';
       const pr = await prWorker.ensurePr(config);
       expect(pr).toBe(null);
     });
+    it('should create PR if pending timeout hit', async () => {
+      config.api.getBranchStatus = jest.fn(() => 'pending');
+      config.api.getBranchLastCommitTime = jest.fn(
+        () => new Date('2017-01-01')
+      );
+      config.prCreation = 'not-pending';
+      config.api.getBranchPr = jest.fn();
+      const pr = await prWorker.ensurePr(config);
+      expect(pr).toMatchObject({ displayNumber: 'New Pull Request' });
+    });
     it('should create PR if no longer pending', async () => {
       config.api.getBranchStatus = jest.fn(() => 'failed');
       config.api.getBranchPr = jest.fn();