test-drive icon indicating copy to clipboard operation
test-drive copied to clipboard

add color to output

Open freevryheid opened this issue 3 years ago • 11 comments
trafficstars

freevryheid avatar Mar 08 '22 13:03 freevryheid

We should add an option to enable/disable the color support. Not all terminals or outputs do support color.

awvwgk avatar Mar 08 '22 13:03 awvwgk

I saw I HackerNews thread about https://no-color.org/ recently. They suggest an informal standard of checking the environment variable NO_COLOR.

Here's what the command-line interface guidelines suggest:

Disable color if your program is not in a terminal or the user requested it. These things should disable colors:

* stdout or stderr is not an interactive terminal (a TTY). It’s best to individually check—if you’re piping stdout to another program, it’s still useful to get colors on stderr.
* The NO_COLOR environment variable is set.
* The TERM environment variable has the value dumb.
* The user passes the option --no-color.
* You may also want to add a MYAPP_NO_COLOR environment variable in case users want to disable color specifically for your program.

ivan-pi avatar Mar 16 '22 14:03 ivan-pi

I'm leaning towards "the user passes the option -no-color", that flags a public logical no_color defined in testdrive.f90. Since this is a dependency in fpm it is counter-intuitive to include checks that fpm already runs to test for color terminals - besides, the user would know their terminal :)

freevryheid avatar Mar 16 '22 17:03 freevryheid

I was thinking instead as an optional input to run_testsuite. Not sure if I'd want finer grained control at the level of individual test suites, or unit tests, neither at thread/image level.

ivan-pi avatar Mar 16 '22 17:03 ivan-pi

I like this PR and agree about having an option to disable colors (env var NO_COLOR seems reasonable).

I wonder about the yellow statuses for unexpected pass and expected fail. Personally I'd expect those to be red (undesired outcome) and green (desired outcome), respectively. In a setting where green and red describe desired and undesired outcomes, to me yellow means some kind of warning. And, there's the issue that the same color is used for two opposite outcomes, thus making the coloring unhelpful. Long story short, I suggest not using the yellow, and using green and red according to the desiredness of the test outcome.

milancurcic avatar Apr 12 '22 14:04 milancurcic

Codecov Report

Attention: Patch coverage is 25.00000% with 12 lines in your changes missing coverage. Please review.

Project coverage is 68.35%. Comparing base (cc59dd7) to head (b3b0037). Report is 17 commits behind head on main.

Files with missing lines Patch % Lines
src/testdrive.F90 25.00% 4 Missing and 8 partials :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #13      +/-   ##
==========================================
- Coverage   69.02%   68.35%   -0.67%     
==========================================
  Files           2        2              
  Lines         481      493      +12     
  Branches      261      269       +8     
==========================================
+ Hits          332      337       +5     
- Misses         23       27       +4     
- Partials      126      129       +3     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Apr 14 '22 15:04 codecov[bot]

I started a project for providing terminal colors in https://github.com/awvwgk/fortty. I don't suggest that we use this project as dependency, but maybe take some inspiration from the terminal_type to conditionally enable color support.

In any case, it seems beneficial to introduce a test_context derived type which carries a shared state between the tests, like the terminal color support, maybe in the future also a timer or a log with all failed tests. This requires some restructuring and maybe a compatibility API to allow adapters of test-drive a graceful switch over (we don't want to break anyones testsuite improving this project).

awvwgk avatar May 25 '22 09:05 awvwgk

What's the status of this PR - what is missing for it to get merged? We are using test-drive in our CI testing and unfortunately it is way harder than necessary to visually find problematic locations in the GitHub Actions logs if everything is the same shade of grey...

sloede avatar Aug 31 '23 07:08 sloede

@freevryheid would you be willing to include the proposed changes in your PR such that it can be merged (even though it's been a year...)

sloede avatar Sep 01 '23 08:09 sloede

@awvwgk can you confirm that the 3 failing jobs (gnu 6, 7, 8) are due to updated ubuntu/macos-latest images (i.e. older GCC versions not available anymore) and we can safely ignore them?

Also, are you OK with ignoring the codecov coverage decrease in this specific case? It seems to me that adding tests to satisfy the coverage here would be overly tedious for little gain.

milancurcic avatar Sep 01 '23 13:09 milancurcic

If we can we should target improving the coverage, but it is not a strict criterium for merging.

Regarding the CI, we should open a new issue on this an fix or remove the respective workflows.

awvwgk avatar Sep 01 '23 13:09 awvwgk