From 648f1c023965e5d2d279fc30b86e3d68b3d2eaf7 Mon Sep 17 00:00:00 2001 From: Torben Sorensen Date: Mon, 15 Dec 2025 00:53:51 -0800 Subject: [PATCH] Refactor: Update mocks in test files for improved structure and clarity --- src/contexts/ApiContext.tsx | 2 +- src/routes/admin.content.routes.test.ts | 21 ++++--- src/routes/admin.jobs.routes.test.ts | 7 ++- src/routes/admin.monitoring.routes.test.ts | 9 ++- src/routes/admin.stats.routes.test.ts | 10 +++- src/routes/admin.system.routes.test.ts | 7 ++- src/routes/admin.users.routes.test.ts | 13 ++++- src/services/db/flyer.db.test.ts | 2 +- src/services/queueService.server.test.ts | 67 ++++++++++------------ src/utils/pdfConverter.test.ts | 11 ++-- 10 files changed, 92 insertions(+), 57 deletions(-) diff --git a/src/contexts/ApiContext.tsx b/src/contexts/ApiContext.tsx index bd71f8f9..5931baa3 100644 --- a/src/contexts/ApiContext.tsx +++ b/src/contexts/ApiContext.tsx @@ -1,5 +1,5 @@ // src/context/ApiContext.tsx -import React, { createContext } from 'react'; +import { createContext } from 'react'; import * as apiClient from '../services/apiClient'; /** diff --git a/src/routes/admin.content.routes.test.ts b/src/routes/admin.content.routes.test.ts index 33787e80..d677968a 100644 --- a/src/routes/admin.content.routes.test.ts +++ b/src/routes/admin.content.routes.test.ts @@ -35,18 +35,23 @@ const { mockedDb } = vi.hoisted(() => { getAllBrands: vi.fn(), deleteFlyer: vi.fn(), }, + recipeRepo: { + deleteRecipe: vi.fn(), + }, + userRepo: { + findUserProfileById: vi.fn(), + deleteUserById: vi.fn(), + }, } } }); -vi.mock('../services/db/index.db', async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - adminRepo: mockedDb.adminRepo, - flyerRepo: mockedDb.flyerRepo, - }; -}); +vi.mock('../services/db/index.db', () => ({ + adminRepo: mockedDb.adminRepo, + flyerRepo: mockedDb.flyerRepo, + recipeRepo: mockedDb.recipeRepo, + userRepo: mockedDb.userRepo, +})); // Mock other dependencies vi.mock('../services/db/recipe.db'); diff --git a/src/routes/admin.jobs.routes.test.ts b/src/routes/admin.jobs.routes.test.ts index 213f6475..b377846a 100644 --- a/src/routes/admin.jobs.routes.test.ts +++ b/src/routes/admin.jobs.routes.test.ts @@ -31,7 +31,12 @@ vi.mock('../services/queueService.server', () => ({ cleanupWorker: {}, weeklyAnalyticsWorker: {}, })); -vi.mock('../services/db/index.db'); // Mock the entire DB service +vi.mock('../services/db/index.db', () => ({ + adminRepo: {}, + flyerRepo: {}, + recipeRepo: {}, + userRepo: {}, +})); vi.mock('../services/geocodingService.server'); vi.mock('node:fs/promises'); diff --git a/src/routes/admin.monitoring.routes.test.ts b/src/routes/admin.monitoring.routes.test.ts index b2e0cf1e..6abbc4ed 100644 --- a/src/routes/admin.monitoring.routes.test.ts +++ b/src/routes/admin.monitoring.routes.test.ts @@ -17,7 +17,14 @@ vi.mock('../lib/queue', () => ({ cleanupQueue: {}, })); -vi.mock('../services/db/index.db'); +vi.mock('../services/db/index.db', () => ({ + adminRepo: { + getActivityLog: vi.fn(), + }, + flyerRepo: {}, + recipeRepo: {}, + userRepo: {}, +})); // Mock the queue service to control worker statuses vi.mock('../services/queueService.server', () => ({ diff --git a/src/routes/admin.stats.routes.test.ts b/src/routes/admin.stats.routes.test.ts index 941b97c8..c5426100 100644 --- a/src/routes/admin.stats.routes.test.ts +++ b/src/routes/admin.stats.routes.test.ts @@ -8,7 +8,15 @@ import { UserProfile } from '../types'; import { mockLogger } from '../tests/utils/mockLogger'; import { createTestApp } from '../tests/utils/createTestApp'; -vi.mock('../services/db/index.db'); +vi.mock('../services/db/index.db', () => ({ + adminRepo: { + getApplicationStats: vi.fn(), + getDailyStatsForLast30Days: vi.fn(), + }, + flyerRepo: {}, + recipeRepo: {}, + userRepo: {}, +})); // Mock other dependencies vi.mock('../services/db/flyer.db'); diff --git a/src/routes/admin.system.routes.test.ts b/src/routes/admin.system.routes.test.ts index b4afc29c..9cbf9c15 100644 --- a/src/routes/admin.system.routes.test.ts +++ b/src/routes/admin.system.routes.test.ts @@ -14,7 +14,12 @@ vi.mock('../services/geocodingService.server', () => ({ })); // Mock other dependencies that are part of the adminRouter setup but not directly tested here -vi.mock('../services/db/admin.db'); +vi.mock('../services/db/index.db', () => ({ + adminRepo: {}, + flyerRepo: {}, + recipeRepo: {}, + userRepo: {}, +})); vi.mock('../services/db/flyer.db'); vi.mock('../services/db/recipe.db'); vi.mock('../services/db/user.db'); diff --git a/src/routes/admin.users.routes.test.ts b/src/routes/admin.users.routes.test.ts index 91c6fbcd..28b5066f 100644 --- a/src/routes/admin.users.routes.test.ts +++ b/src/routes/admin.users.routes.test.ts @@ -9,7 +9,18 @@ import { NotFoundError } from '../services/db/errors.db'; import { mockLogger } from '../tests/utils/mockLogger'; import { createTestApp } from '../tests/utils/createTestApp'; -vi.mock('../services/db/index.db'); +vi.mock('../services/db/index.db', () => ({ + adminRepo: { + getAllUsers: vi.fn(), + updateUserRole: vi.fn(), + }, + userRepo: { + findUserProfileById: vi.fn(), + deleteUserById: vi.fn(), + }, + flyerRepo: {}, + recipeRepo: {}, +})); // Mock other dependencies that are not directly tested but are part of the adminRouter setup vi.mock('../services/db/flyer.db'); diff --git a/src/services/db/flyer.db.test.ts b/src/services/db/flyer.db.test.ts index 28bbf259..e2d923c0 100644 --- a/src/services/db/flyer.db.test.ts +++ b/src/services/db/flyer.db.test.ts @@ -458,8 +458,8 @@ describe('Flyer DB Service', () => { await flyerRepo.deleteFlyer(42, mockLogger); - expect(withTransaction).toHaveBeenCalledTimes(1); expect(mockClientQuery).toHaveBeenCalledWith('DELETE FROM public.flyers WHERE flyer_id = $1', [42]); + expect(withTransaction).toHaveBeenCalledTimes(1); }); it('should throw an error if the flyer to delete is not found', async () => { diff --git a/src/services/queueService.server.test.ts b/src/services/queueService.server.test.ts index d98e2e6a..e0a70ebc 100644 --- a/src/services/queueService.server.test.ts +++ b/src/services/queueService.server.test.ts @@ -1,6 +1,7 @@ // src/services/queueService.server.test.ts -import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; // Use modern 'node:' prefix for built-in modules +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { logger as mockLogger } from './logger.server'; +import { EventEmitter } from 'node:events'; import type { Job, Worker } from 'bullmq'; import type { Mock } from 'vitest'; @@ -18,49 +19,39 @@ interface MockQueueInstance { close: Mock<() => Promise>; } -// --- Hoisted Mocks --- -const mocks = vi.hoisted(() => { - // FIX: Import EventEmitter inside the hoisted block to avoid initialization errors. - const { EventEmitter } = require('node:events'); +// --- Inline Mock Implementations --- - // Define the type for the mock Redis connection instance - type MockRedisConnectionInstance = InstanceType & { ping: Mock }; +// Create a single, shared mock Redis connection instance that we can control in tests. +const mockRedisConnection = new EventEmitter() as EventEmitter & { ping: Mock }; +mockRedisConnection.ping = vi.fn().mockResolvedValue('PONG'); - const mockRedisConnection = new EventEmitter() as MockRedisConnectionInstance; - mockRedisConnection.ping = vi.fn().mockResolvedValue('PONG'); - - const hoistedMocks = { - mockRedisConnection: mockRedisConnection, // Reference the one created above - - MockWorker: vi.fn(function (this: MockWorkerInstance, name: string) { - this.name = name; - this.on = vi.fn(); - this.close = vi.fn().mockResolvedValue(undefined); - this.isRunning = vi.fn().mockReturnValue(true); - return this; - }), - - MockQueue: vi.fn(function (this: MockQueueInstance, name: string) { - this.name = name; - this.add = vi.fn(); - this.close = vi.fn().mockResolvedValue(undefined); - return this; - }), - }; - return hoistedMocks; -}); - -// FIX: Standard function for IORedis constructor +// Mock the 'ioredis' library. The default export is a class constructor. +// We make it a mock function that returns our shared `mockRedisConnection` instance. vi.mock('ioredis', () => ({ default: vi.fn(function() { - return mocks.mockRedisConnection; + return mockRedisConnection; }), })); +// Mock the 'bullmq' library. vi.mock('bullmq', () => ({ - Worker: mocks.MockWorker, - Queue: mocks.MockQueue, + // Mock the Worker class constructor. + Worker: vi.fn(function (this: MockWorkerInstance, name: string) { + this.name = name; + this.on = vi.fn(); + this.close = vi.fn().mockResolvedValue(undefined); + this.isRunning = vi.fn().mockReturnValue(true); + return this; + }), + // Mock the Queue class constructor. + Queue: vi.fn(function (this: MockQueueInstance, name: string) { + this.name = name; + this.add = vi.fn(); + this.close = vi.fn().mockResolvedValue(undefined); + return this; + }), })); + vi.mock('./logger.server', () => ({ logger: { info: vi.fn(), @@ -98,12 +89,12 @@ describe('Queue Service Setup and Lifecycle', () => { afterEach(() => { // Clean up all event listeners on the mock connection to prevent open handles. - mocks.mockRedisConnection.removeAllListeners(); + mockRedisConnection.removeAllListeners(); }); it('should log a success message when Redis connects', () => { // Act: Simulate the 'connect' event on the mock Redis connection - mocks.mockRedisConnection.emit('connect'); + mockRedisConnection.emit('connect'); // Assert: Check if the logger was called with the expected message expect(mockLogger.info).toHaveBeenCalledWith('[Redis] Connection established successfully.'); @@ -111,7 +102,7 @@ describe('Queue Service Setup and Lifecycle', () => { it('should log an error message when Redis connection fails', () => { const redisError = new Error('Connection refused'); - mocks.mockRedisConnection.emit('error', redisError); + mockRedisConnection.emit('error', redisError); expect(mockLogger.error).toHaveBeenCalledWith({ err: redisError }, '[Redis] Connection error.'); }); diff --git a/src/utils/pdfConverter.test.ts b/src/utils/pdfConverter.test.ts index ab144493..daa9a5ad 100644 --- a/src/utils/pdfConverter.test.ts +++ b/src/utils/pdfConverter.test.ts @@ -176,14 +176,17 @@ describe('pdfConverter', () => { }); it('should throw an error if conversion results in zero images for a non-empty PDF', async () => { + // Arrange: Ensure the document appears to have pages mockPdfDocument.numPages = 1; const pdfFile = new File(['pdf-content'], 'flyer.pdf', { type: 'application/pdf' }); - // Mock a page render to fail, which will cause Promise.allSettled to have a rejected promise - const { getDocument } = await import('pdfjs-dist'); - vi.mocked(getDocument).mockReturnValue({ promise: Promise.reject(new Error('Corrupted page')) } as unknown as PDFDocumentLoadingTask); + // Mock getPage to fail for the first page. This simulates a corrupted page + // within an otherwise valid PDF document, which is what the function's + // Promise.allSettled logic is designed to handle. + vi.mocked(mockPdfDocument.getPage).mockRejectedValueOnce(new Error('Corrupted page')); + + // Act & Assert: The function should catch the settled promise and re-throw the reason. - // The function should re-throw the reason for the first failure. await expect(convertPdfToImageFiles(pdfFile)).rejects.toThrow('Corrupted page'); });