Skip to content

Commit

Permalink
run global setup before test files load; closes #4508
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
boneskull committed Nov 13, 2020
1 parent c6856ba commit 6bf7da7
Show file tree
Hide file tree
Showing 7 changed files with 105 additions and 188 deletions.
4 changes: 4 additions & 0 deletions browser-entry.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
49 changes: 41 additions & 8 deletions lib/cli/run-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<Runner>}
* @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
Expand All @@ -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);
});
};

/**
Expand Down
33 changes: 21 additions & 12 deletions lib/mocha.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/plugin-loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
'use strict';

console.log('test fixture loaded');

describe('a suite', function() {
it('should succeed', function(done) {
done();
});
});
49 changes: 29 additions & 20 deletions test/integration/plugins/global-fixtures.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
]),
Expand All @@ -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'
Expand All @@ -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/
);
});
});
Expand All @@ -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() {
Expand Down Expand Up @@ -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')
Expand All @@ -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')
Expand All @@ -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(
Expand All @@ -222,7 +227,7 @@ describe('global setup/teardown', function() {
]),
'when fulfilled',
'to contain once',
/teardown: this.foo = 3/
/teardown: this\.foo = 3/
);
});
});
Expand All @@ -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() {
Expand Down Expand Up @@ -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}/
);
});
});
Expand Down
Loading

0 comments on commit 6bf7da7

Please sign in to comment.