From 3e6ae1aed6d198b73531231adad3f5f94e1ac6cc Mon Sep 17 00:00:00 2001 From: Christopher Hiller Date: Mon, 17 Aug 2020 17:52:39 -0700 Subject: [PATCH 1/3] parallel mode reporter improvements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `Suite`s, `Test`s, etc., now have unique identifiers which can optionally be used to re-created object references by a reporter which opts-in to the behavior - A reporter can swap out the worker’s reporter for a custom one - fixed an issue in rollup config causing weird problems - changed how we define `global.Mocha` in `browser-entry.js` (tangential) - removed `object.assign` polyfill (tangential) - renamed misnamed `buffered-runner.spec.js` to `parallel-buffered-runner.spec.js` - added a few unit tests - refactored some tests; mainly modernizing some syntax --- lib/hook.js | 24 +- lib/nodejs/parallel-buffered-runner.js | 148 ++- lib/nodejs/reporters/parallel-buffered.js | 90 +- lib/nodejs/worker.js | 4 - lib/runnable.js | 6 + lib/runner.js | 52 + lib/suite.js | 42 +- lib/test.js | 8 +- lib/utils.js | 31 + package-lock.json | 44 +- package.json | 1 + ...ec.js => parallel-buffered-runner.spec.js} | 160 ++- test/unit/mocha.spec.js | 10 +- test/unit/runner.spec.js | 73 +- test/unit/suite.spec.js | 1050 ++++++++--------- test/unit/utils.spec.js | 6 + 16 files changed, 1073 insertions(+), 676 deletions(-) rename test/node-unit/{buffered-runner.spec.js => parallel-buffered-runner.spec.js} (79%) diff --git a/lib/hook.js b/lib/hook.js index 6c12c02bb8..8ccbd4947f 100644 --- a/lib/hook.js +++ b/lib/hook.js @@ -1,7 +1,8 @@ 'use strict'; var Runnable = require('./runnable'); -var inherits = require('./utils').inherits; +const {inherits, constants} = require('./utils'); +const {MOCHA_ID_PROP_NAME} = constants; /** * Expose `Hook`. @@ -63,16 +64,21 @@ Hook.prototype.serialize = function serialize() { return { $$isPending: this.isPending(), $$titlePath: this.titlePath(), - ctx: { - currentTest: { - title: this.ctx && this.ctx.currentTest && this.ctx.currentTest.title - } - }, + ctx: + this.ctx && this.ctx.currentTest + ? { + currentTest: { + title: this.ctx.currentTest.title, + [MOCHA_ID_PROP_NAME]: this.ctx.currentTest.id + } + } + : {}, parent: { - root: this.parent.root, - title: this.parent.title + [MOCHA_ID_PROP_NAME]: this.parent.id }, title: this.title, - type: this.type + type: this.type, + [MOCHA_ID_PROP_NAME]: this.id, + __mocha_partial__: true }; }; diff --git a/lib/nodejs/parallel-buffered-runner.js b/lib/nodejs/parallel-buffered-runner.js index a079bbd571..c21264d076 100644 --- a/lib/nodejs/parallel-buffered-runner.js +++ b/lib/nodejs/parallel-buffered-runner.js @@ -12,7 +12,12 @@ const {EVENT_RUN_BEGIN, EVENT_RUN_END} = Runner.constants; const debug = require('debug')('mocha:parallel:parallel-buffered-runner'); const {BufferedWorkerPool} = require('./buffered-worker-pool'); const {setInterval, clearInterval} = global; -const {createMap} = require('../utils'); +const {createMap, constants} = require('../utils'); +const {MOCHA_ID_PROP_NAME} = constants; + +const DEFAULT_WORKER_REPORTER = require.resolve( + './reporters/parallel-buffered' +); /** * List of options to _not_ serialize for transmission to workers @@ -68,7 +73,7 @@ const states = createMap({ /** * This `Runner` delegates tests runs to worker threads. Does not execute any * {@link Runnable}s by itself! - * @private + * @public */ class ParallelBufferedRunner extends Runner { constructor(...args) { @@ -88,6 +93,10 @@ class ParallelBufferedRunner extends Runner { } }); + this._workerReporter = DEFAULT_WORKER_REPORTER; + this._linkPartialObjects = false; + this._linkedObjectMap = new Map(); + this.once(Runner.constants.EVENT_RUN_END, () => { this._state = COMPLETE; }); @@ -98,12 +107,63 @@ class ParallelBufferedRunner extends Runner { * @param {BufferedWorkerPool} pool - Worker pool * @param {Options} options - Mocha options * @returns {FileRunner} Mapping function + * @private */ _createFileRunner(pool, options) { + /** + * Emits event and sets `BAILING` state, if necessary. + * @param {Object} event - Event having `eventName`, maybe `data` and maybe `error` + * @param {number} failureCount - Failure count + */ + const emitEvent = (event, failureCount) => { + this.emit(event.eventName, event.data, event.error); + if ( + this._state !== BAILING && + event.data && + event.data._bail && + (failureCount || event.error) + ) { + debug('run(): nonzero failure count & found bail flag'); + // we need to let the events complete for this file, as the worker + // should run any cleanup hooks + this._state = BAILING; + } + }; + + /** + * Given an event, recursively find any objects in its data that have ID's, and create object references to already-seen objects. + * @param {Object} event - Event having `eventName`, maybe `data` and maybe `error` + */ + const linkEvent = event => { + const stack = [{parent: event, prop: 'data'}]; + while (stack.length) { + const {parent, prop} = stack.pop(); + const obj = parent[prop]; + let newObj; + if (obj && typeof obj === 'object' && obj[MOCHA_ID_PROP_NAME]) { + const id = obj[MOCHA_ID_PROP_NAME]; + newObj = this._linkedObjectMap.has(id) + ? Object.assign(this._linkedObjectMap.get(id), obj) + : obj; + this._linkedObjectMap.set(id, newObj); + parent[prop] = newObj; + } else { + newObj = obj; + } + Object.keys(newObj).forEach(key => { + const value = obj[key]; + if (value && typeof value === 'object' && value[MOCHA_ID_PROP_NAME]) { + stack.push({obj: value, parent: newObj, prop: key}); + } + }); + } + }; + return async file => { debug('run(): enqueueing test file %s', file); try { const {failureCount, events} = await pool.run(file, options); + if (this._state === BAILED) { // short-circuit after a graceful bail. if this happens, // some other worker has bailed. @@ -119,20 +179,18 @@ class ParallelBufferedRunner extends Runner { ); this.failures += failureCount; // can this ever be non-numeric? let event = events.shift(); - while (event) { - this.emit(event.eventName, event.data, event.error); - if ( - this._state !== BAILING && - event.data && - event.data._bail && - (failureCount || event.error) - ) { - debug('run(): nonzero failure count & found bail flag'); - // we need to let the events complete for this file, as the worker - // should run any cleanup hooks - this._state = BAILING; + + if (this._linkPartialObjects) { + while (event) { + linkEvent(event); + emitEvent(event, failureCount); + event = events.shift(); + } + } else { + while (event) { + emitEvent(event, failureCount); + event = events.shift(); } - event = events.shift(); } if (this._state === BAILING) { debug('run(): terminating pool due to "bail" flag'); @@ -166,6 +224,7 @@ class ParallelBufferedRunner extends Runner { * Returns the listener for later call to `process.removeListener()`. * @param {BufferedWorkerPool} pool - Worker pool * @returns {SigIntListener} Listener + * @private */ _bindSigIntListener(pool) { const sigIntListener = async () => { @@ -209,15 +268,19 @@ class ParallelBufferedRunner extends Runner { * @param {{files: string[], options: Options}} opts - Files to run and * command-line options, respectively. */ - run(callback, {files, options} = {}) { + run(callback, {files, options = {}} = {}) { /** * Listener on `Process.SIGINT` which tries to cleanly terminate the worker pool. */ let sigIntListener; + + // assign the reporter the worker will use, which will be different than the + // main process' reporter + options = {...options, reporter: this._workerReporter}; + // This function should _not_ return a `Promise`; its parent (`Runner#run`) // returns this instance, so this should do the same. However, we want to make // use of `async`/`await`, so we use this IIFE. - (async () => { /** * This is an interval that outputs stats about the worker pool every so often @@ -293,6 +356,57 @@ class ParallelBufferedRunner extends Runner { })(); return this; } + + /** + * Toggle partial object linking behavior; used for building object references from + * unique ID's. + * @param {boolean} [value] - If `true`, enable partial object linking, otherwise disable + * @returns {Runner} + * @chainable + * @public + * @example + * // this reporter needs proper object references when run in parallel mode + * class MyReporter() { + * constructor(runner) { + * this.runner.linkPartialObjects(true) + * .on(EVENT_SUITE_BEGIN, suite => { + // this Suite may be the same object... + * }) + * .on(EVENT_TEST_BEGIN, test => { + * // ...as the `test.parent` property + * }); + * } + * } + */ + linkPartialObjects(value) { + this._linkPartialObjects = Boolean(value); + return super.linkPartialObjects(value); + } + + /** + * If this class is the `Runner` in use, then this is going to return `true`. + * + * For use by reporters. + * @returns {true} + * @public + */ + isParallelMode() { + return true; + } + + /** + * Configures an alternate reporter for worker processes to use. Subclasses + * using worker processes should implement this. + * @public + * @param {string} path - Absolute path to alternate reporter for worker processes to use + * @returns {Runner} + * @throws When in serial mode + * @chainable + */ + workerReporter(reporter) { + this._workerReporter = reporter; + return this; + } } module.exports = ParallelBufferedRunner; diff --git a/lib/nodejs/reporters/parallel-buffered.js b/lib/nodejs/reporters/parallel-buffered.js index 3fc8b15491..840d718ed8 100644 --- a/lib/nodejs/reporters/parallel-buffered.js +++ b/lib/nodejs/reporters/parallel-buffered.js @@ -1,7 +1,7 @@ /** * "Buffered" reporter used internally by a worker process when running in parallel mode. - * @module reporters/parallel-buffered - * @private + * @module nodejs/reporters/parallel-buffered + * @public */ 'use strict'; @@ -53,15 +53,16 @@ const EVENT_NAMES = [ const ONCE_EVENT_NAMES = [EVENT_DELAY_BEGIN, EVENT_DELAY_END]; /** - * The `ParallelBuffered` reporter is for use by concurrent runs. Instead of outputting - * to `STDOUT`, etc., it retains a list of events it receives and hands these - * off to the callback passed into {@link Mocha#run}. That callback will then - * return the data to the main process. - * @private + * The `ParallelBuffered` reporter is used by each worker process in "parallel" + * mode, by default. Instead of reporting to to `STDOUT`, etc., it retains a + * list of events it receives and hands these off to the callback passed into + * {@link Mocha#run}. That callback will then return the data to the main + * process. + * @public */ class ParallelBuffered extends Base { /** - * Listens for {@link Runner} events and retains them in an `events` instance prop. + * Calls {@link ParallelBuffered#createListeners} * @param {Runner} runner */ constructor(runner, opts) { @@ -70,50 +71,81 @@ class ParallelBuffered extends Base { /** * Retained list of events emitted from the {@link Runner} instance. * @type {BufferedEvent[]} - * @memberOf Buffered + * @public */ - const events = (this.events = []); + this.events = []; /** - * mapping of event names to listener functions we've created, - * so we can cleanly _remove_ them from the runner once it's completed. + * Map of `Runner` event names to listeners (for later teardown) + * @public + * @type {Map} */ - const listeners = new Map(); + this.listeners = new Map(); - /** - * Creates a listener for event `eventName` and adds it to the `listeners` - * map. This is a defensive measure, so that we don't a) leak memory or b) - * remove _other_ listeners that may not be associated with this reporter. - * @param {string} eventName - Event name - */ - const createListener = eventName => - listeners - .set(eventName, (runnable, err) => { - events.push(SerializableEvent.create(eventName, runnable, err)); - }) - .get(eventName); + this.createListeners(runner); + } + + /** + * Returns a new listener which saves event data in memory to + * {@link ParallelBuffered#events}. Listeners are indexed by `eventName` and stored + * in {@link ParallelBuffered#listeners}. This is a defensive measure, so that we + * don't a) leak memory or b) remove _other_ listeners that may not be + * associated with this reporter. + * + * Subclasses could override this behavior. + * + * @public + * @param {string} eventName - Name of event to create listener for + * @returns {EventListener} + */ + createListener(eventName) { + const listener = (runnable, err) => { + this.events.push(SerializableEvent.create(eventName, runnable, err)); + }; + return this.listeners.set(eventName, listener).get(eventName); + } + /** + * Creates event listeners (using {@link ParallelBuffered#createListener}) for each + * reporter-relevant event emitted by a {@link Runner}. This array is drained when + * {@link ParallelBuffered#done} is called by {@link Runner#run}. + * + * Subclasses could override this behavior. + * @public + * @param {Runner} runner - Runner instance + * @returns {ParallelBuffered} + * @chainable + */ + createListeners(runner) { EVENT_NAMES.forEach(evt => { - runner.on(evt, createListener(evt)); + runner.on(evt, this.createListener(evt)); }); ONCE_EVENT_NAMES.forEach(evt => { - runner.once(evt, createListener(evt)); + runner.once(evt, this.createListener(evt)); }); runner.once(EVENT_RUN_END, () => { debug('received EVENT_RUN_END'); - listeners.forEach((listener, evt) => { + this.listeners.forEach((listener, evt) => { runner.removeListener(evt, listener); - listeners.delete(evt); + this.listeners.delete(evt); }); }); + + return this; } /** * Calls the {@link Mocha#run} callback (`callback`) with the test failure * count and the array of {@link BufferedEvent} objects. Resets the array. + * + * This is called directly by `Runner#run` and should not be called by any other consumer. + * + * Subclasses could override this. + * * @param {number} failures - Number of failed tests * @param {Function} callback - The callback passed to {@link Mocha#run}. + * @public */ done(failures, callback) { callback(SerializableWorkerResult.create(this.events, failures)); diff --git a/lib/nodejs/worker.js b/lib/nodejs/worker.js index 2f002fa0f5..cf5655e80d 100644 --- a/lib/nodejs/worker.js +++ b/lib/nodejs/worker.js @@ -19,8 +19,6 @@ const isDebugEnabled = d.enabled(`mocha:parallel:worker:${process.pid}`); const {serialize} = require('./serializer'); const {setInterval, clearInterval} = global; -const BUFFERED_REPORTER_PATH = require.resolve('./reporters/parallel-buffered'); - let rootHooks; if (workerpool.isMainThread) { @@ -91,8 +89,6 @@ async function run(filepath, serializedOptions = '{}') { } const opts = Object.assign({ui: 'bdd'}, argv, { - // workers only use the `Buffered` reporter. - reporter: BUFFERED_REPORTER_PATH, // if this was true, it would cause infinite recursion. parallel: false, // this doesn't work in parallel mode diff --git a/lib/runnable.js b/lib/runnable.js index 023481dd69..e65b4c4638 100644 --- a/lib/runnable.js +++ b/lib/runnable.js @@ -38,6 +38,12 @@ function Runnable(title, fn) { this._timeout = 2000; this._slow = 75; this._retries = -1; + utils.assignNewMochaID(this); + Object.defineProperty(this, 'id', { + get() { + return utils.getMochaID(this); + } + }); this.reset(); } diff --git a/lib/runner.js b/lib/runner.js index dde356f9d7..bc88876c39 100644 --- a/lib/runner.js +++ b/lib/runner.js @@ -1079,6 +1079,33 @@ Runner.prototype.run = function(fn, opts = {}) { }; /** + * Toggle partial object linking behavior; used for building object references from + * unique ID's. Does nothing in serial mode, because the object references already exist. + * Subclasses can implement this (e.g., `ParallelBufferedRunner`) + * @abstract + * @param {boolean} [value] - If `true`, enable partial object linking, otherwise disable + * @returns {Runner} + * @chainable + * @public + * @example + * // this reporter needs proper object references when run in parallel mode + * class MyReporter() { + * constructor(runner) { + * this.runner.linkPartialObjects(true) + * .on(EVENT_SUITE_BEGIN, suite => { + // this Suite may be the same object... + * }) + * .on(EVENT_TEST_BEGIN, test => { + * // ...as the `test.parent` property + * }); + * } + * } + */ +Runner.prototype.linkPartialObjects = function(value) { + return this; +}; + +/* * Like {@link Runner#run}, but does not accept a callback and returns a `Promise` instead of a `Runner`. * This function cannot reject; an `unhandledRejection` event will bubble up to the `process` object instead. * @public @@ -1106,6 +1133,31 @@ Runner.prototype.abort = function() { return this; }; +/** + * Returns `true` if Mocha is running in parallel mode. For reporters. + * + * Subclasses should return an appropriate value. + * @public + * @returns {false} + */ +Runner.prototype.isParallelMode = function isParallelMode() { + return false; +}; + +/** + * Configures an alternate reporter for worker processes to use. Subclasses + * using worker processes should implement this. + * @public + * @param {string} path - Absolute path to alternate reporter for worker processes to use + * @returns {Runner} + * @throws When in serial mode + * @chainable + * @abstract + */ +Runner.prototype.workerReporter = function() { + throw createUnsupportedError('workerReporter() not supported in serial mode'); +}; + /** * Filter leaks with the given globals flagged as `ok`. * diff --git a/lib/suite.js b/lib/suite.js index 5df005e639..78eb1b8ce0 100644 --- a/lib/suite.js +++ b/lib/suite.js @@ -4,14 +4,24 @@ * Module dependencies. * @private */ -var EventEmitter = require('events').EventEmitter; -var Hook = require('./hook'); -var utils = require('./utils'); -var inherits = utils.inherits; -var debug = require('debug')('mocha:suite'); -var milliseconds = require('ms'); +const {EventEmitter} = require('events'); +const Hook = require('./hook'); +var { + assignNewMochaID, + clamp, + constants, + createMap, + defineConstants, + getMochaID, + inherits, + isString +} = require('./utils'); +const debug = require('debug')('mocha:suite'); +const milliseconds = require('ms'); const errors = require('./errors'); +const {MOCHA_ID_PROP_NAME} = constants; + /** * Expose `Suite`. */ @@ -46,7 +56,7 @@ Suite.create = function(parent, title) { * @param {boolean} [isRoot=false] - Whether this is the root suite. */ function Suite(title, parentContext, isRoot) { - if (!utils.isString(title)) { + if (!isString(title)) { throw errors.createInvalidArgumentTypeError( 'Suite argument "title" must be a string. Received type "' + typeof title + @@ -73,6 +83,14 @@ function Suite(title, parentContext, isRoot) { this._bail = false; this._onlyTests = []; this._onlySuites = []; + assignNewMochaID(this); + + Object.defineProperty(this, 'id', { + get() { + return getMochaID(this); + } + }); + this.reset(); this.on('newListener', function(event) { @@ -144,7 +162,7 @@ Suite.prototype.timeout = function(ms) { // Clamp to range var INT_MAX = Math.pow(2, 31) - 1; var range = [0, INT_MAX]; - ms = utils.clamp(ms, range); + ms = clamp(ms, range); debug('timeout %d', ms); this._timeout = parseInt(ms, 10); @@ -578,11 +596,13 @@ Suite.prototype.serialize = function serialize() { $$fullTitle: this.fullTitle(), $$isPending: this.isPending(), root: this.root, - title: this.title + title: this.title, + id: this.id, + parent: this.parent ? {[MOCHA_ID_PROP_NAME]: this.parent.id} : null }; }; -var constants = utils.defineConstants( +var constants = defineConstants( /** * {@link Suite}-related constants. * @public @@ -670,6 +690,6 @@ var deprecatedEvents = Object.keys(constants) .reduce(function(acc, constant) { acc[constants[constant]] = true; return acc; - }, utils.createMap()); + }, createMap()); Suite.constants = constants; diff --git a/lib/test.js b/lib/test.js index 3fb3e57a4e..f2be51b4f4 100644 --- a/lib/test.js +++ b/lib/test.js @@ -5,6 +5,8 @@ var errors = require('./errors'); var createInvalidArgumentTypeError = errors.createInvalidArgumentTypeError; var isString = utils.isString; +const {MOCHA_ID_PROP_NAME} = utils.constants; + module.exports = Test; /** @@ -98,12 +100,14 @@ Test.prototype.serialize = function serialize() { duration: this.duration, err: this.err, parent: { - $$fullTitle: this.parent.fullTitle() + $$fullTitle: this.parent.fullTitle(), + [MOCHA_ID_PROP_NAME]: this.parent.id }, speed: this.speed, state: this.state, title: this.title, type: this.type, - file: this.file + file: this.file, + [MOCHA_ID_PROP_NAME]: this.id }; }; diff --git a/lib/utils.js b/lib/utils.js index db49b7f901..5fca7aecb3 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -9,11 +9,14 @@ * Module dependencies. */ +const {nanoid} = require('nanoid/non-secure'); var path = require('path'); var util = require('util'); var he = require('he'); const errors = require('./errors'); +const MOCHA_ID_PROP_NAME = '__mocha_id__'; + /** * Inherit the prototype methods from one constructor into another. * @@ -656,3 +659,31 @@ exports.castArray = function castArray(value) { } return [value]; }; + +exports.constants = exports.defineConstants({ + MOCHA_ID_PROP_NAME +}); + +/** + * Creates a new unique identifier + * @returns {string} Unique identifier + */ +exports.uniqueID = () => nanoid(); + +exports.assignNewMochaID = obj => { + const id = exports.uniqueID(); + Object.defineProperty(obj, MOCHA_ID_PROP_NAME, { + get() { + return id; + } + }); + return obj; +}; + +/** + * Retrieves a Mocha ID from an object, if present. + * @param {*} [obj] - Object + * @returns {string|void} + */ +exports.getMochaID = obj => + obj && typeof obj === 'object' ? obj[MOCHA_ID_PROP_NAME] : undefined; diff --git a/package-lock.json b/package-lock.json index 49378b5950..e9635e37cd 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3074,9 +3074,9 @@ }, "dependencies": { "ajv": { - "version": "6.12.4", - "resolved": "https://registry.npmjs.org/ajv/-/ajv-6.12.4.tgz", - "integrity": "sha512-eienB2c9qVQs2KWexhkrdMLVDoIQCz5KSeLxwg9Lzk4DOfBtIK9PQwwufcsn1jjGuf9WZmqPMbGxOzfcuphJCQ==", + "version": "6.12.5", + "resolved": "https://registry.npmjs.org/ajv/-/ajv-6.12.5.tgz", + "integrity": "sha512-lRF8RORchjpKG50/WFf8xmg7sgCLFiYNNnqdKflk63whMQcWR5ngGjiSXkL9bjxy6B2npOK2HSMN49jEBMSkag==", "dev": true, "requires": { "fast-deep-equal": "^3.1.1", @@ -8920,9 +8920,9 @@ } }, "eslint": { - "version": "7.8.1", - "resolved": "https://registry.npmjs.org/eslint/-/eslint-7.8.1.tgz", - "integrity": "sha512-/2rX2pfhyUG0y+A123d0ccXtMm7DV7sH1m3lk9nk2DZ2LReq39FXHueR9xZwshE5MdfSf0xunSaMWRqyIA6M1w==", + "version": "7.10.0", + "resolved": "https://registry.npmjs.org/eslint/-/eslint-7.10.0.tgz", + "integrity": "sha512-BDVffmqWl7JJXqCjAK6lWtcQThZB/aP1HXSH1JKwGwv0LQEdvpR7qzNrUT487RM39B5goWuboFad5ovMBmD8yA==", "dev": true, "requires": { "@babel/code-frame": "^7.0.0", @@ -8933,7 +8933,7 @@ "debug": "^4.0.1", "doctrine": "^3.0.0", "enquirer": "^2.3.5", - "eslint-scope": "^5.1.0", + "eslint-scope": "^5.1.1", "eslint-utils": "^2.1.0", "eslint-visitor-keys": "^1.3.0", "espree": "^7.3.0", @@ -9412,13 +9412,32 @@ "dev": true }, "eslint-scope": { - "version": "5.1.0", - "resolved": "https://registry.npmjs.org/eslint-scope/-/eslint-scope-5.1.0.tgz", - "integrity": "sha512-iiGRvtxWqgtx5m8EyQUJihBloE4EnYeGE/bz1wSPwJE6tZuJUtHlhqDM4Xj2ukE8Dyy1+HCZ4hE0fzIVMzb58w==", + "version": "5.1.1", + "resolved": "https://registry.npmjs.org/eslint-scope/-/eslint-scope-5.1.1.tgz", + "integrity": "sha512-2NxwbF/hZ0KpepYN0cNbo+FN6XoK7GaHlQhgx/hIZl6Va0bF45RQOOwhLIy8lQDbuCiadSLCBnH2CFYquit5bw==", "dev": true, "requires": { - "esrecurse": "^4.1.0", + "esrecurse": "^4.3.0", "estraverse": "^4.1.1" + }, + "dependencies": { + "esrecurse": { + "version": "4.3.0", + "resolved": "https://registry.npmjs.org/esrecurse/-/esrecurse-4.3.0.tgz", + "integrity": "sha512-KmfKL3b6G+RXvP8N1vr3Tq1kL/oCFgn2NYXEtqP8/L3pKapUA4G8cFVaoF3SU323CD4XypR/ffioHmkti6/Tag==", + "dev": true, + "requires": { + "estraverse": "^5.2.0" + }, + "dependencies": { + "estraverse": { + "version": "5.2.0", + "resolved": "https://registry.npmjs.org/estraverse/-/estraverse-5.2.0.tgz", + "integrity": "sha512-BxbNGGNm0RyRYvUdHpIwv9IWzeM9XClbOxwoATuFdOE7ZE6wHL+HQ5T8hoPM+zHvmKzzsEqhgy0GrQ5X13afiQ==", + "dev": true + } + } + } } }, "eslint-utils": { @@ -15694,8 +15713,7 @@ "nanoid": { "version": "3.1.12", "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.1.12.tgz", - "integrity": "sha512-1qstj9z5+x491jfiC4Nelk+f8XBad7LN20PmyWINJEMRSf3wcAjAWysw1qaA8z6NSKe2sjq1hRSDpBH5paCb6A==", - "dev": true + "integrity": "sha512-1qstj9z5+x491jfiC4Nelk+f8XBad7LN20PmyWINJEMRSf3wcAjAWysw1qaA8z6NSKe2sjq1hRSDpBH5paCb6A==" }, "nanomatch": { "version": "1.2.13", diff --git a/package.json b/package.json index f9e24a86f8..edbab206cb 100644 --- a/package.json +++ b/package.json @@ -66,6 +66,7 @@ "log-symbols": "4.0.0", "minimatch": "3.0.4", "ms": "2.1.2", + "nanoid": "3.1.12", "promise.allsettled": "1.0.2", "serialize-javascript": "4.0.0", "strip-json-comments": "3.0.1", diff --git a/test/node-unit/buffered-runner.spec.js b/test/node-unit/parallel-buffered-runner.spec.js similarity index 79% rename from test/node-unit/buffered-runner.spec.js rename to test/node-unit/parallel-buffered-runner.spec.js index ef308f0905..26013f9452 100644 --- a/test/node-unit/buffered-runner.spec.js +++ b/test/node-unit/parallel-buffered-runner.spec.js @@ -8,19 +8,18 @@ const { EVENT_SUITE_BEGIN } = require('../../lib/runner').constants; const rewiremock = require('rewiremock/node'); -const BUFFERED_RUNNER_PATH = require.resolve( - '../../lib/nodejs/parallel-buffered-runner.js' -); const Suite = require('../../lib/suite'); const Runner = require('../../lib/runner'); const sinon = require('sinon'); +const {constants} = require('../../lib/utils'); +const {MOCHA_ID_PROP_NAME} = constants; -describe('buffered-runner', function() { - describe('BufferedRunner', function() { +describe('parallel-buffered-runner', function() { + describe('ParallelBufferedRunner', function() { let run; let BufferedWorkerPool; let terminate; - let BufferedRunner; + let ParallelBufferedRunner; let suite; let warn; let cpuCount; @@ -40,20 +39,31 @@ describe('buffered-runner', function() { stats: sinon.stub().returns({}) }) }; - BufferedRunner = rewiremock.proxy(BUFFERED_RUNNER_PATH, r => ({ - '../../lib/nodejs/buffered-worker-pool': { - BufferedWorkerPool - }, - os: { - cpus: sinon.stub().callsFake(() => new Array(cpuCount)) - }, - '../../lib/utils': r.with({warn}).callThrough() - })); + /** + * @type {ParallelBufferedRunner} + */ + ParallelBufferedRunner = rewiremock.proxy( + () => require('../../lib/nodejs/parallel-buffered-runner'), + r => ({ + '../../lib/nodejs/buffered-worker-pool': { + BufferedWorkerPool + }, + os: { + cpus: sinon.stub().callsFake(() => new Array(cpuCount)) + }, + '../../lib/utils': r.with({warn}).callThrough() + }) + ); }); describe('constructor', function() { it('should start in "IDLE" state', function() { - expect(new BufferedRunner(suite), 'to have property', '_state', 'IDLE'); + expect( + new ParallelBufferedRunner(suite), + 'to have property', + '_state', + 'IDLE' + ); }); }); @@ -61,7 +71,7 @@ describe('buffered-runner', function() { let runner; beforeEach(function() { - runner = new BufferedRunner(suite); + runner = new ParallelBufferedRunner(suite); }); describe('_state', function() { @@ -81,7 +91,7 @@ describe('buffered-runner', function() { let runner; beforeEach(function() { - runner = new BufferedRunner(suite); + runner = new ParallelBufferedRunner(suite); }); describe('EVENT_RUN_END', function() { @@ -94,11 +104,11 @@ describe('buffered-runner', function() { }); describe('instance method', function() { - describe('run', function() { + describe('run()', function() { let runner; beforeEach(function() { - runner = new BufferedRunner(suite); + runner = new ParallelBufferedRunner(suite); }); // the purpose of this is to ensure that--despite using `Promise`s @@ -119,9 +129,67 @@ describe('buffered-runner', function() { ); }); + describe('when instructed to link objects', function() { + beforeEach(function() { + runner.linkPartialObjects(true); + }); + + it('should create object references', function() { + const options = {reporter: runner._workerReporter}; + const someSuite = { + title: 'some suite', + [MOCHA_ID_PROP_NAME]: 'bar' + }; + + run.withArgs('some-file.js', options).resolves({ + failureCount: 0, + events: [ + { + eventName: EVENT_SUITE_END, + data: someSuite + }, + { + eventName: EVENT_TEST_PASS, + data: { + title: 'some test', + [MOCHA_ID_PROP_NAME]: 'foo', + parent: { + // this stub object points to someSuite with id 'bar' + [MOCHA_ID_PROP_NAME]: 'bar' + } + } + }, + { + eventName: EVENT_SUITE_END, + // ensure we are not passing the _same_ someSuite, + // because we won't get the same one from the subprocess + data: {...someSuite} + } + ] + }); + + return expect( + () => + new Promise(resolve => { + runner.run(resolve, {files: ['some-file.js'], options: {}}); + }), + 'to emit from', + runner, + EVENT_TEST_PASS, + { + title: 'some test', + [MOCHA_ID_PROP_NAME]: 'foo', + parent: expect + .it('to be', someSuite) + .and('to have property', 'title', 'some suite') + } + ); + }); + }); + describe('when a worker fails', function() { it('should recover', function(done) { - const options = {}; + const options = {reporter: runner._workerReporter}; run.withArgs('some-file.js', options).rejects(new Error('whoops')); run.withArgs('some-other-file.js', options).resolves({ failureCount: 0, @@ -154,7 +222,7 @@ describe('buffered-runner', function() { }); it('should delegate to Runner#uncaught', function(done) { - const options = {}; + const options = {reporter: runner._workerReporter}; sinon.spy(runner, 'uncaught'); const err = new Error('whoops'); run.withArgs('some-file.js', options).rejects(new Error('whoops')); @@ -236,7 +304,7 @@ describe('buffered-runner', function() { describe('when an event contains an error and has positive failures', function() { describe('when subsequent files have not yet been run', function() { it('should cleanly terminate the thread pool', function(done) { - const options = {}; + const options = {reporter: runner._workerReporter}; const err = { __type: 'Error', message: 'oh no' @@ -286,7 +354,7 @@ describe('buffered-runner', function() { }); describe('when subsequent files already started running', function() { it('should cleanly terminate the thread pool', function(done) { - const options = {}; + const options = {reporter: runner._workerReporter}; const err = { __type: 'Error', message: 'oh no' @@ -398,7 +466,7 @@ describe('buffered-runner', function() { describe('when an event contains an error and has positive failures', function() { describe('when subsequent files have not yet been run', function() { it('should cleanly terminate the thread pool', function(done) { - const options = {}; + const options = {reporter: runner._workerReporter}; const err = { __type: 'Error', message: 'oh no' @@ -442,7 +510,7 @@ describe('buffered-runner', function() { describe('when subsequent files already started running', function() { it('should cleanly terminate the thread pool', function(done) { - const options = {}; + const options = {reporter: runner._workerReporter}; const err = { __type: 'Error', message: 'oh no' @@ -502,7 +570,7 @@ describe('buffered-runner', function() { describe('when subsequent files have not yet been run', function() { it('should cleanly terminate the thread pool', function(done) { - const options = {}; + const options = {reporter: runner._workerReporter}; const err = { __type: 'Error', message: 'oh no' @@ -546,6 +614,44 @@ describe('buffered-runner', function() { }); }); }); + + describe('linkPartialObjects()', function() { + let runner; + + beforeEach(function() { + runner = new ParallelBufferedRunner(suite); + }); + + it('should return the runner', function() { + expect(runner.linkPartialObjects(), 'to be', runner); + }); + + // avoid testing implementation details; don't check _linkPartialObjects + }); + + describe('isParallelMode()', function() { + let runner; + + beforeEach(function() { + runner = new ParallelBufferedRunner(suite); + }); + + it('should return true', function() { + expect(runner.isParallelMode(), 'to be true'); + }); + }); + + describe('workerReporter()', function() { + let runner; + + beforeEach(function() { + runner = new ParallelBufferedRunner(suite); + }); + + it('should return its context', function() { + expect(runner.workerReporter(), 'to be', runner); + }); + }); }); }); }); diff --git a/test/unit/mocha.spec.js b/test/unit/mocha.spec.js index 3834f0adf5..eea44596c7 100644 --- a/test/unit/mocha.spec.js +++ b/test/unit/mocha.spec.js @@ -52,17 +52,19 @@ describe('Mocha', function() { sinon.stub(Mocha.reporters, 'base').returns({}); sinon.stub(Mocha.reporters, 'spec').returns({}); - runner = Object.assign(sinon.createStubInstance(EventEmitter), { + runner = { + ...sinon.createStubInstance(EventEmitter), runAsync: sinon.stub().resolves(0), globals: sinon.stub(), grep: sinon.stub(), dispose: sinon.stub() - }); + }; Runner = sinon.stub(Mocha, 'Runner').returns(runner); // the Runner constructor is the main export, and constants is a static prop. // we don't need the constants themselves, but the object cannot be undefined Runner.constants = {}; - suite = Object.assign(sinon.createStubInstance(EventEmitter), { + suite = { + ...sinon.createStubInstance(EventEmitter), slow: sinon.stub(), timeout: sinon.stub(), bail: sinon.stub(), @@ -72,7 +74,7 @@ describe('Mocha', function() { beforeEach: sinon.stub(), afterAll: sinon.stub(), afterEach: sinon.stub() - }); + }; Suite = sinon.stub(Mocha, 'Suite').returns(suite); Suite.constants = {}; diff --git a/test/unit/runner.spec.js b/test/unit/runner.spec.js index abc5eef877..8f4c61419d 100644 --- a/test/unit/runner.spec.js +++ b/test/unit/runner.spec.js @@ -1,28 +1,31 @@ 'use strict'; -var path = require('path'); -var sinon = require('sinon'); -var Mocha = require('../../lib/mocha'); -var Pending = require('../../lib/pending'); -var Suite = Mocha.Suite; -var Runner = Mocha.Runner; -var Test = Mocha.Test; -var Runnable = Mocha.Runnable; -var Hook = Mocha.Hook; -var noop = Mocha.utils.noop; -var errors = require('../../lib/errors'); -var EVENT_HOOK_BEGIN = Runner.constants.EVENT_HOOK_BEGIN; -var EVENT_HOOK_END = Runner.constants.EVENT_HOOK_END; -var EVENT_TEST_FAIL = Runner.constants.EVENT_TEST_FAIL; -var EVENT_TEST_PASS = Runner.constants.EVENT_TEST_PASS; -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; -var STATE_IDLE = Runner.constants.STATE_IDLE; -var STATE_RUNNING = Runner.constants.STATE_RUNNING; -var STATE_STOPPED = Runner.constants.STATE_STOPPED; +const path = require('path'); +const sinon = require('sinon'); +const Mocha = require('../../lib/mocha'); +const Pending = require('../../lib/pending'); +const {Suite, Runner, Test, Hook, Runnable} = Mocha; +const {noop} = Mocha.utils; +const { + FATAL, + MULTIPLE_DONE, + UNSUPPORTED +} = require('../../lib/errors').constants; + +const { + EVENT_HOOK_BEGIN, + EVENT_HOOK_END, + EVENT_TEST_FAIL, + EVENT_TEST_PASS, + EVENT_TEST_RETRY, + EVENT_TEST_END, + EVENT_RUN_END, + EVENT_SUITE_END, + STATE_IDLE, + STATE_RUNNING, + STATE_STOPPED +} = Runner.constants; +const {STATE_FAILED} = Mocha.Runnable.constants; describe('Runner', function() { afterEach(function() { @@ -429,7 +432,7 @@ describe('Runner', function() { var test = new Test('test', function() {}); suite.addTest(test); var err = new Error(); - err.code = errors.constants.MULTIPLE_DONE; + err.code = MULTIPLE_DONE; expect( function() { runner.fail(test, err); @@ -451,7 +454,7 @@ describe('Runner', function() { }, 'to throw', { - code: errors.constants.FATAL + code: FATAL } ); }); @@ -887,7 +890,7 @@ describe('Runner', function() { describe('when called with a non-Runner context', function() { it('should throw', function() { expect(runner._uncaught.bind({}), 'to throw', { - code: errors.constants.FATAL + code: FATAL }); }); }); @@ -1181,5 +1184,23 @@ describe('Runner', function() { }); }); }); + + describe('linkPartialObjects()', function() { + it('should return the Runner', function() { + expect(runner.linkPartialObjects(), 'to be', runner); + }); + }); + + describe('isParallelMode()', function() { + it('should return false', function() { + expect(runner.isParallelMode(), 'to be false'); + }); + }); + + describe('workerReporter()', function() { + it('should throw', function() { + expect(() => runner.workerReporter(), 'to throw', {code: UNSUPPORTED}); + }); + }); }); }); diff --git a/test/unit/suite.spec.js b/test/unit/suite.spec.js index 05cb75f5de..1f52c9c06f 100644 --- a/test/unit/suite.spec.js +++ b/test/unit/suite.spec.js @@ -1,9 +1,8 @@ 'use strict'; -var Mocha = require('../../lib/mocha'); -var Suite = Mocha.Suite; -var Test = Mocha.Test; -var sinon = require('sinon'); +const Mocha = require('../../lib/mocha'); +const {Suite, Test, Context} = Mocha; +const sinon = require('sinon'); const errors = require('../../lib/errors'); function supportsFunctionNames() { @@ -16,678 +15,661 @@ describe('Suite', function() { sinon.restore(); }); - describe('.clone()', function() { - beforeEach(function() { - this.suite = new Suite('To be cloned', {}, true); - this.suite._timeout = 3043; - this.suite._slow = 101; - this.suite._bail = true; - this.suite.suites.push(1); - this.suite.tests.push('hello'); - this.suite._beforeEach.push(2); - this.suite._beforeAll.push(3); - this.suite._afterEach.push(4); - this.suite._afterAll.push(5); - }); - - it('should copy the title', function() { - expect(this.suite.clone().title, 'to be', 'To be cloned'); - }); + describe('instance method', function() { + let suite; - it('should copy the timeout value', function() { - expect(this.suite.clone().timeout(), 'to be', 3043); - }); + describe('clone()', function() { + beforeEach(function() { + suite = new Suite('To be cloned', {}, true); + suite._timeout = 3043; + suite._slow = 101; + suite._bail = true; + suite.suites.push(1); + suite.tests.push('hello'); + suite._beforeEach.push(2); + suite._beforeAll.push(3); + suite._afterEach.push(4); + suite._afterAll.push(5); + }); + + it('should clone the Suite, omitting children', function() { + expect(suite.clone(), 'to satisfy', { + title: 'To be cloned', + _timeout: 3043, + _slow: 101, + _bail: true, + suites: expect.it('to be empty'), + tests: expect.it('to be empty'), + _beforeEach: expect.it('to be empty'), + _beforeAll: expect.it('to be empty'), + _afterEach: expect.it('to be empty'), + _afterAll: expect.it('to be empty'), + root: true + }).and('not to be', suite); + }); + }); + + describe('reset()', function() { + beforeEach(function() { + suite = new Suite('Suite to be reset', function() {}); + }); - it('should copy the slow value', function() { - expect(this.suite.clone().slow(), 'to be', 101); - }); + it('should reset the `delayed` state', function() { + suite.delayed = true; + suite.reset(); + expect(suite.delayed, 'to be', false); + }); - it('should copy the bail value', function() { - expect(this.suite.clone().bail(), 'to be', true); - }); + it('should forward reset to suites and tests', function() { + const childSuite = new Suite('child suite', suite.context); + const test = new Test('test', function() {}); + suite.addSuite(childSuite); + suite.addTest(test); + const testResetStub = sinon.stub(test, 'reset'); + const suiteResetStub = sinon.stub(childSuite, 'reset'); + suite.reset(); + expect(testResetStub, 'was called once'); + expect(suiteResetStub, 'was called once'); + }); - it('should not copy the values from the suites array', function() { - expect(this.suite.clone().suites, 'to be empty'); - }); + it('should forward reset to all hooks', function() { + suite.beforeEach(function() {}); + suite.afterEach(function() {}); + suite.beforeAll(function() {}); + suite.afterAll(function() {}); + sinon.stub(suite.getHooks('beforeEach')[0], 'reset'); + sinon.stub(suite.getHooks('afterEach')[0], 'reset'); + sinon.stub(suite.getHooks('beforeAll')[0], 'reset'); + sinon.stub(suite.getHooks('afterAll')[0], 'reset'); - it('should not copy the values from the tests array', function() { - expect(this.suite.clone().tests, 'to be empty'); - }); + suite.reset(); - it('should not copy the values from the _beforeEach array', function() { - expect(this.suite.clone()._beforeEach, 'to be empty'); + expect(suite.getHooks('beforeEach')[0].reset, 'was called once'); + expect(suite.getHooks('afterEach')[0].reset, 'was called once'); + expect(suite.getHooks('beforeAll')[0].reset, 'was called once'); + expect(suite.getHooks('afterAll')[0].reset, 'was called once'); + }); }); - it('should not copy the values from the _beforeAll array', function() { - expect(this.suite.clone()._beforeAll, 'to be empty'); - }); + describe('timeout()', function() { + beforeEach(function() { + suite = new Suite('A Suite'); + }); - it('should not copy the values from the _afterEach array', function() { - expect(this.suite.clone()._afterEach, 'to be empty'); - }); + describe('when no argument is passed', function() { + it('should return the timeout value', function() { + expect(suite.timeout(), 'to be', 2000); + }); + }); - it('should not copy the values from the _afterAll array', function() { - expect(this.suite.clone()._afterAll, 'to be empty'); + describe('when argument is passed', function() { + it('should return the Suite object', function() { + const newSuite = suite.timeout(5000); + expect(newSuite.timeout(), 'to be', 5000); + }); + }); }); - it('should copy the root property', function() { - expect(this.suite.clone().root, 'to be', true); - }); - }); + describe('slow()', function() { + beforeEach(function() { + suite = new Suite('A Suite'); + }); - describe('.reset()', function() { - beforeEach(function() { - this.suite = new Suite('Suite to be reset', function() {}); - }); + describe('when given a string', function() { + it('should parse it', function() { + suite.slow('5 seconds'); + expect(suite.slow(), 'to be', 5000); + }); + }); - it('should reset the `delayed` state', function() { - this.suite.delayed = true; - this.suite.reset(); - expect(this.suite.delayed, 'to be', false); - }); + describe('when no argument is passed', function() { + it('should return the slow value', function() { + expect(suite.slow(), 'to be', 75); + }); + }); - 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 = sinon.stub(test, 'reset'); - var suiteResetStub = sinon.stub(childSuite, 'reset'); - this.suite.reset(); - expect(testResetStub, 'was called once'); - expect(suiteResetStub, 'was called once'); + describe('when argument is passed', function() { + it('should return the Suite object', function() { + const newSuite = suite.slow(5000); + expect(newSuite.slow(), 'to be', 5000); + }); + }); }); - 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('bail()', function() { + beforeEach(function() { + suite = new Suite('A Suite'); + suite._bail = true; + }); - describe('.timeout()', function() { - beforeEach(function() { - this.suite = new Suite('A Suite'); - }); + describe('when no argument is passed', function() { + it('should return the bail value', function() { + expect(suite.bail(), 'to be', true); + }); + }); - describe('when no argument is passed', function() { - it('should return the timeout value', function() { - expect(this.suite.timeout(), 'to be', 2000); + describe('when argument is passed', function() { + it('should return the Suite object', function() { + const newSuite = suite.bail(false); + expect(newSuite.bail(), 'to be', false); + }); }); }); - describe('when argument is passed', function() { - it('should return the Suite object', function() { - var newSuite = this.suite.timeout(5000); - expect(newSuite.timeout(), 'to be', 5000); + describe('beforeAll()', function() { + beforeEach(function() { + suite = new Suite('A Suite'); }); - }); - }); - describe('.slow()', function() { - beforeEach(function() { - this.suite = new Suite('A Suite'); - }); + describe('wraps the passed in function in a Hook', function() { + it('adds it to _beforeAll', function() { + const fn = function() {}; + suite.beforeAll(fn); - describe('when given a string', function() { - it('should parse it', function() { - this.suite.slow('5 seconds'); - expect(this.suite.slow(), 'to be', 5000); - }); - }); + expect(suite._beforeAll, 'to have length', 1); + const beforeAllItem = suite._beforeAll[0]; + expect(beforeAllItem.title, 'to match', /^"before all" hook/); + expect(beforeAllItem.fn, 'to be', fn); + }); + + it('appends title to hook', function() { + const fn = function() {}; + suite.beforeAll('test', fn); + + expect(suite._beforeAll, 'to have length', 1); + const beforeAllItem = suite._beforeAll[0]; + expect(beforeAllItem.title, 'to be', '"before all" hook: test'); + expect(beforeAllItem.fn, 'to be', fn); + }); - describe('when no argument is passed', function() { - it('should return the slow value', function() { - expect(this.suite.slow(), 'to be', 75); + it('uses function name if available', function() { + if (!supportsFunctionNames()) { + this.skip(); + return; + } + function namedFn() {} + suite.beforeAll(namedFn); + const beforeAllItem = suite._beforeAll[0]; + expect(beforeAllItem.title, 'to be', '"before all" hook: namedFn'); + expect(beforeAllItem.fn, 'to be', namedFn); + }); }); }); - describe('when argument is passed', function() { - it('should return the Suite object', function() { - var newSuite = this.suite.slow(5000); - expect(newSuite.slow(), 'to be', 5000); + describe('afterAll()', function() { + beforeEach(function() { + suite = new Suite('A Suite'); }); - }); - }); - describe('.bail()', function() { - beforeEach(function() { - this.suite = new Suite('A Suite'); - this.suite._bail = true; - }); + describe('wraps the passed in function in a Hook', function() { + it('adds it to _afterAll', function() { + const fn = function() {}; + suite.afterAll(fn); - describe('when no argument is passed', function() { - it('should return the bail value', function() { - expect(this.suite.bail(), 'to be', true); - }); - }); + expect(suite._afterAll, 'to have length', 1); + const afterAllItem = suite._afterAll[0]; + expect(afterAllItem.title, 'to match', /^"after all" hook/); + expect(afterAllItem.fn, 'to be', fn); + }); + it('appends title to hook', function() { + const fn = function() {}; + suite.afterAll('test', fn); + + expect(suite._afterAll, 'to have length', 1); + const beforeAllItem = suite._afterAll[0]; + expect(beforeAllItem.title, 'to be', '"after all" hook: test'); + expect(beforeAllItem.fn, 'to be', fn); + }); - describe('when argument is passed', function() { - it('should return the Suite object', function() { - var newSuite = this.suite.bail(false); - expect(newSuite.bail(), 'to be', false); + it('uses function name if available', function() { + if (!supportsFunctionNames()) { + this.skip(); + return; + } + function namedFn() {} + suite.afterAll(namedFn); + const afterAllItem = suite._afterAll[0]; + expect(afterAllItem.title, 'to be', '"after all" hook: namedFn'); + expect(afterAllItem.fn, 'to be', namedFn); + }); }); }); - }); - describe('.beforeAll()', function() { - beforeEach(function() { - this.suite = new Suite('A Suite'); - }); - - describe('wraps the passed in function in a Hook', function() { - it('adds it to _beforeAll', function() { - var fn = function() {}; - this.suite.beforeAll(fn); + describe('beforeEach()', function() { + let suite; - expect(this.suite._beforeAll, 'to have length', 1); - var beforeAllItem = this.suite._beforeAll[0]; - expect(beforeAllItem.title, 'to match', /^"before all" hook/); - expect(beforeAllItem.fn, 'to be', fn); + beforeEach(function() { + suite = new Suite('A Suite'); }); - it('appends title to hook', function() { - var fn = function() {}; - this.suite.beforeAll('test', fn); + describe('wraps the passed in function in a Hook', function() { + it('adds it to _beforeEach', function() { + const fn = function() {}; + suite.beforeEach(fn); - expect(this.suite._beforeAll, 'to have length', 1); - var beforeAllItem = this.suite._beforeAll[0]; - expect(beforeAllItem.title, 'to be', '"before all" hook: test'); - expect(beforeAllItem.fn, 'to be', fn); - }); + expect(suite._beforeEach, 'to have length', 1); + const beforeEachItem = suite._beforeEach[0]; + expect(beforeEachItem.title, 'to match', /^"before each" hook/); + expect(beforeEachItem.fn, 'to be', fn); + }); - it('uses function name if available', function() { - if (!supportsFunctionNames()) { - this.skip(); - return; - } - function namedFn() {} - this.suite.beforeAll(namedFn); - var beforeAllItem = this.suite._beforeAll[0]; - expect(beforeAllItem.title, 'to be', '"before all" hook: namedFn'); - expect(beforeAllItem.fn, 'to be', namedFn); - }); - }); - }); + it('appends title to hook', function() { + const fn = function() {}; + suite.beforeEach('test', fn); - describe('.afterAll()', function() { - beforeEach(function() { - this.suite = new Suite('A Suite'); - }); + expect(suite._beforeEach, 'to have length', 1); + const beforeAllItem = suite._beforeEach[0]; + expect(beforeAllItem.title, 'to be', '"before each" hook: test'); + expect(beforeAllItem.fn, 'to be', fn); + }); - describe('wraps the passed in function in a Hook', function() { - it('adds it to _afterAll', function() { - var fn = function() {}; - this.suite.afterAll(fn); - - expect(this.suite._afterAll, 'to have length', 1); - var afterAllItem = this.suite._afterAll[0]; - expect(afterAllItem.title, 'to match', /^"after all" hook/); - expect(afterAllItem.fn, 'to be', fn); - }); - it('appends title to hook', function() { - var fn = function() {}; - this.suite.afterAll('test', fn); - - expect(this.suite._afterAll, 'to have length', 1); - var beforeAllItem = this.suite._afterAll[0]; - expect(beforeAllItem.title, 'to be', '"after all" hook: test'); - expect(beforeAllItem.fn, 'to be', fn); - }); - - it('uses function name if available', function() { - if (!supportsFunctionNames()) { - this.skip(); - return; - } - function namedFn() {} - this.suite.afterAll(namedFn); - var afterAllItem = this.suite._afterAll[0]; - expect(afterAllItem.title, 'to be', '"after all" hook: namedFn'); - expect(afterAllItem.fn, 'to be', namedFn); + it('uses function name if available', function() { + if (!supportsFunctionNames()) { + this.skip(); + return; + } + function namedFn() {} + suite.beforeEach(namedFn); + const beforeEachItem = suite._beforeEach[0]; + expect(beforeEachItem.title, 'to be', '"before each" hook: namedFn'); + expect(beforeEachItem.fn, 'to be', namedFn); + }); }); - }); - }); - describe('.beforeEach()', function() { - var suite; + describe('when the suite is pending', function() { + beforeEach(function() { + suite.pending = true; + }); - beforeEach(function() { - suite = new Suite('A Suite'); + it('should not create a hook', function() { + suite.beforeEach(function() {}); + expect(suite._beforeEach, 'to be empty'); + }); + }); }); - describe('wraps the passed in function in a Hook', function() { - it('adds it to _beforeEach', function() { - var fn = function() {}; - suite.beforeEach(fn); - - expect(suite._beforeEach, 'to have length', 1); - var beforeEachItem = suite._beforeEach[0]; - expect(beforeEachItem.title, 'to match', /^"before each" hook/); - expect(beforeEachItem.fn, 'to be', fn); + describe('afterEach()', function() { + beforeEach(function() { + suite = new Suite('A Suite'); }); - it('appends title to hook', function() { - var fn = function() {}; - suite.beforeEach('test', fn); + describe('wraps the passed in function in a Hook', function() { + it('adds it to _afterEach', function() { + const fn = function() {}; + suite.afterEach(fn); - expect(suite._beforeEach, 'to have length', 1); - var beforeAllItem = suite._beforeEach[0]; - expect(beforeAllItem.title, 'to be', '"before each" hook: test'); - expect(beforeAllItem.fn, 'to be', fn); - }); + expect(suite._afterEach, 'to have length', 1); + const afterEachItem = suite._afterEach[0]; + expect(afterEachItem.title, 'to match', /^"after each" hook/); + expect(afterEachItem.fn, 'to be', fn); + }); + + it('appends title to hook', function() { + const fn = function() {}; + suite.afterEach('test', fn); - it('uses function name if available', function() { - if (!supportsFunctionNames()) { - this.skip(); - return; - } - function namedFn() {} - suite.beforeEach(namedFn); - var beforeEachItem = suite._beforeEach[0]; - expect(beforeEachItem.title, 'to be', '"before each" hook: namedFn'); - expect(beforeEachItem.fn, 'to be', namedFn); + expect(suite._afterEach, 'to have length', 1); + const beforeAllItem = suite._afterEach[0]; + expect(beforeAllItem.title, 'to be', '"after each" hook: test'); + expect(beforeAllItem.fn, 'to be', fn); + }); + + it('uses function name if available', function() { + if (!supportsFunctionNames()) { + this.skip(); + return; + } + function namedFn() {} + suite.afterEach(namedFn); + const afterEachItem = suite._afterEach[0]; + expect(afterEachItem.title, 'to be', '"after each" hook: namedFn'); + expect(afterEachItem.fn, 'to be', namedFn); + }); }); }); - describe('when the suite is pending', function() { - beforeEach(function() { - suite.pending = true; + describe('create()', function() { + let first; + let second; + + before(function() { + first = new Suite('Root suite', {}, true); + second = new Suite('RottenRoot suite', {}, true); + first.addSuite(second); }); - it('should not create a hook', function() { - suite.beforeEach(function() {}); - expect(suite._beforeEach, 'to be empty'); + it('does not create a second root suite', function() { + expect(second.parent, 'to be', first); + expect(first.root, 'to be', true); + expect(second.root, 'to be', false); }); - }); - }); - describe('.afterEach()', function() { - beforeEach(function() { - this.suite = new Suite('A Suite'); + it('does not denote the root suite by being titleless', function() { + const emptyTitleSuite = Suite.create(second, ''); + expect(emptyTitleSuite.parent, 'to be', second); + expect(emptyTitleSuite.root, 'to be', false); + expect(second.root, 'to be', false); + }); }); - describe('wraps the passed in function in a Hook', function() { - it('adds it to _afterEach', function() { - var fn = function() {}; - this.suite.afterEach(fn); + describe('addSuite()', function() { + let first; + let second; - expect(this.suite._afterEach, 'to have length', 1); - var afterEachItem = this.suite._afterEach[0]; - expect(afterEachItem.title, 'to match', /^"after each" hook/); - expect(afterEachItem.fn, 'to be', fn); + beforeEach(function() { + first = new Suite('First suite'); + first.timeout(4002); + first.slow(200); + second = new Suite('Second suite'); + first.addSuite(second); }); - it('appends title to hook', function() { - var fn = function() {}; - this.suite.afterEach('test', fn); - - expect(this.suite._afterEach, 'to have length', 1); - var beforeAllItem = this.suite._afterEach[0]; - expect(beforeAllItem.title, 'to be', '"after each" hook: test'); - expect(beforeAllItem.fn, 'to be', fn); + it('sets the parent on the added Suite', function() { + expect(second.parent, 'to be', first); }); - it('uses function name if available', function() { - if (!supportsFunctionNames()) { - this.skip(); - return; - } - function namedFn() {} - this.suite.afterEach(namedFn); - var afterEachItem = this.suite._afterEach[0]; - expect(afterEachItem.title, 'to be', '"after each" hook: namedFn'); - expect(afterEachItem.fn, 'to be', namedFn); + it('copies the timeout value', function() { + expect(second.timeout(), 'to be', 4002); }); - }); - }); - - describe('.create()', function() { - before(function() { - this.first = new Suite('Root suite', {}, true); - this.second = new Suite('RottenRoot suite', {}, true); - this.first.addSuite(this.second); - }); - it('does not create a second root suite', function() { - expect(this.second.parent, 'to be', this.first); - expect(this.first.root, 'to be', true); - expect(this.second.root, 'to be', false); - }); - - it('does not denote the root suite by being titleless', function() { - var emptyTitleSuite = Suite.create(this.second, ''); - expect(emptyTitleSuite.parent, 'to be', this.second); - expect(emptyTitleSuite.root, 'to be', false); - expect(this.second.root, 'to be', false); - }); - }); + it('copies the slow value', function() { + expect(second.slow(), 'to be', 200); + }); - describe('.addSuite()', function() { - beforeEach(function() { - this.first = new Suite('First suite'); - this.first.timeout(4002); - this.first.slow(200); - this.second = new Suite('Second suite'); - this.first.addSuite(this.second); - }); + it('adds the suite to the suites collection', function() { + expect(first.suites, 'to have length', 1); + expect(first.suites[0], 'to be', second); + }); - it('sets the parent on the added Suite', function() { - expect(this.second.parent, 'to be', this.first); + it('treats suite as pending if its parent is pending', function() { + first.pending = true; + expect(second.isPending(), 'to be', true); + }); }); - it('copies the timeout value', function() { - expect(this.second.timeout(), 'to be', 4002); - }); + describe('addTest()', function() { + let test; - it('copies the slow value', function() { - expect(this.second.slow(), 'to be', 200); - }); + beforeEach(function() { + suite = new Suite('A Suite', new Context()); + suite.timeout(4002); + test = new Test('test'); + suite.addTest(test); + }); - it('adds the suite to the suites collection', function() { - expect(this.first.suites, 'to have length', 1); - expect(this.first.suites[0], 'to be', this.second); - }); + it('sets the parent on the added test', function() { + expect(test.parent, 'to be', suite); + }); - it('treats suite as pending if its parent is pending', function() { - this.first.pending = true; - expect(this.second.isPending(), 'to be', true); - }); - }); + it('copies the timeout value', function() { + expect(test.timeout(), 'to be', 4002); + }); - // describe('.addTest()', function(){ - // beforeEach(function(){ - // this.suite = new Suite('A Suite', new Context); - // this.suite.timeout(4002); - // this.test = new Test('test'); - // this.suite.addTest(this.test); - // }); - // - // it('sets the parent on the added test', function(){ - // expect(this.test.parent, 'to be', this.suite); - // }); - // - // it('copies the timeout value', function(){ - // expect(this.test.timeout(), 'to be', 4002); - // }); - // - // it('adds the test to the tests collection', function(){ - // expect(this.suite.tests, 'to have length', 1); - // expect(this.suite.tests[0], 'to be', this.test); - // }); - // }); - - describe('.fullTitle()', function() { - beforeEach(function() { - this.suite = new Suite('A Suite'); + it('adds the test to the tests collection', function() { + expect(suite.tests, 'to satisfy', [test]).and('to have length', 1); + }); }); - describe('when there is no parent', function() { - it('returns the suite title', function() { - expect(this.suite.fullTitle(), 'to be', 'A Suite'); + describe('fullTitle()', function() { + beforeEach(function() { + suite = new Suite('A Suite'); }); - }); - describe('when there is a parent', function() { - it("returns the combination of parent's and suite's title", function() { - var parentSuite = new Suite('I am a parent'); - parentSuite.addSuite(this.suite); - expect(this.suite.fullTitle(), 'to be', 'I am a parent A Suite'); + describe('when there is no parent', function() { + it('returns the suite title', function() { + expect(suite.fullTitle(), 'to be', 'A Suite'); + }); }); - }); - }); - describe('.titlePath()', function() { - beforeEach(function() { - this.suite = new Suite('A Suite'); + describe('when there is a parent', function() { + it("returns the combination of parent's and suite's title", function() { + const parentSuite = new Suite('I am a parent'); + parentSuite.addSuite(suite); + expect(suite.fullTitle(), 'to be', 'I am a parent A Suite'); + }); + }); }); - describe('when there is no parent', function() { - it('returns the suite title', function() { - expect(this.suite.titlePath(), 'to equal', ['A Suite']); + describe('titlePath()', function() { + beforeEach(function() { + suite = new Suite('A Suite'); }); - }); - describe('when there is a parent', function() { - describe('the parent is the root suite', function() { + describe('when there is no parent', function() { it('returns the suite title', function() { - var rootSuite = new Suite('', {}, true); - rootSuite.addSuite(this.suite); - expect(this.suite.titlePath(), 'to equal', ['A Suite']); + expect(suite.titlePath(), 'to equal', ['A Suite']); }); }); - describe('the parent is not the root suite', function() { - it("returns the concatenation of parent's and suite's title", function() { - var parentSuite = new Suite('I am a parent'); - parentSuite.addSuite(this.suite); - expect(this.suite.titlePath(), 'to equal', [ - 'I am a parent', - 'A Suite' - ]); + describe('when there is a parent', function() { + describe('the parent is the root suite', function() { + it('returns the suite title', function() { + const rootSuite = new Suite('', {}, true); + rootSuite.addSuite(suite); + expect(suite.titlePath(), 'to equal', ['A Suite']); + }); + }); + + describe('the parent is not the root suite', function() { + it("returns the concatenation of parent's and suite's title", function() { + const parentSuite = new Suite('I am a parent'); + parentSuite.addSuite(suite); + expect(suite.titlePath(), 'to equal', ['I am a parent', 'A Suite']); + }); }); }); }); - }); - describe('.total()', function() { - beforeEach(function() { - this.suite = new Suite('A Suite'); - }); + describe('total()', function() { + beforeEach(function() { + suite = new Suite('A Suite'); + }); - describe('when there are no nested suites or tests', function() { - it('should return 0', function() { - expect(this.suite.total(), 'to be', 0); + describe('when there are no nested suites or tests', function() { + it('should return 0', function() { + expect(suite.total(), 'to be', 0); + }); }); - }); - describe('when there are several tests in the suite', function() { - it('should return the number', function() { - this.suite.addTest(new Test('a child test')); - this.suite.addTest(new Test('another child test')); - expect(this.suite.total(), 'to be', 2); + describe('when there are several tests in the suite', function() { + it('should return the number', function() { + suite.addTest(new Test('a child test')); + suite.addTest(new Test('another child test')); + expect(suite.total(), 'to be', 2); + }); }); }); - }); - describe('.eachTest(fn)', function() { - beforeEach(function() { - this.suite = new Suite('A Suite'); - }); + describe('eachTest(fn)', function() { + beforeEach(function() { + suite = new Suite('A Suite'); + }); - describe('when there are no nested suites or tests', function() { - it('should return 0', function() { - var n = 0; - function fn() { - n++; - } - this.suite.eachTest(fn); - expect(n, 'to be', 0); + describe('when there are no nested suites or tests', function() { + it('should return 0', function() { + let n = 0; + function fn() { + n++; + } + suite.eachTest(fn); + expect(n, 'to be', 0); + }); }); - }); - describe('when there are several tests in the suite', function() { - it('should return the number', function() { - this.suite.addTest(new Test('a child test')); - this.suite.addTest(new Test('another child test')); + describe('when there are several tests in the suite', function() { + it('should return the number', function() { + suite.addTest(new Test('a child test')); + suite.addTest(new Test('another child test')); - var n = 0; - function fn() { - n++; - } - this.suite.eachTest(fn); - expect(n, 'to be', 2); + let n = 0; + function fn() { + n++; + } + suite.eachTest(fn); + expect(n, 'to be', 2); + }); }); - }); - describe('when there are several levels of nested suites', function() { - it('should return the number', function() { - this.suite.addTest(new Test('a child test')); - var suite = new Suite('a child suite'); - suite.addTest(new Test('a test in a child suite')); - this.suite.addSuite(suite); + describe('when there are several levels of nested suites', function() { + it('should return the number', function() { + suite.addTest(new Test('a child test')); + const childSuite = new Suite('a child suite'); + childSuite.addTest(new Test('a test in a child suite')); + suite.addSuite(childSuite); - var n = 0; - function fn() { - n++; - } - this.suite.eachTest(fn); - expect(n, 'to be', 2); + let n = 0; + function fn() { + n++; + } + suite.eachTest(fn); + expect(n, 'to be', 2); + }); }); }); - }); - describe('constructor', function() { - beforeEach(function() { - sinon.stub(errors, 'deprecate'); - }); + describe('constructor', function() { + beforeEach(function() { + sinon.stub(errors, 'deprecate'); + }); - /* eslint no-new: off */ - it("should throw an error if the title isn't a string", function() { - expect(function() { - new Suite(undefined, 'root'); - }, 'to throw'); + /* eslint no-new: off */ + it("should throw an error if the title isn't a string", function() { + expect(function() { + new Suite(undefined, 'root'); + }, 'to throw'); - expect(function() { - new Suite(function() {}, 'root'); - }, 'to throw'); - }); + expect(function() { + new Suite(function() {}, 'root'); + }, 'to throw'); + }); - it('should not throw if the title is a string', function() { - expect(function() { - new Suite('Bdd suite', 'root'); - }, 'not to throw'); - }); + it('should not throw if the title is a string', function() { + expect(function() { + new Suite('Bdd suite', 'root'); + }, 'not to throw'); + }); - it('should report listened-for deprecated events as deprecated', function() { - new Suite('foo').on(Suite.constants.EVENT_SUITE_ADD_TEST, function() {}); - expect(errors.deprecate, 'to have a call satisfying', [ - /Event "[^"]+" is deprecated/i - ]); + it('should report listened-for deprecated events as deprecated', function() { + new Suite('foo').on( + Suite.constants.EVENT_SUITE_ADD_TEST, + function() {} + ); + expect(errors.deprecate, 'to have a call satisfying', [ + /Event "[^"]+" is deprecated/i + ]); + }); }); - }); - describe('timeout()', function() { - it('should convert a string to milliseconds', function() { - var suite = new Suite('some suite'); - suite.timeout('100'); - expect(suite.timeout(), 'to be', 100); + describe('timeout()', function() { + it('should convert a string to milliseconds', function() { + const suite = new Suite('some suite'); + suite.timeout('100'); + expect(suite.timeout(), 'to be', 100); + }); }); - }); - describe('hasOnly()', function() { - it('should return true if a test has `only`', function() { - var suite = new Suite('foo'); - var test = new Test('bar'); + describe('hasOnly()', function() { + it('should return true if a test has `only`', function() { + const suite = new Suite('foo'); + const test = new Test('bar'); - suite.appendOnlyTest(test); + suite.appendOnlyTest(test); - expect(suite.hasOnly(), 'to be', true); - }); + expect(suite.hasOnly(), 'to be', true); + }); - it('should return true if a suite has `only`', function() { - var suite = new Suite('foo'); - var nested = new Suite('bar'); + it('should return true if a suite has `only`', function() { + const suite = new Suite('foo'); + const nested = new Suite('bar'); - suite.appendOnlySuite(nested); + suite.appendOnlySuite(nested); - expect(suite.hasOnly(), 'to be', true); - }); + expect(suite.hasOnly(), 'to be', true); + }); - it('should return true if nested suite has `only`', function() { - var suite = new Suite('foo'); - var nested = new Suite('bar'); - var test = new Test('baz'); + it('should return true if nested suite has `only`', function() { + const suite = new Suite('foo'); + const nested = new Suite('bar'); + const test = new Test('baz'); - nested.appendOnlyTest(test); - // `nested` has a `only` test, but `suite` doesn't know about it - suite.suites.push(nested); + nested.appendOnlyTest(test); + // `nested` has a `only` test, but `suite` doesn't know about it + suite.suites.push(nested); - expect(suite.hasOnly(), 'to be', true); - }); + expect(suite.hasOnly(), 'to be', true); + }); - it('should return false if no suite or test is marked `only`', function() { - var suite = new Suite('foo'); - var nested = new Suite('bar'); - var test = new Test('baz'); + it('should return false if no suite or test is marked `only`', function() { + const suite = new Suite('foo'); + const nested = new Suite('bar'); + const test = new Test('baz'); - suite.suites.push(nested); - nested.tests.push(test); + suite.suites.push(nested); + nested.tests.push(test); - expect(suite.hasOnly(), 'to be', false); + expect(suite.hasOnly(), 'to be', false); + }); }); - }); - describe('.filterOnly()', function() { - it('should filter out all other tests and suites if a test has `only`', function() { - var suite = new Suite('a'); - var nested = new Suite('b'); - var test = new Test('c'); - var test2 = new Test('d'); + describe('filterOnly()', function() { + it('should filter out all other tests and suites if a test has `only`', function() { + const suite = new Suite('a'); + const nested = new Suite('b'); + const test = new Test('c'); + const test2 = new Test('d'); - suite.suites.push(nested); - suite.appendOnlyTest(test); - suite.tests.push(test2); + suite.suites.push(nested); + suite.appendOnlyTest(test); + suite.tests.push(test2); - suite.filterOnly(); + suite.filterOnly(); - expect(suite, 'to satisfy', { - suites: expect.it('to be empty'), - tests: expect - .it('to have length', 1) - .and('to have an item satisfying', {title: 'c'}) + expect(suite, 'to satisfy', { + suites: expect.it('to be empty'), + tests: expect + .it('to have length', 1) + .and('to have an item satisfying', {title: 'c'}) + }); }); - }); - it('should filter out all other tests and suites if a suite has `only`', function() { - var suite = new Suite('a'); - var nested1 = new Suite('b'); - var nested2 = new Suite('c'); - var test = new Test('d'); - var nestedTest = new Test('e'); + it('should filter out all other tests and suites if a suite has `only`', function() { + const suite = new Suite('a'); + const nested1 = new Suite('b'); + const nested2 = new Suite('c'); + const test = new Test('d'); + const nestedTest = new Test('e'); - nested1.appendOnlyTest(nestedTest); + nested1.appendOnlyTest(nestedTest); - suite.tests.push(test); - suite.suites.push(nested1); - suite.appendOnlySuite(nested1); - suite.suites.push(nested2); + suite.tests.push(test); + suite.suites.push(nested1); + suite.appendOnlySuite(nested1); + suite.suites.push(nested2); - suite.filterOnly(); + suite.filterOnly(); - expect(suite, 'to satisfy', { - suites: expect - .it('to have length', 1) - .and('to have an item satisfying', {title: 'b'}), - tests: expect.it('to be empty') + expect(suite, 'to satisfy', { + suites: expect + .it('to have length', 1) + .and('to have an item satisfying', {title: 'b'}), + tests: expect.it('to be empty') + }); }); }); - }); - describe('.markOnly()', function() { - it('should call appendOnlySuite on parent', function() { - var suite = new Suite('foo'); - var spy = sinon.spy(); - suite.parent = { - appendOnlySuite: spy - }; - suite.markOnly(); - - expect(spy, 'to have a call exhaustively satisfying', [suite]).and( - 'was called once' - ); + describe('markOnly()', function() { + it('should call appendOnlySuite on parent', function() { + const suite = new Suite('foo'); + const spy = sinon.spy(); + suite.parent = { + appendOnlySuite: spy + }; + suite.markOnly(); + + expect(spy, 'to have a call exhaustively satisfying', [suite]).and( + 'was called once' + ); + }); }); }); }); @@ -699,7 +681,7 @@ describe('Test', function() { new Test(function() {}); }, 'to throw'); - expect(function() { + expect(() => { new Test(undefined, function() {}); }, 'to throw'); }); diff --git a/test/unit/utils.spec.js b/test/unit/utils.spec.js index 5acd79dc50..a953ac51ae 100644 --- a/test/unit/utils.spec.js +++ b/test/unit/utils.spec.js @@ -761,4 +761,10 @@ describe('lib/utils', function() { }); }); }); + + describe('uniqueID()', function() { + it('should return a non-empty string', function() { + expect(utils.uniqueID(), 'to be a string').and('not to be empty'); + }); + }); }); From 2e214f3029404044171acea339732cc814ff9e8e Mon Sep 17 00:00:00 2001 From: Christopher Hiller Date: Tue, 29 Sep 2020 16:27:01 -0700 Subject: [PATCH 2/3] remove browser growl impl from wallaby cfg Signed-off-by: Christopher Hiller --- .wallaby.js | 3 ++- lib/hook.js | 3 +-- lib/suite.js | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.wallaby.js b/.wallaby.js index 59083bb5f4..62573c2f97 100644 --- a/.wallaby.js +++ b/.wallaby.js @@ -17,7 +17,8 @@ module.exports = () => { }, 'package.json', 'test/opts/mocha.opts', - 'mocharc.yml' + 'mocharc.yml', + '!lib/browser/growl.js' ], filesWithNoCoverageCalculated: [ 'test/**/*.fixture.js', diff --git a/lib/hook.js b/lib/hook.js index 8ccbd4947f..890086fad2 100644 --- a/lib/hook.js +++ b/lib/hook.js @@ -78,7 +78,6 @@ Hook.prototype.serialize = function serialize() { }, title: this.title, type: this.type, - [MOCHA_ID_PROP_NAME]: this.id, - __mocha_partial__: true + [MOCHA_ID_PROP_NAME]: this.id }; }; diff --git a/lib/suite.js b/lib/suite.js index 78eb1b8ce0..2f16b2f5f6 100644 --- a/lib/suite.js +++ b/lib/suite.js @@ -9,7 +9,7 @@ const Hook = require('./hook'); var { assignNewMochaID, clamp, - constants, + constants: utilsConstants, createMap, defineConstants, getMochaID, @@ -20,7 +20,7 @@ const debug = require('debug')('mocha:suite'); const milliseconds = require('ms'); const errors = require('./errors'); -const {MOCHA_ID_PROP_NAME} = constants; +const {MOCHA_ID_PROP_NAME} = utilsConstants; /** * Expose `Suite`. From 128820513a561ec0a854bba100c6ca4f90c61d8d Mon Sep 17 00:00:00 2001 From: Christopher Hiller Date: Wed, 30 Sep 2020 14:09:00 -0700 Subject: [PATCH 3/3] make invalid event data throw a fatal error the only time this should happen in a non-unit-test context is when a mocha developer introduces a bug also fix a `rewiremock` call in `ParallelBuffered` reporter tests --- lib/nodejs/parallel-buffered-runner.js | 24 ++++--- .../parallel-buffered-runner.spec.js | 67 +++++++++++++++++-- .../reporters/parallel-buffered.spec.js | 2 +- 3 files changed, 76 insertions(+), 17 deletions(-) diff --git a/lib/nodejs/parallel-buffered-runner.js b/lib/nodejs/parallel-buffered-runner.js index c21264d076..6392101d18 100644 --- a/lib/nodejs/parallel-buffered-runner.js +++ b/lib/nodejs/parallel-buffered-runner.js @@ -14,6 +14,7 @@ const {BufferedWorkerPool} = require('./buffered-worker-pool'); const {setInterval, clearInterval} = global; const {createMap, constants} = require('../utils'); const {MOCHA_ID_PROP_NAME} = constants; +const {createFatalError} = require('../errors'); const DEFAULT_WORKER_REPORTER = require.resolve( './reporters/parallel-buffered' @@ -140,15 +141,20 @@ class ParallelBufferedRunner extends Runner { const {parent, prop} = stack.pop(); const obj = parent[prop]; let newObj; - if (obj && typeof obj === 'object' && obj[MOCHA_ID_PROP_NAME]) { - const id = obj[MOCHA_ID_PROP_NAME]; - newObj = this._linkedObjectMap.has(id) - ? Object.assign(this._linkedObjectMap.get(id), obj) - : obj; - this._linkedObjectMap.set(id, newObj); - parent[prop] = newObj; - } else { - newObj = obj; + if (obj && typeof obj === 'object') { + if (obj[MOCHA_ID_PROP_NAME]) { + const id = obj[MOCHA_ID_PROP_NAME]; + newObj = this._linkedObjectMap.has(id) + ? Object.assign(this._linkedObjectMap.get(id), obj) + : obj; + this._linkedObjectMap.set(id, newObj); + parent[prop] = newObj; + } else { + throw createFatalError( + 'Object missing ID received in event data', + obj + ); + } } Object.keys(newObj).forEach(key => { const value = obj[key]; diff --git a/test/node-unit/parallel-buffered-runner.spec.js b/test/node-unit/parallel-buffered-runner.spec.js index 26013f9452..1d5739343b 100644 --- a/test/node-unit/parallel-buffered-runner.spec.js +++ b/test/node-unit/parallel-buffered-runner.spec.js @@ -22,13 +22,14 @@ describe('parallel-buffered-runner', function() { let ParallelBufferedRunner; let suite; let warn; - let cpuCount; + let fatalError; beforeEach(function() { - cpuCount = 1; suite = new Suite('a root suite', {}, true); warn = sinon.stub(); + fatalError = new Error(); + // tests will want to further define the behavior of these. run = sinon.stub(); terminate = sinon.stub(); @@ -48,10 +49,12 @@ describe('parallel-buffered-runner', function() { '../../lib/nodejs/buffered-worker-pool': { BufferedWorkerPool }, - os: { - cpus: sinon.stub().callsFake(() => new Array(cpuCount)) - }, - '../../lib/utils': r.with({warn}).callThrough() + '../../lib/utils': r.with({warn}).callThrough(), + '../../lib/errors': r + .with({ + createFatalError: sinon.stub().returns(fatalError) + }) + .callThrough() }) ); }); @@ -131,7 +134,7 @@ describe('parallel-buffered-runner', function() { describe('when instructed to link objects', function() { beforeEach(function() { - runner.linkPartialObjects(true); + runner._linkPartialObjects = true; }); it('should create object references', function() { @@ -185,6 +188,54 @@ describe('parallel-buffered-runner', function() { } ); }); + + describe('when event data object is missing an ID', function() { + it('should result in an uncaught exception', function(done) { + const options = {reporter: runner._workerReporter}; + sinon.spy(runner, 'uncaught'); + const someSuite = { + title: 'some suite', + [MOCHA_ID_PROP_NAME]: 'bar' + }; + + run.withArgs('some-file.js', options).resolves({ + failureCount: 0, + events: [ + { + eventName: EVENT_SUITE_END, + data: someSuite + }, + { + eventName: EVENT_TEST_PASS, + data: { + title: 'some test', + // note missing ID right here + parent: { + // this stub object points to someSuite with id 'bar' + [MOCHA_ID_PROP_NAME]: 'bar' + } + } + }, + { + eventName: EVENT_SUITE_END, + // ensure we are not passing the _same_ someSuite, + // because we won't get the same one from the subprocess + data: {...someSuite} + } + ] + }); + + runner.run( + () => { + expect(runner.uncaught, 'to have a call satisfying', [ + fatalError + ]); + done(); + }, + {files: ['some-file.js'], options: {}} + ); + }); + }); }); describe('when a worker fails', function() { @@ -352,6 +403,7 @@ describe('parallel-buffered-runner', function() { ); }); }); + describe('when subsequent files already started running', function() { it('should cleanly terminate the thread pool', function(done) { const options = {reporter: runner._workerReporter}; @@ -463,6 +515,7 @@ describe('parallel-buffered-runner', function() { ); }); }); + describe('when an event contains an error and has positive failures', function() { describe('when subsequent files have not yet been run', function() { it('should cleanly terminate the thread pool', function(done) { diff --git a/test/node-unit/reporters/parallel-buffered.spec.js b/test/node-unit/reporters/parallel-buffered.spec.js index e579b6e10d..91c9e4df15 100644 --- a/test/node-unit/reporters/parallel-buffered.spec.js +++ b/test/node-unit/reporters/parallel-buffered.spec.js @@ -29,7 +29,7 @@ describe('ParallelBuffered', function() { beforeEach(function() { runner = new EventEmitter(); ParallelBuffered = rewiremock.proxy( - require.resolve('../../../lib/nodejs/reporters/parallel-buffered'), + () => require('../../../lib/nodejs/reporters/parallel-buffered'), { '../../../lib/nodejs/serializer': { SerializableEvent: {