test(dashboard): harden hook tests + drop dead App.tsx re-exports (code review)
Address code-review findings on the App.tsx module-breakup: - Add behavior-preservation tests: useBoardScrollRestore real double-rAF restore path; useApprovalBanner clear-on-leave-awaiting + mailbox-refresh dedup; useCapacityRiskBanner threshold-change-clears + dismiss-persist assertion; useChatUnreadBadge room-message + cross-project filter; and a new sseSplitIntegration test co-mounting useMailboxUnread + useApprovalBanner to pin the KTD4 SSE split (no double-fire; awaiting-approval refresh exactly once via callback). - Add unmount/teardown assertions (stash interval clear, auth-token listener removal, dashboard-health cancelled flag). - Drop dead type-only re-exports (ApprovalBannerCandidate, CliActionDeps) from App.tsx — zero importers (verified). All hook tests green; App.test.tsx unchanged (5 pre-existing). typecheck + eslint clean. No production behavior change.
This commit is contained in:
@@ -101,8 +101,6 @@ export {
|
||||
isSessionNeedingInputForBanner,
|
||||
getCliActionDisabledReasonForBanner,
|
||||
executeCliSessionBannerAction,
|
||||
type ApprovalBannerCandidate,
|
||||
type CliActionDeps,
|
||||
} from "./utils/appLifecycle";
|
||||
import { subscribeSse } from "./sse-bus";
|
||||
import { AuthTokenRecoveryDialog } from "./components/AuthTokenRecoveryDialog";
|
||||
|
||||
@@ -0,0 +1,106 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { renderHook, act } from "@testing-library/react";
|
||||
import type { Task } from "@fusion/core";
|
||||
|
||||
interface CapturedSubscription {
|
||||
url: string;
|
||||
onReconnect?: () => void;
|
||||
events: Record<string, (e: MessageEvent) => void>;
|
||||
}
|
||||
|
||||
const { subscriptions } = vi.hoisted(() => ({
|
||||
subscriptions: [] as CapturedSubscription[],
|
||||
}));
|
||||
|
||||
vi.mock("../../sse-bus", () => ({
|
||||
subscribeSse: vi.fn(
|
||||
(
|
||||
url: string,
|
||||
sub: { onReconnect?: () => void; events: Record<string, (e: MessageEvent) => void> },
|
||||
) => {
|
||||
subscriptions.push({ url, onReconnect: sub.onReconnect, events: { ...sub.events } });
|
||||
return () => {};
|
||||
},
|
||||
),
|
||||
}));
|
||||
|
||||
const fetchUnreadCount = vi.fn(async () => ({ unreadCount: 0 }));
|
||||
vi.mock("../../api", () => ({
|
||||
fetchUnreadCount: (...a: unknown[]) => fetchUnreadCount(...a),
|
||||
}));
|
||||
|
||||
import { useMailboxUnread } from "../useMailboxUnread";
|
||||
import { useApprovalBanner } from "../useApprovalBanner";
|
||||
|
||||
function msg(data: object): MessageEvent {
|
||||
return { data: JSON.stringify(data) } as MessageEvent;
|
||||
}
|
||||
|
||||
describe("SSE split (KTD4): mailbox-refresh vs approval-banner", () => {
|
||||
beforeEach(() => {
|
||||
subscriptions.length = 0;
|
||||
fetchUnreadCount.mockReset();
|
||||
fetchUnreadCount.mockResolvedValue({ unreadCount: 0 });
|
||||
});
|
||||
|
||||
it("co-mount keeps the awaiting-approval refresh single-fired and the banner independent", async () => {
|
||||
const mailboxSpy = vi.fn();
|
||||
const tasks: Task[] = [];
|
||||
const onStarPrompt = vi.fn();
|
||||
|
||||
// Two independent mounts → two subscribeSse calls captured separately so
|
||||
// the split handlers never overwrite each other.
|
||||
renderHook(() => useMailboxUnread("p1"));
|
||||
const approval = renderHook(() =>
|
||||
useApprovalBanner({
|
||||
tasks,
|
||||
currentProjectId: "p1",
|
||||
gitHubStarPromptShown: true,
|
||||
onStarPrompt,
|
||||
onMailboxRefresh: mailboxSpy,
|
||||
}),
|
||||
);
|
||||
|
||||
// Drain the mailbox hook's mount fetch so no setState leaks past the test.
|
||||
await act(async () => {
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
});
|
||||
|
||||
// Distinguish the two subscriptions: mailbox listens to message:sent,
|
||||
// the banner listens to task:updated.
|
||||
const mailboxSub = subscriptions.find((s) => "message:sent" in s.events);
|
||||
const approvalSub = subscriptions.find((s) => "task:updated" in s.events);
|
||||
expect(mailboxSub).toBeTruthy();
|
||||
expect(approvalSub).toBeTruthy();
|
||||
|
||||
// (i) approval:requested sets the banner candidate but does NOT fire
|
||||
// mailbox-refresh; the mailbox hook's approval:requested handler
|
||||
// (count refresh) is a distinct function from the banner's.
|
||||
act(() => {
|
||||
approvalSub!.events["approval:requested"]?.(msg({ id: "a1", updatedAt: "2026-01-01T00:00:00Z" }));
|
||||
});
|
||||
expect(approval.result.current.candidate?.dedupeKey).toBe("approval:a1");
|
||||
expect(mailboxSpy).not.toHaveBeenCalled();
|
||||
expect(mailboxSub!.events["approval:requested"]).toBeTruthy();
|
||||
expect(mailboxSub!.events["approval:requested"]).not.toBe(approvalSub!.events["approval:requested"]);
|
||||
|
||||
// (ii) task:updated → awaiting-approval sets the candidate + fires the
|
||||
// mailbox refresh exactly once.
|
||||
act(() => {
|
||||
approvalSub!.events["task:updated"]?.(
|
||||
msg({ id: "t1", status: "awaiting-approval", updatedAt: "2026-01-02T00:00:00Z" }),
|
||||
);
|
||||
});
|
||||
expect(approval.result.current.candidate?.dedupeKey).toBe("task:t1");
|
||||
expect(mailboxSpy).toHaveBeenCalledTimes(1);
|
||||
|
||||
// (iii) a second awaiting-approval for the same task is deduped — no second refresh.
|
||||
act(() => {
|
||||
approvalSub!.events["task:updated"]?.(
|
||||
msg({ id: "t1", status: "awaiting-approval", updatedAt: "2026-01-03T00:00:00Z" }),
|
||||
);
|
||||
});
|
||||
expect(mailboxSpy).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
@@ -136,4 +136,64 @@ describe("useApprovalBanner", () => {
|
||||
});
|
||||
expect(result.current.candidate).toBeNull();
|
||||
});
|
||||
it("re-triggers after leaving and re-entering awaiting-approval (clear-on-leave)", () => {
|
||||
const onMailboxRefresh = vi.fn();
|
||||
const seedTasks: Task[] = [task("t1", "awaiting-approval")];
|
||||
const { result } = renderHook(() =>
|
||||
useApprovalBanner({
|
||||
tasks: seedTasks,
|
||||
currentProjectId: "p1",
|
||||
gitHubStarPromptShown: true,
|
||||
onStarPrompt: vi.fn(),
|
||||
onMailboxRefresh,
|
||||
}),
|
||||
);
|
||||
|
||||
// The seeded awaiting-approval task is already in the seen set, so a repeat
|
||||
// event for it must NOT trigger.
|
||||
act(() => {
|
||||
handlers["task:updated"]?.(msg({ id: "t1", status: "awaiting-approval", updatedAt: "2026-01-01T00:00:00Z" }));
|
||||
});
|
||||
expect(result.current.candidate).toBeNull();
|
||||
expect(onMailboxRefresh).not.toHaveBeenCalled();
|
||||
|
||||
// Task leaves awaiting-approval → the seen-key for t1 is cleared.
|
||||
act(() => {
|
||||
handlers["task:updated"]?.(msg({ id: "t1", status: "approved", updatedAt: "2026-01-02T00:00:00Z" }));
|
||||
});
|
||||
expect(result.current.candidate).toBeNull();
|
||||
|
||||
// Re-entering awaiting-approval re-triggers the candidate + mailbox refresh.
|
||||
act(() => {
|
||||
handlers["task:updated"]?.(msg({ id: "t1", status: "awaiting-approval", updatedAt: "2026-01-03T00:00:00Z" }));
|
||||
});
|
||||
expect(result.current.candidate?.dedupeKey).toBe("task:t1");
|
||||
expect(onMailboxRefresh).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("dedupes mailbox refresh on a repeated awaiting-approval task:updated", () => {
|
||||
const onMailboxRefresh = vi.fn();
|
||||
const tasks: Task[] = [];
|
||||
const { result } = renderHook(() =>
|
||||
useApprovalBanner({
|
||||
tasks,
|
||||
currentProjectId: "p1",
|
||||
gitHubStarPromptShown: true,
|
||||
onStarPrompt: vi.fn(),
|
||||
onMailboxRefresh,
|
||||
}),
|
||||
);
|
||||
|
||||
act(() => {
|
||||
handlers["task:updated"]?.(msg({ id: "t1", status: "awaiting-approval", updatedAt: "2026-01-01T00:00:00Z" }));
|
||||
});
|
||||
expect(result.current.candidate?.dedupeKey).toBe("task:t1");
|
||||
expect(onMailboxRefresh).toHaveBeenCalledTimes(1);
|
||||
|
||||
// A second awaiting-approval for the same task is suppressed by seenApprovalKeys.
|
||||
act(() => {
|
||||
handlers["task:updated"]?.(msg({ id: "t1", status: "awaiting-approval", updatedAt: "2026-01-04T00:00:00Z" }));
|
||||
});
|
||||
expect(onMailboxRefresh).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,9 +1,12 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { renderHook, act } from "@testing-library/react";
|
||||
import { AUTH_TOKEN_RECOVERY_REQUIRED_EVENT } from "../../auth";
|
||||
import { useAuthTokenRecovery } from "../useAuthTokenRecovery";
|
||||
|
||||
describe("useAuthTokenRecovery", () => {
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
it("opens when the daemon auth-failure event fires", () => {
|
||||
const { result } = renderHook(() => useAuthTokenRecovery());
|
||||
|
||||
@@ -15,4 +18,19 @@ describe("useAuthTokenRecovery", () => {
|
||||
|
||||
expect(result.current.open).toBe(true);
|
||||
});
|
||||
it("removes the daemon auth-failure listener on unmount", () => {
|
||||
const addSpy = vi.spyOn(window, "addEventListener");
|
||||
const removeSpy = vi.spyOn(window, "removeEventListener");
|
||||
const { unmount } = renderHook(() => useAuthTokenRecovery());
|
||||
|
||||
const addedCall = addSpy.mock.calls.find(
|
||||
([type]) => type === AUTH_TOKEN_RECOVERY_REQUIRED_EVENT,
|
||||
);
|
||||
expect(addedCall).toBeTruthy();
|
||||
const addedHandler = addedCall![1] as EventListener;
|
||||
|
||||
unmount();
|
||||
|
||||
expect(removeSpy).toHaveBeenCalledWith(AUTH_TOKEN_RECOVERY_REQUIRED_EVENT, addedHandler);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,13 +1,29 @@
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { renderHook } from "@testing-library/react";
|
||||
import { useBoardScrollRestore } from "../useBoardScrollRestore";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { renderHook, act } from "@testing-library/react";
|
||||
import type { TaskView } from "../useViewState";
|
||||
|
||||
vi.mock("../../utils/boardScrollSnapshot", () => ({
|
||||
captureBoardScrollSnapshot: vi.fn(() => ({ x: 10, columns: {} })),
|
||||
captureBoardScrollSnapshot: vi.fn(),
|
||||
restoreBoardScrollSnapshot: vi.fn(() => true),
|
||||
}));
|
||||
|
||||
import { captureBoardScrollSnapshot, restoreBoardScrollSnapshot } from "../../utils/boardScrollSnapshot";
|
||||
import { useBoardScrollRestore } from "../useBoardScrollRestore";
|
||||
|
||||
const mockedCapture = vi.mocked(captureBoardScrollSnapshot);
|
||||
const mockedRestore = vi.mocked(restoreBoardScrollSnapshot);
|
||||
|
||||
describe("useBoardScrollRestore", () => {
|
||||
beforeEach(() => {
|
||||
mockedCapture.mockReset();
|
||||
mockedRestore.mockReset();
|
||||
mockedRestore.mockReturnValue(true);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it("exposes capture and requestRestore without throwing", () => {
|
||||
const { result } = renderHook(() => useBoardScrollRestore("board"));
|
||||
|
||||
@@ -16,4 +32,39 @@ describe("useBoardScrollRestore", () => {
|
||||
expect(() => result.current.capture()).not.toThrow();
|
||||
expect(() => result.current.requestRestore()).not.toThrow();
|
||||
});
|
||||
|
||||
it("restores the captured snapshot after returning to the board view", () => {
|
||||
const sentinel = { boardLeft: 42, boardTop: 7, columnTops: { c1: 3 } };
|
||||
mockedCapture.mockReturnValue(sentinel);
|
||||
|
||||
// Make the double requestAnimationFrame fire synchronously so the restore
|
||||
// lands inside the act() that commits the board-view effect.
|
||||
vi.spyOn(window, "requestAnimationFrame").mockImplementation((cb: FrameRequestCallback) => {
|
||||
cb(0);
|
||||
return 0;
|
||||
});
|
||||
|
||||
const { result, rerender } = renderHook(
|
||||
({ taskView }: { taskView: TaskView }) => useBoardScrollRestore(taskView),
|
||||
{ initialProps: { taskView: "task-detail" } },
|
||||
);
|
||||
|
||||
// Off the board with nothing pending → no restore yet.
|
||||
expect(mockedRestore).not.toHaveBeenCalled();
|
||||
|
||||
act(() => {
|
||||
result.current.capture();
|
||||
result.current.requestRestore();
|
||||
});
|
||||
|
||||
// Restore waits for the view to return to "board".
|
||||
expect(mockedRestore).not.toHaveBeenCalled();
|
||||
|
||||
act(() => {
|
||||
rerender({ taskView: "board" });
|
||||
});
|
||||
|
||||
expect(mockedRestore).toHaveBeenCalledTimes(1);
|
||||
expect(mockedRestore).toHaveBeenCalledWith(sentinel);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -7,7 +7,7 @@ vi.mock("../../utils/projectStorage", () => ({
|
||||
removeScopedItem: vi.fn(),
|
||||
}));
|
||||
|
||||
import { getScopedItem, removeScopedItem } from "../../utils/projectStorage";
|
||||
import { getScopedItem, removeScopedItem, setScopedItem } from "../../utils/projectStorage";
|
||||
import { useCapacityRiskBanner } from "../useCapacityRiskBanner";
|
||||
|
||||
const base = {
|
||||
@@ -41,6 +41,7 @@ describe("useCapacityRiskBanner", () => {
|
||||
});
|
||||
|
||||
expect(result.current.dismissed).toBe(true);
|
||||
expect(setScopedItem).toHaveBeenCalledWith(expect.any(String), "true", "p1");
|
||||
});
|
||||
|
||||
it("clears a prior dismissal when the banner is re-enabled after hydrate", () => {
|
||||
@@ -58,6 +59,24 @@ describe("useCapacityRiskBanner", () => {
|
||||
// Re-enabling the banner resurrects the dismissed banner.
|
||||
rerender({ enabled: true });
|
||||
|
||||
expect(removeScopedItem).toHaveBeenCalledWith(expect.any(String), "p1");
|
||||
expect(result.current.dismissed).toBe(false);
|
||||
});
|
||||
it("clears a prior dismissal when the todo threshold changes after hydrate", () => {
|
||||
vi.mocked(getScopedItem).mockReturnValue("true");
|
||||
const { result, rerender } = renderHook(
|
||||
(props: { threshold: number }) =>
|
||||
useCapacityRiskBanner({ ...base, capacityRiskTodoThreshold: props.threshold }),
|
||||
{ initialProps: { threshold: 3 } },
|
||||
);
|
||||
|
||||
// First settings load hydrates without clearing.
|
||||
expect(result.current.dismissed).toBe(true);
|
||||
expect(removeScopedItem).not.toHaveBeenCalled();
|
||||
|
||||
// Changing the threshold resurrects the previously-dismissed banner.
|
||||
rerender({ threshold: 5 });
|
||||
|
||||
expect(removeScopedItem).toHaveBeenCalledWith(expect.any(String), "p1");
|
||||
expect(result.current.dismissed).toBe(false);
|
||||
});
|
||||
|
||||
@@ -80,4 +80,39 @@ describe("useChatUnreadBadge", () => {
|
||||
rerender({ taskView: "chat" });
|
||||
expect(result.current.chatHasUnreadResponse).toBe(false);
|
||||
});
|
||||
it("marks unread on a non-user chat:room:message:added", () => {
|
||||
const { result } = renderHook(() =>
|
||||
useChatUnreadBadge(undefined, { taskView: "board", quickChatOpen: false }),
|
||||
);
|
||||
|
||||
act(() => {
|
||||
handlers["chat:room:message:added"]?.(message({ role: "assistant" }));
|
||||
});
|
||||
|
||||
expect(result.current.chatHasUnreadResponse).toBe(true);
|
||||
});
|
||||
|
||||
it("ignores user-role chat:room:message:added events", () => {
|
||||
const { result } = renderHook(() =>
|
||||
useChatUnreadBadge(undefined, { taskView: "board", quickChatOpen: false }),
|
||||
);
|
||||
|
||||
act(() => {
|
||||
handlers["chat:room:message:added"]?.(message({ role: "user" }));
|
||||
});
|
||||
|
||||
expect(result.current.chatHasUnreadResponse).toBe(false);
|
||||
});
|
||||
|
||||
it("ignores assistant messages scoped to a different project", () => {
|
||||
const { result } = renderHook(() =>
|
||||
useChatUnreadBadge("p1", { taskView: "board", quickChatOpen: false }),
|
||||
);
|
||||
|
||||
act(() => {
|
||||
handlers["chat:message:added"]?.(message({ role: "assistant", projectId: "p2" }));
|
||||
});
|
||||
|
||||
expect(result.current.chatHasUnreadResponse).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -59,4 +59,27 @@ describe("useDashboardHealth", () => {
|
||||
expect(result.current.refreshError).toBe("nope");
|
||||
expect(result.current.refreshing).toBe(false);
|
||||
});
|
||||
it("does not apply the mount fetch after unmount", async () => {
|
||||
let resolveMount: (value: { status: string }) => void = () => {};
|
||||
fetchDashboardHealth.mockImplementation(
|
||||
() =>
|
||||
new Promise<{ status: string }>((resolve) => {
|
||||
resolveMount = resolve;
|
||||
}),
|
||||
);
|
||||
|
||||
const { result, unmount } = renderHook(() => useDashboardHealth());
|
||||
expect(result.current.health).toBeNull();
|
||||
|
||||
unmount();
|
||||
resolveMount({ status: "ok" });
|
||||
|
||||
await act(async () => {
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
});
|
||||
|
||||
// The cancelled flag suppresses setState — health stays at its initial null.
|
||||
expect(result.current.health).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -58,4 +58,22 @@ describe("useStashOrphanCount", () => {
|
||||
});
|
||||
expect(mockedApi).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
it("stops polling once unmounted", async () => {
|
||||
mockedApi.mockResolvedValue({ count: 1 });
|
||||
const { unmount } = renderHook(() => useStashOrphanCount(undefined));
|
||||
|
||||
// Initial mount fetch.
|
||||
await act(async () => {
|
||||
await vi.advanceTimersByTimeAsync(1);
|
||||
});
|
||||
expect(mockedApi).toHaveBeenCalledTimes(1);
|
||||
|
||||
unmount();
|
||||
|
||||
// Advance well past the 30s interval — the cleared timer must not fire.
|
||||
await act(async () => {
|
||||
await vi.advanceTimersByTimeAsync(60_000);
|
||||
});
|
||||
expect(mockedApi).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user