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.47% -0.11%
==========================================
Files 1049 1053 +4
Lines 120650 121050 +400
Branches 4348 4382 +34
==========================================
+ Hits 94806 94988 +182
- Misses 24285 24514 +229
+ Partials 1559 1548 -11 ☔ 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 |
Use test-count LPT when no grouped runtime cache exists and keep test4 as the only persisted duration source. Retain regression coverage for the custom splitter while removing generated historical timing data. Coding-Agent: Codex Codex-Version: codex-cli 0.144.4 Model: gpt-5.6-sol Reasoning-Effort: xhigh
|
Final timing result; leaving the implementation as-is without further changes. Latest run: https://github.com/deepmodeling/deepmd-kit/actions/runs/29687842539
Compared with the base run (29456447617):
Compared with the preceding committed-seed run (29637394120):
Conclusion: the implementation saves approximately 24-26% of total runner usage, while the CI critical path is effectively unchanged from the base. The shard allocation remains uneven, with groups 1-4 much shorter than groups 5-11, but no further changes are planned for this PR. Coding agent: Codex |
Coding-Agent: Codex Codex-Version: codex-cli 0.144.4 Model: gpt-5.6-sol Reasoning-Effort: xhigh
|
Warm-cache rerun results: https://github.com/deepmodeling/deepmd-kit/actions/runs/29692580279 All 12 Python 3.10 shards passed. The primary comparison below is the nearly concurrent successful This is a like-for-like base comparison: the master run tested
The group numbers are not semantically equivalent because master splits individual tests while the PR splits fixture-sharing units, so the aggregate and critical-path comparisons are the meaningful results:
For the warm-cache-specific comparison against the preceding PR run, cumulative main/full-job time improved by another 2.1%/2.2%, while the slowest main/full job improved by 6.4%/6.0%. The warm cache successfully redistributed ordinary stable units. However, it still does not match durations for the four generated single-unit shards in groups 1-4. Their pytest nodeids contain process-specific memory addresses such as Conclusion: compared with current master, fixture-aware grouping materially reduces both total runner use and the CI critical path. Warm-cache reuse gives a smaller additional improvement, but the volatile-nodeid issue remains if we want the four large units to be estimated correctly. 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 rejectedtest4runtime cache after a successful grouped run has produced measured durationsValidation
pytest.UsageErrorruff check .ruff format .The previous committed-seed run reduced cumulative main-pytest time by 27.8%, but its slowest shard was 66:15 versus the 57:06 base. Static seed data was therefore removed; the current design relies on measured
test4data for steady-state balancing.Live CI validation: https://github.com/deepmodeling/deepmd-kit/actions/runs/29687842539
Coding agent: Codex
Codex version: codex-cli 0.144.4
Model: gpt-5.6-sol
Reasoning effort: xhigh