From 2d7018c23bf69fa086ebc1ad770284c681dba661 Mon Sep 17 00:00:00 2001
From: cjihrig <cjihrig@gmail.com>
Date: Tue, 16 Jul 2024 22:35:40 -0400
Subject: [PATCH] test_runner: refactor and simplify internals

This commit refactors some of the internals of the test runner.
---
 lib/internal/test_runner/harness.js | 42 ++++++++++++++---------------
 lib/internal/test_runner/runner.js  |  2 --
 lib/internal/test_runner/test.js    |  3 +--
 lib/internal/test_runner/utils.js   |  5 ++--
 4 files changed, 23 insertions(+), 29 deletions(-)

diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js
index 27ced12d039f45..fbe166b68ee494 100644
--- a/lib/internal/test_runner/harness.js
+++ b/lib/internal/test_runner/harness.js
@@ -208,7 +208,7 @@ function setup(root) {
       };
     },
     counters: null,
-    shouldColorizeTestFiles: false,
+    shouldColorizeTestFiles: shouldColorizeTestFiles(globalOptions.destinations),
     teardown: exitHandler,
     snapshotManager: null,
   };
@@ -218,8 +218,8 @@ function setup(root) {
 }
 
 let globalRoot;
-let reportersSetup;
-function getGlobalRoot() {
+let asyncBootstrap;
+function lazyBootstrapRoot() {
   if (!globalRoot) {
     globalRoot = createTestTree({ __proto__: null, entryFile: process.argv?.[1] });
     globalRoot.reporter.on('test:fail', (data) => {
@@ -227,26 +227,23 @@ function getGlobalRoot() {
         process.exitCode = kGenericUserError;
       }
     });
-    reportersSetup = setupTestReporters(globalRoot.reporter);
-    globalRoot.harness.shouldColorizeTestFiles ||= shouldColorizeTestFiles(globalRoot);
+    asyncBootstrap = setupTestReporters(globalRoot.reporter);
   }
   return globalRoot;
 }
 
 async function startSubtest(subtest) {
-  if (reportersSetup) {
+  if (asyncBootstrap) {
     // Only incur the overhead of awaiting the Promise once.
-    await reportersSetup;
-    reportersSetup = undefined;
-  }
-
-  const root = getGlobalRoot();
-  if (!root.harness.bootstrapComplete) {
-    root.harness.bootstrapComplete = true;
-    queueMicrotask(() => {
-      root.harness.allowTestsToRun = true;
-      root.processPendingSubtests();
-    });
+    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.start();
@@ -254,12 +251,13 @@ async function startSubtest(subtest) {
 
 function runInParentContext(Factory) {
   function run(name, options, fn, overrides) {
-    const parent = testResources.get(executionAsyncId()) || getGlobalRoot();
+    const parent = testResources.get(executionAsyncId()) || lazyBootstrapRoot();
     const subtest = parent.createSubtest(Factory, name, options, fn, overrides);
-    if (!(parent instanceof Suite)) {
-      return startSubtest(subtest);
+    if (parent instanceof Suite) {
+      return PromiseResolve();
     }
-    return PromiseResolve();
+
+    return startSubtest(subtest);
   }
 
   const test = (name, options, fn) => {
@@ -286,7 +284,7 @@ function runInParentContext(Factory) {
 
 function hook(hook) {
   return (fn, options) => {
-    const parent = testResources.get(executionAsyncId()) || getGlobalRoot();
+    const parent = testResources.get(executionAsyncId()) || lazyBootstrapRoot();
     parent.createHook(hook, fn, {
       __proto__: null,
       ...options,
diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js
index 6fd1cb72f3b508..556bf475d68691 100644
--- a/lib/internal/test_runner/runner.js
+++ b/lib/internal/test_runner/runner.js
@@ -70,7 +70,6 @@ const {
   convertStringToRegExp,
   countCompletedTest,
   kDefaultPattern,
-  shouldColorizeTestFiles,
 } = require('internal/test_runner/utils');
 const { Glob } = require('internal/fs/glob');
 const { once } = require('events');
@@ -552,7 +551,6 @@ function run(options = kEmptyObject) {
   }
 
   const root = createTestTree({ __proto__: null, concurrency, timeout, signal });
-  root.harness.shouldColorizeTestFiles ||= shouldColorizeTestFiles(root);
 
   if (process.env.NODE_TEST_CONTEXT !== undefined) {
     process.emitWarning('node:test run() is being called recursively within a test file. skipping running files.');
diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js
index 46490460caf9e9..1c461a07f16bfa 100644
--- a/lib/internal/test_runner/test.js
+++ b/lib/internal/test_runner/test.js
@@ -75,7 +75,6 @@ const kHookFailure = 'hookFailed';
 const kDefaultTimeout = null;
 const noop = FunctionPrototype;
 const kShouldAbort = Symbol('kShouldAbort');
-const kFilename = process.argv?.[1];
 const kHookNames = ObjectSeal(['before', 'after', 'beforeEach', 'afterEach']);
 const kUnwrapErrors = new SafeSet()
   .add(kTestCodeFailure).add(kHookFailure)
@@ -508,7 +507,7 @@ class Test extends AsyncResource {
       this.diagnostic(warning);
     }
 
-    if (loc === undefined || kFilename === undefined) {
+    if (loc === undefined) {
       this.loc = undefined;
     } else {
       this.loc = {
diff --git a/lib/internal/test_runner/utils.js b/lib/internal/test_runner/utils.js
index 82af14c38e8baa..aae2a756800a0f 100644
--- a/lib/internal/test_runner/utils.js
+++ b/lib/internal/test_runner/utils.js
@@ -131,10 +131,9 @@ function tryBuiltinReporter(name) {
   return require(builtinPath);
 }
 
-function shouldColorizeTestFiles(rootTest) {
+function shouldColorizeTestFiles(destinations) {
   // This function assumes only built-in destinations (stdout/stderr) supports coloring
-  const { reporters, destinations } = parseCommandLine();
-  return ArrayPrototypeSome(reporters, (_, index) => {
+  return ArrayPrototypeSome(destinations, (_, index) => {
     const destination = kBuiltinDestinations.get(destinations[index]);
     return destination && shouldColorize(destination);
   });