test(e2e): stop bounding assertions by idle-machine wall clock#8
Open
Antisophy wants to merge 1 commit into
Open
test(e2e): stop bounding assertions by idle-machine wall clock#8Antisophy wants to merge 1 commit into
Antisophy wants to merge 1 commit into
Conversation
The suite runs one check derivation per test, and nix builds them massively in parallel (max-jobs), so every test executes on a heavily contended machine. Per-assertion timeouts were sized to observed idle-machine performance (5s Playwright default through 90s), which turns correctness assertions into de facto performance assertions: under contention the condition still becomes true, but the clock fires first. Every historically flaky check passes in seconds when run alone, and its failure durations cluster exactly at the timeout values. Remove per-assertion time bounds as a class: assertions and condition waits share one generous budget (9 minutes, via the expect/action defaults and a page default timeout), and the test timeout (10 minutes) becomes a pure hang detector rather than a performance expectation. Condition waits return the moment they are satisfied, so passing runs are exactly as fast as before; only genuine failures report slower. Explicit backend/mock readiness probes deliberately keep tight budgets, so a broken world still fails fast.
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.
Every check derivation runs one Playwright test, and nix builds them massively in parallel (max-jobs), so tests always execute on a heavily contended machine. Per-assertion timeouts were sized to idle-machine performance (the 5s Playwright default through 90s), which turns correctness assertions into de facto performance assertions: under contention the awaited condition still becomes true, but the clock fires first.
Evidence that the wait time is real and the conditions do occur (accounting per your bar for timeout changes):
project-memoryperforms two serial agent CLI cold starts (parent plus spawned subtask); under 32-wide contention, node cold starts alone exceed the 60s budget.nix flake checksweep failed 2-10 of the same ~18 cells. After it: zero timeout-class failures across ~1300 test executions (two full ~600-check sweeps at full parallelism, plus four forced-rebuild rounds of the historical 18-cell flaky set running concurrently).The change removes per-assertion wall-clock bounds as a class instead of bumping individual values: assertions and condition waits share one generous budget (9 minutes, via the
expect/actionTimeout/navigationTimeoutdefaults and a page default timeout in the fixture), and the test timeout (10 minutes) becomes a pure hang detector rather than a performance expectation. Condition waits return the moment they are satisfied, so passing runs are exactly as fast as before; only genuine failures report slower. Explicit backend/mock readiness probes deliberately keep tight budgets, so a broken world still fails fast.What this deliberately does not fix (and instead makes visible): running the suite surfaced genuinely intermittent defects that were previously camouflaged as fast timeout flakes and cleared by re-running. Now they fail unambiguously, burning the full budget with the condition never occurring:
ask-answer.spec.ts:330sometimes fails a synchronous ordering assertion (activeIndex= -1): the waiting-to-active status transition is missed by the test's event capture. No timeout is involved.diff-result-toggle.spec.ts:9occasionally never renders the diff at all, even on an idle machine.These look like worthwhile follow-ups; this PR only removes the timeout anti-pattern so they stop hiding among false failures.
Tested with two full
nix flake checksweeps plus the rebuild rounds described above; the four contention-wedge failures observed during one sweep all pass on re-run (2-5s solo).