test: Rebalance parallel test scheduling and trim the slowest tests - #1037
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1037 +/- ##
==========================================
- Coverage 95.29% 95.27% -0.02%
==========================================
Files 59 59
Lines 5501 5501
==========================================
- Hits 5242 5241 -1
- Misses 259 260 +1
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:
|
vdusek
marked this pull request as ready for review
August 27, 2026 09:12
Pijukatel
approved these changes
Aug 27, 2026
vdusek
added a commit
to apify/crawlee-python
that referenced
this pull request
Aug 28, 2026
…2199) The unit suite spends most of its wall-clock waiting. On a reference CI job (`ubuntu-latest / 3.14`, run 33047203526) the main pass does 578 worker-seconds of work in 131.8s across 8 workers and burns 476 of them idle. `--dist load` seeds each worker with a fixed chunk of consecutive tests and never reassigns, so `gw4` was still running `_playwright/test_utils.py` at 125s with the other seven idle for about a minute. Perfect balance would finish at 72.3s. Same failure mode as apify/apify-client-python#1037. ## Changes 1. **`--dist worksteal` in `addopts`** so an idle worker takes over tests still queued on a busy one. Nothing in the repo uses `xdist_group`, and `--dist` is inert without `-n`. 2. **`proxy` and `disabled_proxy` are session-scoped and sync.** Each teardown blocks ~1s on proxy.py's hardcoded `selector.select(timeout=1)`; with 17 servers built and destroyed that is ~17 worker-seconds of pure waiting. The server is a stateless forwarder, so one per session is enough. Sync because a session-scoped async fixture is incompatible with `asyncio_default_fixture_loop_scope = "function"`, and neither ever awaited. 3. **Dropped `--numprocesses=1` from the `run_alone` pass** - it paid the full xdist worker bootstrap for a 15-test serial suite. 4. **`COVERAGE_CORE=sysmon` on `unit-tests-cov`.** Measured on a 1397-test subset: default core +14%, sysmon +0%. Coverage falls back with a warning where `sys.monitoring` is unavailable, so 3.10 and 3.11 are safe. 5. **`test_infinite_scroll_on_dynamic_page` waits on the page instead of the clock** - four `wait_for_timeout(1000)` sleeps for a `loadMore()` that awaits 100ms, on the critical-path worker. ## Measured Full suite, both passes, same machine, two reps each: **~114s -> ~74s (-35%)**. These runs are without coverage, so change 4 is not reflected. 2456 tests pass on both sides; `ruff format`, `ruff check` and `ty` are clean. ## Not in this PR - `from crawlee.crawlers import HttpCrawler` costs 3.51s, of which 1.28s is scikit-learn and scipy, imported eagerly by `_adaptive_playwright._rendering_type_predictor` (behind the optional `adaptive-crawler` extra). Every worker pays it, as does every `crawlee[all]` cold start. Not a one-liner - `RenderingTypePredictorState` needs `LogisticRegression` at class-creation time. Worth its own issue. - 14 of the 15 matrix jobs compute coverage that is never uploaded; skipping it needs a change in `apify/workflows`. - `tests_concurrency: "8"` may be 2x oversubscription on 4-vCPU runners; worth one run with `nproc` echoed and an `-n4` comparison. - `test_http_2` still hits the real internet - it needs an HTTP/2 endpoint the local uvicorn server does not speak. *✍️ Drafted by Claude Code*
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The integration test job grew from ~3.5 min to ~6.3 min when the suite started running across both built-in HTTP transports. Almost none of that was extra work: with 806 tests the suite is only 1432 worker-seconds, which is ~90s on 16 workers. The job took 353s because one worker held the critical path while the other 15 sat idle.
xdist's default
loadscheduler seeds every worker with a chunk ofitems_per_node // 4consecutive tests and never reassigns them. With 806 tests that chunk is 12, and one worker drew thetest_build.pyblock holding all four variants oftest_build_delete_and_abort(~87s each), which it then ran back to back: 88s, 170s, 261s, 350s. Every other worker was finished by 170s. The same thing happened before the transport matrix, with 2 variants instead of 4 - so the old 3.5 min was already this one test.Three changes:
--dist workstealinaddopts, so an idle worker takes over tests still queued on a busy one. Measured on the unit suite at identical concurrency:1558 passed in 40.65s->1558 passed in 15.58s.--distis inert without-n, so single-file runs are unaffected.test_build_delete_and_abortstarts both Actor builds before awaiting either.build()returns as soon as the build is queued, so the platform can run them concurrently instead of the test serializing two ~40s builds.test_request_queue_unlock_requestspolledlist_headuntil the locked IDs disappeared from it, whichlist_headdoes not reflect - so every variant burned the full 30s deadline and then discarded the poll result. It now polls the outcome it cares about, accumulatingunlocked_countacrossunlock_requests()calls until all three locks are accounted for, and asserts that total.Simulated against the per-test durations reconstructed from the CI logs: 353s -> 100s with change 1 alone, -> 72s with all three.
Change 2 only pays off if the platform builds two versions of the same Actor concurrently. If it serializes them per Actor the test still passes, it just stays at ~90s and the job lands nearer 2 min than 1.5.
✍️ Drafted by Claude Code