git-node: wait time for PR with single approval

One Collaborator approval is enough only if the pull request has been open for more than 7 days Refs: nodejs/node#22255
nodejs · joyeecheung · Dec 3, 2018 · Oct 7, 2018 · Dec 3, 2018 · Dec 3, 2018
commit 1b0cc367c343e63179768aa9a63b4003e3a9330a
diff --git a/lib/pr_checker.js b/lib/pr_checker.js
@@ -5,6 +5,7 @@ const MINUTE = SECOND * 60;
 const HOUR = MINUTE * 60;
 
 const WAIT_TIME = 48;
+const WAIT_TIME_SINGLE_APPROVAL = 168;
 
 const {
   REVIEW_SOURCES: { FROM_COMMENT, FROM_REVIEW_COMMENT }
@@ -132,12 +133,16 @@ class PRChecker {
    */
   getWait(now) {
     const createTime = new Date(this.pr.createdAt);
-    const timeLeft = WAIT_TIME - Math.ceil(
+    const hoursFromCreateTime = Math.ceil(
       (now.getTime() - createTime.getTime()) / HOUR
     );
+    const timeLeft = WAIT_TIME - hoursFromCreateTime;
+    const timeLeftSingleApproval =
+      WAIT_TIME_SINGLE_APPROVAL - hoursFromCreateTime;
 
     return {
-      timeLeft
+      timeLeft,
+      timeLeftSingleApproval
     };
   }
 
@@ -149,12 +154,12 @@ class PRChecker {
     const {
       pr, cli, reviewers, CIStatus
     } = this;
+    const { approved } = reviewers;
     const labels = pr.labels.nodes;
 
     const fast =
       labels.some((l) => ['fast-track'].includes(l.name));
     if (fast) {
-      const { approved } = reviewers;
       if (approved.length > 1 && CIStatus) {
         cli.info('This PR is being fast-tracked');
         return true;
@@ -171,14 +176,25 @@ class PRChecker {
       return false;
     }
 
+    const dateStr = new Date(pr.createdAt).toDateString();
+    cli.info(`This PR was created on ${dateStr}`);
+
     const wait = this.getWait(now);
-    if (wait.timeLeft > 0) {
-      const dateStr = new Date(pr.createdAt).toDateString();
-      cli.info(`This PR was created on ${dateStr}`);
+    if (approved.length > 1 && wait.timeLeft > 0) {
       cli.warn(`Wait at least ${wait.timeLeft} more hours before landing`);
       return false;
+    } else if (approved.length === 1 && wait.timeLeftSingleApproval > 0) {
+      const waitMsg = wait.timeLeft > 0
+        ? ` and ${wait.timeLeft} more hours`
+        : '';
+      cli.warn('Wait at one of the following:');
+      cli.warn(`* another approval${waitMsg}`);
+      cli.warn(`* ${wait.timeLeftSingleApproval} more hours` +
+        ' with existing single approval');
+      return false;
     }
 
+    cli.info('No more waiting required');
     return true;
   }
 

diff --git a/test/fixtures/data.js b/test/fixtures/data.js
@@ -15,6 +15,10 @@ const allGreenReviewers = {
   approved,
   requestedChanges: []
 };
+const singleGreenReviewer = {
+  approved: [approved[0]],
+  requestedChanges: []
+};
 const requestedChangesReviewers = {
   requestedChanges,
   approved: []
@@ -76,6 +80,7 @@ module.exports = {
   approved,
   requestedChanges,
   allGreenReviewers,
+  singleGreenReviewer,
   requestedChangesReviewers,
   approvingReviews,
   requestingChangesReviews,

diff --git a/test/unit/pr_checker.test.js b/test/unit/pr_checker.test.js
@@ -10,6 +10,7 @@ const PRChecker = require('../../lib/pr_checker');
 
 const {
   allGreenReviewers,
+  singleGreenReviewer,
   requestedChangesReviewers,
   approvingReviews,
   requestingChangesReviews,
@@ -169,7 +170,9 @@ describe('PRChecker', () => {
       const cli = new TestCLI();
 
       const expectedLogs = {
-        warn: [['Wait at least 22 more hours before landing']],
+        warn: [
+          ['Wait at least 22 more hours before landing']
+        ],
         info: [['This PR was created on Tue Oct 31 2017']]
       };
 
@@ -197,6 +200,78 @@ describe('PRChecker', () => {
       cli.assertCalledWith(expectedLogs);
     });
 
+    it('should warn about PR with single approval (<48h)', () => {
+      const cli = new TestCLI();
+
+      const expectedLogs = {
+        warn: [
+          ['Wait at one of the following:'],
+          ['* another approval and 22 more hours'],
+          ['* 142 more hours with existing single approval']
+        ],
+        info: [['This PR was created on Tue Oct 31 2017']]
+      };
+
+      const now = new Date('2017-11-01T14:25:41.682Z');
+      const youngPR = Object.assign({}, firstTimerPR, {
+        createdAt: '2017-10-31T13:00:41.682Z'
+      });
+
+      const data = {
+        pr: youngPR,
+        reviewers: singleGreenReviewer,
+        comments: commentsWithLGTM,
+        reviews: approvingReviews,
+        commits: simpleCommits,
+        collaborators,
+        authorIsNew: () => true,
+        getThread() {
+          return PRData.prototype.getThread.call(this);
+        }
+      };
+      const checker = new PRChecker(cli, data, argv);
+
+      const status = checker.checkPRWait(now);
+      assert(!status);
+      cli.assertCalledWith(expectedLogs);
+    });
+
+    it('should warn about PR with single approval (>48h)', () => {
+      const cli = new TestCLI();
+
+      const expectedLogs = {
+        warn: [
+          ['Wait at one of the following:'],
+          ['* another approval'],
+          ['* 94 more hours with existing single approval']
+        ],
+        info: [['This PR was created on Tue Oct 31 2017']]
+      };
+
+      const now = new Date('2017-11-03T14:25:41.682Z');
+      const youngPR = Object.assign({}, firstTimerPR, {
+        createdAt: '2017-10-31T13:00:41.682Z'
+      });
+
+      const data = {
+        pr: youngPR,
+        reviewers: singleGreenReviewer,
+        comments: commentsWithLGTM,
+        reviews: approvingReviews,
+        commits: simpleCommits,
+        collaborators,
+        authorIsNew: () => true,
+        getThread() {
+          return PRData.prototype.getThread.call(this);
+        }
+      };
+      const checker = new PRChecker(cli, data, argv);
+
+      const status = checker.checkPRWait(now);
+      assert(!status);
+      cli.assertCalledWith(expectedLogs);
+    });
+
     it('should log as expected if PR can be fast-tracked', () => {
       const cli = new TestCLI();