Cut one root cause of smoke-all flakiness, and make the rest measurable#319
Merged
Conversation
Opening a project hung for ~60s whenever a single file doc was slow to serve. Two compounding causes in the sync client's connect(): 1. findDoc()'s repo.find() had no deadline, so it waited out automerge-repo's own ~60s unavailable-doc timeout before rejecting. 2. loadFileDocuments() loaded every file doc SERIALLY, so that 60s stall (and any others) summed end-to-end across the whole project. On a large project (the smoke-all extension fixtures share one ~49-file project) or a CPU-starved sync server, one slow doc stalled the entire project open past any caller's budget. In the hub-client this manifested as the Editor/preview never mounting — the dominant cause of the smoke-all E2E flakiness (traced: a file doc's repo.find hung exactly 60.2s while the test's 75s render-wait expired). It is also a real user-facing hang on loaded machines. Fixes: - Bound each repo.find() attempt with AbortSignal.timeout (5s). A timed-out attempt is treated like the existing cold-start "unavailable" race: retried while a peer is connected, then surfaced as `unavailable` so the file degrades gracefully via the existing markFileUnavailable path instead of hanging the open. (A doc that loses the race re-loads when its index entry next changes; eager retry-on-peer-arrival remains the separate "plan D2" gap noted in syncWithFiles.) - Load file docs CONCURRENTLY (Promise.all) so the wait is bounded by the single slowest doc, not the sum. Per-file unavailable handling and the rethrow of genuine (non-unavailable) errors are preserved. Verified: quarto-sync-client suite 102 pass; smoke-all 78/78 with no contention (and ~30% faster); under induced heavy CPU contention the extension-fixture failures drop from ~all to ~2/15 (the rest gated by the shared 49-file project + retries on CI).
The nightly smoke-all suite (78 in-browser WASM render tests) showed failures concentrated on specific fixtures, all surfacing as the same opaque "Timed out after 75000ms waiting for preview iframe to render". This reworks the test harness to (a) reduce the timing races it can fix itself and (b) make the *cause* of each failure observable. Harness changes (hub-client/e2e): - waitForVfsFiles — a best-effort VFS barrier (quiesce escape + 10s cap + 3s grace) so the render/assertions observe a complete VFS without ever introducing a hard failure or stalling on over-broad project file sets. - renderForAssertions — render ONCE for assertions and reuse the result for both HTML matching and diagnostics (was two extra full WASM renders per test on top of the live render). - 30s element-wait for ensureHtmlElements — covers the q2-preview decoration / live-iframe catch-up that the 5s default was too short for. Failure-stage classifier (the durable part): - capturePreviewDiagnostics reads stable DOM markers (ProjectSelector .project-selector/.connecting/.error, Editor .editor-container, PreviewRouter "Loading preview...", the preview iframe + body) plus the WASM test hooks to classify WHERE the boot→render pipeline stalled, and the render-wait emits a single parseable "[smoke-diag] stage=... " line into the timeout error. Every smoke-all failure otherwise looks identical in CI (one symptom, several distinct causes); this lets the offline analyzer bucket nightly failures by cause instead of guessing one at a time. Test-side only, fires only on the timeout path, so passing tests are unaffected. The investigation, measurements, and remaining-cause notes are in claude-notes/plans/2026-06-19-smoke-all-e2e-deflake.md.
gordonwoodhull
force-pushed
the
deflake/smoke-all-e2e
branch
from
e3b35a8 to
6f896c2
Compare
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
1 participant
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.
Background
The nightly
smoke-allE2E suite (78 in-browser WASM render tests) has been flaky for months. Every failure surfaces in CI as the same opaque line:Across the last 8 nightly logs, 114 / 115 smoke-all failures are that line (the 1 other is a content assertion). That single symptom hides several distinct causes — the project never loads, WASM never inits, the render itself hangs — which is why this suite has drawn a long series of "found the root cause" fixes that each only addressed one.
This PR does two things: fixes one of those causes, and makes the cause of every future failure observable so we stop guessing.
1.
fix(sync-client)— boundrepo.find()+ load file docs concurrentlyTraced via instrumentation: on a render-timeout, the app was stuck before the preview ever mounted — in
connect() → loadFileDocuments(). That function loaded every project file doc serially, andfindDoc()'srepo.find()had no deadline, so it waited out automerge-repo's own ~60s unavailable-doc timeout. One slow-to-serve doc → the whole project-open hung ~60s → the 75s render-wait expired. (The extension fixtures share one ~49-file project, so they hit a slow doc most often — which is exactly why they top the flaky list.)repo.find()now gets a per-attemptAbortSignal.timeout(5s); a timed-out attempt degrades into the existingmarkFileUnavailablepath instead of hanging.Promise.all), so the wait is bounded by the single slowest doc, not the sum.This is a real user-facing fix too: opening a project no longer stalls for a minute when one file is slow to sync.
2.
test(hub-e2e)— deflake + classify failures by pipeline stageHarness hygiene (genuine but partial improvements): a best-effort VFS barrier, rendering once for assertions instead of 2–3× per test, and a longer element-wait for q2-preview decoration.
The durable part — failure classification. On a render-timeout, the helper now reads stable DOM markers + WASM hooks and emits a parseable line:
stagelocalizes where the boot→render pipeline stalled:CONNECT_STALL/WASM_NOT_READY/PREVIEW_ROUTER_STALL/EDITOR_NO_PREVIEW/RENDER_STALL/ …. Test-side only, fires only on the timeout path (passing tests unaffected). A companion change to the offline analyzer (e2e-failure-analysis, separate repo) parses this and reports a per-cause breakdown of nightly failures.This already proved its worth: with the sync-client fix in place, the residual failures under load classify as
EDITOR_NO_PREVIEW— a different stage than theCONNECT_STALLthis PR fixes. Concrete proof that more causes remain, now measured rather than guessed.Validation (local, induced CPU contention to mimic CI's 2-core load)
quarto-sync-clientunit suite: 102 pass (no regression).What this does not fix
EDITOR_NO_PREVIEWcause (preview never activates after the project loads) is untouched — that's the next investigation, and the new[smoke-diag]data is how we'll scope it.extensions/_quarto.yml), inflating the slow-doc odds. Re-scoping that, eager retry-on-peer-arrival for unavailable docs, and loading non-active docs lazily are noted as follow-ups in the plan.Notes for review
claude-notes/plans/2026-06-19-smoke-all-e2e-deflake.mdcarries the full investigation trail and measurements.e2e-failure-analysis— its value kicks in once this PR's[smoke-diag]line is onmainand nightlies accumulate.