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: #54353 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
nodejs · Aug 21, 2024 · 69c78ca · 69c78ca
1 parent ed1ede8
commit 69c78ca
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 18 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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,
 };