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
Show file tree
Hide file tree
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
Expand Up @@ -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;
});
Expand Down
2 changes: 1 addition & 1 deletion lib/reporters/dot.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
Expand Down
2 changes: 1 addition & 1 deletion lib/reporters/json-stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -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]));
});
}
Expand Down
2 changes: 1 addition & 1 deletion lib/reporters/json.js
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion lib/reporters/landing.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion lib/reporters/list.js
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

/**
Expand Down
2 changes: 1 addition & 1 deletion lib/reporters/markdown.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion lib/reporters/min.js
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

/**
Expand Down
2 changes: 1 addition & 1 deletion lib/reporters/nyan.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
2 changes: 1 addition & 1 deletion lib/reporters/progress.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion lib/reporters/spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

/**
Expand Down
2 changes: 1 addition & 1 deletion lib/reporters/tap.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion lib/reporters/xunit.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
18 changes: 18 additions & 0 deletions lib/runnable.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
26 changes: 15 additions & 11 deletions lib/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
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
Expand Up @@ -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
Expand Up @@ -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);
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand Down
18 changes: 9 additions & 9 deletions test/reporters/dot.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand All @@ -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();
}
Expand All @@ -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();
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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();
}
Expand Down
Loading