diff --git a/packages/webui/test/browser/native-runtime.spec.mjs b/packages/webui/test/browser/native-runtime.spec.mjs index 5be9f151..06e8d2cd 100644 --- a/packages/webui/test/browser/native-runtime.spec.mjs +++ b/packages/webui/test/browser/native-runtime.spec.mjs @@ -22,14 +22,49 @@ test.beforeAll(async () => { once(child, "exit").then(([code]) => { throw new Error(`Native host exited ${code}: ${logs}`); }), ]); }); +/** + * Waits for a promise, and says what the host was doing when it does not + * arrive. + */ +async function within(promise, ms, makeError) { + let timer; + try { + return await Promise.race([ + promise, + new Promise((_, reject) => { + timer = setTimeout(() => reject(makeError()), ms); + }), + ]); + } finally { + clearTimeout(timer); + } +} + test.afterAll(async () => { if (!child || child.exitCode !== null || !child.connected) return; const stopped = once(child, "message"); child.send("close"); - const [message] = await stopped; + // The host's own trace of where it is in the shutdown is the only thing + // that says which step stopped answering, so a shutdown that does not + // finish has to carry it. + const [message] = await within( + stopped, + 30_000, + () => new Error(`the native host did not finish closing\n${logs}`), + ); expect(message).toMatchObject({ closed: true, terminals: 0 }); expect(message.exited).toBeGreaterThanOrEqual(6); - await once(child, "exit"); + // Once the host has said it closed, it has nothing left to do and it must + // be gone. Node-pty's Windows console-list helper cannot attach to a + // ConPTY shell, so every terminal it kills leaves a five-second fallback + // timer behind, and that timer used to keep the host alive long after the + // acceptance was over — long enough for this hook to miss its deadline on a + // loaded machine. + await within( + once(child, "exit"), + 2_000, + () => new Error(`the native host did not exit\n${logs}`), + ); }); async function rpc(page, operation, body = operation === "listTerminals" ? {} : undefined) { diff --git a/packages/webui/test/browser/support/native-browser-host.ts b/packages/webui/test/browser/support/native-browser-host.ts index a609f8f7..bb62972e 100644 --- a/packages/webui/test/browser/support/native-browser-host.ts +++ b/packages/webui/test/browser/support/native-browser-host.ts @@ -65,13 +65,34 @@ let closing: Promise | undefined; async function close() { if (closing) return closing; closing = (async () => { + // Each step says where it is before it runs: a shutdown that does not + // finish is otherwise a silence, and the suite cannot tell which step + // stopped answering. + const step = (name: string) => { process.stderr.write(`[native-host] ${name}\n`); }; + step("service.close"); await service.close(); + step("terminals exited"); await Promise.all(exits); + step("apiHost.close"); await assembled.host.apiHost.close(); + step("terminals checked"); if (terminal.list().length !== 0) throw new Error("Native terminal service leaked processes"); + step("data dir removed"); await rm(directory, { recursive: true, force: true }); - process.send?.({ closed: true, terminals: 0, exited: exits.length }); - process.disconnect?.(); + step("reported"); + // The acceptance is over and the host has said so, so it goes: the exit is + // taken here rather than left to the event loop. Node-pty's Windows + // console-list helper cannot attach to a ConPTY shell — the shell has a + // pseudo-console, not a console — so every terminal it kills leaves a + // five-second fallback timer behind in this process. Waiting for that + // timer kept the host alive long after there was nothing left to do, and + // under load that was enough for the suite's shutdown hook to miss its + // deadline. The callback runs once the message has been flushed, so the + // report is not lost by exiting here. + process.send?.({ closed: true, terminals: 0, exited: exits.length }, () => { + process.disconnect?.(); + process.exit(0); + }); })(); return closing; }