gh-94759: Collect C-level coverage using llvm-cov by mdboom · Pull Request #94760 · python/cpython · GitHub

This adds a Github Action to collect C-level coverage and publish it to a Github Pages repository.

You can see an example coverage output.

There are two admin things that would need to happen for this to work:

Create a new repository in the python organization for the results, e.g. cpython-c-coverage
Create an organization-scoped token to publish to this repository and use it from this action

Issue: Publish C-level coverage of the CPython test suite #94759

This will be great to have! Publishing to github pages is interesting and is as good a place to start as any.

It is unfortunate that GitHub doesn't support rendering coverage metadata on top of their existing source tree view and within PR code views.

Does every run clobber the previous run's report? What about CI runs on PRs and CI runs from release branches? Can this be configured to commit the coverage results into a branch in the repo matching the reponame+branchname? Or do branches not render in gh-pages? in which case it'd need to be subdirectories in the repo.

Currently, this clobbers the previous run. That is easily changed with a flag, but then we could run into Github repository size limits (each result is around 100MB of HTML). It's probably possible to keep the last N commits, but the tool I'm using to publish doesn't support that directly.

This is currently configured to just run on the main branch once a day. We could do multiple branches that publish to subdirectories if you think there's a good use case for that. (Github pages only publishes a single branch, but subdirectories would work).

If we would get some funding to run a persistent VM with public hostname from ... let's say Azure, then it would be trivial to create a buildbot worker and serve the LCOV results from HTTP server. They are static HTTP and JS files on the file system after all. Just saying :)

I don't mind main branch only and once a day. Anyone working on coverage is presumably doing their own local coverage runs while creating PRs.

Regarding a buildbot configured to host the results, while I could simply set one up it'd probably make more sense for mdboom to do that and be an admin given who's driving this work. :)

Yeah, I'd be happy to admin that if it comes to it. I think it's fine to go with the Github Action here as an MVP, and if the amount of history or frequency of runs isn't good enough, we can revisit migrating to our own VM down the road.

What is the benefit to run the suit daily? If the report is used as a guide for additions into the tests, a weekly run seems to be enough.

What is the benefit to run the suite daily? If the report is used as a guide for additions into the tests, a weekly run seems to be enough.

If it's daily then it's up-to-date more than the weekly. More up-to-date is better than less up-to-date 😉. There's also the benefit that people will have a better idea of what to work on in terms of coverage than not as someone may put in a lot of work for something that's already been fixed earlier in the week.

Not to say that a weekly coverage run isn't good as well, but daily is just better if we can make it happen.

Is there any particular reasons why you are using clang and custom instructions for LCOV instead of make coverage-report? The latter uses gcc and lcov tool.

Is there any particular reasons why you are using clang and custom instructions for LCOV instead of make coverage-report? The latter uses gcc and lcov tool.

Which one produces more useful output? (I really don't care which we use even though everything I do at work is clang/llvm based - I just want line and branch coverage data)

@tiran the idea of using clang comes from faster-cpython/ideas#426 where I believe branch coverage was considered the big win.

Maybe coverage-report should get updated to use LLVM? Or make it configurable for either clang or gcc?

GCC and LCOV can do branch coverge, too. I'll have a branch ready by tomorrow.

I would welcome to have LLVM coverage support in Makefile, too.

Is there any particular reasons why you are using clang and custom instructions for LCOV instead of make coverage-report? The latter uses gcc and lcov tool.

It was an attempt to get branch coverage, not realizing lcov could do it. I still think llvm-cov displays branches within subexpressions in a more readable way, however. I don't know if using gcov for collection + llvm-cov for output would give us the best of both worlds. (Using gcc for the collection would be preferable, probably, since that's what we test with on Linux).

I don't know if using gcov for collection + llvm-cov for output would give us the best of both worlds.

It doesn't look like this is possible. There is an llvm-cov gcov command that takes the gcc coverage data as input and outputs human-readable text file reports. But I can't find a way to use gcc coverage data and output the nice llvm-cov html reports.

@brettcannon wrote:

Maybe coverage-report should get updated to use LLVM? Or make it configurable for either clang or gcc?

@tiran wrote:

I would welcome to have LLVM coverage support in Makefile, too.

