Repository navigation
The hooks job leaves the engine's tests and clippy to the engine job - #459
Conversation
On ubuntu-slim, capped at 15 minutes a job, the pre-commit leg rebuilt the 32 test binaries the engine job had already built for the same commit and was cancelled partway through engine-tests: twice on one pull request, and 2 of the last 7 runs on main. Skipping engine-tests and engine-clippy there, as coverage and kani already are, removes about six and a half minutes from the pre-commit and prek legs.
uphold-reviewWalkthroughThis pull request changes one setting in the CI The Changes
✅ uphold rules: uphold's rules pass · this repository's policy at 0b555c6Rule output✅ uphold reviewThe review document asked about 33 of uphold's records; this change violates none of them. Reviewed with uphold 1.38.0 by uphold-review 0.4.2 · ec8ec73 · claude-opus-5-5 · 35 s · $0.13 list |
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Update hook skip list .github/workflows/test.yml |
The hooks job adds engine-tests and engine-clippy to its SKIP value. |
Priority:
Estimated code review effort: 1 (Trivial) | ~3 minutes
Change: Other
Merge Risk: 
ec8ec
-
This change removes duplicate engine checks from the hooks job while preserving them in the dedicated engine job and local pre-push hooks. No actionable merge-blocking issue was established.
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: the hooks job no longer runs the engine tests and clippy checks because the engine job runs them. Docstring Coverage No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Docstring Coverage
-
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #459 +/- ##
=======================================
Coverage 94.98% 94.98%
=======================================
Files 56 56
Lines 28550 28550
=======================================
Hits 27118 27118
Misses 1432 1432 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The
pre-commitleg of the hooks job runs onubuntu-slim, which cancels any job at 15 minutes. That limit is the runner's own andtimeout-minutescannot raise it. The leg ran every manual-stage hook, includingengine-tests(cargo test) andengine-clippy. Theenginejob already runs both on the same commit.On a passing main run,
scan engine testsalone took 6 minutes of a 13.5-minute leg. Most of that is building and linking the 32 integration-test binaries on one CPU. All three caches (uv, Rusttarget/, pre-commit) hit with exact keys in both the passing and the cancelled runs. The remaining cost is compiling this crate's changed source, which no cache can skip.Effect: the leg was cancelled during
engine-teststwice on one pull request, and in 2 of the last 7 runs on main.prekruns the same hooks and finished at 13.5 minutes on main.Change:
SKIP: coverage,kanibecomesSKIP: coverage,kani,engine-tests,engine-clippy, for the reason the job already gives for coverage and kani. Thelefthookleg never ran either hook. Both hook entries still run at every local pre-push.content-policystays: the guard hooks also go throughcargo run, so skipping it would only move the build into the next hook.Expected: about 6.5 minutes off the
pre-commitandpreklegs.Summary by CodeRabbit