Skip to content

feat: add coverage report generation with cargo-llvm-cov#1301

Open
dblnz wants to merge 1 commit intohyperlight-dev:mainfrom
dblnz:rally/1190-generate-coverage-reports
Open

feat: add coverage report generation with cargo-llvm-cov#1301
dblnz wants to merge 1 commit intohyperlight-dev:mainfrom
dblnz:rally/1190-generate-coverage-reports

Conversation

@dblnz
Copy link
Contributor

@dblnz dblnz commented Mar 11, 2026

Add coverage infrastructure for the Hyperlight project:

Partially addresses #1190

  • Justfile: add coverage, coverage-html, coverage-lcov, and coverage-ci recipes using cargo-llvm-cov for LLVM source-based code coverage
  • CI: add Coverage.yml weekly workflow (Monday 06:00 UTC, manual trigger) running on kvm/amd with self-built guest binaries
  • docs: add how-to-run-coverage.md with local and CI usage instructions

Guest/no_std crates are excluded from coverage because they define #[panic_handler] and cannot be compiled for the host target under coverage instrumentation. Coverage targets host-side crates only.

This PR:

  • only runs this on linux with KVM

@dblnz dblnz added the kind/enhancement For PRs adding features, improving functionality, docs, tests, etc. label Mar 11, 2026
Copy link
Contributor

@jsturtevant jsturtevant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its nice to see a report. Do you have thoughts on how do we ensure we are looking at this report and not just another artifact? Do we post an issue on run? post in release? Review at community meetings?

I ran it locally and I am seeing 70.42%, which quite high! Its nice to see this. I am not to worried about minor changes but would like to see it trending even or even improving over time.

ludfjig
ludfjig previously approved these changes Mar 12, 2026
Copy link
Contributor

@ludfjig ludfjig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome, I also tried it locally and works great. I only have one question: is this the recommended way to generate coverage reports (using cargo-llvm-cov), or are there alternative ways? I'm only asking because I am not familiar with this area.

@dblnz
Copy link
Contributor Author

dblnz commented Mar 16, 2026

This is awesome, I also tried it locally and works great. I only have one question: is this the recommended way to generate coverage reports (using cargo-llvm-cov), or are there alternative ways? I'm only asking because I am not familiar with this area.

From the research I've done, there are two other alternatives to llvm-cov: tarpaulin and grcov.
Tarpaulin uses ptrace by default (only on Linux), can be configured to use LLVM, but it has some limitations (coverage data not returned when non-zero exit, some areas of thread unsafety).
Grcov could also be a good alternative for us, it supports multiple aggregation sources, but it's slightly more complicated to set up. Given that I don't think we need that much flexibility at the moment, I haven't used this one.
I chose llvm-cov due to its accuracy (uses the llvm backend), support across OS (we could enable windows also) and its relatively simple setup.

@dblnz
Copy link
Contributor Author

dblnz commented Mar 16, 2026

Its nice to see a report. Do you have thoughts on how do we ensure we are looking at this report and not just another artifact? Do we post an issue on run? post in release? Review at community meetings?

I ran it locally and I am seeing 70.42%, which quite high! Its nice to see this. I am not to worried about minor changes but would like to see it trending even or even improving over time.

About this, I think we should definitely have this in mind when we do changes. Although, I wouldn't condition any PR on it as it takes a lot to run (same time the PR validations take).
Considering we are still making fundamental changes in the codebase, this metric could change a lot.

So maybe start with having this info as part of the release artifacts is a good start? Then we could see if we need to discuss it more often.

One nice to have thing I was thinking about is to try and correlate the coverage output with the changes made in a PR so that we could have a check saying we are only testing x percent of the newly introduced code. I'll hack at this once I get some more time

@dblnz
Copy link
Contributor Author

dblnz commented Mar 16, 2026

You can check out how the summary looks like at: https://github.com/hyperlight-dev/hyperlight/actions/runs/22959438794?pr=1301

@dblnz
Copy link
Contributor Author

dblnz commented Mar 16, 2026

