test_runner: align stdout and std error with and without --test

lib/internal/test_runner/harness.js

@@ -179,6 +179,7 @@ function setup(root) {
       topLevel: 0,
       suites: 0,
     },
+    shouldColorizeTestFiles: false,
   };
   root.startTime = hrtime();
   return root;

lib/internal/test_runner/reporter/spec.js

@@ -119,7 +119,7 @@ class SpecReporter extends Transform {
         break;
       case 'test:stderr':
       case 'test:stdout':
-        return `${data.message}\n`;
+        return data.message;
       case 'test:diagnostic':
         return `${colors[type]}${this.#indent(data.nesting)}${symbols[type]}${data.message}${white}\n`;
       case 'test:coverage':

lib/internal/test_runner/reporter/tap.js

@@ -5,10 +5,10 @@ const {
   ArrayPrototypePush,
   ObjectEntries,
   RegExpPrototypeSymbolReplace,
+  RegExpPrototypeSymbolSplit,
   SafeMap,
   SafeSet,
   StringPrototypeReplaceAll,
-  StringPrototypeSplit,
   StringPrototypeRepeat,
 } = primordials;
 const { inspectWithNoCustomRetry } = require('internal/errors');
@@ -46,8 +46,14 @@ async function * tapReporter(source) {
         yield `${indent(data.nesting)}# Subtest: ${tapEscape(data.name)}\n`;
         break;
       case 'test:stderr':
-      case 'test:stdout':
-      case 'test:diagnostic':
+      case 'test:stdout': {
+        const lines = RegExpPrototypeSymbolSplit(kLineBreakRegExp, data.message);
+        for (let i = 0; i < lines.length; i++) {
+          if (lines[i].length === 0) continue;
+          yield `# ${tapEscape(lines[i])}\n`;
+        }
+        break;
+      } case 'test:diagnostic':
         yield `${indent(data.nesting)}# ${tapEscape(data.message)}\n`;
         break;
       case 'test:coverage':
@@ -124,7 +130,7 @@ function jsToYaml(indent, name, value, seen) {
       return `${prefix}${inspectWithNoCustomRetry(value, inspectOptions)}\n`;
     }
 
-    const lines = StringPrototypeSplit(value, kLineBreakRegExp);
+    const lines = RegExpPrototypeSymbolSplit(kLineBreakRegExp, value);
 
     if (lines.length === 1) {
       return `${prefix}${inspectWithNoCustomRetry(value, inspectOptions)}\n`;
@@ -224,7 +230,7 @@ function jsToYaml(indent, name, value, seen) {
       const frames = [];
 
       ArrayPrototypeForEach(
-        StringPrototypeSplit(errStack, kLineBreakRegExp),
+        RegExpPrototypeSymbolSplit(kLineBreakRegExp, errStack),
         (frame) => {
           const processed = RegExpPrototypeSymbolReplace(
             kFrameStartRegExp,

lib/internal/test_runner/runner.js

@@ -11,14 +11,12 @@ const {
   ArrayPrototypeSlice,
   ArrayPrototypeSome,
   ArrayPrototypeSort,
-  hardenRegExp,
   ObjectAssign,
   PromisePrototypeThen,
   SafePromiseAll,
   SafePromiseAllReturnVoid,
   SafePromiseAllSettledReturnVoid,
   PromiseResolve,
-  RegExpPrototypeSymbolSplit,
   SafeMap,
   SafeSet,
   StringPrototypeIndexOf,
@@ -75,7 +73,6 @@ const {
 const kFilterArgs = ['--test', '--experimental-test-coverage', '--watch'];
 const kFilterArgValues = ['--test-reporter', '--test-reporter-destination'];
 const kDiagnosticsFilterArgs = ['tests', 'suites', 'pass', 'fail', 'cancelled', 'skipped', 'todo', 'duration_ms'];
-const kSplitLine = hardenRegExp(/\r?\n/);
 
 const kCanceledTests = new SafeSet()
   .add(kCancelledByParent).add(kAborted).add(kTestTimeoutFailure);
@@ -280,15 +277,11 @@ class FileTest extends Test {
     }
 
     if (TypedArrayPrototypeGetLength(nonSerialized) > 0) {
-      const messages = RegExpPrototypeSymbolSplit(kSplitLine, nonSerialized.toString('utf-8'));
-      for (let i = 0; i < messages.length; i++) {
-        const message = messages[i];
-        this.addToReport({
-          __proto__: null,
-          type: 'test:stdout',
-          data: { __proto__: null, file: this.name, message },
-        });
-      }
+      this.addToReport({
+        __proto__: null,
+        type: 'test:stdout',
+        data: { __proto__: null, file: this.name, message: nonSerialized.toString('utf-8') },
+      });
     }
 
     while (bufferHead?.length >= kSerializedSizeHeader) {
@@ -333,6 +326,9 @@ function runTestFile(path, root, inspectPort, filesWatcher, testNamePatterns) {
       stdio.push('ipc');
       env.WATCH_REPORT_DEPENDENCIES = '1';
     }
+    if (root.harness.shouldColorizeTestFiles) {
+      env.FORCE_COLOR = '1';
+    }
 
     const child = spawn(process.execPath, args, { signal: t.signal, encoding: 'utf8', env, stdio });
     runningProcesses.set(path, child);
@@ -362,7 +358,7 @@ function runTestFile(path, root, inspectPort, filesWatcher, testNamePatterns) {
       subtest.addToReport({
         __proto__: null,
         type: 'test:stderr',
-        data: { __proto__: null, file: path, message: line },
+        data: { __proto__: null, file: path, message: line + '\n' },
       });
     });
 

lib/internal/test_runner/utils.js

@@ -16,7 +16,7 @@ const { createWriteStream } = require('fs');
 const { pathToFileURL } = require('internal/url');
 const { createDeferredPromise } = require('internal/util');
 const { getOptionValue } = require('internal/options');
-const { green, red, white } = require('internal/util/colors');
+const { green, red, white, shouldColorize } = require('internal/util/colors');
 
 const {
   codes: {
@@ -115,9 +115,10 @@ function tryBuiltinReporter(name) {
   return require(builtinPath);
 }
 
-async function getReportersMap(reporters, destinations) {
+async function getReportersMap(reporters, destinations, rootTest) {
   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);
@@ -154,7 +155,7 @@ async function getReportersMap(reporters, destinations) {
 
 async function setupTestReporters(rootTest) {
   const { reporters, destinations } = parseCommandLine();
-  const reportersMap = await getReportersMap(reporters, destinations);
+  const reportersMap = await getReportersMap(reporters, destinations, rootTest);
   for (let i = 0; i < reportersMap.length; i++) {
     const { reporter, destination } = reportersMap[i];
     compose(rootTest.reporter, reporter).pipe(destination);

test/fixtures/test-runner/output/arbitrary-output-colored-1.js

@@ -0,0 +1,7 @@
+'use strict';
+
+const test = require('node:test');
+console.log({ foo: 'bar' });
+test('passing test', () => {
+  console.log(1);
+});

test/fixtures/test-runner/output/arbitrary-output-colored.js

@@ -0,0 +1,11 @@
+'use strict';
+const common = require('../../../common');
+const { once } = require('node:events');
+const { spawn } = require('node:child_process');
+const fixtures = require('../../../common/fixtures');
+
+(async function run() {
+  const test = fixtures.path('test-runner/output/arbitrary-output-colored-1.js');
+  await once(spawn(process.execPath, ['--test', test], { stdio: 'inherit', env: { FORCE_COLOR: 1 } }), 'exit');
+  await once(spawn(process.execPath, ['--test', '--test-reporter', 'tap', test], { stdio: 'inherit', env: { FORCE_COLOR: 1 }  }), 'exit');
+})().then(common.mustCall());

test/fixtures/test-runner/output/arbitrary-output-colored.snapshot

@@ -0,0 +1,28 @@
+{ foo: [32m'bar'[39m }
+[33m1[39m
+[32m✔ passing test [90m(*ms)[39m[39m
+[34mℹ tests 1[39m
+[34mℹ suites 0[39m
+[34mℹ pass 1[39m
+[34mℹ fail 0[39m
+[34mℹ cancelled 0[39m
+[34mℹ skipped 0[39m
+[34mℹ todo 0[39m
+[34mℹ duration_ms *[39m
+TAP version 13
+# { foo: [32m'bar'[39m }
+# [33m1[39m
+# Subtest: passing test
+ok 1 - passing test
+  ---
+  duration_ms: *
+  ...
+1..1
+# tests 1
+# suites 0
+# pass 1
+# fail 0
+# cancelled 0
+# skipped 0
+# todo 0
+# duration_ms *

test/parallel/test-runner-output.mjs

@@ -3,6 +3,10 @@ import * as fixtures from '../common/fixtures.mjs';
 import * as snapshot from '../common/assertSnapshot.js';
 import { describe, it } from 'node:test';
 
+const skipForceColors =
+  process.config.variables.icu_gyp_path !== 'tools/icu/icu-generic.gyp' ||
+  process.config.variables.node_shared_openssl;
+
 function replaceTestDuration(str) {
   return str
     .replaceAll(/duration_ms: 0(\r?\n)/g, 'duration_ms: ZERO$1')
@@ -46,8 +50,14 @@ const tests = [
   { name: 'test-runner/output/unresolved_promise.js' },
   { name: 'test-runner/output/default_output.js', transform: specTransform, tty: true },
   { name: 'test-runner/output/arbitrary-output.js' },
+  !skipForceColors ? {
+    name: 'test-runner/output/arbitrary-output-colored.js',
+    transform: snapshot.transform(specTransform, replaceTestDuration), tty: true
+  } : false,
   { name: 'test-runner/output/dot_output_custom_columns.js', transform: specTransform, tty: true },
-].map(({ name, tty, transform }) => ({
+]
+.filter(Boolean)
+.map(({ name, tty, transform }) => ({
   name,
   fn: common.mustCall(async () => {
     await snapshot.spawnAndAssert(fixtures.path(name), transform ?? defaultTransform, { tty });