Skip to content

test: fix WebKit-sensitive browser specs and a slow App.profile case - #3058

Merged
matt2e merged 6 commits into
mainfrom
mtoohey/flake-browser-spec-fixes
Nov 9, 2026
Merged

matt2e merged 6 commits into
mainfrom
mtoohey/flake-browser-spec-fixes

Conversation

@matt2e

@matt2e matt2e commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

Part of a batch of CI flaky-test fixes triaged 6–9 November 2026. Companion PRs: mtoohey/flake-browser-cold-start, mtoohey/flake-browser-animations-settled, mtoohey/flake-timeline-hold-release, mtoohey/flake-rust-tests. This PR is based on main and is independent of the others (no shared files).

Summary

Five independent test-only fixes: four browser specs that were sensitive to WebKit timing or geometry on slow CI runners, and one App.profile vitest journey whose 5 s budget was genuinely too small for the work it does. No product code changes.

Changes

github-checklist: move the pointer off the timeline after opening the PR (tests/browser/github-checklist.spec.mjs)

  • Root cause: "a PR opens on its checklist, with failing checks linked and wrapping in a narrow pane" failed in CI WebKit when a profile preview covered the checklist button. Clicking the PR link leaves the pointer at rest over the timeline. Opening the GitHub panel reflows the timeline under it, and the viewer's avatar (a preview trigger) crosses that point before the panel covers it. A slow runner can hold the avatar there past the hover delay, and the opened preview then intercepts the next click.
  • Fix: move the pointer to the corner after the click, as the next step does anyway. The preview opening on a synthetic hover is intended hover-card behaviour, so the product is unchanged.

image-scroll: allow expectAnchor's tolerance after a placeholder shrinks, and check the shrink is large (tests/browser/image-scroll.spec.mjs)

  • Root cause: "decoded image resizing preserves reading and bottom following across remounts" failed twice in CI WebKit at the toBeLessThan(1) check for the visible row, which received 1.03125. In WebKit the placeholder shrink also measures rows above the viewport; Virtua compensates with whole-pixel scrollTop writes while row geometry is in 1/64 px layout units, so the about-one-pixel residue is rounding, not an anchoring regression (Chromium measured no rows above and stayed within 0.03125 px).
  • Fix: the check uses expectAnchor's < 4 px tolerance (timeline.mjs), which still fails if the next paragraph is kept instead. The follow-up asserts that visible's placeholder is taller than 40 px before it decodes to 10 px, so a wrongly kept paragraph would move the row by over 30 px and the tolerance cannot pass silently if fixture sizes change.

messages: play the inline video at rate 0 instead of looping it (tests/browser/messages.spec.mjs)

  • Root cause: "inline video controls follow playback, hover and keyboard focus" failed in CI WebKit on unrelated branches: after the Pause click the video stayed paused: false for 10 s. Diagnostics from run 41331319960 recorded repeated seeking@0 and stalled around the click. The test set loop = true, and on Linux WebKit (GStreamer) the loop's seek back to 0 during playback can leave the clip re-seeking and stalling, so a Pause landing then leaves the element playing.
  • Fix: set playbackRate = 0 instead of loop so the video plays without reaching its end or seeking; assert the pause with toHaveJSProperty("paused", true); remove the diagnostic instrumentation.

activity-groups: toggle at the settled width, and bound the transition wait (tests/browser/activity-groups.spec.mjs)

  • Root cause: "activity details keep readable text through both handoffs and survive new..." (:423) timed out in CI WebKit inside toggleActivity, which waited without limit for a transitionrun on the panel's height after page.setViewportSize. WebKit delivers the new size 20–100 ms after setViewportSize resolves; rows remeasure and Virtua restores the reading position later. The toggle was clicked during that reflow, and that opening never reported the height transition the helper samples.
  • Fix: after each resize, wait until the page reports the new innerWidth, then for Virtua to go idle. toggleActivity also looks for the running height transition once per frame in case its transitionrun was missed, and if no transition starts within 5 s it rejects with the panel's state instead of waiting out the test.

