Skip to content

Commit

Permalink
Build: Refactor QUnit.start() definition to inject self-reference
Browse files Browse the repository at this point in the history 
Follows-up 05e15ba, which made this into a factory function,
but that has the downside of making the QUnit object not defined in
one object literal, which makes a few other things reasier to reason
about.

In a way, it's more honest to say that start is the product of a
factory function, but I'd prefer to maintain the simplicity of an
uncoupled literal declaration the entire API, in particular in prep
for native ESM export (ref #1551).

I'll accept in return the internal responsiblity to not call start()
"incorrectly" (i.e. before it is ready). This responsibility does not
leak into, complicate, break, or otherwise change the public API, and
is mitigated by a runtime detection, for the benefit of future
contributors and maintainers to QUnit.
  • Loading branch information
@Krinkle
Krinkle committed Aug 25, 2024
1 parent bc65733 commit 612ca88
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 74 deletions.
3 changes: 2 additions & 1 deletion src/core/config.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ const config = {
// 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.
currentModule: null, // initial unnamed module for "global tests", assigned in core.js.
blocking: true,
started: 0,
callbacks: {},
Expand All @@ -144,6 +144,7 @@ const config = {
_moduleStack: [],
_globalHooks: {},
_pq: null, // ProcessingQueue singleton, assigned in core.js
_QUnit: null, // Self-reference to the exported QUnit API, for start.js
_runStarted: false,
_event_listeners: Object.create(null),
_event_memory: {}
Expand Down
9 changes: 6 additions & 3 deletions src/core/core.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { on } from './events.js';
import onUncaughtException from './on-uncaught-exception.js';
import diff from './diff.js';
import version from './version.js';
import { createStartFunction } from './start.js';
import { start } from './start.js';

config.currentModule = unnamedModule;
config._pq = new ProcessingQueue();
Expand Down Expand Up @@ -54,13 +54,16 @@ const QUnit = {

assert: Assert.prototype,
module,
start,
test,

// alias other test flavors for easy access
todo: test.todo,
skip: test.skip,
only: test.only
only: test.only,
};
QUnit.start = createStartFunction(QUnit);

// Inject the exported QUnit API for use by reporters in start()
config._QUnit = QUnit;

export default QUnit;
142 changes: 72 additions & 70 deletions src/core/start.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,89 +12,91 @@ function unblockAndAdvanceQueue () {
config._pq.advance();
}

// Inject the complete QUnit API for use by reporters
export function createStartFunction (QUnit) {
function doStart () {
if (config.started) {
unblockAndAdvanceQueue();
return;
}
function doStart () {
if (config.started) {
unblockAndAdvanceQueue();
return;
}

// QUnit.config.reporters is considered writable between qunit.js and QUnit.start().
// Now, it is time to decide which reporters we'll load.
//
// For config.reporters.html, refer to browser-runner.js and HtmlReporter#onRunStart.
//
if (config.reporters.console) {
reporters.console.init(QUnit);
}
if (config.reporters.perf || (config.reporters.perf === undefined && window && document)) {
reporters.perf.init(QUnit);
}
if (config.reporters.tap) {
reporters.tap.init(QUnit);
}
// QUnit.config.reporters is considered writable between qunit.js and QUnit.start().
// Now that QUnit.start() has been called, it is time to decide which built-in reporters
// to load.
// For config.reporters.html, refer to browser-runner.js and HtmlReporter#onRunStart.

// The test run hasn't officially begun yet
// Record the time of the test run's beginning
config.started = performance.now();
/* istanbul ignore if: internal guard */
if (!config._QUnit) {
throw new ReferenceError('QUnit is undefined. Cannot call start() before qunit.js exports QUnit.');
}

// 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();
}
if (config.reporters.console) {
reporters.console.init(config._QUnit);
}
if (config.reporters.perf || (config.reporters.perf === undefined && window && document)) {
reporters.perf.init(config._QUnit);
}
if (config.reporters.tap) {
reporters.tap.init(config._QUnit);
}

// 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++) {
// 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,
moduleId: config.modules[i].moduleId
});
}
}
// The test run hasn't officially begun yet
// Record the time of the test run's beginning
config.started = performance.now();

// The test run is officially beginning now
emit('runStart', globalSuiteReport.start(true));
runLoggingCallbacks('begin', {
totalTests: Test.count,
modules: modulesLog
}).then(unblockAndAdvanceQueue);
// 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();
}

return function start () {
if (config.current) {
throw new Error('QUnit.start cannot be called inside a test.');
// 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++) {
// 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,
moduleId: config.modules[i].moduleId
});
}
if (config._runStarted) {
if (document && config.autostart) {
throw new Error('QUnit.start() called too many times. Did you call QUnit.start() in browser context when autostart is also enabled? https://qunitjs.com/api/QUnit/start/');
}
throw new Error('QUnit.start() called too many times.');
}

// The test run is officially beginning now
emit('runStart', globalSuiteReport.start(true));
runLoggingCallbacks('begin', {
totalTests: Test.count,
modules: modulesLog
}).then(unblockAndAdvanceQueue);
}

export function start () {
if (config.current) {
throw new Error('QUnit.start cannot be called inside a test.');
}
if (config._runStarted) {
if (document && config.autostart) {
throw new Error('QUnit.start() called too many times. Did you call QUnit.start() in browser context when autostart is also enabled? https://qunitjs.com/api/QUnit/start/');
}
throw new Error('QUnit.start() called too many times.');
}

config._runStarted = true;
config._runStarted = true;

// Add a slight delay to allow definition of more modules and tests.
if (document && document.readyState !== 'complete' && setTimeout) {
// In browser environments, if QUnit.start() is called very early,
// still wait for DOM ready to ensure reliable integration of reporters.
window.addEventListener('load', function () {
setTimeout(function () {
doStart();
});
});
} else if (setTimeout) {
// Add a slight delay to allow definition of more modules and tests.
if (document && document.readyState !== 'complete' && setTimeout) {
// In browser environments, if QUnit.start() is called very early,
// still wait for DOM ready to ensure reliable integration of reporters.
window.addEventListener('load', function () {
setTimeout(function () {
doStart();
});
} else {
});
} else if (setTimeout) {
setTimeout(function () {
doStart();
}
};
});
} else {
doStart();
}
}

0 comments on commit 612ca88

Please sign in to comment.