ci: split fast tests from coverage tracing - #411
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe workflow splits CI testing into separate ChangesCI workflow test and coverage
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #411 +/- ##
===========================================
+ Coverage 83.26% 96.26% +12.99%
===========================================
Files 29 29
Lines 2307 2968 +661
Branches 732 732
===========================================
+ Hits 1921 2857 +936
+ Misses 348 66 -282
- Partials 38 45 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 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/test.yml:
- Around line 134-144: The coverage job is missing the setup dependency, so its
matrix cannot read needs.setup.outputs.packages. Update the coverage job’s needs
सूची to include setup alongside test, and verify the matrix expression in the
coverage job still references needs.setup.outputs.packages so the job can
instantiate correctly.
🪄 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
Run ID: 06d0bcc3-250c-4c84-a949-00336abb7bec
📒 Files selected for processing (1)
.github/workflows/test.yml
What
Splits the single
testjob into two, per @0xNeshi's review:test—sui move test(no tracing): the fast regression gate (~5s even formath/core). Gates merges.coverage—sui move test --trace+sui move coverage lcov+ Codecov: isolated,continue-on-error: true(informational, never blocks a merge), runs only aftertestis green.Why the 30-minute bump wasn't enough
Profiled
math/corelocally (Sui 1.71.1):sui move test(no trace): ~5.5s.sui move test --trace(what CI runs): the single random testmedian_matches_sorted_reference_u256alone takes 7m21s and writes 4.1 GB of traces. Itquick_sort!s a Sui-generated randomvector<u256>that can be very large, and--trace(full mode) records every VM instruction. The whole suite is many GB and never finishes inside 30 min.--trace(full) is mandatory forsui move coverage lcov— the lighter--trace function-only/instruction-onlymodes produce no line records — so the cost can't simply be switched off.The fix
@0xNeshi's split keeps the gate fast and moves the slow trace off the critical path. The coverage job also passes
--rand-num-iters 1: coverage only needs each line executed once, and the full randomized property iterations still run in the fasttestjob, so this cuts trace volume without losing coverage.Deferred follow-ups (
math/core's domain)math/core's ~9 heavymedian/quick_sortrandom tests (one of them produces the 4.1 GB above). Left out here since it changes those property tests.traces/is not gitignored — worth adding so a local--tracerun can't accidentally commit gigabytes.🤖 Generated with Claude Code
Summary by CodeRabbit