diff --git a/canvas/src/components/__tests__/ApprovalBanner.test.tsx b/canvas/src/components/__tests__/ApprovalBanner.test.tsx index 2a3fc758..45b30aa3 100644 --- a/canvas/src/components/__tests__/ApprovalBanner.test.tsx +++ b/canvas/src/components/__tests__/ApprovalBanner.test.tsx @@ -5,10 +5,9 @@ * Covers: renders nothing when no approvals, polls /approvals/pending, * shows approval cards, approve/deny decisions, toast notifications. * - * Note: does NOT mock @/lib/api — uses vi.spyOn on the real module. - * vi.restoreAllMocks() is omitted from afterEach so queued mock values - * (set up via mockResolvedValueOnce in beforeEach) are preserved for the - * component's useEffect to consume. + * Uses vi.hoisted + vi.mock to create self-contained api mocks, avoiding + * cross-file module-cache pollution from other test files that use + * vi.mock("@/lib/api") (e.g. AuditTrailPanel which only provides get, no post). */ import React from "react"; import { render, screen, fireEvent, cleanup, act } from "@testing-library/react"; @@ -21,7 +20,22 @@ vi.mock("@/components/Toaster", () => ({ showToast: vi.fn(), })); -// ─── Helpers ────────────────────────────────────────────────────────────────── +// ─── Mock api (hoisted before module load) ──────────────────────────────────── +// vi.hoisted runs in the same hoisting phase as vi.mock factories, so _mockGet +// and _mockPost are the same function instances the factory returns. +const { _mockGet, _mockPost } = vi.hoisted(() => ({ + _mockGet: vi.fn(), + _mockPost: vi.fn(), +})); + +vi.mock("@/lib/api", () => ({ + api: { + get: _mockGet, + post: _mockPost, + }, +})); + +// ─── Helpers ─────────────────────────────────────────────────────────────────── const pendingApproval = (id = "a1", workspaceId = "ws-1"): { id: string; @@ -41,22 +55,18 @@ const pendingApproval = (id = "a1", workspaceId = "ws-1"): { created_at: "2026-05-10T10:00:00Z", }); -// Shared spy references so individual tests can reset or reject the POST mock -// without needing to call spyOn again (which would create a duplicate spy). -let mockGet: ReturnType; -let mockPost: ReturnType; - -// ─── Tests ──────────────────────────────────────────────────────────────────── +// ─── Tests ───────────────────────────────────────────────────────────────────── describe("ApprovalBanner — empty state", () => { beforeEach(() => { vi.useFakeTimers(); - vi.spyOn(api, "get").mockResolvedValueOnce([]); + _mockGet.mockReset().mockResolvedValue([]); }); afterEach(() => { cleanup(); vi.useRealTimers(); + vi.restoreAllMocks(); }); it("renders nothing when there are no pending approvals", async () => { @@ -76,7 +86,7 @@ describe("ApprovalBanner — empty state", () => { describe("ApprovalBanner — renders approval cards", () => { beforeEach(() => { vi.useFakeTimers(); - mockGet = vi.spyOn(api, "get").mockResolvedValueOnce([ + _mockGet.mockReset().mockResolvedValue([ pendingApproval("a1"), pendingApproval("a2", "ws-2"), ]); @@ -85,6 +95,7 @@ describe("ApprovalBanner — renders approval cards", () => { afterEach(() => { cleanup(); vi.useRealTimers(); + vi.restoreAllMocks(); }); it("renders an alert card for each pending approval", async () => { @@ -92,7 +103,6 @@ describe("ApprovalBanner — renders approval cards", () => { await act(async () => { await vi.runOnlyPendingTimersAsync(); }); const alerts = screen.getAllByRole("alert"); expect(alerts).toHaveLength(2); - mockGet.mockRestore(); }); it("displays the workspace name and action text", async () => { @@ -110,7 +120,7 @@ describe("ApprovalBanner — renders approval cards", () => { }); it("omits the reason div when reason is null", async () => { - vi.spyOn(api, "get").mockResolvedValueOnce([{ + _mockGet.mockReset().mockResolvedValue([{ ...pendingApproval("a1"), reason: null, }]); @@ -140,13 +150,14 @@ describe("ApprovalBanner — renders approval cards", () => { describe("ApprovalBanner — decisions", () => { beforeEach(() => { vi.useFakeTimers(); - mockGet = vi.spyOn(api, "get").mockResolvedValueOnce([pendingApproval("a1")]); - mockPost = vi.spyOn(api, "post").mockResolvedValue({}); + _mockGet.mockReset().mockResolvedValue([pendingApproval("a1")]); + _mockPost.mockReset().mockResolvedValue({}); }); afterEach(() => { cleanup(); vi.useRealTimers(); + vi.restoreAllMocks(); }); it("calls POST /workspaces/:id/approvals/:id/decide on Approve click", async () => { @@ -197,7 +208,7 @@ describe("ApprovalBanner — decisions", () => { }); it("shows an error toast when POST fails", async () => { - mockPost.mockReset().mockRejectedValue(new Error("Network error")); + _mockPost.mockReset().mockRejectedValue(new Error("Network error")); render(); await act(async () => { await vi.runOnlyPendingTimersAsync(); }); fireEvent.click(screen.getAllByRole("button", { name: /approve/i })[0]); @@ -209,9 +220,10 @@ describe("ApprovalBanner — decisions", () => { }); it("keeps the card visible when the POST fails", async () => { - // Reset the post mock before rejecting so the beforeEach's resolved value - // is gone and we get a clean rejection instead of a resolved→rejected queue. - mockPost.mockReset().mockRejectedValue(new Error("Network error")); + // mockReset + mockRejectedValue is safe here because _mockPost is a vi.fn() + // mock created in the vi.hoisted factory — it has no underlying original + // function to fall back to, so mockReset cannot cause the real fetch to fire. + _mockPost.mockReset().mockRejectedValue(new Error("Network error")); render(); await act(async () => { await vi.runOnlyPendingTimersAsync(); }); fireEvent.click(screen.getAllByRole("button", { name: /approve/i })[0]); @@ -223,12 +235,13 @@ describe("ApprovalBanner — decisions", () => { describe("ApprovalBanner — handles empty list from server", () => { beforeEach(() => { vi.useFakeTimers(); - vi.spyOn(api, "get").mockResolvedValueOnce([]); + _mockGet.mockReset().mockResolvedValue([]); }); afterEach(() => { cleanup(); vi.useRealTimers(); + vi.restoreAllMocks(); }); it("shows nothing when the API returns an empty array on first poll", async () => {