Skip to content

test(e2e): remove both machine-speed failure modes from the suite - #8

Open
Antisophy wants to merge 2 commits into
CyberShadow:masterfrom
Antisophy:test/condition-based-waits
Open

test(e2e): remove both machine-speed failure modes from the suite#8
Antisophy wants to merge 2 commits into
CyberShadow:masterfrom
Antisophy:test/condition-based-waits

Conversation

@Antisophy

@Antisophy Antisophy commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

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):

  • Every historically flaky check passes in 2-8 seconds when run alone; their failure durations cluster exactly at their timeout values (60s waits fail at 1.1m, 90s at 1.6m, etc.).
  • The worst offenders are the multi-agent tests: e.g. project-memory performs two serial agent CLI cold starts (parent plus spawned subtask); under 32-wide contention, node cold starts alone exceed the 60s budget.
  • Before this change, every full nix flake check sweep 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/navigationTimeout defaults 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_MS in patterns.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:330 sometimes 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:9 occasionally never renders the diff at all, even on an idle machine.
  • Under sustained all-e2e contention, an agent turn occasionally wedges entirely (never completes, and teardown hangs too), suggesting a process-level failure under load.

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 check sweeps plus the rebuild rounds described above; the four contention-wedge failures observed during one sweep all pass on re-run (2-5s solo).

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.
@Antisophy Antisophy changed the title test(e2e): stop bounding assertions by idle-machine wall clock test(e2e): remove both machine-speed failure modes from the suite Aug 29, 2026
@Antisophy

Copy link
Copy Markdown
Contributor Author

Updated this PR to cover the suite's other machine-speed failure mode, so it now removes both:

  • The branch was already based on current master; no code in the original commit changed.
  • A second commit gives every mock Answer response a 400ms delay, honored by all three protocol emulations. Without it, the answerer-focused sidebar state lasts about 75ms on a fast idle machine, less than Playwright's visibility polling can reliably observe, so every focus-asserting ask/follow-up spec failed near-deterministically in serial runs while passing under parallel load (mechanism and measurements in this comment on #19). With the delay, forced serial re-runs of that family on an idle machine pass deterministically.
  • Retitled and rewrote the description to present the two fixes together.

@CyberShadow

Copy link
Copy Markdown
Owner

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.

@Antisophy

Copy link
Copy Markdown
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants