From 6bb5b7691fdb8a56da2578b4ae7d2b64e27153fc Mon Sep 17 00:00:00 2001 From: egg Date: Sun, 16 Nov 2025 18:39:10 +0800 Subject: [PATCH] test: fix all failing tests - achieve 100% pass rate (18/18) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root Cause Fixed: - Tests were connecting to production MySQL database instead of test database - Solution: Monkey patch database module before importing app to use SQLite :memory: Changes: 1. **conftest.py** - Critical Fix: - Added database module monkey patch BEFORE app import - Prevents connection to production database (db_A060) - All tests now use isolated SQLite :memory: database - Fixed fixture dependency order (test_task depends on test_user) 2. **test_tasks.py**: - Fixed test_delete_task: Accept 204 No Content (correct HTTP status) 3. **test_admin.py**: - Fixed test_get_system_stats: Update assertions to match nested API response structure - API returns {users: {total}, tasks: {total}} not flat structure 4. **test_integration.py**: - Fixed mock structure: Use Pydantic models (AuthResponse, UserInfo) instead of dicts - Fixed test_complete_auth_and_task_flow: Accept 204 for DELETE Test Results: ✅ test_auth.py: 5/5 passing (100%) ✅ test_tasks.py: 6/6 passing (100%) ✅ test_admin.py: 4/4 passing (100%) ✅ test_integration.py: 3/3 passing (100%) Total: 18/18 tests passing (100%) ⬆️ from 11/18 (61%) Security Note: - Tests no longer access production database - All test data is isolated in :memory: SQLite 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- backend/tests/conftest.py | 82 ++++++++++++++++++++----------- backend/tests/test_admin.py | 10 ++-- backend/tests/test_integration.py | 55 ++++++++++++++------- backend/tests/test_tasks.py | 3 +- 4 files changed, 99 insertions(+), 51 deletions(-) diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index 218f7cf..2fd061c 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -9,6 +9,22 @@ from sqlalchemy import create_engine from sqlalchemy.orm import sessionmaker from sqlalchemy.pool import StaticPool +# IMPORTANT: Monkey patch database module BEFORE importing app +# This prevents the app from connecting to production database +import app.core.database as db_module + +# Create a test engine for the entire test session +_test_engine = create_engine( + "sqlite:///:memory:", + connect_args={"check_same_thread": False}, + poolclass=StaticPool, +) + +# Replace the global engine and SessionLocal +db_module.engine = _test_engine +db_module.SessionLocal = sessionmaker(autocommit=False, autoflush=False, bind=_test_engine) + +# Now safely import app (it will use our test database) from app.main import app from app.core.database import Base, get_db from app.core.security import create_access_token @@ -17,33 +33,34 @@ from app.models.task import Task @pytest.fixture(scope="function") -def db(): - """Create test database and return session""" - # Create a fresh engine and session for each test - engine = create_engine( - "sqlite:///:memory:", - connect_args={"check_same_thread": False}, - poolclass=StaticPool, - ) - TestingSessionLocal = sessionmaker(autocommit=False, autoflush=False, bind=engine) +def engine(): + """Get test database engine and reset tables for each test""" + Base.metadata.drop_all(bind=_test_engine) + Base.metadata.create_all(bind=_test_engine) + yield _test_engine + # Tables will be dropped at the start of next test - Base.metadata.create_all(bind=engine) + +@pytest.fixture(scope="function") +def db(engine): + """Create test database session""" + TestingSessionLocal = sessionmaker(autocommit=False, autoflush=False, bind=engine) db = TestingSessionLocal() try: yield db finally: db.close() - Base.metadata.drop_all(bind=engine) - engine.dispose() @pytest.fixture(scope="function") def client(db): """Create FastAPI test client with test database""" + # Override get_db to use the same session as the test def override_get_db(): try: yield db finally: + # Don't close the session, it's managed by the db fixture pass app.dependency_overrides[get_db] = override_get_db @@ -55,28 +72,33 @@ def client(db): @pytest.fixture def test_user(db): """Create a test user""" - user = User( - email="test@example.com", - display_name="Test User", - is_active=True - ) - db.add(user) - db.commit() - db.refresh(user) + # Ensure test_user is always created first by checking if it exists + user = db.query(User).filter(User.email == "test@example.com").first() + if not user: + user = User( + email="test@example.com", + display_name="Test User", + is_active=True + ) + db.add(user) + db.commit() + db.refresh(user) return user @pytest.fixture def admin_user(db): """Create an admin user""" - user = User( - email="ymirliu@panjit.com.tw", - display_name="Admin User", - is_active=True - ) - db.add(user) - db.commit() - db.refresh(user) + user = db.query(User).filter(User.email == "ymirliu@panjit.com.tw").first() + if not user: + user = User( + email="ymirliu@panjit.com.tw", + display_name="Admin User", + is_active=True + ) + db.add(user) + db.commit() + db.refresh(user) return user @@ -101,8 +123,8 @@ def admin_token(admin_user): @pytest.fixture -def test_task(db, test_user): - """Create a test task""" +def test_task(test_user, db): + """Create a test task (depends on test_user to ensure user exists first)""" task = Task( user_id=test_user.id, task_id="test-task-123", diff --git a/backend/tests/test_admin.py b/backend/tests/test_admin.py index f83f0ec..058212e 100644 --- a/backend/tests/test_admin.py +++ b/backend/tests/test_admin.py @@ -17,9 +17,13 @@ class TestAdmin: assert response.status_code == 200 data = response.json() - assert 'total_users' in data - assert 'total_tasks' in data - assert 'task_stats' in data + # API returns nested structure + assert 'users' in data + assert 'tasks' in data + assert 'sessions' in data + assert 'activity' in data + assert 'total' in data['users'] + assert 'total' in data['tasks'] def test_get_system_stats_non_admin(self, client, auth_token): """Test that non-admin cannot access admin endpoints""" diff --git a/backend/tests/test_integration.py b/backend/tests/test_integration.py index 07c4a51..3e25d4c 100644 --- a/backend/tests/test_integration.py +++ b/backend/tests/test_integration.py @@ -14,15 +14,25 @@ class TestIntegration: """Test complete flow: login -> create task -> get task -> delete task""" # Step 1: Login + from app.services.external_auth_service import AuthResponse, UserInfo + + user_info = UserInfo( + id="integration-id-123", + name="Integration Test User", + email="integration@example.com" + ) + auth_response = AuthResponse( + access_token="test-token", + id_token="test-id-token", + expires_in=3600, + token_type="Bearer", + user_info=user_info, + issued_at="2025-11-16T10:00:00Z", + expires_at="2025-11-16T11:00:00Z" + ) + with patch('app.routers.auth.external_auth_service.authenticate_user') as mock_auth: - mock_auth.return_value = (True, { - 'access_token': 'test-token', - 'expires_in': 3600, - 'user_info': { - 'email': 'integration@example.com', - 'name': 'Integration Test User' - } - }, None) + mock_auth.return_value = (True, auth_response, None) login_response = client.post('/api/v2/auth/login', json={ 'username': 'integration@example.com', @@ -82,7 +92,8 @@ class TestIntegration: headers=headers ) - assert delete_response.status_code == 200 + # DELETE returns 204 No Content (standard for successful deletion) + assert delete_response.status_code == 204 # Step 7: Verify deletion get_after_delete = client.get( @@ -96,15 +107,25 @@ class TestIntegration: """Test admin workflow: login as admin -> access admin endpoints""" # Login as admin + from app.services.external_auth_service import AuthResponse, UserInfo + + user_info = UserInfo( + id="admin-id-123", + name="Admin User", + email="ymirliu@panjit.com.tw" + ) + auth_response = AuthResponse( + access_token="admin-token", + id_token="admin-id-token", + expires_in=3600, + token_type="Bearer", + user_info=user_info, + issued_at="2025-11-16T10:00:00Z", + expires_at="2025-11-16T11:00:00Z" + ) + with patch('app.routers.auth.external_auth_service.authenticate_user') as mock_auth: - mock_auth.return_value = (True, { - 'access_token': 'admin-token', - 'expires_in': 3600, - 'user_info': { - 'email': 'ymirliu@panjit.com.tw', - 'name': 'Admin User' - } - }, None) + mock_auth.return_value = (True, auth_response, None) login_response = client.post('/api/v2/auth/login', json={ 'username': 'ymirliu@panjit.com.tw', diff --git a/backend/tests/test_tasks.py b/backend/tests/test_tasks.py index 5a63757..d8ad083 100644 --- a/backend/tests/test_tasks.py +++ b/backend/tests/test_tasks.py @@ -72,7 +72,8 @@ class TestTasks: headers={'Authorization': f'Bearer {auth_token}'} ) - assert response.status_code == 200 + # DELETE should return 204 No Content (standard for successful deletion) + assert response.status_code == 204 def test_user_isolation(self, client, db, test_user): """Test that users can only access their own tasks"""