Pin the visibility gate behavior, not just cadence

Self-review on PR #2723 caught a coverage gap: the existing
"visibility gate" describe block actually tested cadence (10s/30s
timing), not the gate itself. If a refactor dropped the
`if (!visible) return` line, the cadence test would still pass
because the effect would still fire every 30s — the regression would
silently ship.

New test renders with comms-returning mock so the panel renders, clicks
the close button, advances 60s, asserts no further fetches occur.

Discipline-verified: removed `if (!visible) return` from the source,
test fails as expected. Restored, test passes.

Same failure mode as PR #434 (test asserted broken behavior) — pin
what you claim to fix, not the easy substring.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Hongming Wang 2026-05-04 03:18:00 -07:00
parent 26b5b21238
commit e1c99cd24c

View File

@ -99,7 +99,7 @@ describe("CommunicationOverlay — fan-out cap", () => {
});
});
describe("CommunicationOverlay — visibility gate", () => {
describe("CommunicationOverlay — cadence", () => {
it("uses 30s interval cadence (was 10s pre-fix)", async () => {
await act(async () => {
render(<CommunicationOverlay />);
@ -119,3 +119,60 @@ describe("CommunicationOverlay — visibility gate", () => {
expect(mockGet).toHaveBeenCalledTimes(6); // +3 from second tick
});
});
describe("CommunicationOverlay — visibility gate", () => {
// The visibility gate is the dial that drops collapsed-panel polling
// to ZERO. The cadence test above can't catch its removal — if a
// refactor dropped `if (!visible) return`, the cadence test would
// still pass because the effect would still fire every 30s.
//
// Direct probe: render with comms-returning mock so the panel
// actually renders (close button only exists in the expanded panel,
// not the collapsed button-state). Click close, advance the clock,
// assert no further fetches.
it("stops polling after the user collapses the panel", async () => {
// Mock returns one a2a_send so comms.length > 0 → panel renders →
// close button accessible.
mockGet.mockResolvedValue([
{
id: "act-1",
workspace_id: "ws-1",
activity_type: "a2a_send",
source_id: "ws-1",
target_id: "ws-2",
summary: "test",
status: "completed",
duration_ms: 100,
created_at: new Date().toISOString(),
},
]);
const { getByLabelText } = await act(async () => {
return render(<CommunicationOverlay />);
});
// Drain pending microtasks (resolves the await in fetchComms) so
// setComms lands and the panel renders. Don't advance time — that
// would fire the next interval tick and pollute the assertion.
await act(async () => {
await Promise.resolve();
await Promise.resolve();
await Promise.resolve();
});
// Initial mount polled 3 workspaces.
expect(mockGet).toHaveBeenCalledTimes(3);
mockGet.mockClear();
// Click the close button. Synchronous getByLabelText avoids
// findBy's internal setTimeout (deadlocks under useFakeTimers).
const closeBtn = getByLabelText("Close communications panel");
await act(async () => {
fireEvent.click(closeBtn);
});
// Advance well past the 30s cadence — gate should suppress the tick.
await act(async () => {
vi.advanceTimersByTime(60_000);
});
expect(mockGet).not.toHaveBeenCalled();
});
});