ci: add Codecov coverage tracking#492
Closed
QuantumExplorer wants to merge 3 commits intov0.42-devfrom
Closed
Conversation
Add cargo-llvm-cov based coverage workflow that uploads to Codecov, caches baseline coverage for PR comparisons, and posts coverage diff reports as PR comments. Includes per-crate component tracking for all 11 workspace crates. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds end-to-end coverage tooling: Codecov config, a GitHub Actions workflow that runs cargo-llvm-cov and posts coverage, a Python coverage-diff analyzer for LCOV comparisons and new-test detection, and README coverage metrics/ badges. Changes
Sequence DiagramsequenceDiagram
actor Developer
participant Repo as "Git Repository"
participant GHA as "GitHub Actions"
participant Runner as "Runner (Rust + tooling)"
participant Codecov as "Codecov"
participant Diff as "coverage-diff.py"
participant PR as "Pull Request Comment"
Developer->>Repo: Push or open PR with Rust changes
Repo->>GHA: Trigger workflow
GHA->>GHA: Detect relevant Rust changes
GHA->>Runner: Setup Rust, install cargo-llvm-cov
Runner->>Runner: Build & run tests, generate lcov.info
Runner->>Codecov: Upload lcov.info
alt Push to base branch
GHA->>GHA: Cache baseline lcov.info
else Pull request
GHA->>Repo: Restore baseline lcov from base branch
GHA->>Diff: Run coverage-diff.py with baseline and PR lcovs
Diff->>Diff: Parse LCOVs, detect new tests, compute diffs
Diff->>GHA: Output coverage-report.md
GHA->>PR: Create or update PR comment with report
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This was referenced Mar 4, 2026
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/scripts/coverage-diff.py:
- Around line 92-101: The is_production_file() check currently misses files
named plain tests.rs and test_utils.rs; update the logic to also return False
for those filenames (e.g., check filepath.endswith("tests.rs") and
filepath.endswith("test_utils.rs") or check the basename against
["tests.rs","test_utils.rs"]) so these test and test-utility files are excluded
from production classification; keep the existing checks for
"_test.rs"/"_tests.rs" and "tests"/"benches" folders and return
filepath.endswith(".rs") only after the new exclusions.
- Around line 149-157: The test detection logic misses async tests because it
only checks added.startswith("fn "), so update the condition in the block that
uses saw_test_attr to recognize function declarations that may be prefixed by
async/visibility (e.g., accept lines starting with "async fn ", "pub fn ", "pub
async fn " etc.); modify the startswith check and the regex used in fn_match
(currently r"fn\s+(\w+)") to allow optional prefixes like
r"(?:pub\s+)?(?:async\s+)?fn\s+(\w+)" (or equivalent) so new async test
functions are captured and appended to new_tests with current_file. Ensure
saw_test_attr is still reset to False after recording.
In @.github/workflows/coverage.yml:
- Around line 32-36: Update the detect-changes filters by adding coverage infra
patterns to the existing filters block (the "filters" key) under "any-code":
include glob patterns that match the coverage workflow file, any CI coverage
scripts, and coverage config files so edits to the coverage pipeline trigger the
coverage job; modify the "any-code" list to append patterns for the coverage
workflow, coverage scripts folder(s), and coverage config filenames.
- Around line 14-16: The workflow's explicit permissions only set pull-requests:
write which blocks other default scopes; update the permissions block in the
workflow to include contents: read so actions/checkout@v4 can access repository
files (affects the detect-changes and coverage jobs); locate the permissions
section in the .github/workflows/coverage.yml and add the contents: read
permission alongside pull-requests: write.
- Around line 114-140: The "Post PR comment" workflow step must skip write
actions for forked PRs because GITHUB_TOKEN is read-only on forks; update the
step's if condition (the step named "Post PR comment") to only run for
non-forked pull requests, e.g. require github.event_name == 'pull_request' AND
github.event.pull_request.head.repo.full_name == github.repository (or
github.event.pull_request.head.repo.fork == false), so the gh api comment calls
are not executed for forked PRs.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cd9b46d5-368e-43de-b31b-894fc61d44fa
📒 Files selected for processing (4)
.codecov.yml.github/scripts/coverage-diff.py.github/workflows/coverage.ymlREADME.md
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.github/scripts/coverage-diff.py (2)
88-101:⚠️ Potential issue | 🟠 MajorExclude
tests.rs/test_utils.rsfrom production coverage classification.Line 95 only filters
_test.rs/_tests.rs; plaintests.rsandtest_utils.rsare still treated as production, which can skew coverage deltas.🔧 Proposed fix
def is_production_file(filepath): """Return True if the file is production code (not test code).""" - parts = filepath.replace("\\", "/").split("/") + normalized = filepath.replace("\\", "/") + parts = normalized.split("/") + basename = parts[-1] @@ if any(p == "tests" for p in parts): return False - if filepath.endswith("_test.rs") or filepath.endswith("_tests.rs"): + if basename in {"tests.rs", "test_utils.rs"}: + return False + if normalized.endswith("_test.rs") or normalized.endswith("_tests.rs"): + return False + if any(p in {"test-utils", "test_utils"} for p in parts): return False @@ - return filepath.endswith(".rs") + return normalized.endswith(".rs")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/scripts/coverage-diff.py around lines 88 - 101, The is_production_file(filepath) helper currently only filters filenames ending with "_test.rs" or "_tests.rs" and misses files like "tests.rs" and "test_utils.rs"; update the function to also exclude those by checking the basename (e.g., using os.path.basename or parts[-1]) and returning False when the basename is "tests.rs" or "test_utils.rs" in addition to the existing checks so these files aren't classified as production Rust sources.
149-155:⚠️ Potential issue | 🟠 MajorNew test detection misses common Rust test forms.
Line 149 misses
#[tokio::test(...)], and Line 153 only acceptsfn ..., soasync fn,pub fn, andpub async fntests are under-detected.🔧 Proposed fix
- if "#[test]" in added or "#[tokio::test]" in added: + if re.search(r"#\[(?:tokio::)?test(?:\([^]]*\))?\]", added): saw_test_attr = True continue - if saw_test_attr and added.startswith("fn "): - fn_match = re.match(r"fn\s+(\w+)", added) + if saw_test_attr: + fn_match = re.match( + r"(?:pub(?:\([^)]*\))?\s+)?(?:async\s+)?fn\s+(\w+)", + added, + ) if fn_match and current_file: new_tests.append((current_file, fn_match.group(1))) saw_test_attr = False continue🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/scripts/coverage-diff.py around lines 149 - 155, The test detection logic in coverage-diff.py misses attribute forms like #[tokio::test(...)] and function signatures with visibility/async; update the attribute check to look for "#[test" and "#[tokio::test" (e.g., check for "#[tokio::test" or startswith "#[tokio::test") so attributes with parameters are matched, and change the function-name regex used where saw_test_attr is true (the re.match that sets fn_match) to accept optional "pub" and/or "async" tokens (for example use a pattern that allows optional "pub" and "async" before "fn" and still captures the function name) so async fn, pub fn, and pub async fn tests are detected; adjust uses of added, saw_test_attr, current_file, and fn_match accordingly.
🧹 Nitpick comments (1)
.github/scripts/coverage-diff.py (1)
409-409: Use_line_noto satisfy lint and clarify intent.Line 409 does not use
line_no; rename it to_line_noto address B007 cleanly.🔧 Proposed fix
- for line_no, count in file_lines.items(): + for _line_no, count in file_lines.items():🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/scripts/coverage-diff.py at line 409, The loop variable `line_no` in the for-loop inside coverage-diff.py (`for line_no, count in file_lines.items():`) is unused; rename it to `_line_no` to satisfy lint rule B007 and clarify intent while leaving `count` intact; ensure there are no remaining references to `line_no` elsewhere in the function (update them if present) and run linters/tests to confirm the warning is resolved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/scripts/coverage-diff.py:
- Around line 41-42: The except FileNotFoundError that currently returns {}
hides missing required LCOV files; modify the error handling so that when the
missing file is the PR LCOV (e.g., the variable or parameter representing the PR
lcov path / the function that reads the PR LCOV) the FileNotFoundError is
re-raised to fail CI, and only swallow and return {} for optional/base LCOV
reads; update both occurrences of the except FileNotFoundError (the one that
returns {} near the top-level LCOV read and the one around lines ~389-393) to
distinguish required vs optional file paths and re-raise for the required PR
LCOV path.
---
Duplicate comments:
In @.github/scripts/coverage-diff.py:
- Around line 88-101: The is_production_file(filepath) helper currently only
filters filenames ending with "_test.rs" or "_tests.rs" and misses files like
"tests.rs" and "test_utils.rs"; update the function to also exclude those by
checking the basename (e.g., using os.path.basename or parts[-1]) and returning
False when the basename is "tests.rs" or "test_utils.rs" in addition to the
existing checks so these files aren't classified as production Rust sources.
- Around line 149-155: The test detection logic in coverage-diff.py misses
attribute forms like #[tokio::test(...)] and function signatures with
visibility/async; update the attribute check to look for "#[test" and
"#[tokio::test" (e.g., check for "#[tokio::test" or startswith "#[tokio::test")
so attributes with parameters are matched, and change the function-name regex
used where saw_test_attr is true (the re.match that sets fn_match) to accept
optional "pub" and/or "async" tokens (for example use a pattern that allows
optional "pub" and "async" before "fn" and still captures the function name) so
async fn, pub fn, and pub async fn tests are detected; adjust uses of added,
saw_test_attr, current_file, and fn_match accordingly.
---
Nitpick comments:
In @.github/scripts/coverage-diff.py:
- Line 409: The loop variable `line_no` in the for-loop inside coverage-diff.py
(`for line_no, count in file_lines.items():`) is unused; rename it to `_line_no`
to satisfy lint rule B007 and clarify intent while leaving `count` intact;
ensure there are no remaining references to `line_no` elsewhere in the function
(update them if present) and run linters/tests to confirm the warning is
resolved.
- Add contents: read permission for checkout steps - Include coverage infra files in detect-changes filter - Guard PR comment posting against forked PRs - Exclude tests.rs/test_utils.rs from production file classification - Detect async fn test functions (not just sync fn) - Fail fast when required PR LCOV file is missing - Remove unused Path import Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
xdustinface
requested changes
Mar 4, 2026
Collaborator
xdustinface
left a comment
xdustinface
left a comment
There was a problem hiding this comment.
Im all for coverage reporting! 👍 But i spent quite some time polishing the CI and i think it should be integrated into the current CI workflows. Makes it simpler and we anyway don't need to re-run all the tests we just need to generate the coverage files for the existing ubuntu runners for all the test groups and upload/merge them. This way it's also already incorporating the integration tests coming in #464.
Also, claude thinks coverage-diff.py isn't needed at all since codecov has native support for (better) PR comments.
Coverage Diff Report
PR coverage: 41923/65752 lines (63.76%) |
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
Member
Author
|
coverage-diff.py is meant in the future for finding stupid tests that are trying to be added. It might not be perfect but codecov only gives you the % increase total. |
xdustinface
added a commit
that referenced
this pull request
Mar 4, 2026
Inspired by #492 but integrated into the existing workflows.
xdustinface
added a commit
that referenced
this pull request
Mar 4, 2026
Inspired by #492 but integrated into the existing workflows.
xdustinface
added a commit
that referenced
this pull request
Mar 4, 2026
Inspired by #492 but integrated into the existing workflows.
xdustinface
added a commit
that referenced
this pull request
Mar 4, 2026
Inspired by #492 but integrated into the existing workflows.
xdustinface
added a commit
that referenced
this pull request
Mar 4, 2026
Inspired by #492 but integrated into the existing workflows.
Member
Author
|
replaced by #493 |
xdustinface
added a commit
that referenced
this pull request
Mar 4, 2026
Inspired by #492 but integrated into the existing workflows.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
cargo-llvm-covbased coverage workflow (.github/workflows/coverage.yml) that generates LCOV reports and uploads to Codecov.codecov.ymlwith per-crate component tracking for all 11 workspace crates and test file exclusions.github/scripts/coverage-diff.py) that posts PR comments comparing baseline vs PR coverage, detecting new test functions and newly covered/uncovered linesTest plan
CODECOV_TOKENsecret is set in repo settingsv0.42-dev🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Chores