Skip to content

Commit

Permalink
Improve failFast at API level
Browse files Browse the repository at this point in the history
Don't run new test files after:

* A timeout has occurred
* A test has failed
* Running the test file itself caused an error

Refs #1158.
  • Loading branch information
novemberborn committed Feb 11, 2018
1 parent 5c8baff commit f83f8c0
Show file tree
Hide file tree
Showing 9 changed files with 183 additions and 4 deletions.
30 changes: 29 additions & 1 deletion api.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,20 @@ class Api extends EventEmitter {
};

// Track active forks and manage timeouts.
const failFast = apiOptions.failFast === true;
let bailed = false;
const pendingForks = new Set();
let restartTimer;
if (apiOptions.timeout) {
const timeout = ms(apiOptions.timeout);

restartTimer = debounce(() => {
// If failFast is active, prevent new test files from running after
// the current ones are exited.
if (failFast) {
bailed = true;
}

for (const fork of pendingForks) {
fork.exit();
}
Expand All @@ -78,11 +87,20 @@ class Api extends EventEmitter {
runOnlyExclusive: runtimeOptions.runOnlyExclusive,
prefixTitles: apiOptions.explicitTitles || files.length > 1,
base: path.relative(process.cwd(), commonPathPrefix(files)) + path.sep,
failFast: apiOptions.failFast,
failFast,
updateSnapshots: runtimeOptions.updateSnapshots
});

runStatus.on('test', restartTimer);
if (failFast) {
// Prevent new test files from running once a test has failed.
runStatus.on('test', test => {
if (test.error) {
bailed = true;
}
});
}

this.emit('test-run', runStatus, files);

// Bail out early if no files were found.
Expand Down Expand Up @@ -130,6 +148,12 @@ class Api extends EventEmitter {

// Try and run each file, limited by `concurrency`.
return Bluebird.map(files, file => {
// No new files should be run once a test has timed out or failed,
// and failFast is enabled.
if (bailed) {
return null;
}

let forked;
return Bluebird.resolve(
this._computeForkExecArgv().then(execArgv => {
Expand Down Expand Up @@ -157,6 +181,10 @@ class Api extends EventEmitter {

return forked;
}).catch(err => {
// Prevent new test files from running.
if (failFast) {
bailed = true;
}
handleError(Object.assign(err, {file}));
return null;
})
Expand Down
119 changes: 117 additions & 2 deletions test/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ test('display filename prefixes for failed test stack traces in subdirs', t => {
});
});

test('fail-fast mode', t => {
test('fail-fast mode - single file', t => {
const api = apiCreator({
failFast: true
});
Expand All @@ -218,7 +218,7 @@ test('fail-fast mode', t => {
});
});

return api.run([path.join(__dirname, 'fixture/fail-fast.js')])
return api.run([path.join(__dirname, 'fixture/fail-fast/single-file/test.js')])
.then(result => {
t.ok(api.options.failFast);
t.strictDeepEqual(tests, [{
Expand All @@ -233,6 +233,121 @@ test('fail-fast mode', t => {
});
});

test('fail-fast mode - multiple files', t => {
const api = apiCreator({
failFast: true,
serial: true
});

const tests = [];

api.on('test-run', runStatus => {
runStatus.on('test', test => {
tests.push({
ok: !test.error,
title: test.title
});
});
});

return api.run([
path.join(__dirname, 'fixture/fail-fast/multiple-files/fails.js'),
path.join(__dirname, 'fixture/fail-fast/multiple-files/passes.js')
])
.then(result => {
t.ok(api.options.failFast);
t.strictDeepEqual(tests, [{
ok: true,
title: `fails ${figures.pointerSmall} first pass`
}, {
ok: false,
title: `fails ${figures.pointerSmall} second fail`
}]);
t.is(result.passCount, 1);
t.is(result.failCount, 1);
});
});

test('fail-fast mode - crash', t => {
const api = apiCreator({
failFast: true,
serial: true
});

const tests = [];
const errors = [];

api.on('test-run', runStatus => {
runStatus.on('test', test => {
tests.push({
ok: !test.error,
title: test.title
});
});
runStatus.on('error', err => {
errors.push(err);
});
});

return api.run([
path.join(__dirname, 'fixture/fail-fast/crash/crashes.js'),
path.join(__dirname, 'fixture/fail-fast/crash/passes.js')
])
.then(result => {
t.ok(api.options.failFast);
t.strictDeepEqual(tests, []);
t.is(errors.length, 1);
t.is(errors[0].name, 'AvaError');
t.is(errors[0].message, `${path.join('test', 'fixture', 'fail-fast', 'crash', 'crashes.js')} exited with a non-zero exit code: 1`);
t.is(result.passCount, 0);
t.is(result.failCount, 0);
});
});

test('fail-fast mode - timeout', t => {
const api = apiCreator({
failFast: true,
serial: true,
timeout: '100ms'
});

const tests = [];
const errors = [];

api.on('test-run', runStatus => {
runStatus.on('test', test => {
tests.push({
ok: !test.error,
title: test.title
});
});
runStatus.on('error', err => {
errors.push(err);
});
});

return api.run([
path.join(__dirname, 'fixture/fail-fast/timeout/fails.js'),
path.join(__dirname, 'fixture/fail-fast/timeout/passes.js')
])
.then(result => {
t.ok(api.options.failFast);
if (tests.length > 0) {
t.is(tests.length, 1);
// FIXME: The fails.js test file should have exited without the pending
// test completing, but that's not always the case.
t.is(tests[0].title, 'fails › slow pass');
t.is(result.passCount, 1);
} else {
t.is(result.passCount, 0);
}
t.is(errors.length, 1);
t.is(errors[0].name, 'AvaError');
t.is(errors[0].message, 'Exited because no new tests completed within the last 100ms of inactivity');
t.is(result.failCount, 0);
});
});

test('serial execution mode', t => {
const api = apiCreator({
serial: true
Expand Down
3 changes: 3 additions & 0 deletions test/fixture/fail-fast/crash/crashes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import '../../../../'; // eslint-disable-line import/no-unassigned-import

process.exit(1); // eslint-disable-line unicorn/no-process-exit
5 changes: 5 additions & 0 deletions test/fixture/fail-fast/crash/passes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import test from '../../../../';

test('first pass', t => {
t.pass();
});
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import test from '../../';
import test from '../../../../';

test('first pass', t => {
t.pass();
Expand Down
5 changes: 5 additions & 0 deletions test/fixture/fail-fast/multiple-files/passes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import test from '../../../../';

test('first pass', t => {
t.pass();
});
13 changes: 13 additions & 0 deletions test/fixture/fail-fast/single-file/test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import test from '../../../../';

test('first pass', t => {
t.pass();
});

test('second fail', t => {
t.fail();
});

test('third pass', t => {
t.pass();
});
5 changes: 5 additions & 0 deletions test/fixture/fail-fast/timeout/fails.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import test from '../../../../';

test.cb('slow pass', t => {
setTimeout(t.end, 1000);
});
5 changes: 5 additions & 0 deletions test/fixture/fail-fast/timeout/passes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import test from '../../../../';

test('first pass', t => {
t.pass();
});

0 comments on commit f83f8c0

Please sign in to comment.