diff --git a/SHARED_TASK_NOTES.md b/SHARED_TASK_NOTES.md index 54d6e79..6d40ad3 100644 --- a/SHARED_TASK_NOTES.md +++ b/SHARED_TASK_NOTES.md @@ -1,39 +1,30 @@ # Test Coverage Improvement - In Progress -## Current Status -✅ All tests passing (34/34 tests in 5 files) -✅ Test suite is fast (<1 second execution time) +## Current Status (Iteration 4 - 2025-11-21) +✅ All tests passing (52/52 tests in 8 files) +✅ Test suite is fast (~1.5 seconds total, 296ms for tests) ✅ Tests are stable with proper mocking -✅ Coverage increased from 7% to 46.39% -✅ Utility functions have 97% coverage - -## What Was Done -- Added @vitest/coverage-v8 for coverage reporting -- Created comprehensive tests for utility functions: - - votes.test.ts (13 tests - 100% coverage) - - shows.test.ts (11 tests - 100% coverage) - - Spotify.test.ts (6 tests - 100% coverage) - - songs.test.ts (3 tests - 100% coverage) -- Configured vitest for parallel execution and performance -- Created global test setup (src/test/setup.tsx) with Firebase and Google Maps mocks -- Fixed App.test.tsx with proper mocking +✅ Coverage: 48.6% (up from 46.39%) +✅ Utility functions: 97% coverage +✅ Custom hooks now have 100% coverage ## Next Steps for Further Coverage -1. Add tests for custom hooks (useShows, useSongs, useDeleteShow) -2. Add component tests for: - - NewSongForm (currently 23% coverage) - - ShowCalendar (currently 21% coverage) - - Login/Admin components (currently 23-27% coverage) - - Song component (currently 26% coverage) +Priority targets for component tests: +1. NewSongForm (23% coverage) - 138 uncovered lines +2. ShowCalendar (21% coverage) - large component with calendar logic +3. Login (23% coverage) - authentication flow +4. Song component (26% coverage) - main song display logic +5. Admin components (26-42% coverage) -## Performance Notes -- Tests run in parallel with 1-4 threads -- Test timeout: 5 seconds -- Average execution: 800-1000ms for full suite -- All tests are isolated and stable +## Notes for Next Iteration +- Global test setup at src/test/setup.tsx has Firebase/reactfire/Google Maps mocks +- Recently added limitToLast and startAfter to Firebase mock +- Hook tests use @testing-library/react's renderHook +- For future show dates in tests, use Date calculations to avoid test brittleness +- useSongs has search filtering and complex sorting logic (visible > votes > title) +- useShows filters out past shows via formatShows utility ## Configuration - Vitest 3.2.4 with v8 coverage - Happy-dom test environment -- Global mocks in src/test/setup.tsx -- Coverage thresholds set to 80% (not yet met) +- Coverage thresholds: 80% (aspirational, not enforced yet) diff --git a/src/components/Shows/useDeleteShow.test.ts b/src/components/Shows/useDeleteShow.test.ts new file mode 100644 index 0000000..2517306 --- /dev/null +++ b/src/components/Shows/useDeleteShow.test.ts @@ -0,0 +1,92 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { renderHook, act } from '@testing-library/react' +import { useDeleteShow } from './useDeleteShow' +import * as reactfire from 'reactfire' +import * as firebaseDatabase from 'firebase/database' + +vi.mock('reactfire') +vi.mock('firebase/database') + +describe('useDeleteShow', () => { + const mockDatabase = {} + const mockShow = { + id: 'show-123', + date: '2024-03-15', + venue: 'Test Venue', + location: 'Test City', + time: '20:00', + } + + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(reactfire.useDatabase).mockReturnValue(mockDatabase as any) + }) + + it('should call remove with correct show reference', () => { + const mockRef = { path: 'shows/show-123' } + const mockRemove = vi.fn() + + vi.mocked(firebaseDatabase.ref).mockReturnValue(mockRef as any) + vi.mocked(firebaseDatabase.remove).mockImplementation(mockRemove) + + const { result } = renderHook(() => useDeleteShow(mockShow)) + + act(() => { + result.current() + }) + + expect(firebaseDatabase.ref).toHaveBeenCalledWith(mockDatabase, 'shows/show-123') + expect(mockRemove).toHaveBeenCalledWith(mockRef) + }) + + it('should return a memoized callback', () => { + const mockRef = { path: 'shows/show-123' } + vi.mocked(firebaseDatabase.ref).mockReturnValue(mockRef as any) + + const { result, rerender } = renderHook(() => useDeleteShow(mockShow)) + + const firstCallback = result.current + rerender() + const secondCallback = result.current + + expect(firstCallback).toBe(secondCallback) + }) + + it('should update callback when show changes', () => { + const mockRef1 = { path: 'shows/show-123' } + const mockRef2 = { path: 'shows/show-456' } + + vi.mocked(firebaseDatabase.ref) + .mockReturnValueOnce(mockRef1 as any) + .mockReturnValueOnce(mockRef2 as any) + + const { result, rerender } = renderHook( + ({ show }) => useDeleteShow(show), + { initialProps: { show: mockShow } } + ) + + const firstCallback = result.current + + const newShow = { ...mockShow, id: 'show-456' } + rerender({ show: newShow }) + const secondCallback = result.current + + expect(firstCallback).not.toBe(secondCallback) + }) + + it('should handle delete for different show IDs', () => { + const mockRemove = vi.fn() + vi.mocked(firebaseDatabase.remove).mockImplementation(mockRemove) + + const show1 = { ...mockShow, id: 'show-1' } + const show2 = { ...mockShow, id: 'show-2' } + + vi.mocked(firebaseDatabase.ref).mockReturnValue({} as any) + + const { result: result1 } = renderHook(() => useDeleteShow(show1)) + const { result: result2 } = renderHook(() => useDeleteShow(show2)) + + expect(firebaseDatabase.ref).toHaveBeenCalledWith(mockDatabase, 'shows/show-1') + expect(firebaseDatabase.ref).toHaveBeenCalledWith(mockDatabase, 'shows/show-2') + }) +}) diff --git a/src/components/Shows/useShows.test.ts b/src/components/Shows/useShows.test.ts new file mode 100644 index 0000000..befa524 --- /dev/null +++ b/src/components/Shows/useShows.test.ts @@ -0,0 +1,132 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { renderHook } from '@testing-library/react' +import { useShows } from './useShows' +import * as reactfire from 'reactfire' + +vi.mock('reactfire') + +describe('useShows', () => { + const mockDatabase = {} + + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(reactfire.useDatabase).mockReturnValue(mockDatabase as any) + }) + + it('should return loading status when data is loading', () => { + vi.mocked(reactfire.useDatabaseListData).mockReturnValue({ + status: 'loading', + data: undefined, + } as any) + + const { result } = renderHook(() => useShows()) + + expect(result.current.status).toBe('loading') + expect(result.current.shows).toEqual([]) + }) + + it('should return empty array when no shows', () => { + vi.mocked(reactfire.useDatabaseListData).mockReturnValue({ + status: 'success', + data: [], + } as any) + + const { result } = renderHook(() => useShows()) + + expect(result.current.status).toBe('success') + expect(result.current.shows).toEqual([]) + }) + + it('should format and return shows from database', () => { + const futureDate1 = new Date() + futureDate1.setDate(futureDate1.getDate() + 5) + const date1 = futureDate1.toISOString().split('T')[0] + + const futureDate2 = new Date() + futureDate2.setDate(futureDate2.getDate() + 10) + const date2 = futureDate2.toISOString().split('T')[0] + + const futureDate3 = new Date() + futureDate3.setDate(futureDate3.getDate() + 15) + const date3 = futureDate3.toISOString().split('T')[0] + + const mockShows = [ + { id: '1', date: date1, venue: 'Venue A', location: 'City A', time: '20:00' }, + { id: '2', date: date2, venue: 'Venue B', location: 'City B', time: '19:00' }, + { id: '3', date: date3, venue: 'Venue C', location: 'City C', time: '21:00' }, + ] + + vi.mocked(reactfire.useDatabaseListData).mockReturnValue({ + status: 'success', + data: mockShows, + } as any) + + const { result } = renderHook(() => useShows()) + + expect(result.current.shows.length).toBe(3) + expect(result.current.shows[0]._date).toBeDefined() + expect(result.current.shows[0]._location).toBeDefined() + }) + + it('should filter out past shows', () => { + const pastDate = new Date('2020-01-01') + const futureDate = new Date() + futureDate.setDate(futureDate.getDate() + 5) + + const mockShows = [ + { id: '1', date: pastDate.toISOString().split('T')[0], venue: 'Venue A', location: 'City A', time: '20:00' }, + { id: '2', date: futureDate.toISOString().split('T')[0], venue: 'Venue B', location: 'City B', time: '19:00' }, + ] + + vi.mocked(reactfire.useDatabaseListData).mockReturnValue({ + status: 'success', + data: mockShows, + } as any) + + const { result } = renderHook(() => useShows()) + + expect(result.current.shows.length).toBe(1) + expect(result.current.shows[0].id).toBe('2') + }) + + it('should return success status when data is loaded', () => { + const futureDate = new Date() + futureDate.setDate(futureDate.getDate() + 7) + + const mockShows = [ + { id: '1', date: futureDate.toISOString().split('T')[0], venue: 'Venue A', location: 'City A', time: '20:00' }, + ] + + vi.mocked(reactfire.useDatabaseListData).mockReturnValue({ + status: 'success', + data: mockShows, + } as any) + + const { result } = renderHook(() => useShows()) + + expect(result.current.status).toBe('success') + expect(result.current.shows.length).toBe(1) + }) + + it('should memoize shows data', () => { + const futureDate = new Date() + futureDate.setDate(futureDate.getDate() + 7) + + const mockShows = [ + { id: '1', date: futureDate.toISOString().split('T')[0], venue: 'Venue A', location: 'City A', time: '20:00' }, + ] + + vi.mocked(reactfire.useDatabaseListData).mockReturnValue({ + status: 'success', + data: mockShows, + } as any) + + const { result, rerender } = renderHook(() => useShows()) + + const firstRender = result.current.shows + rerender() + const secondRender = result.current.shows + + expect(firstRender).toBe(secondRender) + }) +}) diff --git a/src/components/SongPage/SongList/useSongs.test.ts b/src/components/SongPage/SongList/useSongs.test.ts new file mode 100644 index 0000000..eabccfe --- /dev/null +++ b/src/components/SongPage/SongList/useSongs.test.ts @@ -0,0 +1,150 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { renderHook } from '@testing-library/react' +import { useSongs } from './useSongs' +import * as reactfire from 'reactfire' + +vi.mock('reactfire') + +describe('useSongs', () => { + const mockDatabase = {} + const mockRef = vi.fn() + const mockQuery = vi.fn() + const mockOrderByChild = vi.fn() + + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(reactfire.useDatabase).mockReturnValue(mockDatabase as any) + }) + + it('should return loading status when data is loading', () => { + vi.mocked(reactfire.useDatabaseListData).mockReturnValue({ + status: 'loading', + data: undefined, + } as any) + + const { result } = renderHook(() => useSongs()) + + expect(result.current.status).toBe('loading') + expect(result.current.songs).toEqual([]) + }) + + it('should return empty array when no songs', () => { + vi.mocked(reactfire.useDatabaseListData).mockReturnValue({ + status: 'success', + data: [], + } as any) + + const { result } = renderHook(() => useSongs()) + + expect(result.current.status).toBe('success') + expect(result.current.songs).toEqual([]) + }) + + it('should sort songs by votes (descending)', () => { + const mockSongs = [ + { id: '1', title: 'Song A', votes: 5, visible: true, artist: 'Artist 1' }, + { id: '2', title: 'Song B', votes: 10, visible: true, artist: 'Artist 2' }, + { id: '3', title: 'Song C', votes: 7, visible: true, artist: 'Artist 3' }, + ] + + vi.mocked(reactfire.useDatabaseListData).mockReturnValue({ + status: 'success', + data: mockSongs, + } as any) + + const { result } = renderHook(() => useSongs()) + + expect(result.current.songs[0].votes).toBe(10) + expect(result.current.songs[1].votes).toBe(7) + expect(result.current.songs[2].votes).toBe(5) + }) + + it('should prioritize visible songs over invisible ones', () => { + const mockSongs = [ + { id: '1', title: 'Song A', votes: 10, visible: false, artist: 'Artist 1' }, + { id: '2', title: 'Song B', votes: 5, visible: true, artist: 'Artist 2' }, + ] + + vi.mocked(reactfire.useDatabaseListData).mockReturnValue({ + status: 'success', + data: mockSongs, + } as any) + + const { result } = renderHook(() => useSongs()) + + expect(result.current.songs[0].visible).toBe(true) + expect(result.current.songs[1].visible).toBe(false) + }) + + it('should sort alphabetically when votes are equal', () => { + const mockSongs = [ + { id: '1', title: 'Zebra', votes: 5, visible: true, artist: 'Artist 1' }, + { id: '2', title: 'Apple', votes: 5, visible: true, artist: 'Artist 2' }, + { id: '3', title: 'Banana', votes: 5, visible: true, artist: 'Artist 3' }, + ] + + vi.mocked(reactfire.useDatabaseListData).mockReturnValue({ + status: 'success', + data: mockSongs, + } as any) + + const { result } = renderHook(() => useSongs()) + + expect(result.current.songs[0].title).toBe('Apple') + expect(result.current.songs[1].title).toBe('Banana') + expect(result.current.songs[2].title).toBe('Zebra') + }) + + it('should filter songs by search term (title)', () => { + const mockSongs = [ + { id: '1', title: 'Wonderwall', votes: 5, visible: true, artist: 'Oasis' }, + { id: '2', title: 'Yellow', votes: 10, visible: true, artist: 'Coldplay' }, + { id: '3', title: 'Fix You', votes: 7, visible: true, artist: 'Coldplay' }, + ] + + vi.mocked(reactfire.useDatabaseListData).mockReturnValue({ + status: 'success', + data: mockSongs, + } as any) + + const { result } = renderHook(() => useSongs('yellow')) + + expect(result.current.songs.length).toBe(1) + expect(result.current.songs[0].title).toBe('Yellow') + }) + + it('should filter songs by search term (artist)', () => { + const mockSongs = [ + { id: '1', title: 'Wonderwall', votes: 5, visible: true, artist: 'Oasis' }, + { id: '2', title: 'Yellow', votes: 10, visible: true, artist: 'Coldplay' }, + { id: '3', title: 'Fix You', votes: 7, visible: true, artist: 'Coldplay' }, + ] + + vi.mocked(reactfire.useDatabaseListData).mockReturnValue({ + status: 'success', + data: mockSongs, + } as any) + + const { result } = renderHook(() => useSongs('coldplay')) + + expect(result.current.songs.length).toBe(2) + expect(result.current.songs[0].artist).toBe('Coldplay') + expect(result.current.songs[1].artist).toBe('Coldplay') + }) + + it('should filter case-insensitively', () => { + const mockSongs = [ + { id: '1', title: 'Wonderwall', votes: 5, visible: true, artist: 'Oasis' }, + ] + + vi.mocked(reactfire.useDatabaseListData).mockReturnValue({ + status: 'success', + data: mockSongs, + } as any) + + const { result } = renderHook(() => useSongs('WONDER')) + + expect(result.current.songs.length).toBe(1) + expect(result.current.songs[0].title).toBe('Wonderwall') + }) +}) diff --git a/src/test/setup.tsx b/src/test/setup.tsx index 16418a2..85b469e 100644 --- a/src/test/setup.tsx +++ b/src/test/setup.tsx @@ -12,6 +12,8 @@ vi.mock('firebase/database', () => ({ query: vi.fn(), orderByChild: vi.fn(), update: vi.fn(), + limitToLast: vi.fn(), + startAfter: vi.fn(), })) vi.mock('firebase/auth', () => ({