modelcontextprotocol/inspector
Watch
1
0
Fork
You've already forked inspector
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 2

test(core): make the unsubscribe test actually verify unsubscription

Browse source 
Copilot review round 3.

"stops listening on unmount" asserted that the rendered value stopped
changing after unmount, which proves nothing: React drops a store
notification for an unmounted component whether or not the listener is
still attached, so the test passed with the leak it existed to catch.

Spy the store's add/removeEventListener and assert the exact function
attached is the one detached. Verified it discriminates by deleting the
cleanup, watching this test fail, and restoring.

Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall 2026-08-29 02:55:27 -04:00
commit b088b94019
1 changed files with 16 additions and 7 deletions
Download patch file Download diff file Expand all files Collapse all files

23
clients/web/src/test/core/react/useStoreSnapshot.test.tsx
View file

@ -1,4 +1,4 @@
import { describe, it, expect, beforeEach } from "vitest";
import { describe, it, expect, beforeEach, vi } from "vitest";
import { act, renderHook } from "@testing-library/react";
import { TypedEventTarget } from "@inspector/core/mcp/typedEventTarget";
import { useStoreSnapshot } from "@inspector/core/react/useStoreSnapshot";
@ -257,16 +257,25 @@ describe("useStoreSnapshot", () => {
});
it("stops listening on unmount", () => {
const { result, unmount } = renderHook(() =>
// Assert on the listener, not on the rendered value. Watching
// `result.current` stay put after unmount proves nothing: React drops a
// store notification for an unmounted component either way, so that test
// passes with the listener still attached — which is the leak it was
// supposed to catch.
const addSpy = vi.spyOn(store, "addEventListener");
const removeSpy = vi.spyOn(store, "removeEventListener");
const { unmount } = renderHook(() =>
useStoreSnapshot(store, "itemsChange", readItems, NO_ITEMS),
);
const before = result.current;
expect(addSpy).toHaveBeenCalledWith("itemsChange", expect.any(Function));
const subscribed = addSpy.mock.calls[0]?.[1];
unmount();
// No act(): after unmount there is nothing to re-render, and a listener
// still attached here would be a leak rather than an update.
store.add("a");
expect(result.current).toBe(before);
// The exact function that was attached is the one detached — a cleanup
// that removed some other listener, or none, fails here.
expect(removeSpy).toHaveBeenCalledWith("itemsChange", subscribed);
});
it("unmounts cleanly with no store to unsubscribe from", () => {