I think that's a good idea, but a fair chunk of fiddly work. The Makefile doesn't have any direct support for building with clang on Linux. It works if you set CC, obviously, but you have to do that prior to re-running configure. So I don't think it's possible to just have make coverage-report-llvm work (since the Makefile can't call configure). We could, probably, make a task that errors out if clang isn't being used.

It works if you set CC, obviously, but you have to do that prior to re-running configure

Can you have the coverage-report target do the right thing based on what CC was set to?

I just refactored this so there is a new Makefile target coverage-report-llvm that uses llvm-cov to generate a coverage report -- and then the Github Action just uses that.

Can you have the coverage-report target do the right thing based on what CC was set to?

I hadn't thought of this -- it's probably doable. Given that you have to completely reconfigure to switch between each "mode" that probably makes sense.

I've updated this to automatically choose lcov or llvm-cov depending on the compiler in use.

@erlend-aasland wrote:

Also: please don't force push :) The GitHub UI don't play well with force-pushes. Using git merge --no-ff main makes for a better review experience.

I tried this and unfortunately it created a bunch of co-authored commits for everything that had happened in the meantime and then requested review from ~20 codeowners. I went back to rebasing/force pushing for a cleaner review of the actually relevant commits. Going to read the relevant parts of the devguide for next time...

Co-authored-by: Christian Heimes <christian@python.org>

Are there any additional comments on this?

Create a new repository in the python organization for the results, e.g. cpython-c-coverage

There's a process for this now.
I don't think this repo needs to start under a personal GitHub account -- moving the config and creating a new token would be more trouble than it's worth. That leaves asking the SC as the next step, see here for example: python/steering-council#236

Adding LLVM support to coverage-report would work better as a separate PR, but I won't block on that.

This PR is stale because it has been open for 30 days with no activity.

mdboom requested a review from gpshead as a code owner July 11, 2022 19:31

bedevere-bot added the awaiting review label Jul 11, 2022

mdboom marked this pull request as draft July 11, 2022 19:32

gpshead reviewed Jul 11, 2022

View reviewed changes

gpshead requested review from brettcannon and pablogsal July 11, 2022 21:51

mdboom closed this Jul 12, 2022

mdboom reopened this Jul 12, 2022

mdboom force-pushed the c-level-coverage branch from e93f9f7 to 3d05d90 Compare July 13, 2022 18:11

mdboom requested review from gpshead and tiran July 13, 2022 18:57

mdboom force-pushed the c-level-coverage branch 2 times, most recently from eb15335 to 58e38af Compare July 18, 2022 14:33

tiran reviewed Jul 19, 2022

View reviewed changes

Comment thread configure.ac Outdated

mdboom force-pushed the c-level-coverage branch from 31e22cd to 6ba9b68 Compare July 28, 2022 19:27

mdboom force-pushed the c-level-coverage branch from 6ba9b68 to 0001577 Compare August 5, 2022 19:45

mdboom marked this pull request as ready for review August 5, 2022 19:46

mdboom requested a review from tiran August 5, 2022 19:46

mdboom force-pushed the c-level-coverage branch from 9863146 to 87df0f2 Compare August 22, 2022 12:59

mdboom requested a review from iritkatriel as a code owner August 24, 2022 17:56

mdboom force-pushed the c-level-coverage branch from 08812bc to c9b23a1 Compare August 24, 2022 17:59

mdboom requested review from erlend-aasland and ezio-melotti August 24, 2022 18:02

erlend-aasland removed request for a team, berkerpeksag, brandtbucher, iritkatriel, markshannon, rhettinger and vsajip August 24, 2022 19:25

This was referenced Aug 24, 2022

Update developer guide about C code coverage python/devguide#940

Open

On macOS, Configure script assumes that "gcc" is an alias for "clang", even if it really is GCC #96249

Open

mdboom and others added 8 commits October 31, 2022 11:38

Collect C-level coverage using llvm-cov

e2eed08

Refactor this as a Makefile target

776e423

Automatically select the coverage approach based on the compiler

1ab7870

Update configure.ac

80f0039

Co-authored-by: Christian Heimes <christian@python.org>

Fix lcov coverage

dfa3fcb

Remove unnecessary quotes

50ba4f4

Move more logic into configure

84f042a

Cleanup after recent changes

ceb32ee

mdboom force-pushed the c-level-coverage branch from c9b23a1 to ceb32ee Compare October 31, 2022 15:38

bedevere-bot mentioned this pull request Oct 31, 2022

Publish C-level coverage of the CPython test suite #94759

Open

brettcannon removed their request for review November 25, 2022 22:20

encukou mentioned this pull request May 20, 2024

Metabug: Improving C-level coverage #94808

Open

github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 11, 2026

gh-94759: Collect C-level coverage using llvm-cov by mdboom · Pull Request #94760 · python/cpython · GitHub

Uh oh!

Uh oh!

gh-94759: Collect C-level coverage using llvm-cov - #94760

gh-94759: Collect C-level coverage using llvm-cov#94760mdboom wants to merge 8 commits intopython:mainpython/cpython:mainfrom mdboom:c-level-coveragemdboom/cpython:c-level-coverageCopy head branch name to clipboard

Conversation

mdboom commented Jul 11, 2022 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gpshead left a comment

Choose a reason for hiding this comment

Uh oh!

gpshead Jul 11, 2022

Choose a reason for hiding this comment

Uh oh!

mdboom Jul 12, 2022

Choose a reason for hiding this comment

Uh oh!

tiran Jul 12, 2022

Choose a reason for hiding this comment

Uh oh!

gpshead Jul 12, 2022

Choose a reason for hiding this comment

Uh oh!

mdboom Jul 13, 2022

Choose a reason for hiding this comment

Uh oh!

arhadthedev commented Jul 12, 2022

Uh oh!

brettcannon commented Jul 12, 2022

Uh oh!

tiran commented Jul 12, 2022

Uh oh!

gpshead commented Jul 12, 2022

Uh oh!

brettcannon commented Jul 12, 2022

Uh oh!

tiran commented Jul 12, 2022

Uh oh!

mdboom commented Jul 12, 2022

Uh oh!

mdboom commented Jul 13, 2022

Uh oh!

brettcannon commented Jul 13, 2022

Uh oh!

mdboom commented Jul 13, 2022

Uh oh!

mdboom commented Jul 13, 2022

Uh oh!

Uh oh!

mdboom commented Aug 24, 2022

Uh oh!

mdboom commented Oct 31, 2022

Uh oh!

encukou commented Mar 19, 2024

Uh oh!

github-actions Bot commented Apr 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

gh-94759: Collect C-level coverage using llvm-cov#94760
mdboom wants to merge 8 commits into
python:mainfrom
mdboom:c-level-coverage

mdboom commented Jul 11, 2022 •
edited by bedevere-bot
Loading