Skip to content

Commit 112dacc

Browse files
KrinkleKrinkle
committed
Build: Refactor QUnit.start() definition to inject self-reference
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.
1 parent bc65733 commit 112dacc

File tree

3 files changed

+79
-73
lines changed

3 files changed

+79
-73
lines changed

‎src/core/config.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ const config = {
132132
// These are discouraged per the notice at https://qunitjs.com/api/callbacks/QUnit.done/.
133133
// https://qunitjs.com/api/callbacks/QUnit.on/#the-runend-event
134134
//
135-
currentModule: null, // initial unnamed module for "global tests" assigned in core.js.
135+
currentModule: null, // initial unnamed module for "global tests", assigned in core.js.
136136
blocking: true,
137137
started: 0,
138138
callbacks: {},
@@ -144,6 +144,7 @@ const config = {
144144
_moduleStack: [],
145145
_globalHooks: {},
146146
_pq: null, // ProcessingQueue singleton, assigned in core.js
147+
_QUnit: null, // Self-reference to the exported QUnit API, for start.js
147148
_runStarted: false,
148149
_event_listeners: Object.create(null),
149150
_event_memory: {}

‎src/core/core.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import { on } from './events.js';
1616
import onUncaughtException from './on-uncaught-exception.js';
1717
import diff from './diff.js';
1818
import version from './version.js';
19-
import { createStartFunction } from './start.js';
19+
import { start } from './start.js';
2020

2121
config.currentModule = unnamedModule;
2222
config._pq = new ProcessingQueue();
@@ -54,13 +54,16 @@ const QUnit = {
5454

5555
assert: Assert.prototype,
5656
module,
57+
start,
5758
test,
5859

5960
// alias other test flavors for easy access
6061
todo: test.todo,
6162
skip: test.skip,
6263
only: test.only
6364
};
64-
QUnit.start = createStartFunction(QUnit);
65+
66+
// Inject the exported QUnit API for use by reporters in start()
67+
config._QUnit = QUnit;
6568

6669
export default QUnit;

‎src/core/start.js

Lines changed: 72 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -12,89 +12,91 @@ function unblockAndAdvanceQueue () {
1212
config._pq.advance();
1313
}
1414

15-
// Inject the complete QUnit API for use by reporters
16-
export function createStartFunction (QUnit) {
17-
function doStart () {
18-
if (config.started) {
19-
unblockAndAdvanceQueue();
20-
return;
21-
}
15+
function doStart () {
16+
if (config.started) {
17+
unblockAndAdvanceQueue();
18+
return;
19+
}
2220

23-
// QUnit.config.reporters is considered writable between qunit.js and QUnit.start().
24-
// Now, it is time to decide which reporters we'll load.
25-
//
26-
// For config.reporters.html, refer to browser-runner.js and HtmlReporter#onRunStart.
27-
//
28-
if (config.reporters.console) {
29-
reporters.console.init(QUnit);
30-
}
31-
if (config.reporters.perf || (config.reporters.perf === undefined && window && document)) {
32-
reporters.perf.init(QUnit);
33-
}
34-
if (config.reporters.tap) {
35-
reporters.tap.init(QUnit);
36-
}
21+
// QUnit.config.reporters is considered writable between qunit.js and QUnit.start().
22+
// Now that QUnit.start() has been called, it is time to decide which built-in reporters
23+
// to load.
24+
// For config.reporters.html, refer to browser-runner.js and HtmlReporter#onRunStart.
3725

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

42-
// Delete the unnamed module if no global tests were defined (see config.js)
43-
if (config.modules[0].name === '' && config.modules[0].tests.length === 0) {
44-
config.modules.shift();
45-
}
31+
if (config.reporters.console) {
32+
reporters.console.init(config._QUnit);
33+
}
34+
if (config.reporters.perf || (config.reporters.perf === undefined && window && document)) {
35+
reporters.perf.init(config._QUnit);
36+
}
37+
if (config.reporters.tap) {
38+
reporters.tap.init(config._QUnit);
39+
}
4640

47-
// Create a list of simplified and independent module descriptor objects for
48-
// the QUnit.begin callbacks. This prevents plugins from relying on reading
49-
// from (or writing!) to internal state.
50-
const modulesLog = [];
51-
for (let i = 0; i < config.modules.length; i++) {
52-
// Always omit the unnamed module from the list of module names
53-
// for UI plugins, even if there were glboal tests defined.
54-
if (config.modules[i].name !== '') {
55-
modulesLog.push({
56-
name: config.modules[i].name,
57-
moduleId: config.modules[i].moduleId
58-
});
59-
}
60-
}
41+
// The test run hasn't officially begun yet
42+
// Record the time of the test run's beginning
43+
config.started = performance.now();
6144

62-
// The test run is officially beginning now
63-
emit('runStart', globalSuiteReport.start(true));
64-
runLoggingCallbacks('begin', {
65-
totalTests: Test.count,
66-
modules: modulesLog
67-
}).then(unblockAndAdvanceQueue);
45+
// Delete the unnamed module if no global tests were defined (see config.js)
46+
if (config.modules[0].name === '' && config.modules[0].tests.length === 0) {
47+
config.modules.shift();
6848
}
6949

70-
return function start () {
71-
if (config.current) {
72-
throw new Error('QUnit.start cannot be called inside a test.');
50+
// Create a list of simplified and independent module descriptor objects for
51+
// the QUnit.begin callbacks. This prevents plugins from relying on reading
52+
// from (or writing!) to internal state.
53+
const modulesLog = [];
54+
for (let i = 0; i < config.modules.length; i++) {
55+
// Always omit the unnamed module from the list of module names
56+
// for UI plugins, even if there were glboal tests defined.
57+
if (config.modules[i].name !== '') {
58+
modulesLog.push({
59+
name: config.modules[i].name,
60+
moduleId: config.modules[i].moduleId
61+
});
7362
}
74-
if (config._runStarted) {
75-
if (document && config.autostart) {
76-
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/');
77-
}
78-
throw new Error('QUnit.start() called too many times.');
63+
}
64+
65+
// The test run is officially beginning now
66+
emit('runStart', globalSuiteReport.start(true));
67+
runLoggingCallbacks('begin', {
68+
totalTests: Test.count,
69+
modules: modulesLog
70+
}).then(unblockAndAdvanceQueue);
71+
}
72+
73+
export function start () {
74+
if (config.current) {
75+
throw new Error('QUnit.start cannot be called inside a test.');
76+
}
77+
if (config._runStarted) {
78+
if (document && config.autostart) {
79+
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/');
7980
}
81+
throw new Error('QUnit.start() called too many times.');
82+
}
8083

81-
config._runStarted = true;
84+
config._runStarted = true;
8285

83-
// Add a slight delay to allow definition of more modules and tests.
84-
if (document && document.readyState !== 'complete' && setTimeout) {
85-
// In browser environments, if QUnit.start() is called very early,
86-
// still wait for DOM ready to ensure reliable integration of reporters.
87-
window.addEventListener('load', function () {
88-
setTimeout(function () {
89-
doStart();
90-
});
91-
});
92-
} else if (setTimeout) {
86+
// Add a slight delay to allow definition of more modules and tests.
87+
if (document && document.readyState !== 'complete' && setTimeout) {
88+
// In browser environments, if QUnit.start() is called very early,
89+
// still wait for DOM ready to ensure reliable integration of reporters.
90+
window.addEventListener('load', function () {
9391
setTimeout(function () {
9492
doStart();
9593
});
96-
} else {
94+
});
95+
} else if (setTimeout) {
96+
setTimeout(function () {
9797
doStart();
98-
}
99-
};
98+
});
99+
} else {
100+
doStart();
101+
}
100102
}

0 commit comments

Comments
 (0)