ci: balance fixture-aware test groups#5810
Conversation
Use duration-based contiguous chunks so neighboring tests and their expensive class-scoped setup are less likely to be dispersed across all CI shards.\n\nCoding-Agent: Codex\nCodex-Version: codex-cli 0.144.4\nModel: gpt-5.6-sol\nReasoning-Effort: xhigh
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe Python test workflow now invokes a pytest plugin that groups fixture-sharing tests, balances shards using cached durations, and selects a requested CI group. Tests validate grouping and deterministic balancing, while workflow cache keys move to the ChangesPython test sharding
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CI as CI workflow
participant Pytest
participant Cache as Duration cache
participant Plugin as ci_split plugin
CI->>Pytest: Run with shard options
Pytest->>Plugin: Collect tests and invoke hooks
Plugin->>Cache: Load persisted durations
Cache-->>Plugin: Return duration data
Plugin->>Plugin: Group and balance test units
Plugin->>Pytest: Select requested shard
🚥 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 |
Start the contiguous splitting algorithm with a clean duration cache so setup costs recorded under least-duration sharding do not distort the new chunk boundaries.\n\nCoding-Agent: Codex\nCodex-Version: codex-cli 0.144.4\nModel: gpt-5.6-sol\nReasoning-Effort: xhigh
![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
🤖 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_python.yml:
- Line 59: Update the cache key in the test workflow to include
github.run_attempt alongside the existing run identifiers, Python version, and
matrix group, ensuring reruns use distinct per-shard duration caches.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1c7752db-b768-442a-80a3-b068954bf662
📒 Files selected for processing (1)
.github/workflows/test_python.yml
Include github.run_attempt in each per-shard cache key so workflow reruns cannot restore stale shard-local duration data from an earlier attempt. Coding-Agent: Codex Codex-Version: codex-cli 0.144.4 Model: gpt-5.6-sol Reasoning-Effort: xhigh
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5810 +/- ##
==========================================
- Coverage 78.57% 78.42% -0.16%
==========================================
Files 1049 1053 +4
Lines 120650 120891 +241
Branches 4348 4382 +34
==========================================
- Hits 94806 94804 -2
- Misses 24285 24544 +259
+ Partials 1559 1543 -16 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Keep test classes and module-level tests together, then assign those units with deterministic duration-based LPT bin packing. Warm-start the new cache namespace from existing per-test timings so the first CI run remains balanced. Coding-Agent: Codex Codex-Version: codex-cli 0.144.4 Model: gpt-5.6-sol Reasoning-Effort: xhigh
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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_python.yml:
- Line 69: Append --store-durations to each TF2 test command in the workflow,
including the commands covering source/tests/tf2, source/jax2tf_tests, and
consistent TF2 tests. Do not add --clean-durations there, so those commands
append their timings to the duration file created by the source/tests step.
In `@source/tests/ci_split.py`:
- Around line 80-89: The explicit-group handling around _explicit_group_name
must preserve complete fixture units: a method-level marker must not separate
one method from the remaining methods in its class. Change grouping so explicit
groups merge the entire class or module unit, or reject markers that do not
cover that unit; add a regression covering a marked class with multiple methods
and verify all methods remain together.
- Around line 162-163: Update the legacy-list normalization in the cache restore
flow around the isinstance(data, list) check to catch TypeError and ValueError
from dict(data), and handle them through the existing cache usage error path.
Ensure malformed restored lists do not escape as unhandled collection failures.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8c61f016-a08f-4964-b0ab-764dacbe312c
📒 Files selected for processing (3)
.github/workflows/test_python.ymlsource/tests/ci_split.pysource/tests/test_ci_split.py
njzjz-bot
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Use version-controlled Python 3.10 and 3.13 estimates for expensive fixture units instead of relying on a possibly missing test2 cache. Preserve complete class/module units for explicit markers, retain TF2 timings through independent duration files, and report malformed legacy caches as usage errors. Coding-Agent: Codex Codex-Version: codex-cli 0.144.4 Model: gpt-5.6-sol Reasoning-Effort: xhigh
|
CI timing result for the first committed-seed run: https://github.com/deepmodeling/deepmd-kit/actions/runs/29637394120
Compared with the base run (29456447617):
Compared with the previous duration_based_chunks run (29513326076), the slowest main shard improved from 85:17 to 66:15 (22.3%), although cumulative main-pytest time increased by 5.0%. Conclusion: this run reduced duplicated setup and total runner-minutes, but did not accelerate the CI critical path. The first-run seed remained imbalanced, especially groups 3 and 6. A second run using the newly generated test4 grouped-duration cache is needed to evaluate steady-state performance. Coding agent: Codex |
3 participants
Summary
@pytest.mark.ci_split_group("name")for merging complete classes/modules that share an expensive fixture; partial method-level markers are rejectedtest4cache namespace only after a successful grouped run has produced combined runtime durationsValidation
source/tests/pt_expt/infer/test_deep_eval.pykept the 22TestDeepEvalEnerPt2methods in one unitpytest.UsageErrorruff check .ruff format .Live CI validation: https://github.com/deepmodeling/deepmd-kit/actions/runs/29637394120
Coding agent: Codex
Codex version: codex-cli 0.144.4
Model: gpt-5.6-sol
Reasoning effort: xhigh