pr_checker: ignore private emails

lib/pr_checker.js

@@ -247,8 +247,8 @@ class PRChecker {
       return true;
     }
 
-    const prAuthor = pr.author.login;
-    cli.warn(`PR author is: @${prAuthor}`);
+    const prAuthor = `${pr.author.login}(${pr.author.email})`;
+    cli.warn(`PR author is a new contributor: @${prAuthor}`);
     for (const c of oddCommits) {
       const { oid, author } = c.commit;
       const hash = oid.slice(0, 7);
@@ -263,6 +263,13 @@ class PRChecker {
 
   isOddAuthor(commit) {
     const { pr, collaboratorEmails } = this;
+
+    // They have turned on the private email feature, can't really check
+    // anything, GitHub should know how to link that, see nodejs/node#15489
+    if (!pr.author.email) {
+      return false;
+    }
+
     // If they have added the alternative email to their account,
     // commit.authoredByCommitter should be set to true by Github
     if (commit.authoredByCommitter) {

test/fixtures/data.js

@@ -55,6 +55,7 @@ const collaborators = new Map(
 );
 
 const firstTimerPR = readJSON('first_timer_pr.json');
+const firstTimerPrivatePR = readJSON('first_timer_pr_with_private_email.json');
 const semverMajorPR = readJSON('semver_major_pr.json');
 const fixAndRefPR = readJSON('pr_with_fixes_and_refs.json');
 const conflictingPR = readJSON('conflicting_pr.json');
@@ -83,6 +84,7 @@ module.exports = {
   mulipleCommitsAfterCi,
   collaborators,
   firstTimerPR,
+  firstTimerPrivatePR,
   semverMajorPR,
   fixAndRefPR,
   conflictingPR,

test/fixtures/first_timer_pr_with_private_email.json

@@ -0,0 +1,24 @@
+{
+  "createdAt": "2017-10-24T11:13:43Z",
+  "authorAssociation": "FIRST_TIMER",
+  "author": {
+    "login": "pr_author",
+    "email": ""
+  },
+  "url": "https://github.com/nodejs/node/pull/16438",
+  "bodyHTML": "<p>Awesome changes</p>",
+  "bodyText": "Awesome changes",
+  "labels": {
+    "nodes": [
+      {
+        "name": "test"
+      },
+      {
+        "name": "doc"
+      }
+    ]
+  },
+  "title": "test: awesome changes",
+  "baseRefName": "master",
+  "headRefName": "awesome-changes"
+}

test/unit/pr_checker.test.js

@@ -23,6 +23,7 @@ const {
   mulipleCommitsAfterCi,
   collaborators,
   firstTimerPR,
+  firstTimerPrivatePR,
   semverMajorPR,
   conflictingPR
 } = require('../fixtures/data');
@@ -415,7 +416,7 @@ describe('PRChecker', () => {
 
       const expectedLogs = {
         warn: [
-          ['PR author is: @pr_author'],
+          ['PR author is a new contributor: @pr_author(pr_author@example.com)'],
           ['- commit e3ad7c7 is authored by test@example.com'],
           ['- commit da39a3e is authored by test@example.com']
         ]
@@ -436,6 +437,28 @@ describe('PRChecker', () => {
       assert(!status);
       cli.assertCalledWith(expectedLogs);
     });
+
+    it('should skip checking odd commits for first timers ' +
+      'with private emails', () => {
+      const cli = new TestCLI();
+
+      const expectedLogs = {};
+
+      const options = {
+        pr: firstTimerPrivatePR,
+        reviewers: allGreenReviewers,
+        comments: commentsWithLGTM,
+        reviews: approvingReviews,
+        commits: oddCommits,
+        collaborators
+      };
+      const checker = new PRChecker(cli, options, argv);
+
+      assert(checker.authorIsNew());
+      const status = checker.checkAuthor();
+      assert(status);
+      cli.assertCalledWith(expectedLogs);
+    });
   });
 
   describe('checkCommitsAfterReview', () => {