Skip to content

Commit 81f3f68

Browse files
committed
test(tray): budget the socket-inheritance proof for what it actually does
The test failed on windows-latest at 5159ms. Production allows PowerShell 15s (execFileSync timeout in src/tray/windows.ts), while Bun's default test budget is 5s, and a contended runner lands between the two. The test was failing having done nothing wrong. Raising a budget is not the general answer to a flaky test, and this round is the reason to say so out loud: the sidebar route tests were fixed earlier today by DELETING their real gh spawn, because spawning a binary was incidental to what they claimed. Here the opposite holds. This test launches PowerShell, which launches a Bun child, then rebinds the port to prove the child never inherited the listen socket. The processes are the assertion. There is no version of this proof that fakes them, so the only honest fix is a budget that covers the work. The distinction worth carrying forward is whether the wait is intrinsic to the assertion. It is here; it was not there. The pid-file wait becomes deadline-based rather than a fixed 100x25ms count, and says what went wrong on timeout. A bare false told you the file was missing and nothing about whether PowerShell never started, the child died, or the runner was just slow. No assertion changed: both process.kill liveness checks and the port rebind are untouched.
1 parent cd46a34 commit 81f3f68

1 file changed

Lines changed: 26 additions & 3 deletions

File tree

tests/windows-tray.test.ts

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,22 @@ describe("Windows tray packaging and command safety", () => {
244244
expect(source).not.toContain("Stop-Process");
245245
});
246246

247+
// This test really does launch PowerShell, which really does launch a Bun child, and
248+
// then rebinds the port to prove the child did not inherit the listen socket. Those
249+
// processes ARE the assertion — there is no version of this proof that fakes them.
250+
//
251+
// So the budget has to cover work the test genuinely performs. Production allows
252+
// PowerShell 15s (`execFileSync` timeout in src/tray/windows.ts), while Bun's default
253+
// test budget is 5s; a contended windows-latest runner lands between the two and the
254+
// test fails at ~5.1s having done nothing wrong.
255+
//
256+
// Raising a budget is NOT the general answer to a flaky test. Earlier in this same
257+
// round the sidebar route tests were fixed by DELETING their real `gh` spawn, because
258+
// spawning a binary was incidental to what those tests claimed. The distinction is
259+
// whether the wait is intrinsic to the assertion. Here it is; there it was not.
260+
const PID_FILE_WAIT_MS = 20_000;
261+
const TRAY_LAUNCH_TIMEOUT_MS = 45_000;
262+
247263
test("launches the detached tray host without retaining the proxy listen socket", async () => {
248264
if (process.platform !== "win32") return;
249265
const directory = mkdtempSync(join(tmpdir(), "ocx-tray-inheritance-"));
@@ -267,10 +283,17 @@ describe("Windows tray packaging and command safety", () => {
267283
bun: process.execPath,
268284
cli: childPath,
269285
});
270-
for (let attempt = 0; attempt < 100 && !existsSync(pidPath); attempt += 1) {
286+
const pidDeadline = Date.now() + PID_FILE_WAIT_MS;
287+
while (!existsSync(pidPath) && Date.now() < pidDeadline) {
271288
await Bun.sleep(25);
272289
}
273-
expect(existsSync(pidPath)).toBe(true);
290+
// Name what actually went wrong. A bare `false` here means "the pid file is
291+
// missing" and nothing about whether PowerShell never started, the child died,
292+
// or the runner was simply slow — which is most of the work in diagnosing it.
293+
expect(
294+
existsSync(pidPath),
295+
`tray child never wrote ${pidPath} within ${PID_FILE_WAIT_MS}ms`,
296+
).toBe(true);
274297
childPid = Number(readFileSync(pidPath, "utf8"));
275298
expect(Number.isSafeInteger(childPid) && childPid > 0).toBe(true);
276299
expect(() => process.kill(childPid, 0)).not.toThrow();
@@ -293,7 +316,7 @@ describe("Windows tray packaging and command safety", () => {
293316
}
294317
rmSync(directory, { recursive: true, force: true });
295318
}
296-
});
319+
}, { timeout: TRAY_LAUNCH_TIMEOUT_MS });
297320

298321
test("ships branded multi-size Windows tray icons", () => {
299322
const assets = join(import.meta.dir, "..", "src", "tray", "assets");

0 commit comments

Comments
 (0)