Skip to content

test(studio): kill and retry a test Chrome that hangs after it starts listening - #7337

Merged
miguel-heygen merged 2 commits into
mainfrom
test/studio-chrome-launch-deadline
Nov 6, 2026
Merged

miguel-heygen merged 2 commits into
mainfrom
test/studio-chrome-launch-deadline

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Nov 6, 2026 •

Copy link
Copy Markdown
Collaborator

What

A studio test Chrome that hangs after it starts listening is killed at the 20 s launch limit and launched once more, instead of holding the test until its 60 s timeout.

Also carries a doc-comment correction on createSessionInitBudget (from #7333): each setup step is bounded by the budget on its own, not setup as a whole. No code change there.

Why

On a Windows runner, chromeTestUtils.browser.test.ts timed out at 60 s. Its log shows Chrome printing DevTools listening on ws://... 2.5 s after launch, then nothing: puppeteer.launch never returned, and the launcher never logged a failure or a retry.

Puppeteer's timeout only bounds the wait for that DevTools line. The CDP setup that follows (CdpBrowser._create) is bounded by protocolTimeout, 180 s by default, so the launcher's 20 s retry never fired for a hang in that window.

Related work

Supersedes #7333 (its one comment change is folded in here). Refs #7329, #5961.

How

Each launch attempt gets an AbortController aborted at LAUNCH_TIMEOUT_MS, passed as puppeteer's signal. Aborting kills the Chrome process; the pending CDP calls then reject, the attempt has run for 20 s so it counts as stuck, and the existing retry runs. The timer is cleared when the attempt ends, so a Chrome that launched is never aborted later.

The engine's test launcher (packages/engine/src/services/__fixtures__/loopback.ts) has no retry at all; it is left as it is here.

Test plan

  • Unit tests added/updated: kills and retries a launch that hangs after Chrome announced its DevTools address, with a fake launch that only settles when its signal aborts.
  • Fails without the fix: with chromeTestUtils.ts from main, kills and retries a launch that hangs after Chrome announced its DevTools address fails with Error: Test timed out in 5000ms (1 failed, 7 passed). With the fix: 8 passed, 3 runs in a row.
  • Real Chrome on Linux with the fix: chromeTestUtils.browser.test.ts passed, launch 1/2 ready after 92 ms.
  • oxfmt and oxlint on the changed files; studio tsc --noEmit clean.
  • Not reproduced on a Windows runner: the hang is intermittent there; this PR's Windows shards are the check.
  • Comments follow CONTRIBUTING.md "Comments".

Size

Small on purpose: one test-launcher fix and the comment correction it absorbed. No open PR touches the studio test launcher, so there is nothing to fold it into.

@heygengenesis heygengenesis Bot 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.

Approve at 46bfb16c. I found no blockers.

Fix. signal is a real launch option in puppeteer-core 25.11.0 (the version in bun.lock). ChromeLauncher.launch hands it to @puppeteer/browsers' Process. That registers abort → kill(), so aborting kills Chrome even while CdpBrowser._create is still waiting. Once the socket closes, the pending CDP calls reject. The catch then sees elapsed >= LAUNCH_TIMEOUT_MS and the existing retry runs. clearTimeout in finally means a Chrome that launched successfully is never aborted later. Each attempt has its own controller, so the worst case is 2 × 20 s, still under the 60 s test timeout.

Fails without the fix (checked). I ran chromeTestUtils.test.ts from the head with chromeTestUtils.ts from main. kills and retries a launch that hangs after Chrome announced its DevTools address fails with Test timed out in 5000ms (1 failed, 7 passed). With the fix, 8 passed in each of 3 runs.

Doc comment. createSessionInitBudget checks setup.idle(0), and createStallClock drops any gap over SUSPEND_GAP_MS (15 s). So a step longer than that is not charged to later steps. The new wording ("bounds each step, not setup as a whole") is accurate.

Nit, not blocking: the "Fails without the fix" line calls the test "the new test" rather than naming it. Putting the test name on that line would make it self-contained.

CI was still running when I reviewed (the Windows studio shards were in progress), with no failures so far.

@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

Edit accuracy: accurate 267 (base branch 267), smooth 132 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (8)

Unstable (2)

  • seqplainkeys-none-px-r0-root-z100: tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo false, teleport true / tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true / tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true
  • seqfastkeys-none-px-r0-root-z100: tracking 0.07, pressJump 0.05, drop 0, reload 0, render 0.02, renderKey -, undo false, teleport true / tracking 0.06, pressJump 0.05, drop 0, reload 0, render 0.02, renderKey -, undo true, teleport true / tracking 0.06, pressJump 0.05, drop 0, reload 0, render 0.02, renderKey -, undo true, teleport true

@miguel-heygen
miguel-heygen added this pull request to the merge queue Nov 6, 2026
Merged via the queue into main with commit d2b170c Nov 6, 2026
138 checks passed
@miguel-heygen
miguel-heygen deleted the test/studio-chrome-launch-deadline branch November 6, 2026 22:05
SanHsien added a commit to SanHsien/hyperframes that referenced this pull request Nov 7, 2026
…-com#7336-heygen-com#7358

Adopt heygen-com#7304 (without CLI renderDiagnostic, needs heygen-com#6725), heygen-com#7331, heygen-com#7332, heygen-com#7334,
heygen-com#7337, heygen-com#7336, heygen-com#7339, heygen-com#7289, heygen-com#5594, heygen-com#6900, heygen-com#7322, heygen-com#7342, heygen-com#7343, heygen-com#7350 (onProgress
only), heygen-com#7352, heygen-com#7348, heygen-com#7344, heygen-com#7353, heygen-com#7356 and heygen-com#7355. Defer heygen-com#5708 (needs heygen-com#6911) and
heygen-com#7345 (READ_VERSION 4 vs 7); heygen-com#7307 not applicable. Re-read 14 moved pins, pin 6
new PRs (monitored 126). Make checkReportFile module-private and suppress the
cross-package closeTestChrome duplicate for fallow.

Co-authored-by: Miguel Ángel <miguel.sierra@heygen.com>
Co-authored-by: SophiaHuangCc <129631224+SophiaHuangCc@users.noreply.github.com>
Co-authored-by: Dante <duanjl.china@gmail.com>
Co-authored-by: Muzzy5150 <muzzy5150yt@gmail.com>
Co-authored-by: ump45nose <52391318+ump45nose@users.noreply.github.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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