From 4874f2a0a3a06c958a20011a26ab96e42b624190 Mon Sep 17 00:00:00 2001
From: Chris Chen <yuanson.ch@gmail.com>
Date: Tue, 26 May 2026 20:38:58 -0700
Subject: [PATCH] test: fix spec issues from code review (logout assertion
 order, refresh 5xx test)

---
 .../app/shared/services/auth.service.spec.ts  | 54 ++++++++++++-------
 1 file changed, 36 insertions(+), 18 deletions(-)

diff --git a/APP/src/app/shared/services/auth.service.spec.ts b/APP/src/app/shared/services/auth.service.spec.ts
index 2d1ceec..c9d4a10 100644
--- a/APP/src/app/shared/services/auth.service.spec.ts
+++ b/APP/src/app/shared/services/auth.service.spec.ts
@@ -3,6 +3,9 @@ import {
   HttpClientTestingModule,
   HttpTestingController
 } from '@angular/common/http/testing';
+// NOTE: UserInfo is the interface Task 2 will add to auth.service.ts.
+// Importing it here intentionally causes a compile error until Task 2 is implemented.
+// Task 2 MUST export `UserInfo` (not the old `User`) for these tests to pass.
 import {
   AuthService,
   LoginResultType,
@@ -124,22 +127,36 @@ describe('AuthService', () => {
       expect(service.getToken()).toBeNull();
       expect(service.isAuthenticated()).toBeFalse();
     });
+
+    it('should return false and not throw on 5xx error', () => {
+      let result: boolean | undefined;
+      service.refresh().subscribe(r => result = r);
+      httpMock.expectOne(`${apiConfig.authUrl}/refresh`).flush(
+        { message: 'Server error' },
+        { status: 500, statusText: 'Internal Server Error' }
+      );
+      expect(result).toBeFalse();
+      expect(service.isAuthenticated()).toBeFalse();
+    });
   });
 
   // ── logout() ───────────────────────────────────────────────────────────────
 
   describe('logout()', () => {
-    it('should clear token and user from memory immediately', () => {
-      // Seed state
-      service['accessToken$'].next('some-token');
-      service['currentUser$'].next(MOCK_USER);
-
-      service.logout();
-      httpMock.expectOne(`${apiConfig.authUrl}/logout`).flush(null, { status: 204, statusText: 'No Content' });
-
-      expect(service.getToken()).toBeNull();
-      expect(service.getCurrentUser()).toBeNull();
-      expect(service.isAuthenticated()).toBeFalse();
+    it('should clear token and user from memory immediately', (done) => {
+      // Seed state via the login flow (avoids accessing private BehaviorSubjects)
+      service.login({ email: 'test@example.com', password: 'secret' }).subscribe(() => {
+        // Now token + user are set — call logout and assert BEFORE flushing the HTTP call
+        service.logout();
+        expect(service.getToken()).toBeNull();
+        expect(service.getCurrentUser()).toBeNull();
+        expect(service.isAuthenticated()).toBeFalse();
+        // Now flush the logout HTTP call so httpMock.verify() is satisfied
+        httpMock.expectOne(`${apiConfig.authUrl}/logout`).flush(null, { status: 204, statusText: 'No Content' });
+        done();
+      });
+      // Flush the login call
+      httpMock.expectOne(`${apiConfig.authUrl}/login`).flush(MOCK_API_RESPONSE);
     });
 
     it('should POST to /api/auth/logout with withCredentials', () => {
@@ -151,13 +168,14 @@ describe('AuthService', () => {
     });
 
     it('should not throw if the logout API call fails', () => {
-      expect(() => {
-        service.logout();
-        httpMock.expectOne(`${apiConfig.authUrl}/logout`).flush(
-          { message: 'Server error' },
-          { status: 500, statusText: 'Internal Server Error' }
-        );
-      }).not.toThrow();
+      service.logout();
+      // Flush an error response — service must swallow it
+      httpMock.expectOne(`${apiConfig.authUrl}/logout`).flush(
+        { message: 'Server error' },
+        { status: 500, statusText: 'Internal Server Error' }
+      );
+      // If we reach here without an unhandled error, the test passes
+      expect(service.isAuthenticated()).toBeFalse();
     });
   });