Skip to content

Commit

Permalink
multiple async done() calls result in failure; closes #4151
Browse files Browse the repository at this point in the history 
- added a method in `errors` module to create a "multiple done" err
- modernize `multiple-done.spec.js`
- refactor errors into constants in `errors` module
- remove `Runner#started` prop; replace with `Runner#state` prop + constants
- add a catchall `createFatalError()` function to `errors` module; this is called
  when a test fails twice by other means (unsure what those means are yet)
- force color in Travis CI b/c my eyes
  • Loading branch information
@boneskull
boneskull committed May 19, 2020
1 parent 722ce01 commit db4f885
Show file tree
Hide file tree
Showing 10 changed files with 322 additions and 105 deletions.
3 changes: 2 additions & 1 deletion lib/cli/collect-files.js
View file
Expand Up @@ -5,6 +5,7 @@ const ansi = require('ansi-colors');
const debug = require('debug')('mocha:cli:run:helpers');
const minimatch = require('minimatch');
const utils = require('../utils');
const {NO_FILES_MATCH_PATTERN} = require('../errors').constants;

/**
* Exports a function that collects test files from CLI parameters.
Expand Down Expand Up @@ -34,7 +35,7 @@ module.exports = ({ignore, extension, file, recursive, sort, spec} = {}) => {
try {
newFiles = utils.lookupFiles(arg, extension, recursive);
} catch (err) {
if (err.code === 'ERR_MOCHA_NO_FILES_MATCH_PATTERN') {
if (err.code === NO_FILES_MATCH_PATTERN) {
unmatched.push({message: err.message, pattern: err.pattern});
return;
}
Expand Down
140 changes: 129 additions & 11 deletions lib/errors.js
View file
@@ -1,10 +1,73 @@
'use strict';

var format = require('util').format;

/**
* Factory functions to create throwable error objects
* @module Errors
*/

/**
* Factory functions to create throwable error objects
* When Mocha throw exceptions (or otherwise errors), it attempts to assign a
* `code` property to the `Error` object, for easier handling. These are the
* potential values of `code`.
*/
var constants = {
/**
* An unrecoverable error.
*/
FATAL: 'ERR_MOCHA_FATAL',

/**
* The type of an argument to a function call is invalid
*/
INVALID_ARG_TYPE: 'ERR_MOCHA_INVALID_ARG_TYPE',

/**
* The value of an argument to a function call is invalid
*/
INVALID_ARG_VALUE: 'ERR_MOCHA_INVALID_ARG_VALUE',

/**
* Something was thrown, but it wasn't an `Error`
*/
INVALID_EXCEPTION: 'ERR_MOCHA_INVALID_EXCEPTION',

/**
* An interface (e.g., `Mocha.interfaces`) is unknown or invalid
*/
INVALID_INTERFACE: 'ERR_MOCHA_INVALID_INTERFACE',

/**
* A reporter (.e.g, `Mocha.reporters`) is unknown or invalid
*/
INVALID_REPORTER: 'ERR_MOCHA_INVALID_REPORTER',

/**
* `done()` was called twice in a `Test` or `Hook` callback
*/
MULTIPLE_DONE: 'ERR_MOCHA_MULTIPLE_DONE',

/**
* No files matched the pattern provided by the user
*/
NO_FILES_MATCH_PATTERN: 'ERR_MOCHA_NO_FILES_MATCH_PATTERN',

/**
* Known, but unsupported behavior of some kind
*/
UNSUPPORTED: 'ERR_MOCHA_UNSUPPORTED',

/**
* Invalid state transition occuring in `Mocha` instance
*/
INSTANCE_ALREADY_RUNNING: 'ERR_MOCHA_INSTANCE_ALREADY_RUNNING',

/**
* Invalid state transition occuring in `Mocha` instance
*/
INSTANCE_ALREADY_DISPOSED: 'ERR_MOCHA_INSTANCE_ALREADY_DISPOSED'
};

