From 69c78ca2f5d8307671a6f0341e37adf9a4f46d69 Mon Sep 17 00:00:00 2001
From: cjihrig <cjihrig@gmail.com>
Date: Mon, 12 Aug 2024 20:18:27 -0400
Subject: [PATCH] test_runner: return setup() from parseCommandLine()

Now that parseCommandLine() returns run() compatible arguments,
it makes sense to return setupTestReporters() as the setup()
argument to run(). This also removes another problematic use of
parseCommandLine() in setupTestReporters().

PR-URL: https://github.com/nodejs/node/pull/54353
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
---
 lib/internal/main/test_runner.js    |  6 +-----
 lib/internal/test_runner/harness.js |  6 +++---
 lib/internal/test_runner/runner.js  |  1 +
 lib/internal/test_runner/utils.js   | 20 ++++++++++----------
 4 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/lib/internal/main/test_runner.js b/lib/internal/main/test_runner.js
index 9785a74cd1a665..cc853da7388821 100644
--- a/lib/internal/main/test_runner.js
+++ b/lib/internal/main/test_runner.js
@@ -10,10 +10,7 @@ const {
 } = require('internal/process/pre_execution');
 const { isUsingInspector } = require('internal/util/inspector');
 const { run } = require('internal/test_runner/runner');
-const {
-  parseCommandLine,
-  setupTestReporters,
-} = require('internal/test_runner/utils');
+const { parseCommandLine } = require('internal/test_runner/utils');
 const { exitCodes: { kGenericUserError } } = internalBinding('errors');
 let debug = require('internal/util/debuglog').debuglog('test_runner', (fn) => {
   debug = fn;
@@ -31,7 +28,6 @@ if (isUsingInspector()) {
   options.inspectPort = process.debugPort;
 }
 
-options.setup = setupTestReporters;
 options.globPatterns = ArrayPrototypeSlice(process.argv, 1);
 
 debug('test runner configuration:', options);
diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js
index 9f78319bef5727..9c372c115e90f2 100644
--- a/lib/internal/test_runner/harness.js
+++ b/lib/internal/test_runner/harness.js
@@ -21,7 +21,6 @@ const { kCancelledByParent, Test, Suite } = require('internal/test_runner/test')
 const {
   parseCommandLine,
   reporterScope,
-  setupTestReporters,
   shouldColorizeTestFiles,
 } = require('internal/test_runner/utils');
 const { queueMicrotask } = require('internal/process/task_queues');
@@ -231,13 +230,14 @@ function lazyBootstrapRoot() {
       __proto__: null,
       entryFile: process.argv?.[1],
     };
-    createTestTree(rootTestOptions, parseCommandLine());
+    const globalOptions = parseCommandLine();
+    createTestTree(rootTestOptions, globalOptions);
     globalRoot.reporter.on('test:fail', (data) => {
       if (data.todo === undefined || data.todo === false) {
         process.exitCode = kGenericUserError;
       }
     });
-    globalRoot.harness.bootstrapPromise = setupTestReporters(globalRoot.reporter);
+    globalRoot.harness.bootstrapPromise = globalOptions.setup(globalRoot.reporter);
   }
   return globalRoot;
 }
diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js
index 59263bfada2b12..e994b1aa40ecab 100644
--- a/lib/internal/test_runner/runner.js
+++ b/lib/internal/test_runner/runner.js
@@ -573,6 +573,7 @@ function run(options = kEmptyObject) {
     // parseCommandLine() should not be used here. However, The existing run()
     // behavior has relied on it, so removing it must be done in a semver major.
     ...parseCommandLine(),
+    setup,  // This line can be removed when parseCommandLine() is removed here.
   };
   const root = createTestTree(rootTestOptions, globalOptions);
 
diff --git a/lib/internal/test_runner/utils.js b/lib/internal/test_runner/utils.js
index bbd69aa4211415..e6c421ff870bbd 100644
--- a/lib/internal/test_runner/utils.js
+++ b/lib/internal/test_runner/utils.js
@@ -176,15 +176,6 @@ async function getReportersMap(reporters, destinations) {
 }
 
 const reporterScope = new AsyncResource('TestReporterScope');
-const setupTestReporters = reporterScope.bind(async (rootReporter) => {
-  const { reporters, destinations } = parseCommandLine();
-  const reportersMap = await getReportersMap(reporters, destinations);
-  for (let i = 0; i < reportersMap.length; i++) {
-    const { reporter, destination } = reportersMap[i];
-    compose(rootReporter, reporter).pipe(destination);
-  }
-});
-
 let globalTestOptions;
 
 function parseCommandLine() {
@@ -281,6 +272,15 @@ function parseCommandLine() {
     coverageIncludeGlobs = getOptionValue('--test-coverage-include');
   }
 
+  const setup = reporterScope.bind(async (rootReporter) => {
+    const reportersMap = await getReportersMap(reporters, destinations);
+
+    for (let i = 0; i < reportersMap.length; i++) {
+      const { reporter, destination } = reportersMap[i];
+      compose(rootReporter, reporter).pipe(destination);
+    }
+  });
+
   globalTestOptions = {
     __proto__: null,
     isTestRunner,
@@ -292,6 +292,7 @@ function parseCommandLine() {
     forceExit,
     only,
     reporters,
+    setup,
     shard,
     sourceMaps,
     testNamePatterns,
@@ -480,7 +481,6 @@ module.exports = {
   kDefaultPattern,
   parseCommandLine,
   reporterScope,
-  setupTestReporters,
   shouldColorizeTestFiles,
   getCoverageReport,
 };