test(engine): browser tests kill a Chrome whose close hangs instead of timing out (#5898)

packages/engine/src/services/__fixtures__/loopback.test.ts

@@ -0,0 +1,25 @@
+import { describe, expect, it, vi } from "vitest";
+import type { Browser } from "puppeteer-core";
+import { closeTestChrome } from "./loopback.js";
+
+const fakeBrowser = (close: () => Promise<void>) => {
+  const kill = vi.fn();
+  const disconnect = vi.fn(async () => undefined);
+  const browser = { close, disconnect, process: () => ({ kill }) } as unknown as Browser;
+  return { browser, kill, disconnect };
+};
+
+describe("closeTestChrome", () => {
+  it("kills a Chrome whose close never finishes", async () => {
+    const { browser, kill, disconnect } = fakeBrowser(() => new Promise(() => undefined));
+    await closeTestChrome(browser, 0);
+    expect(kill).toHaveBeenCalledWith("SIGKILL");
+    expect(disconnect).toHaveBeenCalledOnce();
+  });
+
+  it("leaves a Chrome that closed alone", async () => {
+    const { browser, kill } = fakeBrowser(async () => undefined);
+    await closeTestChrome(browser, 60_000);
+    expect(kill).not.toHaveBeenCalled();
+  });
+});

packages/engine/src/services/__fixtures__/loopback.ts

@@ -16,3 +16,23 @@ export function launchTestChrome(executablePath: string | undefined): Promise<Br
     args: ["--no-sandbox", "--disable-dev-shm-usage"],
   });
 }
+
+/** Closes a test Chrome, killing it when close hangs (seen on Windows runners) so afterAll never times out. */
+export async function closeTestChrome(
+  browser: Browser | undefined,
+  timeoutMs = 5_000,
+): Promise<void> {
+  if (!browser) return;
+  let timer: ReturnType<typeof setTimeout> | undefined;
+  const closed = await Promise.race([
+    browser.close().then(
+      () => true,
+      () => false,
+    ),
+    new Promise<boolean>((resolve) => (timer = setTimeout(resolve, timeoutMs, false))),
+  ]);
+  clearTimeout(timer);
+  if (closed) return;
+  browser.process()?.kill("SIGKILL");
+  await browser.disconnect().catch(() => undefined);
+}

packages/engine/src/services/frameCapture-releaseInjectedVideoMedia.browser.test.ts

@@ -8,7 +8,7 @@ import {
   releasableVideoIds,
   releaseInjectedVideoMedia,
 } from "./frameCapture.js";
-import { listenOnLoopback, launchTestChrome } from "./__fixtures__/loopback.js";
+import { closeTestChrome, listenOnLoopback, launchTestChrome } from "./__fixtures__/loopback.js";
 
 const chromePath = resolveHeadlessShellPath();
 const hasBrowser = chromePath !== undefined && spawnSync(chromePath, ["--version"]).status === 0;
@@ -50,7 +50,7 @@ describe.skipIf(!hasBrowser)("releaseInjectedVideoMedia in Chrome", () => {
   });
   afterAll(async () => {
     for (const res of openMedia) res.destroy();
-    await browser?.close();
+    await closeTestChrome(browser);
     await new Promise<void>((resolve) => server.close(() => resolve()));
   });
 

packages/engine/src/services/frameCapture-videoMetadataHints.browser.test.ts

@@ -1,8 +1,9 @@
 import { spawnSync } from "node:child_process";
 import { afterAll, beforeAll, describe, expect, it } from "vitest";
-import puppeteer, { type Browser } from "puppeteer-core";
+import type { Browser } from "puppeteer-core";
 import { resolveHeadlessShellPath } from "./browserManager.js";
 import { applyVideoMetadataHints } from "./frameCapture.js";
+import { closeTestChrome, launchTestChrome } from "./__fixtures__/loopback.js";
 
 const chromePath = resolveHeadlessShellPath();
 const hasBrowser = chromePath !== undefined && spawnSync(chromePath, ["--version"]).status === 0;
@@ -23,15 +24,9 @@ describe.skipIf(!hasBrowser)("applyVideoMetadataHints in Chrome", () => {
   let browser: Browser;
 
   beforeAll(async () => {
-    browser = await puppeteer.launch({
-      executablePath: chromePath,
-      headless: true,
-      args: ["--no-sandbox", "--disable-dev-shm-usage"],
-    });
-  });
-  afterAll(async () => {
-    await browser?.close();
+    browser = await launchTestChrome(chromePath);
   });
+  afterAll(() => closeTestChrome(browser));
 
   it("lays out a video without loaded metadata as it would with it", async () => {
     const page = await browser.newPage();

packages/engine/src/services/videoFrameTransport.browser.test.ts

@@ -4,7 +4,7 @@ import { afterAll, beforeAll, beforeEach, describe, expect, it } from "vitest";
 import type { Browser, Page } from "puppeteer-core";
 import { resolveHeadlessShellPath } from "./browserManager.js";
 import { injectVideoFramesBatch } from "./screenshotService.js";
-import { listenOnLoopback, launchTestChrome } from "./__fixtures__/loopback.js";
+import { closeTestChrome, listenOnLoopback, launchTestChrome } from "./__fixtures__/loopback.js";
 
 const chromePath = resolveHeadlessShellPath();
 const hasBrowser = chromePath !== undefined && spawnSync(chromePath, ["--version"]).status === 0;
@@ -45,7 +45,7 @@ describe.skipIf(!hasBrowser)("video frame transport in Chrome", () => {
     unexpectedFrameRequests = 0;
   });
   afterAll(async () => {
-    await browser?.close();
+    await closeTestChrome(browser);
     server.closeAllConnections();
     await new Promise<void>((resolve) => server.close(() => resolve()));
   });