From 78220db2beb4039f526549269ba5fee75a141e49 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 22 Feb 2026 17:54:12 +0000 Subject: [PATCH] refactor(browser): dedupe control-server test harness --- .../chrome-user-data-dir.test-harness.ts | 18 ++++ .../server-context.chrome-test-harness.ts | 15 +-- .../server.control-server.test-harness.ts | 92 ++++++++++--------- ...s-open-profile-unknown-returns-404.test.ts | 47 +--------- 4 files changed, 74 insertions(+), 98 deletions(-) create mode 100644 src/browser/chrome-user-data-dir.test-harness.ts diff --git a/src/browser/chrome-user-data-dir.test-harness.ts b/src/browser/chrome-user-data-dir.test-harness.ts new file mode 100644 index 000000000..e3edce48a --- /dev/null +++ b/src/browser/chrome-user-data-dir.test-harness.ts @@ -0,0 +1,18 @@ +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { afterAll, beforeAll } from "vitest"; + +type ChromeUserDataDirRef = { + dir: string; +}; + +export function installChromeUserDataDirHooks(chromeUserDataDir: ChromeUserDataDirRef): void { + beforeAll(async () => { + chromeUserDataDir.dir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-chrome-user-data-")); + }); + + afterAll(async () => { + await fs.rm(chromeUserDataDir.dir, { recursive: true, force: true }); + }); +} diff --git a/src/browser/server-context.chrome-test-harness.ts b/src/browser/server-context.chrome-test-harness.ts index 54600408f..95ebe8097 100644 --- a/src/browser/server-context.chrome-test-harness.ts +++ b/src/browser/server-context.chrome-test-harness.ts @@ -1,17 +1,8 @@ -import fs from "node:fs/promises"; -import os from "node:os"; -import path from "node:path"; -import { afterAll, beforeAll, vi } from "vitest"; +import { vi } from "vitest"; +import { installChromeUserDataDirHooks } from "./chrome-user-data-dir.test-harness.js"; const chromeUserDataDir = { dir: "/tmp/openclaw" }; - -beforeAll(async () => { - chromeUserDataDir.dir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-chrome-user-data-")); -}); - -afterAll(async () => { - await fs.rm(chromeUserDataDir.dir, { recursive: true, force: true }); -}); +installChromeUserDataDirHooks(chromeUserDataDir); vi.mock("./chrome.js", () => ({ isChromeCdpReady: vi.fn(async () => true), diff --git a/src/browser/server.control-server.test-harness.ts b/src/browser/server.control-server.test-harness.ts index f4e96f862..5721d9eb1 100644 --- a/src/browser/server.control-server.test-harness.ts +++ b/src/browser/server.control-server.test-harness.ts @@ -1,8 +1,6 @@ -import fs from "node:fs/promises"; -import os from "node:os"; -import path from "node:path"; -import { afterAll, afterEach, beforeAll, beforeEach, vi } from "vitest"; +import { afterEach, beforeEach, vi } from "vitest"; import type { MockFn } from "../test-utils/vitest-mock-fn.js"; +import { installChromeUserDataDirHooks } from "./chrome-user-data-dir.test-harness.js"; import { getFreePort } from "./test-port.js"; export { getFreePort } from "./test-port.js"; @@ -124,14 +122,7 @@ export function getPwMocks(): Record { } const chromeUserDataDir = vi.hoisted(() => ({ dir: "/tmp/openclaw" })); - -beforeAll(async () => { - chromeUserDataDir.dir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-chrome-user-data-")); -}); - -afterAll(async () => { - await fs.rm(chromeUserDataDir.dir, { recursive: true, force: true }); -}); +installChromeUserDataDirHooks(chromeUserDataDir); function makeProc(pid = 123) { const handlers = new Map void>>(); @@ -257,12 +248,52 @@ function mockClearAll(obj: Record unknown }>) { } } +export async function resetBrowserControlServerTestContext(): Promise { + state.reachable = false; + state.cfgAttachOnly = false; + state.createTargetId = null; + + mockClearAll(pwMocks); + mockClearAll(cdpMocks); + + state.testPort = await getFreePort(); + state.cdpBaseUrl = `http://127.0.0.1:${state.testPort + 1}`; + state.prevGatewayPort = process.env.OPENCLAW_GATEWAY_PORT; + process.env.OPENCLAW_GATEWAY_PORT = String(state.testPort - 2); + // Avoid flaky auth coupling: some suites temporarily set gateway env auth + // which would make the browser control server require auth. + state.prevGatewayToken = process.env.OPENCLAW_GATEWAY_TOKEN; + state.prevGatewayPassword = process.env.OPENCLAW_GATEWAY_PASSWORD; + delete process.env.OPENCLAW_GATEWAY_TOKEN; + delete process.env.OPENCLAW_GATEWAY_PASSWORD; +} + +export function restoreGatewayAuthEnv( + prevGatewayToken: string | undefined, + prevGatewayPassword: string | undefined, +): void { + if (prevGatewayToken === undefined) { + delete process.env.OPENCLAW_GATEWAY_TOKEN; + } else { + process.env.OPENCLAW_GATEWAY_TOKEN = prevGatewayToken; + } + if (prevGatewayPassword === undefined) { + delete process.env.OPENCLAW_GATEWAY_PASSWORD; + } else { + process.env.OPENCLAW_GATEWAY_PASSWORD = prevGatewayPassword; + } +} + +export async function cleanupBrowserControlServerTestContext(): Promise { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + restoreGatewayPortEnv(state.prevGatewayPort); + restoreGatewayAuthEnv(state.prevGatewayToken, state.prevGatewayPassword); + await stopBrowserControlServer(); +} + export function installBrowserControlServerHooks() { beforeEach(async () => { - state.reachable = false; - state.cfgAttachOnly = false; - state.createTargetId = null; - cdpMocks.createTargetViaCdp.mockImplementation(async () => { if (state.createTargetId) { return { targetId: state.createTargetId }; @@ -270,19 +301,7 @@ export function installBrowserControlServerHooks() { throw new Error("cdp disabled"); }); - mockClearAll(pwMocks); - mockClearAll(cdpMocks); - - state.testPort = await getFreePort(); - state.cdpBaseUrl = `http://127.0.0.1:${state.testPort + 1}`; - state.prevGatewayPort = process.env.OPENCLAW_GATEWAY_PORT; - process.env.OPENCLAW_GATEWAY_PORT = String(state.testPort - 2); - // Avoid flaky auth coupling: some suites temporarily set gateway env auth - // which would make the browser control server require auth. - state.prevGatewayToken = process.env.OPENCLAW_GATEWAY_TOKEN; - state.prevGatewayPassword = process.env.OPENCLAW_GATEWAY_PASSWORD; - delete process.env.OPENCLAW_GATEWAY_TOKEN; - delete process.env.OPENCLAW_GATEWAY_PASSWORD; + await resetBrowserControlServerTestContext(); // Minimal CDP JSON endpoints used by the server. let putNewCalls = 0; @@ -338,19 +357,6 @@ export function installBrowserControlServerHooks() { }); afterEach(async () => { - vi.unstubAllGlobals(); - vi.restoreAllMocks(); - restoreGatewayPortEnv(state.prevGatewayPort); - if (state.prevGatewayToken === undefined) { - delete process.env.OPENCLAW_GATEWAY_TOKEN; - } else { - process.env.OPENCLAW_GATEWAY_TOKEN = state.prevGatewayToken; - } - if (state.prevGatewayPassword === undefined) { - delete process.env.OPENCLAW_GATEWAY_PASSWORD; - } else { - process.env.OPENCLAW_GATEWAY_PASSWORD = state.prevGatewayPassword; - } - await stopBrowserControlServer(); + await cleanupBrowserControlServerTestContext(); }); } diff --git a/src/browser/server.post-tabs-open-profile-unknown-returns-404.test.ts b/src/browser/server.post-tabs-open-profile-unknown-returns-404.test.ts index ceaf47212..26de7eccc 100644 --- a/src/browser/server.post-tabs-open-profile-unknown-returns-404.test.ts +++ b/src/browser/server.post-tabs-open-profile-unknown-returns-404.test.ts @@ -1,22 +1,14 @@ import { fetch as realFetch } from "undici"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { + cleanupBrowserControlServerTestContext, getBrowserControlServerBaseUrl, - getBrowserControlServerTestState, - getCdpMocks, - getFreePort, installBrowserControlServerHooks, makeResponse, - getPwMocks, - restoreGatewayPortEnv, + resetBrowserControlServerTestContext, startBrowserControlServerFromConfig, - stopBrowserControlServer, } from "./server.control-server.test-harness.js"; -const state = getBrowserControlServerTestState(); -const cdpMocks = getCdpMocks(); -const pwMocks = getPwMocks(); - describe("browser control server", () => { installBrowserControlServerHooks(); @@ -51,25 +43,7 @@ describe("browser control server", () => { describe("profile CRUD endpoints", () => { beforeEach(async () => { - state.reachable = false; - state.cfgAttachOnly = false; - - for (const fn of Object.values(pwMocks)) { - fn.mockClear(); - } - for (const fn of Object.values(cdpMocks)) { - fn.mockClear(); - } - - state.testPort = await getFreePort(); - state.cdpBaseUrl = `http://127.0.0.1:${state.testPort + 1}`; - state.prevGatewayPort = process.env.OPENCLAW_GATEWAY_PORT; - process.env.OPENCLAW_GATEWAY_PORT = String(state.testPort - 2); - - state.prevGatewayToken = process.env.OPENCLAW_GATEWAY_TOKEN; - state.prevGatewayPassword = process.env.OPENCLAW_GATEWAY_PASSWORD; - delete process.env.OPENCLAW_GATEWAY_TOKEN; - delete process.env.OPENCLAW_GATEWAY_PASSWORD; + await resetBrowserControlServerTestContext(); vi.stubGlobal( "fetch", @@ -84,20 +58,7 @@ describe("profile CRUD endpoints", () => { }); afterEach(async () => { - vi.unstubAllGlobals(); - vi.restoreAllMocks(); - restoreGatewayPortEnv(state.prevGatewayPort); - if (state.prevGatewayToken !== undefined) { - process.env.OPENCLAW_GATEWAY_TOKEN = state.prevGatewayToken; - } else { - delete process.env.OPENCLAW_GATEWAY_TOKEN; - } - if (state.prevGatewayPassword !== undefined) { - process.env.OPENCLAW_GATEWAY_PASSWORD = state.prevGatewayPassword; - } else { - delete process.env.OPENCLAW_GATEWAY_PASSWORD; - } - await stopBrowserControlServer(); + await cleanupBrowserControlServerTestContext(); }); it("validates profile create/delete endpoints", async () => {