-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Use console reporter's totals summary for compact reporter #2554
Use console reporter's totals summary for compact reporter #2554
Conversation
Codecov Report
@@ Coverage Diff @@
## devel #2554 +/- ##
==========================================
+ Coverage 91.15% 91.30% +0.15%
==========================================
Files 185 185
Lines 7614 7596 -18
==========================================
- Hits 6940 6935 -5
+ Misses 674 661 -13 |
std::size_t row, | ||
std::ostream& stream, | ||
ColourImpl& colour ) { | ||
for ( auto col : cols ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to be a copy? From a quick glance I would say no, the code does not modify the SummaryColumn
here.
(I know it is like this in the original code, but I am looking through it again here and hoo boy, it is a mess)
Looks good, with two drive-by fixes since you are already touching that code. Honestly you can ignore them if you want 🤷 |
Sure, no problem. I did a refactoring pass as a fixup commit, will squash once approved. Speaking of messes, I originally planned to do a larger refactoring of how |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Yeah, shouldfail/mayfail is also a "fun" mess. It is what happens when the code is evolved over time for 10+ years 😃 |
This changes the compact reporter's summary of test run totals to use the same format as the console reporter. This means that while output is no longer on a single line (two instead), it now includes totals for `failedButOk` test cases and assertions, which were previously missing.
0dea420
to
2e66a84
Compare
This changes the compact reporter's summary of test run totals to use the same format as the console reporter. This means that while output is no longer on a single line (two instead), it now includes totals for
failedButOk
test cases and assertions, which were previously missing.Fixes #878.