Unable to FORCE_COLOR with test runner

[deadbeef84]

Version

v22.9.0

Platform

Linux deadbeef 6.8.0-45-generic #45-Ubuntu SMP PREEMPT_DYNAMIC Fri Aug 30 12:02:04 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

test_runner

What steps will reproduce the bug?

When running the test-runner outside of a tty, it's not possible to force colors.

FORCE_COLOR=1 node --test | cat

How often does it reproduce? Is there a required condition?

No condition.

What is the expected behavior? Why is that the expected behavior?

When FORCE_COLOR is set to 1, 2 or 3, I expect ansi color codes to be written to stdout even if it's not in a tty.

What do you see instead?

It doesn't respect the FORCE_COLOR and no ansi color codes are in the output.

Additional information

No response

[pmarchini]

Hey @deadbeef84, thanks for reporting this. I will take a look and come back to you😁

[RedYetiDev]

I can't reproduce:

require('node:test')('...', () => {
    console.log(require('node:util').styleText('red', 'red'));
});

w/

FORCE_COLOR=1 node --test repro.js | cat

It prints 'red' in red

[cjihrig]

You're testing console.log() and styleText(). I can reproduce the issue.

[cjihrig]

Actually, I believe the issue is that on Node 22, a non-tty causes the test runner to use the TAP reporter which doesn't use color. If I add --test-reporter=spec to the command, I do see colored output.

[RedYetiDev]

I still can't reproduce the issue, even with TAP :thinking: :

[cjihrig]

You're testing console.log() and styleText().

That red text doesn't come from the test runner, and the test runner doesn't strip the colors.

[pmarchini]

I haven't had the chance to take a close look at the issue, but I wonder if https://github.com/nodejs/node/blob/51d81466efc00417711558a08d0ff4206d8bf174/lib/internal/util/colors.js#L26-L42 could be the reason. I suppose that if process.stderr.isTTY is false, then even with force enabled, we're still getting "empty" colors.

WDYT?

[RedYetiDev]

Yes. I tried to fix it in the past, but failed (due to test failures).

See #54289

[RedYetiDev]

This issue is a duplicate of #52249

[pmarchini]

Hey @RedYetiDev, the reported issue is different, even though the fix is related.
I don't know if it makes sense to close this issue as not planned since it is, in fact, an issue, at least from my point of view.
I would expect test cases to also cover this specific scenario in a possible fix.

What do you think about this?
Should we just add a comment in the other issue pointing out this expected behavior as well?
I would expect some tests to be related to what was highlighted in this specific issue by @deadbeef84.

[cjihrig]

I agree with @pmarchini. Let's keep this open for now.

[cjihrig]

could be the reason. I suppose that if process.stderr.isTTY is false, then even with force enabled, we're still getting "empty" colors.

I believe you're right @pmarchini. The current code will only ever colorize output when a TTY is used. We probably don't need that if statement since the next line calls shouldColorize(), which does check the FORCE_COLOR env var.