From 74655a5f1735a10e3e011d90012fda091b65267b Mon Sep 17 00:00:00 2001 From: Torben Sorensen Date: Mon, 15 Dec 2025 22:45:50 -0800 Subject: [PATCH] Refactor: Simplify FlyerUploader tests by improving mock usage and enhancing clarity in polling logic --- src/features/flyer/FlyerUploader.test.tsx | 153 ++++++++++++---------- 1 file changed, 84 insertions(+), 69 deletions(-) diff --git a/src/features/flyer/FlyerUploader.test.tsx b/src/features/flyer/FlyerUploader.test.tsx index fc9f21aa..1bd0122c 100644 --- a/src/features/flyer/FlyerUploader.test.tsx +++ b/src/features/flyer/FlyerUploader.test.tsx @@ -1,24 +1,22 @@ // src/features/flyer/FlyerUploader.test.tsx import React from 'react'; import { render, screen, fireEvent, waitFor, act } from '@testing-library/react'; -import { describe, it, expect, vi, beforeEach, afterEach, type Mocked, type Mock } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach, type Mock } from 'vitest'; import { FlyerUploader } from './FlyerUploader'; import * as aiApiClientModule from '../../services/aiApiClient'; import * as checksumModule from '../../utils/checksum'; -import * as routerDomModule from 'react-router-dom'; +import { useNavigate } from 'react-router-dom'; // Mock dependencies vi.mock('../../services/aiApiClient'); vi.mock('../../services/logger.client', () => ({ - logger: { - info: vi.fn(), - error: vi.fn(), - }, + logger: { info: vi.fn(), error: vi.fn() }, })); vi.mock('../../utils/checksum', () => ({ generateFileChecksum: vi.fn(), })); -// Mock react-router-dom to spy on the navigate function + +// Mock react-router-dom vi.mock('react-router-dom', async () => { const actual = await vi.importActual('react-router-dom'); return { @@ -27,53 +25,57 @@ vi.mock('react-router-dom', async () => { }; }); -// Get the mocked versions of the modules/functions -const mockedAiApiClient = aiApiClientModule as Mocked; -const mockedChecksumModule = checksumModule as Mocked; -const mockedRouterDom = routerDomModule as Mocked; -const navigateSpy = vi.fn(); +const mockedAiApiClient = aiApiClientModule as unknown as { + uploadAndProcessFlyer: Mock; + getJobStatus: Mock; +}; +const mockedChecksumModule = checksumModule as unknown as { + generateFileChecksum: Mock; +}; const renderComponent = (onProcessingComplete = vi.fn()) => { + // We use a real MemoryRouter for the Link components, but we spy on useNavigate + const { MemoryRouter } = require('react-router-dom'); return render( - // We still use MemoryRouter to provide routing context for components like - + - + ); }; -describe('FlyerUploader', { timeout: 20000 }, () => { - beforeEach(() => { vi.clearAllMocks(); +describe('FlyerUploader', () => { + const navigateSpy = vi.fn(); + + beforeEach(() => { + vi.clearAllMocks(); + vi.useFakeTimers(); // Use fake timers for ALL tests in this suite - // Access the mock implementation directly from the mocked module. mockedChecksumModule.generateFileChecksum.mockResolvedValue('mock-checksum'); - - // Correctly type the mock for `useNavigate`. - (mockedRouterDom.useNavigate as Mock).mockReturnValue(navigateSpy); + (useNavigate as Mock).mockReturnValue(navigateSpy); }); afterEach(() => { - // Restore real timers after each test to avoid side effects. - vi.useRealTimers(); // Ensure timers are reset after each test + vi.useRealTimers(); }); it('should render the initial state correctly', () => { renderComponent(); expect(screen.getByText('Upload New Flyer')).toBeInTheDocument(); - expect(screen.getByText('Click to select a file')).toBeInTheDocument(); - expect(screen.getByText('or drag and drop a PDF or image')).toBeInTheDocument(); + expect(screen.getByText(/click to select a file/i)).toBeInTheDocument(); }); it('should handle file upload and start polling', async () => { - // Enable fake timers strictly for this polling test - vi.useFakeTimers(); - + // 1. Setup Mocks mockedAiApiClient.uploadAndProcessFlyer.mockResolvedValue( new Response(JSON.stringify({ jobId: 'job-123' }), { status: 200 }) ); - // Mock the job status to return 'active' on the first poll. - mockedAiApiClient.getJobStatus.mockResolvedValue(new Response(JSON.stringify({ state: 'active', progress: { message: 'Checking...' } }))); + // The component polls immediately upon upload success. + // We mock it to return 'active' so it enters the polling loop. + mockedAiApiClient.getJobStatus.mockResolvedValue( + new Response(JSON.stringify({ state: 'active', progress: { message: 'Checking...' } })) + ); + // 2. Render and Interaction renderComponent(); const file = new File(['content'], 'flyer.pdf', { type: 'application/pdf' }); const input = screen.getByLabelText(/click to select a file/i); @@ -82,26 +84,35 @@ describe('FlyerUploader', { timeout: 20000 }, () => { fireEvent.change(input, { target: { files: [file] } }); }); + // 3. Assertions + // Wait for the upload call await waitFor(() => { - expect(mockedChecksumModule.generateFileChecksum).toHaveBeenCalledWith(file); expect(mockedAiApiClient.uploadAndProcessFlyer).toHaveBeenCalledWith(file, 'mock-checksum'); - // The component should call getJobStatus immediately upon entering polling state - expect(mockedAiApiClient.getJobStatus).toHaveBeenCalled(); }); + + // Wait for the UI to switch to the polling/status view + await screen.findByText('Checking...'); + + // Verify getJobStatus was called (immediate poll) + expect(mockedAiApiClient.getJobStatus).toHaveBeenCalledTimes(1); }); it('should poll for status, complete successfully, and redirect', async () => { - // Enable fake timers strictly for this polling test - vi.useFakeTimers(); - const onProcessingComplete = vi.fn(); + + // 1. Setup Mocks sequence mockedAiApiClient.uploadAndProcessFlyer.mockResolvedValue( new Response(JSON.stringify({ jobId: 'job-123' }), { status: 200 }) ); + + // Sequence: + // Call 1 (Immediate): 'active' -> sets timeout for 3s + // Call 2 (After timeout): 'completed' -> triggers redirect mockedAiApiClient.getJobStatus .mockResolvedValueOnce(new Response(JSON.stringify({ state: 'active', progress: { message: 'Analyzing...' } }))) .mockResolvedValueOnce(new Response(JSON.stringify({ state: 'completed', returnValue: { flyerId: 42 } }))); + // 2. Render & Upload renderComponent(onProcessingComplete); const file = new File(['content'], 'flyer.pdf', { type: 'application/pdf' }); const input = screen.getByLabelText(/click to select a file/i); @@ -110,39 +121,45 @@ describe('FlyerUploader', { timeout: 20000 }, () => { fireEvent.change(input, { target: { files: [file] } }); }); - // 1. Wait for upload to initiate and switch to polling state. - // The first poll happens IMMEDIATELY. We wait for its result ('Analyzing...') to confirm poll 1 finished. - await waitFor(() => expect(screen.getByText('Analyzing...')).toBeInTheDocument()); + // 3. Verify Initial Poll (Active) + // This waits for the immediate poll to finish and update UI + await screen.findByText('Analyzing...'); + expect(mockedAiApiClient.getJobStatus).toHaveBeenCalledTimes(1); - // 2. Now that the first poll is done (state=active), the component scheduled a timeout for 3s. - // Advance timers to trigger the SECOND poll. + // 4. Fast-forward time to trigger the next poll + // The component waits 3000ms. We advance slightly more to be safe. + // 'advanceTimersByTimeAsync' is crucial here as it flushes the promise microtasks. await act(async () => { - await vi.advanceTimersByTimeAsync(3000); + await vi.advanceTimersByTimeAsync(4000); }); - // 3. Wait for the result of the second poll ('completed'). + // 5. Verify Second Poll (Completed) await waitFor(() => { + expect(mockedAiApiClient.getJobStatus).toHaveBeenCalledTimes(2); expect(screen.getByText('Processing complete! Redirecting to flyer 42...')).toBeInTheDocument(); - expect(onProcessingComplete).toHaveBeenCalledTimes(1); }); - // 4. Trigger redirect timeout (1500ms) + // 6. Verify Redirect + // The component has a 1500ms delay before redirecting. await act(async () => { await vi.advanceTimersByTimeAsync(2000); }); + + expect(onProcessingComplete).toHaveBeenCalled(); expect(navigateSpy).toHaveBeenCalledWith('/flyers/42'); }); it('should handle a failed job', async () => { - // Use fake timers to control the polling interval precisely - vi.useFakeTimers(); + // 1. Setup Mocks mockedAiApiClient.uploadAndProcessFlyer.mockResolvedValue( - new Response(JSON.stringify({ jobId: 'job-123' }), { status: 200 }) + new Response(JSON.stringify({ jobId: 'job-fail' }), { status: 200 }) ); + // Immediate failure on first poll mockedAiApiClient.getJobStatus.mockResolvedValue( new Response(JSON.stringify({ state: 'failed', failedReason: 'AI model exploded' })) ); + // 2. Render & Upload renderComponent(); const file = new File(['content'], 'flyer.pdf', { type: 'application/pdf' }); const input = screen.getByLabelText(/click to select a file/i); @@ -151,15 +168,15 @@ describe('FlyerUploader', { timeout: 20000 }, () => { fireEvent.change(input, { target: { files: [file] } }); }); - // The polling happens immediately on successful upload. - // We just wait for the failure message from the FIRST poll. + // 3. Verify Failure State await waitFor(() => { - expect(screen.getByText(/Processing failed: AI model exploded/i)).toBeInTheDocument(); + expect(screen.getByText(/Processing failed: AI model exploded/i)).toBeInTheDocument(); }); + // Should verify "Upload Another Flyer" button is present + expect(screen.getByText('Upload Another Flyer')).toBeInTheDocument(); }); it('should handle a duplicate flyer error (409)', async () => { - // This test does not require polling (fails immediately), so we use Real Timers. mockedAiApiClient.uploadAndProcessFlyer.mockResolvedValue( new Response(JSON.stringify({ flyerId: 99, message: 'Duplicate' }), { status: 409 }) ); @@ -174,22 +191,22 @@ describe('FlyerUploader', { timeout: 20000 }, () => { await waitFor(() => { expect(screen.getByText('This flyer has already been processed. You can view it here:')).toBeInTheDocument(); - expect(screen.getByRole('link', { name: 'Flyer #99' })).toHaveAttribute('href', '/flyers/99'); + // We look for the link specifically + const link = screen.getByRole('link', { name: /Flyer #99/i }); + expect(link).toHaveAttribute('href', '/flyers/99'); }); }); it('should allow the user to stop watching progress', async () => { - // Enable fake timers for polling test - vi.useFakeTimers(); - + // 1. Setup Mocks: Infinite active state mockedAiApiClient.uploadAndProcessFlyer.mockResolvedValue( - new Response(JSON.stringify({ jobId: 'job-123' }), { status: 200 }) + new Response(JSON.stringify({ jobId: 'job-stop' }), { status: 200 }) ); - // Mock getJobStatus to keep the component in a polling state mockedAiApiClient.getJobStatus.mockResolvedValue( new Response(JSON.stringify({ state: 'active', progress: { message: 'Analyzing...' } })) ); + // 2. Render & Upload renderComponent(); const file = new File(['content'], 'flyer.pdf', { type: 'application/pdf' }); const input = screen.getByLabelText(/click to select a file/i); @@ -198,21 +215,19 @@ describe('FlyerUploader', { timeout: 20000 }, () => { fireEvent.change(input, { target: { files: [file] } }); }); - // The FIRST poll happens immediately. Wait for the UI to reflect the active state ('Analyzing...'). - // This ensures we are in the polling loop. - await waitFor(() => expect(screen.getByText('Analyzing...')).toBeInTheDocument()); + // 3. Wait for UI to show Polling state + await screen.findByText('Analyzing...'); - // Wait for the button to appear + // 4. Find and Click Stop Button const stopButton = screen.getByRole('button', { name: 'Stop Watching Progress' }); - expect(stopButton).toBeInTheDocument(); - - // Click the button to cancel polling await act(async () => { - fireEvent.click(stopButton); + fireEvent.click(stopButton); }); - // Assert that the UI has returned to its initial idle state - expect(screen.getByText('Click to select a file')).toBeInTheDocument(); - expect(screen.queryByRole('button', { name: 'Stop Watching Progress' })).not.toBeInTheDocument(); + // 5. Verify Reset to Idle + await waitFor(() => { + expect(screen.getByText(/click to select a file/i)).toBeInTheDocument(); + expect(screen.queryByText('Analyzing...')).not.toBeInTheDocument(); + }); }); }); \ No newline at end of file