test(ci): avoid global module mock leaks
This commit is contained in:
@@ -1,30 +1,20 @@
|
|||||||
import { afterAll, beforeEach, describe, expect, it, mock } from "bun:test"
|
import { describe, expect, it, mock, spyOn } from "bun:test"
|
||||||
import type { ClaudeCodeMcpServer } from "../claude-code-mcp-loader/types"
|
import type { ClaudeCodeMcpServer } from "../claude-code-mcp-loader/types"
|
||||||
import type { OAuthTokenData } from "../mcp-oauth/storage"
|
import type { OAuthTokenData } from "../mcp-oauth/storage"
|
||||||
import type { SkillMcpClientInfo, SkillMcpServerContext } from "./types"
|
import { unsafeTestValue } from "../../../test-support/unsafe-test-value"
|
||||||
|
import { SkillMcpManager } from "./manager"
|
||||||
|
import type { McpClient, SkillMcpClientInfo, SkillMcpServerContext } from "./types"
|
||||||
|
|
||||||
const mockGetOrCreateClient = mock(async () => {
|
type ManagerWithPrivateRetry = {
|
||||||
throw new Error("not used")
|
getOrCreateClientWithRetry: (info: SkillMcpClientInfo, config: ClaudeCodeMcpServer) => Promise<McpClient>
|
||||||
})
|
}
|
||||||
|
|
||||||
const mockGetOrCreateClientWithRetryImpl = mock(async () => ({
|
function stubClientRetry(manager: SkillMcpManager, callTool: McpClient["callTool"]): void {
|
||||||
callTool: mock(async () => ({ content: [{ type: "text", text: "unused" }] })),
|
const client = unsafeTestValue<McpClient>({
|
||||||
close: mock(async () => {}),
|
callTool,
|
||||||
}))
|
close: mock(async () => {}),
|
||||||
|
})
|
||||||
type ManagerModule = typeof import("./manager")
|
spyOn(unsafeTestValue<ManagerWithPrivateRetry>(manager), "getOrCreateClientWithRetry").mockResolvedValue(client)
|
||||||
|
|
||||||
async function importFreshManagerModule(): Promise<ManagerModule> {
|
|
||||||
mock.module("./connection", () => ({
|
|
||||||
getOrCreateClient: mockGetOrCreateClient,
|
|
||||||
getOrCreateClientWithRetryImpl: mockGetOrCreateClientWithRetryImpl,
|
|
||||||
}))
|
|
||||||
|
|
||||||
mock.module("../mcp-oauth/provider", () => ({
|
|
||||||
McpOAuthProvider: class MockMcpOAuthProvider {},
|
|
||||||
}))
|
|
||||||
|
|
||||||
return await import(new URL(`./manager.ts?oauth-retry-test=${Date.now()}-${Math.random()}`, import.meta.url).href)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
function createInfo(): SkillMcpClientInfo {
|
function createInfo(): SkillMcpClientInfo {
|
||||||
@@ -46,19 +36,9 @@ function createContext(): SkillMcpServerContext {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
afterAll(() => {
|
|
||||||
mock.restore()
|
|
||||||
})
|
|
||||||
|
|
||||||
describe("SkillMcpManager post-request OAuth retry", () => {
|
describe("SkillMcpManager post-request OAuth retry", () => {
|
||||||
beforeEach(() => {
|
|
||||||
mockGetOrCreateClient.mockClear()
|
|
||||||
mockGetOrCreateClientWithRetryImpl.mockClear()
|
|
||||||
})
|
|
||||||
|
|
||||||
it("retries the operation after a 401 refresh succeeds", async () => {
|
it("retries the operation after a 401 refresh succeeds", async () => {
|
||||||
// given
|
// given
|
||||||
const { SkillMcpManager } = await importFreshManagerModule()
|
|
||||||
const refresh = mock(async () => ({ accessToken: "refreshed-token" } satisfies OAuthTokenData))
|
const refresh = mock(async () => ({ accessToken: "refreshed-token" } satisfies OAuthTokenData))
|
||||||
const manager = new SkillMcpManager({
|
const manager = new SkillMcpManager({
|
||||||
createOAuthProvider: () => ({
|
createOAuthProvider: () => ({
|
||||||
@@ -74,7 +54,7 @@ describe("SkillMcpManager post-request OAuth retry", () => {
|
|||||||
|
|
||||||
return { content: [{ type: "text", text: "success" }] }
|
return { content: [{ type: "text", text: "success" }] }
|
||||||
})
|
})
|
||||||
mockGetOrCreateClientWithRetryImpl.mockResolvedValue({ callTool, close: mock(async () => {}) })
|
stubClientRetry(manager, callTool)
|
||||||
|
|
||||||
// when
|
// when
|
||||||
const result = await manager.callTool(createInfo(), createContext(), "test-tool", {})
|
const result = await manager.callTool(createInfo(), createContext(), "test-tool", {})
|
||||||
@@ -87,7 +67,6 @@ describe("SkillMcpManager post-request OAuth retry", () => {
|
|||||||
|
|
||||||
it("retries the operation after a 403 refresh succeeds without step-up scope", async () => {
|
it("retries the operation after a 403 refresh succeeds without step-up scope", async () => {
|
||||||
// given
|
// given
|
||||||
const { SkillMcpManager } = await importFreshManagerModule()
|
|
||||||
const refresh = mock(async () => ({ accessToken: "refreshed-token" } satisfies OAuthTokenData))
|
const refresh = mock(async () => ({ accessToken: "refreshed-token" } satisfies OAuthTokenData))
|
||||||
const manager = new SkillMcpManager({
|
const manager = new SkillMcpManager({
|
||||||
createOAuthProvider: () => ({
|
createOAuthProvider: () => ({
|
||||||
@@ -103,7 +82,7 @@ describe("SkillMcpManager post-request OAuth retry", () => {
|
|||||||
|
|
||||||
return { content: [{ type: "text", text: "success" }] }
|
return { content: [{ type: "text", text: "success" }] }
|
||||||
})
|
})
|
||||||
mockGetOrCreateClientWithRetryImpl.mockResolvedValue({ callTool, close: mock(async () => {}) })
|
stubClientRetry(manager, callTool)
|
||||||
|
|
||||||
// when
|
// when
|
||||||
const result = await manager.callTool(createInfo(), createContext(), "test-tool", {})
|
const result = await manager.callTool(createInfo(), createContext(), "test-tool", {})
|
||||||
@@ -116,7 +95,6 @@ describe("SkillMcpManager post-request OAuth retry", () => {
|
|||||||
|
|
||||||
it("propagates the auth error without retry when refresh fails", async () => {
|
it("propagates the auth error without retry when refresh fails", async () => {
|
||||||
// given
|
// given
|
||||||
const { SkillMcpManager } = await importFreshManagerModule()
|
|
||||||
const refresh = mock(async () => {
|
const refresh = mock(async () => {
|
||||||
throw new Error("refresh failed")
|
throw new Error("refresh failed")
|
||||||
})
|
})
|
||||||
@@ -130,7 +108,7 @@ describe("SkillMcpManager post-request OAuth retry", () => {
|
|||||||
const callTool = mock(async () => {
|
const callTool = mock(async () => {
|
||||||
throw new Error("401 Unauthorized")
|
throw new Error("401 Unauthorized")
|
||||||
})
|
})
|
||||||
mockGetOrCreateClientWithRetryImpl.mockResolvedValue({ callTool, close: mock(async () => {}) })
|
stubClientRetry(manager, callTool)
|
||||||
|
|
||||||
// when / then
|
// when / then
|
||||||
await expect(manager.callTool(createInfo(), createContext(), "test-tool", {})).rejects.toThrow("401 Unauthorized")
|
await expect(manager.callTool(createInfo(), createContext(), "test-tool", {})).rejects.toThrow("401 Unauthorized")
|
||||||
@@ -140,7 +118,6 @@ describe("SkillMcpManager post-request OAuth retry", () => {
|
|||||||
|
|
||||||
it("only attempts one refresh when the retried operation returns 401 again", async () => {
|
it("only attempts one refresh when the retried operation returns 401 again", async () => {
|
||||||
// given
|
// given
|
||||||
const { SkillMcpManager } = await importFreshManagerModule()
|
|
||||||
const refresh = mock(async () => ({ accessToken: "refreshed-token" } satisfies OAuthTokenData))
|
const refresh = mock(async () => ({ accessToken: "refreshed-token" } satisfies OAuthTokenData))
|
||||||
const manager = new SkillMcpManager({
|
const manager = new SkillMcpManager({
|
||||||
createOAuthProvider: () => ({
|
createOAuthProvider: () => ({
|
||||||
@@ -152,7 +129,7 @@ describe("SkillMcpManager post-request OAuth retry", () => {
|
|||||||
const callTool = mock(async () => {
|
const callTool = mock(async () => {
|
||||||
throw new Error("401 Unauthorized")
|
throw new Error("401 Unauthorized")
|
||||||
})
|
})
|
||||||
mockGetOrCreateClientWithRetryImpl.mockResolvedValue({ callTool, close: mock(async () => {}) })
|
stubClientRetry(manager, callTool)
|
||||||
|
|
||||||
// when / then
|
// when / then
|
||||||
await expect(manager.callTool(createInfo(), createContext(), "test-tool", {})).rejects.toThrow("401 Unauthorized")
|
await expect(manager.callTool(createInfo(), createContext(), "test-tool", {})).rejects.toThrow("401 Unauthorized")
|
||||||
|
|||||||
@@ -6,10 +6,6 @@ import type { OAuthProviderFactory, OAuthProviderLike } from "./types"
|
|||||||
type OAuthHandlerModule = typeof import("./oauth-handler")
|
type OAuthHandlerModule = typeof import("./oauth-handler")
|
||||||
|
|
||||||
async function importFreshOAuthHandlerModule(): Promise<OAuthHandlerModule> {
|
async function importFreshOAuthHandlerModule(): Promise<OAuthHandlerModule> {
|
||||||
mock.module("../mcp-oauth/provider", () => ({
|
|
||||||
McpOAuthProvider: class MockMcpOAuthProvider {},
|
|
||||||
}))
|
|
||||||
|
|
||||||
return await import(new URL(`./oauth-handler.ts?oauth-handler-test=${Date.now()}-${Math.random()}`, import.meta.url).href)
|
return await import(new URL(`./oauth-handler.ts?oauth-handler-test=${Date.now()}-${Math.random()}`, import.meta.url).href)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -1,47 +1,43 @@
|
|||||||
import { afterEach, describe, expect, it, mock } from "bun:test";
|
import { afterEach, describe, expect, it } from "bun:test";
|
||||||
|
import { mkdirSync, rmSync, unlinkSync, writeFileSync } from "node:fs";
|
||||||
|
import { tmpdir } from "node:os";
|
||||||
|
import { join } from "node:path";
|
||||||
|
|
||||||
|
let testRoot = "";
|
||||||
|
|
||||||
describe("findProjectRoot", () => {
|
describe("findProjectRoot", () => {
|
||||||
afterEach(async () => {
|
afterEach(() => {
|
||||||
const actualFileSystem = await import("node:fs");
|
if (testRoot) {
|
||||||
mock.module("node:fs", () => actualFileSystem);
|
rmSync(testRoot, { recursive: true, force: true });
|
||||||
|
testRoot = "";
|
||||||
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
it("memoizes repeated lookups for the same start path and resets on cache clear", async () => {
|
it("memoizes repeated lookups for the same start path and resets on cache clear", async () => {
|
||||||
// given
|
// given
|
||||||
const actualFileSystem = await import("node:fs");
|
testRoot = join(tmpdir(), `rules-project-root-${Date.now()}-${Math.random()}`);
|
||||||
const projectRoot = "/workspace/project";
|
const projectRoot = join(testRoot, "project");
|
||||||
const startPath = `${projectRoot}/src/file.ts`;
|
const sourceDirectory = join(projectRoot, "src");
|
||||||
const packageJsonPath = `${projectRoot}/package.json`;
|
const startPath = join(sourceDirectory, "file.ts");
|
||||||
|
const packageJsonPath = join(projectRoot, "package.json");
|
||||||
const existsSyncSpy = mock((path: string) => path === packageJsonPath);
|
mkdirSync(sourceDirectory, { recursive: true });
|
||||||
const statSyncSpy = mock(() => ({ isDirectory: () => false }));
|
writeFileSync(startPath, "export const value = 1;\n");
|
||||||
|
writeFileSync(packageJsonPath, "{}\n");
|
||||||
mock.module("node:fs", () => ({
|
|
||||||
...actualFileSystem,
|
|
||||||
existsSync: existsSyncSpy,
|
|
||||||
statSync: statSyncSpy,
|
|
||||||
}));
|
|
||||||
|
|
||||||
const { clearProjectRootCache, findProjectRoot } = await import(
|
const { clearProjectRootCache, findProjectRoot } = await import(
|
||||||
`./project-root-finder.ts?memoization=${Date.now()}`
|
`./project-root-finder.ts?memoization=${Date.now()}-${Math.random()}`
|
||||||
);
|
);
|
||||||
|
|
||||||
// when
|
// when
|
||||||
const firstResult = findProjectRoot(startPath);
|
const firstResult = findProjectRoot(startPath);
|
||||||
const firstExistsSyncCallCount = existsSyncSpy.mock.calls.length;
|
unlinkSync(packageJsonPath);
|
||||||
|
|
||||||
const secondResult = findProjectRoot(startPath);
|
const secondResult = findProjectRoot(startPath);
|
||||||
const secondExistsSyncCallCount = existsSyncSpy.mock.calls.length;
|
|
||||||
|
|
||||||
clearProjectRootCache();
|
clearProjectRootCache();
|
||||||
const thirdResult = findProjectRoot(startPath);
|
const thirdResult = findProjectRoot(startPath);
|
||||||
|
|
||||||
// then
|
// then
|
||||||
expect(firstResult).toBe(projectRoot);
|
expect(firstResult).toBe(projectRoot);
|
||||||
expect(secondResult).toBe(projectRoot);
|
expect(secondResult).toBe(projectRoot);
|
||||||
expect(thirdResult).toBe(projectRoot);
|
expect(thirdResult).toBeNull();
|
||||||
expect(firstExistsSyncCallCount).toBeGreaterThan(0);
|
|
||||||
expect(secondExistsSyncCallCount).toBe(firstExistsSyncCallCount);
|
|
||||||
expect(existsSyncSpy).toHaveBeenCalledTimes(firstExistsSyncCallCount * 2);
|
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user