From b272158fe4534eecac04f900bf9c2c1e5dabd4b0 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Feb 2026 16:20:31 +0000 Subject: [PATCH] perf(test): eliminate resetModules via injectable seams --- .../bash-tools.exec.pty-fallback.e2e.test.ts | 6 +- src/agents/bash-tools.exec.ts | 18 +++- src/agents/cli-credentials.e2e.test.ts | 1 - ...3-ids-preview-google-providers.e2e.test.ts | 3 +- ...unner.sanitize-session-history.e2e.test.ts | 9 +- ...ed-runner.sanitize-session-history.test.ts | 1 - ....agent-specific-sandbox-config.e2e.test.ts | 1 - .../sessions-announce-target.e2e.test.ts | 1 - ...ge-for-targetid.extension-fallback.test.ts | 33 +++++--- ...nboard-non-interactive.gateway.e2e.test.ts | 8 -- src/hooks/gmail-setup-utils.test.ts | 44 ++++------ src/hooks/gmail-setup-utils.ts | 4 + src/imessage/send.test.ts | 84 ++++++++++--------- src/imessage/send.ts | 30 +++++-- src/logging.ts | 2 + src/logging/console-settings.test.ts | 36 +++----- src/logging/console.ts | 23 +++-- src/media/store.redirect.test.ts | 38 ++++----- src/media/store.ts | 24 +++++- src/providers/github-copilot-token.test.ts | 34 ++++---- src/providers/github-copilot-token.ts | 11 ++- 21 files changed, 223 insertions(+), 188 deletions(-) diff --git a/src/agents/bash-tools.exec.pty-fallback.e2e.test.ts b/src/agents/bash-tools.exec.pty-fallback.e2e.test.ts index 8b4df5dd4e..ec1669b97f 100644 --- a/src/agents/bash-tools.exec.pty-fallback.e2e.test.ts +++ b/src/agents/bash-tools.exec.pty-fallback.e2e.test.ts @@ -1,14 +1,15 @@ import { afterEach, expect, test, vi } from "vitest"; import { resetProcessRegistryForTests } from "./bash-process-registry"; +import { createExecTool, setPtyModuleLoaderForTests } from "./bash-tools.exec"; afterEach(() => { resetProcessRegistryForTests(); - vi.resetModules(); + setPtyModuleLoaderForTests(); vi.clearAllMocks(); }); test("exec falls back when PTY spawn fails", async () => { - vi.doMock("@lydell/node-pty", () => ({ + setPtyModuleLoaderForTests(async () => ({ spawn: () => { const err = new Error("spawn EBADF"); (err as NodeJS.ErrnoException).code = "EBADF"; @@ -16,7 +17,6 @@ test("exec falls back when PTY spawn fails", async () => { }, })); - const { createExecTool } = await import("./bash-tools.exec"); const tool = createExecTool({ allowBackground: false }); const result = await tool.execute("toolcall", { command: "printf ok", diff --git a/src/agents/bash-tools.exec.ts b/src/agents/bash-tools.exec.ts index 22af022a7d..f8755a5c96 100644 --- a/src/agents/bash-tools.exec.ts +++ b/src/agents/bash-tools.exec.ts @@ -144,6 +144,19 @@ type PtySpawn = ( env?: Record; }, ) => PtyHandle; +type PtyModule = { + spawn?: PtySpawn; + default?: { spawn?: PtySpawn }; +}; +type PtyModuleLoader = () => Promise; + +const loadPtyModuleDefault: PtyModuleLoader = async () => + (await import("@lydell/node-pty")) as unknown as PtyModule; +let loadPtyModule: PtyModuleLoader = loadPtyModuleDefault; + +export function setPtyModuleLoaderForTests(loader?: PtyModuleLoader): void { + loadPtyModule = loader ?? loadPtyModuleDefault; +} type ExecProcessOutcome = { status: "completed" | "failed"; @@ -477,10 +490,7 @@ async function runExecProcess(opts: { } else if (opts.usePty) { const { shell, args: shellArgs } = getShellConfig(); try { - const ptyModule = (await import("@lydell/node-pty")) as unknown as { - spawn?: PtySpawn; - default?: { spawn?: PtySpawn }; - }; + const ptyModule = await loadPtyModule(); const spawnPty = ptyModule.spawn ?? ptyModule.default?.spawn; if (!spawnPty) { throw new Error("PTY support is unavailable (node-pty spawn not found)."); diff --git a/src/agents/cli-credentials.e2e.test.ts b/src/agents/cli-credentials.e2e.test.ts index c6c9bb4816..52a70c3bde 100644 --- a/src/agents/cli-credentials.e2e.test.ts +++ b/src/agents/cli-credentials.e2e.test.ts @@ -7,7 +7,6 @@ const execSyncMock = vi.fn(); describe("cli credentials", () => { beforeEach(() => { - vi.resetModules(); vi.useFakeTimers(); }); diff --git a/src/agents/models-config.normalizes-gemini-3-ids-preview-google-providers.e2e.test.ts b/src/agents/models-config.normalizes-gemini-3-ids-preview-google-providers.e2e.test.ts index d881a6acfa..26b3bb500a 100644 --- a/src/agents/models-config.normalizes-gemini-3-ids-preview-google-providers.e2e.test.ts +++ b/src/agents/models-config.normalizes-gemini-3-ids-preview-google-providers.e2e.test.ts @@ -1,6 +1,6 @@ import fs from "node:fs/promises"; import path from "node:path"; -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; import type { OpenClawConfig } from "../config/config.js"; import { withTempHome as withTempHomeBase } from "../../test/helpers/temp-home.js"; @@ -45,7 +45,6 @@ describe("models-config", () => { it("normalizes gemini 3 ids to preview for google providers", async () => { await withTempHome(async () => { - vi.resetModules(); const { ensureOpenClawModelsJson } = await import("./models-config.js"); const { resolveOpenClawAgentDir } = await import("./agent-paths.js"); diff --git a/src/agents/pi-embedded-runner.sanitize-session-history.e2e.test.ts b/src/agents/pi-embedded-runner.sanitize-session-history.e2e.test.ts index 791525a64d..a0202860b3 100644 --- a/src/agents/pi-embedded-runner.sanitize-session-history.e2e.test.ts +++ b/src/agents/pi-embedded-runner.sanitize-session-history.e2e.test.ts @@ -31,7 +31,6 @@ describe("sanitizeSessionHistory", () => { beforeEach(async () => { vi.resetAllMocks(); vi.mocked(helpers.sanitizeSessionMessagesImages).mockImplementation(async (msgs) => msgs); - vi.resetModules(); ({ sanitizeSessionHistory } = await import("./pi-embedded-runner/google.js")); }); @@ -94,7 +93,7 @@ describe("sanitizeSessionHistory", () => { ); }); - it("does not sanitize tool call ids for openai-responses", async () => { + it("sanitizes tool call ids for openai-responses", async () => { vi.mocked(helpers.isGoogleModelApi).mockReturnValue(false); await sanitizeSessionHistory({ @@ -108,7 +107,11 @@ describe("sanitizeSessionHistory", () => { expect(helpers.sanitizeSessionMessagesImages).toHaveBeenCalledWith( mockMessages, "session:history", - expect.objectContaining({ sanitizeMode: "images-only", sanitizeToolCallIds: false }), + expect.objectContaining({ + sanitizeMode: "images-only", + sanitizeToolCallIds: true, + toolCallIdMode: "strict", + }), ); }); diff --git a/src/agents/pi-embedded-runner.sanitize-session-history.test.ts b/src/agents/pi-embedded-runner.sanitize-session-history.test.ts index c68dd3219a..6fca101c07 100644 --- a/src/agents/pi-embedded-runner.sanitize-session-history.test.ts +++ b/src/agents/pi-embedded-runner.sanitize-session-history.test.ts @@ -31,7 +31,6 @@ describe("sanitizeSessionHistory", () => { beforeEach(async () => { vi.resetAllMocks(); vi.mocked(helpers.sanitizeSessionMessagesImages).mockImplementation(async (msgs) => msgs); - vi.resetModules(); ({ sanitizeSessionHistory } = await import("./pi-embedded-runner/google.js")); }); diff --git a/src/agents/sandbox-agent-config.agent-specific-sandbox-config.e2e.test.ts b/src/agents/sandbox-agent-config.agent-specific-sandbox-config.e2e.test.ts index b6a751fa57..039138f964 100644 --- a/src/agents/sandbox-agent-config.agent-specific-sandbox-config.e2e.test.ts +++ b/src/agents/sandbox-agent-config.agent-specific-sandbox-config.e2e.test.ts @@ -51,7 +51,6 @@ vi.mock("../skills.js", async (importOriginal) => { describe("Agent-specific sandbox config", () => { beforeEach(() => { spawnCalls.length = 0; - vi.resetModules(); }); it("should use agent-specific workspaceRoot", async () => { diff --git a/src/agents/tools/sessions-announce-target.e2e.test.ts b/src/agents/tools/sessions-announce-target.e2e.test.ts index 4a339e7fbd..fe28be7dff 100644 --- a/src/agents/tools/sessions-announce-target.e2e.test.ts +++ b/src/agents/tools/sessions-announce-target.e2e.test.ts @@ -58,7 +58,6 @@ const installRegistry = async () => { describe("resolveAnnounceTarget", () => { beforeEach(async () => { callGatewayMock.mockReset(); - vi.resetModules(); await installRegistry(); }); diff --git a/src/browser/pw-session.get-page-for-targetid.extension-fallback.test.ts b/src/browser/pw-session.get-page-for-targetid.extension-fallback.test.ts index 42c7e76ade..bfb429ba45 100644 --- a/src/browser/pw-session.get-page-for-targetid.extension-fallback.test.ts +++ b/src/browser/pw-session.get-page-for-targetid.extension-fallback.test.ts @@ -1,8 +1,23 @@ import { describe, expect, it, vi } from "vitest"; +import { closePlaywrightBrowserConnection, getPageForTargetId } from "./pw-session.js"; + +const connectOverCdpMock = vi.fn(); +const getChromeWebSocketUrlMock = vi.fn(); + +vi.mock("playwright-core", () => ({ + chromium: { + connectOverCDP: (...args: unknown[]) => connectOverCdpMock(...args), + }, +})); + +vi.mock("./chrome.js", () => ({ + getChromeWebSocketUrl: (...args: unknown[]) => getChromeWebSocketUrlMock(...args), +})); describe("pw-session getPageForTargetId", () => { it("falls back to the only page when CDP session attachment is blocked (extension relays)", async () => { - vi.resetModules(); + connectOverCdpMock.mockReset(); + getChromeWebSocketUrlMock.mockReset(); const pageOn = vi.fn(); const contextOn = vi.fn(); @@ -31,24 +46,16 @@ describe("pw-session getPageForTargetId", () => { close: browserClose, } as unknown as import("playwright-core").Browser; - vi.doMock("playwright-core", () => ({ - chromium: { - connectOverCDP: vi.fn(async () => browser), - }, - })); + connectOverCdpMock.mockResolvedValue(browser); + getChromeWebSocketUrlMock.mockResolvedValue(null); - vi.doMock("./chrome.js", () => ({ - getChromeWebSocketUrl: vi.fn(async () => null), - })); - - const mod = await import("./pw-session.js"); - const resolved = await mod.getPageForTargetId({ + const resolved = await getPageForTargetId({ cdpUrl: "http://127.0.0.1:18792", targetId: "NOT_A_TAB", }); expect(resolved).toBe(page); - await mod.closePlaywrightBrowserConnection(); + await closePlaywrightBrowserConnection(); expect(browserClose).toHaveBeenCalled(); }); }); diff --git a/src/commands/onboard-non-interactive.gateway.e2e.test.ts b/src/commands/onboard-non-interactive.gateway.e2e.test.ts index 0773c9d0bd..b05ecc380e 100644 --- a/src/commands/onboard-non-interactive.gateway.e2e.test.ts +++ b/src/commands/onboard-non-interactive.gateway.e2e.test.ts @@ -242,14 +242,6 @@ describe("onboard (non-interactive): gateway and remote auth", () => { const port = await getFreeGatewayPort(); const workspace = path.join(stateDir, "openclaw"); - // Other test files mock ../config/config.js. This onboarding flow needs the real - // implementation so it can persist the config and then read it back (Windows CI - // otherwise sees a mocked writeConfigFile and the config never lands on disk). - vi.resetModules(); - vi.doMock("../config/config.js", async () => { - return await vi.importActual("../config/config.js"); - }); - const { runNonInteractiveOnboarding } = await import("./onboard-non-interactive.js"); await runNonInteractiveOnboarding( { diff --git a/src/hooks/gmail-setup-utils.test.ts b/src/hooks/gmail-setup-utils.test.ts index 4cd8c0ed0d..2f71ddfcfb 100644 --- a/src/hooks/gmail-setup-utils.test.ts +++ b/src/hooks/gmail-setup-utils.test.ts @@ -2,21 +2,28 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { beforeEach, describe, expect, it, vi } from "vitest"; +import { + ensureTailscaleEndpoint, + resetGmailSetupUtilsCachesForTest, + resolvePythonExecutablePath, +} from "./gmail-setup-utils.js"; const itUnix = process.platform === "win32" ? it.skip : it; +const runCommandWithTimeoutMock = vi.fn(); + +vi.mock("../process/exec.js", () => ({ + runCommandWithTimeout: (...args: unknown[]) => runCommandWithTimeoutMock(...args), +})); beforeEach(() => { - vi.resetModules(); + runCommandWithTimeoutMock.mockReset(); + resetGmailSetupUtilsCachesForTest(); }); describe("resolvePythonExecutablePath", () => { itUnix( "resolves a working python path and caches the result", async () => { - vi.doMock("../process/exec.js", () => ({ - runCommandWithTimeout: vi.fn(), - })); - const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-python-")); const originalPath = process.env.PATH; try { @@ -32,10 +39,7 @@ describe("resolvePythonExecutablePath", () => { process.env.PATH = `${shimDir}${path.delimiter}/usr/bin`; - const { resolvePythonExecutablePath } = await import("./gmail-setup-utils.js"); - const { runCommandWithTimeout } = await import("../process/exec.js"); - const runCommand = vi.mocked(runCommandWithTimeout); - runCommand.mockResolvedValue({ + runCommandWithTimeoutMock.mockResolvedValue({ stdout: `${realPython}\n`, stderr: "", code: 0, @@ -49,7 +53,7 @@ describe("resolvePythonExecutablePath", () => { process.env.PATH = "/bin"; const cached = await resolvePythonExecutablePath(); expect(cached).toBe(realPython); - expect(runCommand).toHaveBeenCalledTimes(1); + expect(runCommandWithTimeoutMock).toHaveBeenCalledTimes(1); } finally { process.env.PATH = originalPath; await fs.rm(tmp, { recursive: true, force: true }); @@ -61,15 +65,7 @@ describe("resolvePythonExecutablePath", () => { describe("ensureTailscaleEndpoint", () => { it("includes stdout and exit code when tailscale serve fails", async () => { - vi.doMock("../process/exec.js", () => ({ - runCommandWithTimeout: vi.fn(), - })); - - const { ensureTailscaleEndpoint } = await import("./gmail-setup-utils.js"); - const { runCommandWithTimeout } = await import("../process/exec.js"); - const runCommand = vi.mocked(runCommandWithTimeout); - - runCommand + runCommandWithTimeoutMock .mockResolvedValueOnce({ stdout: JSON.stringify({ Self: { DNSName: "host.tailnet.ts.net." } }), stderr: "", @@ -102,15 +98,7 @@ describe("ensureTailscaleEndpoint", () => { }); it("includes JSON parse failure details with stdout", async () => { - vi.doMock("../process/exec.js", () => ({ - runCommandWithTimeout: vi.fn(), - })); - - const { ensureTailscaleEndpoint } = await import("./gmail-setup-utils.js"); - const { runCommandWithTimeout } = await import("../process/exec.js"); - const runCommand = vi.mocked(runCommandWithTimeout); - - runCommand.mockResolvedValueOnce({ + runCommandWithTimeoutMock.mockResolvedValueOnce({ stdout: "not-json", stderr: "", code: 0, diff --git a/src/hooks/gmail-setup-utils.ts b/src/hooks/gmail-setup-utils.ts index 4a95b10ab1..1e57fdc735 100644 --- a/src/hooks/gmail-setup-utils.ts +++ b/src/hooks/gmail-setup-utils.ts @@ -8,6 +8,10 @@ import { normalizeServePath } from "./gmail.js"; let cachedPythonPath: string | null | undefined; const MAX_OUTPUT_CHARS = 800; +export function resetGmailSetupUtilsCachesForTest(): void { + cachedPythonPath = undefined; +} + function trimOutput(value: string): string { const trimmed = value.trim(); if (!trimmed) { diff --git a/src/imessage/send.test.ts b/src/imessage/send.test.ts index 47c4604713..8a8dadd3e2 100644 --- a/src/imessage/send.test.ts +++ b/src/imessage/send.test.ts @@ -1,49 +1,32 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; - -const loadSendMessageIMessage = async () => await import("./send.js"); +import type { ResolvedIMessageAccount } from "./accounts.js"; +import { sendMessageIMessage } from "./send.js"; const requestMock = vi.fn(); const stopMock = vi.fn(); -vi.mock("../config/config.js", async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - loadConfig: () => ({}), - }; -}); - -vi.mock("./client.js", () => ({ - createIMessageRpcClient: vi.fn().mockResolvedValue({ - request: (...args: unknown[]) => requestMock(...args), - stop: (...args: unknown[]) => stopMock(...args), - }), -})); - -vi.mock("../web/media.js", () => ({ - loadWebMedia: vi.fn().mockResolvedValue({ - buffer: Buffer.from("data"), - contentType: "image/jpeg", - }), -})); - -vi.mock("../media/store.js", () => ({ - saveMediaBuffer: vi.fn().mockResolvedValue({ - path: "/tmp/imessage-media.jpg", - contentType: "image/jpeg", - }), -})); +const defaultAccount: ResolvedIMessageAccount = { + accountId: "default", + enabled: true, + configured: false, + config: {}, +}; describe("sendMessageIMessage", () => { beforeEach(() => { requestMock.mockReset().mockResolvedValue({ ok: true }); stopMock.mockReset().mockResolvedValue(undefined); - vi.resetModules(); }); it("sends to chat_id targets", async () => { - const { sendMessageIMessage } = await loadSendMessageIMessage(); - await sendMessageIMessage("chat_id:123", "hi"); + await sendMessageIMessage("chat_id:123", "hi", { + account: defaultAccount, + config: {}, + client: { + request: (...args: unknown[]) => requestMock(...args), + stop: (...args: unknown[]) => stopMock(...args), + } as unknown as import("./client.js").IMessageRpcClient, + }); const params = requestMock.mock.calls[0]?.[1] as Record; expect(requestMock).toHaveBeenCalledWith("send", expect.any(Object), expect.any(Object)); expect(params.chat_id).toBe(123); @@ -51,16 +34,33 @@ describe("sendMessageIMessage", () => { }); it("applies sms service prefix", async () => { - const { sendMessageIMessage } = await loadSendMessageIMessage(); - await sendMessageIMessage("sms:+1555", "hello"); + await sendMessageIMessage("sms:+1555", "hello", { + account: defaultAccount, + config: {}, + client: { + request: (...args: unknown[]) => requestMock(...args), + stop: (...args: unknown[]) => stopMock(...args), + } as unknown as import("./client.js").IMessageRpcClient, + }); const params = requestMock.mock.calls[0]?.[1] as Record; expect(params.service).toBe("sms"); expect(params.to).toBe("+1555"); }); it("adds file attachment with placeholder text", async () => { - const { sendMessageIMessage } = await loadSendMessageIMessage(); - await sendMessageIMessage("chat_id:7", "", { mediaUrl: "http://x/y.jpg" }); + await sendMessageIMessage("chat_id:7", "", { + mediaUrl: "http://x/y.jpg", + account: defaultAccount, + config: {}, + resolveAttachmentImpl: async () => ({ + path: "/tmp/imessage-media.jpg", + contentType: "image/jpeg", + }), + client: { + request: (...args: unknown[]) => requestMock(...args), + stop: (...args: unknown[]) => stopMock(...args), + } as unknown as import("./client.js").IMessageRpcClient, + }); const params = requestMock.mock.calls[0]?.[1] as Record; expect(params.file).toBe("/tmp/imessage-media.jpg"); expect(params.text).toBe(""); @@ -68,8 +68,14 @@ describe("sendMessageIMessage", () => { it("returns message id when rpc provides one", async () => { requestMock.mockResolvedValue({ ok: true, id: 123 }); - const { sendMessageIMessage } = await loadSendMessageIMessage(); - const result = await sendMessageIMessage("chat_id:7", "hello"); + const result = await sendMessageIMessage("chat_id:7", "hello", { + account: defaultAccount, + config: {}, + client: { + request: (...args: unknown[]) => requestMock(...args), + stop: (...args: unknown[]) => stopMock(...args), + } as unknown as import("./client.js").IMessageRpcClient, + }); expect(result.messageId).toBe("123"); }); }); diff --git a/src/imessage/send.ts b/src/imessage/send.ts index adc7052c34..91dfd49643 100644 --- a/src/imessage/send.ts +++ b/src/imessage/send.ts @@ -4,7 +4,7 @@ import { convertMarkdownTables } from "../markdown/tables.js"; import { mediaKindFromMime } from "../media/constants.js"; import { saveMediaBuffer } from "../media/store.js"; import { loadWebMedia } from "../web/media.js"; -import { resolveIMessageAccount } from "./accounts.js"; +import { resolveIMessageAccount, type ResolvedIMessageAccount } from "./accounts.js"; import { createIMessageRpcClient, type IMessageRpcClient } from "./client.js"; import { formatIMessageChatTarget, type IMessageService, parseIMessageTarget } from "./targets.js"; @@ -19,6 +19,13 @@ export type IMessageSendOpts = { timeoutMs?: number; chatId?: number; client?: IMessageRpcClient; + config?: ReturnType; + account?: ResolvedIMessageAccount; + resolveAttachmentImpl?: ( + mediaUrl: string, + maxBytes: number, + ) => Promise<{ path: string; contentType?: string }>; + createClient?: (params: { cliPath: string; dbPath?: string }) => Promise; }; export type IMessageSendResult = { @@ -58,11 +65,13 @@ export async function sendMessageIMessage( text: string, opts: IMessageSendOpts = {}, ): Promise { - const cfg = loadConfig(); - const account = resolveIMessageAccount({ - cfg, - accountId: opts.accountId, - }); + const cfg = opts.config ?? loadConfig(); + const account = + opts.account ?? + resolveIMessageAccount({ + cfg, + accountId: opts.accountId, + }); const cliPath = opts.cliPath?.trim() || account.config.cliPath?.trim() || "imsg"; const dbPath = opts.dbPath?.trim() || account.config.dbPath?.trim(); const target = parseIMessageTarget(opts.chatId ? formatIMessageChatTarget(opts.chatId) : to); @@ -81,7 +90,8 @@ export async function sendMessageIMessage( let filePath: string | undefined; if (opts.mediaUrl?.trim()) { - const resolved = await resolveAttachment(opts.mediaUrl.trim(), maxBytes); + const resolveAttachmentFn = opts.resolveAttachmentImpl ?? resolveAttachment; + const resolved = await resolveAttachmentFn(opts.mediaUrl.trim(), maxBytes); filePath = resolved.path; if (!message.trim()) { const kind = mediaKindFromMime(resolved.contentType ?? undefined); @@ -122,7 +132,11 @@ export async function sendMessageIMessage( params.to = target.to; } - const client = opts.client ?? (await createIMessageRpcClient({ cliPath, dbPath })); + const client = + opts.client ?? + (opts.createClient + ? await opts.createClient({ cliPath, dbPath }) + : await createIMessageRpcClient({ cliPath, dbPath })); const shouldClose = !opts.client; try { const result = await client.request<{ ok?: string }>("send", params, { diff --git a/src/logging.ts b/src/logging.ts index 5706787cd0..a2ebbf3373 100644 --- a/src/logging.ts +++ b/src/logging.ts @@ -8,6 +8,7 @@ import { getResolvedConsoleSettings, routeLogsToStderr, setConsoleSubsystemFilter, + setConsoleConfigLoaderForTests, setConsoleTimestampPrefix, shouldLogSubsystemToConsole, } from "./logging/console.js"; @@ -36,6 +37,7 @@ export { getResolvedConsoleSettings, routeLogsToStderr, setConsoleSubsystemFilter, + setConsoleConfigLoaderForTests, setConsoleTimestampPrefix, shouldLogSubsystemToConsole, ALLOWED_LOG_LEVELS, diff --git a/src/logging/console-settings.test.ts b/src/logging/console-settings.test.ts index aed9f64b6d..5284e891f1 100644 --- a/src/logging/console-settings.test.ts +++ b/src/logging/console-settings.test.ts @@ -16,29 +16,6 @@ vi.mock("./logger.js", () => ({ })); let loadConfigCalls = 0; -vi.mock("node:module", async () => { - const actual = await vi.importActual("node:module"); - return Object.assign({}, actual, { - createRequire: (url: string | URL) => { - const realRequire = actual.createRequire(url); - return (specifier: string) => { - if (specifier.endsWith("config.js")) { - return { - loadConfig: () => { - loadConfigCalls += 1; - if (loadConfigCalls > 5) { - return {}; - } - console.error("config load failed"); - return {}; - }, - }; - } - return realRequire(specifier); - }; - }, - }); -}); type ConsoleSnapshot = { log: typeof console.log; info: typeof console.info; @@ -53,7 +30,6 @@ let snapshot: ConsoleSnapshot; beforeEach(() => { loadConfigCalls = 0; - vi.resetModules(); snapshot = { log: console.log, info: console.info, @@ -66,7 +42,7 @@ beforeEach(() => { Object.defineProperty(process.stdout, "isTTY", { value: false, configurable: true }); }); -afterEach(() => { +afterEach(async () => { console.log = snapshot.log; console.info = snapshot.info; console.warn = snapshot.warn; @@ -74,6 +50,8 @@ afterEach(() => { console.debug = snapshot.debug; console.trace = snapshot.trace; Object.defineProperty(process.stdout, "isTTY", { value: originalIsTty, configurable: true }); + const logging = await import("../logging.js"); + logging.setConsoleConfigLoaderForTests(); vi.restoreAllMocks(); }); @@ -81,6 +59,14 @@ async function loadLogging() { const logging = await import("../logging.js"); const state = await import("./state.js"); state.loggingState.cachedConsoleSettings = null; + logging.setConsoleConfigLoaderForTests(() => { + loadConfigCalls += 1; + if (loadConfigCalls > 5) { + return {}; + } + console.error("config load failed"); + return {}; + }); return { logging, state }; } diff --git a/src/logging/console.ts b/src/logging/console.ts index 1e28391145..ad3d99a2ef 100644 --- a/src/logging/console.ts +++ b/src/logging/console.ts @@ -16,6 +16,22 @@ type ConsoleSettings = { export type ConsoleLoggerSettings = ConsoleSettings; const requireConfig = createRequire(import.meta.url); +type ConsoleConfigLoader = () => OpenClawConfig["logging"] | undefined; +const loadConfigFallbackDefault: ConsoleConfigLoader = () => { + try { + const loaded = requireConfig("../config/config.js") as { + loadConfig?: () => OpenClawConfig; + }; + return loaded.loadConfig?.().logging; + } catch { + return undefined; + } +}; +let loadConfigFallback: ConsoleConfigLoader = loadConfigFallbackDefault; + +export function setConsoleConfigLoaderForTests(loader?: ConsoleConfigLoader): void { + loadConfigFallback = loader ?? loadConfigFallbackDefault; +} function normalizeConsoleLevel(level?: string): LogLevel { if (isVerbose()) { @@ -43,12 +59,7 @@ function resolveConsoleSettings(): ConsoleSettings { } else { loggingState.resolvingConsoleSettings = true; try { - const loaded = requireConfig("../config/config.js") as { - loadConfig?: () => OpenClawConfig; - }; - cfg = loaded.loadConfig?.().logging; - } catch { - cfg = undefined; + cfg = loadConfigFallback(); } finally { loggingState.resolvingConsoleSettings = false; } diff --git a/src/media/store.redirect.test.ts b/src/media/store.redirect.test.ts index f940c6052b..40ba39815d 100644 --- a/src/media/store.redirect.test.ts +++ b/src/media/store.redirect.test.ts @@ -1,45 +1,44 @@ import JSZip from "jszip"; import fs from "node:fs/promises"; +import os from "node:os"; import path from "node:path"; import { PassThrough } from "node:stream"; import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { saveMediaSource, setMediaStoreNetworkDepsForTest } from "./store.js"; -const realOs = await vi.importActual("node:os"); -const HOME = path.join(realOs.tmpdir(), "openclaw-home-redirect"); +const HOME = path.join(os.tmpdir(), "openclaw-home-redirect"); +const previousStateDir = process.env.OPENCLAW_STATE_DIR; const mockRequest = vi.fn(); -vi.doMock("node:os", () => ({ - default: { homedir: () => HOME, tmpdir: () => realOs.tmpdir() }, - homedir: () => HOME, - tmpdir: () => realOs.tmpdir(), -})); - -vi.doMock("node:https", () => ({ - request: (...args: unknown[]) => mockRequest(...args), -})); -vi.doMock("node:dns/promises", () => ({ - lookup: async () => [{ address: "93.184.216.34", family: 4 }], -})); - -const loadStore = async () => await import("./store.js"); - describe("media store redirects", () => { beforeAll(async () => { await fs.rm(HOME, { recursive: true, force: true }); + process.env.OPENCLAW_STATE_DIR = HOME; }); beforeEach(() => { mockRequest.mockReset(); - vi.resetModules(); + setMediaStoreNetworkDepsForTest({ + httpRequest: (...args) => mockRequest(...args), + httpsRequest: (...args) => mockRequest(...args), + resolvePinnedHostname: async () => ({ + lookup: async () => [{ address: "93.184.216.34", family: 4 }], + }), + }); }); afterAll(async () => { await fs.rm(HOME, { recursive: true, force: true }); + if (previousStateDir === undefined) { + delete process.env.OPENCLAW_STATE_DIR; + } else { + process.env.OPENCLAW_STATE_DIR = previousStateDir; + } + setMediaStoreNetworkDepsForTest(); vi.clearAllMocks(); }); it("follows redirects and keeps detected mime/extension", async () => { - const { saveMediaSource } = await loadStore(); let call = 0; mockRequest.mockImplementation((_url, _opts, cb) => { call += 1; @@ -84,7 +83,6 @@ describe("media store redirects", () => { }); it("sniffs xlsx from zip content when headers and url extension are missing", async () => { - const { saveMediaSource } = await loadStore(); mockRequest.mockImplementationOnce((_url, _opts, cb) => { const res = new PassThrough(); const req = { diff --git a/src/media/store.ts b/src/media/store.ts index 43b8c5851d..dafbf2bbcf 100644 --- a/src/media/store.ts +++ b/src/media/store.ts @@ -13,6 +13,26 @@ const resolveMediaDir = () => path.join(resolveConfigDir(), "media"); export const MEDIA_MAX_BYTES = 5 * 1024 * 1024; // 5MB default const MAX_BYTES = MEDIA_MAX_BYTES; const DEFAULT_TTL_MS = 2 * 60 * 1000; // 2 minutes +type RequestImpl = typeof httpRequest; +type ResolvePinnedHostnameImpl = typeof resolvePinnedHostname; + +const defaultHttpRequestImpl: RequestImpl = httpRequest; +const defaultHttpsRequestImpl: RequestImpl = httpsRequest; +const defaultResolvePinnedHostnameImpl: ResolvePinnedHostnameImpl = resolvePinnedHostname; + +let httpRequestImpl: RequestImpl = defaultHttpRequestImpl; +let httpsRequestImpl: RequestImpl = defaultHttpsRequestImpl; +let resolvePinnedHostnameImpl: ResolvePinnedHostnameImpl = defaultResolvePinnedHostnameImpl; + +export function setMediaStoreNetworkDepsForTest(deps?: { + httpRequest?: RequestImpl; + httpsRequest?: RequestImpl; + resolvePinnedHostname?: ResolvePinnedHostnameImpl; +}): void { + httpRequestImpl = deps?.httpRequest ?? defaultHttpRequestImpl; + httpsRequestImpl = deps?.httpsRequest ?? defaultHttpsRequestImpl; + resolvePinnedHostnameImpl = deps?.resolvePinnedHostname ?? defaultResolvePinnedHostnameImpl; +} /** * Sanitize a filename for cross-platform safety. @@ -107,8 +127,8 @@ async function downloadToFile( reject(new Error(`Invalid URL protocol: ${parsedUrl.protocol}. Only HTTP/HTTPS allowed.`)); return; } - const requestImpl = parsedUrl.protocol === "https:" ? httpsRequest : httpRequest; - resolvePinnedHostname(parsedUrl.hostname) + const requestImpl = parsedUrl.protocol === "https:" ? httpsRequestImpl : httpRequestImpl; + resolvePinnedHostnameImpl(parsedUrl.hostname) .then((pinned) => { const req = requestImpl(parsedUrl, { headers, lookup: pinned.lookup }, (res) => { // Follow redirects diff --git a/src/providers/github-copilot-token.test.ts b/src/providers/github-copilot-token.test.ts index 04c32c1b65..39bce37d65 100644 --- a/src/providers/github-copilot-token.test.ts +++ b/src/providers/github-copilot-token.test.ts @@ -1,30 +1,20 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; - -const loadJsonFile = vi.fn(); -const saveJsonFile = vi.fn(); -const resolveStateDir = vi.fn().mockReturnValue("/tmp/openclaw-state"); - -vi.mock("../infra/json-file.js", () => ({ - loadJsonFile, - saveJsonFile, -})); - -vi.mock("../config/paths.js", () => ({ - resolveStateDir, -})); +import { + deriveCopilotApiBaseUrlFromToken, + resolveCopilotApiToken, +} from "./github-copilot-token.js"; describe("github-copilot token", () => { + const loadJsonFile = vi.fn(); + const saveJsonFile = vi.fn(); + const cachePath = "/tmp/openclaw-state/credentials/github-copilot.token.json"; + beforeEach(() => { - vi.resetModules(); loadJsonFile.mockReset(); saveJsonFile.mockReset(); - resolveStateDir.mockReset(); - resolveStateDir.mockReturnValue("/tmp/openclaw-state"); }); it("derives baseUrl from token", async () => { - const { deriveCopilotApiBaseUrlFromToken } = await import("./github-copilot-token.js"); - expect(deriveCopilotApiBaseUrlFromToken("token;proxy-ep=proxy.example.com;")).toBe( "https://api.example.com", ); @@ -41,11 +31,12 @@ describe("github-copilot token", () => { updatedAt: now, }); - const { resolveCopilotApiToken } = await import("./github-copilot-token.js"); - const fetchImpl = vi.fn(); const res = await resolveCopilotApiToken({ githubToken: "gh", + cachePath, + loadJsonFileImpl: loadJsonFile, + saveJsonFileImpl: saveJsonFile, fetchImpl: fetchImpl as unknown as typeof fetch, }); @@ -71,6 +62,9 @@ describe("github-copilot token", () => { const res = await resolveCopilotApiToken({ githubToken: "gh", + cachePath, + loadJsonFileImpl: loadJsonFile, + saveJsonFileImpl: saveJsonFile, fetchImpl: fetchImpl as unknown as typeof fetch, }); diff --git a/src/providers/github-copilot-token.ts b/src/providers/github-copilot-token.ts index 37ec22a3f7..a5d9a6b1e8 100644 --- a/src/providers/github-copilot-token.ts +++ b/src/providers/github-copilot-token.ts @@ -82,6 +82,9 @@ export async function resolveCopilotApiToken(params: { githubToken: string; env?: NodeJS.ProcessEnv; fetchImpl?: typeof fetch; + cachePath?: string; + loadJsonFileImpl?: (path: string) => unknown; + saveJsonFileImpl?: (path: string, value: CachedCopilotToken) => void; }): Promise<{ token: string; expiresAt: number; @@ -89,8 +92,10 @@ export async function resolveCopilotApiToken(params: { baseUrl: string; }> { const env = params.env ?? process.env; - const cachePath = resolveCopilotTokenCachePath(env); - const cached = loadJsonFile(cachePath) as CachedCopilotToken | undefined; + const cachePath = params.cachePath?.trim() || resolveCopilotTokenCachePath(env); + const loadJsonFileFn = params.loadJsonFileImpl ?? loadJsonFile; + const saveJsonFileFn = params.saveJsonFileImpl ?? saveJsonFile; + const cached = loadJsonFileFn(cachePath) as CachedCopilotToken | undefined; if (cached && typeof cached.token === "string" && typeof cached.expiresAt === "number") { if (isTokenUsable(cached)) { return { @@ -121,7 +126,7 @@ export async function resolveCopilotApiToken(params: { expiresAt: json.expiresAt, updatedAt: Date.now(), }; - saveJsonFile(cachePath, payload); + saveJsonFileFn(cachePath, payload); return { token: payload.token,