From 6bf7da79d5f9377334dfdf9ba9ea49035bbc3cd7 Mon Sep 17 00:00:00 2001 From: Christopher Hiller Date: Fri, 13 Nov 2020 13:09:11 -0800 Subject: [PATCH] run global setup before test files load; closes #4508 BREAKING CHANGE `Mocha#run()` was orchestrating if and when to run global setup/teardown fixtures, but `Mocha#run()` must be called after test files have been loaded. This is a problem, because: 1. `--delay` may expect something created in a fixture to be accessible, but it won't be 2. Any future support for async suites is similarly negatively impacted 3. It was inconsistent between "watch" and "single run" mode; "watch" mode already has this behavior! This change causes setup fixtures to run _before_ any test files have been loaded. In Node.js, this happens in `lib/cli/run-helpers`. - Added two functions to `Mocha`; `Mocha#globalSetupEnabled()` and `Mocha#globalTeardownEnabled()` which both return booleans - Correct order of operations is asserted in `test/integration/global-fixtures.spec.js` - Removed low-value (and newly broken) unit tests where `Mocha#run` called `runGlobalSetup`/`runGlobalTeardown` - Add a note to `browser-entry.js` about global fixtures This is breaking because: 1. Browser users now must run setup/teardown fixtures manually. 2. Somebody's test harness might expect test files to be loaded (and suites run) _before_ global setup fixtures execute. --- browser-entry.js | 4 + lib/cli/run-helpers.js | 49 +++++- lib/mocha.js | 33 ++-- lib/plugin-loader.js | 2 +- .../plugins/global-fixtures/global.fixture.js | 9 ++ .../plugins/global-fixtures.spec.js | 49 +++--- test/unit/mocha.spec.js | 147 ------------------ 7 files changed, 105 insertions(+), 188 deletions(-) create mode 100644 test/integration/fixtures/plugins/global-fixtures/global.fixture.js diff --git a/browser-entry.js b/browser-entry.js index dee3789231..f4682d01a5 100644 --- a/browser-entry.js +++ b/browser-entry.js @@ -166,6 +166,10 @@ mocha.setup = function(opts) { /** * Run mocha, returning the Runner. + * Mocha does _not_ automatically run global fixtures in the browser; you must do this manually. + * Use {@link Mocha#globalSetup} and {@link Mocha#globalTeardown} for registration, + * then use {@link Mocha#runGlobalSetup} and {@link Mocha#runGlobalTeardown} to run them. + * @see https://mochajs.org/api/mocha#globalSetup */ mocha.run = function(fn) { diff --git a/lib/cli/run-helpers.js b/lib/cli/run-helpers.js index dfac8b4479..ea1bae5fc2 100644 --- a/lib/cli/run-helpers.js +++ b/lib/cli/run-helpers.js @@ -107,29 +107,49 @@ exports.handleRequires = async (requires = [], {ignoredPlugins = []} = {}) => { }; /** - * Collect and load test files, then run mocha instance. + * Runs Mocha once in serial mode. + * + * 1. Finds test files + * 2. Runs global setup fixtures, if any + * 3. Loads test files + * 4. Runs tests + * 5. Runs global teardown fixtures, if any * @param {Mocha} mocha - Mocha instance - * @param {Options} [opts] - Command line options - * @param {boolean} [opts.exit] - Whether or not to force-exit after tests are complete + * @param {Options} opts - Command line options * @param {Object} fileCollectParams - Parameters that control test * file collection. See `lib/cli/collect-files.js`. * @returns {Promise} * @private */ -const singleRun = async (mocha, {exit}, fileCollectParams) => { +const singleRun = async (mocha, options, fileCollectParams) => { const files = collectFiles(fileCollectParams); debug('single run with %d file(s)', files.length); mocha.files = files; + let context = {}; + if (mocha.globalSetupEnabled() && mocha.hasGlobalSetupFixtures()) { + context = await mocha.runGlobalSetup(context); + } // handles ESM modules await mocha.loadFilesAsync(); - return mocha.run(exit ? exitMocha : exitMochaLater); + + const done = options.exit ? exitMocha : exitMochaLater; + + return mocha.run(async (...args) => { + if (mocha.globalTeardownEnabled() && mocha.hasGlobalTeardownFixtures()) { + await mocha.runGlobalTeardown(context); + } + done(...args); + }); }; /** - * Collect files and run tests (using `BufferedRunner`). + * Run Mocha once in parallel mode. * - * This is `async` for consistency. + * 1. Finds test files + * 2. Runs global setup fixtures, if any + * 3. Runs tests (loading handled by worker pool) + * 4. Runs global teardown fixtures, if any * * @param {Mocha} mocha - Mocha instance * @param {Options} options - Command line options @@ -141,11 +161,24 @@ const singleRun = async (mocha, {exit}, fileCollectParams) => { */ const parallelRun = async (mocha, options, fileCollectParams) => { const files = collectFiles(fileCollectParams); + const done = options.exit ? exitMocha : exitMochaLater; + debug('executing %d test file(s) in parallel mode', files.length); mocha.files = files; + let context = {}; + if (mocha.globalSetupEnabled() && mocha.hasGlobalSetupFixtures()) { + context = await mocha.runGlobalSetup(context); + } + // note that we DO NOT load any files here; this is handled by the worker - return mocha.run(options.exit ? exitMocha : exitMochaLater); + + return mocha.run(async (...args) => { + if (mocha.globalTeardownEnabled() && mocha.hasGlobalTeardownFixtures()) { + await mocha.runGlobalTeardown(context); + } + done(...args); + }); }; /** diff --git a/lib/mocha.js b/lib/mocha.js index 611c33a951..20779deefe 100644 --- a/lib/mocha.js +++ b/lib/mocha.js @@ -1012,20 +1012,11 @@ Mocha.prototype.run = function(fn) { } }; - const runAsync = async runner => { - const context = - this.options.enableGlobalSetup && this.hasGlobalSetupFixtures() - ? await this.runGlobalSetup(runner) - : {}; - const failureCount = await runner.runAsync({ + const runAsync = async runner => + runner.runAsync({ files: this.files, options }); - if (this.options.enableGlobalTeardown && this.hasGlobalTeardownFixtures()) { - await this.runGlobalTeardown(runner, {context}); - } - return failureCount; - }; // no "catch" here is intentional. errors coming out of // Runner#run are considered uncaught/unhandled and caught @@ -1236,7 +1227,7 @@ Mocha.prototype.enableGlobalSetup = function enableGlobalSetup(enabled = true) { * * @chainable * @public - * @param {boolean } [enabled=true] - If `false`, do not run global teardown fixture + * @param {boolean} [enabled=true] - If `false`, do not run global teardown fixture * @returns {Mocha} */ Mocha.prototype.enableGlobalTeardown = function enableGlobalTeardown( @@ -1246,6 +1237,24 @@ Mocha.prototype.enableGlobalTeardown = function enableGlobalTeardown( return this; }; +/** + * Returns `true` if global setup fixtures are enabled. + * @public + * @returns {boolean} + */ +Mocha.prototype.globalSetupEnabled = function globalSetupEnabled() { + return Boolean(this.options.enableGlobalSetup); +}; + +/** + * Returns `true` if global teardown fixtures are enabled. + * @public + * @returns {boolean} + */ +Mocha.prototype.globalTeardownEnabled = function globalSetupEnabled() { + return Boolean(this.options.enableGlobalTeardown); +}; + /** * Returns `true` if one or more global setup fixtures have been supplied. * @public diff --git a/lib/plugin-loader.js b/lib/plugin-loader.js index 075ad91b49..3597ac7110 100644 --- a/lib/plugin-loader.js +++ b/lib/plugin-loader.js @@ -222,7 +222,7 @@ class PluginLoader { // we should explicitly NOT fail if other stuff is exported. // we only care about the plugins we know about. if (requiredModule && typeof requiredModule === 'object') { - return Array.from(this.knownExportNames).reduce( + return [...this.knownExportNames].reduce( (pluginImplFound, pluginName) => { const pluginImpl = requiredModule[pluginName]; if (pluginImpl) { diff --git a/test/integration/fixtures/plugins/global-fixtures/global.fixture.js b/test/integration/fixtures/plugins/global-fixtures/global.fixture.js new file mode 100644 index 0000000000..be26cffb0e --- /dev/null +++ b/test/integration/fixtures/plugins/global-fixtures/global.fixture.js @@ -0,0 +1,9 @@ +'use strict'; + +console.log('test fixture loaded'); + +describe('a suite', function() { + it('should succeed', function(done) { + done(); + }); +}); diff --git a/test/integration/plugins/global-fixtures.spec.js b/test/integration/plugins/global-fixtures.spec.js index 5e3acb1a9d..0159e82c33 100644 --- a/test/integration/plugins/global-fixtures.spec.js +++ b/test/integration/plugins/global-fixtures.spec.js @@ -6,16 +6,17 @@ const { runMochaAsync, runMochaWatchAsync, copyFixture, - DEFAULT_FIXTURE, resolveFixturePath, createTempDir } = require('../helpers'); +const FIXTURE = resolveFixturePath('plugins/global-fixtures/global'); + describe('global setup/teardown', function() { describe('when mocha run in serial mode', function() { it('should execute global setup and teardown', async function() { return expect( - runMochaAsync(DEFAULT_FIXTURE, [ + runMochaAsync(FIXTURE, [ '--require', resolveFixturePath('plugins/global-fixtures/global-setup-teardown') ]), @@ -27,47 +28,47 @@ describe('global setup/teardown', function() { describe('when only global teardown is supplied', function() { it('should run global teardown', async function() { return expect( - runMochaAsync(DEFAULT_FIXTURE, [ + runMochaAsync(FIXTURE, [ '--require', resolveFixturePath('plugins/global-fixtures/global-teardown') ]), 'when fulfilled', - 'to contain once', - /teardown schmeardown/ - ); + 'to contain', + /test fixture loaded[\s\S]+teardown schmeardown/ + ).and('when fulfilled', 'to contain once', /teardown schmeardown/); }); }); describe('when only global setup is supplied', function() { it('should run global setup', async function() { return expect( - runMochaAsync(DEFAULT_FIXTURE, [ + runMochaAsync(FIXTURE, [ '--require', resolveFixturePath('plugins/global-fixtures/global-setup') ]), 'when fulfilled', - 'to contain once', - /setup schmetup/ - ); + 'to contain', + /setup schmetup[\s\S]+test fixture loaded/ + ).and('when fulfilled', 'to contain once', /setup schmetup/); }); }); it('should share context', async function() { return expect( - runMochaAsync(DEFAULT_FIXTURE, [ + runMochaAsync(FIXTURE, [ '--require', resolveFixturePath('plugins/global-fixtures/global-setup-teardown') ]), 'when fulfilled', 'to contain once', - /setup: this\.foo = bar[\s\S]+teardown: this\.foo = bar/ + /setup: this\.foo = bar[\s\S]+test fixture loaded[\s\S]+teardown: this\.foo = bar/ ); }); describe('when supplied multiple functions', function() { it('should execute them sequentially', async function() { return expect( - runMochaAsync(DEFAULT_FIXTURE, [ + runMochaAsync(FIXTURE, [ '--require', resolveFixturePath( 'plugins/global-fixtures/global-setup-teardown-multiple' @@ -76,6 +77,10 @@ describe('global setup/teardown', function() { 'when fulfilled', 'to contain once', /teardown: this.foo = 3/ + ).and( + 'when fulfilled', + 'to contain', + /test fixture loaded[\s\S]+teardown: this\.foo = 3/ ); }); }); @@ -90,7 +95,7 @@ describe('global setup/teardown', function() { tempDir = tempInfo.dirpath; removeTempDir = tempInfo.removeTempDir; testFile = path.join(tempDir, 'test.js'); - copyFixture(DEFAULT_FIXTURE, testFile); + copyFixture(FIXTURE, testFile); }); afterEach(async function() { @@ -187,7 +192,7 @@ describe('global setup/teardown', function() { describe('when mocha run in parallel mode', function() { it('should execute global setup and teardown', async function() { return expect( - runMochaAsync(DEFAULT_FIXTURE, [ + runMochaAsync(FIXTURE, [ '--parallel', '--require', resolveFixturePath('plugins/global-fixtures/global-setup-teardown') @@ -199,7 +204,7 @@ describe('global setup/teardown', function() { it('should share context', async function() { return expect( - runMochaAsync(DEFAULT_FIXTURE, [ + runMochaAsync(FIXTURE, [ '--parallel', '--require', resolveFixturePath('plugins/global-fixtures/global-setup-teardown') @@ -213,7 +218,7 @@ describe('global setup/teardown', function() { describe('when supplied multiple functions', function() { it('should execute them sequentially', async function() { return expect( - runMochaAsync(DEFAULT_FIXTURE, [ + runMochaAsync(FIXTURE, [ '--parallel', '--require', resolveFixturePath( @@ -222,7 +227,7 @@ describe('global setup/teardown', function() { ]), 'when fulfilled', 'to contain once', - /teardown: this.foo = 3/ + /teardown: this\.foo = 3/ ); }); }); @@ -237,7 +242,7 @@ describe('global setup/teardown', function() { tempDir = tempInfo.dirpath; removeTempDir = tempInfo.removeTempDir; testFile = path.join(tempDir, 'test.js'); - copyFixture(DEFAULT_FIXTURE, testFile); + copyFixture(FIXTURE, testFile); }); afterEach(async function() { @@ -285,7 +290,11 @@ describe('global setup/teardown', function() { ), 'when fulfilled', 'to contain once', - /teardown: this.foo = 3/ + /teardown: this\.foo = 3/ + ).and( + 'when fulfilled', + 'to contain once', + /(test fixture loaded[\s\S]+?){2}/ ); }); }); diff --git a/test/unit/mocha.spec.js b/test/unit/mocha.spec.js index 142018b54c..f7fa791d8d 100644 --- a/test/unit/mocha.spec.js +++ b/test/unit/mocha.spec.js @@ -854,153 +854,6 @@ describe('Mocha', function() { }); }); }); - - describe('when global setup fixtures enabled', function() { - beforeEach(function() { - mocha.options.enableGlobalSetup = true; - }); - describe('when global setup fixtures not present', function() { - beforeEach(function() { - sinon.stub(mocha, 'hasGlobalSetupFixtures').returns(false); - }); - - it('should not run global setup fixtures', function(done) { - mocha.run(() => { - expect(mocha.runGlobalSetup, 'was not called'); - done(); - }); - }); - }); - - describe('when global setup fixtures are present', function() { - beforeEach(function() { - sinon.stub(mocha, 'hasGlobalSetupFixtures').returns(true); - }); - - it('should run global setup fixtures', function(done) { - mocha.run(() => { - expect(mocha.runGlobalSetup, 'to have a call satisfying', { - args: [expect.it('to be', runner)] - }).and('was called once'); - done(); - }); - }); - }); - }); - - describe('when global setup fixtures disabled', function() { - beforeEach(function() { - mocha.options.enableGlobalSetup = false; - }); - describe('when global setup fixtures not present', function() { - beforeEach(function() { - sinon.stub(mocha, 'hasGlobalSetupFixtures').returns(false); - }); - - it('should not run global setup fixtures', function(done) { - mocha.run(() => { - expect(mocha.runGlobalSetup, 'was not called'); - done(); - }); - }); - }); - - describe('when global setup fixtures are present', function() { - beforeEach(function() { - sinon.stub(mocha, 'hasGlobalSetupFixtures').returns(true); - }); - - it('should not run global setup fixtures', function(done) { - mocha.run(() => { - expect(mocha.runGlobalSetup, 'was not called'); - done(); - }); - }); - }); - }); - - describe('when global teardown fixtures enabled', function() { - beforeEach(function() { - mocha.options.enableGlobalTeardown = true; - }); - describe('when global teardown fixtures not present', function() { - beforeEach(function() { - sinon.stub(mocha, 'hasGlobalTeardownFixtures').returns(false); - }); - - it('should not run global teardown fixtures', function(done) { - mocha.run(() => { - expect(mocha.runGlobalTeardown, 'was not called'); - done(); - }); - }); - }); - - describe('when global teardown fixtures are present', function() { - beforeEach(function() { - sinon.stub(mocha, 'hasGlobalTeardownFixtures').returns(true); - }); - - it('should run global teardown fixtures', function(done) { - mocha.run(() => { - expect(mocha.runGlobalTeardown, 'to have a call satisfying', { - args: [expect.it('to be', runner), {context: {}}] - }).and('was called once'); - done(); - }); - }); - - describe('when global setup fixtures are present and enabled', function() { - beforeEach(function() { - sinon.stub(mocha, 'hasGlobalSetupFixtures').returns(true); - mocha.options.enableGlobalSetup = true; - }); - - it('should use the same context as returned by `runGlobalSetup`', function(done) { - mocha.run(() => { - expect(mocha.runGlobalTeardown, 'to have a call satisfying', { - args: [ - expect.it('to be', runner), - {context: globalFixtureContext} - ] - }).and('was called once'); - done(); - }); - }); - }); - }); - }); - - describe('when global teardown fixtures disabled', function() { - beforeEach(function() { - mocha.options.enableGlobalTeardown = false; - }); - describe('when global teardown fixtures not present', function() { - beforeEach(function() { - sinon.stub(mocha, 'hasGlobalTeardownFixtures').returns(false); - }); - - it('should not run global teardown fixtures', function(done) { - mocha.run(() => { - expect(mocha.runGlobalTeardown, 'was not called'); - done(); - }); - }); - }); - - describe('when global teardown fixtures are present', function() { - beforeEach(function() { - sinon.stub(mocha, 'hasGlobalTeardownFixtures').returns(true); - }); - - it('should not run global teardown fixtures', function(done) { - mocha.run(() => { - expect(mocha.runGlobalTeardown, 'was not called'); - done(); - }); - }); - }); - }); }); describe('parallelMode()', function() {