splitting unit + integration testing apart attempt #1

src/routes/admin.test.ts

@@ -495,6 +495,7 @@ describe('Admin Routes (/api/admin)', () => {
         expect(response.body).toEqual(mockLogs);
         expect(mockedDb.getActivityLog).toHaveBeenCalledTimes(1);
         // Check that default pagination values were used
+        // This makes the test more robust by verifying the correct parameters were passed.
         expect(mockedDb.getActivityLog).toHaveBeenCalledWith(50, 0);
       });
 

src/routes/auth.test.ts

@@ -96,6 +96,8 @@ describe('Auth Routes (/api/auth)', () => {
       expect(response.status).toBe(201);
       expect(response.body.message).toBe('User registered successfully!');
       expect(response.body.user.email).toBe(newUserEmail);
+      // Assert against more of the mocked data for a more precise test.
+      expect(response.body.user.user_id).toBe(mockNewUser.user_id);
       expect(response.body.token).toBeTypeOf('string');
 
       // Verify that the correct DB functions were called

src/routes/passport.test.ts

@@ -3,14 +3,14 @@ import { describe, it, expect, vi, beforeEach, type Mocked, type Mock } from 'vi
 import { Request, Response, NextFunction } from 'express';
 
 // This variable will hold the verify callback passed to the JwtStrategy constructor.
-// It must be defined before the mock and the imports.
-let verifyCallback: (payload: any, done: (err: any, user?: any, info?: any) => void) => void;
+// It must be defined at the top level to be accessible inside the mock factory.
+let capturedVerifyCallback: (payload: any, done: (err: any, user?: any, info?: any) => void) => void;
 
 // Mock the 'passport-jwt' module to capture the verify callback.
 vi.mock('passport-jwt', () => ({
   // The Strategy constructor is mocked to capture its second argument, which is the verify function.
   Strategy: vi.fn((options, verify) => {
-    verifyCallback = verify; // Capture the callback for testing.
+    capturedVerifyCallback = verify; // Capture the callback for testing.
     // Return a dummy strategy object that satisfies Passport's `use` method.
     return { name: 'jwt', authenticate: vi.fn() };
   }),
@@ -180,7 +180,7 @@ describe('Passport Configuration', () => {
       const done = vi.fn();
 
       // Act: Directly invoke the strategy's verification function
-      await verifyCallback(jwtPayload, done);
+      await capturedVerifyCallback(jwtPayload, done);
 
       // Assert
       expect(mockedDb.findUserProfileById).toHaveBeenCalledWith('user-123');
@@ -194,7 +194,7 @@ describe('Passport Configuration', () => {
       const done = vi.fn();
 
       // Act
-      await verifyCallback(jwtPayload, done);
+      await capturedVerifyCallback(jwtPayload, done);
 
       // Assert
       expect(done).toHaveBeenCalledWith(null, false);
@@ -208,7 +208,7 @@ describe('Passport Configuration', () => {
       const done = vi.fn();
 
       // Act
-      await verifyCallback(jwtPayload, done);
+      await capturedVerifyCallback(jwtPayload, done);
 
       // Assert
       expect(done).toHaveBeenCalledWith(dbError, false);

src/routes/user.test.ts

@@ -347,8 +347,11 @@ describe('User Routes (/api/users)', () => {
         it('should update user preferences successfully', async () => {
           // Arrange
           const preferencesUpdate = { darkMode: true, unitSystem: 'metric' as const };
-          // Mock the DB function that is called to update preferences
-          const updatedProfile = { ...mockUserProfile, preferences: preferencesUpdate };
+          // FIX: The mock should return a profile that accurately reflects the update.
+          // Previously, it returned a profile with empty preferences.
+          const updatedProfile = { 
+            ...mockUserProfile, 
+            preferences: { ...mockUserProfile.preferences, ...preferencesUpdate } };
           vi.mocked(mockedDb.updateUserPreferences).mockResolvedValue(updatedProfile);
 
           // Act

src/services/aiService.server.test.ts

@@ -6,11 +6,7 @@ import type { readFile as ReadFileFn } from 'fs/promises';
 // 1. Hoist the mock function so it's available inside the mock factory
 // We add a logging implementation to the hoisted function.
 const { mockGenerateContent } = vi.hoisted(() => ({
-  mockGenerateContent: vi.fn().mockImplementation((...args: any[]) => {
-    console.log(`[AI MOCK] 🤖 generateContent called. Args length: ${args.length}`);
-    // Default return to prevent crashes if mockResolvedValue isn't set in a specific test
-    return Promise.resolve({ response: { text: () => '[]' } });
-  }),
+  mockGenerateContent: vi.fn().mockResolvedValue({ response: { text: () => '[]' } }),
 }));
 // Mock fs/promises
 const mockReadFile = vi.fn();
@@ -24,24 +20,16 @@ vi.mock('fs/promises', () => ({
 // 2. Mock Google GenAI with a constructible function and logging
 vi.mock('@google/genai', () => {
   return {
-    GoogleGenAI: vi.fn().mockImplementation((config) => {
+    // FIX: Use a standard function for the constructor to support `new GoogleGenAI()`
+    GoogleGenAI: vi.fn(function(config) {
       console.log('[AI MOCK] 🏗️  GoogleGenAI constructor initialized.');
-      
       return {
-        // Path 1: If code uses `genAI.models.generateContent`
         models: {
-          get generateContent() {
-            console.log('[AI MOCK] accessing .models.generateContent');
-            return mockGenerateContent;
-          }
+          generateContent: mockGenerateContent,
         },
-        // Path 2: If code uses `genAI.getGenerativeModel(...)`
-        getGenerativeModel: vi.fn((modelConfig) => {
-          console.log(`[AI MOCK] ⚙️  getGenerativeModel called for model: ${modelConfig?.model}`);
-          return {
-            generateContent: mockGenerateContent
-          };
-        })
+        getGenerativeModel: vi.fn(() => ({
+          generateContent: mockGenerateContent,
+        })),
       };
     }),
   };

src/tests/setup/unit-setup.ts

@@ -51,16 +51,9 @@ afterEach(cleanup);
 // By placing mocks here, they are guaranteed to be hoisted and applied
 // before any test files are executed, preventing initialization errors.
 
-// --- Global Mocks with Logging ---
-
-// Define mock spies with logging implementations
-const mockQuery = vi.fn().mockImplementation((...args) => {
-  // console.log('[PG MOCK] 🔎 Query executed:', args[0]); // Uncomment for very verbose SQL logging
-  return Promise.resolve({ rows: [], rowCount: 0 });
-});
-
+// 1. Create stable spies outside the mock factory so they can be imported by tests.
+const mockQuery = vi.fn().mockResolvedValue({ rows: [], rowCount: 0 });
 const mockRelease = vi.fn();
-
 const mockConnect = vi.fn().mockResolvedValue({
   query: mockQuery,
   release: mockRelease,
@@ -68,38 +61,27 @@ const mockConnect = vi.fn().mockResolvedValue({
 
 const mockPoolInstance = {
   connect: mockConnect,
-  query: mockQuery,
+  query: mockQuery, // This spy is what tests will assert against.
   end: vi.fn(),
   on: vi.fn(),
-  totalCount: 0,
-  idleCount: 0,
+  totalCount: 10, // Match test expectation
+  idleCount: 5,   // Match test expectation
   waitingCount: 0,
 };
 
 /**
  * Mocks the `pg` module. This is a robust mock that ensures `new Pool()` works
- * as a constructor in all tests.
- * FIX: Using a standard function for Pool allows usage with 'new Pool()'
- * LOGGING: Adds console logs to prove the mock is being loaded and instantiated.
+ * as a constructor and that tests can access the correct spy instances.
  */
+// 2. Use the same spy instances in the mock factory.
 vi.mock('pg', () => {
-  // Define as a standard function declaration to ensure it constructs properly
-  const MockPool = function(config: any) {
-    console.log('\n[PG MOCK] 🛠️  Pool constructor called. Config keys:', Object.keys(config || {}));
-    return mockPoolInstance;
-  };
-
+  const PoolConstructor = vi.fn(() => mockPoolInstance);
   return {
     __esModule: true,
-    // We wrap MockPool in vi.fn() so we can still track calls to the constructor if needed
-    default: { 
-      Pool: vi.fn(MockPool),
-      types: { setTypeParser: vi.fn() } 
-    },
-    Pool: vi.fn(MockPool),
-    types: {
-      setTypeParser: vi.fn(),
-    },
+    // Mock both default and named exports for Pool to ensure consistency.
+    default: { Pool: PoolConstructor, types: { setTypeParser: vi.fn() } },
+    Pool: PoolConstructor,
+    types: { setTypeParser: vi.fn() },
   };
 });