test-drive
test-drive copied to clipboard
add color to output
We should add an option to enable/disable the color support. Not all terminals or outputs do support color.
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.
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 :)
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.
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.
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.
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).
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...
@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...)
@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.
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.