From 3fb97a90eed9e552af7a802ee0eb5230c1c9729c Mon Sep 17 00:00:00 2001
From: Colin Ihrig <cjihrig@gmail.com>
Date: Sat, 27 Jul 2024 09:27:03 -0400
Subject: [PATCH] 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: https://github.com/nodejs/node/pull/54013
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
---
 lib/internal/test_runner/harness.js | 39 ++++++++++++++---------------
 lib/internal/test_runner/runner.js  |  2 +-
 2 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js
index e595f2ed1dff61..ac52307cc38be5 100644
--- a/lib/internal/test_runner/harness.js
+++ b/lib/internal/test_runner/harness.js
@@ -28,18 +28,20 @@ const {
 } = require('internal/test_runner/utils');
 const { queueMicrotask } = require('internal/process/task_queues');
 const { bigint: hrtime } = process.hrtime;
-
+const resolvedPromise = PromiseResolve();
 const testResources = new SafeMap();
+let globalRoot;
 
 testResources.set(reporterScope.asyncId(), reporterScope);
 
 function createTestTree(options = kEmptyObject) {
-  return setup(new Test({ __proto__: null, ...options, name: '<root>' }));
+  globalRoot = setup(new Test({ __proto__: null, ...options, name: '<root>' }));
+  return globalRoot;
 }
 
 function createProcessEventHandler(eventName, rootTest) {
   return (err) => {
-    if (!rootTest.harness.bootstrapComplete) {
+    if (rootTest.harness.bootstrapPromise) {
       // Something went wrong during the asynchronous portion of bootstrapping
       // the test runner. Since the test runner is not setup properly, we can't
       // do anything but throw the error.
@@ -196,7 +198,7 @@ function setup(root) {
   root.harness = {
     __proto__: null,
     allowTestsToRun: false,
-    bootstrapComplete: false,
+    bootstrapPromise: resolvedPromise,
     watching: false,
     coverage: FunctionPrototypeBind(collectCoverage, null, root, coverage),
     resetCounters() {
@@ -222,33 +224,30 @@ function setup(root) {
   return root;
 }
 
-let globalRoot;
-let asyncBootstrap;
 function lazyBootstrapRoot() {
   if (!globalRoot) {
-    globalRoot = createTestTree({ __proto__: null, entryFile: process.argv?.[1] });
+    // This is where the test runner is bootstrapped when node:test is used
+    // without the --test flag or the run() API.
+    createTestTree({ __proto__: null, entryFile: process.argv?.[1] });
     globalRoot.reporter.on('test:fail', (data) => {
       if (data.todo === undefined || data.todo === false) {
         process.exitCode = kGenericUserError;
       }
     });
-    asyncBootstrap = setupTestReporters(globalRoot.reporter);
+    globalRoot.harness.bootstrapPromise = setupTestReporters(globalRoot.reporter);
   }
   return globalRoot;
 }
 
-async function startSubtest(subtest) {
-  if (asyncBootstrap) {
+async function startSubtestAfterBootstrap(subtest) {
+  if (subtest.root.harness.bootstrapPromise) {
     // Only incur the overhead of awaiting the Promise once.
-    await asyncBootstrap;
-    asyncBootstrap = undefined;
-    if (!subtest.root.harness.bootstrapComplete) {
-      subtest.root.harness.bootstrapComplete = true;
-      queueMicrotask(() => {
-        subtest.root.harness.allowTestsToRun = true;
-        subtest.root.processPendingSubtests();
-      });
-    }
+    await subtest.root.harness.bootstrapPromise;
+    subtest.root.harness.bootstrapPromise = null;
+    queueMicrotask(() => {
+      subtest.root.harness.allowTestsToRun = true;
+      subtest.root.processPendingSubtests();
+    });
   }
 
   await subtest.start();
@@ -262,7 +261,7 @@ function runInParentContext(Factory) {
       return PromiseResolve();
     }
 
-    return startSubtest(subtest);
+    return startSubtestAfterBootstrap(subtest);
   }
 
   const test = (name, options, fn) => {
diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js
index 31a253c776a81e..7443502848f930 100644
--- a/lib/internal/test_runner/runner.js
+++ b/lib/internal/test_runner/runner.js
@@ -596,7 +596,7 @@ function run(options = kEmptyObject) {
     teardown = undefined;
   }
   const runFiles = () => {
-    root.harness.bootstrapComplete = true;
+    root.harness.bootstrapPromise = null;
     root.harness.allowTestsToRun = true;
     return SafePromiseAllSettledReturnVoid(testFiles, (path) => {
       const subtest = runTestFile(path, filesWatcher, opts);