diff --git a/lib/errors.js b/lib/errors.js index fafee70eee..4d6ead13f9 100644 --- a/lib/errors.js +++ b/lib/errors.js @@ -1,10 +1,24 @@ 'use strict'; + /** + * Factory functions to create throwable error objects * @module Errors */ + /** - * Factory functions to create throwable error objects + * Error constants */ +var constants = { + ERR_MOCHA_FATAL: 'ERR_MOCHA_FATAL', + ERR_MOCHA_INVALID_ARG_TYPE: 'ERR_MOCHA_INVALID_ARG_TYPE', + ERR_MOCHA_INVALID_ARG_VALUE: 'ERR_MOCHA_INVALID_ARG_VALUE', + ERR_MOCHA_INVALID_EXCEPTION: 'ERR_MOCHA_INVALID_EXCEPTION', + ERR_MOCHA_INVALID_INTERFACE: 'ERR_MOCHA_INVALID_INTERFACE', + ERR_MOCHA_INVALID_REPORTER: 'ERR_MOCHA_INVALID_REPORTER', + ERR_MOCHA_MULTIPLE_DONE: 'ERR_MOCHA_MULTIPLE_DONE', + ERR_MOCHA_NO_FILES_MATCH_PATTERN: 'ERR_MOCHA_NO_FILES_MATCH_PATTERN', + ERR_MOCHA_UNSUPPORTED: 'ERR_MOCHA_UNSUPPORTED' +}; /** * Creates an error object to be thrown when no files to be tested could be found using specified pattern. @@ -16,7 +30,7 @@ */ function createNoFilesMatchPatternError(message, pattern) { var err = new Error(message); - err.code = 'ERR_MOCHA_NO_FILES_MATCH_PATTERN'; + err.code = constants.ERR_MOCHA_NO_FILES_MATCH_PATTERN; err.pattern = pattern; return err; } @@ -31,7 +45,7 @@ function createNoFilesMatchPatternError(message, pattern) { */ function createInvalidReporterError(message, reporter) { var err = new TypeError(message); - err.code = 'ERR_MOCHA_INVALID_REPORTER'; + err.code = constants.ERR_MOCHA_INVALID_REPORTER; err.reporter = reporter; return err; } @@ -46,7 +60,7 @@ function createInvalidReporterError(message, reporter) { */ function createInvalidInterfaceError(message, ui) { var err = new Error(message); - err.code = 'ERR_MOCHA_INVALID_INTERFACE'; + err.code = constants.ERR_MOCHA_INVALID_INTERFACE; err.interface = ui; return err; } @@ -60,7 +74,7 @@ function createInvalidInterfaceError(message, ui) { */ function createUnsupportedError(message) { var err = new Error(message); - err.code = 'ERR_MOCHA_UNSUPPORTED'; + err.code = constants.ERR_MOCHA_UNSUPPORTED; return err; } @@ -88,7 +102,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.ERR_MOCHA_INVALID_ARG_TYPE; err.argument = argument; err.expected = expected; err.actual = typeof argument; @@ -107,7 +121,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.ERR_MOCHA_INVALID_ARG_VALUE; err.argument = argument; err.value = value; err.reason = typeof reason !== 'undefined' ? reason : 'is invalid'; @@ -123,12 +137,57 @@ function createInvalidArgumentValueError(message, argument, value, reason) { */ function createInvalidExceptionError(message, value) { var err = new Error(message); - err.code = 'ERR_MOCHA_INVALID_EXCEPTION'; + err.code = constants.ERR_MOCHA_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.ERR_MOCHA_FATAL; err.valueType = typeof value; err.value = value; return err; } +/** + * Creates an error object to be thrown when done() is called multiple times in a test + * + * @public + * @param {string} message - Error message to be displayed. + * @param {Runnable} runnable - Original runnable + * @param {Error} [originalErr] - Original error, if any + * @returns {Error} instance detailing the error condition + */ +function createMultipleDoneError(message, runnable, originalErr) { + var err = new Error(message); + err.code = constants.ERR_MOCHA_MULTIPLE_DONE; + var title = runnable.title; + try { + title = runnable.fullTitle(); + } catch (ignored) { + title += ' (unknown suite)'; + } + + err.runnable = { + file: runnable.file, + type: runnable.type, + title: title, + body: runnable.body + }; + err.valueType = typeof originalErr; + err.value = originalErr; + return err; +} + module.exports = { createInvalidArgumentTypeError: createInvalidArgumentTypeError, createInvalidArgumentValueError: createInvalidArgumentValueError, @@ -137,5 +196,8 @@ module.exports = { createInvalidReporterError: createInvalidReporterError, createMissingArgumentError: createMissingArgumentError, createNoFilesMatchPatternError: createNoFilesMatchPatternError, - createUnsupportedError: createUnsupportedError + createUnsupportedError: createUnsupportedError, + createFatalError: createFatalError, + createMultipleDoneError: createMultipleDoneError, + constants: constants }; diff --git a/lib/runnable.js b/lib/runnable.js index 2d0c428d46..c919cab0b8 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). @@ -306,13 +307,16 @@ Runnable.prototype.run = function(fn) { 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)); - } + self.emit( + 'error', + createMultipleDoneError( + err && err.message + ? err.message + " (and Mocha's done() called multiple times)" + : 'done() called multiple times', + self, + err + ) + ); } // finished diff --git a/lib/runner.js b/lib/runner.js index 948a9b9021..fc6c993f14 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' } ); @@ -131,7 +144,7 @@ function Runner(suite, delay) { this._abort = false; this._delay = delay; this.suite = suite; - this.started = false; + this.state = constants.STATE_IDLE; this.total = suite.total(); this.failures = 0; this.on(constants.EVENT_TEST_END, function(test) { @@ -284,11 +297,21 @@ Runner.prototype.checkGlobals = function(test) { * @param {Error} err */ Runner.prototype.fail = function(test, err) { + if (this.state === constants.STATE_STOPPED) { + if (err.code === errors.constants.ERR_MOCHA_MULTIPLE_DONE) { + throw err; + } + throw createFatalError( + 'Test failed after root suite execution completed!', + err + ); + } if (test.isPending()) { return; } ++this.failures; + debug('total number of failures: %d', this.failures); test.state = STATE_FAILED; if (!isError(err)) { @@ -834,7 +857,7 @@ Runner.prototype.uncaught = function(err) { runnable = new Runnable('Uncaught error outside test suite'); runnable.parent = this.suite; - if (this.started) { + if (this.state === constants.STATE_RUNNING) { this.fail(runnable, err); } else { // Can't recover from this failure @@ -928,7 +951,7 @@ Runner.prototype.run = function(fn) { if (rootSuite.hasOnly()) { rootSuite.filterOnly(); } - self.started = true; + self.state = constants.STATE_RUNNING; if (self._delay) { self.emit(constants.EVENT_DELAY_END); } @@ -949,6 +972,7 @@ Runner.prototype.run = function(fn) { // callback this.on(constants.EVENT_RUN_END, function() { + this.state = constants.STATE_STOPPED; debug(constants.EVENT_RUN_END); process.removeListener('uncaughtException', uncaught); process.on('uncaughtException', self.uncaughtEnd); diff --git a/package-lock.json b/package-lock.json index e66be18a00..d9d1d05ca4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -729,7 +729,7 @@ "aproba": { "version": "1.2.0", "resolved": "https://registry.npmjs.org/aproba/-/aproba-1.2.0.tgz", - "integrity": "sha1-aALmJk79GMeQobDVF/DyYnvyyUo=", + "integrity": "sha512-Y9J6ZjXtoYh8RnXVCMOU/ttDmk1aBjunq9vO0ta5x85WDQiQfUF9sIPBITdbiiIVcBo03Hi3jMxigBtsddlXRw==", "dev": true }, "arch": { @@ -2068,7 +2068,7 @@ "browser-stdout": { "version": "1.3.1", "resolved": "https://registry.npmjs.org/browser-stdout/-/browser-stdout-1.3.1.tgz", - "integrity": "sha1-uqVZ7hTO1zRSIputcyZGfGH6vWA=" + "integrity": "sha512-qhAVI1+Av2X7qelOfAIYwXONood6XlZE/fXaBSmW/T5SzLAmCgzi+eiWE7fUvbHaeNBQH13UftjpXxsfLkMpgw==" }, "browser-sync": { "version": "2.26.7", @@ -4553,7 +4553,7 @@ "diff": { "version": "3.5.0", "resolved": "https://registry.npmjs.org/diff/-/diff-3.5.0.tgz", - "integrity": "sha1-gAwN0eCov7yVg1wgKtIg/jF+WhI=" + "integrity": "sha512-A46qtFgd+g7pDZinpnwiRJtxbC1hpgf0uzP3iG89scHk0AUC7A1TGxf5OiiOUv/JMZR8GOt8hL900hV0bOy5xA==" }, "diffie-hellman": { "version": "5.0.3", @@ -7054,7 +7054,7 @@ "function-bind": { "version": "1.1.1", "resolved": "https://registry.npmjs.org/function-bind/-/function-bind-1.1.1.tgz", - "integrity": "sha1-pWiZ0+o8m6uHS7l3O3xe3pL0iV0=" + "integrity": "sha512-yIovAzMX49sF8Yl58fSCWJ5svSLuaibPxXQJFLmBObTuCr0Mf1KiPopGM9NiFjiYBCbfaa2Fh6breQ6ANVTI0A==" }, "functional-red-black-tree": { "version": "1.0.1", @@ -11100,7 +11100,7 @@ }, "minimist": { "version": "0.0.8", - "resolved": "https://registry.npmjs.org/minimist/-/minimist-0.0.8.tgz", + "resolved": "http://registry.npmjs.org/minimist/-/minimist-0.0.8.tgz", "integrity": "sha1-hX/Kv8M5fSYluCKCYuhqp6ARsF0=" }, "minipass": { @@ -11159,7 +11159,7 @@ }, "mkdirp": { "version": "0.5.1", - "resolved": "https://registry.npmjs.org/mkdirp/-/mkdirp-0.5.1.tgz", + "resolved": "http://registry.npmjs.org/mkdirp/-/mkdirp-0.5.1.tgz", "integrity": "sha1-MAV0OOrGz3+MR2fzhkjWaX11yQM=", "requires": { "minimist": "0.0.8" @@ -11637,7 +11637,7 @@ "npmlog": { "version": "4.1.2", "resolved": "https://registry.npmjs.org/npmlog/-/npmlog-4.1.2.tgz", - "integrity": "sha1-CKfyqL9zRgR3mp76StXMcXq7lUs=", + "integrity": "sha512-2uUqazuKlTaSI/dC8AzicUck7+IrEaOnN/e0jd3Xtt1KcGpwx30v50mL7oPyr/h9bL3E4aZccVwpwP+5W9Vjkg==", "dev": true, "requires": { "are-we-there-yet": "~1.1.2", @@ -12717,7 +12717,7 @@ }, "path-is-absolute": { "version": "1.0.1", - "resolved": "https://registry.npmjs.org/path-is-absolute/-/path-is-absolute-1.0.1.tgz", + "resolved": "http://registry.npmjs.org/path-is-absolute/-/path-is-absolute-1.0.1.tgz", "integrity": "sha1-F0uSaHNVNP+8es5r9TpanhtcX18=" }, "path-is-inside": { @@ -16325,7 +16325,7 @@ }, "strip-eof": { "version": "1.0.0", - "resolved": "https://registry.npmjs.org/strip-eof/-/strip-eof-1.0.0.tgz", + "resolved": "http://registry.npmjs.org/strip-eof/-/strip-eof-1.0.0.tgz", "integrity": "sha1-u0P/VZim6wXYm1n80SnJgzE2Br8=", "dev": true }, @@ -18096,7 +18096,7 @@ }, "wrap-ansi": { "version": "2.1.0", - "resolved": "https://registry.npmjs.org/wrap-ansi/-/wrap-ansi-2.1.0.tgz", + "resolved": "http://registry.npmjs.org/wrap-ansi/-/wrap-ansi-2.1.0.tgz", "integrity": "sha1-2Pw9KE3QV5T+hJc8rs3Rz4JP3YU=", "dev": true, "requires": { diff --git a/package-scripts.js b/package-scripts.js index 0e414e1542..b5acc855e6 100644 --- a/package-scripts.js +++ b/package-scripts.js @@ -17,7 +17,7 @@ function test(testName, mochaParams) { } return `${ process.env.COVERAGE ? coverageCommand : '' - } ${mochaCommand} ${mochaParams}`.trim(); + } ${mochaCommand} ${mochaParams} --color`.trim(); } module.exports = { 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/multiple-done.spec.js b/test/integration/multiple-done.spec.js index 5b592c8877..37bc5a0b77 100644 --- a/test/integration/multiple-done.spec.js +++ b/test/integration/multiple-done.spec.js @@ -1,50 +1,51 @@ 'use strict'; -var assert = require('assert'); -var run = require('./helpers').runMochaJSON; -var args = []; +var runMochaJSON = require('./helpers').runMochaJSON; +var invokeMocha = require('./helpers').invokeMocha; +var ERR_MOCHA_MULTIPLE_DONE = require('../../lib/errors').constants + .ERR_MOCHA_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', 'done() called multiple times'); }); }); 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, + expect( + res, + 'to have failed with error', "second error (and Mocha's done() called multiple times)" ); }); @@ -52,78 +53,120 @@ describe('multiple calls to done()', function() { 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' + } + }); }); }); 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' + } + }); }); }); describe('from a beforeEach hook', function() { before(function(done) { - run('multiple-done-beforeEach.fixture.js', args, function(err, result) { + runMochaJSON('multiple-done-beforeEach', 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'} + }, + { + fullTitle: 'suite "before each" hook in "suite"', + err: {message: 'done() called multiple times'} + } + ]); + }); + }); + + describe('when done() called asynchronously', function() { + before(function(done) { + 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/ + }); + }); + + it('fail with an error containing the information about the test', function() { + expect(res.output, 'to match', /should fail in an async test case/); + }); + + describe('when errored after Runner has completed', function() { + // WARNING: non-deterministic! + before(function() { + if (res.code === 1) { + this.skip(); + } + // the uncaught exception handler catches it + }); + + it('should provide extra information about the runnable', function() { + expect(res.output, 'to match', /multiple-done-async\.fixture\.js/) + .and('to match', /type: 'test'/) + .and('to match', /body: 'function/) + .and('to match', new RegExp(ERR_MOCHA_MULTIPLE_DONE)); }); }); }); diff --git a/test/unit/runner.spec.js b/test/unit/runner.spec.js index b3b3a903a5..42c68f8c2c 100644 --- a/test/unit/runner.spec.js +++ b/test/unit/runner.spec.js @@ -14,6 +14,9 @@ var EVENT_TEST_FAIL = Runner.constants.EVENT_TEST_FAIL; var EVENT_TEST_RETRY = Runner.constants.EVENT_TEST_RETRY; var EVENT_RUN_END = Runner.constants.EVENT_RUN_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; @@ -743,7 +746,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() { @@ -758,20 +761,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'); + }); }); }); });