fix logging tests

src/services/apiClient.test.ts

@@ -1,5 +1,5 @@
 // src/services/apiClient.test.ts
-import { describe, it, expect, vi, afterAll, afterEach, beforeEach } from 'vitest';
+import { describe, it, expect, vi, afterAll, afterEach, beforeEach, beforeAll } from 'vitest';
 import { setupServer } from 'msw/node';
 import { http, HttpResponse } from 'msw';
 
@@ -45,6 +45,9 @@ describe('API Client', () => {
   let capturedHeaders: Headers | null = null;
   let capturedBody: string | FormData | Record<string, unknown> | null = null;
 
+  // Start the MSW server before all tests.
+  beforeAll(() => server.listen());
+
   beforeEach(() => {
     // Define a correctly typed mock function for fetch.
     const mockFetch = (url: RequestInfo | URL, options?: RequestInit): Promise<Response> => {
@@ -66,14 +69,15 @@ describe('API Client', () => {
       return Promise.resolve(new Response(JSON.stringify({ data: 'mock-success' }), { status: 200, headers: new Headers() as Headers }));
     };
 
-    // Assign the mock function to global.fetch and cast it to a Vitest mock.
-    global.fetch = vi.fn(mockFetch);
+    // Use spyOn instead of direct assignment to allow restoration for MSW tests.
+    vi.spyOn(global, 'fetch').mockImplementation(mockFetch);
   });
 
   afterEach(() => {
     server.resetHandlers();
     localStorageMock.clear();
-    vi.clearAllMocks();
+    // Restore all mocks (including global.fetch) to their original implementation.
+    vi.restoreAllMocks();
   });
   afterAll(() => server.close());
 
@@ -98,11 +102,11 @@ describe('API Client', () => {
     it('should handle token refresh on 401 response', async () => {
       localStorage.setItem('authToken', 'expired-token'); // Set an initial token
 
-      // Mock the fetch sequence:
+      // Mock the fetch sequence on the existing spy:
       // 1. Initial API call fails with 401
       // 2. `refreshToken` call succeeds with a new token
       // 3. Retried API call succeeds with the expected user data
-      global.fetch = vi.fn()
+      vi.mocked(global.fetch)
         .mockResolvedValueOnce({ ok: false, status: 401, json: () => Promise.resolve({ message: 'Unauthorized' }) } as Response)
         .mockResolvedValueOnce({ ok: true, status: 200, json: () => Promise.resolve({ token: 'new-refreshed-token' }) } as Response)
         .mockResolvedValueOnce({ ok: true, status: 200, json: () => Promise.resolve({ user_id: 'user-123' }) } as Response);
@@ -110,8 +114,6 @@ describe('API Client', () => {
       // The apiClient's internal refreshToken function will call the refresh endpoint.
       // We don't need a separate MSW handler for it if we are mocking global.fetch directly.
       // This test is now independent of MSW.
-      // The original test had a bug where the refresh endpoint was not mocked correctly for this specific flow.
-      // This new `vi.fn()` chain is more explicit and reliable for this test case.
       server.use(
         http.post('http://localhost/api/auth/refresh-token', () => {
           return HttpResponse.json({ token: 'new-refreshed-token' });
@@ -130,8 +132,8 @@ describe('API Client', () => {
     it('should reject if token refresh fails', async () => {
       localStorage.setItem('authToken', 'expired-token');
 
-      // Also use MSW for this failure case.
-      vi.spyOn(global, 'fetch').mockRestore();
+      // Restore the original fetch implementation so MSW can intercept requests.
+      vi.mocked(global.fetch).mockRestore();
 
       // Mock the initial 401 response.
       server.use(http.get('http://localhost/api/users/profile', () => new HttpResponse(null, { status: 401 })));
@@ -189,9 +191,7 @@ describe('API Client', () => {
   describe('Budget API Functions', () => {
     it('getBudgets should call the correct endpoint', async () => {
       server.use(
-        http.get('http://localhost/api/budgets', () => {
-          return HttpResponse.json([]);
-        })
+        http.get('http://localhost/api/budgets', () => HttpResponse.json([]))
       );
       await apiClient.getBudgets();
       expect(capturedUrl?.pathname).toBe('/api/budgets');
@@ -201,7 +201,7 @@ describe('API Client', () => {
       const budgetData = { name: 'Groceries', amount_cents: 50000, period: 'monthly' as const, start_date: '2024-01-01' };
       await apiClient.createBudget(budgetData);
 
-      expect(capturedUrl?.pathname).toBe('/api/budgets'); // This was a duplicate, fixed.
+      expect(capturedUrl?.pathname).toBe('/api/budgets');
       expect(capturedBody).toEqual(budgetData);
     });
 
@@ -209,13 +209,13 @@ describe('API Client', () => {
       const budgetUpdates = { amount_cents: 60000 };
       await apiClient.updateBudget(123, budgetUpdates);
 
-      expect(capturedUrl?.pathname).toBe('/api/budgets/123'); // This was a duplicate, fixed.
+      expect(capturedUrl?.pathname).toBe('/api/budgets/123');
       expect(capturedBody).toEqual(budgetUpdates);
     });
 
     it('deleteBudget should send a DELETE request to the correct URL', async () => {
       await apiClient.deleteBudget(456);
-      expect(capturedUrl?.pathname).toBe('/api/budgets/456'); // This was a duplicate, fixed.
+      expect(capturedUrl?.pathname).toBe('/api/budgets/456');
     });
 
     it('getSpendingAnalysis should send a GET request with correct query params', async () => {
@@ -239,7 +239,7 @@ describe('API Client', () => {
       await apiClient.fetchLeaderboard(5);
 
       expect(capturedUrl).not.toBeNull(); // This assertion ensures capturedUrl is not null for the next line
-      expect(capturedUrl?.pathname).toBe('/api/achievements/leaderboard');
+      expect(capturedUrl!.pathname).toBe('/api/achievements/leaderboard');
       expect(capturedUrl!.searchParams.get('limit')).toBe('5');
     });
 

src/services/db/user.db.test.ts

@@ -149,11 +149,28 @@ describe('User DB Service', () => {
 
       const mockClient = { query: vi.fn(), release: vi.fn() };
       vi.mocked(mockPoolInstance.connect).mockResolvedValue(mockClient as any);
-      mockClient.query.mockRejectedValue(dbError);
 
-      await expect(userRepo.createUser('exists@example.com', 'pass', {})).rejects.toThrow(UniqueConstraintError);
-      await expect(userRepo.createUser('exists@example.com', 'pass', {})).rejects.toThrow('A user with this email address already exists.');
+      // Simulate the transaction flow:
+      // 1. BEGIN (success)
+      // 2. set_config (success)
+      // 3. INSERT user (failure with unique violation)
+      // 4. ROLLBACK (success)
+      mockClient.query
+        .mockResolvedValueOnce({ rows: [] }) // BEGIN
+        .mockResolvedValueOnce({ rows: [] }) // set_config
+        .mockRejectedValueOnce(dbError)      // INSERT fails
+        .mockResolvedValueOnce({ rows: [] }); // ROLLBACK
+
+      try {
+        await userRepo.createUser('exists@example.com', 'pass', {});
+        expect.fail('Expected createUser to throw UniqueConstraintError');
+      } catch (error: any) {
+        expect(error).toBeInstanceOf(UniqueConstraintError);
+        expect(error.message).toBe('A user with this email address already exists.');
+      }
+
       expect(mockClient.query).toHaveBeenCalledWith('ROLLBACK');
+      expect(mockClient.release).toHaveBeenCalled();
     });
   });