Update coverage bot to Ubuntu 24.04 x86 bot #735
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #735 +/- ##
=======================================
Coverage 79.83% 79.83%
=======================================
Files 9 9
Lines 3963 3963
=======================================
Hits 3164 3164
Misses 799 799 🚀 New features to boost your workflow:
|
mcbarton
force-pushed
the
Coverage-bot-24.04
branch
from
678b38e to
c613b66
Compare
mcbarton
force-pushed
the
Coverage-bot-24.04
branch
from
ad51adf to
184d22a
Compare
mcbarton
changed the title
| lcov --remove coverage.info '${{ github.workspace }}/build/*' --ignore-errors unused --output-file coverage.info | ||
| # output coverage data for debugging (optional) | ||
| lcov --list coverage.info | ||
| rm -rf ./build/ |
There was a problem hiding this comment.
This is a bit of a fudge but without it codecov will ignore the coverage report the ci makes, and makes its own (which will include the unittests folder in the coverage report)
| vers="${CC#*-}" | ||
| lcov --directory build/ --capture --output-file coverage.info --gcov-tool /usr/bin/gcov-${vers} | ||
| lcov --remove coverage.info '/usr/*' "${HOME}"'/.cache/*' ${{ github.workspace }}'/llvm-project/*' ${{ github.workspace }}'/unittests/*' --output-file coverage.info | ||
| lcov --directory build/ --capture --output-file coverage.info --gcov-tool /usr/bin/gcov-${vers} --ignore-errors mismatch |
There was a problem hiding this comment.
Why do we have --ignore-errors mismatch flag?
The lcov --remove is fine.
There was a problem hiding this comment.
Without these changes the ci crashes when you upgrade, since you get a newer lcov version is more restrictive in what it will allow you to get away with. All the changes here were found to be necessary.
There was a problem hiding this comment.
Looking at the docs https://manpages.debian.org/unstable/lcov/lcov.1.en.html#mismatch:
I feel we should try and avoid it. Do we have a CI run without that flag?
There was a problem hiding this comment.
Avoiding this flag would mean dealing with all the mismatch warnings we currently have in the ci now (see https://github.com/compiler-research/CppInterOp/actions/runs/18694188958/job/53307234747?pr=735#step:13:52). If you see all the warnings they are in the test files, and to me dealing with them doesn't feel like it gives any benefit given they aren't effecting the end result, and they can easily be ignored with a flag which lcov itself suggested.
There was a problem hiding this comment.
That run still passes. Just gives you a few warnings. I believe that is fine. We should try and fix those warnings instead of ignoring them. Will approve it if you can remove --ignore-errors flag. We can fix the warnings later.
There was a problem hiding this comment.
That run passes in the link I gave above because --ignore-errors mismatch is there 😅 . It will error without that flag. We cannot fix them later. If we remove that flag then they would need to be fixed in this PR.
There was a problem hiding this comment.
Without that flag is treats all those warnings as errors. --ignore-errors mismatch is literally telling lcov to not treat the mismatches as errors, but warnings instead.
|
Thanks @Vipul-Cariappa . Opened up the issue here #737 . I will try and tackle this issue soon, so we can remove the flag. |
2 participants
Description
Please include a summary of changes, motivation and context for this PR.
Fixes # (issue)
Type of change
Please tick all options which are relevant.
Testing
Please describe the test(s) that you added and ran to verify your changes.
Checklist