From 0a4c11061dba8a3d5e4b64546a451b9d8e9b3091 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 30 Mar 2026 00:23:38 +0100 Subject: [PATCH] test: stabilize targeted harnesses - reduce module-reset mock churn in auth/acp tests - simplify runtime web mock cleanup - make canvas reload test use in-memory websocket tracking --- src/acp/persistent-bindings.lifecycle.test.ts | 17 +- src/acp/persistent-bindings.test.ts | 27 ++- src/agents/model-auth-label.test.ts | 82 ++++----- src/agents/openclaw-tools.web-runtime.test.ts | 30 +--- src/canvas-host/server.test.ts | 167 +++++++++++++----- src/canvas-host/server.ts | 4 +- 6 files changed, 174 insertions(+), 153 deletions(-) diff --git a/src/acp/persistent-bindings.lifecycle.test.ts b/src/acp/persistent-bindings.lifecycle.test.ts index 04f36e80cc9..37c0702d8ed 100644 --- a/src/acp/persistent-bindings.lifecycle.test.ts +++ b/src/acp/persistent-bindings.lifecycle.test.ts @@ -1,5 +1,4 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; -import { importFreshModule } from "../../test/helpers/import-fresh.js"; import type { OpenClawConfig } from "../config/config.js"; const managerMocks = vi.hoisted(() => ({ @@ -32,10 +31,6 @@ vi.mock("./persistent-bindings.resolve.js", () => ({ resolveConfiguredAcpBindingSpecBySessionKey: resolveMocks.resolveConfiguredAcpBindingSpecBySessionKey, })); -type BindingTargetsModule = typeof import("../channels/plugins/binding-targets.js"); -let bindingTargets: BindingTargetsModule; -let bindingTargetsImportScope = 0; - const baseCfg = { session: { mainKey: "main", scope: "per-sender" }, agents: { @@ -43,13 +38,10 @@ const baseCfg = { }, } satisfies OpenClawConfig; +let resetAcpSessionInPlace: typeof import("./persistent-bindings.lifecycle.js").resetAcpSessionInPlace; + beforeEach(async () => { vi.resetModules(); - bindingTargetsImportScope += 1; - bindingTargets = await importFreshModule( - import.meta.url, - `../channels/plugins/binding-targets.js?scope=${bindingTargetsImportScope}`, - ); managerMocks.closeSession.mockReset().mockResolvedValue({ runtimeClosed: true, metaCleared: false, @@ -58,9 +50,10 @@ beforeEach(async () => { managerMocks.updateSessionRuntimeOptions.mockReset().mockResolvedValue(undefined); sessionMetaMocks.readAcpSessionEntry.mockReset().mockReturnValue(undefined); resolveMocks.resolveConfiguredAcpBindingSpecBySessionKey.mockReset().mockReturnValue(null); + ({ resetAcpSessionInPlace } = await import("./persistent-bindings.lifecycle.js")); }); -describe("resetConfiguredBindingTargetInPlace", () => { +describe("resetAcpSessionInPlace", () => { it("does not resolve configured bindings when ACP metadata already exists", async () => { const sessionKey = "agent:claude:acp:binding:demo-binding:default:9373ab192b2317f4"; sessionMetaMocks.readAcpSessionEntry.mockReturnValue({ @@ -75,7 +68,7 @@ describe("resetConfiguredBindingTargetInPlace", () => { throw new Error("configured binding resolution should be skipped"); }); - const result = await bindingTargets.resetConfiguredBindingTargetInPlace({ + const result = await resetAcpSessionInPlace({ cfg: baseCfg, sessionKey, reason: "reset", diff --git a/src/acp/persistent-bindings.test.ts b/src/acp/persistent-bindings.test.ts index ab8eab1a7e2..0d3691364de 100644 --- a/src/acp/persistent-bindings.test.ts +++ b/src/acp/persistent-bindings.test.ts @@ -1,4 +1,4 @@ -import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; import { resolveAgentWorkspaceDir } from "../agents/agent-scope.js"; import type { ChannelConfiguredBindingProvider, ChannelPlugin } from "../channels/plugins/types.js"; import type { OpenClawConfig } from "../config/config.js"; @@ -376,21 +376,8 @@ function mockReadySession(params: { return sessionKey; } -beforeAll(async () => { +beforeEach(async () => { vi.resetModules(); - persistentBindingsResolveModule = await import("./persistent-bindings.resolve.js"); - lifecycleBindingsModule = await import("./persistent-bindings.lifecycle.js"); - persistentBindings = { - resolveConfiguredAcpBindingRecord: - persistentBindingsResolveModule.resolveConfiguredAcpBindingRecord, - resolveConfiguredAcpBindingSpecBySessionKey: - persistentBindingsResolveModule.resolveConfiguredAcpBindingSpecBySessionKey, - ensureConfiguredAcpBindingSession: lifecycleBindingsModule.ensureConfiguredAcpBindingSession, - resetAcpSessionInPlace: lifecycleBindingsModule.resetAcpSessionInPlace, - }; -}); - -beforeEach(() => { setActivePluginRegistry( createTestRegistry([ { @@ -418,6 +405,16 @@ beforeEach(() => { managerMocks.initializeSession.mockReset().mockResolvedValue(undefined); managerMocks.updateSessionRuntimeOptions.mockReset().mockResolvedValue(undefined); sessionMetaMocks.readAcpSessionEntry.mockReset().mockReturnValue(undefined); + persistentBindingsResolveModule = await import("./persistent-bindings.resolve.js"); + lifecycleBindingsModule = await import("./persistent-bindings.lifecycle.js"); + persistentBindings = { + resolveConfiguredAcpBindingRecord: + persistentBindingsResolveModule.resolveConfiguredAcpBindingRecord, + resolveConfiguredAcpBindingSpecBySessionKey: + persistentBindingsResolveModule.resolveConfiguredAcpBindingSpecBySessionKey, + ensureConfiguredAcpBindingSession: lifecycleBindingsModule.ensureConfiguredAcpBindingSession, + resetAcpSessionInPlace: lifecycleBindingsModule.resetAcpSessionInPlace, + }; }); describe("resolveConfiguredAcpBindingRecord", () => { diff --git a/src/agents/model-auth-label.test.ts b/src/agents/model-auth-label.test.ts index 5a7b1b1134f..ec102b66e7c 100644 --- a/src/agents/model-auth-label.test.ts +++ b/src/agents/model-auth-label.test.ts @@ -1,64 +1,42 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; -vi.mock("./auth-profiles.js", () => ({ +const mocks = vi.hoisted(() => ({ ensureAuthProfileStore: vi.fn(), resolveAuthProfileOrder: vi.fn(), resolveAuthProfileDisplayLabel: vi.fn(), -})); - -vi.mock("./model-auth.js", () => ({ resolveUsableCustomProviderApiKey: vi.fn(() => null), resolveEnvApiKey: vi.fn(() => null), })); -type AuthProfilesModule = typeof import("./auth-profiles.js"); -type ModelAuthModule = typeof import("./model-auth.js"); -type ModelAuthLabelModule = typeof import("./model-auth-label.js"); +vi.mock("./auth-profiles.js", () => ({ + ensureAuthProfileStore: mocks.ensureAuthProfileStore, + resolveAuthProfileOrder: mocks.resolveAuthProfileOrder, + resolveAuthProfileDisplayLabel: mocks.resolveAuthProfileDisplayLabel, +})); -let ensureAuthProfileStoreMock: ReturnType< - typeof vi.mocked ->; -let resolveAuthProfileOrderMock: ReturnType< - typeof vi.mocked ->; -let resolveAuthProfileDisplayLabelMock: ReturnType< - typeof vi.mocked ->; -let resolveUsableCustomProviderApiKeyMock: ReturnType< - typeof vi.mocked ->; -let resolveEnvApiKeyMock: ReturnType>; -let resolveModelAuthLabel: ModelAuthLabelModule["resolveModelAuthLabel"]; +vi.mock("./model-auth.js", () => ({ + resolveUsableCustomProviderApiKey: mocks.resolveUsableCustomProviderApiKey, + resolveEnvApiKey: mocks.resolveEnvApiKey, +})); -async function loadModelAuthLabelModule() { - vi.resetModules(); - const authProfilesModule = await import("./auth-profiles.js"); - const modelAuthModule = await import("./model-auth.js"); - const modelAuthLabelModule = await import("./model-auth-label.js"); - ensureAuthProfileStoreMock = vi.mocked(authProfilesModule.ensureAuthProfileStore); - resolveAuthProfileOrderMock = vi.mocked(authProfilesModule.resolveAuthProfileOrder); - resolveAuthProfileDisplayLabelMock = vi.mocked(authProfilesModule.resolveAuthProfileDisplayLabel); - resolveUsableCustomProviderApiKeyMock = vi.mocked( - modelAuthModule.resolveUsableCustomProviderApiKey, - ); - resolveEnvApiKeyMock = vi.mocked(modelAuthModule.resolveEnvApiKey); - resolveModelAuthLabel = modelAuthLabelModule.resolveModelAuthLabel; -} +let resolveModelAuthLabel: typeof import("./model-auth-label.js").resolveModelAuthLabel; describe("resolveModelAuthLabel", () => { beforeEach(async () => { - await loadModelAuthLabelModule(); - ensureAuthProfileStoreMock.mockReset(); - resolveAuthProfileOrderMock.mockReset(); - resolveAuthProfileDisplayLabelMock.mockReset(); - resolveUsableCustomProviderApiKeyMock.mockReset(); - resolveUsableCustomProviderApiKeyMock.mockReturnValue(null); - resolveEnvApiKeyMock.mockReset(); - resolveEnvApiKeyMock.mockReturnValue(null); + if (!resolveModelAuthLabel) { + ({ resolveModelAuthLabel } = await import("./model-auth-label.js")); + } + mocks.ensureAuthProfileStore.mockReset(); + mocks.resolveAuthProfileOrder.mockReset(); + mocks.resolveAuthProfileDisplayLabel.mockReset(); + mocks.resolveUsableCustomProviderApiKey.mockReset(); + mocks.resolveUsableCustomProviderApiKey.mockReturnValue(null); + mocks.resolveEnvApiKey.mockReset(); + mocks.resolveEnvApiKey.mockReturnValue(null); }); it("does not include token value in label for token profiles", () => { - ensureAuthProfileStoreMock.mockReturnValue({ + mocks.ensureAuthProfileStore.mockReturnValue({ version: 1, profiles: { "github-copilot:default": { @@ -69,8 +47,8 @@ describe("resolveModelAuthLabel", () => { }, }, } as never); - resolveAuthProfileOrderMock.mockReturnValue(["github-copilot:default"]); - resolveAuthProfileDisplayLabelMock.mockReturnValue("github-copilot:default"); + mocks.resolveAuthProfileOrder.mockReturnValue(["github-copilot:default"]); + mocks.resolveAuthProfileDisplayLabel.mockReturnValue("github-copilot:default"); const label = resolveModelAuthLabel({ provider: "github-copilot", @@ -85,7 +63,7 @@ describe("resolveModelAuthLabel", () => { it("does not include api-key value in label for api-key profiles", () => { const shortSecret = "abc123"; // pragma: allowlist secret - ensureAuthProfileStoreMock.mockReturnValue({ + mocks.ensureAuthProfileStore.mockReturnValue({ version: 1, profiles: { "openai:default": { @@ -95,8 +73,8 @@ describe("resolveModelAuthLabel", () => { }, }, } as never); - resolveAuthProfileOrderMock.mockReturnValue(["openai:default"]); - resolveAuthProfileDisplayLabelMock.mockReturnValue("openai:default"); + mocks.resolveAuthProfileOrder.mockReturnValue(["openai:default"]); + mocks.resolveAuthProfileDisplayLabel.mockReturnValue("openai:default"); const label = resolveModelAuthLabel({ provider: "openai", @@ -110,7 +88,7 @@ describe("resolveModelAuthLabel", () => { }); it("shows oauth type with profile label", () => { - ensureAuthProfileStoreMock.mockReturnValue({ + mocks.ensureAuthProfileStore.mockReturnValue({ version: 1, profiles: { "anthropic:oauth": { @@ -119,8 +97,8 @@ describe("resolveModelAuthLabel", () => { }, }, } as never); - resolveAuthProfileOrderMock.mockReturnValue(["anthropic:oauth"]); - resolveAuthProfileDisplayLabelMock.mockReturnValue("anthropic:oauth"); + mocks.resolveAuthProfileOrder.mockReturnValue(["anthropic:oauth"]); + mocks.resolveAuthProfileDisplayLabel.mockReturnValue("anthropic:oauth"); const label = resolveModelAuthLabel({ provider: "anthropic", diff --git a/src/agents/openclaw-tools.web-runtime.test.ts b/src/agents/openclaw-tools.web-runtime.test.ts index d8ba3fb663c..45f4649386f 100644 --- a/src/agents/openclaw-tools.web-runtime.test.ts +++ b/src/agents/openclaw-tools.web-runtime.test.ts @@ -1,31 +1,9 @@ -import { afterAll, afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { OpenClawConfig } from "../config/config.js"; import type { RuntimeWebFetchFirecrawlMetadata } from "../secrets/runtime-web-tools.types.js"; import type { RuntimeWebSearchMetadata } from "../secrets/runtime-web-tools.types.js"; import { withFetchPreconnect } from "../test-utils/fetch-mock.js"; -const mockedModuleIds = [ - "../plugins/tools.js", - "../gateway/call.js", - "./tools/agents-list-tool.js", - "./tools/canvas-tool.js", - "./tools/cron-tool.js", - "./tools/gateway-tool.js", - "./tools/image-generate-tool.js", - "./tools/image-tool.js", - "./tools/message-tool.js", - "./tools/nodes-tool.js", - "./tools/pdf-tool.js", - "./tools/session-status-tool.js", - "./tools/sessions-history-tool.js", - "./tools/sessions-list-tool.js", - "./tools/sessions-send-tool.js", - "./tools/sessions-spawn-tool.js", - "./tools/sessions-yield-tool.js", - "./tools/subagents-tool.js", - "./tools/tts-tool.js", -] as const; - vi.mock("../plugins/tools.js", async () => { const actual = await vi.importActual("../plugins/tools.js"); return { @@ -171,12 +149,6 @@ describe("openclaw tools runtime web metadata wiring", () => { secretsRuntime.clearSecretsRuntimeSnapshot(); }); - afterAll(() => { - for (const id of mockedModuleIds) { - vi.doUnmock(id); - } - }); - it("uses runtime-selected provider when higher-precedence provider ref is unresolved", async () => { const snapshot = await prepareAndActivate({ config: asConfig({ diff --git a/src/canvas-host/server.test.ts b/src/canvas-host/server.test.ts index 0a96f4f32c5..fd179f6f18f 100644 --- a/src/canvas-host/server.test.ts +++ b/src/canvas-host/server.test.ts @@ -1,13 +1,15 @@ import fs from "node:fs/promises"; -import { createServer } from "node:http"; +import { createServer, type IncomingMessage } from "node:http"; import { createRequire } from "node:module"; import type { AddressInfo } from "node:net"; import os from "node:os"; import path from "node:path"; +import type { Duplex } from "node:stream"; import { clearTimeout as clearNativeTimeout, setTimeout as scheduleNativeTimeout, } from "node:timers"; +import { setTimeout as sleep } from "node:timers/promises"; import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import { rawDataToString } from "../infra/ws.js"; import { defaultRuntime } from "../runtime.js"; @@ -19,9 +21,34 @@ type MockWatcher = { __emit: (event: string, ...args: unknown[]) => void; }; -const CANVAS_WS_OPEN_TIMEOUT_MS = 2_000; -const CANVAS_RELOAD_TIMEOUT_MS = 4_000; -const CANVAS_RELOAD_TEST_TIMEOUT_MS = 12_000; +const CANVAS_WS_OPEN_TIMEOUT_MS = 4_000; +const CANVAS_RELOAD_TIMEOUT_MS = 10_000; +const CANVAS_RELOAD_TEST_TIMEOUT_MS = 20_000; + +type TrackingWebSocketServerCtor = { + new (...args: unknown[]): { + on: (event: string, cb: (...args: unknown[]) => void) => unknown; + emit: (event: string, ...args: unknown[]) => void; + handleUpgrade: ( + req: IncomingMessage, + socket: Duplex, + head: Buffer, + cb: (ws: TrackingWebSocket) => void, + ) => void; + close: (cb?: (err?: Error) => void) => void; + connectionCount: number; + }; + latestInstance?: { + connectionCount: number; + }; + latestSocket?: TrackingWebSocket; +}; + +type TrackingWebSocket = { + sent: string[]; + on: (event: string, cb: () => void) => TrackingWebSocket; + send: (message: string) => void; +}; function isLoopbackBindDenied(error: unknown) { const code = (error as NodeJS.ErrnoException | undefined)?.code; @@ -267,65 +294,117 @@ describe("canvas host", () => { }); it( - "serves HTML with injection and broadcasts reload on file changes", + "broadcasts reload on file changes", async () => { const dir = await createCaseDir(); const index = path.join(dir, "index.html"); await fs.writeFile(index, "v1", "utf8"); const watcherStart = watcherState.watchers.length; - let server: Awaited>; - try { - server = await startFixtureCanvasHost(dir); - } catch (error) { - if (isLoopbackBindDenied(error)) { - return; + const TrackingWebSocketServerClass = class TrackingWebSocketServer { + static latestInstance: { connectionCount: number } | undefined; + static latestSocket: TrackingWebSocket | undefined; + connectionCount = 0; + readonly handlers = new Map void>>(); + + on(event: string, cb: (...args: unknown[]) => void) { + const list = this.handlers.get(event) ?? []; + list.push(cb); + this.handlers.set(event, list); + return this; } - throw error; - } + + emit(event: string, ...args: unknown[]) { + for (const cb of this.handlers.get(event) ?? []) { + cb(...args); + } + } + + handleUpgrade( + req: IncomingMessage, + socket: Duplex, + head: Buffer, + cb: (ws: TrackingWebSocket) => void, + ) { + void req; + void socket; + void head; + const closeHandlers: Array<() => void> = []; + const ws: TrackingWebSocket = { + sent: [], + on: (event, handler) => { + if (event === "close") { + closeHandlers.push(handler); + } + return ws; + }, + send: (message: string) => { + ws.sent.push(message); + }, + }; + TrackingWebSocketServerClass.latestSocket = ws; + cb(ws); + } + + close(cb?: (err?: Error) => void) { + cb?.(); + } + + constructor(..._args: unknown[]) { + TrackingWebSocketServerClass.latestInstance = this; + this.on("connection", () => { + this.connectionCount += 1; + }); + } + }; + + const handler = await createCanvasHostHandler({ + runtime: quietRuntime, + rootDir: dir, + basePath: CANVAS_HOST_PATH, + allowInTests: true, + watchFactory: watcherState.watchFactory as unknown as Parameters< + typeof createCanvasHostHandler + >[0]["watchFactory"], + webSocketServerClass: + TrackingWebSocketServerClass as unknown as typeof import("ws").WebSocketServer, + }); try { const watcher = watcherState.watchers[watcherStart]; expect(watcher).toBeTruthy(); - - const { res, html } = await fetchCanvasHtml(server.port); - expect(res.status).toBe(200); - expect(html).toContain("v1"); - expect(html).toContain(CANVAS_WS_PATH); - - const ws = new WebSocketClient(`ws://127.0.0.1:${server.port}${CANVAS_WS_PATH}`); - await new Promise((resolve, reject) => { - const timer = scheduleNativeTimeout( - () => reject(new Error("ws open timeout")), - CANVAS_WS_OPEN_TIMEOUT_MS, - ); - ws.on("open", () => { - clearNativeTimeout(timer); - resolve(); - }); - ws.on("error", (err) => { - clearNativeTimeout(timer); - reject(err); - }); - }); + const upgraded = handler.handleUpgrade( + { url: CANVAS_WS_PATH } as IncomingMessage, + {} as Duplex, + Buffer.alloc(0), + ); + expect(upgraded).toBe(true); + expect(TrackingWebSocketServerClass.latestInstance?.connectionCount).toBe(1); + const ws = TrackingWebSocketServerClass.latestSocket; + expect(ws).toBeTruthy(); const msg = new Promise((resolve, reject) => { - const timer = scheduleNativeTimeout( - () => reject(new Error("reload timeout")), - CANVAS_RELOAD_TIMEOUT_MS, - ); - ws.on("message", (data) => { - clearNativeTimeout(timer); - resolve(rawDataToString(data)); - }); + const deadline = Date.now() + CANVAS_RELOAD_TIMEOUT_MS; + const poll = () => { + const value = ws?.sent[0]; + if (value) { + resolve(value); + return; + } + if (Date.now() >= deadline) { + reject(new Error("reload timeout")); + return; + } + void sleep(10).then(poll, reject); + }; + void poll(); }); await fs.writeFile(index, "v2", "utf8"); watcher.__emit("all", "change", index); expect(await msg).toBe("reload"); - ws.terminate(); } finally { - await server.close(); + await handler.close(); } }, CANVAS_RELOAD_TEST_TIMEOUT_MS, diff --git a/src/canvas-host/server.ts b/src/canvas-host/server.ts index d14776f0dce..a42c7c5d6cd 100644 --- a/src/canvas-host/server.ts +++ b/src/canvas-host/server.ts @@ -285,7 +285,9 @@ export async function createCanvasHostHandler( debounce = null; broadcastReload(); }, reloadDebounceMs); - debounce.unref?.(); + if (!testMode) { + debounce.unref?.(); + } }; let watcherClosed = false;