test_runner: remove redundant bootstrap boolean

cjihrig · RafaelGSS · commit 3fb97a90eed9 · 2024-08-05T11:15:45.000-03:00
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>
diff --git 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
@@ -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);

Original file line number	Diff line number	Diff line change
`@@ -596,7 +596,7 @@ function run(options = kEmptyObject) {`
`596`	`596`	`teardown = undefined;`
`597`	`597`	`}`
`598`	`598`	`const runFiles = () => {`
`599`		`- root.harness.bootstrapComplete = true;`
	`599`	`+ root.harness.bootstrapPromise = null;`
`600`	`600`	`root.harness.allowTestsToRun = true;`
`601`	`601`	`return SafePromiseAllSettledReturnVoid(testFiles, (path) => {`
`602`	`602`	`const subtest = runTestFile(path, filesWatcher, opts);`