Skip to content

test(webui): wait for the page's state in the isolated Chromium suite - #235

Open
modacker wants to merge 1 commit into
webuifrom
fix/isolated-chromium-timeout-budget
Open

modacker wants to merge 1 commit into
webuifrom
fix/isolated-chromium-timeout-budget

Conversation

@modacker

Copy link
Copy Markdown
Collaborator

What broke

Mainline run 39879196689 (SHA 2bd9a2a1) went red on a test timeout, not on an assertion:

FAIL packages/webui/test/integration/isolated-chromium-two-pages.test.ts
  > the web tab's toolbar, on a real page > walks a real page's history and re-reads it
Error: Test timed out in 5000ms.
  ❯ packages/webui/test/integration/isolated-chromium-two-pages.test.ts:614:3

Test Files 1 failed | 212 passed (213), Tests 1 failed | 3296 passed (3297), Verification failed at: test:webui.

Re-running the same SHA passed. Nothing in the product had changed — the case failed on its own arithmetic.

Why it failed

The suite slept 750ms after every navigation, every frame and every repaint. The case that walks a page's history did five of those — 3750ms of its 4412ms asleep. The 5000ms budget it inherited (vitest's default; no testTimeout is set in vitest.oss.config.mjs) therefore left about 588ms of headroom, and a runner a little slower than the one it was measured on went over the line.

Measured on a real Chromium, the same case takes 4503ms here against 4412ms on the green CI run — the machine here is the same speed for this workload, so the local number is the CI number.

The sleeps were also guessing about things the provider reports exactly: whether a page is in the target list, which generation it holds, what title it shows, whether a frame arrived.

What this changes

Every sleep in the file becomes a wait for the state the case actually asserts:

const POLL_MS = 50;
const WAIT_MS = 3_000;

async function waitFor(condition, description): Promise<void> { /* ... */ }
  • The case that failed now waits for the title the page shows, for the generation the provider holds for it, for the page's place in the target list, and for the frame that has arrived.
  • waitFor never passes a condition that did not hold. A condition that throws is the page's own state (a loading page answers a read with an error), so it is asked again, and the last error is reported if the wait ends.
  • A wait that never comes true fails the case at the wait deadline, naming what it waited for — not by running a budget out.

Two things came out of the same file:

  • A pre-existing race in the streaming case. Chrome renders the front page and throttles the ones behind it, so opening both streams at once let the second take the front and the first page produce no picture at all — the case passed on CI by luck and failed here every time. Each page's stream is now opened while that page is the one on screen. The assertion that the two frames stay apart is unchanged.
  • Chrome discovery now knows about macOS, the way the provider's own candidate list does. The file used to probe only Linux paths, so on a developer machine it skipped entirely and could not be checked there at all.

The budgets, and why only some of them

Four cases drive a real browser and measured over 700ms (756 / 844 / 1075 / 1199ms on an idle machine). Those four get a 30s budget — the number the repository's other real-browser suites use (browser-panel-live-navigation.test.ts:23, workspace-browser-tab-acceptance.test.ts:67; webui-dependency-check.test.ts:1422 uses 60s for its own gate). The eight that measured under 250ms keep the default 5s, so a case that suddenly takes seconds still says something.

The teeth are in the waits, not in the budget: WAIT_MS is 3000ms, well inside every budget including the default 5s one, so a failure is always the condition's and never the budget's. The longest wait a healthy page took was about 200ms.

How it was checked

  • Before: the failing case took 4503ms here (4412ms on the green CI run); the file took 30–41s.
  • After: that case takes 849–1075ms; the file takes about 5.2s (CI: 39910ms → ~5200ms). 12/12 passed, four consecutive runs.
  • pnpm --filter @mavis/webui typecheck:test passes.
  • node scripts/run-vitest-suite.mjs webui: this file's 12 cases pass. The group reported 2 failed | 207 passed | 4 skipped (213) — the failures are subprocess-spawning static-analysis checks timing out on a loaded machine (webui-dependency-check.test.ts, webui-role-check.test.ts), and all 148 pass when run on their own. Another group run failed only because dist-webui/ had not been built in that checkout. Neither is this file.
  • The teeth: breaking the provider's back/forward on purpose fails the case in 3767ms with timed out waiting for the back navigation to land; breaking its generation counter fails two more, at 3851ms and 4146ms, with timed out waiting for the navigated page to advance its generation. All three are condition failures with a diagnostic — none is vitest's Test timed out in 5000ms.

Not addressed here

  • Two sibling files carry the same shape and were not touched: packages/webui/test/integration/isolated-chromium-screencast.test.ts has no explicit budget and its "keeps delivering frames" case sleeps 3000+500ms — 3500ms of 5000ms, the closest match to this one — and packages/webui/test/integration/browser-use-real-input.test.ts sleeps 750ms per navigation with no budget (a lower risk). Both also discover Chrome through the Linux-only path list, so they skip on a developer machine.
  • The race between the wait and the action it follows is still a race, in the ordinary sense: the condition says the state has been reached, and the assertion then reads it. A swap in that window is not detected.
  • The file needed no registration change: it is already in test/vitest-suites.json and release/public-source.json.

The suite slept 750ms after every navigation, every frame and every
repaint. The case that walks a page's history spent five of those sleeps
— 3750ms of its 4412ms — asleep, so it blew through the 5000ms budget
on a runner a little slower than the one it was measured on. Nothing in
the product had changed: the case failed on mainline on its own
arithmetic.

Every sleep is now a wait for the state the case asserts: the title the
page shows, the generation the provider holds for it, the page's place
in the target list, the frame that has arrived. A wait that never comes
true fails the case at its own deadline with the state it waited for
named, so the cases fail faster and say more than they did — the sleeps
only ever guessed, and several of them guessed about something the
provider already reports exactly.

The streaming case opens each page's stream while that page is the one
on screen, because Chrome renders the front page and throttles the ones
behind it. Before, the second stream took the front and the first page
produced no picture at all: the case passed on CI by luck and failed
here every time.

The four cases that drive a real browser and measured over 700ms are
given a 30s budget, the number the repository's other real-browser
suites use (browser-panel-live-navigation and
workspace-browser-tab-acceptance; webui-dependency-check uses 60s for
its own gate). The eight that measured under 250ms keep the default
budget, so a case that suddenly takes seconds still says something. The
teeth are in the waits: a broken navigation fails at the 3s wait
deadline with the state it waited for named, not by running a budget
out.

Checked against a real Chromium: the case that failed on mainline went
from 4412ms (4503ms here) to 1075ms, and the file from 39910ms to about
5200ms. Breaking the provider's back/forward on purpose fails the case
in 3767ms with "timed out waiting for the back navigation to land";
breaking its generation counter fails two more, at 3851ms and 4146ms.

The suite's Chrome discovery now knows about macOS, the way the
provider's own candidate list does, so the suite runs — and can be
checked — on a developer machine at all.
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