Improve unit test times with isolated parallel workers#225
Improve unit test times with isolated parallel workers#225rolandocortez wants to merge 5 commits into
Conversation
📝 WalkthroughWalkthroughUnit tests now run in parallel with worker-specific temporary workspaces, while E2E tests are divided across three deterministic shards. CI coverage paths, artifact cleanup, archive validation, and workflow gating are updated for parallel execution. ChangesTest Execution
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant Pytest
participant E2EShardPlugin
GitHubActions->>Pytest: pass shard index and shard count
Pytest->>E2EShardPlugin: collect E2E tests
E2EShardPlugin->>Pytest: retain assigned nodeids and deselect others
Pytest->>GitHubActions: execute selected shard
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/README.md (1)
21-25: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd language identifier to fenced code block.
Specifying a language for the fenced code block improves syntax highlighting and resolves a markdownlint warning.
♻️ Proposed fix
-``` +```bash pytest tests/test_unittests.py -n 2 --dist load --download-weights --log-cli-level=INFO --log-file=out.log --log-file-level=DEBUG</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@tests/README.mdaround lines 21 - 25, Update the fenced command block in the
tests README to specify the bash language identifier, preserving the existing
pytest command unchanged.</details> <!-- cr-comment:v1:46eb3ed9d26b48d1f77dd292 --> _Source: Linters/SAST tools_ </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.Nitpick comments:
In@tests/README.md:
- Around line 21-25: Update the fenced command block in the tests README to
specify the bash language identifier, preserving the existing pytest command
unchanged.</details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro **Run ID**: `789f4b11-3fad-4515-ba03-2ef80ad207b3` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 01b44ad8dbefbcceaedbcaef48950f2ab04a354a and 649c30a36d666eec02a0cd601fa4f0301967b36b. </details> <details> <summary>📒 Files selected for processing (6)</summary> * `.github/workflows/unittests.yaml` * `requirements-dev.txt` * `tests/README.md` * `tests/conftest.py` * `tests/helper_functions.py` * `tests/test_unittests.py` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified FilesNo covered modified files...
|
Test Results 6 files 6 suites 15m 14s ⏱️ Results for commit f338409. ♻️ This comment has been updated with latest results. |
4486f07 to
f77bed7
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/test_unittests.py (1)
101-103: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInclude CLI output in assertion failure messages.
When a negative test fails (e.g., the CLI succeeds when it was expected to fail, or fails with an unexpected error code), the assertion message only prints the exit codes. Appending
result.stdoutto these messages will make debugging CI failures significantly easier by revealing the actual error or unexpected output.
tests/test_unittests.py#L101-L103: Append\nOutput:\n{result.stdout}to the assertion message string.tests/test_unittests.py#L150-L152: Append\nOutput:\n{result.stdout}to the assertion message string.tests/test_unittests.py#L196-L198: Append\nOutput:\n{result.stdout}to the assertion message string.tests/test_unittests.py#L238-L240: Append\nOutput:\n{result.stdout}to the assertion message string.🤖 Prompt for 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. In `@tests/test_unittests.py` around lines 101 - 103, Append the CLI output to each exit-code assertion failure message in tests/test_unittests.py at lines 101-103, 150-152, 196-198, and 238-240, using result.stdout with the “Output” label. Update the assertion messages associated with the affected test checks while preserving their existing exit-code details.
🤖 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.
Nitpick comments:
In `@tests/test_unittests.py`:
- Around line 101-103: Append the CLI output to each exit-code assertion failure
message in tests/test_unittests.py at lines 101-103, 150-152, 196-198, and
238-240, using result.stdout with the “Output” label. Update the assertion
messages associated with the affected test checks while preserving their
existing exit-code details.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: afc547ca-3750-449a-9767-84840e0372af
⛔ Files ignored due to path filters (1)
media/coverage_badge.svgis excluded by!**/*.svg
📒 Files selected for processing (8)
.github/workflows/end2end_tests.yaml.github/workflows/unittests.yamlrequirements-dev.txttests/README.mdtests/conftest.pytests/e2e_shards.pytests/helper_functions.pytests/test_unittests.py
🚧 Files skipped from review as they are similar to previous changes (4)
- requirements-dev.txt
- tests/helper_functions.py
- .github/workflows/unittests.yaml
- tests/README.md
2 participants
Purpose
Reduce the CI test critical path by optimizing both the unit-test and E2E workflows.
For unit tests, independent model conversions now run in parallel with two
pytest-xdistworkers. Each worker uses an isolated temporary workspace for downloaded models, subprocess execution, conversion outputs, validation, and cleanup.For E2E tests, the 101 public cases are divided into three duration-balanced serial shards. Tests that use the same physical model weights remain in the same shard, allowing weights to be reused without introducing unsafe concurrent access to shared conversion resources.
Specification
Unit tests
pytest-xdistas a development dependency.-n 2--dist loadE2E tests
--e2e-shard-index--e2e-shard-countDependencies & Potential Impact
Adds:
pytest-xdist>=3.6.1The unit-test workflow deliberately uses two workers instead of
-n auto.In the controlled unit-test benchmark, mean peak aggregate process-tree RSS increased from approximately 1253 MiB during serial execution to 2321 MiB with two workers. This measurement may count some shared pages more than once, but it still supports using a fixed worker count to avoid unnecessary memory pressure on CI runners.
The isolated unit-test workspaces can contain the same model in more than one worker when both workers need it. The measured parallel runs contained 11 weight files representing 10 unique models, with
yolov8npresent in both workspaces.E2E sharding increases the number of concurrent CI jobs, but reduces the expected wall-clock critical path. The three-shard assignment was selected from profiling results and keeps the current physical weight groups isolated between shards.
Based on the successful hosted-run profile, the estimated public E2E critical-path reduction is approximately 65%. The actual runtime improvement will be confirmed by the hosted CI runs for this PR.
The coverage workflow also updated
media/coverage_badge.svgautomatically.No product code or vendored submodules are modified.
Testing & Validation
Unit-test benchmark
A controlled ABBA comparison was run against the same 27-test conversion suite, using a fresh temporary workspace for every execution:
Results:
Final local validation
main.yolov8nE2E conversion passed with both subtests.--delete-weights-nowbehavior remains compatible.pytest-xdist 3.6.1.git diff --checkpassed.The hosted CI matrix will provide the final runtime comparison across Ubuntu, Windows, and macOS.
Deployment Plan
No deployment required.
Rollout:
Rollback:
Monitoring:
AI Usage
Assisted-by: ChatGPT