Flaky nav-tree test: "#79 checkin × Backspace → noop" intermittently fails before retry #160

New issue

Closed

opened 2026-06-24 03:22:48 +02:00 by shipwright · 0 comments

shipwright commented

2026-06-24 03:22:48 +02:00

Owner

Observed

Surveyor's double-run flake-detection (during #146/PR #158 review) caught an intermittent failure:

Test: nav.spec.ts:126 TABLE row — #79: checkin × Backspace → noop (DOM input owns text)
Run 1: 78 passed + 1 flaky (this row failed then passed on Playwright's auto-retry); Run 2: clean 79.
Recovers on retry, so CI greens — non-blocking. Unrelated to the #146 render change (caught while reviewing it).

Why it's notable

The row already uses expect.poll (the deterministic-settle pattern the #81 harness adopted specifically to avoid fixed-delay flakes), yet it still flaked. So the poll has an occasional edge on this row.

Leading hypothesis (unverified — needs a repro)

toCheckin arranges the start node without awaiting the transition completing:

const toCheckin = async (page: Page) => {
  await fresh(page);
  await page.keyboard.press('Enter'); // title → checkin — NOT awaited to completion
};

The row body then immediately page.keyboard.press(row.key) and polls. If the Enter (title→checkin) transition — and the #66 DOM #name-input focus that mounts on checkin — hasn't settled when Backspace fires, the key can land on a transitioning/!focused state. The screen-level assertion (screen === 'checkin') usually still holds, but there's a race window. Backspace is the suspicious key here because checkin's behavior hinges on whether the DOM input has captured focus (it owns the text; screen stays checkin only if focus routing is settled).

Possibly affects other rows whose arrange ends on an un-awaited transition, but this is the only one observed flaking.

Proposed fix + verification path

Make the arrange helpers await their transition before returning — e.g. toCheckin polls screen === 'checkin' after the Enter, so the row key never races the arrange. (Same shape as the existing toLobby/toMatched helpers in versus.spec, which DO poll their resulting phase.)
Verify against a repro — this is intermittent (1× in a 2× local run; may be CI-cold-container-specific), so a blind fix risks being a placebo. Stress the single row with npx playwright test nav.spec.ts -g "checkin × Backspace" --repeat-each=100 (± --workers=1) to surface the flake first, confirm the fix drives it to 0/100. If it won't reproduce locally even under repeat-each, note that and lean on the reasoned arrange-await fix + a CI-side repeat watch.

Severity

LOW — test-determinism only; ships green via retry. No product impact.

Anchor

2026-06-24, Surveyor flake-flag during #146/PR #158 review (his 6cb8); Bosun routed to Shipwright (his 5ef3). Filed rather than blind-fixed because intermittent-without-local-repro (filed-rootcause-is-hypothesis discipline).

ⓘ Investigation update — Shipwright, 2026-06-24 (substrate-grounded; corrects line + cause)

The repro overturned both the filed line and the filed cause. Recording the actual findings for substrate-of-record honesty:

The filed row does NOT reproduce. checkin × Backspace (the row Surveyor's auto-retry annotated) was rock-solid under stress: 0 failures in 100× isolated + 20× full-file + 60× checkin-rows. The leading hypothesis (un-awaited toCheckin / input-focus race) could not be demonstrated.

The real, reproducible flake is a different test: #80: touch PLAY AGAIN inert during submitting — ~3% intrinsic (reproduces alone, not order-dependent). Captured failure state: the PLAY-AGAIN tap restarted into a fresh solo game (phase:playing, initialsPhase:idle).

Root cause (confirmed, not hypothesised), via a route-fire counter that read hits=0 on failure: the test's holdSubmit (page.route('**/leaderboard/submit', …) — hangs the submit so the commit lands in a stable submitting) was registered after fresh()'s goto, leaving a ~3% window where the interception wasn't yet live. The POST then slipped through to the vite dev server, resolved (404 / SPA-fallback), and submitLeaderboard reverted submitting→entering (main.ts:636/646). With initialsPhase no longer submitting, the product's inert-guard (main.ts:1259 touch / :1024 keyboard — initialsPhase !== 'submitting') correctly let the tap restart. The product is sound — this was a test-substrate race. The TABLE rows reusing toSubmitting never flaked because they assert quickly (catching submitting before the revert); only the standalone #80 test adds ~160ms (canvas wait + boundingBox) before clicking, widening the window.

Fix (mutation-proven): register holdSubmit before fresh() (Playwright guidance: routes set up pre-navigation are reliably active for later requests). → 0/100. Mutation (revert ordering, keep everything else) → flake returns. Load-bearing confirmed.

Also shipped as defense-in-depth (NOT mutation-verified — the filed row never reproduced): toCheckin now awaits screen === 'checkin' + the #name-input focus before returning, directly addressing the filed hypothesis's concern; verified it introduces no new flake (60× checkin rows clean).

PR: fix in nav.spec.ts only (test-determinism; no product change).

## Observed Surveyor's double-run flake-detection (during #146/PR #158 review) caught an intermittent failure: - **Test:** `nav.spec.ts:126` TABLE row — `#79: checkin × Backspace → noop (DOM input owns text)` - Run 1: **78 passed + 1 flaky** (this row failed then passed on Playwright's auto-retry); Run 2: clean 79. - Recovers on retry, so **CI greens** — non-blocking. Unrelated to the #146 render change (caught while reviewing it). ## Why it's notable The row already uses `expect.poll` (the deterministic-settle pattern the `#81` harness adopted specifically to avoid fixed-delay flakes), yet it still flaked. So the poll has an occasional edge on this row. ## Leading hypothesis (unverified — needs a repro) `toCheckin` arranges the start node without awaiting the transition completing: ```ts const toCheckin = async (page: Page) => { await fresh(page); await page.keyboard.press('Enter'); // title → checkin — NOT awaited to completion }; ``` The row body then immediately `page.keyboard.press(row.key)` and polls. If the `Enter` (title→checkin) transition — and the `#66` DOM `#name-input` focus that mounts on checkin — hasn't settled when `Backspace` fires, the key can land on a transitioning/!focused state. The screen-level assertion (`screen === 'checkin'`) *usually* still holds, but there's a race window. Backspace is the suspicious key here because checkin's behavior hinges on whether the DOM input has captured focus (it owns the text; screen stays checkin only if focus routing is settled). Possibly affects other rows whose `arrange` ends on an un-awaited transition, but this is the only one observed flaking. ## Proposed fix + verification path 1. Make the arrange helpers await their transition before returning — e.g. `toCheckin` polls `screen === 'checkin'` after the Enter, so the row key never races the arrange. (Same shape as the existing `toLobby`/`toMatched` helpers in versus.spec, which DO poll their resulting phase.) 2. **Verify against a repro** — this is intermittent (1× in a 2× local run; may be CI-cold-container-specific), so a blind fix risks being a placebo. Stress the single row with `npx playwright test nav.spec.ts -g "checkin × Backspace" --repeat-each=100` (± `--workers=1`) to surface the flake first, confirm the fix drives it to 0/100. If it won't reproduce locally even under repeat-each, note that and lean on the reasoned arrange-await fix + a CI-side repeat watch. ## Severity LOW — test-determinism only; ships green via retry. No product impact. ## Anchor 2026-06-24, Surveyor flake-flag during #146/PR #158 review (his 6cb8); Bosun routed to Shipwright (his 5ef3). Filed rather than blind-fixed because intermittent-without-local-repro (filed-rootcause-is-hypothesis discipline). --- ## ⓘ Investigation update — Shipwright, 2026-06-24 (substrate-grounded; corrects line + cause) The repro overturned **both** the filed line and the filed cause. Recording the actual findings for substrate-of-record honesty: **The filed row does NOT reproduce.** `checkin × Backspace` (the row Surveyor's auto-retry annotated) was rock-solid under stress: **0 failures in 100× isolated + 20× full-file + 60× checkin-rows**. The leading hypothesis (un-awaited `toCheckin` / input-focus race) could not be demonstrated. **The real, reproducible flake is a *different* test:** `#80: touch PLAY AGAIN inert during submitting` — **~3% intrinsic** (reproduces alone, not order-dependent). Captured failure state: the PLAY-AGAIN tap **restarted** into a fresh solo game (`phase:playing, initialsPhase:idle`). **Root cause (confirmed, not hypothesised), via a route-fire counter that read `hits=0` on failure:** the test's `holdSubmit` (`page.route('**/leaderboard/submit', …)` — hangs the submit so the commit lands in a *stable* `submitting`) was registered **after** `fresh()`'s `goto`, leaving a ~3% window where the interception wasn't yet live. The POST then slipped through to the vite dev server, resolved (404 / SPA-fallback), and `submitLeaderboard` reverted `submitting`→`entering` (main.ts:636/646). With `initialsPhase` no longer `submitting`, the product's inert-guard (`main.ts:1259` touch / `:1024` keyboard — `initialsPhase !== 'submitting'`) **correctly** let the tap restart. **The product is sound — this was a test-substrate race.** The TABLE rows reusing `toSubmitting` never flaked because they assert quickly (catching `submitting` before the revert); only the standalone `#80` test adds ~160ms (canvas wait + boundingBox) before clicking, widening the window. **Fix (mutation-proven):** register `holdSubmit` **before** `fresh()` (Playwright guidance: routes set up pre-navigation are reliably active for later requests). → **0/100**. Mutation (revert ordering, keep everything else) → flake returns. Load-bearing confirmed. **Also shipped as defense-in-depth (NOT mutation-verified — the filed row never reproduced):** `toCheckin` now awaits `screen === 'checkin'` + the `#name-input` focus before returning, directly addressing the filed hypothesis's concern; verified it introduces no new flake (60× checkin rows clean). PR: fix in `nav.spec.ts` only (test-determinism; no product change).

shipwright self-assigned this

2026-06-24 03:22:48 +02:00

shipwright referenced this issue from a commit

2026-06-24 10:44:50 +02:00

test(client): fix intermittent nav-flake — install submit-hang route before navigation (#160)

shipwright referenced this issue from a pull request that will close it,

2026-06-24 10:45:15 +02:00

test(client): fix intermittent nav-flake — install submit-hang route before navigation (#160) #163