gotestyourself / gotestsum

'go test' runner with output optimized for humans, JUnit XML for CI integration, and a summary of the test results.
Apache License 2.0
2.03k stars 119 forks source link

Detect CI environments for default `no-color` value #330

Closed silverwind closed 1 year ago

silverwind commented 1 year ago

Use go-supportscolor to detect color support for CI environments and more, which among GitHub Actions will enable color support on number of other environments which previously could not display color at all with gotestsum.

dnephin commented 1 year ago

Thank you for the PR! It would be great to enable color by default on any CI systems that support it. I believe color should already be enabled by default in any interactive terminal.

I'd like to avoiding adding another dependency for this. I believe all we need is a few of the lines around here: https://github.com/jwalton/go-supportscolor/blob/198795bbe247f127f76b501948f2f41fc88b8080/supportscolor.go#L213-L215

Some systems, like CircleCI already do use color, because they properly emulate a terminal.

It's also worth noting that it's always possible to enable color using --no-color=false. This change would enable color by default in some environments, but it should have already been possible to enable color in those environments with the command line flag.

silverwind commented 1 year ago

I'd like to avoiding adding another dependency for this

Having it via a dependency ensures that future environments will only need to be supported in one repo instead of hundrets. I see a benefit in doing it via a dependency, even if it does "a bit more" like this one does.

I planned to PR support for a the new Gitea Actions environment into go-supportscolor, but currently it is struggling with 256 colors, that's why I held off from it.

In case you still don't want go-supportscolor, I will alter the PR as per the recommendations, remove the dependency and also include GITEA_ACTIONS=true detection for Gitea Actions which can do 16 color fine currently, and I think that is all this module uses.

--no-color=false: I searched for an CLI option to force-enable color, but it wouldn't have dawned me that this double negation actually would do the trick. Good to know, but I would suggest a --color or --force-color option as that is much more obvious.

dnephin commented 1 year ago

I will alter the PR as per the recommendations, remove the dependency and also include GITEA_ACTIONS=true detection for Gitea Actions

That sounds great! Thank you for making this change!

I would suggest a --color option as that is much more obvious.

I agree it is not very obvious right now. Hiding the --no-color option and changing to --color would be a great improvement. We can continue to default that flag to true when a terminal or supported CI system is detected.

silverwind commented 1 year ago

I have reverted the dependency and have now effectively replicated this block.

silverwind commented 1 year ago

I think the test failure is related to it detecting the CircleCI environment, will update the fixture.