From 29a830dd89d2d587b74c5d7b85ba1f59fb73b688 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 4 Apr 2026 19:52:25 +0900 Subject: [PATCH] test: remove remaining legacy warning mock leaks --- src/hooks/legacy-plugin-toast/hook.test.ts | 61 ++++++++++--------- src/hooks/legacy-plugin-toast/hook.ts | 19 ++++-- .../log-legacy-plugin-startup-warning.test.ts | 47 ++++++++------ .../log-legacy-plugin-startup-warning.ts | 18 ++++-- 4 files changed, 86 insertions(+), 59 deletions(-) diff --git a/src/hooks/legacy-plugin-toast/hook.test.ts b/src/hooks/legacy-plugin-toast/hook.test.ts index 4f889aefa..dedf9834f 100644 --- a/src/hooks/legacy-plugin-toast/hook.test.ts +++ b/src/hooks/legacy-plugin-toast/hook.test.ts @@ -1,5 +1,6 @@ import { afterAll, beforeEach, describe, expect, it, mock } from "bun:test" import type { MigrationResult } from "./auto-migrate-runner" +import { createLegacyPluginToastHook } from "./hook" const mockCheckForLegacyPluginEntry = mock(() => ({ hasLegacyEntry: false, @@ -40,24 +41,6 @@ function createEvent(type: string, parentID?: string) { } } -async function importFreshModule() { - mock.module("../../shared/legacy-plugin-warning", () => ({ - checkForLegacyPluginEntry: mockCheckForLegacyPluginEntry, - })) - - mock.module("../../shared/logger", () => ({ - log: mockLog, - })) - - mock.module("./auto-migrate-runner", () => ({ - autoMigrateLegacyPluginEntry: mockAutoMigrate, - })) - - const module = await import(`./hook?t=${Date.now()}-${Math.random()}`) - mock.restore() - return module -} - describe("createLegacyPluginToastHook", () => { beforeEach(() => { mockCheckForLegacyPluginEntry.mockReset() @@ -77,8 +60,11 @@ describe("createLegacyPluginToastHook", () => { describe("#given no legacy entry exists", () => { it("#then does not show a toast", async () => { // given - const { createLegacyPluginToastHook } = await importFreshModule() - const hook = createLegacyPluginToastHook(createMockCtx()) + const hook = createLegacyPluginToastHook(createMockCtx(), { + checkForLegacyPluginEntry: mockCheckForLegacyPluginEntry, + log: mockLog, + autoMigrateLegacyPluginEntry: mockAutoMigrate, + }) // when await hook.event(createEvent("session.created")) @@ -102,8 +88,11 @@ describe("createLegacyPluginToastHook", () => { to: "oh-my-openagent", configPath: "/tmp/opencode.json", }) - const { createLegacyPluginToastHook } = await importFreshModule() - const hook = createLegacyPluginToastHook(createMockCtx()) + const hook = createLegacyPluginToastHook(createMockCtx(), { + checkForLegacyPluginEntry: mockCheckForLegacyPluginEntry, + log: mockLog, + autoMigrateLegacyPluginEntry: mockAutoMigrate, + }) // when await hook.event(createEvent("session.created")) @@ -129,8 +118,11 @@ describe("createLegacyPluginToastHook", () => { to: null, configPath: "/tmp/opencode.json", }) - const { createLegacyPluginToastHook } = await importFreshModule() - const hook = createLegacyPluginToastHook(createMockCtx()) + const hook = createLegacyPluginToastHook(createMockCtx(), { + checkForLegacyPluginEntry: mockCheckForLegacyPluginEntry, + log: mockLog, + autoMigrateLegacyPluginEntry: mockAutoMigrate, + }) // when await hook.event(createEvent("session.created")) @@ -156,8 +148,11 @@ describe("createLegacyPluginToastHook", () => { to: "oh-my-openagent", configPath: "/tmp/opencode.json", }) - const { createLegacyPluginToastHook } = await importFreshModule() - const hook = createLegacyPluginToastHook(createMockCtx()) + const hook = createLegacyPluginToastHook(createMockCtx(), { + checkForLegacyPluginEntry: mockCheckForLegacyPluginEntry, + log: mockLog, + autoMigrateLegacyPluginEntry: mockAutoMigrate, + }) // when await hook.event(createEvent("session.created")) @@ -176,8 +171,11 @@ describe("createLegacyPluginToastHook", () => { hasCanonicalEntry: false, legacyEntries: ["oh-my-opencode"], }) - const { createLegacyPluginToastHook } = await importFreshModule() - const hook = createLegacyPluginToastHook(createMockCtx()) + const hook = createLegacyPluginToastHook(createMockCtx(), { + checkForLegacyPluginEntry: mockCheckForLegacyPluginEntry, + log: mockLog, + autoMigrateLegacyPluginEntry: mockAutoMigrate, + }) // when await hook.event(createEvent("session.deleted")) @@ -195,8 +193,11 @@ describe("createLegacyPluginToastHook", () => { hasCanonicalEntry: false, legacyEntries: ["oh-my-opencode"], }) - const { createLegacyPluginToastHook } = await importFreshModule() - const hook = createLegacyPluginToastHook(createMockCtx()) + const hook = createLegacyPluginToastHook(createMockCtx(), { + checkForLegacyPluginEntry: mockCheckForLegacyPluginEntry, + log: mockLog, + autoMigrateLegacyPluginEntry: mockAutoMigrate, + }) // when await hook.event(createEvent("session.created", "parent-session-id")) diff --git a/src/hooks/legacy-plugin-toast/hook.ts b/src/hooks/legacy-plugin-toast/hook.ts index 69f971f25..352925cc6 100644 --- a/src/hooks/legacy-plugin-toast/hook.ts +++ b/src/hooks/legacy-plugin-toast/hook.ts @@ -5,8 +5,17 @@ import { log } from "../../shared/logger" import { LEGACY_PLUGIN_NAME, PLUGIN_NAME } from "../../shared/plugin-identity" import { autoMigrateLegacyPluginEntry } from "./auto-migrate-runner" -export function createLegacyPluginToastHook(ctx: PluginInput) { +type LegacyPluginToastDeps = { + checkForLegacyPluginEntry?: typeof checkForLegacyPluginEntry + log?: typeof log + autoMigrateLegacyPluginEntry?: typeof autoMigrateLegacyPluginEntry +} + +export function createLegacyPluginToastHook(ctx: PluginInput, deps: LegacyPluginToastDeps = {}) { let fired = false + const checkForLegacyPluginEntryFn = deps.checkForLegacyPluginEntry ?? checkForLegacyPluginEntry + const logFn = deps.log ?? log + const autoMigrateLegacyPluginEntryFn = deps.autoMigrateLegacyPluginEntry ?? autoMigrateLegacyPluginEntry return { event: async ({ event }: { event: { type: string; properties?: unknown } }) => { @@ -17,13 +26,13 @@ export function createLegacyPluginToastHook(ctx: PluginInput) { fired = true - const result = checkForLegacyPluginEntry() + const result = checkForLegacyPluginEntryFn() if (!result.hasLegacyEntry) return - const migration = autoMigrateLegacyPluginEntry() + const migration = autoMigrateLegacyPluginEntryFn() if (migration.migrated) { - log("[legacy-plugin-toast] Auto-migrated opencode.json plugin entry", { + logFn("[legacy-plugin-toast] Auto-migrated opencode.json plugin entry", { from: migration.from, to: migration.to, }) @@ -39,7 +48,7 @@ export function createLegacyPluginToastHook(ctx: PluginInput) { }) .catch(() => {}) } else { - log("[legacy-plugin-toast] Legacy entry detected but migration failed", { + logFn("[legacy-plugin-toast] Legacy entry detected but migration failed", { legacyEntries: result.legacyEntries, }) diff --git a/src/shared/log-legacy-plugin-startup-warning.test.ts b/src/shared/log-legacy-plugin-startup-warning.test.ts index 421d2a7f3..d8f84d60e 100644 --- a/src/shared/log-legacy-plugin-startup-warning.test.ts +++ b/src/shared/log-legacy-plugin-startup-warning.test.ts @@ -2,6 +2,7 @@ import { afterAll, afterEach, beforeEach, describe, expect, it, mock, spyOn } from "bun:test" import type { LegacyPluginCheckResult } from "./legacy-plugin-warning" +import { logLegacyPluginStartupWarning } from "./log-legacy-plugin-startup-warning" function createLegacyPluginCheckResult( overrides: Partial = {}, @@ -25,22 +26,8 @@ afterAll(() => { }) async function importFreshStartupWarningModule(): Promise { - mock.module("./legacy-plugin-warning", () => ({ - checkForLegacyPluginEntry: mockCheckForLegacyPluginEntry, - })) - - mock.module("./logger", () => ({ - log: mockLog, - })) - - mock.module("./migrate-legacy-plugin-entry", () => ({ - migrateLegacyPluginEntry: mockMigrateLegacyPluginEntry, - })) - - const module = await import(`./log-legacy-plugin-startup-warning?test=${Date.now()}-${Math.random()}`) - mock.restore() consoleWarnSpy = spyOn(console, "warn").mockImplementation(() => {}) - return module + return { logLegacyPluginStartupWarning } } describe("logLegacyPluginStartupWarning", () => { @@ -69,7 +56,11 @@ describe("logLegacyPluginStartupWarning", () => { const { logLegacyPluginStartupWarning } = await importFreshStartupWarningModule() //#when - logLegacyPluginStartupWarning() + logLegacyPluginStartupWarning({ + checkForLegacyPluginEntry: mockCheckForLegacyPluginEntry, + log: mockLog, + migrateLegacyPluginEntry: mockMigrateLegacyPluginEntry, + }) //#then expect(mockLog).toHaveBeenCalledTimes(1) @@ -93,7 +84,11 @@ describe("logLegacyPluginStartupWarning", () => { const { logLegacyPluginStartupWarning } = await importFreshStartupWarningModule() //#when - logLegacyPluginStartupWarning() + logLegacyPluginStartupWarning({ + checkForLegacyPluginEntry: mockCheckForLegacyPluginEntry, + log: mockLog, + migrateLegacyPluginEntry: mockMigrateLegacyPluginEntry, + }) //#then expect(consoleWarnSpy).toHaveBeenCalled() @@ -112,7 +107,11 @@ describe("logLegacyPluginStartupWarning", () => { const { logLegacyPluginStartupWarning } = await importFreshStartupWarningModule() //#when - logLegacyPluginStartupWarning() + logLegacyPluginStartupWarning({ + checkForLegacyPluginEntry: mockCheckForLegacyPluginEntry, + log: mockLog, + migrateLegacyPluginEntry: mockMigrateLegacyPluginEntry, + }) //#then expect(mockMigrateLegacyPluginEntry).toHaveBeenCalledWith("/tmp/opencode.json") @@ -125,7 +124,11 @@ describe("logLegacyPluginStartupWarning", () => { const { logLegacyPluginStartupWarning } = await importFreshStartupWarningModule() //#when - logLegacyPluginStartupWarning() + logLegacyPluginStartupWarning({ + checkForLegacyPluginEntry: mockCheckForLegacyPluginEntry, + log: mockLog, + migrateLegacyPluginEntry: mockMigrateLegacyPluginEntry, + }) //#then expect(mockLog).not.toHaveBeenCalled() @@ -145,7 +148,11 @@ describe("logLegacyPluginStartupWarning", () => { const { logLegacyPluginStartupWarning } = await importFreshStartupWarningModule() //#when - logLegacyPluginStartupWarning() + logLegacyPluginStartupWarning({ + checkForLegacyPluginEntry: mockCheckForLegacyPluginEntry, + log: mockLog, + migrateLegacyPluginEntry: mockMigrateLegacyPluginEntry, + }) //#then const calls = consoleWarnSpy.mock.calls.map((call: string[]) => call[0] ?? "") diff --git a/src/shared/log-legacy-plugin-startup-warning.ts b/src/shared/log-legacy-plugin-startup-warning.ts index 5382ac86e..cc8be67e2 100644 --- a/src/shared/log-legacy-plugin-startup-warning.ts +++ b/src/shared/log-legacy-plugin-startup-warning.ts @@ -4,15 +4,25 @@ import { migrateLegacyPluginEntry } from "./migrate-legacy-plugin-entry" import { toCanonicalEntry } from "./plugin-entry-migrator" import { LEGACY_PLUGIN_NAME, PLUGIN_NAME } from "./plugin-identity" -export function logLegacyPluginStartupWarning(): void { - const result = checkForLegacyPluginEntry() +type LogLegacyPluginStartupWarningDeps = { + checkForLegacyPluginEntry?: typeof checkForLegacyPluginEntry + log?: typeof log + migrateLegacyPluginEntry?: typeof migrateLegacyPluginEntry +} + +export function logLegacyPluginStartupWarning(deps: LogLegacyPluginStartupWarningDeps = {}): void { + const checkForLegacyPluginEntryFn = deps.checkForLegacyPluginEntry ?? checkForLegacyPluginEntry + const logFn = deps.log ?? log + const migrateLegacyPluginEntryFn = deps.migrateLegacyPluginEntry ?? migrateLegacyPluginEntry + + const result = checkForLegacyPluginEntryFn() if (!result.hasLegacyEntry) { return } const suggestedEntries = result.legacyEntries.map(toCanonicalEntry) - log("[OhMyOpenCodePlugin] Legacy plugin entry detected in OpenCode config", { + logFn("[OhMyOpenCodePlugin] Legacy plugin entry detected in OpenCode config", { legacyEntries: result.legacyEntries, suggestedEntries, hasCanonicalEntry: result.hasCanonicalEntry, @@ -24,7 +34,7 @@ export function logLegacyPluginStartupWarning(): void { + ` Attempting auto-migration...`, ) - const migrated = migrateLegacyPluginEntry(result.configPath!) + const migrated = migrateLegacyPluginEntryFn(result.configPath!) if (migrated) { console.warn(`[oh-my-openagent] Auto-migrated opencode.json: ${result.legacyEntries.join(", ")} -> ${suggestedEntries.join(", ")}`) } else {