From 011f1149bf180c5a800c617dae1ff309337be715 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Nov 2025 04:05:03 +0000 Subject: [PATCH 1/6] test: add comprehensive tests to restore coverage above 70% MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit adds extensive test coverage for the Qdrant integration modules that were added in PR #24 but had 0% or low coverage, which caused overall coverage to drop from 80% to 55.72%. New test files added: - test_qdrant_repository.py: Extended existing tests with 40+ new test cases covering all repository methods (batch upload, scroll, update, delete, etc.) - test_qdrant_pool.py: Complete coverage of connection pooling functionality including pool management, acquire/release, cleanup, and error handling - test_qdrant_client.py: Tests for client connection manager and pooled client context manager - test_qdrant_collection.py: Tests for collection initialization, validation, and status management - test_qdrant_metadata.py: Tests for metadata handling utilities including payload creation, validation, merging, and filtering - test_qdrant_point.py: Tests for Qdrant point models including SearchResult, BatchUploadResult, and DeleteResult These tests cover approximately 525 previously untested statements across: - app/repositories/qdrant_repository.py (159 statements) - app/cache/qdrant_pool.py (143 statements) - app/cache/qdrant_metadata.py (69 statements) - app/cache/qdrant_collection.py (56 statements) - app/cache/qdrant_client.py (54 statements) - app/models/qdrant_point.py (20 statements) Expected coverage improvement: 55.72% → >70% --- tests/unit/cache/test_qdrant_client.py | 301 +++++++++++ tests/unit/cache/test_qdrant_collection.py | 251 ++++++++++ tests/unit/cache/test_qdrant_metadata.py | 263 ++++++++++ tests/unit/cache/test_qdrant_pool.py | 466 ++++++++++++++++++ tests/unit/models/test_qdrant_point.py | 376 ++++++++++++++ .../repositories/test_qdrant_repository.py | 410 +++++++++++++++ 6 files changed, 2067 insertions(+) create mode 100644 tests/unit/cache/test_qdrant_client.py create mode 100644 tests/unit/cache/test_qdrant_collection.py create mode 100644 tests/unit/cache/test_qdrant_metadata.py create mode 100644 tests/unit/cache/test_qdrant_pool.py create mode 100644 tests/unit/models/test_qdrant_point.py diff --git a/tests/unit/cache/test_qdrant_client.py b/tests/unit/cache/test_qdrant_client.py new file mode 100644 index 0000000..956bfc2 --- /dev/null +++ b/tests/unit/cache/test_qdrant_client.py @@ -0,0 +1,301 @@ +"""Unit tests for Qdrant client connection manager.""" + +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from app.cache.qdrant_client import ( + QdrantConnectionManager, + create_qdrant_client, + get_pooled_client, +) + + +class TestCreateQdrantClient: + """Tests for create_qdrant_client function.""" + + @pytest.mark.asyncio + async def test_create_qdrant_client_success(self): + """Test successful Qdrant client creation.""" + with patch("app.cache.qdrant_client.AsyncQdrantClient") as mock_client_class: + mock_client = AsyncMock() + mock_client.get_collections.return_value = MagicMock(collections=[]) + mock_client_class.return_value = mock_client + + with patch("app.cache.qdrant_client.config") as mock_config: + mock_config.qdrant_host = "localhost" + mock_config.qdrant_port = 6333 + + client = await create_qdrant_client() + + assert client is mock_client + mock_client.get_collections.assert_called_once() + + @pytest.mark.asyncio + async def test_create_qdrant_client_connection_failure(self): + """Test Qdrant client creation handles connection failure.""" + with patch("app.cache.qdrant_client.AsyncQdrantClient") as mock_client_class: + mock_client = AsyncMock() + mock_client.get_collections.side_effect = Exception("Connection refused") + mock_client_class.return_value = mock_client + + with patch("app.cache.qdrant_client.config") as mock_config: + mock_config.qdrant_host = "localhost" + mock_config.qdrant_port = 6333 + + with pytest.raises(ConnectionError, match="Failed to connect"): + await create_qdrant_client() + + @pytest.mark.asyncio + async def test_create_qdrant_client_uses_config(self): + """Test client creation uses config values.""" + with patch("app.cache.qdrant_client.AsyncQdrantClient") as mock_client_class: + mock_client = AsyncMock() + mock_client.get_collections.return_value = MagicMock(collections=[]) + mock_client_class.return_value = mock_client + + with patch("app.cache.qdrant_client.config") as mock_config: + mock_config.qdrant_host = "qdrant.example.com" + mock_config.qdrant_port = 9999 + + await create_qdrant_client() + + mock_client_class.assert_called_once_with( + host="qdrant.example.com", port=9999, timeout=30 + ) + + +class TestQdrantConnectionManager: + """Tests for QdrantConnectionManager class.""" + + @pytest.fixture + def manager(self): + """Create connection manager.""" + return QdrantConnectionManager() + + @pytest.mark.asyncio + async def test_manager_init(self, manager): + """Test manager initialization.""" + assert manager._client is None + + @pytest.mark.asyncio + async def test_get_client_creates_new(self, manager): + """Test get_client creates new client when none exists.""" + with patch( + "app.cache.qdrant_client.create_qdrant_client" + ) as mock_create_client: + mock_client = AsyncMock() + mock_create_client.return_value = mock_client + + client = await manager.get_client() + + assert client is mock_client + mock_create_client.assert_called_once() + + @pytest.mark.asyncio + async def test_get_client_reuses_existing(self, manager): + """Test get_client reuses existing client.""" + with patch( + "app.cache.qdrant_client.create_qdrant_client" + ) as mock_create_client: + mock_client = AsyncMock() + mock_create_client.return_value = mock_client + + client1 = await manager.get_client() + client2 = await manager.get_client() + + assert client1 is client2 + mock_create_client.assert_called_once() + + @pytest.mark.asyncio + async def test_get_client_raises_on_error(self, manager): + """Test get_client raises error on connection failure.""" + with patch( + "app.cache.qdrant_client.create_qdrant_client" + ) as mock_create_client: + mock_create_client.side_effect = ConnectionError("Connection failed") + + with pytest.raises(ConnectionError, match="Connection failed"): + await manager.get_client() + + @pytest.mark.asyncio + async def test_close_client(self, manager): + """Test closing client connection.""" + with patch( + "app.cache.qdrant_client.create_qdrant_client" + ) as mock_create_client: + mock_client = AsyncMock() + mock_create_client.return_value = mock_client + + await manager.get_client() + await manager.close() + + assert manager._client is None + mock_client.close.assert_called_once() + + @pytest.mark.asyncio + async def test_close_when_no_client(self, manager): + """Test closing when no client exists.""" + await manager.close() # Should not raise error + + @pytest.mark.asyncio + async def test_close_handles_error(self, manager): + """Test close handles errors gracefully.""" + with patch( + "app.cache.qdrant_client.create_qdrant_client" + ) as mock_create_client: + mock_client = AsyncMock() + mock_client.close.side_effect = Exception("Close failed") + mock_create_client.return_value = mock_client + + await manager.get_client() + await manager.close() + + # Client should be set to None even if close fails + assert manager._client is None + + @pytest.mark.asyncio + async def test_health_check_healthy(self, manager): + """Test health check when server is healthy.""" + with patch( + "app.cache.qdrant_client.create_qdrant_client" + ) as mock_create_client: + mock_client = AsyncMock() + mock_client.get_collections.return_value = MagicMock(collections=[]) + mock_create_client.return_value = mock_client + + is_healthy = await manager.health_check() + + assert is_healthy is True + + @pytest.mark.asyncio + async def test_health_check_unhealthy(self, manager): + """Test health check when server is unhealthy.""" + with patch( + "app.cache.qdrant_client.create_qdrant_client" + ) as mock_create_client: + mock_client = AsyncMock() + mock_client.get_collections.side_effect = Exception("Connection failed") + mock_create_client.return_value = mock_client + + is_healthy = await manager.health_check() + + assert is_healthy is False + + @pytest.mark.asyncio + async def test_reconnect_success(self, manager): + """Test successful reconnection.""" + with patch( + "app.cache.qdrant_client.create_qdrant_client" + ) as mock_create_client: + mock_client1 = AsyncMock() + mock_client2 = AsyncMock() + mock_create_client.side_effect = [mock_client1, mock_client2] + + # Initial connection + client1 = await manager.get_client() + assert client1 is mock_client1 + + # Reconnect + success = await manager.reconnect() + + assert success is True + assert manager._client is mock_client2 + mock_client1.close.assert_called_once() + + @pytest.mark.asyncio + async def test_reconnect_failure(self, manager): + """Test reconnection failure.""" + with patch( + "app.cache.qdrant_client.create_qdrant_client" + ) as mock_create_client: + mock_client = AsyncMock() + mock_create_client.side_effect = [ + mock_client, + ConnectionError("Connection failed"), + ] + + # Initial connection + await manager.get_client() + + # Reconnect fails + success = await manager.reconnect() + + assert success is False + mock_client.close.assert_called_once() + + @pytest.mark.asyncio + async def test_reconnect_close_error(self, manager): + """Test reconnection when close fails.""" + with patch( + "app.cache.qdrant_client.create_qdrant_client" + ) as mock_create_client: + mock_client1 = AsyncMock() + mock_client1.close.side_effect = Exception("Close failed") + mock_client2 = AsyncMock() + mock_create_client.side_effect = [mock_client1, mock_client2] + + # Initial connection + await manager.get_client() + + # Reconnect (should handle close error) + success = await manager.reconnect() + + assert success is True + assert manager._client is mock_client2 + + +class TestGetPooledClient: + """Tests for get_pooled_client context manager.""" + + @pytest.mark.asyncio + async def test_get_pooled_client_success(self): + """Test successful pooled client acquisition.""" + mock_client = AsyncMock() + mock_pool = AsyncMock() + mock_pool.acquire.return_value = mock_client + + with patch("app.cache.qdrant_client.get_pool") as mock_get_pool: + mock_get_pool.return_value = mock_pool + + async with get_pooled_client() as client: + assert client is mock_client + + mock_pool.acquire.assert_called_once() + mock_pool.release.assert_called_once_with(mock_client) + + @pytest.mark.asyncio + async def test_get_pooled_client_releases_on_error(self): + """Test pooled client is released even on error.""" + mock_client = AsyncMock() + mock_pool = AsyncMock() + mock_pool.acquire.return_value = mock_client + + with patch("app.cache.qdrant_client.get_pool") as mock_get_pool: + mock_get_pool.return_value = mock_pool + + with pytest.raises(ValueError, match="Test error"): + async with get_pooled_client() as client: + raise ValueError("Test error") + + mock_pool.release.assert_called_once_with(mock_client) + + @pytest.mark.asyncio + async def test_get_pooled_client_multiple_contexts(self): + """Test multiple pooled client contexts.""" + mock_client1 = AsyncMock() + mock_client2 = AsyncMock() + mock_pool = AsyncMock() + mock_pool.acquire.side_effect = [mock_client1, mock_client2] + + with patch("app.cache.qdrant_client.get_pool") as mock_get_pool: + mock_get_pool.return_value = mock_pool + + async with get_pooled_client() as client1: + assert client1 is mock_client1 + + async with get_pooled_client() as client2: + assert client2 is mock_client2 + + assert mock_pool.acquire.call_count == 2 + assert mock_pool.release.call_count == 2 diff --git a/tests/unit/cache/test_qdrant_collection.py b/tests/unit/cache/test_qdrant_collection.py new file mode 100644 index 0000000..3e83c40 --- /dev/null +++ b/tests/unit/cache/test_qdrant_collection.py @@ -0,0 +1,251 @@ +"""Unit tests for Qdrant collection manager.""" + +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest +from qdrant_client.models import Distance + +from app.cache.qdrant_collection import QdrantCollectionManager + + +class TestQdrantCollectionManager: + """Tests for QdrantCollectionManager class.""" + + @pytest.fixture + def mock_repository(self): + """Create mock repository.""" + return AsyncMock() + + @pytest.fixture + def manager(self, mock_repository): + """Create collection manager.""" + return QdrantCollectionManager(mock_repository) + + @pytest.mark.asyncio + async def test_manager_init(self, manager, mock_repository): + """Test manager initialization.""" + assert manager._repository is mock_repository + + @pytest.mark.asyncio + async def test_initialize_creates_new_collection(self, manager, mock_repository): + """Test initialize creates collection when it doesn't exist.""" + mock_repository.collection_exists.return_value = False + mock_repository.create_collection.return_value = True + + result = await manager.initialize() + + assert result is True + mock_repository.collection_exists.assert_called_once() + mock_repository.create_collection.assert_called_once_with(Distance.COSINE) + + @pytest.mark.asyncio + async def test_initialize_with_existing_collection(self, manager, mock_repository): + """Test initialize with existing collection.""" + mock_repository.collection_exists.return_value = True + + result = await manager.initialize() + + assert result is True + mock_repository.collection_exists.assert_called_once() + mock_repository.create_collection.assert_not_called() + + @pytest.mark.asyncio + async def test_initialize_with_recreate(self, manager, mock_repository): + """Test initialize with recreate flag.""" + mock_repository.collection_exists.return_value = True + mock_repository.delete_collection.return_value = True + mock_repository.create_collection.return_value = True + + result = await manager.initialize(recreate=True) + + assert result is True + mock_repository.delete_collection.assert_called_once() + mock_repository.create_collection.assert_called_once() + + @pytest.mark.asyncio + async def test_initialize_with_custom_distance(self, manager, mock_repository): + """Test initialize with custom distance metric.""" + mock_repository.collection_exists.return_value = False + mock_repository.create_collection.return_value = True + + result = await manager.initialize(distance=Distance.EUCLID) + + assert result is True + mock_repository.create_collection.assert_called_once_with(Distance.EUCLID) + + @pytest.mark.asyncio + async def test_initialize_handles_error(self, manager, mock_repository): + """Test initialize handles errors gracefully.""" + mock_repository.collection_exists.side_effect = Exception("Connection failed") + + result = await manager.initialize() + + assert result is False + + @pytest.mark.asyncio + async def test_recreate_deletes_existing_collection(self, manager, mock_repository): + """Test recreate deletes existing collection.""" + mock_repository.collection_exists.return_value = True + mock_repository.delete_collection.return_value = True + mock_repository.create_collection.return_value = True + + result = await manager._recreate_collection(Distance.COSINE) + + assert result is True + mock_repository.delete_collection.assert_called_once() + mock_repository.create_collection.assert_called_once() + + @pytest.mark.asyncio + async def test_recreate_skips_delete_when_not_exists( + self, manager, mock_repository + ): + """Test recreate skips delete when collection doesn't exist.""" + mock_repository.collection_exists.return_value = False + mock_repository.create_collection.return_value = True + + result = await manager._recreate_collection(Distance.COSINE) + + assert result is True + mock_repository.delete_collection.assert_not_called() + mock_repository.create_collection.assert_called_once() + + @pytest.mark.asyncio + async def test_ensure_collection_exists_creates_when_missing( + self, manager, mock_repository + ): + """Test ensure collection creates when missing.""" + mock_repository.collection_exists.return_value = False + mock_repository.create_collection.return_value = True + + result = await manager._ensure_collection_exists(Distance.COSINE) + + assert result is True + mock_repository.create_collection.assert_called_once() + + @pytest.mark.asyncio + async def test_ensure_collection_exists_verifies_when_present( + self, manager, mock_repository + ): + """Test ensure collection verifies when present.""" + mock_repository.collection_exists.return_value = True + + result = await manager._ensure_collection_exists(Distance.COSINE) + + assert result is True + mock_repository.create_collection.assert_not_called() + + @pytest.mark.asyncio + async def test_validate_collection_all_checks_pass(self, manager, mock_repository): + """Test validate collection when all checks pass.""" + mock_repository.collection_exists.return_value = True + mock_repository.ping.return_value = True + mock_repository.get_collection_info.return_value = {"status": "green"} + + result = await manager.validate_collection() + + assert result["exists"] is True + assert result["accessible"] is True + assert result["configured"] is True + + @pytest.mark.asyncio + async def test_validate_collection_not_exists(self, manager, mock_repository): + """Test validate collection when collection doesn't exist.""" + mock_repository.collection_exists.return_value = False + + result = await manager.validate_collection() + + assert result["exists"] is False + assert result["accessible"] is False + assert result["configured"] is False + + @pytest.mark.asyncio + async def test_validate_collection_not_accessible(self, manager, mock_repository): + """Test validate collection when not accessible.""" + mock_repository.collection_exists.return_value = True + mock_repository.ping.return_value = False + + result = await manager.validate_collection() + + assert result["exists"] is True + assert result["accessible"] is False + assert result["configured"] is False + + @pytest.mark.asyncio + async def test_validate_collection_not_configured(self, manager, mock_repository): + """Test validate collection when not properly configured.""" + mock_repository.collection_exists.return_value = True + mock_repository.ping.return_value = True + mock_repository.get_collection_info.return_value = None + + result = await manager.validate_collection() + + assert result["exists"] is True + assert result["accessible"] is True + assert result["configured"] is False + + @pytest.mark.asyncio + async def test_validate_collection_handles_error(self, manager, mock_repository): + """Test validate collection handles errors.""" + mock_repository.collection_exists.side_effect = Exception("Error") + + result = await manager.validate_collection() + + assert result["exists"] is False + assert result["accessible"] is False + assert result["configured"] is False + + @pytest.mark.asyncio + async def test_get_status_not_initialized(self, manager, mock_repository): + """Test get status when collection not initialized.""" + mock_repository.collection_exists.return_value = False + + status = await manager.get_status() + + assert status is not None + assert status["status"] == "not_initialized" + assert "message" in status + + @pytest.mark.asyncio + async def test_get_status_error_getting_info(self, manager, mock_repository): + """Test get status when error getting collection info.""" + mock_repository.collection_exists.return_value = True + mock_repository.ping.return_value = True + mock_repository.get_collection_info.return_value = None + + status = await manager.get_status() + + assert status is not None + assert status["status"] == "error" + assert "message" in status + + @pytest.mark.asyncio + async def test_get_status_ready(self, manager, mock_repository): + """Test get status when collection is ready.""" + mock_repository.collection_exists.return_value = True + mock_repository.ping.return_value = True + mock_repository.get_collection_info.return_value = { + "vectors_count": 100, + "points_count": 100, + "status": "green", + "config": {"vector_size": 384}, + } + + status = await manager.get_status() + + assert status is not None + assert status["status"] == "ready" + assert status["vectors_count"] == 100 + assert status["points_count"] == 100 + assert status["collection_status"] == "green" + assert "config" in status + + @pytest.mark.asyncio + async def test_get_status_handles_exception(self, manager, mock_repository): + """Test get status handles exceptions.""" + mock_repository.collection_exists.side_effect = Exception("Connection error") + + status = await manager.get_status() + + assert status is not None + assert status["status"] == "error" + assert "message" in status diff --git a/tests/unit/cache/test_qdrant_metadata.py b/tests/unit/cache/test_qdrant_metadata.py new file mode 100644 index 0000000..1c02720 --- /dev/null +++ b/tests/unit/cache/test_qdrant_metadata.py @@ -0,0 +1,263 @@ +"""Unit tests for Qdrant metadata handler.""" + +import time +from unittest.mock import patch + +import pytest + +from app.cache.qdrant_metadata import MetadataHandler +from app.models.cache_entry import CacheEntry +from app.models.qdrant_schema import QdrantSchema + + +class TestMetadataHandler: + """Tests for MetadataHandler class.""" + + @pytest.fixture + def cache_entry(self): + """Create sample cache entry.""" + return CacheEntry( + query_hash="abc123", + original_query="What is the weather?", + response="It's sunny today", + provider="openai", + model="gpt-4", + prompt_tokens=10, + completion_tokens=5, + embedding=[0.1, 0.2, 0.3], + ) + + @pytest.fixture + def valid_payload(self): + """Create valid payload.""" + return { + QdrantSchema.FIELD_QUERY_HASH: "abc123", + QdrantSchema.FIELD_ORIGINAL_QUERY: "What is the weather?", + QdrantSchema.FIELD_RESPONSE: "It's sunny today", + QdrantSchema.FIELD_PROVIDER: "openai", + QdrantSchema.FIELD_MODEL: "gpt-4", + QdrantSchema.FIELD_PROMPT_TOKENS: 10, + QdrantSchema.FIELD_COMPLETION_TOKENS: 5, + } + + def test_create_from_cache_entry(self, cache_entry): + """Test creating metadata from cache entry.""" + with patch("time.time", return_value=1234567890.0): + metadata = MetadataHandler.create_from_cache_entry(cache_entry) + + assert metadata[QdrantSchema.FIELD_QUERY_HASH] == "abc123" + assert metadata[QdrantSchema.FIELD_ORIGINAL_QUERY] == "What is the weather?" + assert metadata[QdrantSchema.FIELD_RESPONSE] == "It's sunny today" + assert metadata[QdrantSchema.FIELD_PROVIDER] == "openai" + assert metadata[QdrantSchema.FIELD_MODEL] == "gpt-4" + assert metadata[QdrantSchema.FIELD_PROMPT_TOKENS] == 10 + assert metadata[QdrantSchema.FIELD_COMPLETION_TOKENS] == 5 + assert metadata[QdrantSchema.FIELD_CREATED_AT] == 1234567890.0 + assert metadata[QdrantSchema.FIELD_CACHED_AT] == 1234567890.0 + + def test_validate_payload_valid(self, valid_payload): + """Test validating valid payload.""" + is_valid = MetadataHandler.validate_payload(valid_payload) + + assert is_valid is True + + def test_validate_payload_missing_field(self): + """Test validating payload with missing required field.""" + invalid_payload = { + QdrantSchema.FIELD_QUERY_HASH: "abc123", + # Missing other required fields + } + + is_valid = MetadataHandler.validate_payload(invalid_payload) + + assert is_valid is False + + def test_extract_cache_entry_success(self, valid_payload): + """Test extracting cache entry from valid payload.""" + entry = MetadataHandler.extract_cache_entry(valid_payload) + + assert entry is not None + assert entry.query_hash == "abc123" + assert entry.original_query == "What is the weather?" + assert entry.response == "It's sunny today" + assert entry.provider == "openai" + assert entry.model == "gpt-4" + assert entry.prompt_tokens == 10 + assert entry.completion_tokens == 5 + + def test_extract_cache_entry_with_defaults(self): + """Test extracting cache entry uses defaults for optional fields.""" + payload = { + QdrantSchema.FIELD_QUERY_HASH: "abc123", + QdrantSchema.FIELD_ORIGINAL_QUERY: "Query", + QdrantSchema.FIELD_RESPONSE: "Response", + QdrantSchema.FIELD_PROVIDER: "openai", + QdrantSchema.FIELD_MODEL: "gpt-4", + # Missing token fields - should default to 0 + } + + entry = MetadataHandler.extract_cache_entry(payload) + + assert entry is not None + assert entry.prompt_tokens == 0 + assert entry.completion_tokens == 0 + + def test_extract_cache_entry_missing_required_field(self): + """Test extracting cache entry with missing required field.""" + invalid_payload = { + QdrantSchema.FIELD_QUERY_HASH: "abc123", + # Missing other required fields + } + + entry = MetadataHandler.extract_cache_entry(invalid_payload) + + assert entry is None + + def test_extract_cache_entry_handles_exception(self): + """Test extracting cache entry handles exceptions.""" + invalid_payload = {"invalid": "data"} + + entry = MetadataHandler.extract_cache_entry(invalid_payload) + + assert entry is None + + def test_add_tags_to_empty_payload(self): + """Test adding tags to payload without existing tags.""" + payload = {} + tags = ["tag1", "tag2"] + + result = MetadataHandler.add_tags(payload, tags) + + assert QdrantSchema.FIELD_TAGS in result + assert set(result[QdrantSchema.FIELD_TAGS]) == {"tag1", "tag2"} + + def test_add_tags_to_existing_payload(self): + """Test adding tags to payload with existing tags.""" + payload = {QdrantSchema.FIELD_TAGS: ["tag1", "tag2"]} + tags = ["tag2", "tag3"] + + result = MetadataHandler.add_tags(payload, tags) + + assert set(result[QdrantSchema.FIELD_TAGS]) == {"tag1", "tag2", "tag3"} + + def test_add_metadata_to_empty_payload(self): + """Test adding metadata to payload without existing metadata.""" + payload = {} + metadata = {"key1": "value1", "key2": "value2"} + + result = MetadataHandler.add_metadata(payload, metadata) + + assert QdrantSchema.FIELD_METADATA in result + assert result[QdrantSchema.FIELD_METADATA] == metadata + + def test_add_metadata_to_existing_payload(self): + """Test adding metadata to payload with existing metadata.""" + payload = {QdrantSchema.FIELD_METADATA: {"key1": "old_value", "key2": "value2"}} + metadata = {"key1": "new_value", "key3": "value3"} + + result = MetadataHandler.add_metadata(payload, metadata) + + assert result[QdrantSchema.FIELD_METADATA]["key1"] == "new_value" + assert result[QdrantSchema.FIELD_METADATA]["key2"] == "value2" + assert result[QdrantSchema.FIELD_METADATA]["key3"] == "value3" + + def test_get_field_exists(self, valid_payload): + """Test getting existing field from payload.""" + value = MetadataHandler.get_field(valid_payload, QdrantSchema.FIELD_QUERY_HASH) + + assert value == "abc123" + + def test_get_field_not_exists(self, valid_payload): + """Test getting non-existing field from payload.""" + value = MetadataHandler.get_field(valid_payload, "nonexistent_field") + + assert value is None + + def test_has_field_exists(self, valid_payload): + """Test checking if field exists.""" + exists = MetadataHandler.has_field(valid_payload, QdrantSchema.FIELD_QUERY_HASH) + + assert exists is True + + def test_has_field_not_exists(self, valid_payload): + """Test checking if field doesn't exist.""" + exists = MetadataHandler.has_field(valid_payload, "nonexistent_field") + + assert exists is False + + def test_filter_sensitive_fields(self, valid_payload): + """Test filtering sensitive fields from payload.""" + filtered = MetadataHandler.filter_sensitive_fields(valid_payload) + + assert filtered[QdrantSchema.FIELD_RESPONSE] == "[REDACTED]" + assert filtered[QdrantSchema.FIELD_QUERY_HASH] == "abc123" + # Original should be unchanged + assert valid_payload[QdrantSchema.FIELD_RESPONSE] == "It's sunny today" + + def test_get_metadata_summary(self, valid_payload): + """Test getting metadata summary.""" + summary = MetadataHandler.get_metadata_summary(valid_payload) + + assert summary["query_hash"] == "abc123" + assert summary["provider"] == "openai" + assert summary["model"] == "gpt-4" + assert summary["prompt_tokens"] == 10 + assert summary["completion_tokens"] == 5 + assert summary["has_tags"] is False + assert summary["has_metadata"] is False + + def test_get_metadata_summary_with_tags_and_metadata(self): + """Test getting metadata summary with tags and metadata.""" + payload = { + QdrantSchema.FIELD_QUERY_HASH: "abc123", + QdrantSchema.FIELD_PROVIDER: "openai", + QdrantSchema.FIELD_MODEL: "gpt-4", + QdrantSchema.FIELD_TAGS: ["tag1"], + QdrantSchema.FIELD_METADATA: {"key": "value"}, + } + + summary = MetadataHandler.get_metadata_summary(payload) + + assert summary["has_tags"] is True + assert summary["has_metadata"] is True + + def test_merge_payloads_simple(self): + """Test merging two payloads.""" + base = {"field1": "value1", "field2": "value2"} + updates = {"field2": "new_value2", "field3": "value3"} + + merged = MetadataHandler.merge_payloads(base, updates) + + assert merged["field1"] == "value1" + assert merged["field2"] == "new_value2" + assert merged["field3"] == "value3" + + def test_merge_payloads_combines_tags(self): + """Test merging payloads combines tags.""" + base = {QdrantSchema.FIELD_TAGS: ["tag1", "tag2"]} + updates = {QdrantSchema.FIELD_TAGS: ["tag2", "tag3"]} + + merged = MetadataHandler.merge_payloads(base, updates) + + assert set(merged[QdrantSchema.FIELD_TAGS]) == {"tag1", "tag2", "tag3"} + + def test_merge_payloads_merges_metadata(self): + """Test merging payloads merges metadata dicts.""" + base = {QdrantSchema.FIELD_METADATA: {"key1": "value1", "key2": "value2"}} + updates = {QdrantSchema.FIELD_METADATA: {"key2": "new_value2", "key3": "value3"}} + + merged = MetadataHandler.merge_payloads(base, updates) + + assert merged[QdrantSchema.FIELD_METADATA]["key1"] == "value1" + assert merged[QdrantSchema.FIELD_METADATA]["key2"] == "new_value2" + assert merged[QdrantSchema.FIELD_METADATA]["key3"] == "value3" + + def test_merge_payloads_preserves_original(self): + """Test merging payloads doesn't modify originals.""" + base = {"field1": "value1"} + updates = {"field2": "value2"} + + merged = MetadataHandler.merge_payloads(base, updates) + + assert "field2" not in base + assert "field1" not in updates diff --git a/tests/unit/cache/test_qdrant_pool.py b/tests/unit/cache/test_qdrant_pool.py new file mode 100644 index 0000000..34132bc --- /dev/null +++ b/tests/unit/cache/test_qdrant_pool.py @@ -0,0 +1,466 @@ +"""Unit tests for Qdrant connection pool.""" + +import asyncio +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from app.cache.qdrant_errors import QdrantConnectionError +from app.cache.qdrant_pool import ( + PoolConfig, + PooledConnection, + QdrantConnectionPool, + close_pool, + get_pool, +) + + +class TestPoolConfig: + """Tests for PoolConfig class.""" + + def test_pool_config_defaults(self): + """Test pool config with default values.""" + config = PoolConfig() + + assert config.min_size == 1 + assert config.max_size == 10 + assert config.idle_timeout == 300.0 + assert config.max_lifetime == 3600.0 + assert config.acquire_timeout == 30.0 + + def test_pool_config_custom_values(self): + """Test pool config with custom values.""" + config = PoolConfig( + min_size=2, + max_size=20, + idle_timeout=600.0, + max_lifetime=7200.0, + acquire_timeout=60.0, + ) + + assert config.min_size == 2 + assert config.max_size == 20 + assert config.idle_timeout == 600.0 + assert config.max_lifetime == 7200.0 + assert config.acquire_timeout == 60.0 + + def test_pool_config_min_size_validation(self): + """Test pool config validates min_size >= 1.""" + config = PoolConfig(min_size=0, max_size=10) + + assert config.min_size == 1 + + def test_pool_config_max_size_validation(self): + """Test pool config validates max_size >= min_size.""" + config = PoolConfig(min_size=10, max_size=5) + + assert config.max_size == 10 + + +class TestPooledConnection: + """Tests for PooledConnection class.""" + + @pytest.fixture + def mock_client(self): + """Create mock Qdrant client.""" + return AsyncMock() + + @pytest.fixture + def pooled_conn(self, mock_client): + """Create pooled connection.""" + return PooledConnection(mock_client) + + def test_pooled_connection_init(self, pooled_conn, mock_client): + """Test pooled connection initialization.""" + assert pooled_conn.client is mock_client + assert pooled_conn.in_use is False + assert pooled_conn.use_count == 0 + assert pooled_conn.created_at > 0 + assert pooled_conn.last_used == pooled_conn.created_at + + def test_mark_used(self, pooled_conn): + """Test marking connection as used.""" + initial_count = pooled_conn.use_count + + pooled_conn.mark_used() + + assert pooled_conn.in_use is True + assert pooled_conn.use_count == initial_count + 1 + assert pooled_conn.last_used > pooled_conn.created_at + + def test_mark_released(self, pooled_conn): + """Test marking connection as released.""" + pooled_conn.mark_used() + pooled_conn.mark_released() + + assert pooled_conn.in_use is False + + def test_is_expired_true(self, pooled_conn): + """Test connection is expired when max lifetime exceeded.""" + # Mock time to make connection appear old + with patch("asyncio.get_event_loop") as mock_loop: + mock_loop.return_value.time.return_value = pooled_conn.created_at + 3700 + + is_expired = pooled_conn.is_expired(max_lifetime=3600.0) + + assert is_expired is True + + def test_is_expired_false(self, pooled_conn): + """Test connection is not expired when within lifetime.""" + is_expired = pooled_conn.is_expired(max_lifetime=3600.0) + + assert is_expired is False + + def test_is_idle_expired_true(self, pooled_conn): + """Test connection is idle expired.""" + # Mock time to make connection appear idle + with patch("asyncio.get_event_loop") as mock_loop: + mock_loop.return_value.time.return_value = pooled_conn.last_used + 400 + + is_idle = pooled_conn.is_idle_expired(idle_timeout=300.0) + + assert is_idle is True + + def test_is_idle_expired_false_within_timeout(self, pooled_conn): + """Test connection is not idle expired when within timeout.""" + is_idle = pooled_conn.is_idle_expired(idle_timeout=300.0) + + assert is_idle is False + + def test_is_idle_expired_false_in_use(self, pooled_conn): + """Test connection is not idle expired when in use.""" + pooled_conn.mark_used() + + with patch("asyncio.get_event_loop") as mock_loop: + mock_loop.return_value.time.return_value = pooled_conn.last_used + 400 + + is_idle = pooled_conn.is_idle_expired(idle_timeout=300.0) + + assert is_idle is False + + +class TestQdrantConnectionPool: + """Tests for QdrantConnectionPool class.""" + + @pytest.fixture + def pool_config(self): + """Create pool config for testing.""" + return PoolConfig(min_size=1, max_size=3, acquire_timeout=5.0) + + @pytest.fixture + async def pool(self, pool_config): + """Create connection pool.""" + pool = QdrantConnectionPool(pool_config) + yield pool + # Cleanup + try: + await pool.close() + except Exception: + pass + + @pytest.mark.asyncio + async def test_pool_initialize(self, pool_config): + """Test pool initialization.""" + with patch( + "app.cache.qdrant_pool.create_qdrant_client" + ) as mock_create_client: + mock_create_client.return_value = AsyncMock() + + pool = QdrantConnectionPool(pool_config) + await pool.initialize() + + stats = pool.get_stats() + assert stats["total"] >= pool_config.min_size + assert stats["min_size"] == pool_config.min_size + assert stats["max_size"] == pool_config.max_size + + await pool.close() + + @pytest.mark.asyncio + async def test_pool_initialize_when_closed(self, pool): + """Test initializing closed pool raises error.""" + with patch("app.cache.qdrant_pool.create_qdrant_client") as mock_create: + mock_create.return_value = AsyncMock() + + await pool.initialize() + await pool.close() + + with pytest.raises(QdrantConnectionError, match="Pool is closed"): + await pool.initialize() + + @pytest.mark.asyncio + async def test_pool_acquire_and_release(self, pool_config): + """Test acquiring and releasing connection.""" + with patch( + "app.cache.qdrant_pool.create_qdrant_client" + ) as mock_create_client: + mock_client = AsyncMock() + mock_create_client.return_value = mock_client + + pool = QdrantConnectionPool(pool_config) + await pool.initialize() + + # Acquire connection + client = await pool.acquire() + assert client is not None + + stats = pool.get_stats() + assert stats["in_use"] == 1 + + # Release connection + await pool.release(client) + + stats = pool.get_stats() + assert stats["in_use"] == 0 + + await pool.close() + + @pytest.mark.asyncio + async def test_pool_acquire_timeout(self, pool_config): + """Test acquire timeout when pool is full.""" + # Set very short timeout for testing + pool_config.acquire_timeout = 0.5 + pool_config.max_size = 1 + + with patch( + "app.cache.qdrant_pool.create_qdrant_client" + ) as mock_create_client: + mock_client = AsyncMock() + mock_create_client.return_value = mock_client + + pool = QdrantConnectionPool(pool_config) + await pool.initialize() + + # Acquire the only connection + client = await pool.acquire() + + # Try to acquire when pool is full - should timeout + with pytest.raises(QdrantConnectionError, match="Timeout acquiring"): + await pool.acquire() + + await pool.release(client) + await pool.close() + + @pytest.mark.asyncio + async def test_pool_acquire_when_closed(self, pool): + """Test acquiring from closed pool raises error.""" + with patch("app.cache.qdrant_pool.create_qdrant_client") as mock_create: + mock_create.return_value = AsyncMock() + + await pool.initialize() + await pool.close() + + with pytest.raises(QdrantConnectionError, match="Pool is closed"): + await pool.acquire() + + @pytest.mark.asyncio + async def test_pool_creates_new_connection_when_needed(self, pool_config): + """Test pool creates new connections up to max_size.""" + pool_config.min_size = 1 + pool_config.max_size = 2 + + with patch( + "app.cache.qdrant_pool.create_qdrant_client" + ) as mock_create_client: + mock_create_client.return_value = AsyncMock() + + pool = QdrantConnectionPool(pool_config) + await pool.initialize() + + # Acquire two connections + client1 = await pool.acquire() + client2 = await pool.acquire() + + stats = pool.get_stats() + assert stats["total"] == 2 + assert stats["in_use"] == 2 + + await pool.release(client1) + await pool.release(client2) + await pool.close() + + @pytest.mark.asyncio + async def test_pool_close(self, pool_config): + """Test closing pool.""" + with patch( + "app.cache.qdrant_pool.create_qdrant_client" + ) as mock_create_client: + mock_client = AsyncMock() + mock_create_client.return_value = mock_client + + pool = QdrantConnectionPool(pool_config) + await pool.initialize() + + await pool.close() + + # Verify client was closed + assert mock_client.close.called + + @pytest.mark.asyncio + async def test_pool_close_idempotent(self, pool): + """Test closing pool multiple times is safe.""" + with patch("app.cache.qdrant_pool.create_qdrant_client") as mock_create: + mock_create.return_value = AsyncMock() + + await pool.initialize() + await pool.close() + await pool.close() # Should not raise error + + @pytest.mark.asyncio + async def test_pool_cleanup_expired_connections(self, pool_config): + """Test cleanup of expired connections.""" + pool_config.max_lifetime = 0.1 # Very short lifetime + + with patch( + "app.cache.qdrant_pool.create_qdrant_client" + ) as mock_create_client: + mock_client = AsyncMock() + mock_create_client.return_value = mock_client + + pool = QdrantConnectionPool(pool_config) + await pool.initialize() + + # Acquire and release to mark connection + client = await pool.acquire() + await pool.release(client) + + # Wait for connection to expire + await asyncio.sleep(0.2) + + # Manually trigger cleanup + await pool._cleanup_expired() + + # Connection should be removed and new one created on next acquire + stats = pool.get_stats() + # After cleanup, expired connections are removed + assert stats["total"] >= 0 + + await pool.close() + + @pytest.mark.asyncio + async def test_pool_get_stats(self, pool_config): + """Test getting pool statistics.""" + with patch( + "app.cache.qdrant_pool.create_qdrant_client" + ) as mock_create_client: + mock_create_client.return_value = AsyncMock() + + pool = QdrantConnectionPool(pool_config) + await pool.initialize() + + stats = pool.get_stats() + + assert "total" in stats + assert "in_use" in stats + assert "available" in stats + assert "min_size" in stats + assert "max_size" in stats + assert stats["min_size"] == pool_config.min_size + assert stats["max_size"] == pool_config.max_size + + await pool.close() + + @pytest.mark.asyncio + async def test_pool_release_unknown_client(self, pool): + """Test releasing unknown client doesn't raise error.""" + with patch("app.cache.qdrant_pool.create_qdrant_client") as mock_create: + mock_create.return_value = AsyncMock() + + await pool.initialize() + + unknown_client = AsyncMock() + await pool.release(unknown_client) # Should not raise + + await pool.close() + + @pytest.mark.asyncio + async def test_pool_remove_connection_error_handling(self, pool_config): + """Test connection removal handles errors gracefully.""" + with patch( + "app.cache.qdrant_pool.create_qdrant_client" + ) as mock_create_client: + mock_client = AsyncMock() + mock_client.close.side_effect = Exception("Close failed") + mock_create_client.return_value = mock_client + + pool = QdrantConnectionPool(pool_config) + await pool.initialize() + + # Close should handle error gracefully + await pool.close() + + @pytest.mark.asyncio + async def test_pool_cleanup_loop_error_handling(self, pool_config): + """Test cleanup loop handles errors.""" + with patch( + "app.cache.qdrant_pool.create_qdrant_client" + ) as mock_create_client: + mock_create_client.return_value = AsyncMock() + + pool = QdrantConnectionPool(pool_config) + await pool.initialize() + + # Force error in cleanup + with patch.object(pool, "_cleanup_expired", side_effect=Exception("Error")): + # Wait briefly to let cleanup run + await asyncio.sleep(0.1) + + await pool.close() + + +class TestGlobalPool: + """Tests for global pool functions.""" + + @pytest.mark.asyncio + async def test_get_pool_creates_instance(self): + """Test get_pool creates and initializes pool.""" + with patch( + "app.cache.qdrant_pool.create_qdrant_client" + ) as mock_create_client: + mock_create_client.return_value = AsyncMock() + + pool = await get_pool() + + assert pool is not None + assert isinstance(pool, QdrantConnectionPool) + + await close_pool() + + @pytest.mark.asyncio + async def test_get_pool_returns_same_instance(self): + """Test get_pool returns same instance on multiple calls.""" + with patch( + "app.cache.qdrant_pool.create_qdrant_client" + ) as mock_create_client: + mock_create_client.return_value = AsyncMock() + + pool1 = await get_pool() + pool2 = await get_pool() + + assert pool1 is pool2 + + await close_pool() + + @pytest.mark.asyncio + async def test_close_pool_global(self): + """Test closing global pool.""" + with patch( + "app.cache.qdrant_pool.create_qdrant_client" + ) as mock_create_client: + mock_create_client.return_value = AsyncMock() + + pool = await get_pool() + assert pool is not None + + await close_pool() + + # Next get_pool should create new instance + new_pool = await get_pool() + assert new_pool is not pool + + await close_pool() + + @pytest.mark.asyncio + async def test_close_pool_when_none(self): + """Test closing pool when none exists doesn't raise error.""" + await close_pool() # Should not raise diff --git a/tests/unit/models/test_qdrant_point.py b/tests/unit/models/test_qdrant_point.py new file mode 100644 index 0000000..16da57b --- /dev/null +++ b/tests/unit/models/test_qdrant_point.py @@ -0,0 +1,376 @@ +"""Unit tests for Qdrant point models.""" + +import time +from datetime import datetime +from unittest.mock import patch + +import pytest +from qdrant_client.models import PointStruct + +from app.models.cache_entry import CacheEntry +from app.models.qdrant_point import ( + BatchUploadResult, + DeleteResult, + QdrantPoint, + SearchResult, +) + + +class TestQdrantPoint: + """Tests for QdrantPoint model.""" + + @pytest.fixture + def cache_entry(self): + """Create sample cache entry.""" + return CacheEntry( + query_hash="abc123", + original_query="What is the weather?", + response="It's sunny today", + provider="openai", + model="gpt-4", + prompt_tokens=10, + completion_tokens=5, + embedding=[0.1, 0.2, 0.3], + ) + + def test_qdrant_point_creation(self): + """Test creating QdrantPoint.""" + point = QdrantPoint( + id="test-123", vector=[0.1, 0.2, 0.3], payload={"key": "value"} + ) + + assert point.id == "test-123" + assert point.vector == [0.1, 0.2, 0.3] + assert point.payload == {"key": "value"} + + def test_qdrant_point_auto_id(self): + """Test QdrantPoint generates ID if not provided.""" + point = QdrantPoint(vector=[0.1, 0.2, 0.3]) + + assert point.id is not None + assert len(point.id) > 0 + + def test_from_cache_entry(self, cache_entry): + """Test creating point from cache entry.""" + embedding = [0.1, 0.2, 0.3] + + with patch("time.time", return_value=1234567890.0): + point = QdrantPoint.from_cache_entry(cache_entry, embedding) + + assert point.vector == embedding + assert point.payload["query_hash"] == "abc123" + assert point.payload["original_query"] == "What is the weather?" + assert point.payload["response"] == "It's sunny today" + assert point.payload["provider"] == "openai" + assert point.payload["model"] == "gpt-4" + assert point.payload["prompt_tokens"] == 10 + assert point.payload["completion_tokens"] == 5 + assert point.payload["cached_at"] == 1234567890.0 + assert "created_at" in point.payload + + def test_from_cache_entry_with_embedding_flag(self): + """Test from_cache_entry sets has_embedding flag.""" + entry = CacheEntry( + query_hash="abc123", + original_query="Query", + response="Response", + provider="openai", + model="gpt-4", + embedding=[0.1, 0.2], + ) + + point = QdrantPoint.from_cache_entry(entry, [0.1, 0.2]) + + assert point.payload["has_embedding"] is True + + def test_from_cache_entry_without_embedding(self): + """Test from_cache_entry when entry has no embedding.""" + entry = CacheEntry( + query_hash="abc123", + original_query="Query", + response="Response", + provider="openai", + model="gpt-4", + embedding=None, + ) + + point = QdrantPoint.from_cache_entry(entry, [0.1, 0.2]) + + assert "has_embedding" not in point.payload + + def test_to_qdrant_point(self): + """Test converting to Qdrant PointStruct.""" + point = QdrantPoint( + id="test-123", vector=[0.1, 0.2, 0.3], payload={"key": "value"} + ) + + qdrant_point = point.to_qdrant_point() + + assert isinstance(qdrant_point, PointStruct) + assert qdrant_point.id == "test-123" + assert qdrant_point.vector == [0.1, 0.2, 0.3] + assert qdrant_point.payload == {"key": "value"} + + def test_from_qdrant_point(self): + """Test creating from Qdrant point data.""" + point = QdrantPoint.from_qdrant_point( + point_id="test-123", + vector=[0.1, 0.2, 0.3], + payload={"query_hash": "abc123"}, + ) + + assert point.id == "test-123" + assert point.vector == [0.1, 0.2, 0.3] + assert point.payload["query_hash"] == "abc123" + + +class TestSearchResult: + """Tests for SearchResult model.""" + + @pytest.fixture + def search_result(self): + """Create sample search result.""" + return SearchResult( + point_id="test-123", + score=0.95, + vector=[0.1, 0.2, 0.3], + payload={ + "query_hash": "abc123", + "original_query": "What is the weather?", + "response": "It's sunny today", + "provider": "openai", + "model": "gpt-4", + "prompt_tokens": 10, + "completion_tokens": 5, + "created_at": 1234567890.0, + }, + ) + + def test_search_result_creation(self): + """Test creating SearchResult.""" + result = SearchResult( + point_id="test-123", score=0.95, payload={"query_hash": "abc123"} + ) + + assert result.point_id == "test-123" + assert result.score == 0.95 + assert result.payload["query_hash"] == "abc123" + + def test_query_hash_property(self, search_result): + """Test query_hash property.""" + assert search_result.query_hash == "abc123" + + def test_query_hash_property_missing(self): + """Test query_hash property when missing.""" + result = SearchResult(point_id="test-123", score=0.95, payload={}) + + assert result.query_hash is None + + def test_original_query_property(self, search_result): + """Test original_query property.""" + assert search_result.original_query == "What is the weather?" + + def test_original_query_property_missing(self): + """Test original_query property when missing.""" + result = SearchResult(point_id="test-123", score=0.95, payload={}) + + assert result.original_query is None + + def test_response_property(self, search_result): + """Test response property.""" + assert search_result.response == "It's sunny today" + + def test_response_property_missing(self): + """Test response property when missing.""" + result = SearchResult(point_id="test-123", score=0.95, payload={}) + + assert result.response is None + + def test_provider_property(self, search_result): + """Test provider property.""" + assert search_result.provider == "openai" + + def test_provider_property_missing(self): + """Test provider property when missing.""" + result = SearchResult(point_id="test-123", score=0.95, payload={}) + + assert result.provider is None + + def test_model_property(self, search_result): + """Test model property.""" + assert search_result.model == "gpt-4" + + def test_model_property_missing(self): + """Test model property when missing.""" + result = SearchResult(point_id="test-123", score=0.95, payload={}) + + assert result.model is None + + def test_to_cache_entry_success(self, search_result): + """Test converting to cache entry.""" + entry = search_result.to_cache_entry() + + assert entry is not None + assert entry.query_hash == "abc123" + assert entry.original_query == "What is the weather?" + assert entry.response == "It's sunny today" + assert entry.provider == "openai" + assert entry.model == "gpt-4" + assert entry.prompt_tokens == 10 + assert entry.completion_tokens == 5 + assert entry.embedding == [0.1, 0.2, 0.3] + + def test_to_cache_entry_with_timestamp(self): + """Test converting to cache entry preserves timestamp.""" + result = SearchResult( + point_id="test-123", + score=0.95, + payload={ + "query_hash": "abc123", + "original_query": "Query", + "response": "Response", + "provider": "openai", + "model": "gpt-4", + "created_at": 1234567890.0, + }, + ) + + entry = result.to_cache_entry() + + assert entry is not None + assert isinstance(entry.created_at, datetime) + + def test_to_cache_entry_missing_required_field(self): + """Test converting to cache entry with missing required field.""" + result = SearchResult( + point_id="test-123", + score=0.95, + payload={"query_hash": "abc123"}, # Missing other required fields + ) + + entry = result.to_cache_entry() + + assert entry is None + + def test_to_cache_entry_invalid_timestamp(self): + """Test converting to cache entry with invalid timestamp.""" + result = SearchResult( + point_id="test-123", + score=0.95, + payload={ + "query_hash": "abc123", + "original_query": "Query", + "response": "Response", + "provider": "openai", + "model": "gpt-4", + "created_at": "invalid", # Invalid timestamp + }, + ) + + entry = result.to_cache_entry() + + assert entry is None + + def test_to_cache_entry_defaults_tokens(self): + """Test converting to cache entry uses default token values.""" + result = SearchResult( + point_id="test-123", + score=0.95, + payload={ + "query_hash": "abc123", + "original_query": "Query", + "response": "Response", + "provider": "openai", + "model": "gpt-4", + # No token fields + }, + ) + + entry = result.to_cache_entry() + + assert entry is not None + assert entry.prompt_tokens == 0 + assert entry.completion_tokens == 0 + + +class TestBatchUploadResult: + """Tests for BatchUploadResult model.""" + + def test_batch_upload_result_creation(self): + """Test creating BatchUploadResult.""" + result = BatchUploadResult(total=10, successful=8, failed=2) + + assert result.total == 10 + assert result.successful == 8 + assert result.failed == 2 + + def test_success_rate_calculation(self): + """Test success rate calculation.""" + result = BatchUploadResult(total=10, successful=8, failed=2) + + assert result.success_rate == 0.8 + + def test_success_rate_zero_total(self): + """Test success rate with zero total.""" + result = BatchUploadResult(total=0, successful=0, failed=0) + + assert result.success_rate == 0.0 + + def test_success_rate_complete_success(self): + """Test success rate with complete success.""" + result = BatchUploadResult(total=10, successful=10, failed=0) + + assert result.success_rate == 1.0 + + def test_has_failures_true(self): + """Test has_failures when there are failures.""" + result = BatchUploadResult(total=10, successful=8, failed=2) + + assert result.has_failures is True + + def test_has_failures_false(self): + """Test has_failures when no failures.""" + result = BatchUploadResult(total=10, successful=10, failed=0) + + assert result.has_failures is False + + def test_batch_upload_result_with_details(self): + """Test BatchUploadResult with point IDs and errors.""" + result = BatchUploadResult( + total=10, + successful=8, + failed=2, + point_ids=["id1", "id2", "id3"], + errors=["Error 1", "Error 2"], + ) + + assert len(result.point_ids) == 3 + assert len(result.errors) == 2 + + +class TestDeleteResult: + """Tests for DeleteResult model.""" + + def test_delete_result_success(self): + """Test creating successful DeleteResult.""" + result = DeleteResult(deleted_count=5, success=True, message="Deleted 5 points") + + assert result.deleted_count == 5 + assert result.success is True + assert result.message == "Deleted 5 points" + + def test_delete_result_failure(self): + """Test creating failed DeleteResult.""" + result = DeleteResult(deleted_count=0, success=False, message="Delete failed") + + assert result.deleted_count == 0 + assert result.success is False + assert result.message == "Delete failed" + + def test_delete_result_without_message(self): + """Test DeleteResult without message.""" + result = DeleteResult(deleted_count=5, success=True) + + assert result.deleted_count == 5 + assert result.success is True + assert result.message is None diff --git a/tests/unit/repositories/test_qdrant_repository.py b/tests/unit/repositories/test_qdrant_repository.py index bd7b302..5592479 100644 --- a/tests/unit/repositories/test_qdrant_repository.py +++ b/tests/unit/repositories/test_qdrant_repository.py @@ -246,3 +246,413 @@ async def test_count_points_error(self, repository, mock_client): count = await repository.count_points() assert count == 0 + + @pytest.mark.asyncio + async def test_get_collection_info_success(self, repository, mock_client): + """Test successful collection info retrieval.""" + mock_info = MagicMock() + mock_info.vectors_count = 100 + mock_info.points_count = 100 + mock_info.status = "green" + mock_info.config.params.vectors = MagicMock() + mock_info.config.params.vectors.distance = Distance.COSINE + mock_client.get_collection.return_value = mock_info + + info = await repository.get_collection_info() + + assert info is not None + assert info["vectors_count"] == 100 + assert info["points_count"] == 100 + assert info["status"] == "green" + + @pytest.mark.asyncio + async def test_get_collection_info_error(self, repository, mock_client): + """Test collection info retrieval handles errors.""" + mock_client.get_collection.side_effect = Exception("Get failed") + + info = await repository.get_collection_info() + + assert info is None + + @pytest.mark.asyncio + async def test_point_exists_true(self, repository, mock_client): + """Test point_exists returns True when point exists.""" + mock_point = PointStruct(id="test-123", vector=[0.1], payload={}) + mock_client.retrieve.return_value = [mock_point] + + exists = await repository.point_exists("test-123") + + assert exists is True + + @pytest.mark.asyncio + async def test_point_exists_false(self, repository, mock_client): + """Test point_exists returns False when point doesn't exist.""" + mock_client.retrieve.return_value = [] + + exists = await repository.point_exists("nonexistent") + + assert exists is False + + @pytest.mark.asyncio + async def test_point_exists_error(self, repository, mock_client): + """Test point_exists handles errors.""" + mock_client.retrieve.side_effect = Exception("Retrieve failed") + + exists = await repository.point_exists("test-123") + + assert exists is False + + @pytest.mark.asyncio + async def test_search_similar_with_vectors(self, repository, mock_client): + """Test similarity search with vectors included.""" + query_vector = [0.1, 0.2, 0.3] + mock_scored = ScoredPoint( + id="test-123", + version=1, + score=0.95, + payload={"query_hash": "abc123"}, + vector=[0.1, 0.2, 0.3], + ) + mock_client.search.return_value = [mock_scored] + + results = await repository.search_similar_with_vectors(query_vector, limit=5) + + assert len(results) == 1 + assert results[0].vector is not None + assert results[0].vector == [0.1, 0.2, 0.3] + + @pytest.mark.asyncio + async def test_search_similar_with_threshold(self, repository, mock_client): + """Test similarity search with score threshold.""" + query_vector = [0.1, 0.2, 0.3] + mock_client.search.return_value = [] + + results = await repository.search_similar( + query_vector, limit=5, score_threshold=0.9 + ) + + assert results == [] + mock_client.search.assert_called_once() + + @pytest.mark.asyncio + async def test_batch_upload_success(self, repository, mock_client): + """Test successful batch upload.""" + points = [ + QdrantPoint( + id=f"test-{i}", + vector=[0.1 * i, 0.2 * i, 0.3 * i], + payload={"query_hash": f"hash{i}"}, + ) + for i in range(10) + ] + + result = await repository.batch_upload(points, batch_size=5) + + assert result.total == 10 + assert result.successful == 10 + assert result.failed == 0 + assert len(result.point_ids) == 10 + assert result.success_rate == 100.0 + + @pytest.mark.asyncio + async def test_batch_upload_empty(self, repository, mock_client): + """Test batch upload with empty list.""" + result = await repository.batch_upload([]) + + assert result.total == 0 + assert result.successful == 0 + assert result.failed == 0 + + @pytest.mark.asyncio + async def test_batch_upload_partial_failure(self, repository, mock_client): + """Test batch upload with partial failures.""" + points = [ + QdrantPoint( + id=f"test-{i}", + vector=[0.1 * i, 0.2 * i, 0.3 * i], + payload={"query_hash": f"hash{i}"}, + ) + for i in range(10) + ] + + # First batch succeeds, second fails + mock_client.upsert.side_effect = [None, Exception("Upload failed")] + + result = await repository.batch_upload(points, batch_size=5) + + assert result.total == 10 + assert result.successful == 5 + assert result.failed == 5 + assert len(result.errors) == 1 + + @pytest.mark.asyncio + async def test_batch_upload_with_retry_success(self, repository, mock_client): + """Test batch upload with retry succeeds.""" + points = [ + QdrantPoint( + id=f"test-{i}", vector=[0.1 * i], payload={"query_hash": f"hash{i}"} + ) + for i in range(5) + ] + + result = await repository.batch_upload_with_retry(points, batch_size=5) + + assert result.successful == 5 + assert result.failed == 0 + + @pytest.mark.asyncio + async def test_delete_points_empty(self, repository, mock_client): + """Test deleting empty list of points.""" + result = await repository.delete_points([]) + + assert result.success is True + assert result.deleted_count == 0 + mock_client.delete.assert_not_called() + + @pytest.mark.asyncio + async def test_delete_by_filter_success(self, repository, mock_client): + """Test successful deletion by filter.""" + from qdrant_client.models import FieldCondition, Filter, MatchValue + + filter_obj = Filter( + must=[FieldCondition(key="query_hash", match=MatchValue(value="abc123"))] + ) + + result = await repository.delete_by_filter(filter_obj) + + assert result.success is True + mock_client.delete.assert_called_once() + + @pytest.mark.asyncio + async def test_delete_by_query_hash_success(self, repository, mock_client): + """Test successful deletion by query hash.""" + result = await repository.delete_by_query_hash("abc123") + + assert result.success is True + mock_client.delete.assert_called_once() + + @pytest.mark.asyncio + async def test_update_point_payload_success(self, repository, mock_client): + """Test successful payload update.""" + payload = {"new_field": "new_value"} + + result = await repository.update_point_payload("test-123", payload) + + assert result is True + mock_client.set_payload.assert_called_once() + + @pytest.mark.asyncio + async def test_update_point_payload_error(self, repository, mock_client): + """Test payload update handles errors.""" + mock_client.set_payload.side_effect = Exception("Update failed") + + result = await repository.update_point_payload("test-123", {}) + + assert result is False + + @pytest.mark.asyncio + async def test_update_point_vector_success(self, repository, mock_client): + """Test successful vector update.""" + new_vector = [0.5, 0.6, 0.7] + + result = await repository.update_point_vector("test-123", new_vector) + + assert result is True + mock_client.upsert.assert_called_once() + + @pytest.mark.asyncio + async def test_update_point_vector_error(self, repository, mock_client): + """Test vector update handles errors.""" + mock_client.upsert.side_effect = Exception("Update failed") + + result = await repository.update_point_vector("test-123", [0.1, 0.2]) + + assert result is False + + @pytest.mark.asyncio + async def test_update_point_success(self, repository, mock_client): + """Test successful complete point update.""" + point = QdrantPoint( + id="test-123", vector=[0.1, 0.2, 0.3], payload={"updated": True} + ) + + result = await repository.update_point(point) + + assert result is True + mock_client.upsert.assert_called_once() + + @pytest.mark.asyncio + async def test_update_point_error(self, repository, mock_client): + """Test point update handles errors.""" + point = QdrantPoint(id="test-123", vector=[0.1, 0.2], payload={}) + mock_client.upsert.side_effect = Exception("Update failed") + + result = await repository.update_point(point) + + assert result is False + + @pytest.mark.asyncio + async def test_partial_update_payload_success(self, repository, mock_client): + """Test successful partial payload update.""" + updates = {"field1": "value1", "field2": "value2"} + + result = await repository.partial_update_payload("test-123", updates) + + assert result is True + mock_client.set_payload.assert_called_once() + + @pytest.mark.asyncio + async def test_partial_update_payload_error(self, repository, mock_client): + """Test partial payload update handles errors.""" + mock_client.set_payload.side_effect = Exception("Update failed") + + result = await repository.partial_update_payload("test-123", {"field": "value"}) + + assert result is False + + @pytest.mark.asyncio + async def test_delete_payload_fields_success(self, repository, mock_client): + """Test successful payload fields deletion.""" + fields = ["field1", "field2"] + + result = await repository.delete_payload_fields("test-123", fields) + + assert result is True + mock_client.delete_payload.assert_called_once() + + @pytest.mark.asyncio + async def test_delete_payload_fields_error(self, repository, mock_client): + """Test payload fields deletion handles errors.""" + mock_client.delete_payload.side_effect = Exception("Delete failed") + + result = await repository.delete_payload_fields("test-123", ["field1"]) + + assert result is False + + @pytest.mark.asyncio + async def test_scroll_points_success(self, repository, mock_client): + """Test successful scroll pagination.""" + mock_point = PointStruct( + id="test-123", vector=[0.1, 0.2], payload={"data": "test"} + ) + mock_client.scroll.return_value = ([mock_point], "next_offset") + + points, next_offset = await repository.scroll_points(limit=10) + + assert len(points) == 1 + assert next_offset == "next_offset" + assert points[0].id == "test-123" + + @pytest.mark.asyncio + async def test_scroll_points_no_vectors(self, repository, mock_client): + """Test scroll without vectors.""" + mock_point = PointStruct(id="test-123", vector=None, payload={"data": "test"}) + mock_client.scroll.return_value = ([mock_point], None) + + points, next_offset = await repository.scroll_points( + limit=10, with_vectors=False + ) + + assert len(points) == 1 + assert next_offset is None + + @pytest.mark.asyncio + async def test_scroll_points_error(self, repository, mock_client): + """Test scroll handles errors.""" + mock_client.scroll.side_effect = Exception("Scroll failed") + + points, next_offset = await repository.scroll_points() + + assert points == [] + assert next_offset is None + + @pytest.mark.asyncio + async def test_get_all_points_success(self, repository, mock_client): + """Test getting all points with pagination.""" + # Simulate two pages of results + mock_point1 = PointStruct(id="test-1", vector=[0.1], payload={}) + mock_point2 = PointStruct(id="test-2", vector=[0.2], payload={}) + + mock_client.scroll.side_effect = [ + ([mock_point1], "offset1"), + ([mock_point2], None), + ] + + all_points = await repository.get_all_points(batch_size=1) + + assert len(all_points) == 2 + assert all_points[0].id == "test-1" + assert all_points[1].id == "test-2" + + @pytest.mark.asyncio + async def test_get_all_points_error(self, repository, mock_client): + """Test get all points handles errors gracefully.""" + mock_client.scroll.side_effect = Exception("Scroll failed") + + all_points = await repository.get_all_points() + + assert all_points == [] + + @pytest.mark.asyncio + async def test_count_points_with_filter(self, repository, mock_client): + """Test counting points with filter.""" + from qdrant_client.models import FieldCondition, Filter, MatchValue + + filter_obj = Filter( + must=[FieldCondition(key="query_hash", match=MatchValue(value="abc123"))] + ) + mock_client.count.return_value = MagicMock(count=5) + + count = await repository.count_points(filter_condition=filter_obj) + + assert count == 5 + mock_client.count.assert_called_once() + + @pytest.mark.asyncio + async def test_delete_point_error(self, repository, mock_client): + """Test delete point handles errors.""" + mock_client.delete.side_effect = Exception("Delete failed") + + result = await repository.delete_point("test-123") + + assert result.success is False + assert result.deleted_count == 0 + + @pytest.mark.asyncio + async def test_delete_points_error(self, repository, mock_client): + """Test delete points handles errors.""" + mock_client.delete.side_effect = Exception("Delete failed") + + result = await repository.delete_points(["test-1", "test-2"]) + + assert result.success is False + assert result.deleted_count == 0 + + @pytest.mark.asyncio + async def test_delete_by_filter_error(self, repository, mock_client): + """Test delete by filter handles errors.""" + from qdrant_client.models import Filter + + mock_client.delete.side_effect = Exception("Delete failed") + + result = await repository.delete_by_filter(Filter()) + + assert result.success is False + + @pytest.mark.asyncio + async def test_create_collection_error(self, repository, mock_client): + """Test create collection handles errors.""" + mock_client.get_collections.side_effect = Exception("Check failed") + + result = await repository.create_collection() + + assert result is False + + @pytest.mark.asyncio + async def test_delete_collection_error(self, repository, mock_client): + """Test delete collection handles errors.""" + mock_client.delete_collection.side_effect = Exception("Delete failed") + + result = await repository.delete_collection() + + assert result is False From 063c07b9e841ef1f91c521f2727724a67f582d46 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Nov 2025 05:00:47 +0000 Subject: [PATCH 2/6] style: format test files with black Apply black formatting to test_qdrant_metadata.py and test_qdrant_pool.py to ensure consistent code style. --- tests/unit/cache/test_qdrant_metadata.py | 4 +- tests/unit/cache/test_qdrant_pool.py | 48 ++++++------------------ 2 files changed, 15 insertions(+), 37 deletions(-) diff --git a/tests/unit/cache/test_qdrant_metadata.py b/tests/unit/cache/test_qdrant_metadata.py index 1c02720..bc74b5e 100644 --- a/tests/unit/cache/test_qdrant_metadata.py +++ b/tests/unit/cache/test_qdrant_metadata.py @@ -244,7 +244,9 @@ def test_merge_payloads_combines_tags(self): def test_merge_payloads_merges_metadata(self): """Test merging payloads merges metadata dicts.""" base = {QdrantSchema.FIELD_METADATA: {"key1": "value1", "key2": "value2"}} - updates = {QdrantSchema.FIELD_METADATA: {"key2": "new_value2", "key3": "value3"}} + updates = { + QdrantSchema.FIELD_METADATA: {"key2": "new_value2", "key3": "value3"} + } merged = MetadataHandler.merge_payloads(base, updates) diff --git a/tests/unit/cache/test_qdrant_pool.py b/tests/unit/cache/test_qdrant_pool.py index 34132bc..08d6413 100644 --- a/tests/unit/cache/test_qdrant_pool.py +++ b/tests/unit/cache/test_qdrant_pool.py @@ -161,9 +161,7 @@ async def pool(self, pool_config): @pytest.mark.asyncio async def test_pool_initialize(self, pool_config): """Test pool initialization.""" - with patch( - "app.cache.qdrant_pool.create_qdrant_client" - ) as mock_create_client: + with patch("app.cache.qdrant_pool.create_qdrant_client") as mock_create_client: mock_create_client.return_value = AsyncMock() pool = QdrantConnectionPool(pool_config) @@ -191,9 +189,7 @@ async def test_pool_initialize_when_closed(self, pool): @pytest.mark.asyncio async def test_pool_acquire_and_release(self, pool_config): """Test acquiring and releasing connection.""" - with patch( - "app.cache.qdrant_pool.create_qdrant_client" - ) as mock_create_client: + with patch("app.cache.qdrant_pool.create_qdrant_client") as mock_create_client: mock_client = AsyncMock() mock_create_client.return_value = mock_client @@ -222,9 +218,7 @@ async def test_pool_acquire_timeout(self, pool_config): pool_config.acquire_timeout = 0.5 pool_config.max_size = 1 - with patch( - "app.cache.qdrant_pool.create_qdrant_client" - ) as mock_create_client: + with patch("app.cache.qdrant_pool.create_qdrant_client") as mock_create_client: mock_client = AsyncMock() mock_create_client.return_value = mock_client @@ -259,9 +253,7 @@ async def test_pool_creates_new_connection_when_needed(self, pool_config): pool_config.min_size = 1 pool_config.max_size = 2 - with patch( - "app.cache.qdrant_pool.create_qdrant_client" - ) as mock_create_client: + with patch("app.cache.qdrant_pool.create_qdrant_client") as mock_create_client: mock_create_client.return_value = AsyncMock() pool = QdrantConnectionPool(pool_config) @@ -282,9 +274,7 @@ async def test_pool_creates_new_connection_when_needed(self, pool_config): @pytest.mark.asyncio async def test_pool_close(self, pool_config): """Test closing pool.""" - with patch( - "app.cache.qdrant_pool.create_qdrant_client" - ) as mock_create_client: + with patch("app.cache.qdrant_pool.create_qdrant_client") as mock_create_client: mock_client = AsyncMock() mock_create_client.return_value = mock_client @@ -311,9 +301,7 @@ async def test_pool_cleanup_expired_connections(self, pool_config): """Test cleanup of expired connections.""" pool_config.max_lifetime = 0.1 # Very short lifetime - with patch( - "app.cache.qdrant_pool.create_qdrant_client" - ) as mock_create_client: + with patch("app.cache.qdrant_pool.create_qdrant_client") as mock_create_client: mock_client = AsyncMock() mock_create_client.return_value = mock_client @@ -340,9 +328,7 @@ async def test_pool_cleanup_expired_connections(self, pool_config): @pytest.mark.asyncio async def test_pool_get_stats(self, pool_config): """Test getting pool statistics.""" - with patch( - "app.cache.qdrant_pool.create_qdrant_client" - ) as mock_create_client: + with patch("app.cache.qdrant_pool.create_qdrant_client") as mock_create_client: mock_create_client.return_value = AsyncMock() pool = QdrantConnectionPool(pool_config) @@ -376,9 +362,7 @@ async def test_pool_release_unknown_client(self, pool): @pytest.mark.asyncio async def test_pool_remove_connection_error_handling(self, pool_config): """Test connection removal handles errors gracefully.""" - with patch( - "app.cache.qdrant_pool.create_qdrant_client" - ) as mock_create_client: + with patch("app.cache.qdrant_pool.create_qdrant_client") as mock_create_client: mock_client = AsyncMock() mock_client.close.side_effect = Exception("Close failed") mock_create_client.return_value = mock_client @@ -392,9 +376,7 @@ async def test_pool_remove_connection_error_handling(self, pool_config): @pytest.mark.asyncio async def test_pool_cleanup_loop_error_handling(self, pool_config): """Test cleanup loop handles errors.""" - with patch( - "app.cache.qdrant_pool.create_qdrant_client" - ) as mock_create_client: + with patch("app.cache.qdrant_pool.create_qdrant_client") as mock_create_client: mock_create_client.return_value = AsyncMock() pool = QdrantConnectionPool(pool_config) @@ -414,9 +396,7 @@ class TestGlobalPool: @pytest.mark.asyncio async def test_get_pool_creates_instance(self): """Test get_pool creates and initializes pool.""" - with patch( - "app.cache.qdrant_pool.create_qdrant_client" - ) as mock_create_client: + with patch("app.cache.qdrant_pool.create_qdrant_client") as mock_create_client: mock_create_client.return_value = AsyncMock() pool = await get_pool() @@ -429,9 +409,7 @@ async def test_get_pool_creates_instance(self): @pytest.mark.asyncio async def test_get_pool_returns_same_instance(self): """Test get_pool returns same instance on multiple calls.""" - with patch( - "app.cache.qdrant_pool.create_qdrant_client" - ) as mock_create_client: + with patch("app.cache.qdrant_pool.create_qdrant_client") as mock_create_client: mock_create_client.return_value = AsyncMock() pool1 = await get_pool() @@ -444,9 +422,7 @@ async def test_get_pool_returns_same_instance(self): @pytest.mark.asyncio async def test_close_pool_global(self): """Test closing global pool.""" - with patch( - "app.cache.qdrant_pool.create_qdrant_client" - ) as mock_create_client: + with patch("app.cache.qdrant_pool.create_qdrant_client") as mock_create_client: mock_create_client.return_value = AsyncMock() pool = await get_pool() From 28469304018c873fa2bec7b76eeda4594a2d2dd7 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Nov 2025 05:07:18 +0000 Subject: [PATCH 3/6] test: address review feedback from gemini-code-assist[bot] Fixed 4 issues identified in PR review: 1. Fixed success_rate assertion bug (high priority) - Changed assertion from 100.0 to 1.0 since success_rate returns a decimal between 0.0-1.0, not 0-100 - File: test_qdrant_repository.py::test_batch_upload_success 2. Improved weak assertion in pool cleanup test (medium priority) - Changed from `>= 0` (always passes) to `== 0` - Expired connections are removed even if below min_size when max_lifetime is exceeded - File: test_qdrant_pool.py::test_pool_cleanup_expired_connections 3. Added parameter verification to search test (medium priority) - Changed from assert_called_once() to assert_called_once_with() - Now verifies score_threshold and all other parameters - File: test_qdrant_repository.py::test_search_similar_with_threshold 4. Added parameter verification to count test (medium priority) - Changed from assert_called_once() to assert_called_once_with() - Now verifies count_filter parameter is passed correctly - File: test_qdrant_repository.py::test_count_points_with_filter All review comments from gemini-code-assist[bot] have been addressed. --- tests/unit/cache/test_qdrant_pool.py | 5 ++--- .../repositories/test_qdrant_repository.py | 18 +++++++++++++++--- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/tests/unit/cache/test_qdrant_pool.py b/tests/unit/cache/test_qdrant_pool.py index 08d6413..4cfe3b9 100644 --- a/tests/unit/cache/test_qdrant_pool.py +++ b/tests/unit/cache/test_qdrant_pool.py @@ -318,10 +318,9 @@ async def test_pool_cleanup_expired_connections(self, pool_config): # Manually trigger cleanup await pool._cleanup_expired() - # Connection should be removed and new one created on next acquire + # Connection should be removed (even if below min_size due to max_lifetime) stats = pool.get_stats() - # After cleanup, expired connections are removed - assert stats["total"] >= 0 + assert stats["total"] == 0 await pool.close() diff --git a/tests/unit/repositories/test_qdrant_repository.py b/tests/unit/repositories/test_qdrant_repository.py index 5592479..07ad61b 100644 --- a/tests/unit/repositories/test_qdrant_repository.py +++ b/tests/unit/repositories/test_qdrant_repository.py @@ -332,7 +332,15 @@ async def test_search_similar_with_threshold(self, repository, mock_client): ) assert results == [] - mock_client.search.assert_called_once() + mock_client.search.assert_called_once_with( + collection_name=repository._collection_name, + query_vector=query_vector, + limit=5, + score_threshold=0.9, + query_filter=None, + with_payload=True, + with_vectors=False, + ) @pytest.mark.asyncio async def test_batch_upload_success(self, repository, mock_client): @@ -352,7 +360,7 @@ async def test_batch_upload_success(self, repository, mock_client): assert result.successful == 10 assert result.failed == 0 assert len(result.point_ids) == 10 - assert result.success_rate == 100.0 + assert result.success_rate == 1.0 @pytest.mark.asyncio async def test_batch_upload_empty(self, repository, mock_client): @@ -606,7 +614,11 @@ async def test_count_points_with_filter(self, repository, mock_client): count = await repository.count_points(filter_condition=filter_obj) assert count == 5 - mock_client.count.assert_called_once() + mock_client.count.assert_called_once_with( + collection_name=repository._collection_name, + count_filter=filter_obj, + exact=True, + ) @pytest.mark.asyncio async def test_delete_point_error(self, repository, mock_client): From 7dc752be2ea1432193e7888be144e8ea3dabaf96 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Nov 2025 05:12:55 +0000 Subject: [PATCH 4/6] style: fix flake8 linting errors in test files Fixed all flake8 errors identified in CI: 1. test_qdrant_client.py:278 - Removed unused 'client' variable - Changed `async with get_pooled_client() as client:` to `async with get_pooled_client():` 2. test_qdrant_collection.py:3 - Removed unused imports - Removed `MagicMock` and `patch` from imports 3. test_qdrant_metadata.py:3 - Removed unused `time` import 4. test_qdrant_metadata.py:262 - Fixed unused 'merged' variable - Added assertions to verify merged result is correct 5. test_qdrant_pool.py:4 - Removed unused `MagicMock` import 6. test_qdrant_point.py:3 - Removed unused `time` import All tests now pass flake8 linting with max-line-length=88 and extend-ignore=E203,W503. --- tests/unit/cache/test_qdrant_client.py | 2 +- tests/unit/cache/test_qdrant_collection.py | 2 +- tests/unit/cache/test_qdrant_metadata.py | 3 ++- tests/unit/cache/test_qdrant_pool.py | 2 +- tests/unit/models/test_qdrant_point.py | 1 - 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/unit/cache/test_qdrant_client.py b/tests/unit/cache/test_qdrant_client.py index 956bfc2..930e1ce 100644 --- a/tests/unit/cache/test_qdrant_client.py +++ b/tests/unit/cache/test_qdrant_client.py @@ -275,7 +275,7 @@ async def test_get_pooled_client_releases_on_error(self): mock_get_pool.return_value = mock_pool with pytest.raises(ValueError, match="Test error"): - async with get_pooled_client() as client: + async with get_pooled_client(): raise ValueError("Test error") mock_pool.release.assert_called_once_with(mock_client) diff --git a/tests/unit/cache/test_qdrant_collection.py b/tests/unit/cache/test_qdrant_collection.py index 3e83c40..2c451e0 100644 --- a/tests/unit/cache/test_qdrant_collection.py +++ b/tests/unit/cache/test_qdrant_collection.py @@ -1,6 +1,6 @@ """Unit tests for Qdrant collection manager.""" -from unittest.mock import AsyncMock, MagicMock, patch +from unittest.mock import AsyncMock import pytest from qdrant_client.models import Distance diff --git a/tests/unit/cache/test_qdrant_metadata.py b/tests/unit/cache/test_qdrant_metadata.py index bc74b5e..fca23a7 100644 --- a/tests/unit/cache/test_qdrant_metadata.py +++ b/tests/unit/cache/test_qdrant_metadata.py @@ -1,6 +1,5 @@ """Unit tests for Qdrant metadata handler.""" -import time from unittest.mock import patch import pytest @@ -263,3 +262,5 @@ def test_merge_payloads_preserves_original(self): assert "field2" not in base assert "field1" not in updates + assert merged["field1"] == "value1" + assert merged["field2"] == "value2" diff --git a/tests/unit/cache/test_qdrant_pool.py b/tests/unit/cache/test_qdrant_pool.py index 4cfe3b9..cff1293 100644 --- a/tests/unit/cache/test_qdrant_pool.py +++ b/tests/unit/cache/test_qdrant_pool.py @@ -1,7 +1,7 @@ """Unit tests for Qdrant connection pool.""" import asyncio -from unittest.mock import AsyncMock, MagicMock, patch +from unittest.mock import AsyncMock, patch import pytest diff --git a/tests/unit/models/test_qdrant_point.py b/tests/unit/models/test_qdrant_point.py index 16da57b..6681c51 100644 --- a/tests/unit/models/test_qdrant_point.py +++ b/tests/unit/models/test_qdrant_point.py @@ -1,6 +1,5 @@ """Unit tests for Qdrant point models.""" -import time from datetime import datetime from unittest.mock import patch From bf1cd84c2f59628800560ca246b1624b84e3b95e Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Nov 2025 05:26:54 +0000 Subject: [PATCH 5/6] test: fix 11 failing tests from CI pipeline Fixed all test failures identified in CI run: 1. Fixed get_pooled_client tests (3 failures) - Changed patch path from 'app.cache.qdrant_client.get_pool' to 'app.cache.qdrant_pool.get_pool' since get_pool is imported from qdrant_pool module inside the function 2. Fixed test_from_cache_entry tests (2 failures) - Added missing prompt_tokens and completion_tokens fields - CacheEntry requires these fields, tests were incomplete 3. Fixed DeleteResult validation (2 failures) - Changed deleted_count validation from ge=0 to ge=-1 - Repository uses -1 to indicate "unknown count" for filter-based deletions where Qdrant doesn't return count 4. Fixed test_scroll_points_no_vectors (1 failure) - Changed PointStruct vector from None to [] - Qdrant's PointStruct validation requires a vector, can't be None 5. Fixed test_get_status_handles_exception (1 failure) - Changed expected status from 'error' to 'not_initialized' - validate_collection() catches exceptions and returns exists=False - get_status() then returns 'not_initialized' not 'error' 6. Fixed test_to_cache_entry_invalid_timestamp (1 failure) - Added TypeError to caught exceptions in to_cache_entry() - datetime.fromtimestamp("invalid") raises TypeError not ValueError 7. Fixed test_create_collection_error (1 failure) - Mock create_collection to fail, not just get_collections - collection_exists returns False on error, then create_collection is attempted and needs to be mocked to fail All tests now pass. Coverage: 69.74% (0.26% short of 70% target). --- app/models/qdrant_point.py | 6 +++--- tests/unit/cache/test_qdrant_client.py | 6 +++--- tests/unit/cache/test_qdrant_collection.py | 2 +- tests/unit/models/test_qdrant_point.py | 4 ++++ tests/unit/repositories/test_qdrant_repository.py | 5 +++-- 5 files changed, 14 insertions(+), 9 deletions(-) diff --git a/app/models/qdrant_point.py b/app/models/qdrant_point.py index 2d8ea56..10b4f9e 100644 --- a/app/models/qdrant_point.py +++ b/app/models/qdrant_point.py @@ -152,10 +152,10 @@ def to_cache_entry(self) -> Optional[CacheEntry]: ) return CacheEntry(**kwargs) - except (KeyError, ValidationError, ValueError): + except (KeyError, ValidationError, ValueError, TypeError): # KeyError: missing required field # ValidationError: pydantic validation failed - # ValueError: invalid timestamp + # ValueError/TypeError: invalid timestamp return None @@ -192,6 +192,6 @@ class DeleteResult(BaseModel): Tracks deletion status. """ - deleted_count: int = Field(..., ge=0, description="Number deleted") + deleted_count: int = Field(..., ge=-1, description="Number deleted (-1 for unknown)") success: bool = Field(..., description="Operation success") message: Optional[str] = Field(None, description="Status message") diff --git a/tests/unit/cache/test_qdrant_client.py b/tests/unit/cache/test_qdrant_client.py index 930e1ce..931254c 100644 --- a/tests/unit/cache/test_qdrant_client.py +++ b/tests/unit/cache/test_qdrant_client.py @@ -255,7 +255,7 @@ async def test_get_pooled_client_success(self): mock_pool = AsyncMock() mock_pool.acquire.return_value = mock_client - with patch("app.cache.qdrant_client.get_pool") as mock_get_pool: + with patch("app.cache.qdrant_pool.get_pool") as mock_get_pool: mock_get_pool.return_value = mock_pool async with get_pooled_client() as client: @@ -271,7 +271,7 @@ async def test_get_pooled_client_releases_on_error(self): mock_pool = AsyncMock() mock_pool.acquire.return_value = mock_client - with patch("app.cache.qdrant_client.get_pool") as mock_get_pool: + with patch("app.cache.qdrant_pool.get_pool") as mock_get_pool: mock_get_pool.return_value = mock_pool with pytest.raises(ValueError, match="Test error"): @@ -288,7 +288,7 @@ async def test_get_pooled_client_multiple_contexts(self): mock_pool = AsyncMock() mock_pool.acquire.side_effect = [mock_client1, mock_client2] - with patch("app.cache.qdrant_client.get_pool") as mock_get_pool: + with patch("app.cache.qdrant_pool.get_pool") as mock_get_pool: mock_get_pool.return_value = mock_pool async with get_pooled_client() as client1: diff --git a/tests/unit/cache/test_qdrant_collection.py b/tests/unit/cache/test_qdrant_collection.py index 2c451e0..bdd49ef 100644 --- a/tests/unit/cache/test_qdrant_collection.py +++ b/tests/unit/cache/test_qdrant_collection.py @@ -247,5 +247,5 @@ async def test_get_status_handles_exception(self, manager, mock_repository): status = await manager.get_status() assert status is not None - assert status["status"] == "error" + assert status["status"] == "not_initialized" assert "message" in status diff --git a/tests/unit/models/test_qdrant_point.py b/tests/unit/models/test_qdrant_point.py index 6681c51..3a27e62 100644 --- a/tests/unit/models/test_qdrant_point.py +++ b/tests/unit/models/test_qdrant_point.py @@ -75,6 +75,8 @@ def test_from_cache_entry_with_embedding_flag(self): response="Response", provider="openai", model="gpt-4", + prompt_tokens=10, + completion_tokens=5, embedding=[0.1, 0.2], ) @@ -90,6 +92,8 @@ def test_from_cache_entry_without_embedding(self): response="Response", provider="openai", model="gpt-4", + prompt_tokens=10, + completion_tokens=5, embedding=None, ) diff --git a/tests/unit/repositories/test_qdrant_repository.py b/tests/unit/repositories/test_qdrant_repository.py index 07ad61b..9b97441 100644 --- a/tests/unit/repositories/test_qdrant_repository.py +++ b/tests/unit/repositories/test_qdrant_repository.py @@ -554,7 +554,7 @@ async def test_scroll_points_success(self, repository, mock_client): @pytest.mark.asyncio async def test_scroll_points_no_vectors(self, repository, mock_client): """Test scroll without vectors.""" - mock_point = PointStruct(id="test-123", vector=None, payload={"data": "test"}) + mock_point = PointStruct(id="test-123", vector=[], payload={"data": "test"}) mock_client.scroll.return_value = ([mock_point], None) points, next_offset = await repository.scroll_points( @@ -654,7 +654,8 @@ async def test_delete_by_filter_error(self, repository, mock_client): @pytest.mark.asyncio async def test_create_collection_error(self, repository, mock_client): """Test create collection handles errors.""" - mock_client.get_collections.side_effect = Exception("Check failed") + mock_client.get_collections.return_value = MagicMock(collections=[]) + mock_client.create_collection.side_effect = Exception("Create failed") result = await repository.create_collection() From feb348f3a51e2c19f6c2a86e0d1146e1b2061d7f Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Nov 2025 05:33:40 +0000 Subject: [PATCH 6/6] style: format qdrant_point.py with black Apply black formatting to fix CI formatting check. --- app/models/qdrant_point.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/models/qdrant_point.py b/app/models/qdrant_point.py index 10b4f9e..d30c027 100644 --- a/app/models/qdrant_point.py +++ b/app/models/qdrant_point.py @@ -192,6 +192,8 @@ class DeleteResult(BaseModel): Tracks deletion status. """ - deleted_count: int = Field(..., ge=-1, description="Number deleted (-1 for unknown)") + deleted_count: int = Field( + ..., ge=-1, description="Number deleted (-1 for unknown)" + ) success: bool = Field(..., description="Operation success") message: Optional[str] = Field(None, description="Status message")