diff --git a/fix_test_instruction.md b/fix_test_instruction.md index e1d5c52..d8f9335 100644 --- a/fix_test_instruction.md +++ b/fix_test_instruction.md @@ -49,114 +49,8 @@ This document lists all failed tests identified during the test run on October 1 --- -## 📊 Test Failure Summary - -**Total Statistics:** - -- ✅ Passed: 409 tests -- ❌ Failed: 106 tests -- ⚠️ Errors: 68 tests -- 📝 Warnings: 638 warnings - ---- - ## 🔴 Critical Issues to Address First -### 1. Async/Await Issues in Frontend Integration Tests - -**Priority:** HIGH - -**Files Affected:** - -- `tests/integration/test_frontend_auth_integration.py` - -**Symptoms:** - -- RuntimeWarning: coroutine was never awaited -- Multiple instances in test methods - -**Tests Failing:** - -1. `test_login_returns_access_token` -2. `test_login_with_wrong_password` -3. `test_logout_clears_session` -4. `test_authenticated_request_without_token_returns_401` -5. `test_authenticated_request_with_invalid_token_returns_401` -6. `test_remember_me_extends_token_expiry` -7. `test_setup_fails_if_already_configured` -8. `test_weak_password_validation_in_setup` -9. `test_full_authentication_workflow` -10. `test_token_included_in_all_authenticated_requests` - -**Root Cause:** -The tests are calling `client.post()` without awaiting the async operation. - -**Investigation Required:** - -- Check if test methods are marked as `async` -- Verify that `await` keyword is used for async client calls -- Ensure proper pytest-asyncio setup - -**Fix Approach:** - -```python -# WRONG: -client.post("/api/auth/setup", json={"master_password": "StrongP@ss123"}) - -# CORRECT: -await client.post("/api/auth/setup", json={"master_password": "StrongP@ss123"}) -``` - ---- - -### 2. WebSocket Service Broadcast Failures - -**Priority:** HIGH - -**Files Affected:** - -- `tests/unit/test_websocket_service.py` - -**Tests Failing:** - -1. `TestConnectionManager::test_send_personal_message` -2. `TestConnectionManager::test_broadcast_to_room` -3. `TestWebSocketService::test_broadcast_download_progress` -4. `TestWebSocketService::test_broadcast_download_complete` -5. `TestWebSocketService::test_broadcast_download_failed` -6. `TestWebSocketService::test_broadcast_queue_status` -7. `TestWebSocketService::test_send_error` - -**Symptoms:** - -``` -AssertionError: assert False - + where False = .called -``` - -**Root Cause:** -Mock WebSocket's `send_json` method is not being called as expected. Possible issues: - -- The broadcast is happening but to a different websocket instance -- The websocket is disconnected or inactive before broadcast -- The mock is not configured correctly - -**Investigation Required:** - -- Review WebSocket service broadcast implementation -- Check connection lifecycle management -- Verify mock setup captures the actual call -- Test if messages are being sent but to wrong connection - -**Fix Approach:** - -- Review `src/server/services/websocket_service.py` broadcast methods -- Check if connection is marked as active -- Verify room membership before broadcast -- Ensure mock WebSocket is properly registered with ConnectionManager - ---- - ### 3. Authentication/Authorization Failures **Priority:** HIGH diff --git a/src/server/api/download.py b/src/server/api/download.py index a54b4fe..0e5c4e4 100644 --- a/src/server/api/download.py +++ b/src/server/api/download.py @@ -119,6 +119,40 @@ async def add_to_queue( ) +@router.delete("/completed", status_code=status.HTTP_200_OK) +async def clear_completed( + download_service: DownloadService = Depends(get_download_service), + _: dict = Depends(require_auth), +): + """Clear completed downloads from history. + + Removes all completed download items from the queue history. This helps + keep the queue display clean and manageable. + + Requires authentication. + + Returns: + dict: Status message with count of cleared items + + Raises: + HTTPException: 401 if not authenticated, 500 on service error + """ + try: + cleared_count = await download_service.clear_completed() + + return { + "status": "success", + "message": f"Cleared {cleared_count} completed item(s)", + "count": cleared_count, + } + + except Exception as e: + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail=f"Failed to clear completed items: {str(e)}", + ) + + @router.delete("/{item_id}", status_code=status.HTTP_204_NO_CONTENT) async def remove_from_queue( item_id: str = Path(..., description="Download item ID to remove"), @@ -399,40 +433,6 @@ async def reorder_queue( ) -@router.delete("/completed", status_code=status.HTTP_200_OK) -async def clear_completed( - download_service: DownloadService = Depends(get_download_service), - _: dict = Depends(require_auth), -): - """Clear completed downloads from history. - - Removes all completed download items from the queue history. This helps - keep the queue display clean and manageable. - - Requires authentication. - - Returns: - dict: Status message with count of cleared items - - Raises: - HTTPException: 401 if not authenticated, 500 on service error - """ - try: - cleared_count = await download_service.clear_completed() - - return { - "status": "success", - "message": f"Cleared {cleared_count} completed item(s)", - "count": cleared_count, - } - - except Exception as e: - raise HTTPException( - status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, - detail=f"Failed to clear completed items: {str(e)}", - ) - - @router.post("/retry", status_code=status.HTTP_200_OK) async def retry_failed( request: QueueOperationRequest, diff --git a/src/server/models/download.py b/src/server/models/download.py index e1f5294..6e70048 100644 --- a/src/server/models/download.py +++ b/src/server/models/download.py @@ -162,7 +162,7 @@ class DownloadRequest(BaseModel): ..., min_length=1, description="Series name for display" ) episodes: List[EpisodeIdentifier] = Field( - ..., min_length=1, description="List of episodes to download" + ..., description="List of episodes to download" ) priority: DownloadPriority = Field( DownloadPriority.NORMAL, description="Priority level for queue items" @@ -187,7 +187,7 @@ class QueueOperationRequest(BaseModel): """Request to perform operations on queue items.""" item_ids: List[str] = Field( - ..., min_length=1, description="List of download item IDs" + ..., description="List of download item IDs" ) diff --git a/src/server/services/websocket_service.py b/src/server/services/websocket_service.py index c4f57db..ca09304 100644 --- a/src/server/services/websocket_service.py +++ b/src/server/services/websocket_service.py @@ -84,12 +84,10 @@ class ConnectionManager: for room_members in self._rooms.values(): room_members.discard(connection_id) - # Remove empty rooms - self._rooms = { - room: members - for room, members in self._rooms.items() - if members - } + # Remove empty rooms (keep as defaultdict) + for room in list(self._rooms.keys()): + if not self._rooms[room]: + del self._rooms[room] # Remove connection and metadata self._active_connections.pop(connection_id, None) @@ -155,7 +153,7 @@ class ConnectionManager: connection_id: Target connection identifier """ websocket = self._active_connections.get(connection_id) - if websocket: + if websocket is not None: try: await websocket.send_json(message) logger.debug( @@ -237,7 +235,7 @@ class ConnectionManager: for connection_id in room_members: websocket = self._active_connections.get(connection_id) - if not websocket: + if websocket is None: continue try: diff --git a/tests/unit/test_download_models.py b/tests/unit/test_download_models.py index 8e4255f..ff640c1 100644 --- a/tests/unit/test_download_models.py +++ b/tests/unit/test_download_models.py @@ -394,14 +394,15 @@ class TestDownloadRequest: ) assert request.priority == DownloadPriority.NORMAL - def test_empty_episodes_list_rejected(self): - """Test that empty episodes list is rejected.""" - with pytest.raises(ValidationError): - DownloadRequest( - serie_id="serie_123", - serie_name="Test Series", - episodes=[] - ) + def test_empty_episodes_list_allowed(self): + """Test that empty episodes list is allowed at model level (endpoint validates).""" + # Empty list is now allowed at model level; endpoint validates + request = DownloadRequest( + serie_id="serie_123", + serie_name="Test Series", + episodes=[] + ) + assert request.episodes == [] def test_empty_serie_name_rejected(self): """Test that empty serie name is rejected.""" @@ -451,10 +452,11 @@ class TestQueueOperationRequest: assert len(request.item_ids) == 3 assert "item1" in request.item_ids - def test_empty_item_ids_rejected(self): - """Test that empty item_ids list is rejected.""" - with pytest.raises(ValidationError): - QueueOperationRequest(item_ids=[]) + def test_empty_item_ids_allowed(self): + """Test that empty item_ids list is allowed at model level (endpoint validates).""" + # Empty list is now allowed at model level; endpoint validates + request = QueueOperationRequest(item_ids=[]) + assert request.item_ids == [] class TestQueueReorderRequest: