feat(land): add skipChecks option

skipChecks will allow skipping certain checks. Useful to skip "commit
after last approval" on commit queue.

Ref: nodejs/node#34770

Author: mmarchini

components/git/land.js

@@ -4,11 +4,14 @@ const { parsePRFromURL } = require('../../lib/links');
 const getMetadata = require('../metadata');
 const CLI = require('../../lib/cli');
 const Request = require('../../lib/request');
+const PRChecker = require('../../lib/pr_checker');
 const { runPromise } = require('../../lib/run');
 const LandingSession = require('../../lib/landing_session');
 const epilogue = require('./epilogue');
 const yargs = require('yargs');
 
+const availableChecks = Object.values(PRChecker.availableChecks);
+
 const landOptions = {
   apply: {
     describe: 'Apply a patch with the given PR id',
@@ -42,6 +45,21 @@ const landOptions = {
     describe: 'Land a backport PR onto a staging branch',
     default: false,
     type: 'boolean'
+  },
+  skipChecks: {
+    describe: 'Comma-separated list of checks to skip while landing. ' +
+    `Available options are: ${availableChecks}`,
+    coerce: (arg) => {
+      const checks = arg.split(',');
+      for (const check of checks) {
+        if (!availableChecks.includes(check)) {
+          throw new Error(`Invalid check '${check}'. Available options are: ` +
+            `${availableChecks}`);
+        }
+      }
+      return checks;
+    },
+    type: 'string'
   }
 };
 
@@ -89,7 +107,10 @@ function handler(argv) {
 
   const provided = [];
   for (const type of Object.keys(landOptions)) {
-    if (type === 'yes') continue;  // --yes is not an action
+    if (['yes', 'skipChecks'].includes(type)) {
+      // Those are not actions
+      continue;
+    }
     if (argv[type]) {
       provided.push(type);
     }
@@ -160,7 +181,7 @@ async function main(state, argv, cli, req, dir) {
       return;
     }
     session = new LandingSession(cli, req, dir, argv.prid, argv.backport);
-    const metadata = await getMetadata(session.argv, cli);
+    const metadata = await getMetadata(session.argv, cli, argv.skipChecks);
     if (argv.backport) {
       const split = metadata.metadata.split('\n')[0];
       if (split === 'PR-URL: ') {

components/metadata.js

@@ -9,7 +9,7 @@ const MetadataGenerator = require('../lib/metadata_gen');
 
 const fs = require('fs');
 
-module.exports = async function getMetadata(argv, cli) {
+module.exports = async function getMetadata(argv, cli, skipChecks = []) {
   const credentials = await auth({
     github: true,
     jenkins: true
@@ -40,7 +40,7 @@ module.exports = async function getMetadata(argv, cli) {
   cli.separator();
 
   const checker = new PRChecker(cli, data, request, argv);
-  const status = await checker.checkAll(argv.checkComments);
+  const status = await checker.checkAll(argv.checkComments, skipChecks);
   return {
     status,
     request,

lib/pr_checker.js

@@ -28,6 +28,16 @@ const { PRBuild } = require('./ci/build-types/pr_build');
 
 const GIT_CONFIG_GUIDE_URL = 'https://github.com/nodejs/node/blob/99b1ada/doc/guides/contributing/pull-requests.md#step-1-fork';
 
+const CHECKS = {
+  COMMIT_AFTER_REVIEW: 'commit-after-review',
+  CI: 'ci',
+  REVIEWS_AND_WAIT: 'reviews-and-wait',
+  MERGEABLE_STATE: 'mergeable-state',
+  PR_STATE: 'pr-state',
+  GIT_CONFIG: 'git-config',
+  AUTHOR: 'author'
+};
+
 class PRChecker {
   /**
    * @param {{}} cli
@@ -55,6 +65,10 @@ class PRChecker {
     );
   }
 
+  static get availableChecks() {
+    return CHECKS;
+  }
+
   get waitTimeSingleApproval() {
     if (this.argv.waitTimeSingleApproval === undefined) {
       return WAIT_TIME_SINGLE_APPROVAL;
@@ -69,24 +83,33 @@ class PRChecker {
     return this.argv.waitTimeMultiApproval;
   }
 
-  async checkAll(checkComments = false) {
-    const status = [
-      this.checkCommitsAfterReview(),
-      await this.checkCI(),
-      this.checkReviewsAndWait(new Date(), checkComments),
-      this.checkMergeableState(),
-      this.checkPRState(),
-      this.checkGitConfig()
+  async checkAll(checkComments = false, exclude = []) {
+    const checks = [
+      [CHECKS.COMMIT_AFTER_REVIEW, async() => this.checkCommitsAfterReview()],
+      [CHECKS.CI, async() => this.checkCI()],
+      [CHECKS.REVIEWS_AND_WAIT,
+        async() => this.checkReviewsAndWait(new Date(), checkComments)],
+      [CHECKS.MERGEABLE_STATE, async() => this.checkMergeableState()],
+      [CHECKS.PR_STATE, async() => this.checkPRState()],
+      [CHECKS.GIT_CONFIG, async() => this.checkGitConfig()]
     ];
 
     if (this.data.authorIsNew()) {
-      status.push(this.checkAuthor());
+      checks.push([CHECKS.AUTHOR, async() => this.checkAuthor()]);
+    }
+
+    const status = [];
+    for (const [checkName, check] of checks) {
+      if (exclude.includes(checkName)) {
+        continue;
+      }
+      status.push(await check());
     }
 
     // TODO: check for pre-backport, Github API v4
     // does not support reading files changed
 
-    return status.every((i) => i);
+    return status.every(s => s);
   }
 
   getTSC(people) {

test/unit/pr_checker.test.js

@@ -59,6 +59,7 @@ describe('PRChecker', () => {
         return PRData.prototype.getThread.call(this);
       }
     };
+    const checks = PRChecker.availableChecks;
     const checker = new PRChecker(cli, data, {}, argv);
 
     let checkReviewsAndWaitStub;
@@ -69,18 +70,20 @@ describe('PRChecker', () => {
     let checkPRState;
     let checkGitConfigStub;
 
-    before(() => {
-      checkReviewsAndWaitStub = sinon.stub(checker, 'checkReviewsAndWait');
-      checkCIStub = sinon.stub(checker, 'checkCI');
-      checkAuthorStub = sinon.stub(checker, 'checkAuthor');
+    beforeEach(() => {
+      checkReviewsAndWaitStub =
+        sinon.stub(checker, 'checkReviewsAndWait').returns(true);
+      checkCIStub = sinon.stub(checker, 'checkCI').returns(true);
+      checkAuthorStub = sinon.stub(checker, 'checkAuthor').returns(true);
       checkCommitsAfterReviewStub =
-        sinon.stub(checker, 'checkCommitsAfterReview');
-      checkMergeableStateStub = sinon.stub(checker, 'checkMergeableState');
-      checkPRState = sinon.stub(checker, 'checkPRState');
-      checkGitConfigStub = sinon.stub(checker, 'checkGitConfig');
+        sinon.stub(checker, 'checkCommitsAfterReview').returns(true);
+      checkMergeableStateStub =
+        sinon.stub(checker, 'checkMergeableState').returns(true);
+      checkPRState = sinon.stub(checker, 'checkPRState').returns(true);
+      checkGitConfigStub = sinon.stub(checker, 'checkGitConfig').returns(true);
     });
 
-    after(() => {
+    afterEach(() => {
       checkReviewsAndWaitStub.restore();
       checkCIStub.restore();
       checkAuthorStub.restore();
@@ -92,7 +95,7 @@ describe('PRChecker', () => {
 
     it('should run necessary checks', async() => {
       const status = await checker.checkAll();
-      assert.strictEqual(status, false);
+      assert.strictEqual(status, true);
       assert.strictEqual(checkReviewsAndWaitStub.calledOnce, true);
       assert.strictEqual(checkCIStub.calledOnce, true);
       assert.strictEqual(checkAuthorStub.calledOnce, true);
@@ -101,6 +104,31 @@ describe('PRChecker', () => {
       assert.strictEqual(checkPRState.calledOnce, true);
       assert.strictEqual(checkGitConfigStub.calledOnce, true);
     });
+
+    it('should run no checks if all excluded', async() => {
+      const status = await checker.checkAll(false, Object.values(checks));
+      assert.strictEqual(status, true);
+      assert.strictEqual(checkReviewsAndWaitStub.called, false);
+      assert.strictEqual(checkCIStub.called, false);
+      assert.strictEqual(checkAuthorStub.called, false);
+      assert.strictEqual(checkCommitsAfterReviewStub.called, false);
+      assert.strictEqual(checkMergeableStateStub.called, false);
+      assert.strictEqual(checkPRState.called, false);
+      assert.strictEqual(checkGitConfigStub.called, false);
+    });
+
+    it('should skip excluded checks', async() => {
+      const status =
+        await checker.checkAll(false, [checks.COMMIT_AFTER_REVIEW, checks.CI]);
+      assert.strictEqual(status, true);
+      assert.strictEqual(checkReviewsAndWaitStub.calledOnce, true);
+      assert.strictEqual(checkCIStub.called, false);
+      assert.strictEqual(checkAuthorStub.calledOnce, true);
+      assert.strictEqual(checkCommitsAfterReviewStub.called, false);
+      assert.strictEqual(checkMergeableStateStub.calledOnce, true);
+      assert.strictEqual(checkPRState.calledOnce, true);
+      assert.strictEqual(checkGitConfigStub.calledOnce, true);
+    });
   });
 
   describe('checkReviewsAndWait', () => {