-
Notifications
You must be signed in to change notification settings - Fork 277
Cleanup coverage CI job #6606
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
Cleanup coverage CI job #6606
Conversation
cmake --target coverage already takes care of running lcov. We ended up doing exactly the same work twice, just logging to different output files. This should save about 90 seconds of CI job execution time.
Codecov Report
@@ Coverage Diff @@
## develop #6606 +/- ##
===========================================
+ Coverage 76.49% 76.62% +0.12%
===========================================
Files 1578 1578
Lines 181464 181180 -284
===========================================
+ Hits 138816 138833 +17
+ Misses 42648 42347 -301
Continue to review full report at Codecov.
|
Code marked "UNREACHABLE" just artificially reduces our coverage numbers. Tell lcov about this (it would by default exclude lines containing "LCOV_EXCL_LINE" only). This changes the coverage reported at present by +0.12 percentage points.
0935b7a
to
8aa6be8
Compare
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.
A clear improvement on what we have, thanks!
@@ -700,7 +700,9 @@ jobs: | |||
- name: Print ccache stats | |||
run: ccache -s | |||
- name: Run CTest and collect coverage statistics | |||
run: cmake --build build --target coverage -- -j2 | |||
run: | | |||
echo "lcov_excl_line = UNREACHABLE" > ~/.lcovrc |
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.
Good catch! I'm not massively familiar with lcov (to my shame) so apologies if these are dumb questions... but:
- Does this exclude only lines containing UNREACHABLE, or also any other lines following that in the same block/trace?
- Do we want/need to do something similar for branch coverage with
lcov_excl_br_line
?
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.
Yes, it only affects exactly that one line that has "UNREACHABLE." There are ways to mark entire blocks (details in https://linux.die.net/man/1/geninfo), which we could consider if deemed useful.
I don't think we measure branch coverage at this time, but if needed we could certainly extend the configuration.
This is an attempt to remove any code marked "UNREACHABLE" from the
coverage report. (Such code just artificially reduces our coverage
numbers.)
While at it I also suspected that we might be running lcov twice.
See individual commit messages for numeric data points.