diff --git a/src/routes/system.routes.test.ts b/src/routes/system.routes.test.ts index 860f4a11..0410ad64 100644 --- a/src/routes/system.routes.test.ts +++ b/src/routes/system.routes.test.ts @@ -1,10 +1,15 @@ // src/routes/system.routes.test.ts import { describe, it, expect, vi, beforeEach } from 'vitest'; import supertest from 'supertest'; -import { exec, type ExecException, type ExecOptions } from 'child_process'; -import { geocodingService } from '../services/geocodingService.server'; import { createTestApp } from '../tests/utils/createTestApp'; +// 1. Mock the Service Layer +// This decouples the route test from the service's implementation details. +vi.mock('../services/systemService', () => ({ + systemService: { + getPm2Status: vi.fn(), + }, +})); // 2. Mock Geocoding vi.mock('../services/geocodingService.server', () => ({ geocodingService: { @@ -24,46 +29,24 @@ vi.mock('../services/logger.server', () => ({ })); // Import the router AFTER all mocks are defined to ensure systemService picks up the mocked util.promisify +import { systemService } from '../services/systemService'; import systemRouter from './system.routes'; +import { geocodingService } from '../services/geocodingService.server'; describe('System Routes (/api/system)', () => { const app = createTestApp({ router: systemRouter, basePath: '/api/system' }); beforeEach(() => { - // We cast here to get type-safe access to mock functions like .mockImplementation vi.clearAllMocks(); }); describe('GET /pm2-status', () => { it('should return success: true when pm2 process is online', async () => { // Arrange: Simulate a successful `pm2 describe` output for an online process. - const pm2OnlineOutput = ` - ┌─ PM2 info ────────────────┐ - │ status │ online │ - └───────────┴───────────┘ - `; - - type ExecCallback = (error: ExecException | null, stdout: string, stderr: string) => void; - - // A robust mock for `exec` that handles its multiple overloads. - // This avoids the complex and error-prone `...args` signature. - vi.mocked(exec).mockImplementation( - ( - command: string, - options?: ExecOptions | ExecCallback | null, - callback?: ExecCallback | null, - ) => { - // The actual callback can be the second or third argument. - const actualCallback = ( - typeof options === 'function' ? options : callback - ) as ExecCallback; - if (actualCallback) { - actualCallback(null, pm2OnlineOutput, ''); - } - // Return a minimal object that satisfies the ChildProcess type for .unref() - return { unref: () => {} } as ReturnType; - }, - ); + vi.mocked(systemService.getPm2Status).mockResolvedValue({ + success: true, + message: 'Application is online and running under PM2.', + }); // Act const response = await supertest(app).get('/api/system/pm2-status'); @@ -77,28 +60,10 @@ describe('System Routes (/api/system)', () => { }); it('should return success: false when pm2 process is stopped or errored', async () => { - const pm2StoppedOutput = `│ status │ stopped │`; - - vi.mocked(exec).mockImplementation( - ( - command: string, - options?: - | ExecOptions - | ((error: ExecException | null, stdout: string, stderr: string) => void) - | null, - callback?: ((error: ExecException | null, stdout: string, stderr: string) => void) | null, - ) => { - const actualCallback = (typeof options === 'function' ? options : callback) as ( - error: ExecException | null, - stdout: string, - stderr: string, - ) => void; - if (actualCallback) { - actualCallback(null, pm2StoppedOutput, ''); - } - return { unref: () => {} } as ReturnType; - }, - ); + vi.mocked(systemService.getPm2Status).mockResolvedValue({ + success: false, + message: 'Application process exists but is not online.', + }); const response = await supertest(app).get('/api/system/pm2-status'); @@ -109,33 +74,10 @@ describe('System Routes (/api/system)', () => { it('should return success: false when pm2 process does not exist', async () => { // Arrange: Simulate `pm2 describe` failing because the process isn't found. - const processNotFoundOutput = - "[PM2][ERROR] Process or Namespace flyer-crawler-api doesn't exist"; - const processNotFoundError = new Error( - 'Command failed: pm2 describe flyer-crawler-api', - ) as ExecException; - processNotFoundError.code = 1; - - vi.mocked(exec).mockImplementation( - ( - command: string, - options?: - | ExecOptions - | ((error: ExecException | null, stdout: string, stderr: string) => void) - | null, - callback?: ((error: ExecException | null, stdout: string, stderr: string) => void) | null, - ) => { - const actualCallback = (typeof options === 'function' ? options : callback) as ( - error: ExecException | null, - stdout: string, - stderr: string, - ) => void; - if (actualCallback) { - actualCallback(processNotFoundError, processNotFoundOutput, ''); - } - return { unref: () => {} } as ReturnType; - }, - ); + vi.mocked(systemService.getPm2Status).mockResolvedValue({ + success: false, + message: 'Application process is not running under PM2.', + }); // Act const response = await supertest(app).get('/api/system/pm2-status'); @@ -150,55 +92,17 @@ describe('System Routes (/api/system)', () => { it('should return 500 if pm2 command produces stderr output', async () => { // Arrange: Simulate a successful exit code but with content in stderr. - const stderrOutput = 'A non-fatal warning occurred.'; - - vi.mocked(exec).mockImplementation( - ( - command: string, - options?: - | ExecOptions - | ((error: ExecException | null, stdout: string, stderr: string) => void) - | null, - callback?: ((error: ExecException | null, stdout: string, stderr: string) => void) | null, - ) => { - const actualCallback = (typeof options === 'function' ? options : callback) as ( - error: ExecException | null, - stdout: string, - stderr: string, - ) => void; - if (actualCallback) { - actualCallback(null, 'Some stdout', stderrOutput); - } - return { unref: () => {} } as ReturnType; - }, - ); + const serviceError = new Error('PM2 command produced an error: A non-fatal warning occurred.'); + vi.mocked(systemService.getPm2Status).mockRejectedValue(serviceError); const response = await supertest(app).get('/api/system/pm2-status'); expect(response.status).toBe(500); - expect(response.body.message).toBe(`PM2 command produced an error: ${stderrOutput}`); + expect(response.body.message).toBe(serviceError.message); }); it('should return 500 on a generic exec error', async () => { - vi.mocked(exec).mockImplementation( - ( - command: string, - options?: - | ExecOptions - | ((error: ExecException | null, stdout: string, stderr: string) => void) - | null, - callback?: ((error: ExecException | null, stdout: string, stderr: string) => void) | null, - ) => { - const actualCallback = (typeof options === 'function' ? options : callback) as ( - error: ExecException | null, - stdout: string, - stderr: string, - ) => void; - if (actualCallback) { - actualCallback(new Error('System error') as ExecException, '', 'stderr output'); - } - return { unref: () => {} } as ReturnType; - }, - ); + const serviceError = new Error('System error'); + vi.mocked(systemService.getPm2Status).mockRejectedValue(serviceError); // Act const response = await supertest(app).get('/api/system/pm2-status'); diff --git a/src/services/analyticsService.server.test.ts b/src/services/analyticsService.server.test.ts index 8311d9c4..a102880e 100644 --- a/src/services/analyticsService.server.test.ts +++ b/src/services/analyticsService.server.test.ts @@ -127,19 +127,16 @@ describe('AnalyticsService', () => { throw new Error('Processing failed'); }); // "Successfully generated..." - // Wrap the async operation that is expected to reject in a function. - // This prevents an "unhandled rejection" error by ensuring the `expect.rejects` - // is actively waiting for the promise to reject when the timers are advanced. - const testFunction = async () => { - const promise = service.processWeeklyReportJob(job); - // Advance timers to trigger the part of the code that throws. - await vi.advanceTimersByTimeAsync(30000); - // Await the promise to allow the rejection to be caught by `expect.rejects`. - await promise; - }; + // Get the promise from the service method. + const promise = service.processWeeklyReportJob(job); - // Now, assert that the entire operation rejects as expected. - await expect(testFunction()).rejects.toThrow('Processing failed'); + // Advance timers to trigger the part of the code that throws. + await vi.advanceTimersByTimeAsync(30000); + + // Now, assert that the promise rejects as expected. + // This structure avoids an unhandled promise rejection that can occur + // when awaiting a rejecting promise inside a helper function without a try/catch. + await expect(promise).rejects.toThrow('Processing failed'); // Verify the side effect (error logging) after the rejection is confirmed. expect(mockLoggerInstance.error).toHaveBeenCalledWith( diff --git a/src/services/systemService.test.ts b/src/services/systemService.test.ts index 4bf00ae0..484b7350 100644 --- a/src/services/systemService.test.ts +++ b/src/services/systemService.test.ts @@ -1,7 +1,7 @@ // src/services/systemService.test.ts -import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { exec, type ExecException } from 'child_process'; +import { describe, it, expect, vi, beforeEach, Mock } from 'vitest'; import { logger } from './logger.server'; +import type { ExecException } from 'child_process'; // Mock logger vi.mock('./logger.server', () => ({ @@ -12,12 +12,19 @@ vi.mock('./logger.server', () => ({ }, })); -// Import service AFTER mocks to ensure top-level promisify uses the mock -import { systemService } from './systemService'; +// Import the class, not the singleton instance, to apply Dependency Injection +import { SystemService } from './systemService'; describe('SystemService', () => { + let systemService: SystemService; + let mockExecAsync: Mock; + beforeEach(() => { vi.clearAllMocks(); + // Create a mock function for our dependency + mockExecAsync = vi.fn(); + // Instantiate the service with the mock dependency + systemService = new SystemService(mockExecAsync); }); describe('getPm2Status', () => { @@ -29,10 +36,7 @@ describe('SystemService', () => { │ 0 │ flyer-crawler-api │ online │ └────┴──────────────────────┴──────────┘ `; - vi.mocked(exec).mockImplementation((cmd, callback: any) => { - callback(null, stdout, ''); - return {} as any; - }); + mockExecAsync.mockResolvedValue({ stdout, stderr: '' }); const result = await systemService.getPm2Status(); @@ -50,10 +54,7 @@ describe('SystemService', () => { │ 0 │ flyer-crawler-api │ stopped │ └────┴──────────────────────┴──────────┘ `; - vi.mocked(exec).mockImplementation((cmd, callback: any) => { - callback(null, stdout, ''); - return {} as any; - }); + mockExecAsync.mockResolvedValue({ stdout, stderr: '' }); const result = await systemService.getPm2Status(); @@ -64,12 +65,11 @@ describe('SystemService', () => { }); it('should throw error if stderr has content', async () => { - vi.mocked(exec).mockImplementation((cmd, callback: any) => { - callback(null, 'some stdout', 'some stderr warning'); - return {} as any; - }); + mockExecAsync.mockResolvedValue({ stdout: 'some stdout', stderr: 'some stderr warning' }); - await expect(systemService.getPm2Status()).rejects.toThrow('PM2 command produced an error: some stderr warning'); + await expect(systemService.getPm2Status()).rejects.toThrow( + 'PM2 command produced an error: some stderr warning', + ); }); it('should return success: false when process does not exist', async () => { @@ -77,10 +77,7 @@ describe('SystemService', () => { error.code = 1; error.stderr = "[PM2][ERROR] Process or Namespace flyer-crawler-api doesn't exist"; - vi.mocked(exec).mockImplementation((cmd, callback: any) => { - callback(error, '', error.stderr); - return {} as any; - }); + mockExecAsync.mockRejectedValue(error); const result = await systemService.getPm2Status(); @@ -89,7 +86,7 @@ describe('SystemService', () => { message: 'Application process is not running under PM2.', }); expect(logger.warn).toHaveBeenCalledWith( - expect.stringContaining('PM2 process "flyer-crawler-api" not found') + expect.stringContaining('PM2 process "flyer-crawler-api" not found'), ); }); }); diff --git a/src/services/systemService.ts b/src/services/systemService.ts index f713b1d1..425ad6e1 100644 --- a/src/services/systemService.ts +++ b/src/services/systemService.ts @@ -1,16 +1,24 @@ // src/services/systemService.ts -import { exec } from 'child_process'; +import { exec as nodeExec, type ExecException } from 'child_process'; import { promisify } from 'util'; import { logger } from './logger.server'; -const execAsync = promisify(exec); -logger.debug({ typeOfExec: typeof exec, isExecFunction: typeof exec === 'function' }, 'SystemService: Initializing execAsync'); -logger.debug({ typeOfPromisify: typeof promisify, isPromisifyFunction: typeof promisify === 'function' }, 'SystemService: Initializing execAsync'); +// Define a type for the exec function for better type safety and testability. +// It matches the signature of a promisified child_process.exec. +export type ExecAsync = ( + command: string, +) => Promise<{ stdout: string; stderr: string }>; + +export class SystemService { + private execAsync: ExecAsync; + + constructor(execAsync: ExecAsync) { + this.execAsync = execAsync; + } -class SystemService { async getPm2Status(): Promise<{ success: boolean; message: string }> { try { - const { stdout, stderr } = await execAsync('pm2 describe flyer-crawler-api'); + const { stdout, stderr } = await this.execAsync('pm2 describe flyer-crawler-api'); // If the command runs but produces output on stderr, treat it as an error. // This handles cases where pm2 might issue warnings but still exit 0. @@ -23,7 +31,7 @@ class SystemService { ? 'Application is online and running under PM2.' : 'Application process exists but is not online.'; return { success: isOnline, message }; - } catch (error: any) { + } catch (error: ExecException | any) { // If the command fails (non-zero exit code), check if it's because the process doesn't exist. // This is a normal "not found" case, not a system error. // The error message can be in stdout or stderr depending on the pm2 version. @@ -42,4 +50,6 @@ class SystemService { } } -export const systemService = new SystemService(); \ No newline at end of file +// Instantiate the service with the real dependency for the application +const realExecAsync = promisify(nodeExec); +export const systemService = new SystemService(realExecAsync); \ No newline at end of file diff --git a/src/tests/setup/tests-setup-unit.ts b/src/tests/setup/tests-setup-unit.ts index 25e62088..1a093d03 100644 --- a/src/tests/setup/tests-setup-unit.ts +++ b/src/tests/setup/tests-setup-unit.ts @@ -143,12 +143,10 @@ vi.mock('util', async (importOriginal) => { }); // Mock 'child_process' using the robust `importOriginal` pattern. -// This preserves the module's structure and prevents "No default export" errors. -vi.mock('child_process', async (importOriginal) => { - const actual = await importOriginal(); +// This is needed because some services (systemService, workers.server) import it at the top level. +vi.mock('child_process', () => { return { - ...actual, - // Provide a mock function for `exec` that can be implemented per-test. + __esModule: true, // Handle CJS/ESM interop exec: vi.fn(), }; }); @@ -174,6 +172,7 @@ vi.mock('crypto', () => ({ randomBytes: vi.fn().mockReturnValue({ toString: vi.fn().mockReturnValue('mocked-random-string'), }), + randomUUID: vi.fn().mockReturnValue('mocked-random-string'), }, }));