diff --git a/packages/studio/tests/e2e/edit-accuracy/case.mjs b/packages/studio/tests/e2e/edit-accuracy/case.mjs index bc96bd492e..5fb2dc36d3 100644 --- a/packages/studio/tests/e2e/edit-accuracy/case.mjs +++ b/packages/studio/tests/e2e/edit-accuracy/case.mjs @@ -19,6 +19,7 @@ import { visibleQuad, } from "./geometry.mjs"; import { frameSamplerScript, scoreTeleport, startFrames, stopFrames } from "./teleport.mjs"; +import { terminateWindowsProcessTree } from "../../../../cli/src/utils/processTree.ts"; export const VIEWPORT = { width: 1600, height: 900 }; const STEPS = 20; @@ -39,6 +40,12 @@ const up = (port) => const liveServers = new Set(); /** Signals the server's process group; a group that already exited is not an error. */ function signalGroup(child, signal) { + // Windows has no process groups, so taskkill /T ends the server and its children. + if (process.platform === "win32") + return void terminateWindowsProcessTree(child.pid).catch((error) => { + // taskkill exits 128 when the process is already gone. + if (!/status 128$/.test(error.message)) throw error; + }); try { process.kill(-child.pid, signal); } catch (error) { @@ -52,16 +59,16 @@ export function killServers() { const announcedPort = (log) => /http:\/\/localhost:(\d+)/.exec(log.join(""))?.[1]; +/** Starts Studio at `port` or, when that is busy, the next free one the CLI binds; returns the port it serves. */ // fallow-ignore-next-line complexity export async function startServer(cli, dir, port, log, home) { - // The CLI quietly takes the next free port when asked for a busy one, so only the port it announces counts. - if (await up(port)) throw new Error(`port ${port} is already serving`); const child = spawn( "node", [cli, "preview", dir, "--port", String(port), "--no-open", "--foreground", "--force-new"], { stdio: ["ignore", "pipe", "pipe"], detached: true, + windowsHide: true, // A per-case HOME keeps Studio's undo history inside the case's tmp dir. env: { ...process.env, @@ -78,12 +85,8 @@ export async function startServer(cli, dir, port, log, home) { for (const deadline = Date.now() + 60_000; Date.now() < deadline; await sleep(200)) { if (child.exitCode !== null) throw new Error(`studio exited ${child.exitCode}: ${log.join("").slice(-500)}`); - const announced = announcedPort(log); - if (announced && announced !== String(port)) { - await stopServer(child); - throw new Error(`studio moved from port ${port} to ${announced}`); - } - if (announced && (await up(port))) return child; + const announced = Number(announcedPort(log)); + if (announced && (await up(announced))) return { child, port: announced }; } await stopServer(child); throw new Error("studio did not start in 60s"); diff --git a/packages/studio/tests/e2e/edit-accuracy/run.mjs b/packages/studio/tests/e2e/edit-accuracy/run.mjs index 39c9b843a2..5ea5866b33 100644 --- a/packages/studio/tests/e2e/edit-accuracy/run.mjs +++ b/packages/studio/tests/e2e/edit-accuracy/run.mjs @@ -112,13 +112,14 @@ async function runOne(spec, browser, decoder, port) { let result; let server; try { - server = await startServer(opt.cli, dir, port, log, join(root, "home")); + const served = await startServer(opt.cli, dir, port, log, join(root, "home")); + server = served.child; const { keyRender, ...measured } = await (spec.steps ? runSequence : runCase)({ browser, spec, dir, files, - url: `http://127.0.0.1:${port}/#project/case`, + url: `http://127.0.0.1:${served.port}/#project/case`, evidence, }); await stopServer(server); diff --git a/packages/studio/tests/e2e/edit-accuracy/server.test.mjs b/packages/studio/tests/e2e/edit-accuracy/server.test.mjs new file mode 100644 index 0000000000..daa9e7b805 --- /dev/null +++ b/packages/studio/tests/e2e/edit-accuracy/server.test.mjs @@ -0,0 +1,28 @@ +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; +import { startServer, stopServer } from "./case.mjs"; + +// Stands in for the CLI when the asked-for port is busy: it binds another port and announces that one. +const FAKE_CLI = ` +const server = require("node:http").createServer((req, res) => res.end("[]")); +server.listen(0, "127.0.0.1", () => console.log("Studio: http://localhost:" + server.address().port)); +`; + +describe("starting the bench's Studio server", () => { + it("serves on the port the server bound, not the one asked for", async () => { + const root = mkdtempSync(join(tmpdir(), "hf-bench-server-")); + const cli = join(root, "cli.cjs"); + writeFileSync(cli, FAKE_CLI); + const asked = 1; + const { child, port } = await startServer(cli, root, asked, [], root); + try { + expect(port).not.toBe(asked); + expect((await fetch(`http://127.0.0.1:${port}/api/projects`)).ok).toBe(true); + } finally { + await stopServer(child); + rmSync(root, { recursive: true, force: true }); + } + }); +});