Skip to content

Commit 7417805

Browse files
OpenStaxClaudeclaude
authored andcommitted
Add tests for 100% code coverage
Added tests for two uncovered scenarios in chatController.ts: - Line 154: Handles popup blocking when window.open returns null - Line 164: Ignores postMessage events from sources other than the opened popup window These tests improve the security and robustness of the chat controller by ensuring: 1. The controller gracefully handles popup blockers without throwing errors 2. PostMessage communication is validated to only accept messages from the intended popup window 🤖 Generated with [Claude Code](https://claude.com/claude-code) Add test coverage for chatController and businessHours branches - Add test for line 175 else branch in chatController.ts (when popup is NOT closed) - Add test for when nextState is undefined (not in business hours) - Add test for state reuse when hours haven't changed (object reference stability) These tests address the coverage gaps identified in code review. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent d286dee commit 7417805

File tree

1 file changed

+131
-2
lines changed

1 file changed

+131
-2
lines changed

src/components/HelpMenu/hooks.spec.tsx

Lines changed: 131 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,14 +110,51 @@ describe('useBusinessHours', () => {
110110
const response = makeResponse({
111111
hours: makeBusinessHoursResponse(Date.now(), makeBusinessHours(start, end))
112112
});
113-
113+
114114
const { unmount } = renderHook(() =>
115115
useBusinessHours(response, 5000)
116116
);
117-
117+
118118
unmount();
119119
expect(clearTimeoutSpy).toHaveBeenCalled(); // ensure the cleanup cleared the timer
120120
});
121+
122+
it('does not set a timeout when nextState is undefined (not in business hours)', () => {
123+
const setTimeoutSpy = jest.spyOn(global, 'setTimeout');
124+
const start = Date.now() + 10000;
125+
const end = Date.now() + 20000;
126+
const response = makeResponse({
127+
hours: makeBusinessHoursResponse(Date.now(), makeBusinessHours(start, end))
128+
});
129+
130+
renderHook(() =>
131+
useBusinessHours(response, 0)
132+
);
133+
134+
// When not in business hours (nextState is undefined), no timeout should be set
135+
expect(setTimeoutSpy).not.toHaveBeenCalled();
136+
});
137+
138+
it('updates state only when hours actually change (reuses same object reference)', () => {
139+
const start = Date.now() - 1000;
140+
const end = Date.now() + 1000;
141+
const response = makeResponse({
142+
hours: makeBusinessHoursResponse(Date.now(), makeBusinessHours(start, end))
143+
});
144+
145+
const { result, rerender } = renderHook(() =>
146+
useBusinessHours(response, 0)
147+
);
148+
149+
const firstResult = result.current;
150+
expect(firstResult).toBeDefined();
151+
152+
// Rerender with the same response
153+
rerender();
154+
155+
// Should return the same object reference (not a new object)
156+
expect(result.current).toBe(firstResult);
157+
});
121158
});
122159

123160
describe('formatBusinessHoursRange', () => {
@@ -571,4 +608,96 @@ describe('useChatController', () => {
571608

572609
expect(firstPopup.postMessage).toHaveBeenCalledTimes(0);
573610
});
611+
612+
/** 2.9. `openChat` handles popup blocking gracefully (line 154) */
613+
it('handles popup blocking gracefully when window.open returns null', () => {
614+
// Mock window.open to return null (simulating popup blocker)
615+
mockOpen.mockReturnValue(null);
616+
617+
const { result } = renderHook(() => useChatController(path, preChatFields));
618+
619+
// Clear any existing calls (from other code)
620+
mockAddEventListener.mockClear();
621+
mockSetInterval.mockClear();
622+
623+
// This should not throw an error
624+
act(() => {
625+
result.current.openChat?.();
626+
});
627+
628+
// Verify that no message listeners were added since popup failed
629+
const messageListenerCalls = mockAddEventListener.mock.calls.filter(
630+
(call) => call[0] === 'message'
631+
);
632+
expect(messageListenerCalls).toHaveLength(0);
633+
634+
// Verify that no interval was set since popup failed
635+
expect(mockSetInterval).not.toHaveBeenCalled();
636+
});
637+
638+
/** 2.10. `handleMessage` ignores messages from sources other than the popup (line 164) */
639+
it('ignores messages from sources other than the opened popup window', () => {
640+
const mockPopup = createMockPopup();
641+
mockOpen.mockReturnValue(mockPopup);
642+
643+
const { result } = renderHook(() => useChatController(path, preChatFields));
644+
645+
act(() => {
646+
result.current.openChat?.();
647+
});
648+
649+
// Verify message listener was added
650+
expect(mockAddEventListener).toHaveBeenCalledWith('message', expect.any(Function), false);
651+
652+
// Create a message event from a different source (not our popup)
653+
const differentSource = createMockPopup();
654+
const event: MessageEvent = {
655+
source: differentSource, // Different source than mockPopup
656+
data: { type: 'ready' } as any,
657+
} as any;
658+
659+
// Get the handleMessage callback
660+
const handleMessage = mockAddEventListener.mock.calls.find(
661+
(args) => args[0] === 'message'
662+
)?.[1];
663+
expect(handleMessage).toBeDefined();
664+
665+
act(() => {
666+
handleMessage(event);
667+
});
668+
669+
// Verify that postMessage was NOT called on our popup
670+
// because the message came from a different source
671+
expect(mockPopup.postMessage).not.toHaveBeenCalled();
672+
});
673+
674+
/** 2.11. Tests the else branch of line 175 - when popup is still open (not closed) */
675+
it('continues polling when popup is still open (line 175 else branch)', () => {
676+
const mockPopup = createMockPopup();
677+
mockOpen.mockReturnValue(mockPopup);
678+
679+
const { result } = renderHook(() => useChatController(path, preChatFields));
680+
681+
act(() => {
682+
result.current.openChat?.();
683+
});
684+
685+
// Get the checkClosed callback from setInterval
686+
const checkClosed = mockSetInterval.mock.calls[0][0];
687+
688+
// Simulate the interval running while popup is still open (closed = false)
689+
mockPopup.closed = false;
690+
691+
// Clear mocks to verify what happens in this tick
692+
mockRemoveEventListener.mockClear();
693+
mockClearInterval.mockClear();
694+
695+
act(() => {
696+
checkClosed();
697+
});
698+
699+
// When popup is NOT closed, cleanup should NOT happen
700+
expect(mockRemoveEventListener).not.toHaveBeenCalled();
701+
expect(mockClearInterval).not.toHaveBeenCalled();
702+
});
574703
});

0 commit comments

Comments
 (0)