test_runner: fixed test object is incorrectly passed to setup()

nodejs · Dec 11, 2023 · 6a3ebf7 · 6a3ebf7
1 parent 6e90fed
commit 6a3ebf7
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 7 deletions.
diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js
@@ -23,6 +23,7 @@ const {
   parseCommandLine,
   reporterScope,
   setupTestReporters,
+  colorizeTestFiles,
 } = require('internal/test_runner/utils');
 const { bigint: hrtime } = process.hrtime;
 
@@ -205,7 +206,8 @@ function getGlobalRoot() {
         process.exitCode = kGenericUserError;
       }
     });
-    reportersSetup = setupTestReporters(globalRoot);
+    reportersSetup = setupTestReporters(globalRoot.reporter);
+    colorizeTestFiles(globalRoot);
   }
   return globalRoot;
 }

diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js
@@ -70,6 +70,7 @@ const {
   convertStringToRegExp,
   countCompletedTest,
   kDefaultPattern,
+  colorizeTestFiles,
 } = require('internal/test_runner/utils');
 const { Glob } = require('internal/fs/glob');
 const { once } = require('events');
@@ -491,6 +492,7 @@ function run(options = kEmptyObject) {
     return root.reporter;
   }
   let testFiles = files ?? createTestFileList();
+  colorizeTestFiles(root);
 
   if (shard) {
     testFiles = ArrayPrototypeFilter(testFiles, (_, index) => index % shard.total === shard.index - 1);
@@ -512,7 +514,7 @@ function run(options = kEmptyObject) {
     });
   };
 
-  PromisePrototypeThen(PromisePrototypeThen(PromiseResolve(setup?.(root)), runFiles), postRun);
+  PromisePrototypeThen(PromisePrototypeThen(PromiseResolve(setup?.(root.reporter)), runFiles), postRun);
 
   return root.reporter;
 }

diff --git a/lib/internal/test_runner/utils.js b/lib/internal/test_runner/utils.js
@@ -3,6 +3,7 @@ const {
   ArrayPrototypeJoin,
   ArrayPrototypeMap,
   ArrayPrototypeFlatMap,
+  ArrayPrototypeForEach,
   ArrayPrototypePush,
   ArrayPrototypeReduce,
   ObjectGetOwnPropertyDescriptor,
@@ -128,10 +129,17 @@ function tryBuiltinReporter(name) {
   return require(builtinPath);
 }
 
-async function getReportersMap(reporters, destinations, rootTest) {
+function colorizeTestFiles(rootTest) {
+  const { reporters, destinations } = parseCommandLine();
+  ArrayPrototypeForEach(reporters, (_, index) => {
+    const destination = kBuiltinDestinations.get(destinations[index]) ?? createWriteStream(destinations[index]);
+    rootTest.harness.shouldColorizeTestFiles ||= shouldColorize(destination);
+  });
+}
+
+async function getReportersMap(reporters, destinations) {
   return SafePromiseAllReturnArrayLike(reporters, async (name, i) => {
     const destination = kBuiltinDestinations.get(destinations[i]) ?? createWriteStream(destinations[i]);
-    rootTest.harness.shouldColorizeTestFiles ||= shouldColorize(destination);
 
     // Load the test reporter passed to --test-reporter
     let reporter = tryBuiltinReporter(name);
@@ -166,12 +174,12 @@ async function getReportersMap(reporters, destinations, rootTest) {
 }
 
 const reporterScope = new AsyncResource('TestReporterScope');
-const setupTestReporters = reporterScope.bind(async (rootTest) => {
+const setupTestReporters = reporterScope.bind(async (rootReporter) => {
   const { reporters, destinations } = parseCommandLine();
-  const reportersMap = await getReportersMap(reporters, destinations, rootTest);
+  const reportersMap = await getReportersMap(reporters, destinations);
   for (let i = 0; i < reportersMap.length; i++) {
     const { reporter, destination } = reportersMap[i];
-    compose(rootTest.reporter, reporter).pipe(destination);
+    compose(rootReporter, reporter).pipe(destination);
   }
 });
 
@@ -413,6 +421,7 @@ function getCoverageReport(pad, summary, symbol, color, table) {
 }
 
 module.exports = {
+  colorizeTestFiles,
   convertStringToRegExp,
   countCompletedTest,
   createDeferredCallback,

diff --git a/test/parallel/test-runner-run.mjs b/test/parallel/test-runner-run.mjs
@@ -1,10 +1,14 @@
+// Flags: --expose-internals
+
 import * as common from '../common/index.mjs';
 import * as fixtures from '../common/fixtures.mjs';
 import { join } from 'node:path';
 import { describe, it, run } from 'node:test';
 import { dot, spec, tap } from 'node:test/reporters';
 import assert from 'node:assert';
+import stream from 'internal/test_runner/tests_stream';
 
+const { TestsStream } = stream;
 const testFixtures = fixtures.path('test-runner');
 
 describe('require(\'node:test\').run', { concurrency: true }, () => {
@@ -465,6 +469,19 @@ describe('require(\'node:test\').run', { concurrency: true }, () => {
           code: 'ERR_INVALID_ARG_TYPE'
         }));
     });
+
+    it('should pass instance of stream to setup', async () => {
+      const stream = run({
+        files: [join(testFixtures, 'default-behavior/test/random.cjs')],
+        setup: common.mustCall((root) => {
+          assert(root instanceof TestsStream);
+        }),
+      });
+      stream.on('test:fail', common.mustNotCall());
+      stream.on('test:pass', common.mustCall());
+      // eslint-disable-next-line no-unused-vars
+      for await (const _ of stream);
+    });
   });
 
   it('should run with no files', async () => {