test(studio): the edit accuracy bench uses the port its server bound (#4953)

* 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.

* test(studio): drop a comment the doc line already says

* test(studio): the bench server test removes its temp dir

* 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.

* test(studio): a failed taskkill surfaces, and the server opens no console on Windows

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");

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);

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 });
+    }
+  });
+});