diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index da3bd744ffc1..43de627d23bd 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -1,6 +1,7 @@ 'use strict'; const { + JSONStringify, ArrayIsArray, ArrayPrototypeEvery, ArrayPrototypeFilter, @@ -528,7 +529,7 @@ function runTestFile(path, filesWatcher, opts) { const subtest = opts.root.createSubtest(FileTest, testPath, testOpts, async (t) => { const args = getRunArgs(path, opts); const stdio = ['pipe', 'pipe', 'pipe']; - const env = { __proto__: null, NODE_TEST_CONTEXT: 'child-v8', ...(opts.env || process.env) }; + const env = { __proto__: null, ...(opts.env || process.env) }; // Acquire a worker ID from the pool for process isolation mode let workerId; @@ -542,9 +543,15 @@ function runTestFile(path, filesWatcher, opts) { stdio.push('ipc'); env.WATCH_REPORT_DEPENDENCIES = '1'; } - if (opts.root.harness.shouldColorizeTestFiles) { - env.FORCE_COLOR = '1'; - } + // Encode the test-runner context and colorize flag in NODE_TEST_CONTEXT as + // a JSON string. Using a dedicated field avoids setting FORCE_COLOR, which + // would bleed into user code and override explicit stream.isTTY=false checks + // in util.styleText(). See https://github.com/nodejs/node/issues/57921. + env.NODE_TEST_CONTEXT = JSONStringify({ + __proto__: null, + context: 'child-v8', + colorize: opts.root.harness.shouldColorizeTestFiles, + }); const child = spawn( process.execPath, args, diff --git a/lib/internal/test_runner/utils.js b/lib/internal/test_runner/utils.js index 38b54c31685c..e89e7d90688d 100644 --- a/lib/internal/test_runner/utils.js +++ b/lib/internal/test_runner/utils.js @@ -256,8 +256,13 @@ function parseCommandLine() { const randomSeedOption = getOptionValue('--test-random-seed'); let randomSeed; const rerunFailuresFilePath = getOptionValue('--test-rerun-failures'); - const isChildProcess = process.env.NODE_TEST_CONTEXT === 'child'; - const isChildProcessV8 = process.env.NODE_TEST_CONTEXT === 'child-v8'; + const nodeTestContext = process.env.NODE_TEST_CONTEXT; + const parsedTestContext = nodeTestContext?.[0] === '{' ? (() => { + try { return JSONParse(nodeTestContext); } catch { return null; } + })() : null; + const isChildProcess = nodeTestContext === 'child'; + const isChildProcessV8 = nodeTestContext === 'child-v8' || + parsedTestContext?.context === 'child-v8'; let globalSetupPath; let concurrency; let coverageExcludeGlobs; diff --git a/lib/internal/util/colors.js b/lib/internal/util/colors.js index 0b37694b513f..d987508dc0c4 100644 --- a/lib/internal/util/colors.js +++ b/lib/internal/util/colors.js @@ -6,6 +6,25 @@ function lazyInternalTTY() { return internalTTy; } +let testContext; +function getTestContext() { + if (testContext === undefined) { + const raw = process.env.NODE_TEST_CONTEXT; + if (raw !== undefined && raw[0] === '{') { + try { + // NODE_TEST_CONTEXT is a JSON object when set by the test runner's + // process isolation mode. We read it lazily and cache the result. + testContext = JSON.parse(raw); + } catch { + testContext = null; + } + } else { + testContext = null; + } + } + return testContext; +} + module.exports = { blue: '', green: '', @@ -16,12 +35,22 @@ module.exports = { reset: '', hasColors: false, shouldColorize(stream) { + // Process-level FORCE_COLOR has the highest priority. if (process.env.FORCE_COLOR !== undefined) { return lazyInternalTTY().getColorDepth() > 2; } - return stream?.isTTY && ( - typeof stream.getColorDepth === 'function' ? - stream.getColorDepth() > 2 : true); + // The stream's own isTTY capability is checked next. + if (stream?.isTTY) { + return typeof stream.getColorDepth === 'function' ? + stream.getColorDepth() > 2 : true; + } + // When running as a test-runner child process, use the parent's colorize + // decision (encoded in NODE_TEST_CONTEXT) as a last resort. This avoids + // injecting FORCE_COLOR into the child, which would override the user's + // explicit stream.isTTY=false checks in util.styleText(). + // See https://github.com/nodejs/node/issues/57921. + const ctx = getTestContext(); + return ctx?.colorize === true && lazyInternalTTY().getColorDepth() > 2; }, refresh() { if (module.exports.shouldColorize(process.stderr)) { diff --git a/test/parallel/test-util-styletext.js b/test/parallel/test-util-styletext.js index 3db01bec1c3a..83329a4c5057 100644 --- a/test/parallel/test-util-styletext.js +++ b/test/parallel/test-util-styletext.js @@ -199,8 +199,23 @@ if (fd !== -1) { { isTTY: true, env: { NO_COLOR: '1' }, expected: noChange }, { isTTY: true, env: { FORCE_COLOR: '1' }, expected: styled }, { isTTY: true, env: { FORCE_COLOR: '1', NODE_DISABLE_COLORS: '1' }, expected: styled }, + // FORCE_COLOR overrides isTTY for a stream — this is intentional: the user + // can always force colours at the process level via FORCE_COLOR. { isTTY: false, env: { FORCE_COLOR: '1', NO_COLOR: '1', NODE_DISABLE_COLORS: '1' }, expected: styled }, { isTTY: true, env: { FORCE_COLOR: '1', NO_COLOR: '1', NODE_DISABLE_COLORS: '1' }, expected: styled }, + // NODE_TEST_CONTEXT with colorize=true enables colour for non-TTY streams + // without clobbering FORCE_COLOR. + // Regression test for https://github.com/nodejs/node/issues/57921 + { + isTTY: false, + env: { NODE_TEST_CONTEXT: JSON.stringify({ context: 'child-v8', colorize: true }) }, + expected: styled, + }, + { + isTTY: false, + env: { NODE_TEST_CONTEXT: JSON.stringify({ context: 'child-v8', colorize: false }) }, + expected: noChange, + }, ].forEach((testCase) => { writeStream.isTTY = testCase.isTTY; process.env = { @@ -221,3 +236,46 @@ if (fd !== -1) { } else { common.skip('Could not create TTY fd'); } + +// Regression test for https://github.com/nodejs/node/issues/57921: +// When `node --test` runs a file in isolation mode it used to inject +// FORCE_COLOR=1 into the child process, causing util.styleText() to +// colorise streams whose isTTY is explicitly false. +// The fix encodes the colorize intent in NODE_TEST_CONTEXT instead, +// so that FORCE_COLOR is no longer clobbered and user code is unaffected. +{ + const originalEnv = process.env; + + // With NODE_TEST_CONTEXT.colorize=true (new mechanism) a non-TTY stream + // should be colorised when no explicit stream isTTY override is present, + // but FORCE_COLOR must NOT be set. + process.env = { + ...process.env, + NODE_TEST_CONTEXT: JSON.stringify({ context: 'child-v8', colorize: true }), + }; + delete process.env.FORCE_COLOR; + + // No stream supplied — falls through to NODE_TEST_CONTEXT.colorize + assert.strictEqual( + util.styleText('red', 'test', { validateStream: false }), + styled, + 'NODE_TEST_CONTEXT.colorize=true should colorize when no stream is supplied', + ); + + process.env = originalEnv; + + // Without FORCE_COLOR and without NODE_TEST_CONTEXT, a non-TTY stream + // should produce no colour — the original bug would only appear under --test. + process.env = { ...process.env }; + delete process.env.FORCE_COLOR; + delete process.env.NODE_TEST_CONTEXT; + + const nonTTYStream = { isTTY: false }; + assert.strictEqual( + util.styleText('red', 'test', { stream: nonTTYStream, validateStream: false }), + noChange, + 'A non-TTY stream with no FORCE_COLOR or NODE_TEST_CONTEXT must not be colourised', + ); + + process.env = originalEnv; +}