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.

Sign up for GitHub

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

Jump to bottom

Problem matchers do not work on colored output #2341

Open
ian-h-chamberlain opened this issue Dec 25, 2022 · 1 comment · May be fixed by #2430
Open

Problem matchers do not work on colored output #2341

ian-h-chamberlain opened this issue Dec 25, 2022 · 1 comment · May be fixed by #2430
Labels
bug Something isn't working keep Label can be added as soon as we are sure the work on the issue is necessary

Comments

@ian-h-chamberlain
Copy link

ian-h-chamberlain commented Dec 25, 2022 •
edited
Loading

Describe the bug
Color codes appear to affect regex matching of problem matchers

To Reproduce
Steps to reproduce the behavior:

  1. Create a problem matcher that matches plaintext, e.g. "regexp": "Hello world"
  2. Run an action that produces color output, for example: bash -c "printf '\e[31mHello\e[0m world!\n'"
  3. Observe that no match appears in the actions output or inline on a PR "files" tab.

As a more real-life example, I have created a minimal reproduction with colored output and missing the expected annotations:
https://github.com/ian-h-chamberlain/actions-color-repro/actions/runs/3777593125

However, when I updated the workflow to disable color output, annotations appear as expected:
ian-h-chamberlain/actions-color-repro#1
https://github.com/ian-h-chamberlain/actions-color-repro/actions/runs/3777594485

Expected behavior
Most (all?) are written against plain (un-colored) text, but it would be nice to have colorized output in the action logs while still supporting colored output.

This document indicates that color codes should be stripped before applying matchers.

Runner Version and Platform

Version of your runner? Current runner version: '2.299.1'

OS of the machine running the runner? ubuntu-latest

What's not working?

See the "expected behavior" above.

Other notes

I believe this logic is incomplete for stripping ANSI color codes:

runner/src/Runner.Worker/Handlers/OutputManager.cs

Line 19 in a1244d2

private static readonly Regex _colorCodeRegex = new(@"\x0033\[[0-9;]*m?", RegexOptions.Compiled | RegexOptions.CultureInvariant);

runner/src/Runner.Worker/Handlers/OutputManager.cs

Line 100 in a1244d2

var stripped = line.Contains(_colorCodePrefix) ? _colorCodeRegex.Replace(line, string.Empty) : line;

Here is a sample logs from a more realistic run which I believe show a wider range of ANSI control codes that might be encountered by tools that use color: 6_Run clippy linting.txt
@ian-h-chamberlain ian-h-chamberlain added the bug Something isn't working label Dec 25, 2022
ian-h-chamberlain added a commit to corewa-rs/corewars that referenced this issue Dec 25, 2022
@ian-h-chamberlain
Try disabling color output
14b230d 
Workaround for actions/runner#2341 I guess...
Color output would be nice but annotations seem nicer.
@ruvceskistefan ruvceskistefan added the keep Label can be added as soon as we are sure the work on the issue is necessary label Dec 26, 2022
@ian-h-chamberlain ian-h-chamberlain linked a pull request Feb 12, 2023 that will close this issue
Open
@ian-h-chamberlain
Copy link
Author

@ruvceskistefan since you triaged this to add the keep label, maybe you know who might be able to review the PR to fix this (#2430) ? It's a fairly small change and this would be a nice quality of life improvement for the runner. Thanks!

jonasbb added a commit to actions-rust-lang/setup-rust-toolchain that referenced this issue Mar 17, 2023
@jonasbb
Add new problem matcher that is aware of color codes
12a4c2d 
The action runner currently fails to strip color codes from the output.
This means that many matchers currectly do not work.

actions/runner#2341
actions/runner#2430

The new matcher is copied from kaj/rsass which is MIT licensed.

https://github.com/kaj/rsass/blob/3e5d6c0600324fe27b0a0257d3befd4bb345fedb/.github/workflows/rust-problem-matcher.json
@jonasbb jonasbb mentioned this issue Mar 17, 2023
Merged
@chdsbd chdsbd mentioned this issue Apr 3, 2023
Merged
kodiakhq bot pushed a commit to chdsbd/foodieyak that referenced this issue Apr 3, 2023
@chdsbd
add problem matcher for vitest (#103)
aeb1962 
We don't support colored output because of a bug in GitHub Actions: actions/runner#2341
jmarrec added a commit to jmarrec/test-Github-Actions that referenced this issue Jun 20, 2023
@jmarrec
Try to retain some colors while still having the problem matcher matc…
affbccd 
…h (cf actions/runner#2341 - problem matcher do not work on colored output)
@henryiii henryiii mentioned this issue Jul 13, 2023
Open
toofar added a commit to qutebrowser/qutebrowser that referenced this issue Nov 19, 2023
@toofar
Update pytest summary problem matcher for colored output
116d021 
We think that at some point pytest added color codes to the summary
output which broke these matcher regex. It looks like there is an issue
about stripping color codes here: actions/runner#2341

I've added wildcard (or no-space) elements instead of trying to match
the color codes exactly (eg `\033\[31m` etc) because 1) that's not very
readable 2) I was having trouble getting that to work with egrep.

The goal is to match the target strings but not to match then later in
the line as part of a test log or whatever. For the '= short test
summary =' that should be pretty unique. For the ERROR|FAILED I reckon
that could be a bit more common. I'm just matching with any mount of
non-space characters because I reckon a space will crop up pretty early
in any line where ERROR isn't the first work.

I think this new matching will only apply to new or rebased PRs after it
is landed.
toofar added a commit to qutebrowser/qutebrowser that referenced this issue Nov 19, 2023
@toofar
Update pytest summary problem matcher for colored output
27c5cc8 
We think that at some point pytest added color codes to the summary
output which broke these matcher regex. It looks like there is an issue
about stripping color codes here: actions/runner#2341

I've added wildcard (or no-space) elements instead of trying to match
the color codes exactly (eg `\033\[31m` etc) because 1) that's not very
readable 2) I was having trouble getting that to work with egrep.

The goal is to match the target strings but not to match then later in
the line as part of a test log or whatever. For the '= short test
summary =' that should be pretty unique. For the ERROR|FAILED I reckon
that could be a bit more common. I'm just matching with any mount of
non-space characters because I reckon a space will crop up pretty early
in any line where ERROR isn't the first work.

I think this new matching will only apply to new or rebased PRs after it
is landed.

ref: #5390 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working keep Label can be added as soon as we are sure the work on the issue is necessary
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix color code detection in OutputManager ian-h-chamberlain/runner
2 participants
@ian-h-chamberlain @ruvceskistefan