diff --git a/src/core/browser/browser-runner.js b/src/core/browser/browser-runner.js index d6545f84a..5e0f3b501 100644 --- a/src/core/browser/browser-runner.js +++ b/src/core/browser/browser-runner.js @@ -83,7 +83,7 @@ export function initBrowser (QUnit, window, document) { // as otherwise it will miss the first few or even all synchronous events. // // Priot to QUnit 3.0, the reporter was initialised here, between error handler (above), - // and start (below). As of QUnit 3.0, reporters are initialized by doBegin() within + // and start (below). As of QUnit 3.0, reporters are initialized by doStart() within // QUnit.start(), which is logically the same place, but decoupled from initBrowser(). function autostart () { diff --git a/src/core/config.js b/src/core/config.js index 75b6fb198..c85c71ce6 100644 --- a/src/core/config.js +++ b/src/core/config.js @@ -97,58 +97,29 @@ const config = { // List of defined modules (read-only). modules: [], - // Internal: The first unnamed module - // - // By being defined as the intial value for currentModule, it is the - // receptacle and implied parent for any global tests. It is as if we - // called `QUnit.module( "" );` before any other tests were defined. - // - // If we reach begin() and no tests were put in it, we dequeue it as if it - // never existed, and in that case never expose it to the events and - // callbacks API. - // - // When global tests are defined, then this unnamed module will execute - // as any other module, including moduleStart/moduleDone events etc. - // - // Since this module isn't explicitly created by the user, they have no - // access to add hooks for it. The hooks object is defined to comply - // with internal expectations of test.js, but they will be empty. - // To apply hooks, place tests explicitly in a QUnit.module(), and use - // its hooks accordingly. - // - // For global hooks that apply to all tests and all modules, use QUnit.hooks. - // - // NOTE: This is *not* a "global module". It is not an ancestor of all modules - // and tests. It is merely the parent for any tests defined globally, - // before the first QUnit.module(). As such, the events for this unnamed - // module will fire as normal, right after its last test, and *not* at - // the end of the test run. - // - // NOTE: This also should probably also not become a global module, unless - // we keep it out of the public API. For example, it would likely not - // improve the user interface and plugin behaviour if all modules became - // wrapped between the start and end events of this module, and thus - // needlessly add indentation, indirection, or other visible noise. - // Unit tests for the callbacks API would detect that as a regression. - currentModule: { - name: '', - tests: [], - childModules: [], - testsRun: 0, - testsIgnored: 0, - hooks: { - before: [], - beforeEach: [], - afterEach: [], - after: [] - } - }, - // Semi-internal state. // // These are undocumented but defacto stable for certain limited use cases, // in order to maintain ecosystem compat with popular QUnit 2.x plugins and integrations. // + // - currentModule: This object represents the most recent `QUnit.module()` call, + // and is used by functions like `QUnit.test()` to determine their module parent. + // It is also referred to from `config.modules` and `config._moduleStack`. + // + // This starts out with an unnamed placeholder module to hold any "global" tests. + // The unnamed module was introduced in QUnit 1.16. When we reach doStart() in start.js, + // if no global tests exist, the unnamed module will be removed `config.modules`, as if + // it never existed, and thus never exposed to the events and callbacks API. + // + // Note that this unnamed initial module is not a "root" module, it is not an ancestor + // to any other modules. Doing so would negatively impact developer experience by ading + // needless indentation, indirection, and other visible noise in test results (or require + // workarounds to prevent that). Since the unnamed module is a regular module, it will + // "end" after the last global test (i.e. before the first named module), and not e.g. + // at the end of the test run. + // To set global hooks, use `QUnit.hooks` instead. + // To listen for the end of the run, handle the "runEnd" event from `QUnit.on()`. + // // - blocking: Whether new tests will be defined and queued, or executed immediately. // In other words, whether QUnit.start() has been called yet. // @@ -160,6 +131,8 @@ const config = { // - stats: Internal assertion counts. Use `QUnit.on('runEnd')` instead. // These are discouraged per the notice at https://qunitjs.com/api/callbacks/QUnit.done/. // https://qunitjs.com/api/callbacks/QUnit.on/#the-runend-event + // + currentModule: null, // initial unnamed module for "global tests" assigned in core.js. blocking: true, started: 0, callbacks: {}, @@ -168,8 +141,9 @@ const config = { // Internal state, exposed to ease in-process resets // Ref https://github.com/qunitjs/qunit/pull/1598 + _moduleStack: [], _globalHooks: {}, - _pq: null, // ProcessingQueue singleton, assigned in /src/core.js + _pq: null, // ProcessingQueue singleton, assigned in core.js _runStarted: false, _event_listeners: Object.create(null), _event_memory: {} @@ -271,7 +245,4 @@ if (urlParams.seed === true) { readFlatPreconfigString(urlParams.seed, 'seed'); } -// Push a loose unnamed module to the modules collection -config.modules.push(config.currentModule); - export default config; diff --git a/src/core/core.js b/src/core/core.js index 17ea04be8..f60f97501 100644 --- a/src/core/core.js +++ b/src/core/core.js @@ -1,7 +1,7 @@ import { window } from './globals.js'; import equiv from './equiv.js'; import dump from './dump.js'; -import { runSuite, module } from './module.js'; +import { unnamedModule, module } from './module.js'; import Assert from './assert.js'; import { test, pushFailure } from './test.js'; import reporters from './reporters.js'; @@ -18,14 +18,7 @@ import diff from './diff.js'; import version from './version.js'; import { createStartFunction } from './start.js'; -// The "currentModule" object would ideally be defined using the createModule() -// function. Since it isn't, add the missing suiteReport property to it now that -// we have loaded all source code required to do so. -// -// TODO: Consider defining currentModule in core.js or module.js in its entirely -// rather than partly in config.js and partly here. -config.currentModule.suiteReport = runSuite; - +config.currentModule = unnamedModule; config._pq = new ProcessingQueue(); const QUnit = { diff --git a/src/core/module.js b/src/core/module.js index e195eebe9..bb5ced1fa 100644 --- a/src/core/module.js +++ b/src/core/module.js @@ -2,31 +2,45 @@ import config from './config.js'; import SuiteReport from './reports/suite.js'; import { extend, generateHash, isAsyncFunction } from './utilities.js'; -const moduleStack = []; +export const globalSuiteReport = new SuiteReport(); -export const runSuite = new SuiteReport(); +// This also pushes the unnamed module to config.modules. +// It is important that we not push this into the moduleStack, +// since it is not meant to be a parent to any other modules. +export const unnamedModule = createModule('', {}, {}, globalSuiteReport); function isParentModuleInQueue () { const modulesInQueue = config.modules .filter(module => !module.ignored) .map(module => module.moduleId); - return moduleStack.some(module => modulesInQueue.includes(module.moduleId)); + return config._moduleStack.some(module => modulesInQueue.includes(module.moduleId)); } -function createModule (name, testEnvironment, modifiers) { - const parentModule = moduleStack.length ? moduleStack.slice(-1)[0] : null; - const parentReport = parentModule ? parentModule.suiteReport : runSuite; +/** + * This does: + * - Create a module object + * - Link it to and from a parent module (if one is on the stack) + * - Link it to a parent SuiteReport (if one is on the stack) + * - Push it to `config.modules` + * + * It does NOT push it to config._moduleStack. That's only relevant for + * scoped modules, and is the responsibility of processModule(). + * + * @param {string} name + * @param {Object} testEnvironment + * @param {Object} modifiers + * @param {SuiteReport} suiteReport Force the report, for use by the initial unnamedModule + * @return {Object} + */ +function createModule (name, testEnvironment, modifiers, suiteReport) { + const parentModule = config._moduleStack.length + ? config._moduleStack[config._moduleStack.length - 1] + : null; const moduleName = parentModule !== null ? [parentModule.name, name].join(' > ') : name; const skip = (parentModule !== null && parentModule.skip) || modifiers.skip; const todo = (parentModule !== null && parentModule.todo) || modifiers.todo; - let env = {}; - if (parentModule) { - env = Object.create(parentModule.testEnvironment || {}); - } - extend(env, testEnvironment); - const module = { name: moduleName, parentModule: parentModule, @@ -36,13 +50,20 @@ function createModule (name, testEnvironment, modifiers) { afterEach: [], after: [] }, - testEnvironment: env, + testEnvironment: extend( + // Live inheritence as of QUnit 3. https://github.com/qunitjs/qunit/pull/1762 + (parentModule ? Object.create(parentModule.testEnvironment || {}) : {}), + testEnvironment + ), tests: [], moduleId: generateHash(moduleName), testsRun: 0, testsIgnored: 0, childModules: [], - suiteReport: new SuiteReport(name, parentReport), + suiteReport: suiteReport || new SuiteReport( + name, + parentModule ? parentModule.suiteReport : globalSuiteReport + ), // Initialised by test.js when the module start executing, // i.e. before the first test in this module (or a child). @@ -62,10 +83,11 @@ function createModule (name, testEnvironment, modifiers) { } config.modules.push(module); + return module; } -function setHookFromEnvironment (hooks, environment, name) { +function setHookFromEnvironment (environment, hooks, name) { const potentialHook = environment[name]; if (typeof potentialHook === 'function') { hooks[name].push(potentialHook); @@ -97,11 +119,10 @@ function processModule (name, options, scope, modifiers = {}) { // Transfer any initial hooks from the options object to the 'hooks' object const testEnvironment = module.testEnvironment; const hooks = module.hooks; - - setHookFromEnvironment(hooks, testEnvironment, 'before'); - setHookFromEnvironment(hooks, testEnvironment, 'beforeEach'); - setHookFromEnvironment(hooks, testEnvironment, 'afterEach'); - setHookFromEnvironment(hooks, testEnvironment, 'after'); + setHookFromEnvironment(testEnvironment, hooks, 'before'); + setHookFromEnvironment(testEnvironment, hooks, 'beforeEach'); + setHookFromEnvironment(testEnvironment, hooks, 'afterEach'); + setHookFromEnvironment(testEnvironment, hooks, 'after'); const moduleFns = { before: makeSetHook(module, 'before'), @@ -114,7 +135,7 @@ function processModule (name, options, scope, modifiers = {}) { config.currentModule = module; if (typeof scope === 'function') { - moduleStack.push(module); + config._moduleStack.push(module); try { const cbReturnValue = scope.call(module.testEnvironment, moduleFns); @@ -126,7 +147,7 @@ function processModule (name, options, scope, modifiers = {}) { // we let this bubble up to global error handlers. But, not until after // we teardown internal state to ensure correct module nesting. // Ref https://github.com/qunitjs/qunit/issues/1478. - moduleStack.pop(); + config._moduleStack.pop(); config.currentModule = module.parentModule || prevModule; } } diff --git a/src/core/on-uncaught-exception.js b/src/core/on-uncaught-exception.js index cf6f89cd4..21f6744f2 100644 --- a/src/core/on-uncaught-exception.js +++ b/src/core/on-uncaught-exception.js @@ -1,5 +1,5 @@ import config from './config.js'; -import { runSuite } from './module.js'; +import { globalSuiteReport } from './module.js'; import { sourceFromStacktrace } from './stacktrace.js'; import { errorString } from './utilities.js'; import { emit } from './events.js'; @@ -42,7 +42,7 @@ export default function onUncaughtException (error) { // Increase "bad assertion" stats despite no longer pushing an assertion in this case. // This ensures "runEnd" and "QUnit.done()" handlers behave as expected, since the "bad" // count is typically how reporters decide on the boolean outcome of the test run. - runSuite.globalFailureCount++; + globalSuiteReport.globalFailureCount++; config.stats.bad++; config.stats.all++; emit('error', error); diff --git a/src/core/processing-queue.js b/src/core/processing-queue.js index fb7eceae3..e107bf3a1 100644 --- a/src/core/processing-queue.js +++ b/src/core/processing-queue.js @@ -2,7 +2,7 @@ import config from './config.js'; import { generateHash, performance } from './utilities.js'; import { runLoggingCallbacks } from './callbacks.js'; import Promise from './promise.js'; -import { runSuite } from './module.js'; +import { globalSuiteReport } from './module.js'; import onUncaughtException from './on-uncaught-exception.js'; import { emit } from './events.js'; import { setTimeout } from './globals.js'; @@ -67,7 +67,7 @@ class ProcessingQueue { /** * Process the first task on the taskQueue as a promise. - * Each task is a function added by Test#queue() in /src/test.js + * Each task is a function added by Test#queue() in test.js */ processTaskQueue (start) { if (this.taskQueue.length && !config.blocking) { @@ -187,7 +187,7 @@ class ProcessingQueue { this.finished = true; - const runEnd = runSuite.end(true); + const runEnd = globalSuiteReport.end(true); emit('runEnd', runEnd); runLoggingCallbacks('done', { diff --git a/src/core/reporters/TapReporter.js b/src/core/reporters/TapReporter.js index b819bd37f..19b620345 100644 --- a/src/core/reporters/TapReporter.js +++ b/src/core/reporters/TapReporter.js @@ -244,14 +244,14 @@ export default class TapReporter { } } - onRunEnd (runSuite) { + onRunEnd (runEnd) { this.ended = true; - this.log(`1..${runSuite.testCounts.total}`); - this.log(`# pass ${runSuite.testCounts.passed}`); - this.log(kleur.yellow(`# skip ${runSuite.testCounts.skipped}`)); - this.log(kleur.cyan(`# todo ${runSuite.testCounts.todo}`)); - this.log(kleur.red(`# fail ${runSuite.testCounts.failed}`)); + this.log(`1..${runEnd.testCounts.total}`); + this.log(`# pass ${runEnd.testCounts.passed}`); + this.log(kleur.yellow(`# skip ${runEnd.testCounts.skipped}`)); + this.log(kleur.cyan(`# todo ${runEnd.testCounts.todo}`)); + this.log(kleur.red(`# fail ${runEnd.testCounts.failed}`)); } logAssertion (error, severity) { diff --git a/src/core/reports/suite.js b/src/core/reports/suite.js index ae461d3b1..f17c9d993 100644 --- a/src/core/reports/suite.js +++ b/src/core/reports/suite.js @@ -7,7 +7,7 @@ export default class SuiteReport { // When an "error" event is emitted from onUncaughtException(), the // "runEnd" event should report the status as failed. The "runEnd" event data - // is tracked through this property (via the "runSuite" instance). + // is tracked through this property (via the "globalSuiteReport" instance). this.globalFailureCount = 0; this.tests = []; diff --git a/src/core/stacktrace.js b/src/core/stacktrace.js index 588a8c5cb..67ce75218 100644 --- a/src/core/stacktrace.js +++ b/src/core/stacktrace.js @@ -2,8 +2,8 @@ // // This should reduce a raw stack trace like this: // -// > foo.broken()@/src/foo.js -// > Bar@/src/bar.js +// > foo.broken()@/example/foo.js +// > Bar@/example/bar.js // > @/test/bar.test.js // > @/lib/qunit.js:500:12 // > @/lib/qunit.js:100:28 @@ -13,8 +13,8 @@ // // and shorten it to show up until the end of the user's bar.test.js code. // -// > foo.broken()@/src/foo.js -// > Bar@/src/bar.js +// > foo.broken()@/example/foo.js +// > Bar@/example/bar.js // > @/test/bar.test.js // // QUnit will obtain one example trace (once per process/pageload suffices), diff --git a/src/core/start.js b/src/core/start.js index ad15ed2f3..9d718f8b0 100644 --- a/src/core/start.js +++ b/src/core/start.js @@ -2,7 +2,7 @@ import { runLoggingCallbacks } from './callbacks.js'; import config from './config.js'; import { emit } from './events.js'; import { window, document, setTimeout } from './globals.js'; -import { runSuite } from './module.js'; +import { globalSuiteReport } from './module.js'; import Test from './test.js'; import reporters from './reporters.js'; import { performance } from './utilities.js'; @@ -14,7 +14,7 @@ function unblockAndAdvanceQueue () { // Inject the complete QUnit API for use by reporters export function createStartFunction (QUnit) { - function doBegin () { + function doStart () { if (config.started) { unblockAndAdvanceQueue(); return; @@ -39,14 +39,18 @@ export function createStartFunction (QUnit) { // Record the time of the test run's beginning config.started = performance.now(); - // Delete the loose unnamed module if unused. + // Delete the unnamed module if no global tests were defined (see config.js) if (config.modules[0].name === '' && config.modules[0].tests.length === 0) { config.modules.shift(); } + // Create a list of simplified and independent module descriptor objects for + // the QUnit.begin callbacks. This prevents plugins from relying on reading + // from (or writing!) to internal state. const modulesLog = []; for (let i = 0; i < config.modules.length; i++) { - // Don't expose the unnamed global test module to plugins. + // Always omit the unnamed module from the list of module names + // for UI plugins, even if there were glboal tests defined. if (config.modules[i].name !== '') { modulesLog.push({ name: config.modules[i].name, @@ -56,7 +60,7 @@ export function createStartFunction (QUnit) { } // The test run is officially beginning now - emit('runStart', runSuite.start(true)); + emit('runStart', globalSuiteReport.start(true)); runLoggingCallbacks('begin', { totalTests: Test.count, modules: modulesLog @@ -82,15 +86,15 @@ export function createStartFunction (QUnit) { // still wait for DOM ready to ensure reliable integration of reporters. window.addEventListener('load', function () { setTimeout(function () { - doBegin(); + doStart(); }); }); } else if (setTimeout) { setTimeout(function () { - doBegin(); + doStart(); }); } else { - doBegin(); + doStart(); } }; } diff --git a/src/core/test.js b/src/core/test.js index 05cb638b0..d90816701 100644 --- a/src/core/test.js +++ b/src/core/test.js @@ -111,8 +111,8 @@ export default function Test (settings) { Test.count = 0; -function getNotStartedModules (startModule) { - let module = startModule; +function getModulesForStartEvent (startingModule) { + let module = startingModule; const modules = []; while (module && module.testsRun === 0) { @@ -120,8 +120,8 @@ function getNotStartedModules (startModule) { module = module.parentModule; } - // The above push modules from the child to the parent - // return a reversed order with the top being the top most parent module + // The above starts from the child and moves up to the parent. + // Return this in reversed order, such that we start with top-most parent. return modules.reverse(); } @@ -136,7 +136,7 @@ Test.prototype = { before: function () { const module = this.module; - const notStartedModules = getNotStartedModules(module); + const notStartedModules = getModulesForStartEvent(module); // ensure the callbacks are executed serially for each module let moduleStartChain = Promise.resolve(); @@ -243,7 +243,7 @@ Test.prototype = { return; } - // The 'after' hook should only execute when there are not tests left and + // The 'after' hook should only execute when there are no tests left and // when the 'after' and 'finish' tasks are the only tasks left to process if (hookName === 'after' && !lastTestWithinModuleExecuted(hookOwner) && @@ -417,39 +417,36 @@ Test.prototype = { // generating stack trace is expensive, so using a getter will help defer this until we need it get source () { return test.stack; } }).then(function () { - if (allTestsExecuted(module)) { - const completedModules = [module]; - - // Check if the parent modules, iteratively, are done. If that the case, - // we emit the `suiteEnd` event and trigger `moduleDone` callback. - let parent = module.parentModule; - while (parent && allTestsExecuted(parent)) { - completedModules.push(parent); - parent = parent.parentModule; - } + // Emit the `suiteEnd` event and `moduleDone` callbacks for modules + // that are completed as of now. + const completedModules = []; + let parent = module; + while (parent && allTestsExecuted(parent)) { + completedModules.push(parent); + parent = parent.parentModule; + } - let moduleDoneChain = Promise.resolve(); - completedModules.forEach(completedModule => { - moduleDoneChain = moduleDoneChain.then(() => { - return logSuiteEnd(completedModule); - }); + let moduleDoneChain = Promise.resolve(); + completedModules.forEach(completedModule => { + moduleDoneChain = moduleDoneChain.then(() => { + return logSuiteEnd(completedModule); }); - return moduleDoneChain; - } + }); + return moduleDoneChain; }).then(function () { config.current = undefined; }); function logSuiteEnd (module) { - // Reset `module.hooks` to ensure that anything referenced in these hooks + // Empty `module.hooks` to ensure that anything referenced in these hooks // has been released to be garbage collected. Descendant modules that were // entirely skipped, e.g. due to filtering, will never have this method - // called for them, but might have hooks with references pinning data in - // memory (even if the hooks weren't actually executed), so we reset the + // called for them, but might have hooks with references that hold data in + // memory (even if the hooks were never executed), so we empty the // hooks on all descendant modules here as well. This is safe because we // will never call this as long as any descendant modules still have tests - // to run. This also means that in multi-tiered nesting scenarios we might - // reset the hooks multiple times on some modules, but that's harmless. + // to run. This also means that for deeply nested modules, we might empty + // the hooks on completed child modules multiple times. That's harmless. const modules = [module]; while (modules.length) { const nextModule = modules.shift();