testkit: reject pending commands when the socket closes; unit tests
commit 1cd98c6430cc05d68983d937809cbd4c1313545c · parent 50823648d1
· browse this tree
Ximianth, 2026-10-10 05:46:38 UTC
Before, a command outstanding when the websocket closed (Chrome died, or close() was called) never settled, and a cmd() sent afterwards silently queued forever. Now close/error on the socket rejects every waiter, and cmd() rejects up front if the socket isn't open. Cdp gains a CdpSocket interface (the slice of WebSocket it actually uses) and a static fromSocket, so the tests can drive it with a scripted fake: no Chrome, no ws dependency. vitest is now part of the testkit's check script; the build excludes *.test.ts. The two close tests were watched failing (the promise never settled) before the fix.
diff --git a/package-lock.json b/package-lock.json index 21b8010..8445a80 100644 --- a/package-lock.json +++ b/package-lock.json @@ -439,6 +439,7 @@ "os": [ "aix" ], + "peer": true, "engines": { "node": ">=18" } @@ -456,6 +457,7 @@ "os": [ "android" ], + "peer": true, "engines": { "node": ">=18" } @@ -473,6 +475,7 @@ "os": [ "android" ], + "peer": true, "engines": { "node": ">=18" } @@ -490,6 +493,7 @@ "os": [ "android" ], + "peer": true, "engines": { "node": ">=18" } @@ -507,6 +511,7 @@ "os": [ "darwin" ], + "peer": true, "engines": { "node": ">=18" } @@ -524,6 +529,7 @@ "os": [ "darwin" ], + "peer": true, "engines": { "node": ">=18" } @@ -541,6 +547,7 @@ "os": [ "freebsd" ], + "peer": true, "engines": { "node": ">=18" } @@ -558,6 +565,7 @@ "os": [ "freebsd" ], + "peer": true, "engines": { "node": ">=18" } @@ -575,6 +583,7 @@ "os": [ "linux" ], + "peer": true, "engines": { "node": ">=18" } @@ -592,6 +601,7 @@ "os": [ "linux" ], + "peer": true, "engines": { "node": ">=18" } @@ -609,6 +619,7 @@ "os": [ "linux" ], + "peer": true, "engines": { "node": ">=18" } @@ -626,6 +637,7 @@ "os": [ "linux" ], + "peer": true, "engines": { "node": ">=18" } @@ -643,6 +655,7 @@ "os": [ "linux" ], + "peer": true, "engines": { "node": ">=18" } @@ -660,6 +673,7 @@ "os": [ "linux" ], + "peer": true, "engines": { "node": ">=18" } @@ -677,6 +691,7 @@ "os": [ "linux" ], + "peer": true, "engines": { "node": ">=18" } @@ -694,6 +709,7 @@ "os": [ "linux" ], + "peer": true, "engines": { "node": ">=18" } @@ -711,6 +727,7 @@ "os": [ "linux" ], + "peer": true, "engines": { "node": ">=18" } @@ -728,6 +745,7 @@ "os": [ "netbsd" ], + "peer": true, "engines": { "node": ">=18" } @@ -745,6 +763,7 @@ "os": [ "netbsd" ], + "peer": true, "engines": { "node": ">=18" } @@ -762,6 +781,7 @@ "os": [ "openbsd" ], + "peer": true, "engines": { "node": ">=18" } @@ -779,6 +799,7 @@ "os": [ "openbsd" ], + "peer": true, "engines": { "node": ">=18" } @@ -796,6 +817,7 @@ "os": [ "openharmony" ], + "peer": true, "engines": { "node": ">=18" } @@ -813,6 +835,7 @@ "os": [ "sunos" ], + "peer": true, "engines": { "node": ">=18" } @@ -830,6 +853,7 @@ "os": [ "win32" ], + "peer": true, "engines": { "node": ">=18" } @@ -847,6 +871,7 @@ "os": [ "win32" ], + "peer": true, "engines": { "node": ">=18" } @@ -864,6 +889,7 @@ "os": [ "win32" ], + "peer": true, "engines": { "node": ">=18" } @@ -2846,6 +2872,9 @@ "packages/yukey-testkit": { "version": "0.0.0", "license": "Apache-2.0 OR MIT", + "devDependencies": { + "vitest": "5.0.3" + }, "engines": { "node": ">=22" } diff --git a/packages/yukey-testkit/package.json b/packages/yukey-testkit/package.json index d81f38c..1795f15 100644 --- a/packages/yukey-testkit/package.json +++ b/packages/yukey-testkit/package.json @@ -30,9 +30,13 @@ ], "scripts": { "build": "tsc -p tsconfig.build.json", - "check": "tsc --noEmit", + "check": "tsc --noEmit && vitest run", "clean": "rm -rf dist", - "prepack": "npm run clean && npm run build" + "prepack": "npm run clean && npm run build", + "test": "vitest run" + }, + "devDependencies": { + "vitest": "5.0.3" }, "engines": { "node": ">=22" diff --git a/packages/yukey-testkit/src/cdp.test.ts b/packages/yukey-testkit/src/cdp.test.ts new file mode 100644 index 0000000..5ae07a4 --- /dev/null +++ b/packages/yukey-testkit/src/cdp.test.ts @@ -0,0 +1,141 @@ +// Cdp against a fake socket: no Chrome, no network. The fake records every +// frame sent and lets a test script the replies and the socket's lifecycle. + +import { describe, expect, it } from "vitest"; +import { Cdp, type CdpSocket } from "./cdp.js"; + +interface Frame { + id: number; + method: string; + params: Record<string, unknown>; +} + +class FakeSocket implements CdpSocket { + readyState: number = WebSocket.OPEN; + readonly sent: Frame[] = []; + private readonly listeners = new Map<string, ((ev: { data: unknown }) => void)[]>(); + // How to answer each frame; the default acknowledges with an empty result, undefined never answers. + reply: (frame: Frame) => Record<string, unknown> | undefined = () => ({ result: {} }); + + addEventListener(type: string, listener: (ev: { data: unknown }) => void): void { + const list = this.listeners.get(type) ?? []; + list.push(listener); + this.listeners.set(type, list); + } + + // Like the real thing, a frame sent on a socket that isn't open is dropped silently. + send(data: string): void { + if (this.readyState !== WebSocket.OPEN) return; + const frame = JSON.parse(data) as Frame; + this.sent.push(frame); + const reply = this.reply(frame); + if (reply !== undefined) this.emit("message", { data: JSON.stringify({ id: frame.id, ...reply }) }); + } + + close(): void { + this.readyState = WebSocket.CLOSED; + this.emit("close", { data: undefined }); + } + + emit(type: string, ev: { data: unknown }): void { + for (const l of this.listeners.get(type) ?? []) l(ev); + } +} + +// Whether a promise has settled (either way) once the microtask queue drains. +async function settled(p: Promise<unknown>): Promise<boolean> { + let done = false; + p.then(() => (done = true), () => (done = true)); + await new Promise<void>((r) => setImmediate(r)); + return done; +} + +describe("Cdp.key", () => { + async function keyParams(key: string, opts?: { shift?: boolean }): Promise<Record<string, unknown>[]> { + const sock = new FakeSocket(); + await Cdp.fromSocket(sock).key(key, opts); + for (const f of sock.sent) expect(f.method).toBe("Input.dispatchKeyEvent"); + return sock.sent.map((f) => f.params); + } + + it("sends Tab as a rawKeyDown/keyUp pair with the virtual key code", async () => { + expect(await keyParams("Tab")).toEqual([ + { type: "rawKeyDown", windowsVirtualKeyCode: 9, code: "Tab", key: "Tab", modifiers: 0 }, + { type: "keyUp", windowsVirtualKeyCode: 9, code: "Tab", key: "Tab", modifiers: 0 }, + ]); + }); + + it("sends shift+Tab with the shift modifier bit on both halves", async () => { + const params = await keyParams("Tab", { shift: true }); + expect(params.map((p) => p.modifiers)).toEqual([8, 8]); + }); + + it("sends Enter as a full keyDown carrying its text", async () => { + expect(await keyParams("Enter")).toEqual([ + { type: "keyDown", windowsVirtualKeyCode: 13, code: "Enter", key: "Enter", modifiers: 0, text: "\r" }, + { type: "keyUp", windowsVirtualKeyCode: 13, code: "Enter", key: "Enter", modifiers: 0 }, + ]); + }); + + it("sends space as a full keyDown carrying its text", async () => { + expect(await keyParams(" ")).toEqual([ + { type: "keyDown", windowsVirtualKeyCode: 32, code: "Space", key: " ", modifiers: 0, text: " " }, + { type: "keyUp", windowsVirtualKeyCode: 32, code: "Space", key: " ", modifiers: 0 }, + ]); + }); + + it("sends a printable character with its text and KeyX code", async () => { + expect(await keyParams("a")).toEqual([ + { type: "keyDown", windowsVirtualKeyCode: 65, code: "KeyA", key: "a", modifiers: 0, text: "a" }, + { type: "keyUp", windowsVirtualKeyCode: 65, code: "KeyA", key: "a", modifiers: 0 }, + ]); + }); +}); + +describe("Cdp.cmd", () => { + it("resolves with the reply's result", async () => { + const sock = new FakeSocket(); + sock.reply = () => ({ result: { answer: 42 } }); + expect(await Cdp.fromSocket(sock).cmd("Any.thing", { x: 1 })).toEqual({ answer: 42 }); + expect(sock.sent).toEqual([{ id: 1, method: "Any.thing", params: { x: 1 } }]); + }); + + it("rejects on an error reply, with the error in the message", async () => { + const sock = new FakeSocket(); + sock.reply = () => ({ error: { code: -32601, message: "no such method" } }); + await expect(Cdp.fromSocket(sock).cmd("Nope.nope")).rejects.toThrow(/no such method/); + }); + + it("records a method message as an event", async () => { + const sock = new FakeSocket(); + const cdp = Cdp.fromSocket(sock); + sock.emit("message", { data: JSON.stringify({ method: "Runtime.consoleAPICalled", params: { type: "log" } }) }); + expect(cdp.events).toEqual([{ method: "Runtime.consoleAPICalled", params: { type: "log" } }]); + }); + + it("rejects an outstanding command when the socket closes", async () => { + const sock = new FakeSocket(); + sock.reply = () => undefined; + const pending = Cdp.fromSocket(sock).cmd("Slow.thing"); + sock.close(); + expect(await settled(pending)).toBe(true); + await expect(pending).rejects.toThrow(/closed/); + }); + + it("rejects a command sent after the socket closed", async () => { + const sock = new FakeSocket(); + const cdp = Cdp.fromSocket(sock); + sock.close(); + const late = cdp.cmd("Late.thing"); + expect(await settled(late)).toBe(true); + await expect(late).rejects.toThrow(/closed/); + }); +}); + +describe("Cdp.js", () => { + it("returns the evaluated value", async () => { + const sock = new FakeSocket(); + sock.reply = () => ({ result: { result: { type: "number", value: 2 } } }); + expect(await Cdp.fromSocket(sock).js("1+1")).toBe(2); + }); +}); diff --git a/packages/yukey-testkit/src/cdp.ts b/packages/yukey-testkit/src/cdp.ts index 8d3f5b6..47e1a2a 100644 --- a/packages/yukey-testkit/src/cdp.ts +++ b/packages/yukey-testkit/src/cdp.ts @@ -33,8 +33,19 @@ export interface CdpEvent { params: unknown; } +// The slice of WebSocket that Cdp uses. Cdp.connect supplies a real one; +// Cdp.fromSocket takes anything with this shape, which is how the unit tests +// drive the client without a browser. +export interface CdpSocket { + readonly readyState: number; + addEventListener(type: "message", listener: (ev: { data: unknown }) => void): void; + addEventListener(type: "close" | "error", listener: () => void): void; + send(data: string): void; + close(): void; +} + export class Cdp { - private readonly ws: WebSocket; + private readonly ws: CdpSocket; private msgid = 0; private readonly pending = new Map<number, PendingCommand>(); // Every protocol event received on this connection, in arrival order. @@ -42,9 +53,11 @@ export class Cdp { // so this stays empty unless a scenario asks for events. readonly events: CdpEvent[] = []; - private constructor(ws: WebSocket) { + private constructor(ws: CdpSocket) { this.ws = ws; ws.addEventListener("message", (ev) => this.onMessage(String(ev.data))); + ws.addEventListener("close", () => this.failPending()); + ws.addEventListener("error", () => this.failPending()); } static async connect(wsUrl: string): Promise<Cdp> { @@ -58,10 +71,24 @@ export class Cdp { return new Cdp(ws); } + // Wrap an already-open socket. + static fromSocket(ws: CdpSocket): Cdp { + return new Cdp(ws); + } + close(): void { this.ws.close(); } + // Nothing outstanding can be answered once the socket is gone; reject it all + // so a dead browser surfaces as an error rather than a hang. + private failPending(): void { + for (const waiter of this.pending.values()) { + waiter.reject(new Error("CDP connection closed")); + } + this.pending.clear(); + } + private onMessage(raw: string): void { const msg = JSON.parse(raw) as { id?: number; @@ -86,6 +113,9 @@ export class Cdp { } cmd(method: string, params: Record<string, unknown> = {}): Promise<unknown> { + if (this.ws.readyState !== WebSocket.OPEN) { + return Promise.reject(new Error(`CDP connection closed; cannot send ${method}`)); + } this.msgid += 1; const id = this.msgid; const sent = new Promise<unknown>((resolve, reject) => { diff --git a/packages/yukey-testkit/tsconfig.build.json b/packages/yukey-testkit/tsconfig.build.json index cdf9958..0d86915 100644 --- a/packages/yukey-testkit/tsconfig.build.json +++ b/packages/yukey-testkit/tsconfig.build.json @@ -7,5 +7,8 @@ "declaration": true, "declarationMap": true, "sourceMap": true - } + }, + "exclude": [ + "src/**/*.test.ts" + ] } diff --git a/packages/yukey-testkit/tsconfig.json b/packages/yukey-testkit/tsconfig.json index d991313..89940ad 100644 --- a/packages/yukey-testkit/tsconfig.json +++ b/packages/yukey-testkit/tsconfig.json @@ -3,11 +3,18 @@ "target": "ES2022", "module": "NodeNext", "moduleResolution": "NodeNext", - "lib": ["ES2022"], - "types": ["node"], + "lib": [ + "ES2022" + ], + "types": [ + "node" + ], "strict": true, "noUncheckedIndexedAccess": true, "noEmit": true }, - "include": ["src"] + "include": [ + "src", + "vitest.config.ts" + ] } diff --git a/packages/yukey-testkit/vitest.config.ts b/packages/yukey-testkit/vitest.config.ts new file mode 100644 index 0000000..eb088c4 --- /dev/null +++ b/packages/yukey-testkit/vitest.config.ts @@ -0,0 +1,10 @@ +// Tests live beside the code they cover (src/foo.test.ts next to src/foo.ts). +// Everything here is node-only; there's no DOM to emulate. +import { defineConfig } from "vitest/config"; + +export default defineConfig({ + test: { + environment: "node", + include: ["src/**/*.test.ts"], + }, +});