test(e2e): remove both machine-speed failure modes from the suite - #8
test(e2e): remove both machine-speed failure modes from the suite#8Antisophy wants to merge 2 commits 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.
…rvable The router focuses the answering task while a question is delivered and focuses back when the answer returns. With the mock answering instantly, the answerer-focused sidebar state lasts about 75ms on an idle machine, less than Playwright's visibility polling can reliably observe, so every spec asserting the intermediate focus times out on a page that already moved on correctly. The ask/follow-up spec family therefore failed near-deterministically in serial runs on a quiet machine while passing under load, long misread as random flakiness. Give every Answer response a 400ms delay (ANSWER_DELAY_MS) carried on the intent and honored by all three protocol emulations before the tool call streams, so the focused state comfortably outlives the polling interval. The copilot proxy already delayed single Answer calls by 500ms for its own event-ordering reasons; it now honors the larger of the two delays on both tool-call branches.
|
Updated this PR to cover the suite's other machine-speed failure mode, so it now removes both:
|
|
I think the first commit is worth doing; the trade-off is worthwhile. I'm not sure about the second commit; it seems to codify that "answering questions is slow". However, with a local model (and even with GPT in some setups) answers may indeed come quicker than a sufficiently loaded client may observe. Maybe a more nuanced fix is to introduce a "wait" mock-LLM command, and have the tests that want to observe intermediate states use it, or something like that. Of course, the best fix would be to make intermediate states observable regardless of timing or load, via some mechanism that allows observing events before any coalescing, if feasible. Maybe the above "wait" command could actually suspend the LLM until it is unpaused via some side channel (direct /resume HTTP call OSLT) to remove all reliance on timing. |
|
True. Hard-coding a delay is not necessarily ideal. It is admittedly a stopgap to prevent having to spend hours of CPU time re-running tests that are almost guaranteed to succeed with the delay and almost guaranteed to not succeed without it. |
This removes both of the suite's machine-speed failure modes: assertions whose wall-clock bounds fire under parallel-build contention (a loaded machine), and assertions on transient UI states that a fast idle machine passes through more quickly than Playwright can observe. The two have opposite load profiles, which is much of why the flakiness looked random.
Loaded machine: wall-clock bounds
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.Idle machine: transient focus states
The ask flow focuses the answering task while a question is delivered and focuses back when the answer returns. The mock answered instantly, so the answerer-focused sidebar state lasted about 75ms on an idle machine, less than Playwright's visibility polling can reliably observe, and every spec asserting the intermediate focus timed out on a page that had already moved on correctly. That inverts the timeout class's profile: these specs failed near-deterministically in serial runs on a quiet machine while passing under wide parallelism, where contention stretched the round trip into observability.
Answer responses now carry a 400ms delay (
ANSWER_DELAY_MSinpatterns.mjs), honored by all three protocol emulations before the tool call streams, so the focused state comfortably outlives the polling interval. Mechanism details and measurements are in this comment on #19. With the delay in place, forced serial re-runs of the previously failing family on an idle machine pass deterministically.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).