tests: harden parallel test execution - #967
Conversation
|
Warning Review limit reached
Next review available in: 42 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThe PR separates serial tests from xdist runs, adds serial marker policy and fork-test safeguards, changes scheduling modes, caps automatic workers, scopes asyncio leak checks, updates CI lanes, and adds downstream consumer type-contract validation. ChangesValidation harness and execution lanes
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CIWorkflow
participant PytestXdist
participant SerialPytest
participant JUnitArtifacts
CIWorkflow->>PytestXdist: Run non-serial tests with selected xdist distribution
CIWorkflow->>SerialPytest: Run serial fork tests without xdist
PytestXdist->>JUnitArtifacts: Write parallel JUnit report
SerialPytest->>JUnitArtifacts: Write serial JUnit report
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c49876559
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/ci.yml:
- Around line 99-108: Add tests/core/test_cancel_token.py to the serial pytest
command in both serial validation lanes: .github/workflows/ci.yml lines 99-108
and .github/workflows/nightly-validation.yml lines 46-56. Keep the existing test
selections and serial marker unchanged.
In `@justfile`:
- Around line 36-41: Update the pytest marker selectors used by test-fast and
cov to exclude the documented latency marker alongside the other
timing-sensitive lanes, or revise the test-fast comment to say that only socket
tests are excluded. Keep the comment, marker expressions, and documented
validation behavior consistent.
- Around line 24-29: The serial pytest command in the justfile only disables
faulthandler, not the pytest-timeout plugin. Update the command using the
existing serial test invocation to override pytest-timeout with timeout=0, and
ensure every serial test explicitly configures its required timeout; otherwise
revise the documentation to avoid claiming pytest helper threads are disabled.
In `@tests/test_pytest_harness_policy.py`:
- Around line 72-75: Extend test_xdist_auto_workers_respect_explicit_override
with cases for a non-integer override and numeric values below 1. For each case,
set PYTEST_XDIST_AUTO_NUM_WORKERS and assert that _xdist_auto_worker_count()
raises pytest.UsageError, covering the validation path in conftest.py.
In `@tests/typecheck/test_consumer_contracts.py`:
- Around line 89-91: Add the pytest.mark.integration_external decorator to
test_downstream_consumer_type_contracts, alongside its existing slow and timeout
markers, so the locally spawned mypy process is classified as an external
integration test.
🪄 Autofix
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1f5b0cf3-019c-4c34-ba5d-2423e943c109
📒 Files selected for processing (22)
.github/workflows/ci.yml.github/workflows/nightly-validation.ymlCONTRIBUTING.mddocs/development/08-development-and-testing.mddocs/reference/validation-vocabulary.mdjustfilepyproject.tomlsrc/easycat/validation/_slice_runner.pytests/_pytest_targets.pytests/cli/test_validate_runner.pytests/config/test_easyconfig_preset_typing.pytests/conftest.pytests/core/test_cancel_token.pytests/runtime/test_artifacts.pytests/runtime/test_crash_sweep.pytests/test_public_api.pytests/test_pytest_harness_policy.pytests/test_session_event_typing.pytests/test_validation_markers.pytests/test_voice_app_typing.pytests/typecheck/test_consumer_contracts.pytests/vad/test_silero.py
💤 Files with no reviewable changes (4)
- tests/test_session_event_typing.py
- tests/test_voice_app_typing.py
- tests/config/test_easyconfig_preset_typing.py
- tests/test_public_api.py
- Render the scaffold agent.py beside server.py in the consumer contract test so mypy resolves 'from agent import make_agent' instead of silently degrading it to Any under ignore_missing_imports. - Disable pytest-timeout (-o timeout=0) in every serial lane so no helper thread can start before a direct-fork test, even for a future serial test that forgets its own timeout(0) marker. - Exclude the documented 'latency' marker from test-fast, cov, the quick validation slice, and the CI coverage job so the 'timing-sensitive lanes are excluded' claim holds by construction. - Cover invalid PYTEST_XDIST_AUTO_NUM_WORKERS overrides. - Guard that the CI serial file lists cover every serial-marked file. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
What changed
-n autoat 8 workers and use per-test load scheduling in socket-free quick and coverage lanesWhy
The suite mixed xdist workers, direct
os.fork()calls, a global async autouse fixture on synchronous tests, and several repeated cold mypy processes. That combination created deadlock risk, noisy teardown behavior, uneven scheduling, and redundant work.Impact
The quick lane remains broad while excluding process-global and xdist-unsafe tests. Static typing assertions are preserved in one dedicated CI step. The final local quick run completed in 2:44, compared with 3:51 before separating slow and serial work.
Validation
src/easycat: passedSummary by CodeRabbit
Bug Fixes
Documentation
Tests