I am also looking at enabling the branch coverage report (it needs nightly).

@jsturtevant
Copy link
Contributor

So maybe start with having this info as part of the release artifacts is a good start? Then we could see if we need to discuss it more often.

Another thought is if it drops by %5 to create an issue (we have some machinery that does this as part of other ci workflows). that would inform us that we've been going down.

Otherwise this looks great and is a good first start. Feel free to move forward with this and do follow ups.

@dblnz
Copy link
Contributor Author

dblnz commented Mar 24, 2026

So maybe start with having this info as part of the release artifacts is a good start? Then we could see if we need to discuss it more often.

Another thought is if it drops by %5 to create an issue (we have some machinery that does this as part of other ci workflows). that would inform us that we've been going down.

Otherwise this looks great and is a good first start. Feel free to move forward with this and do follow ups.

Now that I'm back, I'll move this forward by addressing all the comments and suggestions.
Thanks for having a look at this!

@dblnz dblnz force-pushed the rally/1190-generate-coverage-reports branch from 808b8eb to 9622c18 Compare March 24, 2026 19:02
@dblnz dblnz requested review from jsturtevant and ludfjig March 24, 2026 21:55
@ludfjig ludfjig requested a review from Copilot March 24, 2026 22:31
ludfjig
ludfjig previously approved these changes Mar 24, 2026
Copy link
Contributor

@ludfjig ludfjig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds Rust coverage report generation for Hyperlight using cargo-llvm-cov, including local just recipes, a scheduled CI workflow, and documentation for running coverage.

Changes:

  • Introduces just recipes to collect coverage data and emit text/HTML/LCOV reports.
  • Adds a weekly (and manually triggerable) GitHub Actions workflow to generate and upload coverage artifacts.
  • Adds documentation describing local and CI coverage usage and outputs.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 11 comments.

File Description
docs/how-to-run-coverage.md New guide documenting local + CI coverage workflows and outputs.
Justfile Adds cargo-llvm-cov install/bootstrap and coverage run/report recipes.
.github/workflows/Coverage.yml Adds scheduled CI workflow to run coverage and publish summaries/artifacts.

@dblnz dblnz force-pushed the rally/1190-generate-coverage-reports branch from 9622c18 to 90badac Compare March 25, 2026 12:10
Add coverage infrastructure for the Hyperlight project:

- Justfile: add coverage-run, coverage, coverage-html, coverage-lcov,
  and coverage-ci recipes using cargo-llvm-cov for LLVM source-based
  code coverage. Tests run once via coverage-run; report recipes just
  generate the desired output format from collected profdata.
- CI: add Coverage.yml weekly workflow (Monday 06:00 UTC, manual trigger)
  running on kvm/amd with self-built guest binaries
- coverage-ci mirrors test-like-ci by running multiple test phases with
  different feature combinations (default, single-driver, crashdump,
  tracing) and merging profdata into a single unified report
- Coverage summary is displayed in the GitHub Actions Job Summary for
  quick viewing; full HTML report is downloadable as an artifact
- docs: add how-to-run-coverage.md with local and CI usage instructions

Guest/no_std crates are excluded from coverage because they define
coverage instrumentation. Coverage targets host-side crates only.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Doru Blânzeanu <dblnz@pm.me>
@dblnz dblnz force-pushed the rally/1190-generate-coverage-reports branch from 90badac to f2d5799 Compare March 25, 2026 12:14
@dblnz dblnz requested a review from Copilot March 25, 2026 16:04
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

Comment on lines +494 to +498
# tests with default features (all drivers; skip stress tests — too slow under instrumentation)
cargo +nightly test {{ coverage-packages }} --tests -- --skip stress_test

# tests with single driver + build-metadata
cargo +nightly test {{ coverage-packages }} --no-default-features --features build-metadata,{{ if hypervisor == "mshv3" { "mshv3" } else { "kvm" } }} --tests -- --skip stress_test
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

