Skip to content

test(e2e): the drawer conformance run samples every step on the page clock and fails an undersampled one - #545

Merged
arrzdev merged 3 commits into
mainfrom
test/drawer-keyboard-min-samples
Nov 2, 2026
Merged

arrzdev merged 3 commits into
mainfrom
test/drawer-keyboard-min-samples

Conversation

@arrzdev

@arrzdev arrzdev commented Nov 2, 2026 •

Copy link
Copy Markdown
Owner

playground/e2e/drawer-keyboard.spec.ts only asserted that each frame-sampled step had at least one sample (> 0, loosened from > 2 after a starved WebKit runner yielded 2 samples for "release 70%→0"). With one or two samples a step cannot show a reversal or an overshoot, so its geometry checks pass without reading anything, and a frame-starved page hides exactly the motion bugs this run exists to catch.

What changes:

  • The harness fails an undersampled step. Every step of /lab/drawer-keyboard gets a geometry check, sampled enough to judge, that fails with undersampled: N samples (needs >=3) below 3 samples. The e2e spec and the throttled stress spec both fail on it, as they do on any other geometry failure.
  • The e2e spec no longer depends on the runner's frame rate. It installs Playwright's page.clock, pauses it before the sheet opens and steps it 16ms at a time. Under ?virtual the harness holds every CSS animation on the page and sets its currentTime from performance.now() on each frame and each DOM change, so the sheet's transitions run on the same virtual timeline as the engine's timers and re-aim maths. Each step now reads 44 or 45 samples on both engines, every run.
  • samples.length >= MIN_SAMPLES (3) is asserted again for every frame-sampled step.
  • The known content-edge overshoot is still reported. On the page clock it reads 844 → 506, turned 13px at 116ms on both engines, and the comment now quotes that figure.
  • docs/design/behaviors.md says how the spec samples.

The harness change is limited to ?virtual; without the flag nothing is held. No product code changes.

Reproduce:

  • On the first commit alone, a step sampled fewer than 3 times is reported undersampled fails: the clock jumps 400ms per frame, the steps get 2 samples, and no check says so (expect(s.check?.pass).toBe(false), received undefined).

Verify:

pnpm --dir playground test:e2e e2e/drawer-keyboard.spec.ts --project=chromium --repeat-each=10
  20 passed (47.0s)
pnpm --dir playground test:e2e e2e/drawer-keyboard.spec.ts --project=webkit --repeat-each=10
  20 passed (1.2m)
pnpm --dir playground test:e2e:stress e2e/stress-drawer-keyboard.spec.ts --project=chromium -g "9 ·"
  1 passed (26.3s): 123 checks, UNEXPECTED geometry failures: none
pnpm gate
  exit 0: biome x2, tsc x2, vitest 5011 passed, check-colour, build:check (publint, attw)

Per-step samples, both engines: 44–45 on every step except "drag with the keyboard up" (not frame-sampled, exempt as before).

Tested on: Linux VM, Playwright 1.61.1 chromium and webkit (iPhone 13 profile).
Not tested: the CI E2E workflow (disabled).

@arrzdev
arrzdev marked this pull request as ready for review November 2, 2026 08:38
@arrzdev

arrzdev commented Nov 2, 2026

Copy link
Copy Markdown
Owner Author

Review: changes requested (one flaky test; the rest checks out).

Blocking: a step sampled fewer than 3 times is reported undersampled fails about 1 run in 4.
playwright test e2e/drawer-keyboard.spec.ts -g undersampled --project=chromium --repeat-each=40 --workers=1: 9 of 40 failed. In the --repeat-each=10 run of the whole file it was 1 of 20. Every failure looks the same: after ~20s (the 5000 iterations of runOnClock), __drawerConformance is still undefined and the test dies on results.map with TypeError: Cannot read properties of undefined (reading 'map'). The page snapshot at that point has the sheet open and no step results on the overlay, so the run never produced its first step.

I did not pin down the cause. A likely one: the run starts from a setTimeout(…, 600) in the page, and run returns silently without publishing when [data-pwa-drawer] > div is not there yet. fastForward(400) fires that timer on the second call regardless of whether the drawer has mounted. Without ?virtual there is also nothing holding the sheet's real-time CSS transition to the stepped clock. Whatever the cause, the test needs to wait for the sheet to be ready before it starts stepping, or step with runFor like the other test does, which passed 10 of 10 here.

Also: when the loop runs out, runOnClock should throw a message ("the run never published after N advances") instead of returning undefined, so the next failure says what happened.

Checked, fine

  • First commit alone (dd9e72b1, before the page change): the undersampled test fails 4 of 4 with Expected: false, Received: undefined at expect(s.check?.pass, s.step), for the right reason (no check reports the 2-sample steps).
  • ?virtual is gated by URLSearchParams(...).has("virtual") inside an effect, and followPageClock?.() is a no-op while it is null, so a run without the flag touches nothing. Looping animations (endTime of Infinity) are never finished. The observer and the rAF loop are never torn down; that is fine for a full page load with the flag, but they would keep holding animations across a client-side navigation away from the lab page. Say so in the comment, or stop them in the effect cleanup.
  • the conformance run passes every geometry check: --repeat-each=10 on chromium, 10 of 10 green, and every step reads 44-45 samples (drag with the keyboard up has 0 and is skipped by the assertion). That matches the "~44 samples" in docs/design/behaviors.md.

Nits

  • docs/design/behaviors.md: the new sentence makes the line run well past the wrap width of the paragraph around it; re-wrap.
  • The comment says the known overshoot is the same on "both engines" on the page clock (turned 13px at 116ms). I only ran chromium, so I did not verify the webkit number.

…starves, and a run that cannot start says so
@arrzdev

arrzdev commented Nov 2, 2026

Copy link
Copy Markdown
Owner Author

Thanks, fixed in the new commit on top.

  • Flaky undersampled test. Your guess was right: a 400ms jump could fire the page's 600ms start timer before [data-pwa-drawer] > div had rendered, and run() returned without publishing. The test now moves the clock in 16ms frames until the sheet content is in the DOM, and only then jumps 400ms per frame.
  • No more silent return. A run that finds no sheet content now publishes a run aborted step (every scenario ran, failed), the same way a scenario that throws does. A reader of __drawerConformance never waits on a run that never began.
  • runOnClock throws the conformance run never published window.__drawerConformance (5000 clock steps) instead of returning undefined.
  • Nit: the long line in docs/design/behaviors.md is re-wrapped.

Verify:

playwright test e2e/drawer-keyboard.spec.ts -g undersampled --project=chromium --repeat-each=40 --workers=1
  40 passed (1.9m)          (before: 9/40 failed, your run)
playwright test e2e/drawer-keyboard.spec.ts --project=chromium --repeat-each=10
  20 passed (45.3s)
playwright test e2e/drawer-keyboard.spec.ts --project=webkit --repeat-each=10
  20 passed (1.2m)
pnpm gate
  exit 0 (vitest 5011 passed)

@arrzdev

arrzdev commented Nov 2, 2026

Copy link
Copy Markdown
Owner Author

Re-review: approve.

The new top commit fixes the flake. On chromium the whole file at --repeat-each=10 --workers=1 gives 20 of 20 passed, and -g undersampled --repeat-each=40 --workers=1 gives 40 of 40 passed (it was 9 of 40 failing). The run aborted step for a run with no sheet content, the clear throw from runOnClock and the re-wrapped behaviors.md line are all as described. Not run by me: webkit.

@arrzdev
arrzdev merged commit 0c3f90c into main Nov 2, 2026
3 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.

1 participant