-
Notifications
You must be signed in to change notification settings - Fork 50
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
Publish C-level coverage for the test suite #426
Comments
Here's the old coverage GH Action for CPython that still runs on older branches: https://github.com/python/cpython/actions/workflows/coverage.yml . |
Looking at this, is my understanding correct that the upload token for Codecov.io is broken and therefore we probably haven't been publishing anything from this for a while? I'm not convinced that Codecov.io provides a ton of value anyway -- I was planning on just publishing what |
Probably.
It was honestly just the easiest solution at the time. |
This PR (on my own fork) produced this output. |
An alternative (or supplement) to |
@erlend-aasland Can you share your commands/workflow? |
Sure! For the record, I'm on macOS, using the XCode supplied toolchain.
py-cov.sh script#!/bin/sh
set -ve
MACOS_VER="12.2"
PY_MAJOR="3"
PY_MINOR="12"
PREFIX="build/lib.macosx-${MACOS_VER}-x86_64-${PY_MAJOR}.${PY_MINOR}-pydebug/"
SUFFIX="cpython-${PY_MAJOR}${PY_MINOR}d-darwin.so"
SQLITE_LIB="${PREFIX}/_sqlite3.${SUFFIX}"
BIN=$SQLITE_LIB # python.exe, $SQLITE_LIB, etc.
PROFRAW=default.profraw
PROFDATA=default.profdata
SRC=Modules/_sqlite
REPORT=llvm-report
IGNORE="(clinic|Include)"
llvm-profdata merge \
-sparse $PROFRAW \
-o $PROFDATA
llvm-cov report \
-ignore-filename-regex="$IGNORE" \
-instr-profile=$PROFDATA \
$BIN \
$SRC
llvm-cov show \
-format=html \
-output-dir=$REPORT \
-ignore-filename-regex="$IGNORE" \
-instr-profile=$PROFDATA \
$BIN \
$SRC
rm -f $PROFDATA llvm-cov report, sample output
See llvm-report.zip for sample HTML output. It's pretty crude, but it works for me :) |
@erlend-aasland: This is a fantastic suggestion. The branch results are pretty helpful, and I think they may provide some performance hints to the Faster CPython project. (It's a shame it doesn't collect branch stats for switch statements, but the I also experimented with In short, this seems superior to what gcov/lcov provides, and it definitely seems like we should use this instead if possible. I've put the results from each system up here: Notes about how these were generatedI did this on Debian bookworm and needed to apt install `clang-13` and `llvm-13`.
llvm-cov breaks when multiple shared objects try to write to the same object file, so I needed to use the trick in this comment to have an output file per shared object.
|
I've got a branch lying around where I've added llvm-cov support to configure/make. I'll see if I can restore it and rebase against main. |
Sure, that would be very helpful. |
FYI lcov can do branch coverage. It is just disabled by default. I have working code that generated branch coverage with GCC and LCOV. I would like to see llvm-cov support in Makefile, too. |
There is also https://github.com/gcovr/gcovr . It's a replacement for gcov/lcov/genhtml and written in our favorite language. |
Thanks for pointing out Yeah, I agree on As for gcovr, I had looked at it, but it doesn't seem like anything it offers is really relevant and I thought it better to not add another moving part. But if there's something specific it does that lcov doesn't let me know. |
|
Thanks for sharing those. Based on this, I think my entirely subjective thoughts are: |
I agree with you. The llvm-cov output looks more modern and presents more information. |
I'm not sure if that is relevant/useful here, but coverage.py recently gained an option to export to lcov format, so you can merge C and Python coverage into one report. Example for a Python/C extension: https://gnome.pages.gitlab.gnome.org/pygobject/gi/index.html coverage.py is prettier of course, but having everything in one place is nice too. |
@lazka: That's cool to know about. I was planning on doing Python coverage once this landed (C coverage is a bit more urgent to my team at the moment), and it might make sense to do something like this. |
Should be closed as "not planned" instead? |
One thing we've learned from the frame.setlineno set of issues is that the C-level coverage of the test suite isn't perfect.
If we started publishing the C level coverage of the test suite somewhere that would help us to file bugs to proactively improve coverage of the test suite.
I've seen projects that do this on pull requests -- there is usually so much noise in the results that IME it's not terribly useful. I think just a nightly (or even weekly) publishing of coverage results on
main
would go a long way, though.It should be possible to do this with Github Actions.
The text was updated successfully, but these errors were encountered: