ci: collect + upload coverage with cargo-llvm-cov#64
Merged
Conversation
Run cargo-llvm-cov (prebuilt binary via taiki-e/install-action) on the `stable` matrix entry to emit lcov.info, then upload to Codecov non-blockingly. MSRV entry is unchanged. Requires CODECOV_TOKEN to be set as a repo secret. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
WalkthroughThis PR adds code coverage reporting to the continuous integration pipeline. The test workflow now installs the LLVM coverage tool, generates a coverage report, and uploads results to Codecov for the stable Rust version only. ChangesCoverage Reporting Setup
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/test.yml (1)
98-100: ⚡ Quick winUse the repo coverage wrapper to avoid local/CI drift.
Line 100 bypasses
scripts/coverage.sh, so CI may diverge from repo-default coverage flags (notably--ignore-filename-regex='src/bin/').Proposed refactor
- name: cargo llvm-cov (lcov) if: matrix.rust == 'stable' - run: cargo llvm-cov --all-features --workspace --lcov --output-path lcov.info + run: bash scripts/coverage.sh --workspace --lcov --output-path lcov.info🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/test.yml around lines 98 - 100, Replace the direct cargo llvm-cov invocation in the GitHub Actions step with the repository coverage wrapper script (scripts/coverage.sh) so CI uses the repo-default flags (e.g., --ignore-filename-regex='src/bin/') and avoids local/CI drift; keep the existing step name and the matrix.rust conditional, and ensure the wrapper is invoked so it still produces the lcov output (lcov.info) expected by downstream steps.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/test.yml:
- Around line 91-100: The workflow currently runs the "Install cargo-llvm-cov"
and "cargo llvm-cov (lcov)" steps without adding the LLVM Rust toolchain
components; update the job to install llvm-tools-preview before invoking cargo
llvm-cov (or replace the direct run with a call to scripts/coverage.sh which
already handles the setup). Specifically, add a step (before the "cargo llvm-cov
(lcov)" step) that runs rustup component add llvm-tools-preview --toolchain
stable (or the appropriate ${{ matrix.rust }} value) so llvm-cov/llvm-profdata
are available, or change the "cargo llvm-cov (lcov)" step to run
scripts/coverage.sh which performs the component installation and error
handling.
---
Nitpick comments:
In @.github/workflows/test.yml:
- Around line 98-100: Replace the direct cargo llvm-cov invocation in the GitHub
Actions step with the repository coverage wrapper script (scripts/coverage.sh)
so CI uses the repo-default flags (e.g., --ignore-filename-regex='src/bin/') and
avoids local/CI drift; keep the existing step name and the matrix.rust
conditional, and ensure the wrapper is invoked so it still produces the lcov
output (lcov.info) expected by downstream steps.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: bbc9e433-efa7-44e1-b6e2-b7210e0bfd0a
📒 Files selected for processing (1)
.github/workflows/test.yml
Comment on lines
+91
to
+100
| - name: Install cargo-llvm-cov | ||
| if: matrix.rust == 'stable' | ||
| # taiki-e/install-action v2.79.7 | ||
| uses: taiki-e/install-action@d9be7d8cda89035c9c843f78bd44d4f72d8403d4 # v2.79.7 | ||
| with: | ||
| tool: cargo-llvm-cov | ||
|
|
||
| - name: cargo llvm-cov (lcov) | ||
| if: matrix.rust == 'stable' | ||
| run: cargo llvm-cov --all-features --workspace --lcov --output-path lcov.info |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Checking coverage prereq references..."
rg -n "cargo llvm-cov|llvm-tools-preview|components:" .github/workflows/test.yml scripts/coverage.shRepository: agentruntimecontrolprotocol/rust-sdk
Length of output: 774
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== test.yml relevant sections ==="
nl -ba .github/workflows/test.yml | sed -n '40,130p'
echo "=== coverage.sh relevant sections ==="
nl -ba scripts/coverage.sh | sed -n '1,120p'Repository: agentruntimecontrolprotocol/rust-sdk
Length of output: 176
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== test.yml relevant sections ==="
cat -n .github/workflows/test.yml | sed -n '40,130p'
echo
echo "=== coverage.sh relevant sections ==="
cat -n scripts/coverage.sh | sed -n '1,120p'Repository: agentruntimecontrolprotocol/rust-sdk
Length of output: 6250
Install llvm-tools-preview (or use scripts/coverage.sh) before cargo llvm-cov.
test.yml installs only rustfmt, clippy and then runs cargo llvm-cov for the stable matrix (lines 56, 98-100) without installing llvm-tools-preview. scripts/coverage.sh shows cargo llvm-cov needs llvm-cov/llvm-profdata and exits with an error instructing rustup component add llvm-tools-preview when they’re missing (lines 4-6, 42-46).
Proposed fix
- name: Install cargo-llvm-cov
if: matrix.rust == 'stable'
# taiki-e/install-action v2.79.7
uses: taiki-e/install-action@d9be7d8cda89035c9c843f78bd44d4f72d8403d4 # v2.79.7
with:
tool: cargo-llvm-cov
+ - name: Install llvm-tools-preview
+ if: matrix.rust == 'stable'
+ run: rustup component add llvm-tools-preview
+
- name: cargo llvm-cov (lcov)
if: matrix.rust == 'stable'
run: cargo llvm-cov --all-features --workspace --lcov --output-path lcov.info📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - name: Install cargo-llvm-cov | |
| if: matrix.rust == 'stable' | |
| # taiki-e/install-action v2.79.7 | |
| uses: taiki-e/install-action@d9be7d8cda89035c9c843f78bd44d4f72d8403d4 # v2.79.7 | |
| with: | |
| tool: cargo-llvm-cov | |
| - name: cargo llvm-cov (lcov) | |
| if: matrix.rust == 'stable' | |
| run: cargo llvm-cov --all-features --workspace --lcov --output-path lcov.info | |
| - name: Install cargo-llvm-cov | |
| if: matrix.rust == 'stable' | |
| # taiki-e/install-action v2.79.7 | |
| uses: taiki-e/install-action@d9be7d8cda89035c9c843f78bd44d4f72d8403d4 # v2.79.7 | |
| with: | |
| tool: cargo-llvm-cov | |
| - name: Install llvm-tools-preview | |
| if: matrix.rust == 'stable' | |
| run: rustup component add llvm-tools-preview | |
| - name: cargo llvm-cov (lcov) | |
| if: matrix.rust == 'stable' | |
| run: cargo llvm-cov --all-features --workspace --lcov --output-path lcov.info |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/test.yml around lines 91 - 100, The workflow currently
runs the "Install cargo-llvm-cov" and "cargo llvm-cov (lcov)" steps without
adding the LLVM Rust toolchain components; update the job to install
llvm-tools-preview before invoking cargo llvm-cov (or replace the direct run
with a call to scripts/coverage.sh which already handles the setup).
Specifically, add a step (before the "cargo llvm-cov (lcov)" step) that runs
rustup component add llvm-tools-preview --toolchain stable (or the appropriate
${{ matrix.rust }} value) so llvm-cov/llvm-profdata are available, or change the
"cargo llvm-cov (lcov)" step to run scripts/coverage.sh which performs the
component installation and error handling.
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
1 participant
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-covviataiki-e/install-action(prebuilt binary, no source compile).cargo llvm-cov --all-features --workspace --lcovon thestablematrix entry only — same coverage on MSRV.lcov.infoto Codecov.fail_ci_if_error: falseso a Codecov outage cannot break CI.Pre-merge action required
CODECOV_TOKENas a repo secret (gh secret set CODECOV_TOKEN --repo agentruntimecontrolprotocol/rust-sdk).Test plan
main.🤖 Generated with Claude Code
Summary by CodeRabbit