lib,test_runner: fix styleText colorizing non-TTY streams under --test - #65558
Open
piyushrajyadav wants to merge 1 commit into
Open
lib,test_runner: fix styleText colorizing non-TTY streams under --test#65558piyushrajyadav wants to merge 1 commit into
piyushrajyadav wants to merge 1 commit into
Conversation
When
ode --test runs files in process-isolation mode it used to set
FORCE_COLOR=1 in the child's environment so that test-runner output
arrives pre-colored. A side-effect was that util.styleText() inside
the child process would colorize *any* stream, even those with an
explicit isTTY=false, because shouldColorize() short-circuits to
getColorDepth() as soon as FORCE_COLOR is defined.
Fix: stop setting FORCE_COLOR in the child environment. Instead, encode
the parent's colorize decision as a JSON field in NODE_TEST_CONTEXT
(the existing internal-only variable that child processes already use
to identify themselves as test-runner workers):
NODE_TEST_CONTEXT = JSON.stringify({ context: 'child-v8', colorize: <bool> })
In lib/internal/util/colors.js shouldColorize() is updated to use that
field as a *last-resort* fallback when neither FORCE_COLOR nor
stream.isTTY is truthy. The priority order is now:
1. FORCE_COLOR (user-controlled, highest priority)
2. stream.isTTY / stream.getColorDepth()
3. NODE_TEST_CONTEXT.colorize (test-runner internal, last resort)
lib/internal/test_runner/utils.js and runner.js are updated to parse
the JSON form of NODE_TEST_CONTEXT alongside the legacy plain-string
values ('child' / 'child-v8') for backwards compatibility.
Fixes: nodejs#57921
Refs: nodejs#57935
Signed-off-by: piyushrajyadav <piyushyadavrajyadav@gmail.com>
piyushrajyadav
force-pushed
the
fix-styletext-istty-force-color
branch
from
August 26, 2026 08:05
9229f1f to
d734d0c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes: #57921
Refs: #57935 (previous attempt, closed stale)
Problem
When
ode --test runs files in process-isolation mode it sets FORCE_COLOR=1 in the child's environment to carry test-reporter colour through. A side-effect is that util.styleText() in user code inside the child colorises any stream β even those with isTTY = false β because shouldColorize() short-circuits to getColorDepth() as soon as FORCE_COLOR is defined, ignoring the caller's stream entirely.
js // With node --test, FORCE_COLOR=1 is injected into the child env. // Both of these then return ANSI-styled text, even though streamNoTTY.isTTY = false: styleText('bgYellow', 'TTY', { stream: streamTTY }); // β correct styleText('bgYellow', 'No TTY', { stream: streamNoTTY }); // β WRONG (bug)Approach
Previous PR #57935 explored a few directions. cjihrig suggested not hacking FORCE_COLOR at all and using NODE_TEST_CONTEXT (the existing test-runner-internal env var) as a communication channel instead.
This PR implements exactly that:
lib/internal/test_runner/runner.js
lib/internal/util/colors.js β shouldColorize() priority order
js shouldColorize(stream) { if (process.env.FORCE_COLOR !== undefined) { return lazyInternalTTY().getColorDepth() > 2; } if (stream?.isTTY) { return typeof stream.getColorDepth === 'function' ? stream.getColorDepth() > 2 : true; } // Last resort: test-runner internal colorize flag const ctx = getTestContext(); return ctx?.colorize === true && lazyInternalTTY().getColorDepth() > 2; },lib/internal/test_runner/utils.js
Testing