Skip to content

Commit ac4b2e8

Browse files
jugglinmikeChristopher Hiller
authored and
Christopher Hiller
committed
Fail when test resolution method is overspecified
Users may register `Runnable`s as asynchronous in one of two ways: - Via callback (by defining the body function to have an arity of one) - Via promise (by returning a Promise object from the body function) When both a callback function is specified *and* a Promise object is returned, the `Runnable`'s resolution condition is ambiguous. Practically speaking, users are most likely to make this mistake as they transition between asynchronous styles. Currently, Mocha silently prefers the callback amd ignores the Promise object. Update the implementation of the `Runnable` class to fail immediately when the test resolution method is over-specified in this way.
1 parent b7edc81 commit ac4b2e8

File tree

2 files changed

+15
-2
lines changed

2 files changed

+15
-2
lines changed

lib/runnable.js

+7-2
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,8 @@ Runnable.prototype.run = function(fn){
182182
, start = new Date
183183
, ctx = this.ctx
184184
, finished
185-
, emitted;
185+
, emitted
186+
, result;
186187

187188
// Some times the ctx exists but it is not runnable
188189
if (ctx && ctx.runnable) ctx.runnable(this);
@@ -214,7 +215,7 @@ Runnable.prototype.run = function(fn){
214215
this.resetTimeout();
215216

216217
try {
217-
this.fn.call(ctx, function(err){
218+
result = this.fn.call(ctx, function(err){
218219
if (err instanceof Error || toString.call(err) === "[object Error]") return done(err);
219220
if (null != err) {
220221
if (Object.prototype.toString.call(err) === '[object Object]') {
@@ -225,6 +226,10 @@ Runnable.prototype.run = function(fn){
225226
}
226227
done();
227228
});
229+
230+
if (result && typeof result.then === 'function') {
231+
return done(new Error('Asynchronous resolution method is overspecified. Specify a callback *or* return a Promise, not both.'));
232+
}
228233
} catch (err) {
229234
done(err);
230235
}
+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
describe('overspecified asynchronous resolution method', function() {
2+
it('should fail when multiple methods are used', function(done) {
3+
setTimeout(done, 0);
4+
5+
// uncomment
6+
// return { then: function() {} };
7+
});
8+
});

0 commit comments

Comments
 (0)