beyhan/airship
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

test(server): wait for the reply, not for the clock

Browse Source 
The windows leg failed on `admits a same-origin page` — `expected false to
be true` against `reply.startsWith("HTTP/1.1 101 ")`. Not the Origin gate:
`originMatchesHost` is URL parsing and cannot differ by platform. The
reply simply had not arrived yet.

Every case here read until a fixed 600ms `HOLD_OPEN_MS` elapsed, because a
completed upgrade never closes and something has to end the read. That is
right for a refusal, where the silence *is* the assertion — but for a case
asserting that something arrives it is a race, and on the windows runner
all four of those sat at 603-628ms against the 600ms budget. One of them
lost by 28ms. The others passed by three.

What costs the time is `git:health`, written beside `hello`: it spawns git
against a cwd that is deliberately not a repository, and two process
spawns on a cold Windows runner do not fit in a few hundred milliseconds.

So `exchange` takes an optional `until` and stops the moment the awaited
bytes are in. The ceiling for those is 10s and is only ever reached when
the server is genuinely broken, so it can be generous without costing
anything; `HOLD_OPEN_MS` stays exactly as it was for the refusals, which
still need a quiet window rather than an early return — returning early
there would make "and nothing followed" vacuous.

The refusals never actually paid it: the server closes a refused socket,
so they resolve on `close`. The six that paid it were the four
101/hello cases and the two 200s, none of which close on their own, and
all six now name what they are waiting for. Package test time drops from
4.2s to 128ms as a side effect.
This commit is contained in:
Nayan
2026-08-16 13:05:37 +05:30
parent 49549c21e3
commit 1392c3b461
1 changed files with 56 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/server/src/server.integration.test.ts
+56 -13
View File
@@ -20,7 +20,10 @@ import { denyResponse } from "./access";
import { type RunningServer, startServer } from "./index";
const WS_KEY = "dGhlIHNhbXBsZSBub25jZQ==";
/** How long a refused socket is watched to prove nothing follows the refusal. */
const HOLD_OPEN_MS = 600;
/** Ceiling on waiting for a reply that should arrive; only hit when broken. */
const REPLY_CEILING_MS = 10_000;
/** The first non-loopback IPv4 on this machine, when it has one. */
const lanAddress = Object.values(os.networkInterfaces())
@@ -58,26 +61,58 @@ async function startUpstream(): Promise<Upstream> {
}
/**
* Write one raw request, collect every byte until the peer closes. A
* completed upgrade never closes on its own, so the socket is destroyed
* after a beat and whatever arrived — status line, headers, the first
* WebSocket frames — is what gets asserted on.
* Write one raw request, collect every byte until the peer closes. A completed
* upgrade never closes on its own, so something has to end the read.
*
* `until` is what ends it for a test asserting that something *arrives*: the
* read stops the moment the awaited bytes are in, so the assertion never races
* a wall clock. Without it the read runs the full `HOLD_OPEN_MS`, which is what
* a test asserting that something *never* arrives needs — there, the silence is
* the assertion, and returning early would make it vacuous.
*
* The two are separate constants because they are paid at opposite times. The
* quiet window is paid on every negative case; the ceiling is only ever reached
* when the server is genuinely broken, so it can be generous. It has to be:
* `hello` is written beside `git:health`, which spawns git against a cwd that
* is not a repository, and two process spawns on a cold Windows runner do not
* fit in a few hundred milliseconds. A fixed 600ms hold made every positive
* case here a coin flip on that runner, and `admits a same-origin page` is the
* one that lost it.
*/
function exchange(port: number, request: string): Promise<string> {
function exchange(
port: number,
request: string,
until?: (data: string) => boolean
): Promise<string> {
return new Promise((resolve, reject) => {
const socket = net.connect({ host: "127.0.0.1", port }, () => {
socket.write(request);
});
let data = "";
const timer = setTimeout(
() => socket.destroy(),
until ? REPLY_CEILING_MS : HOLD_OPEN_MS
);
timer.unref();
socket.on("data", (chunk: Buffer) => {
data += chunk.toString("latin1");
if (until?.(data)) {
socket.destroy();
}
});
socket.on("error", reject);
socket.on("close", () => resolve(data));
setTimeout(() => socket.destroy(), HOLD_OPEN_MS).unref();
socket.on("close", () => {
clearTimeout(timer);
resolve(data);
});
});
}
/** The upgrade completed and the server said its piece. */
function said(...markers: string[]) {
return (data: string) => markers.every((marker) => data.includes(marker));
}
function upgradeRequest(
pathname: string,
headers: Record<string, string>
@@ -157,7 +192,8 @@ describe("the editor server, over raw sockets", () => {
it("admits a client with no Origin at all, and says hello", async () => {
const reply = await exchange(
port,
upgradeRequest("/__airship/ws", { host: `localhost:${port}` })
upgradeRequest("/__airship/ws", { host: `localhost:${port}` }),
said('"type":"hello"')
);
expect(reply.startsWith("HTTP/1.1 101 ")).toBe(true);
expect(reply).toContain('"type":"hello"');
@@ -169,7 +205,8 @@ describe("the editor server, over raw sockets", () => {
upgradeRequest("/__airship/ws", {
host: `localhost:${port}`,
origin: `http://localhost:${port}`,
})
}),
said('"type":"hello"')
);
expect(reply.startsWith("HTTP/1.1 101 ")).toBe(true);
expect(reply).toContain('"type":"hello"');
@@ -181,7 +218,8 @@ describe("the editor server, over raw sockets", () => {
// second before greying it out is worse than one that never offered it.
const reply = await exchange(
port,
upgradeRequest("/__airship/ws", { host: `localhost:${port}` })
upgradeRequest("/__airship/ws", { host: `localhost:${port}` }),
said('"type":"git:health"')
);
expect(reply).toContain('"type":"git:health"');
});
@@ -209,7 +247,8 @@ describe("the editor server, over raw sockets", () => {
it("serves an IP-literal Host — literals cannot be rebound", async () => {
const reply = await exchange(
port,
`GET / HTTP/1.1\r\nhost: 127.0.0.1:${port}\r\n\r\n`
`GET / HTTP/1.1\r\nhost: 127.0.0.1:${port}\r\n\r\n`,
said("HTTP/1.1 200 ")
);
expect(reply.startsWith("HTTP/1.1 200 ")).toBe(true);
});
@@ -222,9 +261,12 @@ describe("the editor server, over raw sockets", () => {
upgradeRequest("/", {
host: `localhost:${port}`,
"sec-websocket-protocol": "vite-hmr",
})
}),
said("HTTP/1.1 101 ")
);
expect(reply.startsWith("HTTP/1.1 101 ")).toBe(true);
// Recorded upstream when it received the upgrade, which is strictly
// before the 101 waited on above was relayed back.
expect(upstream.upgrades).toContain("vite-hmr");
});
@@ -265,7 +307,8 @@ describe("the editor server, over raw sockets", () => {
const widePort = Number(new URL(wide.url).port);
const reply = await exchange(
widePort,
`GET / HTTP/1.1\r\nhost: localhost:${widePort}\r\n\r\n`
`GET / HTTP/1.1\r\nhost: localhost:${widePort}\r\n\r\n`,
said("HTTP/1.1 200 ")
);
expect(reply.startsWith("HTTP/1.1 200 ")).toBe(true);
} finally {