feat: check large pull request approvals in git node land

lib/pr_checker.js

@@ -26,6 +26,10 @@ const WAIT_TIME_SINGLE_APPROVAL = 24 * 7;
 const GITHUB_SUCCESS_CONCLUSIONS = ['SUCCESS', 'NEUTRAL', 'SKIPPED'];
 const GITHUB_ACTIONS_APP = 'github-actions';
 
+// Both semver-major and large pull requests take this approval path.
+// https://github.com/nodejs/node/blob/main/doc/contributing/large-pull-requests.md
+const REQUIRED_TSC_APPROVALS = 2;
+
 const FAST_TRACK_RE = /^Fast-track has been requested by @(.+?)\. Please 👍 to approve\.$/;
 const FAST_TRACK_MIN_APPROVALS = 2;
 const GIT_CONFIG_GUIDE_URL = 'https://github.com/nodejs/node/blob/99b1ada/doc/guides/contributing/pull-requests.md#step-1-fork';
@@ -43,6 +47,7 @@ export const PR_CHECK_REASON_CODES = Object.freeze({
   MISSING_FULL_JENKINS_CI: 'missing-full-jenkins-ci',
   MISSING_GITHUB_CI: 'missing-github-ci',
   MISSING_JENKINS_CI: 'missing-jenkins-ci',
+  MISSING_LARGE_PR_TSC_APPROVAL: 'missing-large-pr-tsc-approval',
   MISSING_TSC_APPROVAL: 'missing-tsc-approval',
   NEW_CONTRIBUTOR: 'new-contributor',
   NO_COMMITS: 'no-commits',
@@ -196,6 +201,10 @@ export default class PRChecker {
 
     let isFastTracked = labels.includes('fast-track');
     const isSemverMajor = labels.includes('semver-major');
+    // Large pull requests follow the same approval path as semver-major
+    // changes. Whether a pull request is large is a judgement the project
+    // records with a label, rather than something derived from the diff.
+    const isLargePR = labels.includes('large-pr');
     // NOTE: a semver-major PR with fast-track should have either one of
     // these labels removed because that doesn't make sense
     if (isFastTracked) {
@@ -206,16 +215,26 @@ export default class PRChecker {
       return false;
     }
 
-    if (isSemverMajor) {
+    if (isSemverMajor || isLargePR) {
       const tscApproved = approved
         .filter((p) => p.reviewer.isTSC())
         .map((p) => p.reviewer.login);
-      if (tscApproved.length < 2) {
-        const message = 'semver-major requires at least 2 TSC approvals';
+      if (tscApproved.length < REQUIRED_TSC_APPROVALS) {
+        // A pull request can carry both labels; report it as semver-major,
+        // since that is the stronger statement about the change.
+        const code = isSemverMajor
+          ? PR_CHECK_REASON_CODES.MISSING_TSC_APPROVAL
+          : PR_CHECK_REASON_CODES.MISSING_LARGE_PR_TSC_APPROVAL;
+        const subject = isSemverMajor
+          ? 'semver-major requires'
+          : 'large pull requests require';
+        const message =
+          `${subject} at least ${REQUIRED_TSC_APPROVALS} ` +
+          'TSC approvals';
         cli.error(message);
-        this.addReason(PR_CHECK_REASON_CODES.MISSING_TSC_APPROVAL, message, {
+        this.addReason(code, message, {
           approvals: tscApproved.length,
-          required: 2
+          required: REQUIRED_TSC_APPROVALS
         });
         return false; // 7 day rule doesn't matter here
       }

test/fixtures/data.js

@@ -86,6 +86,7 @@ export const firstTimerPR = readJSON('first_timer_pr.json');
 export const firstTimerPrivatePR =
   readJSON('first_timer_pr_with_private_email.json');
 export const semverMajorPR = readJSON('semver_major_pr.json');
+export const largePR = readJSON('large_pr.json');
 export const fixAndRefPR = readJSON('pr_with_fixes_and_refs.json');
 export const fixCrossPR = readJSON('pr_with_fixes_cross.json');
 export const duplicateRefPR = readJSON('pr_with_duplicate_refs.json');

test/fixtures/large_pr.json

@@ -0,0 +1,22 @@
+{
+  "createdAt": "2017-10-24T11:13:43Z",
+  "authorAssociation": "COLLABORATOR",
+  "author": {
+    "login": "pr_author",
+    "email": "pr_author@example.com",
+    "name": "Their Github Account email"
+  },
+  "url": "https://github.com/nodejs/node/pull/16438",
+  "bodyHTML": "<p>Awesome changes</p>",
+  "bodyText": "Awesome changes",
+  "labels": {
+    "nodes": [
+      {
+        "name": "large-pr"
+      }
+    ]
+  },
+  "title": "lib: awesome changes",
+  "baseRefName": "main",
+  "headRefName": "awesome-changes"
+}

test/unit/pr_checker.test.js

@@ -40,6 +40,7 @@ import {
   firstTimerPR,
   firstTimerPrivatePR,
   semverMajorPR,
+  largePR,
   conflictingPR,
   closedPR,
   mergedPR,
@@ -61,6 +62,7 @@ const {
   MISSING_APPROVAL,
   MISSING_GITHUB_CI,
   MISSING_JENKINS_CI,
+  MISSING_LARGE_PR_TSC_APPROVAL,
   MISSING_TSC_APPROVAL,
   REQUESTED_CHANGES,
   STALE_REVIEW,
@@ -176,6 +178,55 @@ describe('PRChecker', () => {
       cli.assertCalledWith(expectedLogs);
     });
 
+    it('should error when large PR has only 1 TSC approval', () => {
+      const cli = new TestCLI();
+
+      const expectedLogs = {
+        error: [
+          ['large pull requests require at least 2 TSC approvals']
+        ],
+        ok: [
+          ['Approvals: 4'],
+          ['- Foo User (@foo): https://github.com/nodejs/node/pull/16438#pullrequestreview-71480624'],
+          ['- Quux User (@Quux): LGTM'],
+          ['- Baz User (@Baz): https://github.com/nodejs/node/pull/16438#pullrequestreview-71488236'],
+          ['- Bar User (@bar) (TSC): lgtm']
+        ],
+        info: [
+          ['This PR was created on Fri, 23 Nov 2018 17:50:44 GMT'],
+          ['- Quux User (@Quux) approved in via LGTM in comments'],
+          ['- Bar User (@bar) approved in via LGTM in comments']
+        ]
+      };
+      const pr = Object.assign({}, largePR, {
+        createdAt: GT_7D
+      });
+
+      const data = {
+        pr,
+        reviewers: allGreenReviewers,
+        comments: commentsWithLGTM,
+        reviews: approvingReviews,
+        commits: simpleCommits,
+        collaborators,
+        authorIsNew: () => false,
+        getThread() {
+          return PRData.prototype.getThread.call(this);
+        }
+      };
+      const checker = new PRChecker(cli, data, {}, argv);
+
+      const status = checker.checkReviewsAndWait(new Date(NOW), true);
+      assert(!status);
+      assert.deepStrictEqual(checker.reasons, [{
+        code: MISSING_LARGE_PR_TSC_APPROVAL,
+        message: 'large pull requests require at least 2 TSC approvals',
+        approvals: 1,
+        required: 2
+      }]);
+      cli.assertCalledWith(expectedLogs);
+    });
+
     it('should error when PR has change requests', () => {
       const cli = new TestCLI();