Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

do not eat exceptions thrown asynchronously from passed tests; closes #3226 #3257

Merged
merged 1 commit into from
Mar 5, 2018
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/reporters/base.js
Original file line number Diff line number Diff line change
@@ -302,7 +302,7 @@ function Base (runner) {
failures.push(test);
});

runner.on('end', function () {
runner.once('end', function () {
stats.end = new Date();
stats.duration = stats.end - stats.start;
});
2 changes: 1 addition & 1 deletion lib/reporters/dot.js
Original file line number Diff line number Diff line change
@@ -56,7 +56,7 @@ function Dot (runner) {
process.stdout.write(color('fail', Base.symbols.bang));
});

runner.on('end', function () {
runner.once('end', function () {
console.log();
self.epilogue();
});
2 changes: 1 addition & 1 deletion lib/reporters/json-stream.js
Original file line number Diff line number Diff line change
@@ -39,7 +39,7 @@ function List (runner) {
console.log(JSON.stringify(['fail', test]));
});

runner.on('end', function () {
runner.once('end', function () {
process.stdout.write(JSON.stringify(['end', self.stats]));
});
}
2 changes: 1 addition & 1 deletion lib/reporters/json.js
Original file line number Diff line number Diff line change
@@ -43,7 +43,7 @@ function JSONReporter (runner) {
pending.push(test);
});

runner.on('end', function () {
runner.once('end', function () {
var obj = {
stats: self.stats,
tests: tests.map(clean),
2 changes: 1 addition & 1 deletion lib/reporters/landing.js
Original file line number Diff line number Diff line change
@@ -81,7 +81,7 @@ function Landing (runner) {
stream.write('\u001b[0m');
});

runner.on('end', function () {
runner.once('end', function () {
cursor.show();
console.log();
self.epilogue();
2 changes: 1 addition & 1 deletion lib/reporters/list.js
Original file line number Diff line number Diff line change
@@ -54,7 +54,7 @@ function List (runner) {
console.log(color('fail', ' %d) %s'), ++n, test.fullTitle());
});

runner.on('end', self.epilogue.bind(self));
runner.once('end', self.epilogue.bind(self));
}

/**
2 changes: 1 addition & 1 deletion lib/reporters/markdown.js
Original file line number Diff line number Diff line change
@@ -91,7 +91,7 @@ function Markdown (runner) {
buf += '```\n\n';
});

runner.on('end', function () {
runner.once('end', function () {
process.stdout.write('# TOC\n');
process.stdout.write(generateTOC(runner.suite));
process.stdout.write(buf);
2 changes: 1 addition & 1 deletion lib/reporters/min.js
Original file line number Diff line number Diff line change
@@ -29,7 +29,7 @@ function Min (runner) {
process.stdout.write('\u001b[1;3H');
});

runner.on('end', this.epilogue.bind(this));
runner.once('end', this.epilogue.bind(this));
}

/**
2 changes: 1 addition & 1 deletion lib/reporters/nyan.js
Original file line number Diff line number Diff line change
@@ -52,7 +52,7 @@ function NyanCat (runner) {
self.draw();
});

runner.on('end', function () {
runner.once('end', function () {
Base.cursor.show();
for (var i = 0; i < self.numberOfLines; i++) {
write('\n');
2 changes: 1 addition & 1 deletion lib/reporters/progress.js
Original file line number Diff line number Diff line change
@@ -80,7 +80,7 @@ function Progress (runner, options) {

// tests are complete, output some stats
// and the failures if any
runner.on('end', function () {
runner.once('end', function () {
cursor.show();
console.log();
self.epilogue();
2 changes: 1 addition & 1 deletion lib/reporters/spec.js
Original file line number Diff line number Diff line change
@@ -72,7 +72,7 @@ function Spec (runner) {
console.log(indent() + color('fail', ' %d) %s'), ++n, test.title);
});

runner.on('end', self.epilogue.bind(self));
runner.once('end', self.epilogue.bind(self));
}

/**
2 changes: 1 addition & 1 deletion lib/reporters/tap.js
Original file line number Diff line number Diff line change
@@ -51,7 +51,7 @@ function TAP (runner) {
}
});

runner.on('end', function () {
runner.once('end', function () {
console.log('# tests ' + (passes + failures));
console.log('# pass ' + passes);
console.log('# fail ' + failures);
2 changes: 1 addition & 1 deletion lib/reporters/xunit.js
Original file line number Diff line number Diff line change
@@ -78,7 +78,7 @@ function XUnit (runner, options) {
tests.push(test);
});

runner.on('end', function () {
runner.once('end', function () {
self.write(tag('testsuite', {
name: suiteName,
tests: stats.tests,
18 changes: 18 additions & 0 deletions lib/runnable.js
Original file line number Diff line number Diff line change
@@ -142,6 +142,24 @@ Runnable.prototype.isPending = function () {
return this.pending || (this.parent && this.parent.isPending());
};

/**
* Return `true` if this Runnable has failed.
* @return {boolean}
* @private
*/
Runnable.prototype.isFailed = function () {
return !this.isPending() && this.state === 'failed';
};

/**
* Return `true` if this Runnable has passed.
* @return {boolean}
* @private
*/
Runnable.prototype.isPassed = function () {
return !this.isPending() && this.state === 'passed';
};

/**
* Set or get number of retries.
*
26 changes: 15 additions & 11 deletions lib/runner.js
Original file line number Diff line number Diff line change
@@ -726,21 +726,25 @@ Runner.prototype.uncaught = function (err) {

runnable.clearTimeout();

// Ignore errors if complete or pending
if (runnable.state || runnable.isPending()) {
// Ignore errors if already failed or pending
// See #3226
if (runnable.isFailed() || runnable.isPending()) {
return;
}
// we cannot recover gracefully if a Runnable has already passed
// then fails asynchronously
var alreadyPassed = runnable.isPassed();
// this will change the state to "failed" regardless of the current value
this.fail(runnable, err);
if (!alreadyPassed) {
// recover from test
if (runnable.type === 'test') {
this.emit('test end', runnable);
this.hookUp('afterEach', this.next);
return;
}

// recover from test
if (runnable.type === 'test') {
this.emit('test end', runnable);
this.hookUp('afterEach', this.next);
return;
}

// recover from hooks
if (runnable.type === 'hook') {
// recover from hooks
var errSuite = this.suite;
// if hook failure is in afterEach block
if (runnable.fullTitle().indexOf('after each') > -1) {
11 changes: 11 additions & 0 deletions test/integration/fixtures/uncaught-fatal.fixture.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
'use strict';

it('should bail if a successful test asynchronously fails', function(done) {
done();
process.nextTick(function () {
throw new Error('global error');
});
});

it('should not actually get run', function () {
});
20 changes: 20 additions & 0 deletions test/integration/uncaught.spec.js
Original file line number Diff line number Diff line change
@@ -40,4 +40,24 @@ describe('uncaught exceptions', function () {
done();
});
});

it('handles uncaught exceptions from which Mocha cannot recover', function (done) {
run('uncaught-fatal.fixture.js', args, function (err, res) {
if (err) {
done(err);
return;
}
assert.equal(res.stats.pending, 0);
assert.equal(res.stats.passes, 1);
assert.equal(res.stats.failures, 1);

assert.equal(res.failures[0].title,
'should bail if a successful test asynchronously fails');
assert.equal(res.passes[0].title,
'should bail if a successful test asynchronously fails');

assert.equal(res.code, 1);
done();
});
});
});
21 changes: 11 additions & 10 deletions test/reporters/doc.spec.js
Original file line number Diff line number Diff line change
@@ -6,9 +6,10 @@ var Doc = reporters.Doc;
describe('Doc reporter', function () {
var stdout;
var stdoutWrite;
var runner = {};
var runner;
beforeEach(function () {
stdout = [];
runner = {};
stdoutWrite = process.stdout.write;
process.stdout.write = function (string) {
stdout.push(string);
@@ -24,7 +25,7 @@ describe('Doc reporter', function () {
title: expectedTitle
};
it('should log html with indents and expected title', function () {
runner.on = function (event, callback) {
runner.on = runner.once = function (event, callback) {
if (event === 'suite') {
callback(suite);
}
@@ -44,7 +45,7 @@ describe('Doc reporter', function () {
title: unescapedTitle
};
expectedTitle = '&#x3C;div&#x3E;' + expectedTitle + '&#x3C;/div&#x3E;';
runner.on = function (event, callback) {
runner.on = runner.once = function (event, callback) {
if (event === 'suite') {
callback(suite);
}
@@ -64,7 +65,7 @@ describe('Doc reporter', function () {
root: true
};
it('should not log any html', function () {
runner.on = function (event, callback) {
runner.on = runner.once = function (event, callback) {
if (event === 'suite') {
callback(suite);
}
@@ -82,7 +83,7 @@ describe('Doc reporter', function () {
root: false
};
it('should log expected html with indents', function () {
runner.on = function (event, callback) {
runner.on = runner.once = function (event, callback) {
if (event === 'suite end') {
callback(suite);
}
@@ -100,7 +101,7 @@ describe('Doc reporter', function () {
root: true
};
it('should not log any html', function () {
runner.on = function (event, callback) {
runner.on = runner.once = function (event, callback) {
if (event === 'suite end') {
callback(suite);
}
@@ -123,7 +124,7 @@ describe('Doc reporter', function () {
}
};
it('should log html with indents and expected title and body', function () {
runner.on = function (event, callback) {
runner.on = runner.once = function (event, callback) {
if (event === 'pass') {
callback(test);
}
@@ -144,7 +145,7 @@ describe('Doc reporter', function () {

var expectedEscapedTitle = '&#x3C;div&#x3E;' + expectedTitle + '&#x3C;/div&#x3E;';
var expectedEscapedBody = '&#x3C;div&#x3E;' + expectedBody + '&#x3C;/div&#x3E;';
runner.on = function (event, callback) {
runner.on = runner.once = function (event, callback) {
if (event === 'pass') {
callback(test);
}
@@ -171,7 +172,7 @@ describe('Doc reporter', function () {
}
};
it('should log html with indents and expected title, body and error', function () {
runner.on = function (event, callback) {
runner.on = runner.once = function (event, callback) {
if (event === 'fail') {
callback(test, expectedError);
}
@@ -195,7 +196,7 @@ describe('Doc reporter', function () {
var expectedEscapedTitle = '&#x3C;div&#x3E;' + expectedTitle + '&#x3C;/div&#x3E;';
var expectedEscapedBody = '&#x3C;div&#x3E;' + expectedBody + '&#x3C;/div&#x3E;';
var expectedEscapedError = '&#x3C;div&#x3E;' + expectedError + '&#x3C;/div&#x3E;';
runner.on = function (event, callback) {
runner.on = runner.once = function (event, callback) {
if (event === 'fail') {
callback(test, unescapedError);
}
18 changes: 9 additions & 9 deletions test/reporters/dot.spec.js
Original file line number Diff line number Diff line change
@@ -31,7 +31,7 @@ describe('Dot reporter', function () {

describe('on start', function () {
it('should return a new line', function () {
runner.on = function (event, callback) {
runner.on = runner.once = function (event, callback) {
if (event === 'start') {
callback();
}
@@ -50,7 +50,7 @@ describe('Dot reporter', function () {
Base.window.width = 2;
});
it('should return a new line and then a coma', function () {
runner.on = function (event, callback) {
runner.on = runner.once = function (event, callback) {
if (event === 'pending') {
callback();
}
@@ -66,7 +66,7 @@ describe('Dot reporter', function () {
});
describe('if window width is equal to or less than 1', function () {
it('should return a coma', function () {
runner.on = function (event, callback) {
runner.on = runner.once = function (event, callback) {
if (event === 'pending') {
callback();
}
@@ -91,7 +91,7 @@ describe('Dot reporter', function () {
duration: 1,
slow: function () { return 2; }
};
runner.on = function (event, callback) {
runner.on = runner.once = function (event, callback) {
if (event === 'pass') {
callback(test);
}
@@ -113,7 +113,7 @@ describe('Dot reporter', function () {
duration: 1,
slow: function () { return 2; }
};
runner.on = function (event, callback) {
runner.on = runner.once = function (event, callback) {
if (event === 'pass') {
callback(test);
}
@@ -132,7 +132,7 @@ describe('Dot reporter', function () {
duration: 2,
slow: function () { return 1; }
};
runner.on = function (event, callback) {
runner.on = runner.once = function (event, callback) {
if (event === 'pass') {
callback(test);
}
@@ -158,7 +158,7 @@ describe('Dot reporter', function () {
err: 'some error'
}
};
runner.on = function (event, callback) {
runner.on = runner.once = function (event, callback) {
if (event === 'fail') {
callback(test);
}
@@ -179,7 +179,7 @@ describe('Dot reporter', function () {
err: 'some error'
}
};
runner.on = function (event, callback) {
runner.on = runner.once = function (event, callback) {
if (event === 'fail') {
callback(test);
}
@@ -195,7 +195,7 @@ describe('Dot reporter', function () {
});
describe('on end', function () {
it('should call the epilogue', function () {
runner.on = function (event, callback) {
runner.on = runner.once = function (event, callback) {
if (event === 'end') {
callback();
}
Loading