`styleText()`: isTTY check fails with `--test`

[regseb]

Version

v22.13.1

Platform

Linux 6.8.0-57-generic #59~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Wed Mar 19 17:07:41 UTC 2 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

Ubuntu 22.04.5 LTS

What steps will reproduce the bug?

• Create index.mjs:

  import { Writable } from "node:stream";
  import { styleText } from "node:util";
  
  const streamTTY = new class extends Writable {
      isTTY = true;
  };
  const streamNoTTY = new class extends Writable {
      isTTY = false;
  };
  
  console.log(styleText("bgYellow", "TTY", { stream: streamTTY }));
  console.log(styleText("bgYellow", "No TTY", { stream: streamNoTTY }));

• Run node index.mjs

• Run node --test index.mjs

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

Always.

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

The text No TTY isn't stylized with --test.

• node index.mjs

  ^[[43mTTY^[[49m
  No TTY
  

• node --test index.mjs

  ^[[43mTTY^[[49m
  No TTY
  ✔ index.mjs (27.022452ms)
  ...
  

What do you see instead?

The text No TTY is stylized with --test.

• node index.mjs

  ^[[43mTTY^[[49m
  No TTY
  

• node --test index.mjs

  ^[[43mTTY^[[49m
  ^[[43mNo TTY^[[49m
  ✔ index.mjs (27.022452ms)
  ...
  

Additional information

With the --test option, the styleText() function doesn't check whether the stream is TTY.

---

With the command line: node --test index.mjs > out.txt, the file contains the correct style:

TAP version 13
# ^[[43mTTY^[[49m
# No TTY
# Subtest: index.mjs
ok 1 - index.mjs
  ---
  duration_ms: 25.169793
  ...
1..1
# tests 1
# suites 0
# pass 1
# fail 0
# cancelled 0
# skipped 0
# todo 0
# duration_ms 30.191704

[regseb]

I think there's a difference with --test, because process.env.FORCE_COLOR is set to 1. In this case: the shouldColorize() method forces the color (by getColorDepth() unless FORCE_COLOR is different from '', '1', 'true', '2' and '3').

node/lib/internal/util/colors.js

Lines 18 to 25 in 6cdcaa9

	shouldColorize(stream) {
	if (process.env.FORCE_COLOR !== undefined) {
	return lazyInternalTTY().getColorDepth() > 2;
	}
	return stream?.isTTY && (
	typeof stream.getColorDepth === 'function' ?
	stream.getColorDepth() > 2 : true);
	},

I think it's this line that forces the color:

node/lib/internal/test_runner/runner.js

Lines 375 to 377 in 6cdcaa9

	if (opts.root.harness.shouldColorizeTestFiles) {
	env.FORCE_COLOR = '1';
	}

The shouldColorizeTestFiles() method uses shouldColorize().

node/lib/internal/test_runner/utils.js

Lines 145 to 151 in 6cdcaa9

	function shouldColorizeTestFiles(destinations) {
	// This function assumes only built-in destinations (stdout/stderr) supports coloring
	return ArrayPrototypeSome(destinations, (_, index) => {
	const destination = kBuiltinDestinations.get(destinations[index]);
	return destination && shouldColorize(destination);
	});
	}

I don't think you should force color when it's supported. It should be the functions that stylize the test results that should call shouldColorize() to check whether they are adding color. This already seems to be the case in lib/internal/test_runner/reporter/utils.js.