From 72a2034adecbeb2873ef7c4d10a7760b6d1cb3e4 Mon Sep 17 00:00:00 2001
From: Zheyu Zhang <zheyuzhang03@gmail.com>
Date: Mon, 6 Nov 2017 19:46:25 -0600
Subject: [PATCH] feat: check commits after the last ci run (#69)

Fixes: https://github.com/joyeecheung/node-core-utils/issues/66

* fixup: format warn message && format code

* fixup: set status false if exist commit after last ci

* fix: define lastCI as oldest one in latesst cis of all types

* test: add more commit cases in commits_after_ci.json

* feat: add bullet in warning message

* style: format code

* fix: log commit's title only
---
 lib/pr_checker.js                   | 25 +++++++++++++++-
 test/fixtures/commits_after_ci.json | 44 +++++++++++++++++++++++++++++
 test/fixtures/data.js               |  2 ++
 test/unit/pr_checker.test.js        | 35 ++++++++++++++++++++++-
 4 files changed, 104 insertions(+), 2 deletions(-)
 create mode 100644 test/fixtures/commits_after_ci.json

diff --git a/lib/pr_checker.js b/lib/pr_checker.js
index a6773067..c4a18835 100644
--- a/lib/pr_checker.js
+++ b/lib/pr_checker.js
@@ -159,7 +159,7 @@ class PRChecker {
   // TODO: we might want to check CI status when it's less flaky...
   // TODO: not all PR requires CI...labels?
   checkCI() {
-    const { pr, logger, comments, reviews } = this;
+    const { pr, logger, comments, reviews, commits } = this;
     const prNode = {
       publishedAt: pr.createdAt,
       bodyText: pr.bodyText
@@ -175,9 +175,32 @@ class PRChecker {
       logger.warn('No full CI runs detected');
     }
 
+    let lastCI;
     for (const [type, ci] of ciMap) {
       const name = CI_TYPES.get(type).name;
       logger.info(`Last ${name} CI on ${ci.date}: ${ci.link}`);
+      if (!lastCI || lastCI.date > ci.date) {
+        lastCI = {
+          typeName: name,
+          date: ci.date
+        };
+      }
+    }
+
+    if (lastCI) {
+      let didLogHeader = false;
+      commits.forEach((c) => {
+        const commit = c.commit;
+        if (commit.committedDate > lastCI.date) {
+          if (!didLogHeader) {
+            status = false;
+            didLogHeader = true;
+            logger.warn(`Commits pushed after the last ${lastCI.typeName} ` +
+            'CI run:');
+          }
+          logger.warn(`- ${commit.message.split('\n')[0]}`);
+        }
+      });
     }
 
     return status;
diff --git a/test/fixtures/commits_after_ci.json b/test/fixtures/commits_after_ci.json
new file mode 100644
index 00000000..8755d755
--- /dev/null
+++ b/test/fixtures/commits_after_ci.json
@@ -0,0 +1,44 @@
+  {
+    "commits": [{
+      "commit": {
+        "committedDate": "2017-10-25T11:27:02Z",
+        "oid": "6c0945cbeea2cbbc97d13a3d9e3fe68bd145b985",
+        "message": "fixup: adjust spelling",
+        "author": {
+          "login": "bar"
+        }
+      }
+    },{
+      "commit": {
+        "committedDate": "2017-10-26T12:10:20Z",
+        "oid": "9d098ssiskj8dhd39js0sjd0cn2ng4is9n40sj12d",
+        "message": "doc: add api description README",
+        "author": {
+          "login": "foo"
+        }
+      }
+    },{
+      "commit": {
+        "committedDate": "2017-10-28T06:40:50Z",
+        "oid": "7dsjdw8olmxvfdh820jxnska0o28wjnjdsw9jdazxz",
+        "message": "feat: add something",
+        "author": {
+          "login": "bar"
+        }
+      }
+    },{
+      "commit": {
+        "committedDate": "2017-10-23T08:02:30Z",
+        "oid": "3edy729kzmdh74shd937dfnalxmdj38kjdnmaj87l9",
+        "message": "style: format code",
+        "author": {
+          "login": "Quo"
+        }
+      }
+    }],
+    "comment": [{
+      "bodyText": "CI: https://ci.nodejs.org/job/node-test-pull-request/10984/",
+      "publishedAt": "2017-10-24T11:19:25Z"
+    }]
+  }
+  
\ No newline at end of file
diff --git a/test/fixtures/data.js b/test/fixtures/data.js
index 6bc6f23a..5c1804ff 100644
--- a/test/fixtures/data.js
+++ b/test/fixtures/data.js
@@ -39,6 +39,7 @@ const multipleCommitsAfterReview = {
   commits: readJSON('multiple_commits_after_review_commits.json'),
   reviews: readJSON('multiple_commits_after_review_reviews.json')
 };
+const commitsAfterCi = readJSON('commits_after_ci.json');
 
 collabArr.forEach((c) => {
   Object.setPrototypeOf(c, Collaborator.prototype);
@@ -70,6 +71,7 @@ module.exports = {
   simpleCommits,
   singleCommitAfterReview,
   multipleCommitsAfterReview,
+  commitsAfterCi,
   collaborators,
   firstTimerPR,
   semverMajorPR,
diff --git a/test/unit/pr_checker.test.js b/test/unit/pr_checker.test.js
index ea2139e8..85766d9f 100644
--- a/test/unit/pr_checker.test.js
+++ b/test/unit/pr_checker.test.js
@@ -18,6 +18,7 @@ const {
   multipleCommitsAfterReview,
   oddCommits,
   simpleCommits,
+  commitsAfterCi,
   collaborators,
   firstTimerPR,
   semverMajorPR
@@ -265,7 +266,7 @@ describe('PRChecker', () => {
         reviewers: allGreenReviewers,
         comments: commentsWithCI,
         reviews: approvingReviews,
-        commits: simpleCommits,
+        commits: [],
         collaborators
       });
 
@@ -273,6 +274,38 @@ describe('PRChecker', () => {
       assert(status);
       assert.deepStrictEqual(logger.logs, expectedLogs);
     });
+
+    it('should check commits after last ci', () => {
+      const logger = new TestLogger();
+      const {commits, comment} = commitsAfterCi;
+
+      const expectedLogs = {
+        warn: [
+          ['Commits pushed after the last Full CI run:'],
+          ['- fixup: adjust spelling'],
+          ['- doc: add api description README'],
+          ['- feat: add something']
+        ],
+        info: [
+          ['Last Full CI on 2017-10-24T11:19:25Z: https://ci.nodejs.org/job/node-test-pull-request/10984/']
+        ],
+        error: [],
+        trace: []
+      };
+
+      const checker = new PRChecker(logger, {
+        pr: firstTimerPR,
+        reviewers: allGreenReviewers,
+        comments: comment,
+        reviews: approvingReviews,
+        commits: commits,
+        collaborators
+      });
+
+      const status = checker.checkCI();
+      assert(!status);
+      assert.deepStrictEqual(logger.logs, expectedLogs);
+    });
   });
 
   describe('authorIsNew/checkAuthor', () => {