Skip to content

Commit

Permalink
multiple async done() calls result in failure; closes #4151 (#4152)
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
- remove `Runner#uncaughtEnd`; move logic to `Runner#uncaught`, since we can now rely on the value of `Runner#state`.
- upgrade `unexpected-eventemitter`
- fix potential listener leak in `Runner#run`
  • Loading branch information
@boneskull @craigtaub
boneskull authored and craigtaub committed May 21, 2020
1 parent 5fd44cc commit 792292a
Show file tree
Hide file tree
Showing 13 changed files with 518 additions and 829 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 @@ -276,7 +277,7 @@ Runnable.prototype.run = function(fn) {
var start = new Date();
var ctx = this.ctx;
var finished;
var emitted;
var errorWasHandled = false;

// Sometimes the ctx exists, but it is not runnable
if (ctx && ctx.runnable) {
Expand All @@ -285,17 +286,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 @@ -346,7 +341,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 @@ -365,7 +360,7 @@ Runnable.prototype.run = function(fn) {
callFn(this.fn);
}
} catch (err) {
emitted = true;
errorWasHandled = true;
if (err instanceof Pending) {
return done();
} else if (this.allowUncaught) {
Expand Down

0 comments on commit 792292a

Please sign in to comment.