Skip to content

Commit 2c720a3

Browse files
boneskullboneskull
committed
do not eat exceptions thrown asynchronously from passed tests; closes #3226 
1 parent 3537061 commit 2c720a3

30 files changed

+204
-115
lines changed

‎lib/reporters/base.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,7 @@ function Base (runner) {
302302
failures.push(test);
303303
});
304304

305-
runner.on('end', function () {
305+
runner.once('end', function () {
306306
stats.end = new Date();
307307
stats.duration = stats.end - stats.start;
308308
});

‎lib/reporters/dot.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ function Dot (runner) {
5656
process.stdout.write(color('fail', Base.symbols.bang));
5757
});
5858

59-
runner.on('end', function () {
59+
runner.once('end', function () {
6060
console.log();
6161
self.epilogue();
6262
});

‎lib/reporters/json-stream.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ function List (runner) {
3939
console.log(JSON.stringify(['fail', test]));
4040
});
4141

42-
runner.on('end', function () {
42+
runner.once('end', function () {
4343
process.stdout.write(JSON.stringify(['end', self.stats]));
4444
});
4545
}

‎lib/reporters/json.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ function JSONReporter (runner) {
4343
pending.push(test);
4444
});
4545

46-
runner.on('end', function () {
46+
runner.once('end', function () {
4747
var obj = {
4848
stats: self.stats,
4949
tests: tests.map(clean),

‎lib/reporters/landing.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ function Landing (runner) {
8181
stream.write('\u001b[0m');
8282
});
8383

84-
runner.on('end', function () {
84+
runner.once('end', function () {
8585
cursor.show();
8686
console.log();
8787
self.epilogue();

‎lib/reporters/list.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ function List (runner) {
5454
console.log(color('fail', ' %d) %s'), ++n, test.fullTitle());
5555
});
5656

57-
runner.on('end', self.epilogue.bind(self));
57+
runner.once('end', self.epilogue.bind(self));
5858
}
5959

6060
/**

‎lib/reporters/markdown.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ function Markdown (runner) {
9191
buf += '```\n\n';
9292
});
9393

94-
runner.on('end', function () {
94+
runner.once('end', function () {
9595
process.stdout.write('# TOC\n');
9696
process.stdout.write(generateTOC(runner.suite));
9797
process.stdout.write(buf);

‎lib/reporters/min.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ function Min (runner) {
2929
process.stdout.write('\u001b[1;3H');
3030
});
3131

32-
runner.on('end', this.epilogue.bind(this));
32+
runner.once('end', this.epilogue.bind(this));
3333
}
3434

3535
/**

‎lib/reporters/nyan.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ function NyanCat (runner) {
5252
self.draw();
5353
});
5454

55-
runner.on('end', function () {
55+
runner.once('end', function () {
5656
Base.cursor.show();
5757
for (var i = 0; i < self.numberOfLines; i++) {
5858
write('\n');

‎lib/reporters/progress.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ function Progress (runner, options) {
8080

8181
// tests are complete, output some stats
8282
// and the failures if any
83-
runner.on('end', function () {
83+
runner.once('end', function () {
8484
cursor.show();
8585
console.log();
8686
self.epilogue();

‎lib/reporters/spec.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ function Spec (runner) {
7272
console.log(indent() + color('fail', ' %d) %s'), ++n, test.title);
7373
});
7474

75-
runner.on('end', self.epilogue.bind(self));
75+
runner.once('end', self.epilogue.bind(self));
7676
}
7777

7878
/**

‎lib/reporters/tap.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ function TAP (runner) {
5151
}
5252
});
5353

54-
runner.on('end', function () {
54+
runner.once('end', function () {
5555
console.log('# tests ' + (passes + failures));
5656
console.log('# pass ' + passes);
5757
console.log('# fail ' + failures);

‎lib/reporters/xunit.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ function XUnit (runner, options) {
7878
tests.push(test);
7979
});
8080

81-
runner.on('end', function () {
81+
runner.once('end', function () {
8282
self.write(tag('testsuite', {
8383
name: suiteName,
8484
tests: stats.tests,

‎lib/runnable.js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,24 @@ Runnable.prototype.isPending = function () {
142142
return this.pending || (this.parent && this.parent.isPending());
143143
};
144144

145+
/**
146+
* Return `true` if this Runnable has failed.
147+
* @return {boolean}
148+
* @private
149+
*/
150+
Runnable.prototype.isFailed = function () {
151+
return !this.isPending() && this.state === 'failed';
152+
};
153+
154+
/**
155+
* Return `true` if this Runnable has passed.
156+
* @return {boolean}
157+
* @private
158+
*/
159+
Runnable.prototype.isPassed = function () {
160+
return !this.isPending() && this.state === 'passed';
161+
};
162+
145163
/**
146164
* Set or get number of retries.
147165
*

‎lib/runner.js

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -726,21 +726,25 @@ Runner.prototype.uncaught = function (err) {
726726

727727
runnable.clearTimeout();
728728

729-
// Ignore errors if complete or pending
730-
if (runnable.state || runnable.isPending()) {
729+
// Ignore errors if already failed or pending
730+
// See #3226
731+
if (runnable.isFailed() || runnable.isPending()) {
731732
return;
732733
}
734+
// we cannot recover gracefully if a Runnable has already passed
735+
// then fails asynchronously
736+
var alreadyPassed = runnable.isPassed();
737+
// this will change the state to "failed" regardless of the current value
733738
this.fail(runnable, err);
739+
if (!alreadyPassed) {
740+
// recover from test
741+
if (runnable.type === 'test') {
742+
this.emit('test end', runnable);
743+
this.hookUp('afterEach', this.next);
744+
return;
745+
}
734746

735-
// recover from test
736-
if (runnable.type === 'test') {
737-
this.emit('test end', runnable);
738-
this.hookUp('afterEach', this.next);
739-
return;
740-
}
741-
742-
// recover from hooks
743-
if (runnable.type === 'hook') {
747+
// recover from hooks
744748
var errSuite = this.suite;
745749
// if hook failure is in afterEach block
746750
if (runnable.fullTitle().indexOf('after each') > -1) {

‎test/integration/fixtures/uncaught-fatal.fixture.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
'use strict';
2+
3+
it('should bail if a successful test asynchronously fails', function(done) {
4+
done();
5+
process.nextTick(function () {
6+
throw new Error('global error');
7+
});
8+
});
9+
10+
it('should not actually get run', function () {
11+
});

‎test/integration/uncaught.spec.js

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,4 +40,24 @@ describe('uncaught exceptions', function () {
4040
done();
4141
});
4242
});
43+
44+
it('handles uncaught exceptions from which Mocha cannot recover', function (done) {
45+
run('uncaught-fatal.fixture.js', args, function (err, res) {
46+
if (err) {
47+
done(err);
48+
return;
49+
}
50+
assert.equal(res.stats.pending, 0);
51+
assert.equal(res.stats.passes, 1);
52+
assert.equal(res.stats.failures, 1);
53+
54+
assert.equal(res.failures[0].title,
55+
'should bail if a successful test asynchronously fails');
56+
assert.equal(res.passes[0].title,
57+
'should bail if a successful test asynchronously fails');
58+
59+
assert.equal(res.code, 1);
60+
done();
61+
});
62+
});
4363
});

‎test/reporters/doc.spec.js

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,10 @@ var Doc = reporters.Doc;
66
describe('Doc reporter', function () {
77
var stdout;
88
var stdoutWrite;
9-
var runner = {};
9+
var runner;
1010
beforeEach(function () {
1111
stdout = [];
12+
runner = {};
1213
stdoutWrite = process.stdout.write;
1314
process.stdout.write = function (string) {
1415
stdout.push(string);
@@ -24,7 +25,7 @@ describe('Doc reporter', function () {
2425
title: expectedTitle
2526
};
2627
it('should log html with indents and expected title', function () {
27-
runner.on = function (event, callback) {
28+
runner.on = runner.once = function (event, callback) {
2829
if (event === 'suite') {
2930
callback(suite);
3031
}
@@ -44,7 +45,7 @@ describe('Doc reporter', function () {
4445
title: unescapedTitle
4546
};
4647
expectedTitle = '&#x3C;div&#x3E;' + expectedTitle + '&#x3C;/div&#x3E;';
47-
runner.on = function (event, callback) {
48+
runner.on = runner.once = function (event, callback) {
4849
if (event === 'suite') {
4950
callback(suite);
5051
}
@@ -64,7 +65,7 @@ describe('Doc reporter', function () {
6465
root: true
6566
};
6667
it('should not log any html', function () {
67-
runner.on = function (event, callback) {
68+
runner.on = runner.once = function (event, callback) {
6869
if (event === 'suite') {
6970
callback(suite);
7071
}
@@ -82,7 +83,7 @@ describe('Doc reporter', function () {
8283
root: false
8384
};
8485
it('should log expected html with indents', function () {
85-
runner.on = function (event, callback) {
86+
runner.on = runner.once = function (event, callback) {
8687
if (event === 'suite end') {
8788
callback(suite);
8889
}
@@ -100,7 +101,7 @@ describe('Doc reporter', function () {
100101
root: true
101102
};
102103
it('should not log any html', function () {
103-
runner.on = function (event, callback) {
104+
runner.on = runner.once = function (event, callback) {
104105
if (event === 'suite end') {
105106
callback(suite);
106107
}
@@ -123,7 +124,7 @@ describe('Doc reporter', function () {
123124
}
124125
};
125126
it('should log html with indents and expected title and body', function () {
126-
runner.on = function (event, callback) {
127+
runner.on = runner.once = function (event, callback) {
127128
if (event === 'pass') {
128129
callback(test);
129130
}
@@ -144,7 +145,7 @@ describe('Doc reporter', function () {
144145

145146
var expectedEscapedTitle = '&#x3C;div&#x3E;' + expectedTitle + '&#x3C;/div&#x3E;';
146147
var expectedEscapedBody = '&#x3C;div&#x3E;' + expectedBody + '&#x3C;/div&#x3E;';
147-
runner.on = function (event, callback) {
148+
runner.on = runner.once = function (event, callback) {
148149
if (event === 'pass') {
149150
callback(test);
150151
}
@@ -171,7 +172,7 @@ describe('Doc reporter', function () {
171172
}
172173
};
173174
it('should log html with indents and expected title, body and error', function () {
174-
runner.on = function (event, callback) {
175+
runner.on = runner.once = function (event, callback) {
175176
if (event === 'fail') {
176177
callback(test, expectedError);
177178
}
@@ -195,7 +196,7 @@ describe('Doc reporter', function () {
195196
var expectedEscapedTitle = '&#x3C;div&#x3E;' + expectedTitle + '&#x3C;/div&#x3E;';
196197
var expectedEscapedBody = '&#x3C;div&#x3E;' + expectedBody + '&#x3C;/div&#x3E;';
197198
var expectedEscapedError = '&#x3C;div&#x3E;' + expectedError + '&#x3C;/div&#x3E;';
198-
runner.on = function (event, callback) {
199+
runner.on = runner.once = function (event, callback) {
199200
if (event === 'fail') {
200201
callback(test, unescapedError);
201202
}

‎test/reporters/dot.spec.js

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ describe('Dot reporter', function () {
3131

3232
describe('on start', function () {
3333
it('should return a new line', function () {
34-
runner.on = function (event, callback) {
34+
runner.on = runner.once = function (event, callback) {
3535
if (event === 'start') {
3636
callback();
3737
}
@@ -50,7 +50,7 @@ describe('Dot reporter', function () {
5050
Base.window.width = 2;
5151
});
5252
it('should return a new line and then a coma', function () {
53-
runner.on = function (event, callback) {
53+
runner.on = runner.once = function (event, callback) {
5454
if (event === 'pending') {
5555
callback();
5656
}
@@ -66,7 +66,7 @@ describe('Dot reporter', function () {
6666
});
6767
describe('if window width is equal to or less than 1', function () {
6868
it('should return a coma', function () {
69-
runner.on = function (event, callback) {
69+
runner.on = runner.once = function (event, callback) {
7070
if (event === 'pending') {
7171
callback();
7272
}
@@ -91,7 +91,7 @@ describe('Dot reporter', function () {
9191
duration: 1,
9292
slow: function () { return 2; }
9393
};
94-
runner.on = function (event, callback) {
94+
runner.on = runner.once = function (event, callback) {
9595
if (event === 'pass') {
9696
callback(test);
9797
}
@@ -113,7 +113,7 @@ describe('Dot reporter', function () {
113113
duration: 1,
114114
slow: function () { return 2; }
115115
};
116-
runner.on = function (event, callback) {
116+
runner.on = runner.once = function (event, callback) {
117117
if (event === 'pass') {
118118
callback(test);
119119
}
@@ -132,7 +132,7 @@ describe('Dot reporter', function () {
132132
duration: 2,
133133
slow: function () { return 1; }
134134
};
135-
runner.on = function (event, callback) {
135+
runner.on = runner.once = function (event, callback) {
136136
if (event === 'pass') {
137137
callback(test);
138138
}
@@ -158,7 +158,7 @@ describe('Dot reporter', function () {
158158
err: 'some error'
159159
}
160160
};
161-
runner.on = function (event, callback) {
161+
runner.on = runner.once = function (event, callback) {
162162
if (event === 'fail') {
163163
callback(test);
164164
}
@@ -179,7 +179,7 @@ describe('Dot reporter', function () {
179179
err: 'some error'
180180
}
181181
};
182-
runner.on = function (event, callback) {
182+
runner.on = runner.once = function (event, callback) {
183183
if (event === 'fail') {
184184
callback(test);
185185
}
@@ -195,7 +195,7 @@ describe('Dot reporter', function () {
195195
});
196196
describe('on end', function () {
197197
it('should call the epilogue', function () {
198-
runner.on = function (event, callback) {
198+
runner.on = runner.once = function (event, callback) {
199199
if (event === 'end') {
200200
callback();
201201
}

0 commit comments

Comments
 (0)