feat: check CI failures before land (#422)

johnfrench3 · May 26, 2020 · 4b1e1ab · 4b1e1ab
1 parent 3bae10f
commit 4b1e1ab
Show file tree

Hide file tree

Showing 4 changed files with 177 additions and 130 deletions.
diff --git a/components/metadata.js b/components/metadata.js
@@ -11,7 +11,8 @@ const fs = require('fs');
 
 module.exports = async function getMetadata(argv, cli) {
   const credentials = await auth({
-    github: true
+    github: true,
+    jenkins: true
   });
   const request = new Request(credentials);
 
@@ -38,8 +39,8 @@ module.exports = async function getMetadata(argv, cli) {
   cli.write(metadata);
   cli.separator();
 
-  const checker = new PRChecker(cli, data, argv);
-  const status = checker.checkAll(argv.checkComments);
+  const checker = new PRChecker(cli, data, request, argv);
+  const status = await checker.checkAll(argv.checkComments);
   return {
     status,
     request,

diff --git a/lib/ci/ci_type_parser.js b/lib/ci/ci_type_parser.js
@@ -26,6 +26,11 @@ const CI_TYPE_ENUM = {
   JOB_CI: 1 << 2
 };
 
+const CI_PROVIDERS = {
+  JENKINS: 'jenkins',
+  GITHUB: 'github-check'
+};
+
 const { JOB_CI, FULL_CI, LITE_CI } = CI_TYPE_ENUM;
 
 const CI_TYPES = new Map([
@@ -209,6 +214,7 @@ module.exports = {
     CITGM, PR, COMMIT, BENCHMARK, LIBUV, V8, NOINTL,
     LINTER, LITE_PR, LITE_COMMIT
   },
+  CI_PROVIDERS,
   isFullCI,
   isLiteCI,
   JobParser,

diff --git a/lib/pr_checker.js b/lib/pr_checker.js
@@ -20,9 +20,11 @@ const {
 const {
   JobParser,
   CI_TYPES,
+  CI_PROVIDERS,
   isLiteCI,
   isFullCI
 } = require('./ci/ci_type_parser');
+const { PRBuild } = require('./ci/ci_result_parser');
 
 const GIT_CONFIG_GUIDE_URL = 'https://github.com/nodejs/node/blob/99b1ada/doc/guides/contributing/pull-requests.md#step-1-fork';
 
@@ -31,8 +33,9 @@ class PRChecker {
    * @param {{}} cli
    * @param {PRData} data
    */
-  constructor(cli, data, argv) {
+  constructor(cli, data, request, argv) {
     this.cli = cli;
+    this.request = request;
     this.data = data;
     const {
       pr, reviewers, comments, reviews, commits, collaborators
@@ -66,10 +69,10 @@ class PRChecker {
     return this.argv.waitTimeMultiApproval;
   }
 
-  checkAll(checkComments = false) {
+  async checkAll(checkComments = false) {
     const status = [
       this.checkCommitsAfterReview(),
-      this.checkCI(),
+      await this.checkCI(),
       this.checkReviewsAndWait(new Date(), checkComments),
       this.checkMergeableState(),
       this.checkPRState(),
@@ -201,32 +204,42 @@ class PRChecker {
     return cis.find(ci => isFullCI(ci) || isLiteCI(ci));
   }
 
-  checkCI() {
-    const ciType = this.argv.ciType || 'jenkins';
-    if (ciType === 'jenkins') {
-      return this.checkJenkinsCI();
-    } else if (ciType === 'github-check') {
-      return this.checkGitHubCI();
+  async checkCI() {
+    const ciType = this.argv.ciType || CI_PROVIDERS.JENKINS;
+    const providers = Object.values(CI_PROVIDERS);
+
+    if (!providers.includes(ciType)) {
+      this.cli.error(
+        `Invalid ciType ${ciType} - must be one of ${providers.join(', ')}`);
+      return false;
     }
-    this.cli.error(`Invalid ciType: ${ciType}`);
-    return false;
+
+    let status = false;
+    if (ciType === CI_PROVIDERS.JENKINS) {
+      status = await this.checkJenkinsCI();
+    } else if (ciType === CI_PROVIDERS.GITHUB) {
+      status = this.checkGitHubCI();
+    }
+
+    return status;
   }
 
   // TODO: we might want to check CI status when it's less flaky...
   // TODO: not all PR requires CI...labels?
-  checkJenkinsCI() {
-    const { cli, commits, argv } = this;
+  async checkJenkinsCI() {
+    const { cli, commits, request, argv } = this;
     const { maxCommits } = argv;
     const thread = this.data.getThread();
     const ciMap = new JobParser(thread).parse();
+
     let status = true;
     if (!ciMap.size) {
       cli.error('No CI runs detected');
       this.CIStatus = false;
       return false;
     } else if (!this.hasFullOrLiteCI(ciMap)) {
       status = false;
-      cli.error('No full CI runs or lite CI runs detected');
+      cli.error('No full or lite Jenkins CI runs detected');
     }
 
     let lastCI;
@@ -236,7 +249,8 @@ class PRChecker {
       if (!lastCI || lastCI.date < ci.date) {
         lastCI = {
           typeName: name,
-          date: ci.date
+          date: ci.date,
+          jobId: ci.jobid
         };
       }
     }
@@ -270,6 +284,16 @@ class PRChecker {
           cli.warn(infoMsg);
         }
       }
+
+      // Check the last CI run for its results.
+      const build = new PRBuild(cli, request, lastCI.jobId);
+      const { result, failures } = await build.getResults();
+
+      if (result === 'FAILURE') {
+        cli.error(
+          `${failures.length} failure(s) on the last Jenkins CI run`);
+        status = false;
+      }
     }
 
     this.CIStatus = status;
@@ -298,12 +322,12 @@ class PRChecker {
     // GitHub new Check API
     for (const { status, conclusion } of checkSuites.nodes) {
       if (status !== 'COMPLETED') {
-        cli.error('CI is still running');
+        cli.error('GitHub CI is still running');
         return false;
       }
 
       if (!['SUCCESS', 'NEUTRAL'].includes(conclusion)) {
-        cli.error('Last CI failed');
+        cli.error('Last GitHub CI failed');
         return false;
       }
     }
@@ -312,17 +336,17 @@ class PRChecker {
     if (commit.status) {
       const { state } = commit.status;
       if (state === 'PENDING') {
-        cli.error('CI is still running');
+        cli.error('GitHub CI is still running');
         return false;
       }
 
       if (!['SUCCESS', 'EXPECTED'].includes(state)) {
-        cli.error('Last CI failed');
+        cli.error('Last GitHub CI failed');
         return false;
       }
     }
 
-    cli.info('Last CI run was successful');
+    cli.info('Last GitHub CI successful');
     this.CIStatus = true;
     return true;
   }