Skip to content
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

Track code coverage metrics #453

Closed
jswrenn opened this issue Oct 3, 2023 · 5 comments
Closed

Track code coverage metrics #453

jswrenn opened this issue Oct 3, 2023 · 5 comments
Assignees
Labels
compatibility-nonbreaking Changes that are (likely to be) non-breaking experience-medium This issue is of medium difficulty, and requires some experience google-20%-project Potential 20% project for a Google employee help wanted Extra attention is needed

Comments

@jswrenn
Copy link
Collaborator

jswrenn commented Oct 3, 2023

As part of upholding our design ethos (#405), we would like to be able to promise that we maintain exceptionally high code coverage. To do this, we need to track code coverage in CI.

Mentorship Instructions

We're unfamiliar with the best-practices of monitoring code coverage in Rust. We are hoping that the contributor to this issue can help us navigate the complexities of applying code coverage metrics to this repo, which contains:

  • multiple packages (namely, zerocopy and zerocopy-derive)
  • unit tests
  • integration tests
  • ui tests

In CI, these tests are run in multi ways (e.g., via miri, kani, and binary) on multiple platforms. Ideally, our coverage metrics captures as many of these sources of coverage as possible.

Unresolved Questions

  • How should we report these metrics on PRs?
@jswrenn jswrenn added Hacktoberfest help wanted Extra attention is needed experience-medium This issue is of medium difficulty, and requires some experience labels Oct 3, 2023
@joshlf joshlf added compatibility-nonbreaking Changes that are (likely to be) non-breaking and removed Hacktoberfest labels Oct 23, 2023
@tommy-gilligan
Copy link
Contributor

I'm keen to take this. I've used tarpaulin a bit before.

How should we report these metrics on PRs?

My go-to here would be Github Actions artifacts.

@joshlf
Copy link
Member

joshlf commented Feb 15, 2024

That'd be awesome; it's all yours! I've assigned it to you.

Github Actions artifacts sounds reasonable for reporting the metrics.

@tommy-gilligan
Copy link
Contributor

Maybe I spoke too soon recommending Tarpaulin. Nevertheless, here is an example build using Tarpaulin https://github.com/tommy-gilligan/zerocopy/actions/runs/7986802695 .

I think I'll also try cargo-llvm-cov

Regardless of which tool is used, would it be useful to aggregate across different axes of ci.yml matrix? What kind of aggregations would be useful?

PS. I'm finding navigating workflow jobs a bit tricky because the (necessarily) long names get truncated. I'm curious if you have a handy way of accessing the same information in a more accessible way?

Screenshot 2024-02-21 at 8 38 40 pm

@joshlf
Copy link
Member

joshlf commented Feb 21, 2024

Maybe I spoke too soon recommending Tarpaulin. Nevertheless, here is an example build using Tarpaulin https://github.com/tommy-gilligan/zerocopy/actions/runs/7986802695 .

Where is the coverage information reported there? I don't see it.

@jswrenn is the maintainer of itertools; here's their configuration: https://github.com/rust-itertools/itertools/blob/master/.github/workflows/coverage.yml

I don't personally have experience with this, so I can't promise that some or any of it will be applicable here, but it might be a useful place to start 🤷‍♂️

Regardless of which tool is used, would it be useful to aggregate across different axes of ci.yml matrix? What kind of aggregations would be useful?

Here's a rough outline of what we'd want in order of priority (highest priority to lowest). Let's start with the first and then we can try for the lower ones if it turns out to be feasible.

  • Report coverage only from a particular matrix combination (in particular, Linux x86_64 with --all-features on the nightly toolchain)
  • Report coverage from all matrix combinations which have the Linux x86_64 target
  • Report coverage from all matrix combinations

For the first of these, there's no need to solve the "do we report separately or aggregate" question since there's just one combination being reported.

PS. I'm finding navigating workflow jobs a bit tricky because the (necessarily) long names get truncated. I'm curious if you have a handy way of accessing the same information in a more accessible way?

Unfortunately none that I'm aware of, but I also haven't tried hard to find one.

@joshlf joshlf added the google-20%-project Potential 20% project for a Google employee label May 7, 2024
joshlf added a commit that referenced this issue May 16, 2024
joshlf added a commit that referenced this issue May 16, 2024
joshlf added a commit that referenced this issue May 16, 2024
joshlf added a commit that referenced this issue May 16, 2024
joshlf added a commit that referenced this issue May 16, 2024
github-merge-queue bot pushed a commit that referenced this issue May 17, 2024
* [ci] Add code coverage reporting for PRs and main

Makes progress on #453

* [WIP] Inline into `ci.yml`
@joshlf
Copy link
Member

joshlf commented May 19, 2024

I added basic coverage in #1274. There are still a lot of improvements that could be made, especially support for zerocopy-derive.

@joshlf joshlf closed this as completed Nov 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compatibility-nonbreaking Changes that are (likely to be) non-breaking experience-medium This issue is of medium difficulty, and requires some experience google-20%-project Potential 20% project for a Google employee help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants