Skip to content

Commit 3fb97a9

Browse files
cjihrigRafaelGSS
authored andcommitted
test_runner: remove redundant bootstrap boolean
The test runner bootstrap process awaits a Promise and then sets a boolean flag. This commit consolidates the Promise and boolean into a single value. This commit also ensures that the globalRoot test is always assigned in createTestTree() in order to better consolidate the CLI/run() and non-CLI configuration. PR-URL: #54013 Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
1 parent e471e32 commit 3fb97a9

File tree

2 files changed

+20
-21
lines changed

2 files changed

+20
-21
lines changed

lib/internal/test_runner/harness.js

+19-20
Original file line numberDiff line numberDiff line change
@@ -28,18 +28,20 @@ const {
2828
} = require('internal/test_runner/utils');
2929
const { queueMicrotask } = require('internal/process/task_queues');
3030
const { bigint: hrtime } = process.hrtime;
31-
31+
const resolvedPromise = PromiseResolve();
3232
const testResources = new SafeMap();
33+
let globalRoot;
3334

3435
testResources.set(reporterScope.asyncId(), reporterScope);
3536

3637
function createTestTree(options = kEmptyObject) {
37-
return setup(new Test({ __proto__: null, ...options, name: '<root>' }));
38+
globalRoot = setup(new Test({ __proto__: null, ...options, name: '<root>' }));
39+
return globalRoot;
3840
}
3941

4042
function createProcessEventHandler(eventName, rootTest) {
4143
return (err) => {
42-
if (!rootTest.harness.bootstrapComplete) {
44+
if (rootTest.harness.bootstrapPromise) {
4345
// Something went wrong during the asynchronous portion of bootstrapping
4446
// the test runner. Since the test runner is not setup properly, we can't
4547
// do anything but throw the error.
@@ -196,7 +198,7 @@ function setup(root) {
196198
root.harness = {
197199
__proto__: null,
198200
allowTestsToRun: false,
199-
bootstrapComplete: false,
201+
bootstrapPromise: resolvedPromise,
200202
watching: false,
201203
coverage: FunctionPrototypeBind(collectCoverage, null, root, coverage),
202204
resetCounters() {
@@ -222,33 +224,30 @@ function setup(root) {
222224
return root;
223225
}
224226

225-
let globalRoot;
226-
let asyncBootstrap;
227227
function lazyBootstrapRoot() {
228228
if (!globalRoot) {
229-
globalRoot = createTestTree({ __proto__: null, entryFile: process.argv?.[1] });
229+
// This is where the test runner is bootstrapped when node:test is used
230+
// without the --test flag or the run() API.
231+
createTestTree({ __proto__: null, entryFile: process.argv?.[1] });
230232
globalRoot.reporter.on('test:fail', (data) => {
231233
if (data.todo === undefined || data.todo === false) {
232234
process.exitCode = kGenericUserError;
233235
}
234236
});
235-
asyncBootstrap = setupTestReporters(globalRoot.reporter);
237+
globalRoot.harness.bootstrapPromise = setupTestReporters(globalRoot.reporter);
236238
}
237239
return globalRoot;
238240
}
239241

240-
async function startSubtest(subtest) {
241-
if (asyncBootstrap) {
242+
async function startSubtestAfterBootstrap(subtest) {
243+
if (subtest.root.harness.bootstrapPromise) {
242244
// Only incur the overhead of awaiting the Promise once.
243-
await asyncBootstrap;
244-
asyncBootstrap = undefined;
245-
if (!subtest.root.harness.bootstrapComplete) {
246-
subtest.root.harness.bootstrapComplete = true;
247-
queueMicrotask(() => {
248-
subtest.root.harness.allowTestsToRun = true;
249-
subtest.root.processPendingSubtests();
250-
});
251-
}
245+
await subtest.root.harness.bootstrapPromise;
246+
subtest.root.harness.bootstrapPromise = null;
247+
queueMicrotask(() => {
248+
subtest.root.harness.allowTestsToRun = true;
249+
subtest.root.processPendingSubtests();
250+
});
252251
}
253252

254253
await subtest.start();
@@ -262,7 +261,7 @@ function runInParentContext(Factory) {
262261
return PromiseResolve();
263262
}
264263

265-
return startSubtest(subtest);
264+
return startSubtestAfterBootstrap(subtest);
266265
}
267266

268267
const test = (name, options, fn) => {

lib/internal/test_runner/runner.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -596,7 +596,7 @@ function run(options = kEmptyObject) {
596596
teardown = undefined;
597597
}
598598
const runFiles = () => {
599-
root.harness.bootstrapComplete = true;
599+
root.harness.bootstrapPromise = null;
600600
root.harness.allowTestsToRun = true;
601601
return SafePromiseAllSettledReturnVoid(testFiles, (path) => {
602602
const subtest = runTestFile(path, filesWatcher, opts);

0 commit comments

Comments
 (0)