coverage-run uses cargo test ... --tests, which (unlike the existing just test flow) does not run library unit tests (#[cfg(test)] in the crate). This will significantly under-report coverage for crates like hyperlight-common that primarily have unit tests. Consider running cargo test with --lib as well (e.g., --lib --tests), or otherwise align the target selection with the existing test-unit/test-integration split while keeping doctests excluded if desired.

Copilot uses AI. Check for mistakes.
cargo +nightly test {{ coverage-packages }} --no-default-features -F executable_heap,{{ if hypervisor == "mshv3" { "mshv3" } else { "kvm" } }} --test integration_test -- execute_on_heap

# crashdump tests + example
cargo +nightly test {{ coverage-packages }} --no-default-features --features crashdump,{{ if hypervisor == "mshv3" { "mshv3" } else { "kvm" } }} --tests -- test_crashdump
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This crashdump coverage phase also uses cargo test ... --tests, which can exclude unit tests in hyperlight-host (e.g., crashdump-related #[cfg(test)] modules). If the intent is to cover host-side unit tests, include --lib (or otherwise ensure unit tests are executed) so the crashdump feature’s library tests contribute to the coverage data.

Suggested change
cargo +nightly test {{ coverage-packages }} --no-default-features --features crashdump,{{ if hypervisor == "mshv3" { "mshv3" } else { "kvm" } }} --tests -- test_crashdump
cargo +nightly test {{ coverage-packages }} --no-default-features --features crashdump,{{ if hypervisor == "mshv3" { "mshv3" } else { "kvm" } }} --lib --tests -- test_crashdump

Copilot uses AI. Check for mistakes.
Comment on lines +514 to +515
cargo +nightly test -p hyperlight-common --no-default-features --features trace_guest --tests -- --skip stress_test
cargo +nightly test -p hyperlight-host --no-default-features --features trace_guest,{{ if hypervisor == "mshv3" { "mshv3" } else { "kvm" } }} --tests -- --skip stress_test
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tracing coverage phases use cargo test ... --tests, which may skip unit tests inside the crates (notably hyperlight-common, which has many #[test] unit tests). To avoid under-reporting, run with --lib --tests (or otherwise ensure unit tests are included) for the tracing feature configurations too.

Suggested change
cargo +nightly test -p hyperlight-common --no-default-features --features trace_guest --tests -- --skip stress_test
cargo +nightly test -p hyperlight-host --no-default-features --features trace_guest,{{ if hypervisor == "mshv3" { "mshv3" } else { "kvm" } }} --tests -- --skip stress_test
cargo +nightly test -p hyperlight-common --no-default-features --features trace_guest --lib --tests -- --skip stress_test
cargo +nightly test -p hyperlight-host --no-default-features --features trace_guest,{{ if hypervisor == "mshv3" { "mshv3" } else { "kvm" } }} --lib --tests -- --skip stress_test

Copilot uses AI. Check for mistakes.
Comment on lines +5 to +10
## Prerequisites

- A working Rust toolchain
- Rust nightly toolchain (required for branch coverage; installed automatically by the `just` recipes)
- [cargo-llvm-cov](https://github.com/taiki-e/cargo-llvm-cov) (installed automatically by the `just` recipes)
- Guest binaries must be built first: `just guests`
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This guide implies coverage is broadly runnable, but the PR description/CI workflow indicate coverage is only supported on Linux with KVM (and the just recipes are bash-specific). Please call this out explicitly in the prerequisites/CI sections so Windows/macOS users don’t assume it will work locally.

Copilot uses AI. Check for mistakes.
steps:
- uses: actions/checkout@v6

- uses: hyperlight-dev/ci-setup-workflow@v1.8.0
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This workflow pins hyperlight-dev/ci-setup-workflow to v1.8.0, while the rest of the repo’s workflows are on v1.9.0. Aligning this to v1.9.0 avoids running an older CI bootstrap and keeps behavior consistent across workflows.

Suggested change
- uses: hyperlight-dev/ci-setup-workflow@v1.8.0
- uses: hyperlight-dev/ci-setup-workflow@v1.9.0

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/enhancement For PRs adding features, improving functionality, docs, tests, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants