From 4db53c93dfd0243f210fcf5b72ac3ffcdffd763c Mon Sep 17 00:00:00 2001 From: Lukas Date: Sun, 19 Oct 2025 20:27:30 +0200 Subject: [PATCH] fixed tests --- fix_test_instruction.md | 53 ------ tests/integration/test_websocket.py | 257 +++++++++++++++------------- 2 files changed, 142 insertions(+), 168 deletions(-) diff --git a/fix_test_instruction.md b/fix_test_instruction.md index d8f9335..3cd1953 100644 --- a/fix_test_instruction.md +++ b/fix_test_instruction.md @@ -51,59 +51,6 @@ This document lists all failed tests identified during the test run on October 1 ## 🔴 Critical Issues to Address First -### 3. Authentication/Authorization Failures - -**Priority:** HIGH - -**Files Affected:** - -- `tests/api/test_config_endpoints.py` -- `tests/api/test_download_endpoints.py` -- `tests/integration/test_auth_flow.py` - -**Tests Failing:** - -#### Config Endpoints (7 failures) - -1. `test_validate_invalid_config` -2. `test_update_config_unauthorized` -3. `test_list_backups` (403 instead of 200) -4. `test_create_backup` (403 instead of expected) -5. `test_restore_backup` (403 instead of expected) -6. `test_delete_backup` (403 instead of expected) -7. `test_config_persistence` (403 instead of expected) - -#### Download Endpoints (2 failures) - -8. `test_get_queue_status_unauthorized` -9. `test_queue_endpoints_require_auth` - -#### Auth Flow Integration (43 failures) - -Multiple test classes affected - all authentication flow tests - -**Symptoms:** - -- 403 Forbidden responses when 200 expected -- Authentication/authorization not working as expected -- Tokens not being validated correctly - -**Root Cause Options:** - -1. Middleware not properly checking authentication -2. Test fixtures not setting up auth correctly -3. Token generation/validation broken -4. Session management issues - -**Investigation Required:** - -- Review `src/server/middleware/auth.py` -- Check `src/server/services/auth_service.py` -- Verify test fixtures for authentication setup -- Check if routes are properly protected - ---- - ### 4. Frontend Integration Test Errors **Priority:** HIGH diff --git a/tests/integration/test_websocket.py b/tests/integration/test_websocket.py index 545d8f8..3646ae6 100644 --- a/tests/integration/test_websocket.py +++ b/tests/integration/test_websocket.py @@ -10,18 +10,14 @@ This module tests the complete WebSocket integration including: - Concurrent client management """ import asyncio -import json -from typing import Any, Dict, List -from unittest.mock import AsyncMock, Mock, patch +import uuid +from unittest.mock import AsyncMock import pytest from httpx import ASGITransport, AsyncClient -from starlette.websockets import WebSocketDisconnect from src.server.fastapi_app import app -from src.server.models.download import DownloadPriority, DownloadStatus from src.server.services.auth_service import auth_service -from src.server.services.progress_service import ProgressType from src.server.services.websocket_service import ( ConnectionManager, get_websocket_service, @@ -85,10 +81,6 @@ class TestWebSocketConnection: async def test_websocket_endpoint_exists(self, client, auth_token): """Test that WebSocket endpoint is available.""" - # This test verifies the endpoint exists - # Full WebSocket testing requires WebSocket client - - # Verify the WebSocket route is registered routes = [route.path for route in app.routes] websocket_routes = [ path for path in routes if "ws" in path or "websocket" in path @@ -100,42 +92,44 @@ class TestWebSocketConnection: ): """Test that connection manager tracks active connections.""" manager = websocket_service.manager + connection_id = "test-conn-1" - # Initially no connections - initial_count = len(manager.active_connections) + initial_count = await manager.get_connection_count() - # Add a connection - await manager.connect(mock_websocket, room="test-room") + mock_websocket.accept = AsyncMock() + await manager.connect(mock_websocket, connection_id) - assert len(manager.active_connections) == initial_count + 1 - assert mock_websocket in manager.active_connections + assert await manager.get_connection_count() == initial_count + 1 + assert connection_id in manager._active_connections async def test_disconnect_removes_connection( self, websocket_service, mock_websocket ): """Test that disconnecting removes connection from manager.""" manager = websocket_service.manager + connection_id = "test-conn-2" - # Connect - await manager.connect(mock_websocket, room="test-room") - assert mock_websocket in manager.active_connections + mock_websocket.accept = AsyncMock() + await manager.connect(mock_websocket, connection_id) + assert connection_id in manager._active_connections - # Disconnect - manager.disconnect(mock_websocket) - assert mock_websocket not in manager.active_connections + await manager.disconnect(connection_id) + assert connection_id not in manager._active_connections async def test_room_assignment_on_connection( self, websocket_service, mock_websocket ): - """Test that connections are assigned to rooms.""" + """Test that connections can join rooms.""" manager = websocket_service.manager + connection_id = "test-conn-3" room = "test-room-1" - await manager.connect(mock_websocket, room=room) + mock_websocket.accept = AsyncMock() + await manager.connect(mock_websocket, connection_id) + await manager.join_room(connection_id, room) - # Verify connection is in the room assert room in manager._rooms - assert mock_websocket in manager._rooms[room] + assert connection_id in manager._rooms[room] async def test_multiple_rooms_support( self, websocket_service @@ -144,15 +138,20 @@ class TestWebSocketConnection: manager = websocket_service.manager ws1 = AsyncMock() + ws1.accept = AsyncMock() ws2 = AsyncMock() + ws2.accept = AsyncMock() ws3 = AsyncMock() + ws3.accept = AsyncMock() - # Connect to different rooms - await manager.connect(ws1, room="room-1") - await manager.connect(ws2, room="room-2") - await manager.connect(ws3, room="room-1") + await manager.connect(ws1, "conn-1") + await manager.connect(ws2, "conn-2") + await manager.connect(ws3, "conn-3") + + await manager.join_room("conn-1", "room-1") + await manager.join_room("conn-2", "room-2") + await manager.join_room("conn-3", "room-1") - # Verify room structure assert "room-1" in manager._rooms assert "room-2" in manager._rooms assert len(manager._rooms["room-1"]) == 2 @@ -168,20 +167,20 @@ class TestMessageBroadcasting: """Test broadcasting message to all connected clients.""" manager = websocket_service.manager - # Create mock connections ws1 = AsyncMock() + ws1.accept = AsyncMock() ws2 = AsyncMock() + ws2.accept = AsyncMock() ws3 = AsyncMock() + ws3.accept = AsyncMock() - await manager.connect(ws1, room="room-1") - await manager.connect(ws2, room="room-1") - await manager.connect(ws3, room="room-2") + await manager.connect(ws1, "conn-1") + await manager.connect(ws2, "conn-2") + await manager.connect(ws3, "conn-3") - # Broadcast to all message = {"type": "test", "data": "broadcast to all"} await manager.broadcast(message) - # All connections should receive message ws1.send_json.assert_called_once() ws2.send_json.assert_called_once() ws3.send_json.assert_called_once() @@ -193,18 +192,23 @@ class TestMessageBroadcasting: manager = websocket_service.manager ws1 = AsyncMock() + ws1.accept = AsyncMock() ws2 = AsyncMock() + ws2.accept = AsyncMock() ws3 = AsyncMock() + ws3.accept = AsyncMock() - await manager.connect(ws1, room="downloads") - await manager.connect(ws2, room="downloads") - await manager.connect(ws3, room="system") + await manager.connect(ws1, "conn-1") + await manager.connect(ws2, "conn-2") + await manager.connect(ws3, "conn-3") + + await manager.join_room("conn-1", "downloads") + await manager.join_room("conn-2", "downloads") + await manager.join_room("conn-3", "system") - # Broadcast to specific room message = {"type": "download_progress", "data": {}} await manager.broadcast_to_room(message, room="downloads") - # Only room members should receive assert ws1.send_json.call_count == 1 assert ws2.send_json.call_count == 1 assert ws3.send_json.call_count == 0 @@ -215,7 +219,8 @@ class TestMessageBroadcasting: """Test broadcasting JSON-formatted messages.""" manager = websocket_service.manager ws = AsyncMock() - await manager.connect(ws, room="test") + ws.accept = AsyncMock() + await manager.connect(ws, "conn-1") message = { "type": "queue_update", @@ -237,20 +242,19 @@ class TestMessageBroadcasting: """Test that broadcasting handles disconnected clients gracefully.""" manager = websocket_service.manager - # Mock connection that will fail failing_ws = AsyncMock() + failing_ws.accept = AsyncMock() failing_ws.send_json.side_effect = RuntimeError("Connection closed") working_ws = AsyncMock() + working_ws.accept = AsyncMock() - await manager.connect(failing_ws, room="test") - await manager.connect(working_ws, room="test") + await manager.connect(failing_ws, "conn-1") + await manager.connect(working_ws, "conn-2") - # Broadcast should handle failure message = {"type": "test", "data": "test"} await manager.broadcast(message) - # Working connection should still receive working_ws.send_json.assert_called_once() @@ -263,9 +267,10 @@ class TestProgressIntegration: """Test that download progress updates broadcast via WebSocket.""" manager = websocket_service.manager ws = AsyncMock() - await manager.connect(ws, room="downloads") + ws.accept = AsyncMock() + await manager.connect(ws, "conn-1") + await manager.join_room("conn-1", "downloads") - # Simulate progress update broadcast message = { "type": "download_progress", "data": { @@ -287,7 +292,9 @@ class TestProgressIntegration: """Test download completion notification via WebSocket.""" manager = websocket_service.manager ws = AsyncMock() - await manager.connect(ws, room="downloads") + ws.accept = AsyncMock() + await manager.connect(ws, "conn-1") + await manager.join_room("conn-1", "downloads") message = { "type": "download_complete", @@ -308,7 +315,9 @@ class TestProgressIntegration: """Test download failure notification via WebSocket.""" manager = websocket_service.manager ws = AsyncMock() - await manager.connect(ws, room="downloads") + ws.accept = AsyncMock() + await manager.connect(ws, "conn-1") + await manager.join_room("conn-1", "downloads") message = { "type": "download_failed", @@ -333,7 +342,9 @@ class TestQueueStatusBroadcasting: """Test broadcasting queue status updates.""" manager = websocket_service.manager ws = AsyncMock() - await manager.connect(ws, room="queue") + ws.accept = AsyncMock() + await manager.connect(ws, "conn-1") + await manager.join_room("conn-1", "queue") message = { "type": "queue_status", @@ -356,7 +367,9 @@ class TestQueueStatusBroadcasting: """Test notification when item is added to queue.""" manager = websocket_service.manager ws = AsyncMock() - await manager.connect(ws, room="queue") + ws.accept = AsyncMock() + await manager.connect(ws, "conn-1") + await manager.join_room("conn-1", "queue") message = { "type": "queue_item_added", @@ -378,7 +391,9 @@ class TestQueueStatusBroadcasting: """Test notification when item is removed from queue.""" manager = websocket_service.manager ws = AsyncMock() - await manager.connect(ws, room="queue") + ws.accept = AsyncMock() + await manager.connect(ws, "conn-1") + await manager.join_room("conn-1", "queue") message = { "type": "queue_item_removed", @@ -403,9 +418,14 @@ class TestSystemMessaging: manager = websocket_service.manager ws1 = AsyncMock() + ws1.accept = AsyncMock() ws2 = AsyncMock() - await manager.connect(ws1, room="system") - await manager.connect(ws2, room="system") + ws2.accept = AsyncMock() + await manager.connect(ws1, "conn-1") + await manager.connect(ws2, "conn-2") + + await manager.join_room("conn-1", "system") + await manager.join_room("conn-2", "system") message = { "type": "system_notification", @@ -427,7 +447,9 @@ class TestSystemMessaging: """Test broadcasting error messages.""" manager = websocket_service.manager ws = AsyncMock() - await manager.connect(ws, room="errors") + ws.accept = AsyncMock() + await manager.connect(ws, "conn-1") + await manager.join_room("conn-1", "errors") message = { "type": "error", @@ -452,16 +474,17 @@ class TestConcurrentConnections: """Test multiple clients receiving broadcasts in same room.""" manager = websocket_service.manager - # Create multiple connections - clients = [AsyncMock() for _ in range(5)] - for client in clients: - await manager.connect(client, room="shared-room") + clients = [] + for i in range(5): + ws = AsyncMock() + ws.accept = AsyncMock() + clients.append(ws) + await manager.connect(ws, f"conn-{i}") + await manager.join_room(f"conn-{i}", "shared-room") - # Broadcast to all message = {"type": "test", "data": "multi-client test"} await manager.broadcast_to_room(message, room="shared-room") - # All clients should receive for client in clients: client.send_json.assert_called_once_with(message) @@ -471,16 +494,21 @@ class TestConcurrentConnections: """Test concurrent broadcasts to different rooms.""" manager = websocket_service.manager - # Setup rooms with clients downloads_ws = AsyncMock() + downloads_ws.accept = AsyncMock() queue_ws = AsyncMock() + queue_ws.accept = AsyncMock() system_ws = AsyncMock() + system_ws.accept = AsyncMock() - await manager.connect(downloads_ws, room="downloads") - await manager.connect(queue_ws, room="queue") - await manager.connect(system_ws, room="system") + await manager.connect(downloads_ws, "conn-1") + await manager.connect(queue_ws, "conn-2") + await manager.connect(system_ws, "conn-3") + + await manager.join_room("conn-1", "downloads") + await manager.join_room("conn-2", "queue") + await manager.join_room("conn-3", "system") - # Concurrent broadcasts await asyncio.gather( manager.broadcast_to_room( {"type": "download_progress"}, "downloads" @@ -493,7 +521,6 @@ class TestConcurrentConnections: ) ) - # Each client should receive only their room's message downloads_ws.send_json.assert_called_once() queue_ws.send_json.assert_called_once() system_ws.send_json.assert_called_once() @@ -508,13 +535,12 @@ class TestConnectionErrorHandling: """Test handling of message send failures.""" manager = websocket_service.manager - # Connection that will fail on send failing_ws = AsyncMock() + failing_ws.accept = AsyncMock() failing_ws.send_json.side_effect = RuntimeError("Send failed") - await manager.connect(failing_ws, room="test") + await manager.connect(failing_ws, "conn-1") - # Should handle error gracefully message = {"type": "test", "data": "test"} try: await manager.broadcast_to_room(message, room="test") @@ -527,23 +553,23 @@ class TestConnectionErrorHandling: """Test handling multiple concurrent send failures.""" manager = websocket_service.manager - # Multiple failing connections failing_clients = [] for i in range(3): ws = AsyncMock() + ws.accept = AsyncMock() ws.send_json.side_effect = RuntimeError(f"Failed {i}") failing_clients.append(ws) - await manager.connect(ws, room="test") + await manager.connect(ws, f"conn-{i}") + await manager.join_room(f"conn-{i}", "test") - # Add one working connection working_ws = AsyncMock() - await manager.connect(working_ws, room="test") + working_ws.accept = AsyncMock() + await manager.connect(working_ws, "conn-working") + await manager.join_room("conn-working", "test") - # Broadcast should continue despite failures message = {"type": "test", "data": "test"} await manager.broadcast_to_room(message, room="test") - # Working connection should still receive working_ws.send_json.assert_called_once() async def test_cleanup_after_disconnect( @@ -552,16 +578,18 @@ class TestConnectionErrorHandling: """Test proper cleanup after client disconnect.""" manager = websocket_service.manager ws = AsyncMock() + ws.accept = AsyncMock() room = "test-room" + connection_id = "test-conn" - # Connect and then disconnect - await manager.connect(ws, room=room) - manager.disconnect(ws) + await manager.connect(ws, connection_id) + await manager.join_room(connection_id, room) - # Verify cleanup - assert ws not in manager.active_connections + await manager.disconnect(connection_id) + + assert connection_id not in manager._active_connections if room in manager._rooms: - assert ws not in manager._rooms[room] + assert connection_id not in manager._rooms[room] class TestMessageFormatting: @@ -573,9 +601,9 @@ class TestMessageFormatting: """Test that messages have required structure.""" manager = websocket_service.manager ws = AsyncMock() - await manager.connect(ws, room="test") + ws.accept = AsyncMock() + await manager.connect(ws, "conn-1") - # Valid message structure valid_message = { "type": "test_message", "data": {"key": "value"}, @@ -590,7 +618,8 @@ class TestMessageFormatting: """Test broadcasting different message types.""" manager = websocket_service.manager ws = AsyncMock() - await manager.connect(ws, room="test") + ws.accept = AsyncMock() + await manager.connect(ws, "conn-1") message_types = [ "download_progress", @@ -605,7 +634,6 @@ class TestMessageFormatting: message = {"type": msg_type, "data": {}} await manager.broadcast(message) - # Should have received all message types assert ws.send_json.call_count == len(message_types) @@ -652,15 +680,14 @@ class TestRoomManagement: """Test that room is created when first client connects.""" manager = websocket_service.manager ws = AsyncMock() + ws.accept = AsyncMock() room = "new-room" - # Room should not exist initially assert room not in manager._rooms - # Connect to room - await manager.connect(ws, room=room) + await manager.connect(ws, "conn-1") + await manager.join_room("conn-1", room) - # Room should now exist assert room in manager._rooms async def test_room_cleanup_when_empty( @@ -669,14 +696,14 @@ class TestRoomManagement: """Test that empty rooms are cleaned up.""" manager = websocket_service.manager ws = AsyncMock() + ws.accept = AsyncMock() room = "temp-room" + connection_id = "conn-1" - # Connect and disconnect - await manager.connect(ws, room=room) - manager.disconnect(ws) + await manager.connect(ws, connection_id) + await manager.join_room(connection_id, room) + await manager.disconnect(connection_id) - # Room should be cleaned up if empty - # (Implementation may vary) if room in manager._rooms: assert len(manager._rooms[room]) == 0 @@ -686,13 +713,13 @@ class TestRoomManagement: """Test client room membership.""" manager = websocket_service.manager ws = AsyncMock() + ws.accept = AsyncMock() - # Connect to room - await manager.connect(ws, room="room-1") + await manager.connect(ws, "conn-1") + await manager.join_room("conn-1", "room-1") - # Verify in room assert "room-1" in manager._rooms - assert ws in manager._rooms["room-1"] + assert "conn-1" in manager._rooms["room-1"] class TestCompleteWebSocketWorkflow: @@ -704,17 +731,15 @@ class TestCompleteWebSocketWorkflow: """Test complete workflow of download notifications.""" manager = websocket_service.manager ws = AsyncMock() - await manager.connect(ws, room="downloads") + ws.accept = AsyncMock() + await manager.connect(ws, "conn-1") + await manager.join_room("conn-1", "downloads") - # Simulate download lifecycle - - # 1. Download started await manager.broadcast_to_room( {"type": "download_started", "data": {"item_id": "dl-1"}}, "downloads" ) - # 2. Progress updates for progress in [25, 50, 75]: await manager.broadcast_to_room( { @@ -724,13 +749,11 @@ class TestCompleteWebSocketWorkflow: "downloads" ) - # 3. Download complete await manager.broadcast_to_room( {"type": "download_complete", "data": {"item_id": "dl-1"}}, "downloads" ) - # Client should have received all notifications assert ws.send_json.call_count == 5 async def test_multi_room_workflow( @@ -739,16 +762,21 @@ class TestCompleteWebSocketWorkflow: """Test workflow involving multiple rooms.""" manager = websocket_service.manager - # Setup clients in different rooms download_ws = AsyncMock() + download_ws.accept = AsyncMock() queue_ws = AsyncMock() + queue_ws.accept = AsyncMock() system_ws = AsyncMock() + system_ws.accept = AsyncMock() - await manager.connect(download_ws, room="downloads") - await manager.connect(queue_ws, room="queue") - await manager.connect(system_ws, room="system") + await manager.connect(download_ws, "conn-1") + await manager.connect(queue_ws, "conn-2") + await manager.connect(system_ws, "conn-3") + + await manager.join_room("conn-1", "downloads") + await manager.join_room("conn-2", "queue") + await manager.join_room("conn-3", "system") - # Broadcast to each room await manager.broadcast_to_room( {"type": "download_update"}, "downloads" ) @@ -759,7 +787,6 @@ class TestCompleteWebSocketWorkflow: {"type": "system_update"}, "system" ) - # Each client should only receive their room's messages download_ws.send_json.assert_called_once() queue_ws.send_json.assert_called_once() system_ws.send_json.assert_called_once()