From 3a58213c9f584ae4eee267820b897124e7a9afd2 Mon Sep 17 00:00:00 2001 From: Nico Jansen Date: Tue, 21 Apr 2020 22:20:47 +0200 Subject: [PATCH 01/17] Add ability to run tests in a mocha instance multiple times --- lib/errors.js | 33 ++++++++++- lib/mocha.js | 56 +++++++++++++++++- lib/runnable.js | 13 ++++- lib/runner.js | 76 +++++++++++++++++++++---- lib/suite.js | 29 +++++++++- lib/test.js | 13 ++++- test/unit/mocha.spec.js | 114 +++++++++++++++++++++++++++++++++++++ test/unit/runnable.spec.js | 18 ++++++ test/unit/runner.spec.js | 58 ++++++++++++++++++- test/unit/suite.spec.js | 24 ++++++++ test/unit/test.spec.js | 36 +++++++++++- 11 files changed, 447 insertions(+), 23 deletions(-) diff --git a/lib/errors.js b/lib/errors.js index 099bc579ab..78c5b6f48c 100644 --- a/lib/errors.js +++ b/lib/errors.js @@ -149,6 +149,35 @@ function createInvalidPluginError(message, pluginType, pluginId) { } } +/** + * Creates an error object to be thrown when a mocha object's `run` method is executed while it is already disposed. + * @param {string} message The error message to be displayed. + * @param {boolean} autoDispose the value of `autoDispose` + * @param {Mocha} instance the mocha instance that throw this error + */ +function createMochaInstanceAlreadyDisposedError( + message, + autoDispose, + instance +) { + var err = new Error(message); + err.code = 'ERR_MOCHA_INSTANCE_ALREADY_DISPOSED'; + err.autoDispose = autoDispose; + err.instance = instance; + return err; +} + +/** + * Creates an error object to be thrown when a mocha object's `run` method is called while a test run is in progress. + * @param {string} message The error message to be displayed. + */ +function createMochaInstanceAlreadyRunningError(message, instance) { + var err = new Error(message); + err.code = 'ERR_MOCHA_INSTANCE_ALREADY_RUNNING'; + err.instance = instance; + return err; +} + module.exports = { createInvalidArgumentTypeError: createInvalidArgumentTypeError, createInvalidArgumentValueError: createInvalidArgumentValueError, @@ -158,5 +187,7 @@ module.exports = { createMissingArgumentError: createMissingArgumentError, createNoFilesMatchPatternError: createNoFilesMatchPatternError, createUnsupportedError: createUnsupportedError, - createInvalidPluginError: createInvalidPluginError + createInvalidPluginError: createInvalidPluginError, + createMochaInstanceAlreadyDisposedError: createMochaInstanceAlreadyDisposedError, + createMochaInstanceAlreadyRunningError: createMochaInstanceAlreadyRunningError }; diff --git a/lib/mocha.js b/lib/mocha.js index 017daa1e2c..af7fce9a15 100644 --- a/lib/mocha.js +++ b/lib/mocha.js @@ -18,6 +18,10 @@ var esmUtils = utils.supportsEsModules() ? require('./esm-utils') : undefined; var createStatsCollector = require('./stats-collector'); var createInvalidReporterError = errors.createInvalidReporterError; var createInvalidInterfaceError = errors.createInvalidInterfaceError; +var createMochaInstanceAlreadyDisposedError = + errors.createMochaInstanceAlreadyDisposedError; +var createMochaInstanceAlreadyRunningError = + errors.createMochaInstanceAlreadyRunningError; var EVENT_FILE_PRE_REQUIRE = Suite.constants.EVENT_FILE_PRE_REQUIRE; var EVENT_FILE_POST_REQUIRE = Suite.constants.EVENT_FILE_POST_REQUIRE; var EVENT_FILE_REQUIRE = Suite.constants.EVENT_FILE_REQUIRE; @@ -97,6 +101,7 @@ function Mocha(options) { this.options = options; // root suite this.suite = new exports.Suite('', new exports.Context(), true); + this._autoDispose = true; this.grep(options.grep) .fgrep(options.fgrep) @@ -488,6 +493,31 @@ Mocha.prototype.checkLeaks = function(checkLeaks) { return this; }; +/** + * Enables or disables whether or not to dispose after each test run. + * @public + * @see [Mocha.dispose](/#dispose) + * @default true + * @param {boolean} autoDispose + * @return {Mocha} this + * @chainable + */ +Mocha.prototype.autoDispose = function(autoDispose) { + this._autoDispose = autoDispose !== false; + return this; +}; + +/** + * Manually dispose this mocha instance. Mark this instance as `disposed` and unable to run more tests. + * It also removes function references to tests functions and hooks, so variables trapped in closures can be cleaned by the garbage collector. + * @public + * @returns {void} + */ +Mocha.prototype.dispose = function() { + this._isDisposed = true; + this.suite.dispose(); +}; + /** * Displays full stack trace upon test failure. * @@ -825,13 +855,31 @@ Object.defineProperty(Mocha.prototype, 'version', { * mocha.run(failures => process.exitCode = failures ? 1 : 0); */ Mocha.prototype.run = function(fn) { + if (this._runIsInProgress) { + throw createMochaInstanceAlreadyRunningError( + 'Mocha instance is currently running, cannot start a second test run until this one is done', + this + ); + } + this._runIsInProgress = true; + if (this._isDisposed) { + throw createMochaInstanceAlreadyDisposedError( + 'Mocha instance is already disposed, cannot start a new test run. Please create a new mocha instance. Be sure to set disable `autoDispose` when you want to reuse the same mocha instance for multiple test runs.', + this._autoDispose, + this + ); + } + if (this._dirty) { + this.suite.reset(); + } if (this.files.length && !this.loadAsync) { this.loadFiles(); } + var self = this; var suite = this.suite; var options = this.options; options.files = this.files; - var runner = new exports.Runner(suite, options.delay); + var runner = new exports.Runner(suite, options.delay, this._autoDispose); createStatsCollector(runner); var reporter = new this._reporter(runner, options); runner.checkLeaks = options.checkLeaks === true; @@ -856,6 +904,12 @@ Mocha.prototype.run = function(fn) { exports.reporters.Base.hideDiff = !options.diff; function done(failures) { + self._dirty = true; + runner.dispose(); + if (self._autoDispose) { + self.dispose(); + } + self._runIsInProgress = false; fn = fn || utils.noop; if (reporter.done) { reporter.done(failures, fn); diff --git a/lib/runnable.js b/lib/runnable.js index bdd6fffe5c..d8b63109b1 100644 --- a/lib/runnable.js +++ b/lib/runnable.js @@ -36,10 +36,8 @@ function Runnable(title, fn) { this._timeout = 2000; this._slow = 75; this._enableTimeouts = true; - this.timedOut = false; this._retries = -1; - this._currentRetry = 0; - this.pending = false; + this.reset(); } /** @@ -47,6 +45,15 @@ function Runnable(title, fn) { */ utils.inherits(Runnable, EventEmitter); +/** + * Resets the state initially or for a next run. + */ +Runnable.prototype.reset = function() { + this.timedOut = false; + this._currentRetry = 0; + this.pending = false; +}; + /** * Get current timeout value in msecs. * diff --git a/lib/runner.js b/lib/runner.js index aabffda96a..f152b6837a 100644 --- a/lib/runner.js +++ b/lib/runner.js @@ -123,16 +123,22 @@ module.exports = Runner; * @class * @param {Suite} suite Root suite * @param {boolean} [delay] Whether to delay execution of root suite until ready. + * @param {boolean} [cleanReferencesAfterRun] Whether to clean references to test fns and hooks when a suite is done. */ -function Runner(suite, delay) { +function Runner(suite, delay, cleanReferencesAfterRun) { var self = this; this._globals = []; this._abort = false; this._delay = delay; this.suite = suite; this.started = false; + this._cleanReferencesAfterRun = cleanReferencesAfterRun; this.total = suite.total(); this.failures = 0; + /** + * @types {[[EventEmitter, string, function]]} + */ + this._eventListeners = []; this.on(constants.EVENT_TEST_END, function(test) { if (test.type === 'test' && test.retriedTest() && test.parent) { var idx = @@ -162,6 +168,53 @@ Runner.immediately = global.setImmediate || process.nextTick; */ inherits(Runner, EventEmitter); +/** + * Replacement for `target.on(eventName, listener)` that does bookkeeping to remove them when this runner instance is disposed. + * @param target {EventEmitter} + * @param eventName {string} + * @param fn {function} + */ +Runner.prototype._addEventListener = function(target, eventName, listener) { + target.on(eventName, listener); + this._eventListeners.push([target, eventName, listener]); +}; + +/** + * Replacement for `target.removeListener(eventName, listener)` that also updates the bookkeeping. + * @param target {EventEmitter} + * @param eventName {string} + * @param fn {function} + */ +Runner.prototype._removeEventListener = function(target, eventName, listener) { + var eventListenerIndex = this._eventListeners.findIndex(function( + eventListenerDescriptor + ) { + return ( + eventListenerDescriptor[0] === target && + eventListenerDescriptor[1] === eventName && + eventListenerDescriptor[2] === listener + ); + }); + if (eventListenerIndex !== -1) { + var removedListener = this._eventListeners.splice(eventListenerIndex, 1)[0]; + removedListener[0].removeListener(removedListener[1], removedListener[2]); + } +}; + +/** + * Removes all event handlers set during a run on this instance. + * Remark: this does _not_ clean/dispose the tests or suites themselves. + */ +Runner.prototype.dispose = function() { + this.removeAllListeners(); + this._eventListeners.forEach(function(eventListenerDescriptor) { + eventListenerDescriptor[0].removeListener( + eventListenerDescriptor[1], + eventListenerDescriptor[2] + ); + }); +}; + /** * Run tests with full titles matching `re`. Updates runner.total * with number of tests matched. @@ -378,7 +431,7 @@ Runner.prototype.hook = function(name, fn) { self.emit(constants.EVENT_HOOK_BEGIN, hook); if (!hook.listeners('error').length) { - hook.on('error', function(err) { + self._addEventListener(hook, 'error', function(err) { self.failHook(hook, err); }); } @@ -530,7 +583,7 @@ Runner.prototype.runTest = function(fn) { if (this.asyncOnly) { test.asyncOnly = true; } - test.on('error', function(err) { + this._addEventListener(test, 'error', function(err) { self.fail(test, err); }); if (this.allowUncaught) { @@ -920,21 +973,24 @@ Runner.prototype.run = function(fn) { } // references cleanup to avoid memory leaks - this.on(constants.EVENT_SUITE_END, function(suite) { - suite.cleanReferences(); - }); + if (this._cleanReferencesAfterRun) { + this.on(constants.EVENT_SUITE_END, function(suite) { + suite.cleanReferences(); + }); + } // callback this.on(constants.EVENT_RUN_END, function() { - process.removeListener('uncaughtException', uncaught); - process.on('uncaughtException', self.uncaughtEnd); + debug(constants.EVENT_RUN_END); + self._removeEventListener(process, 'uncaughtException', uncaught); + self._addEventListener(process, 'uncaughtException', self.uncaughtEnd); debug('run(): emitted %s', constants.EVENT_RUN_END); fn(self.failures); }); // uncaught exception - process.removeListener('uncaughtException', self.uncaughtEnd); - process.on('uncaughtException', uncaught); + self._removeEventListener(process, 'uncaughtException', self.uncaughtEnd); + self._addEventListener(process, 'uncaughtException', uncaught); if (this._delay) { // for reporters, I guess. diff --git a/lib/suite.js b/lib/suite.js index 191d946b50..c82f672072 100644 --- a/lib/suite.js +++ b/lib/suite.js @@ -61,20 +61,20 @@ function Suite(title, parentContext, isRoot) { this.ctx = new Context(); this.suites = []; this.tests = []; + this.root = isRoot === true; this.pending = false; + this._retries = -1; this._beforeEach = []; this._beforeAll = []; this._afterEach = []; this._afterAll = []; - this.root = isRoot === true; this._timeout = 2000; this._enableTimeouts = true; this._slow = 75; this._bail = false; - this._retries = -1; this._onlyTests = []; this._onlySuites = []; - this.delayed = false; + this.reset(); this.on('newListener', function(event) { if (deprecatedEvents[event]) { @@ -92,6 +92,19 @@ function Suite(title, parentContext, isRoot) { */ inherits(Suite, EventEmitter); +/** + * Resets the state initially or for a next run. + */ +Suite.prototype.reset = function() { + this.delayed = false; + this.suites.forEach(function(suite) { + suite.reset(); + }); + this.tests.forEach(function(test) { + test.reset(); + }); +}; + /** * Return a clone of this `Suite`. * @@ -511,6 +524,16 @@ Suite.prototype.getHooks = function getHooks(name) { return this['_' + name]; }; +/** + * cleans all references from this suite and all child suites. + */ +Suite.prototype.dispose = function() { + this.suites.forEach(function(suite) { + suite.dispose(); + }); + this.cleanReferences(); +}; + /** * Cleans up the references to all the deferred functions * (before/after/beforeEach/afterEach) and tests of a Suite. diff --git a/lib/test.js b/lib/test.js index 87f1ccce7d..40a465648e 100644 --- a/lib/test.js +++ b/lib/test.js @@ -26,9 +26,9 @@ function Test(title, fn) { 'string' ); } - Runnable.call(this, title, fn); - this.pending = !fn; this.type = 'test'; + Runnable.call(this, title, fn); + this.reset(); } /** @@ -36,6 +36,15 @@ function Test(title, fn) { */ utils.inherits(Test, Runnable); +/** + * Resets the state initially or for a next run. + */ +Test.prototype.reset = function() { + Runnable.prototype.reset.call(this); + this.pending = !this.fn; + delete this.state; +}; + /** * Set or get retried test * diff --git a/test/unit/mocha.spec.js b/test/unit/mocha.spec.js index a317ca7c39..a1f76bedea 100644 --- a/test/unit/mocha.spec.js +++ b/test/unit/mocha.spec.js @@ -22,6 +22,10 @@ describe('Mocha', function() { sandbox.stub(Mocha.prototype, 'global').returnsThis(); }); + it('should set _autoDispose to true', function() { + expect(new Mocha()._autoDispose, 'to be', true); + }); + describe('when "options.timeout" is `undefined`', function() { it('should not attempt to set timeout', function() { // eslint-disable-next-line no-new @@ -89,6 +93,25 @@ describe('Mocha', function() { }); }); + describe('#autoDispose()', function() { + it('should set the _autoDispose attribute', function() { + var mocha = new Mocha(opts); + mocha.autoDispose(); + expect(mocha._autoDispose, 'to be', true); + }); + + it('should set the _autoDispose attribute to false', function() { + var mocha = new Mocha(opts); + mocha.autoDispose(false); + expect(mocha._autoDispose, 'to be', false); + }); + + it('should be chainable', function() { + var mocha = new Mocha(opts); + expect(mocha.autoDispose(), 'to be', mocha); + }); + }); + describe('#bail()', function() { it('should set the suite._bail to true if there is no arguments', function() { var mocha = new Mocha(opts); @@ -191,6 +214,15 @@ describe('Mocha', function() { }); }); + describe('#dispose()', function() { + it('should set dispose the root suite', function() { + var mocha = new Mocha(opts); + var disposeStub = sandbox.stub(mocha.suite, 'dispose'); + mocha.dispose(); + expect(disposeStub, 'was called once'); + }); + }); + describe('#forbidOnly()', function() { it('should set the forbidOnly option to true', function() { var mocha = new Mocha(opts); @@ -447,6 +479,88 @@ describe('Mocha', function() { mocha.run().on('end', done); }); + it('should throw if a run is in progress', function() { + var mocha = new Mocha(opts); + var runStub = sandbox.stub(Mocha.Runner.prototype, 'run'); + mocha.run(); + expect( + function() { + mocha.run(); + }, + 'to throw', + { + message: + 'Mocha instance is currently running, cannot start a second test run until this one is done', + code: 'ERR_MOCHA_INSTANCE_ALREADY_RUNNING', + instance: mocha + } + ); + expect(runStub, 'was called once'); + }); + + it('should throw the instance is already disposed', function() { + var mocha = new Mocha(opts); + var runStub = sandbox.stub(Mocha.Runner.prototype, 'run'); + mocha.dispose(); + expect( + function() { + mocha.run(); + }, + 'to throw', + { + message: + 'Mocha instance is already disposed, cannot start a new test run. Please create a new mocha instance. Be sure to set disable `autoDispose` when you want to reuse the same mocha instance for multiple test runs.', + code: 'ERR_MOCHA_INSTANCE_ALREADY_DISPOSED', + instance: mocha + } + ); + expect(runStub, 'was called times', 0); + }); + + it('should throw if a run for a second time', function() { + var mocha = new Mocha(opts); + var runStub = sandbox.stub(Mocha.Runner.prototype, 'run'); + mocha.run(); + runStub.callArg(0); + expect( + function() { + mocha.run(); + }, + 'to throw', + { + message: + 'Mocha instance is already disposed, cannot start a new test run. Please create a new mocha instance. Be sure to set disable `autoDispose` when you want to reuse the same mocha instance for multiple test runs.', + code: 'ERR_MOCHA_INSTANCE_ALREADY_DISPOSED', + instance: mocha + } + ); + expect(runStub, 'was called once'); + }); + + it('should allow multiple runs if `autoDispose` is disabled', function() { + var mocha = new Mocha(opts); + var runStub = sandbox.stub(Mocha.Runner.prototype, 'run'); + mocha.autoDispose(false); + mocha.run(); + runStub.callArg(0); + mocha.run(); + runStub.callArg(0); + expect(runStub, 'called times', 2); + }); + + it('should reset between runs', function() { + var mocha = new Mocha(opts); + var runStub = sandbox.stub(Mocha.Runner.prototype, 'run'); + var disposeStub = sandbox.stub(Mocha.Runner.prototype, 'dispose'); + var resetStub = sandbox.stub(Mocha.Suite.prototype, 'reset'); + mocha.autoDispose(false); + mocha.run(); + runStub.callArg(0); + mocha.run(); + expect(resetStub, 'was called once'); + expect(disposeStub, 'was called once'); + }); + describe('#reporter("xunit")#run(fn)', function() { // :TBD: Why does specifying reporter differentiate this test from preceding one it('should not raise errors if callback was not provided', function() { diff --git a/test/unit/runnable.spec.js b/test/unit/runnable.spec.js index 2c079c3b40..afa77d407d 100644 --- a/test/unit/runnable.spec.js +++ b/test/unit/runnable.spec.js @@ -179,6 +179,24 @@ describe('Runnable(title, fn)', function() { }); }); + describe('#reset', function() { + var run; + + beforeEach(function() { + run = new Runnable(); + }); + + it('should reset current run state', function() { + run.timedOut = true; + this._currentRetry = 5; + this.pending = true; + run.reset(); + expect(run.timedOut, 'to be', false); + expect(run._currentRetry, 'to be', 0); + expect(run.pending, 'to be', false); + }); + }); + describe('.title', function() { it('should be present', function() { expect(new Runnable('foo').title, 'to be', 'foo'); diff --git a/test/unit/runner.spec.js b/test/unit/runner.spec.js index 692559f1ed..6aed62763a 100644 --- a/test/unit/runner.spec.js +++ b/test/unit/runner.spec.js @@ -15,6 +15,7 @@ var EVENT_TEST_FAIL = Runner.constants.EVENT_TEST_FAIL; var EVENT_TEST_RETRY = Runner.constants.EVENT_TEST_RETRY; 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; describe('Runner', function() { @@ -24,7 +25,7 @@ describe('Runner', function() { beforeEach(function() { suite = new Suite('Suite', 'root'); - runner = new Runner(suite); + runner = new Runner(suite, undefined, true); runner.checkLeaks = true; sandbox = sinon.createSandbox(); }); @@ -452,13 +453,66 @@ describe('Runner', function() { done(); }); }); - // karma-mocha is inexplicably doing this with a Hook it('should not throw an exception if something emits EVENT_TEST_END with a non-Test object', function() { expect(function() { runner.emit(EVENT_TEST_END, {}); }, 'not to throw'); }); + + it('should clean references after a run', function() { + runner = new Runner(suite, false, true); + var cleanReferencesStub = sandbox.stub(suite, 'cleanReferences'); + runner.run(); + runner.emit(EVENT_SUITE_END, suite); + expect(cleanReferencesStub, 'was called once'); + }); + + it('should not clean references after a run when `cleanReferencesAfterRun` is `false`', function() { + runner = new Runner(suite, false, false); + var cleanReferencesStub = sandbox.stub(suite, 'cleanReferences'); + runner.run(); + runner.emit(EVENT_SUITE_END, suite); + expect(cleanReferencesStub, 'was not called'); + }); + }); + + describe('.dispose', function() { + it('should remove all listeners from itself', function() { + runner.on('disposeShouldRemoveThis', noop); + runner.dispose(); + expect(runner.listenerCount('disposeShouldRemoveThis'), 'to be', 0); + }); + + it('should remove "error" listeners from a test', function() { + var fn = sandbox.stub(); + runner.test = new Test('test for dispose', fn); + runner.runTest(noop); + // sanity check + expect(runner.test.listenerCount('error'), 'to be', 1); + runner.dispose(); + expect(runner.test.listenerCount('error'), 'to be', 0); + }); + + it('should remove "uncaughtException" listeners from the process', function() { + var normalUncaughtExceptionListenerCount = process.listenerCount( + 'uncaughtException' + ); + sandbox.stub(); + runner.run(noop); + // sanity check + expect( + process.listenerCount('uncaughtException'), + 'to be', + normalUncaughtExceptionListenerCount + 1 + ); + runner.dispose(); + expect( + process.listenerCount('uncaughtException'), + 'to be', + normalUncaughtExceptionListenerCount + ); + }); }); describe('.runTest(fn)', function() { diff --git a/test/unit/suite.spec.js b/test/unit/suite.spec.js index 1be948e1c6..2fd32889bc 100644 --- a/test/unit/suite.spec.js +++ b/test/unit/suite.spec.js @@ -80,6 +80,30 @@ describe('Suite', function() { }); }); + describe('.reset()', function() { + beforeEach(function() { + this.suite = new Suite('Suite to be reset', function() {}); + }); + + it('should reset the `delayed` state', function() { + this.suite.delayed = true; + this.suite.reset(); + expect(this.suite.delayed, 'to be', false); + }); + + it('should forward reset to suites and tests', function() { + var childSuite = new Suite('child suite', this.suite.context); + var test = new Test('test', function() {}); + this.suite.addSuite(childSuite); + this.suite.addTest(test); + var testResetStub = sandbox.stub(test, 'reset'); + var suiteResetStub = sandbox.stub(childSuite, 'reset'); + this.suite.reset(); + expect(testResetStub, 'was called once'); + expect(suiteResetStub, 'was called once'); + }); + }); + describe('.timeout()', function() { beforeEach(function() { this.suite = new Suite('A Suite'); diff --git a/test/unit/test.spec.js b/test/unit/test.spec.js index 7bf739d47e..a0c6d82284 100644 --- a/test/unit/test.spec.js +++ b/test/unit/test.spec.js @@ -1,10 +1,24 @@ 'use strict'; +var sinon = require('sinon'); var mocha = require('../../lib/mocha'); var Test = mocha.Test; -var sinon = require('sinon'); +var Runnable = mocha.Runnable; describe('Test', function() { + /** + * @type {sinon.SinonSandbox} + */ + var sandbox; + + beforeEach(function() { + sandbox = sinon.createSandbox(); + }); + + afterEach(function() { + sandbox.restore(); + }); + describe('.clone()', function() { beforeEach(function() { this._test = new Test('To be cloned', function() {}); @@ -61,6 +75,26 @@ describe('Test', function() { }); }); + describe('.reset()', function() { + beforeEach(function() { + this._test = new Test('Test to be reset', function() {}); + }); + + it('should reset the run state', function() { + this._test.state = 'pending'; + this._test.pending = true; + this._test.reset(); + expect(this._test, 'not to have property', 'state'); + expect(this._test.pending, 'to be', false); + }); + + it('should call Runnable.reset', function() { + var runnableResetStub = sandbox.stub(Runnable.prototype, 'reset'); + this._test.reset(); + expect(runnableResetStub, 'was called once'); + }); + }); + describe('.isPending()', function() { beforeEach(function() { this._test = new Test('Is it skipped', function() {}); From 89279b985fee40785ecdb274a203ddb94aeabecc Mon Sep 17 00:00:00 2001 From: Nico Jansen Date: Wed, 22 Apr 2020 07:19:32 +0200 Subject: [PATCH 02/17] Rename `autoDispsoe` to `cleanReferencesAfterRun`, Rename since we cannot dispose the entire mocha instance after a test run. We should keep the process.on('uncaughtException') handlers in place in order to not break existing functionality. --- lib/errors.js | 6 ++--- lib/mocha.js | 34 +++++++++++++----------- test/unit/mocha.spec.js | 57 ++++++++++++++++++++++++++++------------- 3 files changed, 61 insertions(+), 36 deletions(-) diff --git a/lib/errors.js b/lib/errors.js index 78c5b6f48c..29ee916e63 100644 --- a/lib/errors.js +++ b/lib/errors.js @@ -152,17 +152,17 @@ function createInvalidPluginError(message, pluginType, pluginId) { /** * Creates an error object to be thrown when a mocha object's `run` method is executed while it is already disposed. * @param {string} message The error message to be displayed. - * @param {boolean} autoDispose the value of `autoDispose` + * @param {boolean} cleanReferencesAfterRun the value of `autoDispose` * @param {Mocha} instance the mocha instance that throw this error */ function createMochaInstanceAlreadyDisposedError( message, - autoDispose, + cleanReferencesAfterRun, instance ) { var err = new Error(message); err.code = 'ERR_MOCHA_INSTANCE_ALREADY_DISPOSED'; - err.autoDispose = autoDispose; + err.cleanReferencesAfterRun = cleanReferencesAfterRun; err.instance = instance; return err; } diff --git a/lib/mocha.js b/lib/mocha.js index af7fce9a15..acc09aa9c8 100644 --- a/lib/mocha.js +++ b/lib/mocha.js @@ -101,7 +101,7 @@ function Mocha(options) { this.options = options; // root suite this.suite = new exports.Suite('', new exports.Context(), true); - this._autoDispose = true; + this._cleanReferencesAfterRun = true; this.grep(options.grep) .fgrep(options.fgrep) @@ -496,14 +496,14 @@ Mocha.prototype.checkLeaks = function(checkLeaks) { /** * Enables or disables whether or not to dispose after each test run. * @public - * @see [Mocha.dispose](/#dispose) + * @see [Mocha.cleanReferencesAfterRun](/#cleanReferencesAfterRun) * @default true - * @param {boolean} autoDispose + * @param {boolean} cleanReferencesAfterRun * @return {Mocha} this * @chainable */ -Mocha.prototype.autoDispose = function(autoDispose) { - this._autoDispose = autoDispose !== false; +Mocha.prototype.cleanReferencesAfterRun = function(cleanReferencesAfterRun) { + this._cleanReferencesAfterRun = cleanReferencesAfterRun !== false; return this; }; @@ -514,7 +514,13 @@ Mocha.prototype.autoDispose = function(autoDispose) { * @returns {void} */ Mocha.prototype.dispose = function() { + if (this._runIsInProgress) { + throw createMochaInstanceAlreadyRunningError( + 'Cannot dispose while the mocha instance is still running tests.' + ); + } this._isDisposed = true; + this._previousRunner && this._previousRunner.dispose(); this.suite.dispose(); }; @@ -857,19 +863,20 @@ Object.defineProperty(Mocha.prototype, 'version', { Mocha.prototype.run = function(fn) { if (this._runIsInProgress) { throw createMochaInstanceAlreadyRunningError( - 'Mocha instance is currently running, cannot start a second test run until this one is done', + 'Mocha instance is currently running tests, cannot start a next test run until this one is done', this ); } this._runIsInProgress = true; - if (this._isDisposed) { + if (this._isDisposed || this._referencesCleaned) { throw createMochaInstanceAlreadyDisposedError( - 'Mocha instance is already disposed, cannot start a new test run. Please create a new mocha instance. Be sure to set disable `autoDispose` when you want to reuse the same mocha instance for multiple test runs.', - this._autoDispose, + 'Mocha instance is already disposed, cannot start a new test run. Please create a new mocha instance. Be sure to set disable `cleanReferencesAfterRun` when you want to reuse the same mocha instance for multiple test runs.', + this._cleanReferencesAfterRun, this ); } - if (this._dirty) { + if (this._previousRunner) { + this._previousRunner.dispose(); this.suite.reset(); } if (this.files.length && !this.loadAsync) { @@ -904,12 +911,9 @@ Mocha.prototype.run = function(fn) { exports.reporters.Base.hideDiff = !options.diff; function done(failures) { - self._dirty = true; - runner.dispose(); - if (self._autoDispose) { - self.dispose(); - } + self._previousRunner = runner; self._runIsInProgress = false; + self._referencesCleaned = self._cleanReferencesAfterRun; fn = fn || utils.noop; if (reporter.done) { reporter.done(failures, fn); diff --git a/test/unit/mocha.spec.js b/test/unit/mocha.spec.js index a1f76bedea..1c3c1b8720 100644 --- a/test/unit/mocha.spec.js +++ b/test/unit/mocha.spec.js @@ -22,8 +22,8 @@ describe('Mocha', function() { sandbox.stub(Mocha.prototype, 'global').returnsThis(); }); - it('should set _autoDispose to true', function() { - expect(new Mocha()._autoDispose, 'to be', true); + it('should set _cleanReferencesAfterRun to true', function() { + expect(new Mocha()._cleanReferencesAfterRun, 'to be', true); }); describe('when "options.timeout" is `undefined`', function() { @@ -93,22 +93,22 @@ describe('Mocha', function() { }); }); - describe('#autoDispose()', function() { - it('should set the _autoDispose attribute', function() { + describe('#cleanReferencesAfterRun()', function() { + it('should set the _cleanReferencesAfterRun attribute', function() { var mocha = new Mocha(opts); - mocha.autoDispose(); - expect(mocha._autoDispose, 'to be', true); + mocha.cleanReferencesAfterRun(); + expect(mocha._cleanReferencesAfterRun, 'to be', true); }); - it('should set the _autoDispose attribute to false', function() { + it('should set the _cleanReferencesAfterRun attribute to false', function() { var mocha = new Mocha(opts); - mocha.autoDispose(false); - expect(mocha._autoDispose, 'to be', false); + mocha.cleanReferencesAfterRun(false); + expect(mocha._cleanReferencesAfterRun, 'to be', false); }); it('should be chainable', function() { var mocha = new Mocha(opts); - expect(mocha.autoDispose(), 'to be', mocha); + expect(mocha.cleanReferencesAfterRun(), 'to be', mocha); }); }); @@ -215,12 +215,22 @@ describe('Mocha', function() { }); describe('#dispose()', function() { - it('should set dispose the root suite', function() { + it('should dispose the root suite', function() { var mocha = new Mocha(opts); var disposeStub = sandbox.stub(mocha.suite, 'dispose'); mocha.dispose(); expect(disposeStub, 'was called once'); }); + + it('should dispose previous test runner', function() { + var mocha = new Mocha(opts); + var runStub = sandbox.stub(Mocha.Runner.prototype, 'run'); + var disposeStub = sandbox.stub(Mocha.Runner.prototype, 'dispose'); + mocha.run(); + runStub.callArg(0); + mocha.dispose(); + expect(disposeStub, 'was called once'); + }); }); describe('#forbidOnly()', function() { @@ -490,7 +500,7 @@ describe('Mocha', function() { 'to throw', { message: - 'Mocha instance is currently running, cannot start a second test run until this one is done', + 'Mocha instance is currently running tests, cannot start a next test run until this one is done', code: 'ERR_MOCHA_INSTANCE_ALREADY_RUNNING', instance: mocha } @@ -509,8 +519,9 @@ describe('Mocha', function() { 'to throw', { message: - 'Mocha instance is already disposed, cannot start a new test run. Please create a new mocha instance. Be sure to set disable `autoDispose` when you want to reuse the same mocha instance for multiple test runs.', + 'Mocha instance is already disposed, cannot start a new test run. Please create a new mocha instance. Be sure to set disable `cleanReferencesAfterRun` when you want to reuse the same mocha instance for multiple test runs.', code: 'ERR_MOCHA_INSTANCE_ALREADY_DISPOSED', + cleanReferencesAfterRun: true, instance: mocha } ); @@ -529,7 +540,7 @@ describe('Mocha', function() { 'to throw', { message: - 'Mocha instance is already disposed, cannot start a new test run. Please create a new mocha instance. Be sure to set disable `autoDispose` when you want to reuse the same mocha instance for multiple test runs.', + 'Mocha instance is already disposed, cannot start a new test run. Please create a new mocha instance. Be sure to set disable `cleanReferencesAfterRun` when you want to reuse the same mocha instance for multiple test runs.', code: 'ERR_MOCHA_INSTANCE_ALREADY_DISPOSED', instance: mocha } @@ -537,10 +548,10 @@ describe('Mocha', function() { expect(runStub, 'was called once'); }); - it('should allow multiple runs if `autoDispose` is disabled', function() { + it('should allow multiple runs if `cleanReferencesAfterRun` is disabled', function() { var mocha = new Mocha(opts); var runStub = sandbox.stub(Mocha.Runner.prototype, 'run'); - mocha.autoDispose(false); + mocha.cleanReferencesAfterRun(false); mocha.run(); runStub.callArg(0); mocha.run(); @@ -551,13 +562,23 @@ describe('Mocha', function() { it('should reset between runs', function() { var mocha = new Mocha(opts); var runStub = sandbox.stub(Mocha.Runner.prototype, 'run'); - var disposeStub = sandbox.stub(Mocha.Runner.prototype, 'dispose'); var resetStub = sandbox.stub(Mocha.Suite.prototype, 'reset'); - mocha.autoDispose(false); + mocha.cleanReferencesAfterRun(false); mocha.run(); runStub.callArg(0); mocha.run(); expect(resetStub, 'was called once'); + }); + + it('should dispose the previous runner when the next run starts', function() { + var mocha = new Mocha(opts); + var runStub = sandbox.stub(Mocha.Runner.prototype, 'run'); + var disposeStub = sandbox.stub(Mocha.Runner.prototype, 'dispose'); + mocha.cleanReferencesAfterRun(false); + mocha.run(); + runStub.callArg(0); + expect(disposeStub, 'was not called'); + mocha.run(); expect(disposeStub, 'was called once'); }); From 077560b40f37f3df0b8fe619b85fac7a56e3814e Mon Sep 17 00:00:00 2001 From: Nico Jansen Date: Wed, 22 Apr 2020 07:40:06 +0200 Subject: [PATCH 03/17] Allow `unloadFiles` to reset `referencesCleaned`. --- lib/mocha.js | 1 + test/unit/mocha.spec.js | 51 ++++++++++++++++++++++++++--------------- 2 files changed, 33 insertions(+), 19 deletions(-) diff --git a/lib/mocha.js b/lib/mocha.js index acc09aa9c8..dcf5872e7d 100644 --- a/lib/mocha.js +++ b/lib/mocha.js @@ -394,6 +394,7 @@ Mocha.unloadFile = function(file) { */ Mocha.prototype.unloadFiles = function() { this.files.forEach(Mocha.unloadFile); + this._referencesCleaned = false; return this; }; diff --git a/test/unit/mocha.spec.js b/test/unit/mocha.spec.js index 1c3c1b8720..0ba8848259 100644 --- a/test/unit/mocha.spec.js +++ b/test/unit/mocha.spec.js @@ -93,25 +93,6 @@ describe('Mocha', function() { }); }); - describe('#cleanReferencesAfterRun()', function() { - it('should set the _cleanReferencesAfterRun attribute', function() { - var mocha = new Mocha(opts); - mocha.cleanReferencesAfterRun(); - expect(mocha._cleanReferencesAfterRun, 'to be', true); - }); - - it('should set the _cleanReferencesAfterRun attribute to false', function() { - var mocha = new Mocha(opts); - mocha.cleanReferencesAfterRun(false); - expect(mocha._cleanReferencesAfterRun, 'to be', false); - }); - - it('should be chainable', function() { - var mocha = new Mocha(opts); - expect(mocha.cleanReferencesAfterRun(), 'to be', mocha); - }); - }); - describe('#bail()', function() { it('should set the suite._bail to true if there is no arguments', function() { var mocha = new Mocha(opts); @@ -150,6 +131,25 @@ describe('Mocha', function() { }); }); + describe('#cleanReferencesAfterRun()', function() { + it('should set the _cleanReferencesAfterRun attribute', function() { + var mocha = new Mocha(opts); + mocha.cleanReferencesAfterRun(); + expect(mocha._cleanReferencesAfterRun, 'to be', true); + }); + + it('should set the _cleanReferencesAfterRun attribute to false', function() { + var mocha = new Mocha(opts); + mocha.cleanReferencesAfterRun(false); + expect(mocha._cleanReferencesAfterRun, 'to be', false); + }); + + it('should be chainable', function() { + var mocha = new Mocha(opts); + expect(mocha.cleanReferencesAfterRun(), 'to be', mocha); + }); + }); + describe('#color()', function() { it('should set the color option to true', function() { var mocha = new Mocha(opts); @@ -597,4 +597,17 @@ describe('Mocha', function() { }); }); }); + + describe('#unloadFiles()', function() { + it('should reset referencesCleaned and allow for next run', function() { + var mocha = new Mocha(opts); + var runStub = sandbox.stub(Mocha.Runner.prototype, 'run'); + mocha.run(); + runStub.callArg(0); + mocha.unloadFiles(); + expect(function() { + mocha.run(); + }, 'not to throw'); + }); + }); }); From 2784791e44a60622fd865d00d44bea7df81c5e39 Mon Sep 17 00:00:00 2001 From: Nico Jansen Date: Wed, 22 Apr 2020 08:26:31 +0200 Subject: [PATCH 04/17] Complete rename of _cleanReferencesAfterRun --- lib/mocha.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/mocha.js b/lib/mocha.js index dcf5872e7d..6ad5be0aad 100644 --- a/lib/mocha.js +++ b/lib/mocha.js @@ -887,7 +887,11 @@ Mocha.prototype.run = function(fn) { var suite = this.suite; var options = this.options; options.files = this.files; - var runner = new exports.Runner(suite, options.delay, this._autoDispose); + var runner = new exports.Runner( + suite, + options.delay, + this._cleanReferencesAfterRun + ); createStatsCollector(runner); var reporter = new this._reporter(runner, options); runner.checkLeaks = options.checkLeaks === true; From b16def030e208f3f39906589ae040b2850bd5efd Mon Sep 17 00:00:00 2001 From: Nico Jansen Date: Wed, 22 Apr 2020 10:35:52 +0200 Subject: [PATCH 05/17] Add integration test for running a suite multiple times --- ...ple-runs-different-output-suite.fixture.js | 19 +++++ .../multiple-runs-different-output.fixture.js | 22 ++++++ ...previous-is-still-running-suite.fixture.js | 5 ++ ...un-if-previous-is-still-running.fixture.js | 8 ++ test/integration/multiple-runs.spec.js | 74 +++++++++++++++++++ 5 files changed, 128 insertions(+) create mode 100644 test/integration/fixtures/multiple-runs/multiple-runs-different-output-suite.fixture.js create mode 100644 test/integration/fixtures/multiple-runs/multiple-runs-different-output.fixture.js create mode 100644 test/integration/fixtures/multiple-runs/start-second-run-if-previous-is-still-running-suite.fixture.js create mode 100644 test/integration/fixtures/multiple-runs/start-second-run-if-previous-is-still-running.fixture.js create mode 100644 test/integration/multiple-runs.spec.js diff --git a/test/integration/fixtures/multiple-runs/multiple-runs-different-output-suite.fixture.js b/test/integration/fixtures/multiple-runs/multiple-runs-different-output-suite.fixture.js new file mode 100644 index 0000000000..903f661bf9 --- /dev/null +++ b/test/integration/fixtures/multiple-runs/multiple-runs-different-output-suite.fixture.js @@ -0,0 +1,19 @@ +describe('Multiple runs', () => { + + /** + * Shared state! Bad practice, but nice for this test + */ + let i = 0; + + it('should skip, fail and pass respectively', function () { + switch (i++) { + case 0: + this.skip(); + case 1: + throw new Error('Expected error'); + default: + // this is fine ☕ + break; + } + }); +}); diff --git a/test/integration/fixtures/multiple-runs/multiple-runs-different-output.fixture.js b/test/integration/fixtures/multiple-runs/multiple-runs-different-output.fixture.js new file mode 100644 index 0000000000..01951a7710 --- /dev/null +++ b/test/integration/fixtures/multiple-runs/multiple-runs-different-output.fixture.js @@ -0,0 +1,22 @@ +'use strict'; +const Mocha = require('../../../../lib/mocha'); + +const mocha = new Mocha( { reporter: 'json' } ); +if(process.argv.indexOf('--no-clean-references') >= 0) { + mocha.cleanReferencesAfterRun(false); +} +if(process.argv.indexOf('--directly-dispose') >= 0){ + mocha.dispose(); +} +mocha.addFile(require.resolve('./multiple-runs-different-output-suite.fixture.js')); +console.log('['); +mocha.run(() => { + console.log(','); + mocha.run(() => { + console.log(','); + mocha.run(() => { + console.log(']'); + }); + }); +}); + diff --git a/test/integration/fixtures/multiple-runs/start-second-run-if-previous-is-still-running-suite.fixture.js b/test/integration/fixtures/multiple-runs/start-second-run-if-previous-is-still-running-suite.fixture.js new file mode 100644 index 0000000000..a8ecaf76e5 --- /dev/null +++ b/test/integration/fixtures/multiple-runs/start-second-run-if-previous-is-still-running-suite.fixture.js @@ -0,0 +1,5 @@ +describe('slow suite', () => { + it('should be slow', (done) => { + setTimeout(200, done); + }); +}); diff --git a/test/integration/fixtures/multiple-runs/start-second-run-if-previous-is-still-running.fixture.js b/test/integration/fixtures/multiple-runs/start-second-run-if-previous-is-still-running.fixture.js new file mode 100644 index 0000000000..0cc4db3a92 --- /dev/null +++ b/test/integration/fixtures/multiple-runs/start-second-run-if-previous-is-still-running.fixture.js @@ -0,0 +1,8 @@ +'use strict'; +const Mocha = require('../../../../lib/mocha'); + +const mocha = new Mocha( { reporter: 'json' } ); +mocha.addFile(require.resolve('./start-second-run-if-previous-is-still-running-suite.fixture.js')); +mocha.run(); +mocha.run(); + diff --git a/test/integration/multiple-runs.spec.js b/test/integration/multiple-runs.spec.js new file mode 100644 index 0000000000..ccb0f3c892 --- /dev/null +++ b/test/integration/multiple-runs.spec.js @@ -0,0 +1,74 @@ +'use strict'; + +var invokeNode = require('./helpers').invokeNode; + +describe('multiple runs', function(done) { + it('should should be allowed to run multiple times if cleanReferences is turned off', function(done) { + var path = require.resolve( + './fixtures/multiple-runs/multiple-runs-different-output.fixture.js' + ); + invokeNode([path, '--no-clean-references'], function(err, res) { + expect(err, 'to be null'); + expect(res.code, 'to be', 0); + var results = JSON.parse(res.output); + expect(results, 'to have length', 3); + expect(results[0].pending, 'to have length', 1); + expect(results[0].failures, 'to have length', 0); + expect(results[0].passes, 'to have length', 0); + expect(results[1].pending, 'to have length', 0); + expect(results[1].failures, 'to have length', 1); + expect(results[1].passes, 'to have length', 0); + expect(results[2].pending, 'to have length', 0); + expect(results[2].failures, 'to have length', 0); + expect(results[2].passes, 'to have length', 1); + done(); + }); + }); + + it('should should not be allowed if cleanReferences is true', function(done) { + var path = require.resolve( + './fixtures/multiple-runs/multiple-runs-different-output.fixture.js' + ); + invokeNode( + [path], + function(err, res) { + expect(err, 'to be null'); + expect(res.code, 'not to be', 0); + expect(res.output, 'to contain', 'ERR_MOCHA_INSTANCE_ALREADY_DISPOSED'); + done(); + }, + {stdio: ['ignore', 'pipe', 'pipe']} + ); + }); + + it('should should not be allowed if the instance is disposed', function(done) { + var path = require.resolve( + './fixtures/multiple-runs/multiple-runs-different-output.fixture.js' + ); + invokeNode( + [path, '--directly-dispose'], + function(err, res) { + expect(err, 'to be null'); + expect(res.code, 'not to be', 0); + expect(res.output, 'to contain', 'ERR_MOCHA_INSTANCE_ALREADY_DISPOSED'); + done(); + }, + {stdio: ['ignore', 'pipe', 'pipe']} + ); + }); + + it('should should not allowed to run while a previous run is in progress', function(done) { + var path = require.resolve( + './fixtures/multiple-runs/start-second-run-if-previous-is-still-running.fixture' + ); + invokeNode( + [path], + function(err, res) { + expect(err, 'to be null'); + expect(res.output, 'to contain', 'ERR_MOCHA_INSTANCE_ALREADY_RUNNING'); + done(); + }, + {stdio: ['ignore', 'pipe', 'pipe']} + ); + }); +}); From 201bd80785cd42b62c42454c50a95a2e4bdfbff1 Mon Sep 17 00:00:00 2001 From: Nico Jansen Date: Wed, 22 Apr 2020 10:54:00 +0200 Subject: [PATCH 06/17] improve api docs --- lib/mocha.js | 4 +++- test/integration/multiple-runs.spec.js | 8 ++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/mocha.js b/lib/mocha.js index 6ad5be0aad..5322b1aa8d 100644 --- a/lib/mocha.js +++ b/lib/mocha.js @@ -496,8 +496,10 @@ Mocha.prototype.checkLeaks = function(checkLeaks) { /** * Enables or disables whether or not to dispose after each test run. + * Disable this to ensure you can run the test suite multiple times. + * If disabled, be sure to dispose mocha when you're done to prevent memory leaks. * @public - * @see [Mocha.cleanReferencesAfterRun](/#cleanReferencesAfterRun) + * @see [Mocha.dispose](/#dispose) * @default true * @param {boolean} cleanReferencesAfterRun * @return {Mocha} this diff --git a/test/integration/multiple-runs.spec.js b/test/integration/multiple-runs.spec.js index ccb0f3c892..cde1b4393e 100644 --- a/test/integration/multiple-runs.spec.js +++ b/test/integration/multiple-runs.spec.js @@ -3,7 +3,7 @@ var invokeNode = require('./helpers').invokeNode; describe('multiple runs', function(done) { - it('should should be allowed to run multiple times if cleanReferences is turned off', function(done) { + it('should be allowed to run multiple times if cleanReferences is turned off', function(done) { var path = require.resolve( './fixtures/multiple-runs/multiple-runs-different-output.fixture.js' ); @@ -25,7 +25,7 @@ describe('multiple runs', function(done) { }); }); - it('should should not be allowed if cleanReferences is true', function(done) { + it('should not be allowed if cleanReferences is true', function(done) { var path = require.resolve( './fixtures/multiple-runs/multiple-runs-different-output.fixture.js' ); @@ -41,7 +41,7 @@ describe('multiple runs', function(done) { ); }); - it('should should not be allowed if the instance is disposed', function(done) { + it('should not be allowed if the instance is disposed', function(done) { var path = require.resolve( './fixtures/multiple-runs/multiple-runs-different-output.fixture.js' ); @@ -57,7 +57,7 @@ describe('multiple runs', function(done) { ); }); - it('should should not allowed to run while a previous run is in progress', function(done) { + it('should not allowed to run while a previous run is in progress', function(done) { var path = require.resolve( './fixtures/multiple-runs/start-second-run-if-previous-is-still-running.fixture' ); From 184911c984dd9478bc3fb87951cbd606c0dc8fbb Mon Sep 17 00:00:00 2001 From: Nico Jansen Date: Wed, 22 Apr 2020 10:59:27 +0200 Subject: [PATCH 07/17] Docs: fix dead link --- lib/mocha.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mocha.js b/lib/mocha.js index 5322b1aa8d..8cca6b53d2 100644 --- a/lib/mocha.js +++ b/lib/mocha.js @@ -499,7 +499,7 @@ Mocha.prototype.checkLeaks = function(checkLeaks) { * Disable this to ensure you can run the test suite multiple times. * If disabled, be sure to dispose mocha when you're done to prevent memory leaks. * @public - * @see [Mocha.dispose](/#dispose) + * @see {@link Mocha#dispose} * @default true * @param {boolean} cleanReferencesAfterRun * @return {Mocha} this From e21359e6eea5a2cb37ddab8c525d245e7361a4c1 Mon Sep 17 00:00:00 2001 From: Nico Jansen Date: Wed, 22 Apr 2020 11:17:53 +0200 Subject: [PATCH 08/17] Make sure tests run on older node versions --- .../multiple-runs-different-output.fixture.js | 29 ++++++++++++------- ...un-if-previous-is-still-running.fixture.js | 8 +++-- test/integration/multiple-runs.spec.js | 2 +- 3 files changed, 26 insertions(+), 13 deletions(-) diff --git a/test/integration/fixtures/multiple-runs/multiple-runs-different-output.fixture.js b/test/integration/fixtures/multiple-runs/multiple-runs-different-output.fixture.js index 01951a7710..a73f85380f 100644 --- a/test/integration/fixtures/multiple-runs/multiple-runs-different-output.fixture.js +++ b/test/integration/fixtures/multiple-runs/multiple-runs-different-output.fixture.js @@ -1,22 +1,31 @@ 'use strict'; const Mocha = require('../../../../lib/mocha'); -const mocha = new Mocha( { reporter: 'json' } ); -if(process.argv.indexOf('--no-clean-references') >= 0) { +const mocha = new Mocha({ reporter: 'json' }); +if (process.argv.indexOf('--no-clean-references') >= 0) { mocha.cleanReferencesAfterRun(false); } -if(process.argv.indexOf('--directly-dispose') >= 0){ +if (process.argv.indexOf('--directly-dispose') >= 0) { mocha.dispose(); } mocha.addFile(require.resolve('./multiple-runs-different-output-suite.fixture.js')); console.log('['); -mocha.run(() => { - console.log(','); +try { mocha.run(() => { console.log(','); - mocha.run(() => { - console.log(']'); - }); + try { + mocha.run(() => { + console.log(','); + mocha.run(() => { + console.log(']'); + }); + }); + } catch (err) { + console.error(err.code); + throw err; + } }); -}); - +} catch (err) { + console.error(err.code); + throw err; +} diff --git a/test/integration/fixtures/multiple-runs/start-second-run-if-previous-is-still-running.fixture.js b/test/integration/fixtures/multiple-runs/start-second-run-if-previous-is-still-running.fixture.js index 0cc4db3a92..1b031334b1 100644 --- a/test/integration/fixtures/multiple-runs/start-second-run-if-previous-is-still-running.fixture.js +++ b/test/integration/fixtures/multiple-runs/start-second-run-if-previous-is-still-running.fixture.js @@ -1,8 +1,12 @@ 'use strict'; const Mocha = require('../../../../lib/mocha'); -const mocha = new Mocha( { reporter: 'json' } ); +const mocha = new Mocha({ reporter: 'json' }); mocha.addFile(require.resolve('./start-second-run-if-previous-is-still-running-suite.fixture.js')); mocha.run(); -mocha.run(); +try { + mocha.run(); +} catch (err) { + console.error(err.code); +} diff --git a/test/integration/multiple-runs.spec.js b/test/integration/multiple-runs.spec.js index cde1b4393e..be935ca583 100644 --- a/test/integration/multiple-runs.spec.js +++ b/test/integration/multiple-runs.spec.js @@ -2,7 +2,7 @@ var invokeNode = require('./helpers').invokeNode; -describe('multiple runs', function(done) { +describe.only('multiple runs', function(done) { it('should be allowed to run multiple times if cleanReferences is turned off', function(done) { var path = require.resolve( './fixtures/multiple-runs/multiple-runs-different-output.fixture.js' From b2903db7618908ef3f4da2c73dbb053d466076a3 Mon Sep 17 00:00:00 2001 From: Nico Jansen Date: Wed, 22 Apr 2020 11:28:07 +0200 Subject: [PATCH 09/17] =?UTF-8?q?Remove=20`.only`=20=F0=9F=98=85?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/integration/multiple-runs.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/multiple-runs.spec.js b/test/integration/multiple-runs.spec.js index be935ca583..cde1b4393e 100644 --- a/test/integration/multiple-runs.spec.js +++ b/test/integration/multiple-runs.spec.js @@ -2,7 +2,7 @@ var invokeNode = require('./helpers').invokeNode; -describe.only('multiple runs', function(done) { +describe('multiple runs', function(done) { it('should be allowed to run multiple times if cleanReferences is turned off', function(done) { var path = require.resolve( './fixtures/multiple-runs/multiple-runs-different-output.fixture.js' From e6d1a106d6fcf725981935c00edbcb7cb63fd80f Mon Sep 17 00:00:00 2001 From: Nico Jansen Date: Wed, 22 Apr 2020 11:47:18 +0200 Subject: [PATCH 10/17] Implement `process.listenerCount` in the browser --- browser-entry.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/browser-entry.js b/browser-entry.js index 3e9cbbaf90..114d2f7213 100644 --- a/browser-entry.js +++ b/browser-entry.js @@ -52,6 +52,17 @@ process.removeListener = function(e, fn) { } }; +/** + * Implements listenerCount for 'uncaughtException'. + */ + +process.listenerCount = function(name) { + if (name === 'uncaughtException') { + return uncaughtExceptionHandlers.length; + } + return 0; +}; + /** * Implements uncaughtException listener. */ From ae8b458f0a4c81d27958186a15ebe5257cc137f5 Mon Sep 17 00:00:00 2001 From: Nico Jansen Date: Fri, 24 Apr 2020 18:02:53 +0200 Subject: [PATCH 11/17] Implement mocha states in a finite state machine --- lib/errors.js | 2 +- lib/mocha.js | 83 +++++++++++++++++++++++++++++++++++++++------------ 2 files changed, 65 insertions(+), 20 deletions(-) diff --git a/lib/errors.js b/lib/errors.js index 29ee916e63..a85c8c24b9 100644 --- a/lib/errors.js +++ b/lib/errors.js @@ -152,7 +152,7 @@ function createInvalidPluginError(message, pluginType, pluginId) { /** * Creates an error object to be thrown when a mocha object's `run` method is executed while it is already disposed. * @param {string} message The error message to be displayed. - * @param {boolean} cleanReferencesAfterRun the value of `autoDispose` + * @param {boolean} cleanReferencesAfterRun the value of `cleanReferencesAfterRun` * @param {Mocha} instance the mocha instance that throw this error */ function createMochaInstanceAlreadyDisposedError( diff --git a/lib/mocha.js b/lib/mocha.js index 8cca6b53d2..ed2c765e1b 100644 --- a/lib/mocha.js +++ b/lib/mocha.js @@ -29,6 +29,30 @@ var sQuote = utils.sQuote; exports = module.exports = Mocha; +/** + * A Mocha instance is a finite state machine. + * These are the states it can be in. + */ +var mochaStates = utils.defineConstants({ + /** + * Initial state of the mocha instance + */ + INIT: 'init', + /** + * Mocha instance is running tests + */ + RUNNING: 'running', + /** + * Mocha instance is done running tests and references to test functions and hooks are cleaned. + * You can reset this state by unloading the test files. + */ + REFERENCES_CLEANED: 'referencesCleaned', + /** + * Mocha instance is disposed and can no longer be used. + */ + DISPOSED: 'disposed' +}); + /** * To require local UIs and reporters when running in node. */ @@ -393,8 +417,16 @@ Mocha.unloadFile = function(file) { * @chainable */ Mocha.prototype.unloadFiles = function() { + if (this._state === mochaStates.DISPOSED) { + throw createMochaInstanceAlreadyDisposedError( + 'Mocha instance is already disposed, it cannot be used again.', + this._cleanReferencesAfterRun, + this + ); + } + this.files.forEach(Mocha.unloadFile); - this._referencesCleaned = false; + this._state = mochaStates.INIT; return this; }; @@ -517,12 +549,12 @@ Mocha.prototype.cleanReferencesAfterRun = function(cleanReferencesAfterRun) { * @returns {void} */ Mocha.prototype.dispose = function() { - if (this._runIsInProgress) { + if (this._state === mochaStates.RUNNING) { throw createMochaInstanceAlreadyRunningError( 'Cannot dispose while the mocha instance is still running tests.' ); } - this._isDisposed = true; + this._state = mochaStates.DISPOSED; this._previousRunner && this._previousRunner.dispose(); this.suite.dispose(); }; @@ -824,6 +856,28 @@ Mocha.prototype.forbidPending = function(forbidPending) { return this; }; +/** + * Throws an error if mocha is in the wrong state to be able to transition to a "running" state. + */ +Mocha.prototype._guardRunningStateTransition = function() { + if (this._state === mochaStates.RUNNING) { + throw createMochaInstanceAlreadyRunningError( + 'Mocha instance is currently running tests, cannot start a next test run until this one is done', + this + ); + } + if ( + this._state === mochaStates.DISPOSED || + this._state === mochaStates.REFERENCES_CLEANED + ) { + throw createMochaInstanceAlreadyDisposedError( + 'Mocha instance is already disposed, cannot start a new test run. Please create a new mocha instance. Be sure to set disable `cleanReferencesAfterRun` when you want to reuse the same mocha instance for multiple test runs.', + this._cleanReferencesAfterRun, + this + ); + } +}; + /** * Mocha version as specified by "package.json". * @@ -864,20 +918,8 @@ Object.defineProperty(Mocha.prototype, 'version', { * mocha.run(failures => process.exitCode = failures ? 1 : 0); */ Mocha.prototype.run = function(fn) { - if (this._runIsInProgress) { - throw createMochaInstanceAlreadyRunningError( - 'Mocha instance is currently running tests, cannot start a next test run until this one is done', - this - ); - } - this._runIsInProgress = true; - if (this._isDisposed || this._referencesCleaned) { - throw createMochaInstanceAlreadyDisposedError( - 'Mocha instance is already disposed, cannot start a new test run. Please create a new mocha instance. Be sure to set disable `cleanReferencesAfterRun` when you want to reuse the same mocha instance for multiple test runs.', - this._cleanReferencesAfterRun, - this - ); - } + this._guardRunningStateTransition(); + this._state = mochaStates.RUNNING; if (this._previousRunner) { this._previousRunner.dispose(); this.suite.reset(); @@ -919,8 +961,11 @@ Mocha.prototype.run = function(fn) { function done(failures) { self._previousRunner = runner; - self._runIsInProgress = false; - self._referencesCleaned = self._cleanReferencesAfterRun; + if (self._cleanReferencesAfterRun) { + self._state = mochaStates.REFERENCES_CLEANED; + } else { + self._state = mochaStates.INIT; + } fn = fn || utils.noop; if (reporter.done) { reporter.done(failures, fn); From 87e9f94b0a654fb0350dcd3fd364011e2c6d240a Mon Sep 17 00:00:00 2001 From: Nico Jansen Date: Fri, 24 Apr 2020 18:04:31 +0200 Subject: [PATCH 12/17] Fix some small remarks --- lib/mocha.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/mocha.js b/lib/mocha.js index ed2c765e1b..d5779fe3ec 100644 --- a/lib/mocha.js +++ b/lib/mocha.js @@ -532,7 +532,6 @@ Mocha.prototype.checkLeaks = function(checkLeaks) { * If disabled, be sure to dispose mocha when you're done to prevent memory leaks. * @public * @see {@link Mocha#dispose} - * @default true * @param {boolean} cleanReferencesAfterRun * @return {Mocha} this * @chainable @@ -546,7 +545,6 @@ Mocha.prototype.cleanReferencesAfterRun = function(cleanReferencesAfterRun) { * Manually dispose this mocha instance. Mark this instance as `disposed` and unable to run more tests. * It also removes function references to tests functions and hooks, so variables trapped in closures can be cleaned by the garbage collector. * @public - * @returns {void} */ Mocha.prototype.dispose = function() { if (this._state === mochaStates.RUNNING) { From ed82346c87dcc5fdd63fc9a25a79f0c474f525f4 Mon Sep 17 00:00:00 2001 From: Nico Jansen Date: Fri, 24 Apr 2020 18:26:30 +0200 Subject: [PATCH 13/17] Make integration tests more damp --- .../multiple-runs/clean-references.fixture.js | 6 ++++ .../fixtures/multiple-runs/dispose.fixture.js | 6 ++++ .../multiple-runs-different-output.fixture.js | 31 ------------------- ...ns-with-different-output-suite.fixture.js} | 0 .../multiple-runs/run-thrice-helper.js | 24 ++++++++++++++ .../multiple-runs/run-thrice.fixture.js | 6 ++++ test/integration/multiple-runs.spec.js | 10 +++--- 7 files changed, 46 insertions(+), 37 deletions(-) create mode 100644 test/integration/fixtures/multiple-runs/clean-references.fixture.js create mode 100644 test/integration/fixtures/multiple-runs/dispose.fixture.js delete mode 100644 test/integration/fixtures/multiple-runs/multiple-runs-different-output.fixture.js rename test/integration/fixtures/multiple-runs/{multiple-runs-different-output-suite.fixture.js => multiple-runs-with-different-output-suite.fixture.js} (100%) create mode 100644 test/integration/fixtures/multiple-runs/run-thrice-helper.js create mode 100644 test/integration/fixtures/multiple-runs/run-thrice.fixture.js diff --git a/test/integration/fixtures/multiple-runs/clean-references.fixture.js b/test/integration/fixtures/multiple-runs/clean-references.fixture.js new file mode 100644 index 0000000000..2f204a0b74 --- /dev/null +++ b/test/integration/fixtures/multiple-runs/clean-references.fixture.js @@ -0,0 +1,6 @@ +'use strict'; +const Mocha = require('../../../../lib/mocha'); + +const mocha = new Mocha({ reporter: 'json' }); +mocha.cleanReferencesAfterRun(true); +require('./run-thrice-helper')(mocha); diff --git a/test/integration/fixtures/multiple-runs/dispose.fixture.js b/test/integration/fixtures/multiple-runs/dispose.fixture.js new file mode 100644 index 0000000000..c0d3c4d7ba --- /dev/null +++ b/test/integration/fixtures/multiple-runs/dispose.fixture.js @@ -0,0 +1,6 @@ +'use strict'; +const Mocha = require('../../../../lib/mocha'); + +const mocha = new Mocha({ reporter: 'json' }); +mocha.dispose(); +require('./run-thrice-helper')(mocha); diff --git a/test/integration/fixtures/multiple-runs/multiple-runs-different-output.fixture.js b/test/integration/fixtures/multiple-runs/multiple-runs-different-output.fixture.js deleted file mode 100644 index a73f85380f..0000000000 --- a/test/integration/fixtures/multiple-runs/multiple-runs-different-output.fixture.js +++ /dev/null @@ -1,31 +0,0 @@ -'use strict'; -const Mocha = require('../../../../lib/mocha'); - -const mocha = new Mocha({ reporter: 'json' }); -if (process.argv.indexOf('--no-clean-references') >= 0) { - mocha.cleanReferencesAfterRun(false); -} -if (process.argv.indexOf('--directly-dispose') >= 0) { - mocha.dispose(); -} -mocha.addFile(require.resolve('./multiple-runs-different-output-suite.fixture.js')); -console.log('['); -try { - mocha.run(() => { - console.log(','); - try { - mocha.run(() => { - console.log(','); - mocha.run(() => { - console.log(']'); - }); - }); - } catch (err) { - console.error(err.code); - throw err; - } - }); -} catch (err) { - console.error(err.code); - throw err; -} diff --git a/test/integration/fixtures/multiple-runs/multiple-runs-different-output-suite.fixture.js b/test/integration/fixtures/multiple-runs/multiple-runs-with-different-output-suite.fixture.js similarity index 100% rename from test/integration/fixtures/multiple-runs/multiple-runs-different-output-suite.fixture.js rename to test/integration/fixtures/multiple-runs/multiple-runs-with-different-output-suite.fixture.js diff --git a/test/integration/fixtures/multiple-runs/run-thrice-helper.js b/test/integration/fixtures/multiple-runs/run-thrice-helper.js new file mode 100644 index 0000000000..58f2c9de5e --- /dev/null +++ b/test/integration/fixtures/multiple-runs/run-thrice-helper.js @@ -0,0 +1,24 @@ +module.exports = function (mocha) { + mocha.addFile(require.resolve('./multiple-runs-with-different-output-suite.fixture.js')); + console.log('['); + try { + mocha.run(() => { + console.log(','); + try { + mocha.run(() => { + console.log(','); + mocha.run(() => { + console.log(']'); + }); + }); + } catch (err) { + console.error(err.code); + throw err; + } + }); + } catch (err) { + console.error(err.code); + throw err; + } + +} diff --git a/test/integration/fixtures/multiple-runs/run-thrice.fixture.js b/test/integration/fixtures/multiple-runs/run-thrice.fixture.js new file mode 100644 index 0000000000..3c63ec3725 --- /dev/null +++ b/test/integration/fixtures/multiple-runs/run-thrice.fixture.js @@ -0,0 +1,6 @@ +'use strict'; +const Mocha = require('../../../../lib/mocha'); + +const mocha = new Mocha({ reporter: 'json' }); +mocha.cleanReferencesAfterRun(false); +require('./run-thrice-helper')(mocha); diff --git a/test/integration/multiple-runs.spec.js b/test/integration/multiple-runs.spec.js index cde1b4393e..9c8040e2b8 100644 --- a/test/integration/multiple-runs.spec.js +++ b/test/integration/multiple-runs.spec.js @@ -5,9 +5,9 @@ var invokeNode = require('./helpers').invokeNode; describe('multiple runs', function(done) { it('should be allowed to run multiple times if cleanReferences is turned off', function(done) { var path = require.resolve( - './fixtures/multiple-runs/multiple-runs-different-output.fixture.js' + './fixtures/multiple-runs/run-thrice.fixture.js' ); - invokeNode([path, '--no-clean-references'], function(err, res) { + invokeNode([path], function(err, res) { expect(err, 'to be null'); expect(res.code, 'to be', 0); var results = JSON.parse(res.output); @@ -27,7 +27,7 @@ describe('multiple runs', function(done) { it('should not be allowed if cleanReferences is true', function(done) { var path = require.resolve( - './fixtures/multiple-runs/multiple-runs-different-output.fixture.js' + './fixtures/multiple-runs/clean-references.fixture.js' ); invokeNode( [path], @@ -42,9 +42,7 @@ describe('multiple runs', function(done) { }); it('should not be allowed if the instance is disposed', function(done) { - var path = require.resolve( - './fixtures/multiple-runs/multiple-runs-different-output.fixture.js' - ); + var path = require.resolve('./fixtures/multiple-runs/dispose.fixture.js'); invokeNode( [path, '--directly-dispose'], function(err, res) { From 7d55b7ac82bff059b6b7e8efec9639e66a13294e Mon Sep 17 00:00:00 2001 From: Nico Jansen Date: Fri, 24 Apr 2020 18:26:59 +0200 Subject: [PATCH 14/17] Keep `Runner` api backward compatible --- lib/runner.js | 26 ++++++++++++++++---------- test/unit/runnable.spec.js | 4 ++-- test/unit/runner.spec.js | 9 ++++++--- 3 files changed, 24 insertions(+), 15 deletions(-) diff --git a/lib/runner.js b/lib/runner.js index f152b6837a..d87c41820d 100644 --- a/lib/runner.js +++ b/lib/runner.js @@ -121,23 +121,29 @@ module.exports = Runner; * @extends external:EventEmitter * @public * @class - * @param {Suite} suite Root suite - * @param {boolean} [delay] Whether to delay execution of root suite until ready. - * @param {boolean} [cleanReferencesAfterRun] Whether to clean references to test fns and hooks when a suite is done. + * @param {Suite} suite - Root suite + * @param {Object|boolean} [opts] - Options. If `boolean`, whether or not to delay execution of root suite until ready (for backwards compatibility). + * @param {boolean} [opts.delay] - Whether to delay execution of root suite until ready. + * @param {boolean} [opts.cleanReferencesAfterRun] - Whether to clean references to test fns and hooks when a suite is done. */ -function Runner(suite, delay, cleanReferencesAfterRun) { +function Runner(suite, opts) { + if (opts === undefined) { + opts = {}; + } + if (typeof opts === 'boolean') { + this._delay = opts; + opts = {}; + } else { + this._delay = opts.delay; + } var self = this; this._globals = []; this._abort = false; - this._delay = delay; this.suite = suite; this.started = false; - this._cleanReferencesAfterRun = cleanReferencesAfterRun; + this._opts = opts; this.total = suite.total(); this.failures = 0; - /** - * @types {[[EventEmitter, string, function]]} - */ this._eventListeners = []; this.on(constants.EVENT_TEST_END, function(test) { if (test.type === 'test' && test.retriedTest() && test.parent) { @@ -973,7 +979,7 @@ Runner.prototype.run = function(fn) { } // references cleanup to avoid memory leaks - if (this._cleanReferencesAfterRun) { + if (this._opts.cleanReferencesAfterRun) { this.on(constants.EVENT_SUITE_END, function(suite) { suite.cleanReferences(); }); diff --git a/test/unit/runnable.spec.js b/test/unit/runnable.spec.js index afa77d407d..33cc661ab1 100644 --- a/test/unit/runnable.spec.js +++ b/test/unit/runnable.spec.js @@ -191,9 +191,9 @@ describe('Runnable(title, fn)', function() { this._currentRetry = 5; this.pending = true; run.reset(); - expect(run.timedOut, 'to be', false); + expect(run.timedOut, 'to be false'); expect(run._currentRetry, 'to be', 0); - expect(run.pending, 'to be', false); + expect(run.pending, 'to be false'); }); }); diff --git a/test/unit/runner.spec.js b/test/unit/runner.spec.js index 6aed62763a..84bc5bf115 100644 --- a/test/unit/runner.spec.js +++ b/test/unit/runner.spec.js @@ -25,7 +25,7 @@ describe('Runner', function() { beforeEach(function() { suite = new Suite('Suite', 'root'); - runner = new Runner(suite, undefined, true); + runner = new Runner(suite, {cleanReferencesAfterRun: true}); runner.checkLeaks = true; sandbox = sinon.createSandbox(); }); @@ -461,7 +461,7 @@ describe('Runner', function() { }); it('should clean references after a run', function() { - runner = new Runner(suite, false, true); + runner = new Runner(suite, {delay: false, cleanReferencesAfterRun: true}); var cleanReferencesStub = sandbox.stub(suite, 'cleanReferences'); runner.run(); runner.emit(EVENT_SUITE_END, suite); @@ -469,7 +469,10 @@ describe('Runner', function() { }); it('should not clean references after a run when `cleanReferencesAfterRun` is `false`', function() { - runner = new Runner(suite, false, false); + runner = new Runner(suite, { + delay: false, + cleanReferencesAfterRun: false + }); var cleanReferencesStub = sandbox.stub(suite, 'cleanReferences'); runner.run(); runner.emit(EVENT_SUITE_END, suite); From cfb06e0443a1fdec25d9a9a5aea05f10e0c540c1 Mon Sep 17 00:00:00 2001 From: Nico Jansen Date: Fri, 24 Apr 2020 19:07:03 +0200 Subject: [PATCH 15/17] Unload files when disposed --- lib/mocha.js | 12 ++++++------ test/unit/mocha.spec.js | 19 +++++++++++++++++++ 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/lib/mocha.js b/lib/mocha.js index d5779fe3ec..c3ef01a2b1 100644 --- a/lib/mocha.js +++ b/lib/mocha.js @@ -552,9 +552,10 @@ Mocha.prototype.dispose = function() { 'Cannot dispose while the mocha instance is still running tests.' ); } - this._state = mochaStates.DISPOSED; + this.unloadFiles(); this._previousRunner && this._previousRunner.dispose(); this.suite.dispose(); + this._state = mochaStates.DISPOSED; }; /** @@ -929,11 +930,10 @@ Mocha.prototype.run = function(fn) { var suite = this.suite; var options = this.options; options.files = this.files; - var runner = new exports.Runner( - suite, - options.delay, - this._cleanReferencesAfterRun - ); + var runner = new exports.Runner(suite, { + delay: options.delay, + cleanReferencesAfterRun: this._cleanReferencesAfterRun + }); createStatsCollector(runner); var reporter = new this._reporter(runner, options); runner.checkLeaks = options.checkLeaks === true; diff --git a/test/unit/mocha.spec.js b/test/unit/mocha.spec.js index 0ba8848259..2c0da424e0 100644 --- a/test/unit/mocha.spec.js +++ b/test/unit/mocha.spec.js @@ -231,6 +231,13 @@ describe('Mocha', function() { mocha.dispose(); expect(disposeStub, 'was called once'); }); + + it('should unload the files', function() { + var mocha = new Mocha(opts); + var unloadFilesStub = sandbox.stub(mocha, 'unloadFiles'); + mocha.dispose(); + expect(unloadFilesStub, 'was called once'); + }); }); describe('#forbidOnly()', function() { @@ -609,5 +616,17 @@ describe('Mocha', function() { mocha.run(); }, 'not to throw'); }); + + it('should not be allowed when the current instance is already disposed', function() { + var mocha = new Mocha(opts); + mocha.dispose(); + expect( + function() { + mocha.unloadFiles(); + }, + 'to throw', + 'Mocha instance is already disposed, it cannot be used again.' + ); + }); }); }); From 9bbb7a718d8a920ddbcd606de66ba4f0d2103ca3 Mon Sep 17 00:00:00 2001 From: Nico Jansen Date: Sun, 26 Apr 2020 10:58:58 +0200 Subject: [PATCH 16/17] Runnable.reset should also reset `err` and `state` --- lib/runnable.js | 2 ++ test/unit/runnable.spec.js | 9 +++++++-- test/unit/test.spec.js | 2 -- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/lib/runnable.js b/lib/runnable.js index d8b63109b1..0bdcf97dd5 100644 --- a/lib/runnable.js +++ b/lib/runnable.js @@ -52,6 +52,8 @@ Runnable.prototype.reset = function() { this.timedOut = false; this._currentRetry = 0; this.pending = false; + delete this.state; + delete this.err; }; /** diff --git a/test/unit/runnable.spec.js b/test/unit/runnable.spec.js index 33cc661ab1..f19451e598 100644 --- a/test/unit/runnable.spec.js +++ b/test/unit/runnable.spec.js @@ -188,12 +188,17 @@ describe('Runnable(title, fn)', function() { it('should reset current run state', function() { run.timedOut = true; - this._currentRetry = 5; - this.pending = true; + run._currentRetry = 5; + run.pending = true; + run.err = new Error(); + run.state = 'error'; + run.reset(); expect(run.timedOut, 'to be false'); expect(run._currentRetry, 'to be', 0); expect(run.pending, 'to be false'); + expect(run.err, 'to be undefined'); + expect(run.state, 'to be undefined'); }); }); diff --git a/test/unit/test.spec.js b/test/unit/test.spec.js index a0c6d82284..e828703bb3 100644 --- a/test/unit/test.spec.js +++ b/test/unit/test.spec.js @@ -81,10 +81,8 @@ describe('Test', function() { }); it('should reset the run state', function() { - this._test.state = 'pending'; this._test.pending = true; this._test.reset(); - expect(this._test, 'not to have property', 'state'); expect(this._test.pending, 'to be', false); }); From fba01827032d705602b1ee374456788960a5fa06 Mon Sep 17 00:00:00 2001 From: Nico Jansen Date: Sun, 26 Apr 2020 11:01:31 +0200 Subject: [PATCH 17/17] Also reset hooks --- lib/hook.js | 8 ++++ lib/suite.js | 15 ++++--- ...ns-with-flaky-before-each-suite.fixture.js | 18 ++++++++ ...ple-runs-with-flaky-before-each.fixture.js | 13 ++++++ test/integration/multiple-runs.spec.js | 19 +++++++- test/unit/hook.spec.js | 44 +++++++++++++++++++ test/unit/suite.spec.js | 18 ++++++++ 7 files changed, 128 insertions(+), 7 deletions(-) create mode 100644 test/integration/fixtures/multiple-runs/multiple-runs-with-flaky-before-each-suite.fixture.js create mode 100644 test/integration/fixtures/multiple-runs/multiple-runs-with-flaky-before-each.fixture.js create mode 100644 test/unit/hook.spec.js diff --git a/lib/hook.js b/lib/hook.js index 71440d23d0..6560715fc5 100644 --- a/lib/hook.js +++ b/lib/hook.js @@ -27,6 +27,14 @@ function Hook(title, fn) { */ inherits(Hook, Runnable); +/** + * Resets the state for a next run. + */ +Hook.prototype.reset = function() { + Runnable.prototype.reset.call(this); + delete this._error; +}; + /** * Get or set the test `err`. * diff --git a/lib/suite.js b/lib/suite.js index c82f672072..db393bbaeb 100644 --- a/lib/suite.js +++ b/lib/suite.js @@ -97,12 +97,15 @@ inherits(Suite, EventEmitter); */ Suite.prototype.reset = function() { this.delayed = false; - this.suites.forEach(function(suite) { - suite.reset(); - }); - this.tests.forEach(function(test) { - test.reset(); - }); + function doReset(thingToReset) { + thingToReset.reset(); + } + this.suites.forEach(doReset); + this.tests.forEach(doReset); + this._beforeEach.forEach(doReset); + this._afterEach.forEach(doReset); + this._beforeAll.forEach(doReset); + this._afterAll.forEach(doReset); }; /** diff --git a/test/integration/fixtures/multiple-runs/multiple-runs-with-flaky-before-each-suite.fixture.js b/test/integration/fixtures/multiple-runs/multiple-runs-with-flaky-before-each-suite.fixture.js new file mode 100644 index 0000000000..7863fb223e --- /dev/null +++ b/test/integration/fixtures/multiple-runs/multiple-runs-with-flaky-before-each-suite.fixture.js @@ -0,0 +1,18 @@ +describe('Multiple runs', () => { + + /** + * Shared state! Bad practice, but nice for this test + */ + let i = 0; + + beforeEach(function () { + if (i++ === 0) { + throw new Error('Expected error for this test'); + } + }); + + + it('should be a dummy test', function () { + // this is fine ☕ + }); +}); diff --git a/test/integration/fixtures/multiple-runs/multiple-runs-with-flaky-before-each.fixture.js b/test/integration/fixtures/multiple-runs/multiple-runs-with-flaky-before-each.fixture.js new file mode 100644 index 0000000000..1a4707705f --- /dev/null +++ b/test/integration/fixtures/multiple-runs/multiple-runs-with-flaky-before-each.fixture.js @@ -0,0 +1,13 @@ +'use strict'; +const Mocha = require('../../../../lib/mocha'); + +const mocha = new Mocha({ reporter: 'json' }); +mocha.cleanReferencesAfterRun(false); +mocha.addFile(require.resolve('./multiple-runs-with-flaky-before-each-suite.fixture.js')); +console.log('['); +mocha.run(() => { + console.log(','); + mocha.run(() => { + console.log(']'); + }); +}); diff --git a/test/integration/multiple-runs.spec.js b/test/integration/multiple-runs.spec.js index 9c8040e2b8..61d672d4b2 100644 --- a/test/integration/multiple-runs.spec.js +++ b/test/integration/multiple-runs.spec.js @@ -55,7 +55,7 @@ describe('multiple runs', function(done) { ); }); - it('should not allowed to run while a previous run is in progress', function(done) { + it('should not be allowed to run while a previous run is in progress', function(done) { var path = require.resolve( './fixtures/multiple-runs/start-second-run-if-previous-is-still-running.fixture' ); @@ -69,4 +69,21 @@ describe('multiple runs', function(done) { {stdio: ['ignore', 'pipe', 'pipe']} ); }); + + it('should reset the hooks between runs', function(done) { + var path = require.resolve( + './fixtures/multiple-runs/multiple-runs-with-flaky-before-each.fixture' + ); + invokeNode([path], function(err, res) { + expect(err, 'to be null'); + expect(res.code, 'to be', 0); + var results = JSON.parse(res.output); + expect(results, 'to have length', 2); + expect(results[0].failures, 'to have length', 1); + expect(results[0].passes, 'to have length', 0); + expect(results[1].passes, 'to have length', 1); + expect(results[1].failures, 'to have length', 0); + done(); + }); + }); }); diff --git a/test/unit/hook.spec.js b/test/unit/hook.spec.js new file mode 100644 index 0000000000..b02a6c5120 --- /dev/null +++ b/test/unit/hook.spec.js @@ -0,0 +1,44 @@ +'use strict'; +var sinon = require('sinon'); +var Mocha = require('../../lib/mocha'); +var Hook = Mocha.Hook; +var Runnable = Mocha.Runnable; + +describe(Hook.name, function() { + var hook; + + beforeEach(function() { + hook = new Hook('Some hook', function() {}); + }); + + afterEach(function() { + sinon.restore(); + }); + + describe('error', function() { + it('should set the hook._error', function() { + var expectedError = new Error('Expected error'); + hook.error(expectedError); + expect(hook._error, 'to be', expectedError); + }); + it('should get the hook._error when called without arguments', function() { + var expectedError = new Error('Expected error'); + hook._error = expectedError; + expect(hook.error(), 'to be', expectedError); + }); + }); + + describe('reset', function() { + it('should call Runnable.reset', function() { + var runnableResetStub = sinon.stub(Runnable.prototype, 'reset'); + hook.reset(); + expect(runnableResetStub, 'was called once'); + }); + + it('should reset the error state', function() { + hook.error(new Error('Expected error for test')); + hook.reset(); + expect(hook.error(), 'to be undefined'); + }); + }); +}); diff --git a/test/unit/suite.spec.js b/test/unit/suite.spec.js index 2fd32889bc..a5063b7f91 100644 --- a/test/unit/suite.spec.js +++ b/test/unit/suite.spec.js @@ -102,6 +102,24 @@ describe('Suite', function() { expect(testResetStub, 'was called once'); expect(suiteResetStub, 'was called once'); }); + + it('should forward reset to all hooks', function() { + this.suite.beforeEach(function() {}); + this.suite.afterEach(function() {}); + this.suite.beforeAll(function() {}); + this.suite.afterAll(function() {}); + sinon.stub(this.suite.getHooks('beforeEach')[0], 'reset'); + sinon.stub(this.suite.getHooks('afterEach')[0], 'reset'); + sinon.stub(this.suite.getHooks('beforeAll')[0], 'reset'); + sinon.stub(this.suite.getHooks('afterAll')[0], 'reset'); + + this.suite.reset(); + + expect(this.suite.getHooks('beforeEach')[0].reset, 'was called once'); + expect(this.suite.getHooks('afterEach')[0].reset, 'was called once'); + expect(this.suite.getHooks('beforeAll')[0].reset, 'was called once'); + expect(this.suite.getHooks('afterAll')[0].reset, 'was called once'); + }); }); describe('.timeout()', function() {