App.profile: give the dirty-draft Settings journeys the neighbours' 15 s budget (src/app/App.profile.test.tsx)

  • Root cause: "retains a dirty Profile through a same-scope pending/unavailable Settings destination" (:936) timed out at vitest's default 5 s on loaded CI runners. Each Settings navigation renders the whole app again; timed on a loaded machine the journey takes about 5.9 s. Typing the draft one keystroke at a time also re-rendered the app per character.
  • Fix: paste the draft instead of typing it (the test is about the draft's content, not how it was entered) and use the neighbouring tests' 15_000 ms timeout.

Verification

These commits were verified locally on the combined flakey-monday branch before being split into this PR; no tests were re-run on the split branch. CI on this PR validates the split.

  • github-checklist: not reproduced locally (8/8 WebKit passes before the change); the mechanism was confirmed by probing the trigger geometry after the panel opens. After the change the spec passed 20/20 (10 repeats each in WebKit and Chromium).
  • image-scroll: WebKit 10/10 with 1 worker; Chromium 5/5 with 2 workers. The follow-up's :481 test passed 5/5 in WebKit and 3/3 in Chromium (1 worker). In runs with 2–3 workers, 19 more runs timed out in page.goto before reaching the check (local CPU contention; this spec starts a cold dev server per test). None of the runs that reached the check failed it.
  • messages video: not reproduced locally before the change. After it, 10/10 in WebKit and 5/5 in Chromium (1 worker).
  • activity-groups: not reproduced locally before the change. After it, the spec passed 48/48 across WebKit and Chromium. A forced no-transition toggle failed in about 5 s with the panel state.
  • App.profile: the test file passed 10/10 in a loop.

🤖 Generated with Claude Code

matt2e and others added 6 commits November 9, 2026 15:42
…ng the PR

"a PR opens on its checklist, with failing checks linked and wrapping in
a narrow pane" failed in CI WebKit when a profile preview covered the
checklist button. Clicking the PR link leaves the pointer at rest over
the timeline. Opening the GitHub panel reflows the timeline under it,
and the viewer's avatar (a preview trigger) crosses that point before
the panel covers it. A slow runner can hold the avatar there past the
hover delay, and the opened preview then intercepts the next click.

The test now moves the pointer to the corner after the click, as the
next step does anyway. The preview opening on a synthetic hover is the
intended hover-card behaviour, so the product is unchanged.

Verification: not reproduced locally (8/8 WebKit passes before the
change). The mechanism was confirmed by probing the trigger geometry
after the panel opens. After the change the spec passed 20/20 (10
repeats each in WebKit and Chromium).

Signed-off-by: mtoohey <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
(cherry picked from commit 69daf0e30fe84a00df1ed703bc20299cccb1f8cc)
…r shrinks

"decoded image resizing preserves reading and bottom following across
remounts" failed twice in CI WebKit at the `toBeLessThan(1)` check for
the visible row, which received 1.03125.

The check asserts that Virtua keeps the first visible message when its
decoding placeholder shrinks, rather than the next paragraph. Keeping
the paragraph would move the row by most of the placeholder's height.
In WebKit the shrink also measures rows above the viewport. Virtua
compensates for them with whole-pixel scrollTop writes, while row
geometry is in 1/64 px layout units. Measured locally across that
shrink, scrollTop moved from 2785 to 3380 and the row settled 0.96875
px away; Chromium measured no rows above and stayed within 0.03125 px.
So the about-one-pixel residue is rounding, not an anchoring
regression.

The check now uses expectAnchor's `< 4` px tolerance (timeline.mjs),
which still fails if the paragraph is kept instead.

Verification: WebKit 10/10 with 1 worker; Chromium 5/5 with 2 workers.
In runs with 2-3 workers, 19 more runs timed out in `page.goto` before
reaching the check. That came from local CPU contention: the spec starts
a cold dev server per test (see the U1 commit's remaining gap). None of
the runs that reached the check failed it.

Signed-off-by: mtoohey <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
(cherry picked from commit 31ba06d957da293d9ce2f13de83c6d906ab94a6e)
"inline video controls follow playback, hover and keyboard focus" failed
in CI WebKit on unrelated branches: after the Pause click, the video
stayed `paused: false` for 10 s. The diagnostic added for run
41331319960 recorded repeated `seeking@0` events and `stalled` around
the click.

The test set `loop = true` so the 2 s clip kept playing through the
visibility assertions. On Linux WebKit (GStreamer), the loop's seek back
to 0 during playback can leave the clip re-seeking to 0 and stalling,
and a Pause landing then leaves the element playing. The controls under
test only need the element to be playing, not its clock to advance.

- Set `playbackRate = 0` instead of `loop`: the video plays without
  reaching its end or seeking.
- Assert the pause with `toHaveJSProperty("paused", true)`.
- Remove the diagnostic instrumentation, now that the failure is
  explained.

Verification: not reproduced locally before the change. After it, the
test passed 10/10 in WebKit and 5/5 in Chromium (1 worker; the spec
starts its own cold dev server per test, which times out under heavier
local parallelism).

Signed-off-by: mtoohey <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
(cherry picked from commit 3f86261ab8f2062d95481be4f411c4c9c589aaf0)
…nsition wait

"activity details keep readable text through both handoffs and survive
new..." (activity-groups.spec.mjs:423) timed out in CI WebKit inside
`toggleActivity`, which waited without limit for a `transitionrun` on
the panel's height after `page.setViewportSize`.

WebKit delivers the new size to the page 20-100 ms after
`setViewportSize` resolves. The rows then remeasure and Virtua restores
the reading position in later rendering updates. The toggle was clicked
during that reflow, and in the CI trace, that opening never reported the
height transition the helper samples. Nothing ended the wait, so the
test ran out its 120 s budget with no hint of why.

- After each resize, wait until the page reports the new `innerWidth`,
  then for Virtua to go idle, so the toggle happens at the settled
  width.
- `toggleActivity` also looks for the running height transition once
  per frame, in case its `transitionrun` was missed. If no transition
  starts within 5 s, it rejects with the panel's state (connected,
  expanded, attributes, height, transition) instead of waiting out the
  test.

Verification: not reproduced locally before the change. After it, the
spec passed 48/48 across WebKit and Chromium. A forced no-transition
toggle failed in about 5 s with the panel state.

Signed-off-by: mtoohey <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
(cherry picked from commit e11e2769d6ea2443767011fdd498a7c1f4f68650)
… 15 s budget

"retains a dirty Profile through a same-scope pending/unavailable
Settings destination" (App.profile.test.tsx:936) timed out at vitest's
default 5 s on loaded CI runners.

Each Settings navigation in the journey renders the whole app again.
Timed on a loaded machine: setup 1.0 s, open Profile 1.3 s, Back 1.0 s,
clear 0.5 s, Back 1.1 s, about 5.9 s in total. So the budget is
genuinely too small for the work, as it already was for the
neighbouring tests, which use 15 s. Typing the draft one keystroke at a
time also re-rendered the app per character.

- Paste the draft instead of typing it: the test is about the draft's
  content, not how it was entered.
- Use the neighbouring tests' 15_000 ms timeout.

Verification: the test file passed 10/10 in a loop.
Signed-off-by: mtoohey <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
(cherry picked from commit 2a92bdfd96fe3cdaf3427a66d2ca69a8d6760ceb)
…he anchor tolerance

31ba06d957 widened the check that the visible row stays put after its
placeholder decodes to expectAnchor's `< 4` px. That tolerance separates
WebKit's rounding from an anchoring regression only while the
placeholder shrinks by much more than 4 px: keeping the paragraph after
it would move the row by about the shrink. The spec checked the height
of a different image (`beforeHeight > 10`), not `visible`'s.

Assert that `visible`'s placeholder is taller than 40 px. The image
decodes to 10 px, so a wrongly kept paragraph would move the row by over
30 px. The placeholder is about 320 px in both engines today. If fixture
sizes change, the test fails here instead of the tolerance passing
silently.

Verification: the :481 test passed 5/5 in WebKit and 3/3 in Chromium
(1 worker).

Signed-off-by: mtoohey <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
(cherry picked from commit 51c260c80311b72bd2423c5e4faa8a01ad0ca09d)
@matt2e
matt2e requested a review from a team as a code owner November 9, 2026 04:45

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Star Lord’s automated source review via Wes’s account, with independent Rocket, Mantis and Groot coverage.

No actionable findings at 644d98d6ae33ef1da8cc753924894b911bed3066 against base/merge-base a905c4865f1ebc6672f71e1bc65cc94cfc84054d.

The five test changes preserve their targeted contracts: activity still samples real transition/readability states; checklist interaction assertions remain; the >40 px placeholder guard keeps incorrect paragraph anchoring well outside the <4 px tolerance; rate-zero video retains play/pause, hover and focus assertions; and the profile journey retains the same draft and field identity through both unavailable and pending destinations. No browser cases or engine coverage were removed.

Read-only CI evidence: run 42563529284 succeeded, including required browser shards. Its synthetic merge ecd749ce combines the reviewed head and base. Artifact reports show six selected browser journeys passing ten repetitions per engine (120 passes, no failures/skips), both profile variants passing ten repetitions each, and the full profile file passing 24/24. Selected browser shard wall times were 150–223 s, summed test execution 229–329 s per shard; the slowest selected case was image resize at 21.819 s in WebKit. Profile repetitions took 1.683–4.303 s each. No comparable before-run timing baseline was inspected.

Source-only review: we ran no tests or apps and did not independently reproduce the reported flakes. Author-reported local runs used the combined branch; the hosted evidence above is from this split PR. Passing repetitions do not establish complete flake elimination.

@kazemek21 kazemek21 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kyle reviewed this PR and the Nexus review-tracker notes and approved without comments.

Posted by Codex (gpt-6.1-sol) on behalf of kylez.

@matt2e
matt2e merged commit 254a5f5 into main Nov 9, 2026
46 checks passed
@matt2e
matt2e deleted the mtoohey/flake-browser-spec-fixes branch November 9, 2026 21:51
matt2e added a commit that referenced this pull request Nov 14, 2026
…fore the Play click (#3446)

## Why

`composer-attachments.spec.mjs`, "local previews stay interactive during
eager upload and removal collapses the composer", hung once in CI: [11
Nov, Flake check, WebKit
7/9](https://github.com/block/buzz-app/actions/runs/43114573078/job/130840308592),
passed on re-run. The "Play video" click stayed in "performing click
action" past its 10 s action timeout, the test ran to its 120 s timeout,
and the error context holds no page snapshot.

This PR is a mitigation. The root cause is not pinned down.

## What the trace shows

The test pauses the review viewer's autoplaying 2 s clip with `loop =
true; pause()` and then clicks "Play video". The failed attempt's trace
(artifact 13898342784) shows:

- the viewer opening at 12.07 s and the clip at 1.250 s in the dialog
screenshot at 12.52 s;
- a blank stage with the clip reset to 0 at 13.17 s under the same blob
source;
- the clip playing at 1.417 s at 14.08 s, the `loop = true; pause()`
evaluate at 14.11 s (paused at 1.583 s), and the Play click at 14.13 s;
- a last screencast frame at 14.40 s with the clip back on its first
frame. No frame followed in the remaining 110 s and the click never
returned.

The clip was mid-way and never reached its end after `loop` was set, so
a loop seek did not occur. The page froze when the click resumed the
paused pipeline. The exact WebKit mechanism is unknown.

## Change

Set `playbackRate = 0` instead of `loop`, as `messages.spec.mjs` does
since #3058. A zero-rate play turns `paused` false without advancing the
clock, so the Play click leaves the clip on its paused frame and the
clip can neither end nor loop while the eager-upload assertions run. The
Pause button, `paused`, same-element and same-source assertions are
unchanged.

The second commit corrects the code comment, which first attributed the
stall to the loop seek. It now says the cause is not pinned down and
that rate 0 removes both the end and the loop seek.

No timeouts or assertions were relaxed. No browser cases were added or
removed.

## Evidence

Local runs on Linux, WebKit, `-g "local previews stay interactive"`.

| Scope | Unchanged test | With rate 0 |
|---|---|---|
| Idle host, `--workers=1 --repeat-each=10` | 10/10 passed, 12.5 to 13.0
s | 10/10 passed, 13.0 to 17.2 s |
| Pinned to 4 CPUs shared with 5 busy loops, `--repeat-each=6` | 6/6
passed, 24.9 to 31.2 s | 6/6 passed, 26.5 to 31.9 s |
| Whole file, `--workers=2` | | WebKit 10/10, Chromium 10/10 |

A scratch copy that waited for `ended` before taking over passed 3/3
with rate 0 and 3/3 with `loop`, so the ended path needs no extra seek.
After the comment correction, `user-status.spec.mjs` and this spec
passed 11/11 together on WebKit.

## Not reproduced, not run

- The stall was not reproduced locally, with or without the change. The
contended runs matched the 27 to 29 s of the job's passing repeats and
still passed.
- Because the mechanism is unknown, this change removes two candidate
triggers but is not shown to remove the hang. The test needs watching
after it lands.
- No human has tested this yet, so `buzz-review-completed` is not set.

## Left alone

- The earlier reset of the same video element at about 13 s in the
failing run.
- This spec's cold per-test dev server.
- Open PR #3097 edits other parts of `composer-attachments.spec.mjs`. A
trial merge against this change is clean.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Signed-off-by: mtoohey <[email protected]>
Signed-off-by: Matt Toohey <[email protected]>
Co-authored-by: Claude Fable 5.1 <[email protected]>
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.

3 participants