Skip to content

test(#599): fix resize-then-measure race in e2e specs - #602

Merged
BorisTyshkevich merged 1 commit into
mainfrom
test/e2e-resize-race-599
Aug 4, 2026
Merged

test(#599): fix resize-then-measure race in e2e specs#602
BorisTyshkevich merged 1 commit into
mainfrom
test/e2e-resize-race-599

Conversation

@BorisTyshkevich

Copy link
Copy Markdown
Collaborator

What & why

Closes #599. Fixes a resize-then-measure race that made two e2e specs intermittently
flaky under parallel runs — surfaced originally while shipping #587 (phase 2 of #593),
and picked up now because the exact same failure signature hit CI twice in a row on an
unrelated PR (#601, phase 3 of #593) before passing on a third rerun.

Both sites read UI state (a bounding box, a persisted-then-republished data field)
immediately after triggering an async operation (page.setViewportSize, a click that
applies optimistically then commits separately), with no wait for the operation's real
completion signal:

  1. tests/e2e/inspector-dock-layout.spec.js:147setViewportSize resolves as
    soon as the viewport is applied, but the displayed width is recomputed by
    app-shell.ts's reclampInspectorWidth, run from a real window 'resize' event
    listener dispatched asynchronously relative to that resolution. Under parallel
    load, the assertion sometimes read the pre-resize width.
  2. tests/e2e/tile-open-workbench.spec.js:365 — the widen press applies
    optimistically first (runCommand, src/ui/dashboard.ts — its own comment says
    so), while the actual persisted commit is a separate, fire-and-forget
    app.mutateWorkspace(...).then(...) call. The test read the persisted value before
    that commit had necessarily landed.

Both are fixed with expect.poll(...) instead of a bare read on the next microtask —
same expected values, same assertions, just polled until they settle. The second fix
matches the pattern the same file's own later "narrow tile" test already uses for the
identical kind of read (its own "Read defensively" comment).

No production code changed — this is test-infrastructure only. The full audit
#599 asked for (checking both entire spec files for any other resize/state-change
immediately followed by a bare geometry/data read) found no further instance: every
other candidate site is either already polled or is a synchronous local-state read
gated behind a prior assertion that already guarantees the read is safe.

Verification

  • Reproduced the actual CI failure: with the fix reverted, --repeat-each=15 --workers=12 (chromium+webkit) on inspector-dock-layout.spec.js reproduced
    Expected: 320, Received: 500 — the exact signature seen on CI.
  • With the fix restored, the same stress level (--repeat-each=15 --workers=12, 240
    runs) passed twice in a row: 240/240 both times.
  • Additional stress on inspector-dock-layout.spec.js: 3 rounds × 128 = 384/384
    passed (workers 8 and 12).
  • Additional stress on the widen test: 3 rounds × 16 = 48/48 passed at
    --repeat-each=8 --workers=12.
  • Isolated runs, 8× each (both projects): both specs 8/8 pass.
  • Full parallel suite (--project=chromium --project=webkit), run 3×: 414 passed / 4
    skipped
    , 0 failed, every time — matching test(e2e): resize-then-measure race makes inspector-dock-layout and tile-open-workbench flaky under parallel runs #599's originally reported clean baseline.
  • Coordinator independently re-ran the full local gate (check:types, check:arch,
    check:schemas, check:examples, npm test, npm run build) — green — plus 3 spot
    runs of both fixed spec files together — 29/29 passed each time.

Checklist

🤖 Generated with Claude Code

https://claude.ai/code/session_01R32bb4VZGPgNo3tB9iVSKF

… tile-open-workbench specs

Two e2e specs raced an async settle with no wait, both surfaced by #587's
"surface out-of-scope findings" rule and reproduced again on CI for #601.

- inspector-dock-layout.spec.js:155 read `.inspector-host`'s boundingBox()
  immediately after `page.setViewportSize` — but the displayed width is
  recomputed by app-shell.ts's `reclampInspectorWidth`, a real `window`
  'resize' event LISTENER dispatched asynchronously relative to
  `setViewportSize`'s own resolution. Under `--repeat-each=15 --workers=12`
  this reproduced the exact CI signature ("Expected 320, Received 500");
  wrapping the read in `expect.poll` closes it (confirmed: 0 failures across
  the same stress level, run twice, after the fix).

- tile-open-workbench.spec.js:364 read the committed `window.__dashboard()`
  state immediately after `widen.click()` — but the widen press applies
  OPTIMISTICALLY first (`runCommand`, src/ui/dashboard.ts) while the actual
  persisted commit is a separate, fire-and-forget `app.mutateWorkspace` call.
  The geometry assertions right above it are safe (same optimistic doc, no
  gap); only the persisted-state read raced. Same `expect.poll` fix, mirroring
  the pattern the file's own later "narrow tile" test already uses for the
  identical read.

Audited both files in full for the same shape (state/viewport change
immediately followed by a bare geometry or persisted-state read); every other
instance is either already polled or gated behind a prior polling assertion
whose pass already implies the read is safe, so no other site needed a
change. No production code touched — the optimistic-apply-then-commit
behavior powering both races is working as designed.

Verified: full gate green (types/arch/schemas/examples/unit/build); both
specs run 8x in isolation with 0 failures; the fixed assertions stress-tested
at `--repeat-each=15 --workers=12` (chromium+webkit) with 0 failures across
multiple rounds; full parallel suite (`--project=chromium --project=webkit`)
run 3x, 414 passed/4 skipped every time.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R32bb4VZGPgNo3tB9iVSKF

Copy link
Copy Markdown
Collaborator Author

ChatGPT review pass 1

Reviewed head: 573ef7a83f477fb72bdb84dadab07c83c85199a2

Verdict: no actionable findings.

I reviewed the complete two-file PR diff, both full spec files, issue #599, the relevant app-shell.ts resize path, the dashboard.ts optimistic/commit path, the existing defensive poll in the narrow-tile test, and the current base divergence.

  • The poll timeout is the Playwright expect timeout (5 seconds here; no repository override), not the 30-second test timeout. That is sufficient for the intended gaps: the resize handler is a direct, non-debounced style write, and the dashboard assertion waits for the serialized commit/republish path. A missing update remains a hard timeout failure rather than being masked.
  • ?.items?.['t-sales'] ?? null cannot satisfy the expected object. Missing or transient state produces null, so the poll keeps retrying and fails on timeout if the commit never appears.
  • The polls preserve the old successful-test ordering: the prior bare assertions could only pass after observing the same settled values. Subsequent actions therefore do not acquire a new timing dependency.
  • I found no additional unguarded state-change → geometry/persisted-state read of the same class elsewhere in either file.
  • The current PR merge ref passed the canonical Chromium e2e job: 209/209 tests.

The head is five base commits behind current main, but those changes do not overlap these specs or the relevant production paths, and GitHub reports the PR mergeable.

Execution note: direct clone and focused local Playwright execution were not possible in this review runtime because its execution container could not resolve github.com. I inspected canonical GitHub content and CI logs and ran a local semantic check of the optional-chaining fallback.

@BorisTyshkevich
BorisTyshkevich merged commit 51bb190 into main Aug 4, 2026
8 checks passed
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.

test(e2e): resize-then-measure race makes inspector-dock-layout and tile-open-workbench flaky under parallel runs

1 participant