From 616240511105fd9b1cd5c194d0c935c4c0cd2b08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Sat, 3 Oct 2026 11:13:46 -0400 Subject: [PATCH 1/5] test(studio): the edit accuracy bench uses the port its server bound A busy port made the CLI bind the next free one, and the bench then failed the case with "studio moved from port N to N+1" instead of using it. --- .../studio/tests/e2e/edit-accuracy/case.mjs | 12 +++------ .../studio/tests/e2e/edit-accuracy/run.mjs | 5 ++-- .../tests/e2e/edit-accuracy/server.test.mjs | 27 +++++++++++++++++++ 3 files changed, 34 insertions(+), 10 deletions(-) create mode 100644 packages/studio/tests/e2e/edit-accuracy/server.test.mjs diff --git a/packages/studio/tests/e2e/edit-accuracy/case.mjs b/packages/studio/tests/e2e/edit-accuracy/case.mjs index bc96bd492e..4ca714cc36 100644 --- a/packages/studio/tests/e2e/edit-accuracy/case.mjs +++ b/packages/studio/tests/e2e/edit-accuracy/case.mjs @@ -52,10 +52,9 @@ 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"], @@ -78,12 +77,9 @@ 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; + // The CLI announces the port it bound, which may not be the one asked for. + 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..8149ab36ac --- /dev/null +++ b/packages/studio/tests/e2e/edit-accuracy/server.test.mjs @@ -0,0 +1,27 @@ +import { mkdtempSync, 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); + } + }); +}); From fb46c3c3b957229120a9db8295499633bc3332b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Sat, 3 Oct 2026 11:15:58 -0400 Subject: [PATCH 2/5] test(studio): drop a comment the doc line already says --- packages/studio/tests/e2e/edit-accuracy/case.mjs | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/studio/tests/e2e/edit-accuracy/case.mjs b/packages/studio/tests/e2e/edit-accuracy/case.mjs index 4ca714cc36..d7d370ae58 100644 --- a/packages/studio/tests/e2e/edit-accuracy/case.mjs +++ b/packages/studio/tests/e2e/edit-accuracy/case.mjs @@ -77,7 +77,6 @@ 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)}`); - // The CLI announces the port it bound, which may not be the one asked for. const announced = Number(announcedPort(log)); if (announced && (await up(announced))) return { child, port: announced }; } From 7382935101fb529422645847e2cd045f0647e447 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Sat, 3 Oct 2026 11:23:37 -0400 Subject: [PATCH 3/5] test(studio): the bench server test removes its temp dir --- packages/studio/tests/e2e/edit-accuracy/server.test.mjs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/studio/tests/e2e/edit-accuracy/server.test.mjs b/packages/studio/tests/e2e/edit-accuracy/server.test.mjs index 8149ab36ac..daa9e7b805 100644 --- a/packages/studio/tests/e2e/edit-accuracy/server.test.mjs +++ b/packages/studio/tests/e2e/edit-accuracy/server.test.mjs @@ -1,4 +1,4 @@ -import { mkdtempSync, writeFileSync } from "node:fs"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { describe, expect, it } from "vitest"; @@ -22,6 +22,7 @@ describe("starting the bench's Studio server", () => { expect((await fetch(`http://127.0.0.1:${port}/api/projects`)).ok).toBe(true); } finally { await stopServer(child); + rmSync(root, { recursive: true, force: true }); } }); }); From d18404b8235a0050882bcc0671881c47de67e765 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Sat, 3 Oct 2026 11:50:41 -0400 Subject: [PATCH 4/5] test(studio): the bench stops its Studio server on Windows too Windows has no process groups, so the group kill never reached the server and the stop waited forever (the new server test timed out on windows-latest). taskkill /T, the CLI helper for the same job, ends the tree there. --- packages/studio/tests/e2e/edit-accuracy/case.mjs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/studio/tests/e2e/edit-accuracy/case.mjs b/packages/studio/tests/e2e/edit-accuracy/case.mjs index d7d370ae58..d6332a638d 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,9 @@ 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(() => undefined); try { process.kill(-child.pid, signal); } catch (error) { From b454628c4a71747890e2bdf0957378c1afa930c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Sat, 3 Oct 2026 11:55:39 -0400 Subject: [PATCH 5/5] test(studio): a failed taskkill surfaces, and the server opens no console on Windows --- packages/studio/tests/e2e/edit-accuracy/case.mjs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/studio/tests/e2e/edit-accuracy/case.mjs b/packages/studio/tests/e2e/edit-accuracy/case.mjs index d6332a638d..5fb2dc36d3 100644 --- a/packages/studio/tests/e2e/edit-accuracy/case.mjs +++ b/packages/studio/tests/e2e/edit-accuracy/case.mjs @@ -42,7 +42,10 @@ const liveServers = new Set(); 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(() => undefined); + 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) { @@ -65,6 +68,7 @@ export async function startServer(cli, dir, port, log, home) { { 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,