Skip to content

Commit 864023e

Browse files
jantmanclaude
andcommitted
Slideshow Management Fixes - Convert remaining window.confirm overrides to vi.spyOn.
Follow-up to Copilot review feedback on PR #23: the previous commit added afterEach(() => vi.restoreAllMocks()) to the Slideshows and SlideshowDetail test suites, but each file still had one pre-existing test mocking window.confirm by direct assignment, which vi.restoreAllMocks() does not restore. If such a test failed before its manual cleanup line, the mock could leak into later tests and make the suites order-dependent. Convert those two remaining tests ('handles delete slideshow' and 'handles delete item') to vi.spyOn(window, 'confirm').mockReturnValue(true) and drop their manual save/restore, so every window.confirm/window.prompt mock in both suites is now a spy covered by the afterEach restore. Full frontend suite passes (137 passed); type-check and lint clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent ed52658 commit 864023e

2 files changed

Lines changed: 6 additions & 14 deletions

File tree

‎frontend/src/test/SlideshowDetail.test.tsx‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -204,9 +204,8 @@ describe('SlideshowDetail', () => {
204204
.mockResolvedValueOnce({ success: true, data: mockSlideshow })
205205
.mockResolvedValueOnce({ success: true, data: mockItems.slice(1) });
206206

207-
// Mock window.confirm
208-
const originalConfirm = window.confirm;
209-
window.confirm = vi.fn(() => true);
207+
// Spy on window.confirm; restored by afterEach even if the test fails
208+
const confirmSpy = vi.spyOn(window, 'confirm').mockReturnValue(true);
210209

211210
render(
212211
<TestWrapper>
@@ -222,14 +221,11 @@ describe('SlideshowDetail', () => {
222221
const deleteButtons = screen.getAllByTitle('Delete');
223222
fireEvent.click(deleteButtons[0]);
224223

225-
expect(window.confirm).toHaveBeenCalledWith('Are you sure you want to delete "Test Image"?');
224+
expect(confirmSpy).toHaveBeenCalledWith('Are you sure you want to delete "Test Image"?');
226225

227226
await waitFor(() => {
228227
expect(mockApiCall).toHaveBeenCalledWith('/api/v1/slideshow-items/1', { method: 'DELETE' });
229228
});
230-
231-
// Cleanup
232-
window.confirm = originalConfirm;
233229
});
234230

235231
it('handles delete of inactive item with permanent-delete confirmation', async () => {

‎frontend/src/test/Slideshows.test.tsx‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -143,9 +143,8 @@ describe('Slideshows', () => {
143143
.mockResolvedValueOnce({ success: true })
144144
.mockResolvedValueOnce({ success: true, data: mockSlideshows.slice(1) });
145145

146-
// Mock window.confirm
147-
const originalConfirm = window.confirm;
148-
window.confirm = vi.fn(() => true);
146+
// Spy on window.confirm; restored by afterEach even if the test fails
147+
const confirmSpy = vi.spyOn(window, 'confirm').mockReturnValue(true);
149148

150149
render(
151150
<TestWrapper>
@@ -161,14 +160,11 @@ describe('Slideshows', () => {
161160
const deleteButtons = screen.getAllByTitle('Delete');
162161
fireEvent.click(deleteButtons[0]);
163162

164-
expect(window.confirm).toHaveBeenCalledWith('Are you sure you want to delete the slideshow "Test Slideshow 1"?');
163+
expect(confirmSpy).toHaveBeenCalledWith('Are you sure you want to delete the slideshow "Test Slideshow 1"?');
165164

166165
await waitFor(() => {
167166
expect(mockApiCall).toHaveBeenCalledWith('/api/v1/slideshows/1', { method: 'DELETE' });
168167
});
169-
170-
// Cleanup
171-
window.confirm = originalConfirm;
172168
});
173169

174170
it('handles duplicate slideshow', async () => {

0 commit comments

Comments
 (0)