From 67e3eb85d742aedbf418c14251ac60afefe24d64 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 3 Mar 2026 01:14:38 +0000 Subject: [PATCH] refactor(tests): dedupe browser and config cli test setup --- src/browser/chrome.test.ts | 235 +++++++++++++++++-------------------- src/cli/config-cli.test.ts | 162 ++++++++++++------------- 2 files changed, 184 insertions(+), 213 deletions(-) diff --git a/src/browser/chrome.test.ts b/src/browser/chrome.test.ts index d5a6bc5b1d0..467a09be0f2 100644 --- a/src/browser/chrome.test.ts +++ b/src/browser/chrome.test.ts @@ -21,6 +21,8 @@ import { DEFAULT_OPENCLAW_BROWSER_PROFILE_NAME, } from "./constants.js"; +type StopChromeTarget = Parameters[0]; + async function readJson(filePath: string): Promise> { const raw = await fsp.readFile(filePath, "utf-8"); return JSON.parse(raw) as Record; @@ -35,6 +37,67 @@ async function readDefaultProfileFromLocalState( return infoCache.Default as Record; } +async function withMockChromeCdpServer(params: { + wsPath: string; + onConnection?: (wss: WebSocketServer) => void; + run: (baseUrl: string) => Promise; +}) { + const server = createServer((req, res) => { + if (req.url === "/json/version") { + const addr = server.address() as AddressInfo; + res.writeHead(200, { "Content-Type": "application/json" }); + res.end( + JSON.stringify({ + webSocketDebuggerUrl: `ws://127.0.0.1:${addr.port}${params.wsPath}`, + }), + ); + return; + } + res.writeHead(404); + res.end(); + }); + const wss = new WebSocketServer({ noServer: true }); + server.on("upgrade", (req, socket, head) => { + if (req.url !== params.wsPath) { + socket.destroy(); + return; + } + wss.handleUpgrade(req, socket, head, (ws) => { + wss.emit("connection", ws, req); + }); + }); + params.onConnection?.(wss); + await new Promise((resolve, reject) => { + server.listen(0, "127.0.0.1", () => resolve()); + server.once("error", reject); + }); + try { + const addr = server.address() as AddressInfo; + await params.run(`http://127.0.0.1:${addr.port}`); + } finally { + await new Promise((resolve) => wss.close(() => resolve())); + await new Promise((resolve) => server.close(() => resolve())); + } +} + +async function stopChromeWithProc(proc: ReturnType, timeoutMs: number) { + await stopOpenClawChrome( + { + proc, + cdpPort: 12345, + } as unknown as StopChromeTarget, + timeoutMs, + ); +} + +function makeChromeTestProc(overrides?: Partial<{ killed: boolean; exitCode: number | null }>) { + return { + killed: overrides?.killed ?? false, + exitCode: overrides?.exitCode ?? null, + kill: vi.fn(), + }; +} + describe("browser chrome profile decoration", () => { let fixtureRoot = ""; let fixtureCount = 0; @@ -143,14 +206,6 @@ describe("browser chrome helpers", () => { return vi.spyOn(fs, "existsSync").mockImplementation((p) => match(String(p))); } - function makeProc(overrides?: Partial<{ killed: boolean; exitCode: number | null }>) { - return { - killed: overrides?.killed ?? false, - exitCode: overrides?.exitCode ?? null, - kill: vi.fn(), - }; - } - afterEach(() => { vi.unstubAllEnvs(); vi.unstubAllGlobals(); @@ -248,129 +303,63 @@ describe("browser chrome helpers", () => { }); it("reports cdpReady only when Browser.getVersion command succeeds", async () => { - const server = createServer((req, res) => { - if (req.url === "/json/version") { - const addr = server.address() as AddressInfo; - res.writeHead(200, { "Content-Type": "application/json" }); - res.end( - JSON.stringify({ - webSocketDebuggerUrl: `ws://127.0.0.1:${addr.port}/devtools/browser/health`, - }), - ); - return; - } - res.writeHead(404); - res.end(); + await withMockChromeCdpServer({ + wsPath: "/devtools/browser/health", + onConnection: (wss) => { + wss.on("connection", (ws) => { + ws.on("message", (raw) => { + let message: { id?: unknown; method?: unknown } | null = null; + try { + const text = + typeof raw === "string" + ? raw + : Buffer.isBuffer(raw) + ? raw.toString("utf8") + : Array.isArray(raw) + ? Buffer.concat(raw).toString("utf8") + : Buffer.from(raw).toString("utf8"); + message = JSON.parse(text) as { id?: unknown; method?: unknown }; + } catch { + return; + } + if (message?.method === "Browser.getVersion" && message.id === 1) { + ws.send( + JSON.stringify({ + id: 1, + result: { product: "Chrome/Mock" }, + }), + ); + } + }); + }); + }, + run: async (baseUrl) => { + await expect(isChromeCdpReady(baseUrl, 300, 400)).resolves.toBe(true); + }, }); - const wss = new WebSocketServer({ noServer: true }); - server.on("upgrade", (req, socket, head) => { - if (req.url !== "/devtools/browser/health") { - socket.destroy(); - return; - } - wss.handleUpgrade(req, socket, head, (ws) => { - wss.emit("connection", ws, req); - }); - }); - wss.on("connection", (ws) => { - ws.on("message", (raw) => { - let message: { id?: unknown; method?: unknown } | null = null; - try { - const text = - typeof raw === "string" - ? raw - : Buffer.isBuffer(raw) - ? raw.toString("utf8") - : Array.isArray(raw) - ? Buffer.concat(raw).toString("utf8") - : Buffer.from(raw).toString("utf8"); - message = JSON.parse(text) as { id?: unknown; method?: unknown }; - } catch { - return; - } - if (message?.method === "Browser.getVersion" && message.id === 1) { - ws.send( - JSON.stringify({ - id: 1, - result: { product: "Chrome/Mock" }, - }), - ); - } - }); - }); - - await new Promise((resolve, reject) => { - server.listen(0, "127.0.0.1", () => resolve()); - server.once("error", reject); - }); - const addr = server.address() as AddressInfo; - await expect(isChromeCdpReady(`http://127.0.0.1:${addr.port}`, 300, 400)).resolves.toBe(true); - - await new Promise((resolve) => wss.close(() => resolve())); - await new Promise((resolve) => server.close(() => resolve())); }); it("reports cdpReady false when websocket opens but command channel is stale", async () => { - const server = createServer((req, res) => { - if (req.url === "/json/version") { - const addr = server.address() as AddressInfo; - res.writeHead(200, { "Content-Type": "application/json" }); - res.end( - JSON.stringify({ - webSocketDebuggerUrl: `ws://127.0.0.1:${addr.port}/devtools/browser/stale`, - }), - ); - return; - } - res.writeHead(404); - res.end(); + await withMockChromeCdpServer({ + wsPath: "/devtools/browser/stale", + // Simulate a stale command channel: WS opens but never responds to commands. + onConnection: (wss) => wss.on("connection", (_ws) => {}), + run: async (baseUrl) => { + await expect(isChromeCdpReady(baseUrl, 300, 150)).resolves.toBe(false); + }, }); - const wss = new WebSocketServer({ noServer: true }); - server.on("upgrade", (req, socket, head) => { - if (req.url !== "/devtools/browser/stale") { - socket.destroy(); - return; - } - wss.handleUpgrade(req, socket, head, (ws) => { - wss.emit("connection", ws, req); - }); - }); - // Simulate a stale command channel: WS opens but never responds to commands. - wss.on("connection", (_ws) => {}); - - await new Promise((resolve, reject) => { - server.listen(0, "127.0.0.1", () => resolve()); - server.once("error", reject); - }); - const addr = server.address() as AddressInfo; - await expect(isChromeCdpReady(`http://127.0.0.1:${addr.port}`, 300, 150)).resolves.toBe(false); - - await new Promise((resolve) => wss.close(() => resolve())); - await new Promise((resolve) => server.close(() => resolve())); }); it("stopOpenClawChrome no-ops when process is already killed", async () => { - const proc = makeProc({ killed: true }); - await stopOpenClawChrome( - { - proc, - cdpPort: 12345, - } as unknown as Parameters[0], - 10, - ); + const proc = makeChromeTestProc({ killed: true }); + await stopChromeWithProc(proc, 10); expect(proc.kill).not.toHaveBeenCalled(); }); it("stopOpenClawChrome sends SIGTERM and returns once CDP is down", async () => { vi.stubGlobal("fetch", vi.fn().mockRejectedValue(new Error("down"))); - const proc = makeProc(); - await stopOpenClawChrome( - { - proc, - cdpPort: 12345, - } as unknown as Parameters[0], - 10, - ); + const proc = makeChromeTestProc(); + await stopChromeWithProc(proc, 10); expect(proc.kill).toHaveBeenCalledWith("SIGTERM"); }); @@ -382,14 +371,8 @@ describe("browser chrome helpers", () => { json: async () => ({ webSocketDebuggerUrl: "ws://127.0.0.1/devtools" }), } as unknown as Response), ); - const proc = makeProc(); - await stopOpenClawChrome( - { - proc, - cdpPort: 12345, - } as unknown as Parameters[0], - 1, - ); + const proc = makeChromeTestProc(); + await stopChromeWithProc(proc, 1); expect(proc.kill).toHaveBeenNthCalledWith(1, "SIGTERM"); expect(proc.kill).toHaveBeenNthCalledWith(2, "SIGKILL"); }); diff --git a/src/cli/config-cli.test.ts b/src/cli/config-cli.test.ts index c2130dc2df4..d503e6113ef 100644 --- a/src/cli/config-cli.test.ts +++ b/src/cli/config-cli.test.ts @@ -60,6 +60,52 @@ function setSnapshotOnce(snapshot: ConfigFileSnapshot) { mockReadConfigFileSnapshot.mockResolvedValueOnce(snapshot); } +function withRuntimeDefaults(resolved: OpenClawConfig): OpenClawConfig { + return { + ...resolved, + agents: { + ...resolved.agents, + defaults: { + model: "gpt-5.2", + } as never, + } as never, + }; +} + +function makeInvalidSnapshot(params: { + issues: ConfigFileSnapshot["issues"]; + path?: string; +}): ConfigFileSnapshot { + return { + path: params.path ?? "/tmp/custom-openclaw.json", + exists: true, + raw: "{}", + parsed: {}, + resolved: {}, + valid: false, + config: {}, + issues: params.issues, + warnings: [], + legacyIssues: [], + }; +} + +async function runValidateJsonAndGetPayload() { + await expect(runConfigCommand(["config", "validate", "--json"])).rejects.toThrow("__exit__:1"); + const raw = mockLog.mock.calls.at(0)?.[0]; + expect(typeof raw).toBe("string"); + return JSON.parse(String(raw)) as { + valid: boolean; + path: string; + issues: Array<{ + path: string; + message: string; + allowedValues?: string[]; + allowedValuesHiddenCount?: number; + }>; + }; +} + let registerConfigCli: typeof import("./config-cli.js").registerConfigCli; let sharedProgram: Command; @@ -90,13 +136,7 @@ describe("config cli", () => { logging: { level: "debug" }, }; const runtimeMerged: OpenClawConfig = { - ...resolved, - agents: { - ...resolved.agents, - defaults: { - model: "gpt-5.2", - } as never, - } as never, + ...withRuntimeDefaults(resolved), }; setSnapshot(resolved, runtimeMerged); @@ -194,23 +234,16 @@ describe("config cli", () => { }); it("prints issues and exits 1 when config is invalid", async () => { - setSnapshotOnce({ - path: "/tmp/custom-openclaw.json", - exists: true, - raw: "{}", - parsed: {}, - resolved: {}, - valid: false, - config: {}, - issues: [ - { - path: "agents.defaults.suppressToolErrorWarnings", - message: "Unrecognized key(s) in object", - }, - ], - warnings: [], - legacyIssues: [], - }); + setSnapshotOnce( + makeInvalidSnapshot({ + issues: [ + { + path: "agents.defaults.suppressToolErrorWarnings", + message: "Unrecognized key(s) in object", + }, + ], + }), + ); await expect(runConfigCommand(["config", "validate"])).rejects.toThrow("__exit__:1"); @@ -222,30 +255,13 @@ describe("config cli", () => { }); it("returns machine-readable JSON with --json for invalid config", async () => { - setSnapshotOnce({ - path: "/tmp/custom-openclaw.json", - exists: true, - raw: "{}", - parsed: {}, - resolved: {}, - valid: false, - config: {}, - issues: [{ path: "gateway.bind", message: "Invalid enum value" }], - warnings: [], - legacyIssues: [], - }); - - await expect(runConfigCommand(["config", "validate", "--json"])).rejects.toThrow( - "__exit__:1", + setSnapshotOnce( + makeInvalidSnapshot({ + issues: [{ path: "gateway.bind", message: "Invalid enum value" }], + }), ); - const raw = mockLog.mock.calls.at(0)?.[0]; - expect(typeof raw).toBe("string"); - const payload = JSON.parse(String(raw)) as { - valid: boolean; - path: string; - issues: Array<{ path: string; message: string }>; - }; + const payload = await runValidateJsonAndGetPayload(); expect(payload.valid).toBe(false); expect(payload.path).toBe("/tmp/custom-openclaw.json"); expect(payload.issues).toEqual([{ path: "gateway.bind", message: "Invalid enum value" }]); @@ -253,42 +269,20 @@ describe("config cli", () => { }); it("preserves allowed-values metadata in --json output", async () => { - setSnapshotOnce({ - path: "/tmp/custom-openclaw.json", - exists: true, - raw: "{}", - parsed: {}, - resolved: {}, - valid: false, - config: {}, - issues: [ - { - path: "update.channel", - message: 'Invalid input (allowed: "stable", "beta", "dev")', - allowedValues: ["stable", "beta", "dev"], - allowedValuesHiddenCount: 0, - }, - ], - warnings: [], - legacyIssues: [], - }); - - await expect(runConfigCommand(["config", "validate", "--json"])).rejects.toThrow( - "__exit__:1", + setSnapshotOnce( + makeInvalidSnapshot({ + issues: [ + { + path: "update.channel", + message: 'Invalid input (allowed: "stable", "beta", "dev")', + allowedValues: ["stable", "beta", "dev"], + allowedValuesHiddenCount: 0, + }, + ], + }), ); - const raw = mockLog.mock.calls.at(0)?.[0]; - expect(typeof raw).toBe("string"); - const payload = JSON.parse(String(raw)) as { - valid: boolean; - path: string; - issues: Array<{ - path: string; - message: string; - allowedValues?: string[]; - allowedValuesHiddenCount?: number; - }>; - }; + const payload = await runValidateJsonAndGetPayload(); expect(payload.valid).toBe(false); expect(payload.path).toBe("/tmp/custom-openclaw.json"); expect(payload.issues).toEqual([ @@ -406,13 +400,7 @@ describe("config cli", () => { logging: { level: "debug" }, }; const runtimeMerged: OpenClawConfig = { - ...resolved, - agents: { - ...resolved.agents, - defaults: { - model: "gpt-5.2", - }, - } as never, + ...withRuntimeDefaults(resolved), }; setSnapshot(resolved, runtimeMerged);