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

PR-URL: #50982
Reviewed-By: Raz Luvaton <rluvaton@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>

lib/internal/test_runner/harness.js

@@ -23,6 +23,7 @@ const {
   parseCommandLine,
   reporterScope,
   setupTestReporters,
+  shouldColorizeTestFiles,
 } = 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);
+    globalRoot.harness.shouldColorizeTestFiles ||= shouldColorizeTestFiles(globalRoot);
   }
   return globalRoot;
 }

lib/internal/test_runner/runner.js

@@ -70,6 +70,7 @@ const {
   countCompletedTest,
   doesPathMatchFilter,
   isSupportedFileType,
+  shouldColorizeTestFiles,
 } = require('internal/test_runner/utils');
 const { basename, join, resolve } = require('path');
 const { once } = require('events');
@@ -531,6 +532,8 @@ function run(options) {
   }
 
   const root = createTestTree({ __proto__: null, concurrency, timeout, signal });
+  root.harness.shouldColorizeTestFiles ||= shouldColorizeTestFiles(root);
+
   if (process.env.NODE_TEST_CONTEXT !== undefined) {
     return root.reporter;
   }
@@ -556,7 +559,7 @@ function run(options) {
     });
   };
 
-  PromisePrototypeThen(PromisePrototypeThen(PromiseResolve(setup?.(root)), runFiles), postRun);
+  PromisePrototypeThen(PromisePrototypeThen(PromiseResolve(setup?.(root.reporter)), runFiles), postRun);
 
   return root.reporter;
 }

lib/internal/test_runner/utils.js

@@ -5,6 +5,7 @@ const {
   ArrayPrototypeFlatMap,
   ArrayPrototypePush,
   ArrayPrototypeReduce,
+  ArrayPrototypeSome,
   ObjectGetOwnPropertyDescriptor,
   MathFloor,
   MathMax,
@@ -134,10 +135,18 @@ function tryBuiltinReporter(name) {
   return require(builtinPath);
 }
 
-async function getReportersMap(reporters, destinations, rootTest) {
+function shouldColorizeTestFiles(rootTest) {
+  // This function assumes only built-in destinations (stdout/stderr) supports coloring
+  const { reporters, destinations } = parseCommandLine();
+  return ArrayPrototypeSome(reporters, (_, index) => {
+    const destination = kBuiltinDestinations.get(destinations[index]);
+    return destination && 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);
@@ -172,12 +181,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);
   }
 });
 
@@ -428,5 +437,6 @@ module.exports = {
   parseCommandLine,
   reporterScope,
   setupTestReporters,
+  shouldColorizeTestFiles,
   getCoverageReport,
 };

test/parallel/test-runner-reporters.js

@@ -155,4 +155,23 @@ describe('node:test reporters', { concurrency: true }, () => {
     assert.strictEqual(child.stdout.toString(), 'Going to throw an error\n');
     assert.match(child.stderr.toString(), /Emitted 'error' event on Duplex instance/);
   });
+
+  it('should support stdout as a destination with spec reporter', async () => {
+    process.env.FORCE_COLOR = '1';
+    const file = tmpdir.resolve(`${tmpFiles++}.txt`);
+    const child = spawnSync(process.execPath,
+                            ['--test', '--test-reporter', 'spec', '--test-reporter-destination', file, testFile]);
+    assert.strictEqual(child.stderr.toString(), '');
+    assert.strictEqual(child.stdout.toString(), '');
+    const fileConent = fs.readFileSync(file, 'utf8');
+    assert.match(fileConent, /▶ nested/);
+    assert.match(fileConent, /✔ ok/);
+    assert.match(fileConent, /✖ failing/);
+    assert.match(fileConent, /ℹ tests 4/);
+    assert.match(fileConent, /ℹ pass 2/);
+    assert.match(fileConent, /ℹ fail 2/);
+    assert.match(fileConent, /ℹ cancelled 0/);
+    assert.match(fileConent, /ℹ skipped 0/);
+    assert.match(fileConent, /ℹ todo 0/);
+  });
 });

test/parallel/test-runner-run.mjs

@@ -459,6 +459,21 @@ describe('require(\'node:test\').run', { concurrency: true }, () => {
     });
   });
 
+  describe('validation', () => {
+    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.strictEqual(root.constructor.name, '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 () => {
     const stream = run({
       files: undefined