/**
* Creates an error object to be thrown when no files to be tested could be found using specified pattern.
Expand All @@ -16,7 +79,7 @@
*/
function createNoFilesMatchPatternError(message, pattern) {
var err = new Error(message);
err.code = 'ERR_MOCHA_NO_FILES_MATCH_PATTERN';
err.code = constants.NO_FILES_MATCH_PATTERN;
err.pattern = pattern;
return err;
}
Expand All @@ -31,7 +94,7 @@ function createNoFilesMatchPatternError(message, pattern) {
*/
function createInvalidReporterError(message, reporter) {
var err = new TypeError(message);
err.code = 'ERR_MOCHA_INVALID_REPORTER';
err.code = constants.INVALID_REPORTER;
err.reporter = reporter;
return err;
}
Expand All @@ -46,7 +109,7 @@ function createInvalidReporterError(message, reporter) {
*/
function createInvalidInterfaceError(message, ui) {
var err = new Error(message);
err.code = 'ERR_MOCHA_INVALID_INTERFACE';
err.code = constants.INVALID_INTERFACE;
err.interface = ui;
return err;
}
Expand All @@ -60,7 +123,7 @@ function createInvalidInterfaceError(message, ui) {
*/
function createUnsupportedError(message) {
var err = new Error(message);
err.code = 'ERR_MOCHA_UNSUPPORTED';
err.code = constants.UNSUPPORTED;
return err;
}

Expand Down Expand Up @@ -88,7 +151,7 @@ function createMissingArgumentError(message, argument, expected) {
*/
function createInvalidArgumentTypeError(message, argument, expected) {
var err = new TypeError(message);
err.code = 'ERR_MOCHA_INVALID_ARG_TYPE';
err.code = constants.INVALID_ARG_TYPE;
err.argument = argument;
err.expected = expected;
err.actual = typeof argument;
Expand All @@ -107,7 +170,7 @@ function createInvalidArgumentTypeError(message, argument, expected) {
*/
function createInvalidArgumentValueError(message, argument, value, reason) {
var err = new TypeError(message);
err.code = 'ERR_MOCHA_INVALID_ARG_VALUE';
err.code = constants.INVALID_ARG_VALUE;
err.argument = argument;
err.value = value;
err.reason = typeof reason !== 'undefined' ? reason : 'is invalid';
Expand All @@ -123,7 +186,22 @@ function createInvalidArgumentValueError(message, argument, value, reason) {
*/
function createInvalidExceptionError(message, value) {
var err = new Error(message);
err.code = 'ERR_MOCHA_INVALID_EXCEPTION';
err.code = constants.INVALID_EXCEPTION;
err.valueType = typeof value;
err.value = value;
return err;
}

/**
* Creates an error object to be thrown when an unrecoverable error occurs.
*
* @public
* @param {string} message - Error message to be displayed.
* @returns {Error} instance detailing the error condition
*/
function createFatalError(message, value) {
var err = new Error(message);
err.code = constants.FATAL;
err.valueType = typeof value;
err.value = value;
return err;
Expand Down Expand Up @@ -161,7 +239,7 @@ function createMochaInstanceAlreadyDisposedError(
instance
) {
var err = new Error(message);
err.code = 'ERR_MOCHA_INSTANCE_ALREADY_DISPOSED';
err.code = constants.INSTANCE_ALREADY_DISPOSED;
err.cleanReferencesAfterRun = cleanReferencesAfterRun;
err.instance = instance;
return err;
Expand All @@ -173,11 +251,48 @@ function createMochaInstanceAlreadyDisposedError(
*/
function createMochaInstanceAlreadyRunningError(message, instance) {
var err = new Error(message);
err.code = 'ERR_MOCHA_INSTANCE_ALREADY_RUNNING';
err.code = constants.INSTANCE_ALREADY_RUNNING;
err.instance = instance;
return err;
}

/*
* Creates an error object to be thrown when done() is called multiple times in a test
*
* @public
* @param {Runnable} runnable - Original runnable
* @param {Error} [originalErr] - Original error, if any
* @returns {Error} instance detailing the error condition
*/
function createMultipleDoneError(runnable, originalErr) {
var title;
try {
title = format('<%s>', runnable.fullTitle());
if (runnable.parent.root) {
title += ' (of root suite)';
}
} catch (ignored) {
title = format('<%s> (of unknown suite)', runnable.title);
}
var message = format(
'done() called multiple times in %s %s',
runnable.type ? runnable.type : 'unknown runnable',
title
);
if (runnable.file) {
message += format(' of file %s', runnable.file);
}
if (originalErr) {
message += format('; in addition, done() received error: %s', originalErr);
}

var err = new Error(message);
err.code = constants.MULTIPLE_DONE;
err.valueType = typeof originalErr;
err.value = originalErr;
return err;
}

module.exports = {
createInvalidArgumentTypeError: createInvalidArgumentTypeError,
createInvalidArgumentValueError: createInvalidArgumentValueError,
Expand All @@ -189,5 +304,8 @@ module.exports = {
createUnsupportedError: createUnsupportedError,
createInvalidPluginError: createInvalidPluginError,
createMochaInstanceAlreadyDisposedError: createMochaInstanceAlreadyDisposedError,
createMochaInstanceAlreadyRunningError: createMochaInstanceAlreadyRunningError
createMochaInstanceAlreadyRunningError: createMochaInstanceAlreadyRunningError,
createFatalError: createFatalError,
createMultipleDoneError: createMultipleDoneError,
constants: constants
};
23 changes: 9 additions & 14 deletions lib/runnable.js
View file
Expand Up @@ -5,8 +5,9 @@ var Pending = require('./pending');
var debug = require('debug')('mocha:runnable');
var milliseconds = require('ms');
var utils = require('./utils');
var createInvalidExceptionError = require('./errors')
.createInvalidExceptionError;
var errors = require('./errors');
var createInvalidExceptionError = errors.createInvalidExceptionError;
var createMultipleDoneError = errors.createMultipleDoneError;

/**
* Save timer references to avoid Sinon interfering (see GH-237).
Expand Down Expand Up @@ -262,7 +263,7 @@ Runnable.prototype.run = function(fn) {
var start = new Date();
var ctx = this.ctx;
var finished;
var emitted;
var errorWasHandled = false;

if (this.isPending()) return fn();

Expand All @@ -273,17 +274,11 @@ Runnable.prototype.run = function(fn) {

// called multiple times
function multiple(err) {
if (emitted) {
if (errorWasHandled) {
return;
}
emitted = true;
var msg = 'done() called multiple times';
if (err && err.message) {
err.message += " (and Mocha's " + msg + ')';
self.emit('error', err);
} else {
self.emit('error', new Error(msg));
}
errorWasHandled = true;
self.emit('error', createMultipleDoneError(self, err));
}

// finished
Expand Down Expand Up @@ -334,7 +329,7 @@ Runnable.prototype.run = function(fn) {
callFnAsync(this.fn);
} catch (err) {
// handles async runnables which actually run synchronously
emitted = true;
errorWasHandled = true;
if (err instanceof Pending) {
return; // done() is already called in this.skip()
} else if (this.allowUncaught) {
Expand All @@ -349,7 +344,7 @@ Runnable.prototype.run = function(fn) {
try {
callFn(this.fn);
} catch (err) {
emitted = true;
errorWasHandled = true;
if (err instanceof Pending) {
return done();
} else if (this.allowUncaught) {
Expand Down
32 changes: 28 additions & 4 deletions lib/runner.js
View file
Expand Up @@ -27,6 +27,7 @@ var type = utils.type;
var errors = require('./errors');
var createInvalidExceptionError = errors.createInvalidExceptionError;
var createUnsupportedError = errors.createUnsupportedError;
var createFatalError = errors.createFatalError;

/**
* Non-enumerable globals.
Expand Down Expand Up @@ -109,7 +110,19 @@ var constants = utils.defineConstants(
/**
* Emitted when {@link Test} execution has failed, but will retry
*/
EVENT_TEST_RETRY: 'retry'
EVENT_TEST_RETRY: 'retry',
/**
* Initial state of Runner
*/
STATE_IDLE: 'idle',
/**
* State set to this value when the Runner has started running
*/
STATE_RUNNING: 'running',
/**
* State set to this value when the Runner has stopped
*/
STATE_STOPPED: 'stopped'
}
);

Expand Down Expand Up @@ -140,8 +153,8 @@ function Runner(suite, opts) {
this._globals = [];
this._abort = false;
this.suite = suite;
this.started = false;
this._opts = opts;
this.state = constants.STATE_IDLE;
this.total = suite.total();
this.failures = 0;
this._eventListeners = [];
Expand Down Expand Up @@ -351,8 +364,18 @@ Runner.prototype.fail = function(test, err, force) {
if (test.isPending() && !force) {
return;
}
if (this.state === constants.STATE_STOPPED) {
if (err.code === errors.constants.MULTIPLE_DONE) {
throw err;
}
throw createFatalError(
'Test failed after root suite execution completed!',
err
);
}

++this.failures;
debug('total number of failures: %d', this.failures);
test.state = STATE_FAILED;

if (!isError(err)) {
Expand Down Expand Up @@ -881,7 +904,7 @@ Runner.prototype.uncaught = function(err) {
debug('uncaught(): no current Runnable; created a phony one');
runnable.parent = this.suite;

if (this.started) {
if (this.state === constants.STATE_RUNNING) {
debug('uncaught(): failing gracefully');
this.fail(runnable, err);
} else {
Expand Down Expand Up @@ -957,7 +980,7 @@ Runner.prototype.run = function(fn) {
rootSuite.filterOnly();
debug('run(): filtered exclusive Runnables');
}
self.started = true;
self.state = constants.STATE_RUNNING;
if (self._delay) {
self.emit(constants.EVENT_DELAY_END);
debug('run(): "delay" ended');
Expand Down Expand Up @@ -985,6 +1008,7 @@ Runner.prototype.run = function(fn) {

// callback
this.on(constants.EVENT_RUN_END, function() {
self.state = constants.STATE_STOPPED;
debug(constants.EVENT_RUN_END);
self._removeEventListener(process, 'uncaughtException', uncaught);
self._addEventListener(process, 'uncaughtException', self.uncaughtEnd);
Expand Down
3 changes: 3 additions & 0 deletions package-scripts.js
View file
Expand Up @@ -15,6 +15,9 @@ function test(testName, mochaParams) {
if (process.env.CI && !/^only-/.test(testName)) {
mochaParams += ' --forbid-only';
}
if (process.env.TRAVIS) {
mochaParams += ' --color'; // force color in travis-ci
}
return `${
process.env.COVERAGE ? coverageCommand : ''
} ${mochaCommand} ${mochaParams}`.trim();
Expand Down

0 comments on commit db4f885

Please sign in to comment.