Uh oh!
There was an error while loading. Please reload this page.
ci: keep coverage-instrumented build between runs - #4296
Conversation
Every Rust workspace job cold-compiled ~700 crates (~2.5 min) because target/llvm-cov-target was unconditionally deleted twice per run: by the end-of-job cleanup and by the stale-artifact loop at job start (and the wallet job's loop could wipe it from the shared runner workspace on wallet-only PRs). Those deletions predate the size/free-disk guard, which already removes the whole target/ when it exceeds the caps — currently ~23GB against a 120GB cap with 178GB free, so unconditional pruning is redundant. Keep the instrumented build and remove only per-run profile data (profraw/profdata) at end of job, plus the same purge at the start of the test phase so a cancelled run whose cleanup never executed cannot leak stale profraw into the next run's coverage report. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Warning Review limit reached
Next review available in:21 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe workflows now preserve ChangesCoverage cleanup workflows
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@## v4.2-dev #4296 +/- ##
============================================
+ Coverage 87.60% 87.78% +0.17%
============================================
Files 2703 2677 -26 Lines 344985 342371 -2614 ============================================
- Hits 302238 300548 -1690 + Misses 42747 41823 -924
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/tests-rs-workspace.yml:
- Around line 236-245: Purge *.profraw and *.profdata files from
target/llvm-cov-target before either disk-prune guard, independently of the
coverage-cache reuse result. In
.github/workflows/tests-rs-workspace.yml#L236-L245, move the profile-only find
cleanup ahead of disk pruning and remove its reuse dependency; in
.github/workflows/tests-rs-wallet.yml#L54-L62, add the same cleanup before the
wallet disk-prune step.
🪄 Autofix
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: 28b7b481-eacb-4a4f-9be8-acf329488f7f
📒 Files selected for processing (2)
.github/workflows/tests-rs-wallet.yml.github/workflows/tests-rs-workspace.yml
Uh oh!
There was an error while loading. Please reload this page.
Review follow-up: stale profraw/profdata from a cancelled run were purged only after the disk guard had already measured target/, so they could inflate the size check and cost the whole instrumented build. Move the purge into the prune step of both Rust workflows, unconditionally and ahead of the du measurement, and drop the now redundant reuse-gated purge from the stale-coverage step. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The install steps redirected stderr to /dev/null and swallowed the exit code, so a failed nextest install on a freshly provisioned runner passed silently and the job died later in the test step with "no such command: nextest". Install only when the tool is missing and assert it runs afterwards, so a provisioning problem fails at the step that owns it with the real error visible. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
Issue being fixed or feature implemented
Every Rust workspace test job cold-compiles ~700 crates (668 external dependencies from
proc-macro2up, plus 37 workspace crates) before running a single test — ~2.5 minutes of the ~4.5-minute test step. Analysis of a recent run showed why: the coverage-instrumented build lives intarget/llvm-cov-target(10GB), and it was unconditionally deleted twice per run — by the end-of-job "Remove cargo-llvm-cov build artifacts" step and again by the stale-artifact loop at job start. The wallet fast-path job's loop could also wipe it from the shared runner workspace on wallet-only PRs.These deletions date from when the runners were tight on disk (#3667). They're redundant now: the "Prune runner disk before tests" size guard already removes the whole
target/when it exceeds the caps, and current numbers are ~23GB total against a 120GB cap with 178GB free.What was done?
target/llvm-cov-targetfrom the stale-artifact prune loops in bothtests-rs-workspace.ymlandtests-rs-wallet.yml, with comments explaining why it must be kept.*.profraw/*.profdata— the per-run droppings, one profraw per test process, regenerated every run).always()cleanup never executed cannot leak stale profraw into the next run's coverage report. (The old design was immune to this by wiping the whole directory at start; the new design is immune via the start-of-run purge.)With the instrumented build retained, cargo's fingerprinting recompiles only the crates a PR actually touches, cutting ~2–2.5 minutes off every Rust workspace job. Combined with the shielded coverage cache (#4293), a typical unrelated Rust PR's job should drop from ~13 minutes to roughly 3.5–4 minutes.
How Has This Been Tested?
Compilinglines (668 external deps — a full cold build), ~2m27s compile before the first test executed, 117s of actual test execution.target/13GB +llvm-cov-target10GB ≈ 23GB, against a 120GB cap and 178GB free onmac-runner-2.Breaking Changes
None.
Checklist:
For repository code-owners and collaborators only
🤖 Generated with Claude Code
Summary by CodeRabbit