Skip to content

test: Rebalance parallel test scheduling and trim the slowest tests - #1037

Merged
vdusek merged 4 commits into
masterfrom
optimize-test-scheduling
Aug 27, 2026
Merged

test: Rebalance parallel test scheduling and trim the slowest tests#1037
vdusek merged 4 commits into
masterfrom
optimize-test-scheduling

Conversation

@vdusek

@vdusek vdusek commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

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 load scheduler seeds every worker with a chunk of items_per_node // 4 consecutive tests and never reassigns them. With 806 tests that chunk is 12, and one worker drew the test_build.py block holding all four variants of test_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:

  1. --dist worksteal in addopts, 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. --dist is inert without -n, so single-file runs are unaffected.
  2. test_build_delete_and_abort starts 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.
  3. test_request_queue_unlock_requests polled list_head until the locked IDs disappeared from it, which list_head does not reflect - so every variant burned the full 30s deadline and then discarded the poll result. It now polls the outcome it cares about, accumulating unlocked_count across unlock_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

@vdusek vdusek added adhoc Ad-hoc unplanned task added during the sprint. t-tooling Issues with this label are in the ownership of the tooling team. labels Aug 27, 2026
@vdusek vdusek self-assigned this Aug 27, 2026
@github-actions github-actions Bot added this to the 148th sprint - Tooling team milestone Aug 27, 2026
@github-actions github-actions Bot added the tested Temporary label used only programatically for some analytics. label Aug 27, 2026
@codecov

codecov Bot commented Aug 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.27%. Comparing base (19509a2) to head (23b8ab1).
⚠️ Report is 1 commits behind head on master.

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     
Flag Coverage Δ
integration 92.01% <ø> (-0.19%) ⬇️
unit 87.36% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@vdusek
vdusek marked this pull request as ready for review August 27, 2026 09:12
@vdusek
vdusek requested a review from Pijukatel August 27, 2026 09:12
@vdusek
vdusek merged commit 79e205b into master Aug 27, 2026
30 checks passed
@vdusek
vdusek deleted the optimize-test-scheduling branch August 27, 2026 09:21
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*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

adhoc Ad-hoc unplanned task added during the sprint. t-tooling Issues with this label are in the ownership of the tooling team. tested Temporary label used only programatically for some analytics.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants