Skip to content

test(browser): fix three flaky journeys seen across open PRs - #2957

Merged
baxen merged 1 commit into
mainfrom
mbot/ci-flakes-1106
Nov 7, 2026
Merged

baxen merged 1 commit into
mainfrom
mbot/ci-flakes-1106

Conversation

@baxen

@baxen baxen commented Nov 7, 2026

Copy link
Copy Markdown
Collaborator

Summary

These are fixes for three browser-journey flakes that are failing open PRs which don't touch the code under test. The change is test-only. No browser cases are added or removed, and no assertion is relaxed.

The red Browser journeys (chromium, 3/9) / (webkit, 4/14) shards on most open PRs come from a different problem: sidebar-geometry.spec.mjs:224 is broken on main. #2945 and #2946 already fix it, so this PR leaves it alone.

1. channel-completion.spec.mjs: cold dev-server load timed out

Evidence: Flake check on #2935 failed channel-completion.spec.mjs:7 on both engines with page.goto: Timeout 15000ms exceeded on /tests/fixtures/mentions.html?channels&stream&test-controls (chromium, webkit). #2935 doesn't touch that page.

Cause: this is the same cold load #2776 fixed for mentions.spec.mjs. Each test here started its own Vite dev server for the same ~1,600-module fixture.

Fix:

  • beforeAll builds mentions.html once and serves it with build/preview, using the same config as mentions.spec.mjs.
  • The build keeps the dev server's CSS. Tailwind's optimizer and Vite's CSS minifier each round the 20/14 line-height token to 1.42857, so line boxes become 19.99998px. In WebKit that put the popup 4.03125px above the composer, and the test asserts exactly 4. I confirmed this locally: it fails 5/5 with either setting left on and passes with both off. Turning both off keeps the geometry the test checked before.

2. profiles.spec.mjs:940: reopening Members right after closing the profile

Evidence: Flake check (webkit, 8/9) on #2909 failed in repeat 8 of 10 (job). The trace shows this sequence:

  1. Escape closes the profile panel.
  2. Focus returns to "Manage members of One" within 40ms.
  3. Enter is pressed 10ms later.
  4. The DOM after that Enter shows the Profile panel again and no Members dialog, so the next row.click times out.

Fix: after each Escape, wait for the address to leave the profile (no "detail" in the #buzz= hash) before reopening Members. This is a lifecycle barrier, not a delay. One step in this test already waited for the panel to unmount before reopening; the other steps didn't wait at all.

Product follow-up (not fixed here): the order of a dismissal's history return and a quick reopen matters.

  • If I delay history.go by 300ms, the unpatched test takes a different path: the dismissal's go(-1) never runs, and the URL stays on the profile visit under the reopened dialog.
  • A browser-history unit sketch (deferred traversal: push(channel), push(panel), dismiss([panel]), then push(settings) before the traversal commits) lands on channel. The new settings visit is undone.

Someone who owns navigation should decide whether a push during a pending dismissal return should be deferred or replayed. That is a navigation-history change, not a test fix.

3. timeline.mjs edge(): one wheel gesture stopped short

Evidence: Browser journeys (webkit, 6/14) on #2937 failed history-prefix.spec.mjs:37 with wheel reaches timeline edge, Received: 199 (job). The trace shows the one -3251px wheel ended with scrollend and the geometry then held still, so the poll could never pass.

Fix: edge() repeats the gesture, at most four times, until the edge is reached. Every extra gesture must reduce the distance, so an immobile timeline still fails. upper() already follows this rule. The case of already being at the edge is unchanged: it still sends one small wheel so production can start paging.

Not in this PR, from the same survey of the 40 most recently updated open PRs

Validation

All runs were local on macOS with tests/browser/playwright.ci.config.mjs and --no-deps. These are not hosted CI measurements.

  • channel-completion.spec.mjs, --repeat-each=5, both engines: 20/20 passed with the change. Before the CSS settings were added, the built fixture failed WebKit 5/5 (4.03125).
  • profiles.spec.mjs:940, --repeat-each=24 --workers=6, WebKit: passed. It also passed with history.go/back/forward delayed by 300ms, in both engines.
  • history-prefix, history-loading, scroll, timeline-setup and new-message-boundary (all callers of edge()/end()), both engines: 70/70 passed.
  • Full history-prefix, channel-completion and profiles files, both engines: 43/44 passed. The one failure was profiles.spec.mjs:6 in WebKit, a test this PR doesn't touch. It then passed 4/4 both with and without this change.

Deferred and remaining gaps:

  • I couldn't reproduce the cold-load timeout or the WebKit stopping short locally, so I have no local fail-before for those two. The evidence is the CI traces linked above.
  • The other edge() callers (24 spec files) are left to CI.
  • A green run doesn't prove these flakes are gone.

- channel-completion: build the mentions fixture once and serve the
  bundle, as mentions.spec.mjs does (#2776). Each test started its own
  dev server, and the first cold navigation hit the 15s timeout on both
  engines in #2935's flake check. The build keeps the dev server's
  unrounded CSS: Tailwind's optimizer and Vite's CSS minifier round the
  20/14 line-height to 1.42857, moving the popup 1/32px off the 4px gap
  WebKit measures.
- profiles: after Escape closes the profile panel, wait for the address
  to leave the profile before reopening Members. Focus returns before
  the panel's history return lands; in #2909's WebKit flake check the
  reopened dialog was replaced by the profile panel.
- timeline edge(): repeat the edge gesture while it makes progress. One
  Linux WebKit wheel ended 199px short of the top in history-prefix
  (#2937); each extra gesture must still move, so an immobile timeline
  fails.

Co-authored-by: mbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@buzz.block.builderlab.xyz>
Signed-off-by: Bradley Axen <baxen@squareup.com>
@baxen
baxen marked this pull request as ready for review November 7, 2026 02:18
@baxen
baxen requested a review from a team as a code owner November 7, 2026 02:18
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 7, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-07T02:20:47.287365Z 6305c03 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6305c03fd9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@@ -1,86 +1,167 @@
import { test, expect } from "./playwright-test.mjs";
import { createServer } from "./vite-server.mjs";
import { build, preview } from "./vite-server.mjs";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Add the missing Signed-off-by trailer

When this commit is submitted in a PR, the required hosted DCO Check will fail because the commit message contains no Signed-off-by trailer. Re-create the commit with git commit --signoff so every commit carries the required attribution.

AGENTS.md reference: AGENTS.md:L268-L273

Useful? React with 👍 / 👎.

@baxen
baxen merged commit bdc015f into main Nov 7, 2026
59 checks passed
@baxen
baxen deleted the mbot/ci-flakes-1106 branch November 7, 2026 02:30
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.

2 participants