-
-
Notifications
You must be signed in to change notification settings - Fork 8.4k
ci: Sanitize and get coverage from mpy-cross too. #17997
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
Conversation
There are no new findings, but this ensures the code paths unique to mpy-cross are checked with the sanitizers. Signed-off-by: Jeff Epler <jepler@gmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## master #17997 +/- ##
===========================================
- Coverage 98.38% 87.75% -10.64%
===========================================
Files 171 183 +12
Lines 22296 25149 +2853
===========================================
+ Hits 21937 22070 +133
- Misses 359 3079 +2720 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
c588a2c
to
5e1e39b
Compare
Code size report:
|
Signed-off-by: Jeff Epler <jepler@gmail.com>
5e1e39b
to
4454c9c
Compare
hmm does anyone spot what I messed up about the codecov upload? |
I don't understand how this works. It looks like it's merging results from running |
I had hoped that it would be able to integrate line coverage data from both mpy-cross and micropython in a single report. It does appear to ingest two "gcno" files for sources that appear in both,
but unfortunately it looks like the earlier one is used and the latter one is ignored(?) Locally using "gcovr" I can create merged coverage .. in this case, the line coverage of But if it's not going to work with the codecov website then it's not super useful; and the line coverage increase is only about 150 lines anyway, in my local testing. If we could get it to combine in codecov, AND we added tests that f.e. covered all the emitters, there would be some benefit. As it is, there doesn't seem to be. I'll close this up for now, but might reopen a similar PR someday if I work out how to get codecov to see all the coverage data, and can actually cover any significant additional code. |
An alternative would be to enable the dynamic compiler option on the unix coverage build, and add some mpy-cross behaviour to unix coverage, then use unix coverage to build all tests using all architectures. |
Summary
There are no new findings, but this ensures the code paths unique to mpy-cross are checked with the sanitizers.
I think that new coverage may turn up as well (e.g., MICROPY_DYNAMIC_COMPILER) if I've got the CI rules right.
Testing
I ran the coverage, ubsan, and asan commands in ci.sh locally.
Trade-offs and Alternatives
Not a trade-off per se, but specifically adding mpy-cross tests that would exercise all emitters when built with coverage & sanitizers would potentially have benefits.