Repository navigation
test: fix WebKit-sensitive browser specs and a slow App.profile case - #3058
Conversation
…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)
wesbillman
left a comment
There was a problem hiding this comment.
🤖 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
left a comment
There was a problem hiding this comment.
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.
…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]>
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 onmainand 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)image-scroll: allow expectAnchor's tolerance after a placeholder shrinks, and check the shrink is large (
tests/browser/image-scroll.spec.mjs)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).< 4px tolerance (timeline.mjs), which still fails if the next paragraph is kept instead. The follow-up asserts thatvisible'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)paused: falsefor 10 s. Diagnostics from run 41331319960 recorded repeatedseeking@0andstalledaround the click. The test setloop = 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.playbackRate = 0instead ofloopso the video plays without reaching its end or seeking; assert the pause withtoHaveJSProperty("paused", true); remove the diagnostic instrumentation.activity-groups: toggle at the settled width, and bound the transition wait (
tests/browser/activity-groups.spec.mjs)toggleActivity, which waited without limit for atransitionrunon the panel's height afterpage.setViewportSize. WebKit delivers the new size 20–100 ms aftersetViewportSizeresolves; 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.innerWidth, then for Virtua to go idle.toggleActivityalso looks for the running height transition once per frame in case itstransitionrunwas 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)Verification
These commits were verified locally on the combined
flakey-mondaybranch before being split into this PR; no tests were re-run on the split branch. CI on this PR validates the split.page.gotobefore 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.🤖 Generated with Claude Code