In PR's, exclusively run the designated service tests e.g. [cran discord]

.travis.yml

@@ -9,7 +9,7 @@ script:
   - npm run lint
   - npm run test:js
   - if [ "$TRAVIS_EVENT_TYPE" == cron ]; then npm run test:services; fi
-  - if [ "$TRAVIS_EVENT_TYPE" == pull_request ]; then npm run test:services -- --pr; fi
+  - if [ "$TRAVIS_EVENT_TYPE" == pull_request ]; then npm run test:services:pr; fi
 
 branches:
   except:

package.json

@@ -45,6 +45,9 @@
     "lint": "eslint '**/*.js'",
     "test:js": "mocha lib '*.spec.js'",
     "test:services": "mocha --delay service-tests/runner/cli.js",
+    "test:services:pr:prepare": "node service-tests/runner/pull-request-services-cli.js > pull-request-services.log",
+    "test:services:pr:run": "mocha --delay service-tests/runner/cli.js --stdin < pull-request-services.log",
+    "test:services:pr": "npm run test:services:pr:prepare && npm run test:services:pr:run",
     "test": "npm run lint && npm run test:js"
   },
   "bin": {
@@ -76,6 +79,7 @@
     "nock": "^9.0.13",
     "node-fetch": "^1.6.3",
     "opn-cli": "^3.1.0",
+    "read-all-stdin-sync": "^1.0.5",
     "sinon": "^2.1.0"
   },
   "engines": {

service-tests/runner/cli.js

@@ -6,57 +6,50 @@
 // Run some services:
 //   npm run test:services -- --only=service1,service2,service3
 //
-// Infer the current PR from the Travis environment, and look for bracketed,
-// space-separated service names in the pull request title. If none are found,
-// do not run any tests. For example:
-// Pull request title: [travis sonar] Support user token authentication
-//   npm run test:services -- --pr
-// is equivalent to
-//   npm run test:services -- --only=travis,sonar
+// Alternatively, pass a newline-separated list of services to stdin.
+//   echo "service1\nservice2\nservice3" | npm run test:services -- --stdin
+//
+// Service tests are run in CI in two cases: scheduled builds and pull
+// requests. The scheduled builds run _all_ the service tests, whereas the
+// pull requests run service tests designated in the PR title. In this way,
+// affected services can be proven working during code review without needing
+// to run all the slow (and likely flaky) service tests.
+//
+// Example pull request titles:
+//
+// - [Travis] Fix timeout issues
+// - [Travis Sonar] Support user token authentication
+// - [CRAN CPAN CTAN] Add test coverage
+//
+// The pull request script test:services:pr is split into two parts. First the
+// :prepare script infers the pull request context, fetches the PR title, and
+// writes the list of affected services to a file. Then the :run script reads
+// the list of affected services and runs the appropriate tests.
+//
+// There are three reasons to separate these two steps into separate processes
+// and build stages:
+//
+// 1. Generating the list of services to test is necessarily asynchronous, and
+//    in Mocha, exclusive tests (`it.only` and `describe.only`) can only be
+//    applied synchronously. In other words, if you try to add exclusive tests
+//    in an asynchronous callback, all the tests will run. This is true even
+//    when using `_mocha --delay`, as we are. Undoubtedly this could be fixed,
+//    though it's not worth it. The problem is obscure and therefore low
+//    for Mocha, which is quite backlogged. There is an easy workaround, which
+//    is to generate the list of services to test in a separate process.
+// 2. Executing these two steps of the test runner separately makes the process
+//    easier to reason about and much easier to debug on a dev machine.
+// 3. Getting "pipefail" to work cross platform with an npm script seems tricky.
+//    Relying on npm scripts is safer. Using "pre" makes it impossible to run
+//    the second step without the first.
 
 'use strict';
 
-const difference = require('lodash.difference');
-const fetch = require('node-fetch');
 const minimist = require('minimist');
+const readAllStdinSync = require('read-all-stdin-sync');
 const Runner = require('./runner');
 const serverHelpers = require('../../lib/in-process-server-test-helpers');
 
-function makeBasicAuthHeader(username, password) {
-  return 'Basic ' + new Buffer(`${username}:${password}`).toString('base64');
-}
-
-function getTitle (repoSlug, pullRequest) {
-  const uri = `https://api.github.com/repos/${repoSlug}/pulls/${pullRequest}`;
-  const options = { headers: { 'User-Agent': 'badges/shields' }};
-  if (process.env.GITHUB_TOKEN) {
-    options.headers.Authorization = makeBasicAuthHeader('', process.env.GITHUB_TOKEN);
-  }
-  return fetch(uri, options)
-    .then(res => {
-      if (! res.ok) {
-        throw Error(`${res.status} ${res.statusText}`);
-      }
-
-      return res.json();
-    })
-    .then(json => json.title);
-}
-
-// [Travis] Fix timeout issues => ['travis']
-// [Travis Sonar] Support user token authentication -> ['travis', 'sonar']
-// [CRAN CPAN CTAN] Add test coverage => ['cran', 'cpan', 'ctan']
-function servicesForTitle (title) {
-  const matches = title.match(/\[(.+)\]/);
-  if (matches === null) {
-    return [];
-  }
-
-  const services = matches[1].toLowerCase().split(' ');
-  const blacklist = ['wip'];
-  return difference(services, blacklist);
-}
-
 let server;
 before('Start running the server', function () {
   this.timeout(5000);
@@ -70,40 +63,30 @@ runner.prepare();
 runner.beforeEach = () => { serverHelpers.reset(server); };
 
 const args = minimist(process.argv.slice(3));
-const prOption = args.pr;
-const serviceOption = args.only;
+const stdinOption = args.stdin;
+const onlyOption = args.only;
 
-if (prOption !== undefined) {
-  const repoSlug = process.env.TRAVIS_REPO_SLUG;
-  const pullRequest = process.env.TRAVIS_PULL_REQUEST;
-  if (repoSlug === undefined || pullRequest === undefined) {
-    console.error('Please set TRAVIS_REPO_SLUG and TRAVIS_PULL_REQUEST.');
-    process.exit(-1);
-  }
-  console.info(`PR: ${repoSlug}#${pullRequest}`);
+let onlyServices;
 
-  getTitle(repoSlug, pullRequest)
-    .then(title => {
-      console.info(`Title: ${title}`);
-      const services = servicesForTitle(title);
-      if (services.length === 0) {
-        console.info('No services found. Nothing to do.');
-      } else {
-        console.info(`Services: (${services.length} found) ${services.join(', ')}\n`);
-        runner.only(services);
-        runner.toss();
-        run();
-      }
-    }).catch(err => {
-      console.error(err);
-      process.exit(1);
-    });
-} else {
-  if (serviceOption !== undefined) {
-    runner.only(serviceOption.split(','));
-  }
+if (stdinOption && onlyOption) {
+  console.error('Do not use --only with --stdin');
+} else if (stdinOption) {
+  const allStdin = readAllStdinSync().trim();
+  onlyServices = allStdin ? allStdin.split('\n') : [];
+} else if (onlyOption) {
+  onlyServices = onlyOption.split(',');
+}
 
-  runner.toss();
-  // Invoke run() asynchronously, because Mocha will not start otherwise.
-  process.nextTick(run);
+if (typeof onlyServices === 'undefined') {
+  console.info('Running all service tests.');
+} else if (onlyServices.length === 0) {
+  console.info('No service tests to run. Exiting.');
+  process.exit(0);
+} else {
+  console.info(`Running tests for ${onlyServices.length} services: ${onlyServices.join(', ')}.\n`);
+  runner.only(onlyServices);
 }
+
+runner.toss();
+// Invoke run() asynchronously, because Mocha will not start otherwise.
+process.nextTick(run);

service-tests/runner/pull-request-services-cli.js

@@ -0,0 +1,79 @@
+// Infer the current PR from the Travis environment, and look for bracketed,
+// space-separated service names in the pull request title.
+//
+// Output the list of services.
+//
+// Pull request title: [travis sonar] Support user token authentication
+//
+// Output:
+// travis
+// sonar
+//
+// Example:
+//   TRAVIS_REPO_SLUG=badges/shields TRAVIS_PULL_REQUEST=1108 npm run test:pr:services:prepare
+// With authentication:
+//   GITHUB_TOKEN=... TRAVIS_REPO_SLUG=badges/shields TRAVIS_PULL_REQUEST=1108 npm run test:pr:services:prepare
+
+'use strict';
+
+const difference = require('lodash.difference');
+const fetch = require('node-fetch');
+
+function makeBasicAuthHeader(username, password) {
+  return 'Basic ' + new Buffer(`${username}:${password}`).toString('base64');
+}
+
+function getTitle (repoSlug, pullRequest) {
+  const uri = `https://api.github.com/repos/${repoSlug}/pulls/${pullRequest}`;
+  const options = { headers: { 'User-Agent': 'badges/shields' }};
+  if (process.env.GITHUB_TOKEN) {
+    console.error('Authenticating with token.');
+    options.headers.Authorization = makeBasicAuthHeader('', process.env.GITHUB_TOKEN);
+  }
+  return fetch(uri, options)
+    .then(res => {
+      if (! res.ok) {
+        throw Error(`${res.status} ${res.statusText}`);
+      }
+
+      return res.json();
+    })
+    .then(json => json.title);
+}
+
+// [Travis] Fix timeout issues => ['travis']
+// [Travis Sonar] Support user token authentication -> ['travis', 'sonar']
+// [CRAN CPAN CTAN] Add test coverage => ['cran', 'cpan', 'ctan']
+function servicesForTitle (title) {
+  const matches = title.match(/\[(.+)\]/);
+  if (matches === null) {
+    return [];
+  }
+
+  const services = matches[1].toLowerCase().split(' ');
+  const blacklist = ['wip'];
+  return difference(services, blacklist);
+}
+
+const repoSlug = process.env.TRAVIS_REPO_SLUG;
+const pullRequest = process.env.TRAVIS_PULL_REQUEST;
+if (repoSlug === undefined || pullRequest === undefined) {
+  console.error('Please set TRAVIS_REPO_SLUG and TRAVIS_PULL_REQUEST.');
+  process.exit(-1);
+}
+console.error(`PR: ${repoSlug}#${pullRequest}`);
+
+getTitle(repoSlug, pullRequest)
+  .then(title => {
+    console.error(`Title: ${title}`);
+    const services = servicesForTitle(title);
+    if (services.length === 0) {
+      console.error('No services found. Nothing to do.');
+    } else {
+      console.error(`Services: (${services.length} found) ${services.join(', ')}\n`);
+      console.log(services.join('\n'));
+    }
+  }).catch(err => {
+    console.error(err);
+    process.exit(1);
+  });