Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Colorized Output in GitHub Actions #2370

Closed
NickCraver opened this issue Mar 22, 2020 · 13 comments
Closed

Support Colorized Output in GitHub Actions #2370

NickCraver opened this issue Mar 22, 2020 · 13 comments
Milestone

Comments

@NickCraver
Copy link
Member

This may be a feature request more than an issue, but I'm out of my depth and having trouble finding where the breakdown occurs. Asking the experts! In response to a Twitter thread yesterday, I got some awesome feedback. One key piece of information is that GitHub Actions does support colorized console output (example). But I've never seen it in our dotnet test runs.

Here's the monochromatic experience today:
monochrome GitHub Actions

We're using xUnit which as far as I can tell only disables colorized output if -nocolor is passed to the console runner. That's not the case, so I'm confused where in the pipe we're dropping having better output. It will need a custom reporter (or at least detection) when GitHub Actions has a more full-fledged test results UI, but for now: colorized output would be a huge time saver and just generally make it a better experience.

Perhaps the fact that it's wrapped in the GitHub Actions runner at all is the issue? There is #1762 related to being wrapped in another process - a result of IsOutputRedirected-style behavior breaking all? If it's detection-based, it looks like the GITHUB_ACTIONS environmental variable is how it's done elsewhere (example PR.

To get more info, I tried the /clp:ForceConsoleColor workaround for dotnet build (also currently monochrome in GitHub Actions). This did not work (example run here). We'd expect "Build Succeeded." to be green as it is locally:
local build success
...but it's monochrome:
Actions build success

So...yeah I'm not sure what's breaking down where and maybe it's much more global that dotnet test. I just trying to even find where we can fix this. Is dotnet test eating the colorization? Or am I missing it and xUnit isn't even trying to output it from a switch I've missed? Is is all dotnet * with Actions due to wrapping? Happy to close this issue and file it in the right place, and happy to assist in PR-ing towards a fix. Thanks for your time!

cc @bradwilson @clairernovotny @ethomson @livarcocc

@bradwilson
Copy link

xUnit.net does not attempt any colorization here. Our messages are passed to the VSTest API with just text and a level. All colorization in dotnet test is owned by VSTest.

@NickCraver
Copy link
Member Author

Looking further into this - is the issue more root level? That we are writing colors all the way down to System.ConsolePal? Here's the Unix version in play for my GitHub Actions example, specifically this area which seems like a global disabling mechanism:

private static void WriteSetColorString(bool foreground, ConsoleColor color)
{
    // Changing the color involves writing an ANSI character sequence out to the output stream.
    // We only want to do this if we know that sequence will be interpreted by the output.
    // rather than simply displayed visibly.
    if (Console.IsOutputRedirected)
        return;

I'm not sure how GitHub Actions works here but my assumption is that Console.IsOutputRedirected == true in the scenario.

/cc @deanward81 - thanks for the pointer!

@Tyrrrz
Copy link

Tyrrrz commented Mar 22, 2020

I made a small proof of concept using GitHub Actions commands. This reports test failures to the GH Actions platform, letting you see them in the Annotations view and navigate to the exact/file that triggered it. This is not exactly the solution to "support colorized output" but may solve the underlying issue that GitHub Actions doesn't show the list of failed tests nicely.

https://github.com/Tyrrrz/GitHubActionsTestLogger

https://github.com/Tyrrrz/GitHubActionsTestLogger/runs/525594126?check_suite_focus=true

image

image

Good enough?

@nohwnd nohwnd added this to the 16.7.0 milestone Mar 23, 2020
@nohwnd
Copy link
Member

nohwnd commented Mar 23, 2020

Adding this for consideration in our 16.7.0 milestone. If anyone wants to do the research and PR this then even better. Having full colloring would definitely be nice.

@ejsmith
Copy link

ejsmith commented Mar 23, 2020

I'm curious, @NickCraver does color work on AppVeyor builds?

@ejsmith
Copy link

ejsmith commented Mar 23, 2020

Here is how the NodeJS world is detecting color support: https://github.com/chalk/supports-color/blob/master/index.js

@ejsmith
Copy link

ejsmith commented Mar 23, 2020

Also, here is the link to the current source location for the color check after the dotnet repos were merged. https://github.com/dotnet/runtime/blob/4f9ae42d861fcb4be2fcd5d3d55d5f227d30e723/src/libraries/System.Console/src/System/ConsolePal.Unix.cs#L791

@ejsmith
Copy link

ejsmith commented Mar 23, 2020

I'm curious, @NickCraver does color work on AppVeyor builds?

I checked and AppVeyor does not work either. So this is a general usability issue with .NET Core. Someone could make the world a lot better place by submitting a PR for this. I think it might be over my head, but I am going to look into it.

@Tyrrrz
Copy link

Tyrrrz commented Mar 23, 2020

I think introducing heuristics similar to Chalk's is quite dangerous as they will become maintenance hell and will effectively turn dotnet into a bottleneck when new CIs are introduced/need to be added.

It makes more sense instead to add an environment variable, e.g. DOTNET_FORCE_CONSOLE_COLOR=1 or something that would cause ConsolePal to short-circuit and ignore the check for Console.IsOutputRedirected. Then the different CI providers or volunteers can ensure this environment variable is set on the agent. For example, with GitHub Actions, it could be set as part of the setup-dotnet action, similar to this.

Also, it sounds like this issue should be moved to a different repo?

@ejsmith
Copy link

ejsmith commented Mar 23, 2020

@Tyrrrz I kind of agree with you, but at the same time it would probably take a big effort to get all CI services to agree to and add some environment variable. I agree that this whole thing should be controlled by an environment variable, but I just don't see it being that big of an issue to try and give it a smart default by checking for known environments.

@ejsmith
Copy link

ejsmith commented Mar 23, 2020

So I was looking at how Deno implements this and they basically just assume color and look for a NO_COLOR environment variable to disable color. I looked and apparently this is somewhat of a standard https://no-color.org/

@ejsmith
Copy link

ejsmith commented Mar 23, 2020

I think we can close this issue as I have created a new issue in the correct repo here: dotnet/runtime#33980

@nohwnd
Copy link
Member

nohwnd commented Mar 24, 2020

@ejsmith great :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants