diff --git a/lib/cli/collect-files.js b/lib/cli/collect-files.js index 61d54ac4b3..4145f4333c 100644 --- a/lib/cli/collect-files.js +++ b/lib/cli/collect-files.js @@ -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. @@ -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; } diff --git a/lib/errors.js b/lib/errors.js index a85c8c24b9..1e665e5fb3 100644 --- a/lib/errors.js +++ b/lib/errors.js @@ -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. @@ -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; } @@ -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; } @@ -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; } @@ -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; } @@ -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; @@ -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'; @@ -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; @@ -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; @@ -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, @@ -189,5 +304,8 @@ module.exports = { createUnsupportedError: createUnsupportedError, createInvalidPluginError: createInvalidPluginError, createMochaInstanceAlreadyDisposedError: createMochaInstanceAlreadyDisposedError, - createMochaInstanceAlreadyRunningError: createMochaInstanceAlreadyRunningError + createMochaInstanceAlreadyRunningError: createMochaInstanceAlreadyRunningError, + createFatalError: createFatalError, + createMultipleDoneError: createMultipleDoneError, + constants: constants }; diff --git a/lib/runnable.js b/lib/runnable.js index 4d58070f5d..342152c3c2 100644 --- a/lib/runnable.js +++ b/lib/runnable.js @@ -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). @@ -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(); @@ -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 @@ -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) { @@ -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) { diff --git a/lib/runner.js b/lib/runner.js index 2f38fa8e96..e1cf9b4c57 100644 --- a/lib/runner.js +++ b/lib/runner.js @@ -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. @@ -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' } ); @@ -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 = []; @@ -348,8 +361,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)) { @@ -878,7 +901,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 { @@ -954,7 +977,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'); @@ -982,6 +1005,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); diff --git a/package-scripts.js b/package-scripts.js index 7406018366..caa54a37ca 100644 --- a/package-scripts.js +++ b/package-scripts.js @@ -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(); diff --git a/test/integration/fixtures/multiple-done-async.fixture.js b/test/integration/fixtures/multiple-done-async.fixture.js new file mode 100644 index 0000000000..36f0dd336d --- /dev/null +++ b/test/integration/fixtures/multiple-done-async.fixture.js @@ -0,0 +1,20 @@ +'use strict'; + +// The suite below should result in an additional error, but does +// not. Uncomment once this bug is resolved. + +// describe('suite', function() { +// beforeEach(function(done) { +// done(); +// done(); +// }); + +// it('test', function() {}); +// }); + +it('should fail in an async test case', function (done) { + process.nextTick(function () { + done(); + setTimeout(done); + }); +}); diff --git a/test/integration/fixtures/multiple-done-beforeEach.fixture.js b/test/integration/fixtures/multiple-done-before-each.fixture.js similarity index 100% rename from test/integration/fixtures/multiple-done-beforeEach.fixture.js rename to test/integration/fixtures/multiple-done-before-each.fixture.js diff --git a/test/integration/multiple-done.spec.js b/test/integration/multiple-done.spec.js index 5b592c8877..2019df25c5 100644 --- a/test/integration/multiple-done.spec.js +++ b/test/integration/multiple-done.spec.js @@ -1,129 +1,159 @@ 'use strict'; -var assert = require('assert'); -var run = require('./helpers').runMochaJSON; -var args = []; +var runMochaJSON = require('./helpers').runMochaJSON; +var invokeMocha = require('./helpers').invokeMocha; +var MULTIPLE_DONE = require('../../lib/errors').constants.MULTIPLE_DONE; describe('multiple calls to done()', function() { var res; describe('from a spec', function() { before(function(done) { - run('multiple-done.fixture.js', args, function(err, result) { + runMochaJSON('multiple-done', function(err, result) { res = result; done(err); }); }); - it('results in failures', function() { - assert.strictEqual(res.stats.pending, 0, 'wrong "pending" count'); - assert.strictEqual(res.stats.passes, 1, 'wrong "passes" count'); - assert.strictEqual(res.stats.failures, 1, 'wrong "failures" count'); + it('results in failure', function() { + expect(res, 'to have failed test count', 1) + .and('to have passed test count', 1) + .and('to have pending test count', 0) + .and('to have failed'); }); it('throws a descriptive error', function() { - assert.strictEqual( - res.failures[0].err.message, - 'done() called multiple times' - ); + expect(res, 'to have failed with error', { + message: /done\(\) called multiple times in test \(of root suite\) of file.+multiple-done\.fixture\.js/, + code: MULTIPLE_DONE + }); }); }); describe('with error passed on second call', function() { before(function(done) { - run('multiple-done-with-error.fixture.js', args, function(err, result) { + runMochaJSON('multiple-done-with-error', function(err, result) { res = result; done(err); }); }); - it('results in failures', function() { - assert.strictEqual(res.stats.pending, 0, 'wrong "pending" count'); - assert.strictEqual(res.stats.passes, 1, 'wrong "passes" count'); - assert.strictEqual(res.stats.failures, 1, 'wrong "failures" count'); + it('results in failure', function() { + expect(res, 'to have failed test count', 1) + .and('to have passed test count', 1) + .and('to have pending test count', 0) + .and('to have failed'); }); it('should throw a descriptive error', function() { - assert.strictEqual( - res.failures[0].err.message, - "second error (and Mocha's done() called multiple times)" - ); + expect(res, 'to have failed with error', { + message: /done\(\) called multiple times in test \(of root suite\) of file.+multiple-done-with-error\.fixture\.js; in addition, done\(\) received error: Error: second error/, + code: MULTIPLE_DONE + }); }); }); describe('with multiple specs', function() { before(function(done) { - run('multiple-done-specs.fixture.js', args, function(err, result) { + runMochaJSON('multiple-done-specs', function(err, result) { res = result; done(err); }); }); - it('results in a failure', function() { - assert.strictEqual(res.stats.pending, 0); - assert.strictEqual(res.stats.passes, 2); - assert.strictEqual(res.stats.failures, 1); - assert.strictEqual(res.code, 1); + it('results in failure', function() { + expect(res, 'to have failed test count', 1) + .and('to have passed test count', 2) + .and('to have pending test count', 0) + .and('to have failed'); }); it('correctly attributes the error', function() { - assert.strictEqual(res.failures[0].fullTitle, 'suite test1'); - assert.strictEqual( - res.failures[0].err.message, - 'done() called multiple times' - ); + expect(res.failures[0], 'to satisfy', { + fullTitle: 'suite test1', + err: { + message: /done\(\) called multiple times in test of file.+multiple-done-specs\.fixture\.js/, + code: MULTIPLE_DONE + } + }); }); }); describe('from a before hook', function() { before(function(done) { - run('multiple-done-before.fixture.js', args, function(err, result) { + runMochaJSON('multiple-done-before', function(err, result) { res = result; done(err); }); }); - it('results in a failure', function() { - assert.strictEqual(res.stats.pending, 0); - assert.strictEqual(res.stats.passes, 1); - assert.strictEqual(res.stats.failures, 1); - assert.strictEqual(res.code, 1); + it('results in failure', function() { + expect(res, 'to have failed test count', 1) + .and('to have passed test count', 1) + .and('to have pending test count', 0) + .and('to have failed'); }); it('correctly attributes the error', function() { - assert.strictEqual( - res.failures[0].fullTitle, - 'suite "before all" hook in "suite"' - ); - assert.strictEqual( - res.failures[0].err.message, - 'done() called multiple times' - ); + expect(res.failures[0], 'to satisfy', { + fullTitle: 'suite "before all" hook in "suite"', + err: { + message: /done\(\) called multiple times in hook of file.+multiple-done-before\.fixture\.js/ + } + }); }); }); - describe('from a beforeEach hook', function() { + describe('from a "before each" hook', function() { before(function(done) { - run('multiple-done-beforeEach.fixture.js', args, function(err, result) { + runMochaJSON('multiple-done-before-each', function(err, result) { res = result; done(err); }); }); it('results in a failure', function() { - assert.strictEqual(res.stats.pending, 0); - assert.strictEqual(res.stats.passes, 2); - assert.strictEqual(res.stats.failures, 2); - assert.strictEqual(res.code, 2); + expect(res, 'to have failed test count', 2) + .and('to have passed test count', 2) + .and('to have pending test count', 0) + .and('to have exit code', 2); }); it('correctly attributes the errors', function() { - assert.strictEqual(res.failures.length, 2); - res.failures.forEach(function(failure) { - assert.strictEqual( - failure.fullTitle, - 'suite "before each" hook in "suite"' - ); - assert.strictEqual(failure.err.message, 'done() called multiple times'); + expect(res.failures, 'to satisfy', [ + { + fullTitle: 'suite "before each" hook in "suite"', + err: { + message: /done\(\) called multiple times in hook of file.+multiple-done-before-each\.fixture\.js/ + } + }, + { + fullTitle: 'suite "before each" hook in "suite"', + err: { + message: /done\(\) called multiple times in hook of file.+multiple-done-before-each\.fixture\.js/ + } + } + ]); + }); + }); + + describe('when done() called asynchronously', function() { + before(function(done) { + // we can't be sure that mocha won't fail with an uncaught exception here, which would cause any JSON + // output to be befouled; we need to run "raw" and capture STDERR + invokeMocha( + require.resolve('./fixtures/multiple-done-async.fixture.js'), + function(err, result) { + res = result; + done(err); + }, + 'pipe' + ); + }); + + it('results in error', function() { + expect(res, 'to satisfy', { + code: expect.it('to be greater than', 0), + output: /done\(\) called multiple times in test \(of root suite\) of file.+multiple-done-async\.fixture\.js/ }); }); }); diff --git a/test/unit/runnable.spec.js b/test/unit/runnable.spec.js index bdd2dc145e..4f00dadfd7 100644 --- a/test/unit/runnable.spec.js +++ b/test/unit/runnable.spec.js @@ -325,7 +325,11 @@ describe('Runnable(title, fn)', function() { runnable.on('error', errorSpy).on('error', function(err) { process.nextTick(function() { expect(errorSpy, 'was called times', 1); - expect(err.message, 'to be', 'done() called multiple times'); + expect( + err.message, + 'to match', + /done\(\) called multiple times/ + ); expect(callbackSpy, 'was called times', 1); done(); }); @@ -355,8 +359,8 @@ describe('Runnable(title, fn)', function() { expect(errorSpy, 'was called times', 1); expect( err.message, - 'to be', - "fail (and Mocha's done() called multiple times)" + 'to match', + /done\(\) called multiple times.+received error: Error: fail/ ); expect(callbackSpy, 'was called times', 1); done(); diff --git a/test/unit/runner.spec.js b/test/unit/runner.spec.js index d36d0f2f1f..b3f475f199 100644 --- a/test/unit/runner.spec.js +++ b/test/unit/runner.spec.js @@ -17,6 +17,9 @@ var EVENT_TEST_END = Runner.constants.EVENT_TEST_END; var EVENT_RUN_END = Runner.constants.EVENT_RUN_END; var EVENT_SUITE_END = Runner.constants.EVENT_SUITE_END; var STATE_FAILED = Runnable.constants.STATE_FAILED; +var STATE_IDLE = Runner.constants.STATE_IDLE; +var STATE_RUNNING = Runner.constants.STATE_RUNNING; +var STATE_STOPPED = Runner.constants.STATE_STOPPED; describe('Runner', function() { var sandbox; @@ -851,7 +854,7 @@ describe('Runner', function() { describe('when Runner has already started', function() { beforeEach(function() { - runner.started = true; + runner.state = STATE_RUNNING; }); it('should not emit start/end events', function() { @@ -866,20 +869,39 @@ describe('Runner', function() { }); }); - describe('when Runner has not already started', function() { - beforeEach(function() { - runner.started = false; + describe('when Runner not running', function() { + describe('when idle', function() { + beforeEach(function() { + runner.state = STATE_IDLE; + }); + + it('should emit start/end events for the benefit of reporters', function() { + expect( + function() { + runner.uncaught(err); + }, + 'to emit from', + runner, + 'start' + ).and('to emit from', runner, 'end'); + }); }); - it('should emit start/end events for the benefit of reporters', function() { - expect( - function() { - runner.uncaught(err); - }, - 'to emit from', - runner, - 'start' - ).and('to emit from', runner, 'end'); + describe('when stopped', function() { + beforeEach(function() { + runner.state = STATE_STOPPED; + }); + + it('should emit start/end events for the benefit of reporters', function() { + expect( + function() { + runner.uncaught(err); + }, + 'to emit from', + runner, + 'start' + ).and('to emit from', runner, 'end'); + }); }); }); });