ComputerSurge/shields
3
0
Fork 0
Code Pull Requests 1 Activity

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

Browse Source 
The intended behavior of the bracketed [github], [bower], [discord] service names in the pull request title is to trigger the designated service tests. That way, affected services can be proven working during code review, without needing to run tests on a dev machine, nor running all the slow (and flaky) service tests.

Example pull request titles:

- [Travis] Fix timeout issues
- [Travis Sonar] Support user token authentication
- [CRAN CPAN CTAN] Add test coverage

The observed behavior is that, whenever bracketed service names are provided, all of the service tests run.

This is due to a Mocha limitation, which is that exclusive tests (it.only and describe.only) can only be applied synchronously. In other words, if we try to fetch the PR title and then add exclusive tests in the callback, all the tests will run anyway. This is true even when using _mocha --delay, as we are, and is true whether I use request or node-fetch.

Undoubtedly this could be fixed, though it's not worth it. The problem is obscure and therefore low priority for Mocha, which is quite backlogged.

And, there is an easy workaround, which is to generate the list of services to test in a separate process.

The pull request script test:services:pr is now 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.

In addition to sidestepping the Mocha bug, this setup makes it easier to reason about and debug these two steps of the test runner on a dev machine, and since I can't get pipefail to work – and want to be able to run the steps separately – I'm not using Node's built in pre scripts.

Overall, separating these concerns this makes the test runner easier to reason about.
This commit is contained in:
Paul Melnikow
2017-10-01 20:03:29 -04:00
committed by GitHub
parent 5d68488426
commit 0068f31af2
4 changed files with 145 additions and 79 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
.travis.yml
View File

@@ -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:

4
package.json
View File

@@ -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": {

139
service-tests/runner/cli.js
View File

@@ -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(','));
}
runner.toss();
// Invoke run() asynchronously, because Mocha will not start otherwise.
process.nextTick(run);
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(',');
}
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);

79
service-tests/runner/pull-request-services-cli.js Normal file
View File

@@ -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);
});