Color output

[walles]

Summary

If TESTIFY_COLOR=true is in the environment, then expected and actual values will be coloured in green and red respectively.

Motivation

Stolen from #994:

1. @boyan-soubachov says "It's an interesting idea."
2. @mvdkleijn says "I like this idea."

Unlike #994, this PR is configured once for the whole run by setting an environment variable.

Related issues

• Closes Colorize "expected" and "actual" #946
• Related to Colorize 'expected' and 'actual' #994

[walles]

@boyan-soubachov could you have a look at this?

[walles]

@alxn you had opinions on the sibling PR which should not be an issue with this one.

Do you think this one would work?

[alxn]

Using an env-var to turn it on seems fine. At a later date someone can try to do the isatty check.

[vegerot]

Any updates?

[walles]

Any updates?

Not sure who you are asking here @vegerot, but the next step here is getting a review from some project maintainer.

Looking at the most recent merges, maintainers include:

• @boyan-soubachov
• @mvdkleijn

[vegerot]

While I understand the concerns about portability, file redirection, etc., it feels like all the other test runners I know (GoogleTest, Jest, Deno Test, ScalaTest, XCTest, etc.) all show colors when appropriate (jest does mess up sometimes when piping into other programs).

While I know this is a tricky problem to solve, it is definitely not intractable given these other frameworks

[ysmood]

Will it be false positive the failureMessage that contains those keywords?

[walles]

Getting the colors wrong would be possible if:

• failureMessage contains linefeeds
• one or more lines in failureMessage starts with either expected: or actual : (note : and spacing)

Just having the words expected or actual in the failureMessage would not trigger any miscoloring.

So yes, getting the colors wrong here is possible.

[GerardRodes]

Instead of defining a new env variable, Could we define this behavior based on the current environment?
Maybe with something like this: https://wiki.archlinux.org/title/Color_output_in_console#Enumerate_supported_colors

This is how people is doing it in js:
https://github.com/chalk/supports-color/blob/ee88ebcfca649233bb35c9b0db226059883a77b8/index.js#L59

[walles]

That JS function takes a streamIsTTY parameter:

function _supportsColor(haveStream, {streamIsTTY, sniffFlags = true} = {}) {

And I don't know how to figure that out :(

testify/assert/colorize.go

Lines 21 to 25 in c602661

	func figureOutShouldColorize() bool {
	// FIXME: The auto response should be true on terminals and false on
	// not-terminals, but my attempts at checking os.Stdout() terminalness have
	// all failed. Patches welcome!
	autoResponse := false

... so I can't really do it like that unfortunately.

[walles]

And in the long run, somebody will want to override the default behavior, and then the environment variable is likely to be needed anyway.

[walles]

@GerardRodes is this OK or do you have some suggestion for how to improve it?

[walles]

I think leaving this open doesn't provide any value, closing.

Feel free to reopen if a reviewer becomes available!