From e4466c72a2208699f5720fa6b3bf0d48644227cd Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 30 Mar 2026 00:27:52 +0100 Subject: [PATCH] test: stabilize runner and acp mocks - reuse the shared cli-runner harness in claude runner tests - make ACP session metadata and startup tests use stable static mocks --- src/acp/runtime/session-meta.test.ts | 21 +++----- src/acp/server.startup.test.ts | 16 +++--- src/agents/claude-cli-runner.test.ts | 49 +++++++----------- src/agents/cli-runner.test-support.ts | 74 +++++++-------------------- 4 files changed, 54 insertions(+), 106 deletions(-) diff --git a/src/acp/runtime/session-meta.test.ts b/src/acp/runtime/session-meta.test.ts index ca137e3f2f0..dc8c02b0c53 100644 --- a/src/acp/runtime/session-meta.test.ts +++ b/src/acp/runtime/session-meta.test.ts @@ -10,25 +10,20 @@ const hoisted = vi.hoisted(() => { }; }); -vi.mock("../../config/sessions.js", async () => { - const actual = await vi.importActual( - "../../config/sessions.js", - ); - return { - ...actual, - resolveAllAgentSessionStoreTargets: (cfg: OpenClawConfig, opts: unknown) => - hoisted.resolveAllAgentSessionStoreTargetsMock(cfg, opts), - loadSessionStore: (storePath: string) => hoisted.loadSessionStoreMock(storePath), - }; -}); - +vi.mock("../../config/sessions.js", () => ({ + loadSessionStore: (storePath: string) => hoisted.loadSessionStoreMock(storePath), + resolveAllAgentSessionStoreTargets: (cfg: OpenClawConfig, opts: unknown) => + hoisted.resolveAllAgentSessionStoreTargetsMock(cfg, opts), + resolveStorePath: vi.fn(() => "/tmp/sessions.json"), + updateSessionStore: vi.fn(), +})); let listAcpSessionEntries: typeof import("./session-meta.js").listAcpSessionEntries; describe("listAcpSessionEntries", () => { beforeEach(async () => { vi.resetModules(); - ({ listAcpSessionEntries } = await import("./session-meta.js")); vi.clearAllMocks(); + ({ listAcpSessionEntries } = await import("./session-meta.js")); }); it("reads ACP sessions from resolved configured store targets", async () => { diff --git a/src/acp/server.startup.test.ts b/src/acp/server.startup.test.ts index 35c43478ec9..cb50eb66374 100644 --- a/src/acp/server.startup.test.ts +++ b/src/acp/server.startup.test.ts @@ -12,7 +12,7 @@ type GatewayClientAuth = { }; type ResolveGatewayConnectionAuth = (params: unknown) => Promise; -const mockState = { +const mockState = vi.hoisted(() => ({ gateways: [] as MockGatewayClient[], gatewayAuth: [] as GatewayClientAuth[], agentSideConnectionCtor: vi.fn(), @@ -21,7 +21,7 @@ const mockState = { token: undefined, password: undefined, })), -}; +})); class MockGatewayClient { private callbacks: GatewayClientCallbacks; @@ -63,13 +63,11 @@ vi.mock("../config/config.js", () => ({ mode: "local", }, }), -})); - -vi.mock("../gateway/auth.js", () => ({ - resolveGatewayAuth: () => ({}), + resolveGatewayPort: vi.fn(() => 18_789), })); vi.mock("../gateway/call.js", () => ({ + callGateway: vi.fn(), buildGatewayConnectionDetails: ({ url }: { url?: string }) => { if (typeof url === "string" && url.trim().length > 0) { return { @@ -92,6 +90,10 @@ vi.mock("../gateway/client.js", () => ({ GatewayClient: MockGatewayClient, })); +vi.mock("../infra/is-main.js", () => ({ + isMainModule: () => false, +})); + vi.mock("./translator.js", () => ({ AcpGatewayAgent: class { start(): void { @@ -149,7 +151,7 @@ describe("serveAcpGateway startup", () => { ({ serveAcpGateway } = await import("./server.js")); }); - beforeEach(() => { + beforeEach(async () => { mockState.gateways.length = 0; mockState.gatewayAuth.length = 0; mockState.agentSideConnectionCtor.mockReset(); diff --git a/src/agents/claude-cli-runner.test.ts b/src/agents/claude-cli-runner.test.ts index f982e19588d..c7202de7700 100644 --- a/src/agents/claude-cli-runner.test.ts +++ b/src/agents/claude-cli-runner.test.ts @@ -1,18 +1,8 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; - -const mocks = vi.hoisted(() => ({ - spawn: vi.fn(), -})); - -vi.mock("../process/supervisor/index.js", () => ({ - getProcessSupervisor: () => ({ - spawn: (...args: unknown[]) => mocks.spawn(...args), - cancel: vi.fn(), - cancelScope: vi.fn(), - reconcileOrphans: async () => {}, - getRecord: vi.fn(), - }), -})); +import { + setupClaudeCliRunnerTestModule, + supervisorSpawnMock, +} from "./cli-runner.test-support.js"; function createDeferred() { let resolve: (value: T) => void = () => {}; @@ -52,8 +42,7 @@ function createManagedRun( let runClaudeCliAgent: typeof import("./claude-cli-runner.js").runClaudeCliAgent; async function loadFreshClaudeCliRunnerModuleForTest() { - vi.resetModules(); - ({ runClaudeCliAgent } = await import("./claude-cli-runner.js")); + runClaudeCliAgent = await setupClaudeCliRunnerTestModule(); } function successExit(payload: { message: string; session_id: string }) { @@ -81,11 +70,11 @@ async function waitForCalls(mockFn: { mock: { calls: unknown[][] } }, count: num describe("runClaudeCliAgent", () => { beforeEach(async () => { await loadFreshClaudeCliRunnerModuleForTest(); - mocks.spawn.mockClear(); + supervisorSpawnMock.mockClear(); }); it("starts a new session with --session-id when none is provided", async () => { - mocks.spawn.mockResolvedValueOnce( + supervisorSpawnMock.mockResolvedValueOnce( createManagedRun(Promise.resolve(successExit({ message: "ok", session_id: "sid-1" }))), ); @@ -99,16 +88,16 @@ describe("runClaudeCliAgent", () => { runId: "run-1", }); - expect(mocks.spawn).toHaveBeenCalledTimes(1); - const spawnInput = mocks.spawn.mock.calls[0]?.[0] as { argv: string[]; mode: string }; + expect(supervisorSpawnMock).toHaveBeenCalledTimes(1); + const spawnInput = supervisorSpawnMock.mock.calls[0]?.[0] as { argv: string[]; mode: string }; expect(spawnInput.mode).toBe("child"); expect(spawnInput.argv).toContain("claude"); expect(spawnInput.argv).toContain("--session-id"); expect(spawnInput.argv).toContain("hi"); }); - it("uses --resume when a claude session id is provided", async () => { - mocks.spawn.mockResolvedValueOnce( + it("starts fresh when only a legacy claude session id is provided", async () => { + supervisorSpawnMock.mockResolvedValueOnce( createManagedRun(Promise.resolve(successExit({ message: "ok", session_id: "sid-2" }))), ); @@ -123,11 +112,11 @@ describe("runClaudeCliAgent", () => { claudeSessionId: "c9d7b831-1c31-4d22-80b9-1e50ca207d4b", }); - expect(mocks.spawn).toHaveBeenCalledTimes(1); - const spawnInput = mocks.spawn.mock.calls[0]?.[0] as { argv: string[] }; - expect(spawnInput.argv).toContain("--resume"); - expect(spawnInput.argv).toContain("c9d7b831-1c31-4d22-80b9-1e50ca207d4b"); - expect(spawnInput.argv).not.toContain("--session-id"); + expect(supervisorSpawnMock).toHaveBeenCalledTimes(1); + const spawnInput = supervisorSpawnMock.mock.calls[0]?.[0] as { argv: string[] }; + expect(spawnInput.argv).not.toContain("--resume"); + expect(spawnInput.argv).not.toContain("c9d7b831-1c31-4d22-80b9-1e50ca207d4b"); + expect(spawnInput.argv).toContain("--session-id"); expect(spawnInput.argv).toContain("hi"); }); @@ -135,7 +124,7 @@ describe("runClaudeCliAgent", () => { const firstDeferred = createDeferred>(); const secondDeferred = createDeferred>(); - mocks.spawn + supervisorSpawnMock .mockResolvedValueOnce(createManagedRun(firstDeferred.promise)) .mockResolvedValueOnce(createManagedRun(secondDeferred.promise)); @@ -159,11 +148,11 @@ describe("runClaudeCliAgent", () => { runId: "run-2", }); - await waitForCalls(mocks.spawn, 1); + await waitForCalls(supervisorSpawnMock, 1); firstDeferred.resolve(successExit({ message: "ok", session_id: "sid-1" })); - await waitForCalls(mocks.spawn, 2); + await waitForCalls(supervisorSpawnMock, 2); secondDeferred.resolve(successExit({ message: "ok", session_id: "sid-2" })); diff --git a/src/agents/cli-runner.test-support.ts b/src/agents/cli-runner.test-support.ts index d38f23fbde5..9d936575319 100644 --- a/src/agents/cli-runner.test-support.ts +++ b/src/agents/cli-runner.test-support.ts @@ -1,24 +1,16 @@ import fs from "node:fs/promises"; import { beforeEach, vi } from "vitest"; +import { buildAnthropicCliBackend } from "../../extensions/anthropic/test-api.js"; +import { buildGoogleGeminiCliBackend } from "../../extensions/google/test-api.js"; +import { buildOpenAICodexCliBackend } from "../../extensions/openai/test-api.js"; import type { OpenClawConfig } from "../config/config.js"; import { createEmptyPluginRegistry } from "../plugins/registry.js"; import { setActivePluginRegistry } from "../plugins/runtime.js"; -import type { CliBackendPlugin } from "../plugins/types.js"; -import { loadBundledPluginTestApiSync } from "../test-utils/bundled-plugin-public-surface.js"; -import { mergeMockedModule } from "../test-utils/vitest-module-mocks.js"; +import { setCliRunnerExecuteTestDeps } from "./cli-runner/execute.js"; +import { setCliRunnerPrepareTestDeps } from "./cli-runner/prepare.js"; import type { EmbeddedContextFile } from "./pi-embedded-helpers.js"; import type { WorkspaceBootstrapFile } from "./workspace.js"; -const { buildAnthropicCliBackend } = loadBundledPluginTestApiSync<{ - buildAnthropicCliBackend: () => CliBackendPlugin; -}>("anthropic"); -const { buildGoogleGeminiCliBackend } = loadBundledPluginTestApiSync<{ - buildGoogleGeminiCliBackend: () => CliBackendPlugin; -}>("google"); -const { buildOpenAICodexCliBackend } = loadBundledPluginTestApiSync<{ - buildOpenAICodexCliBackend: () => CliBackendPlugin; -}>("openai"); - export const supervisorSpawnMock = vi.fn(); export const enqueueSystemEventMock = vi.fn(); export const requestHeartbeatNowMock = vi.fn(); @@ -39,7 +31,7 @@ const hoisted = vi.hoisted(() => { }; }); -vi.mock("../process/supervisor/index.js", () => ({ +setCliRunnerExecuteTestDeps({ getProcessSupervisor: () => ({ spawn: (...args: unknown[]) => supervisorSpawnMock(...args), cancel: vi.fn(), @@ -47,25 +39,14 @@ vi.mock("../process/supervisor/index.js", () => ({ reconcileOrphans: vi.fn(), getRecord: vi.fn(), }), -})); - -vi.mock("../infra/system-events.js", () => ({ enqueueSystemEvent: (...args: unknown[]) => enqueueSystemEventMock(...args), -})); - -vi.mock("../infra/heartbeat-wake.js", async (importOriginal) => { - return await mergeMockedModule( - await importOriginal(), - () => ({ - requestHeartbeatNow: (...args: unknown[]) => requestHeartbeatNowMock(...args), - }), - ); + requestHeartbeatNow: (...args: unknown[]) => requestHeartbeatNowMock(...args), }); -vi.mock("./bootstrap-files.js", () => ({ +setCliRunnerPrepareTestDeps({ makeBootstrapWarn: () => () => {}, resolveBootstrapContextForRun: hoisted.resolveBootstrapContextForRunMock, -})); +}); type MockRunExit = { reason: @@ -160,37 +141,18 @@ export async function setupCliRunnerTestModule() { bootstrapFiles: [], contextFiles: [], }); - - vi.resetModules(); - vi.doMock("../process/supervisor/index.js", () => ({ - getProcessSupervisor: () => ({ - spawn: (...args: unknown[]) => supervisorSpawnMock(...args), - cancel: vi.fn(), - cancelScope: vi.fn(), - reconcileOrphans: vi.fn(), - getRecord: vi.fn(), - }), - })); - vi.doMock("../infra/system-events.js", () => ({ - enqueueSystemEvent: (...args: unknown[]) => enqueueSystemEventMock(...args), - })); - vi.doMock("../infra/heartbeat-wake.js", async () => { - return await mergeMockedModule( - await vi.importActual( - "../infra/heartbeat-wake.js", - ), - () => ({ - requestHeartbeatNow: (...args: unknown[]) => requestHeartbeatNowMock(...args), - }), - ); - }); - vi.doMock("./bootstrap-files.js", () => ({ - makeBootstrapWarn: () => () => {}, - resolveBootstrapContextForRun: hoisted.resolveBootstrapContextForRunMock, - })); return (await import("./cli-runner.js")).runCliAgent; } +export async function setupClaudeCliRunnerTestModule() { + const runCliAgent = await setupCliRunnerTestModule(); + return (params: Parameters[0]) => + runCliAgent({ + ...params, + provider: params.provider ?? "claude-cli", + }); +} + export function stubBootstrapContext(params: { bootstrapFiles: WorkspaceBootstrapFile[]; contextFiles: EmbeddedContextFile[];