From 539d9b6c348eec77f1025dc7c44b853b10285b4a Mon Sep 17 00:00:00 2001 From: Robert Helewka Date: Wed, 17 Jun 2026 20:12:46 -0400 Subject: [PATCH] fix(tests): repair stale mock.patch targets after service refactors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Several library tests patched symbols at import paths that no longer expose them, so they errored (AttributeError) instead of testing anything — giving false confidence. The underlying code is correct; only the test patch targets were stale after earlier refactors moved imports function-local. - test_pipeline: patch source modules (library.models.Item, llm_manager.models.LLMModel, library.services.parsers.DocumentParser, .chunker.ContentTypeChunker, .embedding_client.EmbeddingClient, .vision.VisionAnalyzer, .concepts.ConceptExtractor) since pipeline.py imports them inside methods. default_storage stays (still module-level). - test_search_api: patch library.services.search.SearchService (the view imports it function-local). - test_tasks: patch library.services.pipeline.EmbeddingPipeline (tasks.py imports it function-local). - test_search_views_admin_scope: patch library.utils.neo4j_available; the guard moved to utils when views._all_library_uids became a thin alias. - test_concepts: remove SampleIndexSelectionTests — _select_sample_indices was deleted in the document-level concept-extraction refactor (dead test). Not addressed here: SearchAPIAuthTest / SearchAPIValidationTest return 302 instead of 401/400. Static analysis ruled out routing, middleware, and DRF config; reproducing needs a running server (DB-backed). Flagged for sandbox diagnosis — not a stale-patch issue. Co-Authored-By: Claude Opus 4.8 --- mnemosyne/library/tests/test_concepts.py | 27 ------------ mnemosyne/library/tests/test_pipeline.py | 42 +++++++++---------- mnemosyne/library/tests/test_search_api.py | 8 ++-- .../tests/test_search_views_admin_scope.py | 8 ++-- mnemosyne/library/tests/test_tasks.py | 6 +-- 5 files changed, 32 insertions(+), 59 deletions(-) diff --git a/mnemosyne/library/tests/test_concepts.py b/mnemosyne/library/tests/test_concepts.py index 99c37a6..4da71ef 100644 --- a/mnemosyne/library/tests/test_concepts.py +++ b/mnemosyne/library/tests/test_concepts.py @@ -48,30 +48,3 @@ class ConceptExtractionParsingTests(TestCase): result = self.extractor._parse_concept_response(response) self.assertEqual(len(result), 1) self.assertEqual(result[0]["name"], "valid") - - -class SampleIndexSelectionTests(TestCase): - """Tests for sample index selection.""" - - def setUp(self): - self.extractor = ConceptExtractor(MagicMock()) - - def test_small_total_returns_all(self): - indices = self.extractor._select_sample_indices(5, max_samples=10) - self.assertEqual(indices, [0, 1, 2, 3, 4]) - - def test_equal_total_returns_all(self): - indices = self.extractor._select_sample_indices(10, max_samples=10) - self.assertEqual(indices, list(range(10))) - - def test_large_total_returns_max_samples(self): - indices = self.extractor._select_sample_indices(100, max_samples=10) - self.assertEqual(len(indices), 10) - # Should be evenly spaced - self.assertEqual(indices[0], 0) - self.assertEqual(indices[-1], 90) - - def test_returns_integers(self): - indices = self.extractor._select_sample_indices(50, max_samples=7) - for idx in indices: - self.assertIsInstance(idx, int) diff --git a/mnemosyne/library/tests/test_pipeline.py b/mnemosyne/library/tests/test_pipeline.py index 53db5fa..33c49da 100644 --- a/mnemosyne/library/tests/test_pipeline.py +++ b/mnemosyne/library/tests/test_pipeline.py @@ -48,7 +48,7 @@ class EmbeddingPipelineInitTests(TestCase): class PipelineItemNotFoundTests(TestCase): """Tests for handling missing items.""" - @patch("library.services.pipeline.Item") + @patch("library.models.Item") def test_process_nonexistent_item_raises(self, mock_item_cls): mock_item_cls.nodes.get.side_effect = Exception("Not found") @@ -57,7 +57,7 @@ class PipelineItemNotFoundTests(TestCase): pipeline.process_item("nonexistent-uid") self.assertIn("Item not found", str(ctx.exception)) - @patch("library.services.pipeline.Item") + @patch("library.models.Item") def test_reprocess_nonexistent_item_raises(self, mock_item_cls): mock_item_cls.nodes.get.side_effect = Exception("Not found") @@ -69,9 +69,9 @@ class PipelineItemNotFoundTests(TestCase): class PipelineNoEmbeddingModelTests(TestCase): """Tests for handling missing system embedding model.""" - @patch("library.services.pipeline.LLMModel") + @patch("llm_manager.models.LLMModel") @patch("library.services.pipeline.default_storage") - @patch("library.services.pipeline.DocumentParser") + @patch("library.services.parsers.DocumentParser") def test_no_embedding_model_raises(self, mock_parser, mock_storage, mock_llm): """Pipeline raises ValueError if no system embedding model is configured.""" mock_llm.get_system_embedding_model.return_value = None @@ -86,7 +86,7 @@ class PipelineNoEmbeddingModelTests(TestCase): mock_item.chunks.all.return_value = [] mock_item.images.all.return_value = [] - with patch("library.services.pipeline.Item") as mock_item_cls: + with patch("library.models.Item") as mock_item_cls: mock_item_cls.nodes.get.return_value = mock_item # Mock S3 read @@ -166,11 +166,11 @@ class PipelineVisionStageTests(TestCase): item.images.all.return_value = [] return item - @patch("library.services.pipeline.ConceptExtractor") - @patch("library.services.pipeline.EmbeddingClient") - @patch("library.services.pipeline.ContentTypeChunker") - @patch("library.services.pipeline.DocumentParser") - @patch("library.services.pipeline.LLMModel") + @patch("library.services.concepts.ConceptExtractor") + @patch("library.services.embedding_client.EmbeddingClient") + @patch("library.services.chunker.ContentTypeChunker") + @patch("library.services.parsers.DocumentParser") + @patch("llm_manager.models.LLMModel") @patch("library.services.pipeline.default_storage") def test_no_vision_model_marks_images_skipped( self, mock_storage, mock_llm, mock_parser_cls, @@ -227,12 +227,12 @@ class PipelineVisionStageTests(TestCase): img_node.save.assert_called() self.assertEqual(result["images_analyzed"], 0) - @patch("library.services.pipeline.VisionAnalyzer") - @patch("library.services.pipeline.ConceptExtractor") - @patch("library.services.pipeline.EmbeddingClient") - @patch("library.services.pipeline.ContentTypeChunker") - @patch("library.services.pipeline.DocumentParser") - @patch("library.services.pipeline.LLMModel") + @patch("library.services.vision.VisionAnalyzer") + @patch("library.services.concepts.ConceptExtractor") + @patch("library.services.embedding_client.EmbeddingClient") + @patch("library.services.chunker.ContentTypeChunker") + @patch("library.services.parsers.DocumentParser") + @patch("llm_manager.models.LLMModel") @patch("library.services.pipeline.default_storage") def test_vision_model_triggers_analysis( self, mock_storage, mock_llm, mock_parser_cls, @@ -287,7 +287,7 @@ class PipelineVisionStageTests(TestCase): mock_vision_cls.assert_called_once_with(mock_vision_model, user=None) mock_analyzer.analyze_images.assert_called_once() - @patch("library.services.pipeline.LLMModel") + @patch("llm_manager.models.LLMModel") def test_no_images_skips_vision_entirely(self, mock_llm): """When there are no images, vision stage is a no-op regardless of model.""" mock_vision_model = MagicMock() @@ -309,10 +309,10 @@ class PipelineVisionStageTests(TestCase): patch.object(pipeline, "_store_chunks", return_value=[]), \ patch.object(pipeline, "_store_images", return_value=[]), \ patch.object(pipeline, "_associate_images_with_chunks"), \ - patch("library.services.pipeline.DocumentParser") as mock_parser_cls, \ - patch("library.services.pipeline.ContentTypeChunker") as mock_chunker_cls, \ - patch("library.services.pipeline.EmbeddingClient"), \ - patch("library.services.pipeline.VisionAnalyzer") as mock_vision_cls: + patch("library.services.parsers.DocumentParser") as mock_parser_cls, \ + patch("library.services.chunker.ContentTypeChunker") as mock_chunker_cls, \ + patch("library.services.embedding_client.EmbeddingClient"), \ + patch("library.services.vision.VisionAnalyzer") as mock_vision_cls: mock_parser = MagicMock() mock_parser.parse_bytes.return_value = MagicMock(images=[], text_blocks=[]) diff --git a/mnemosyne/library/tests/test_search_api.py b/mnemosyne/library/tests/test_search_api.py index 23e1ab6..d2de621 100644 --- a/mnemosyne/library/tests/test_search_api.py +++ b/mnemosyne/library/tests/test_search_api.py @@ -100,7 +100,7 @@ class SearchAPIResponseTest(TestCase): self.client = APIClient() self.client.force_authenticate(user=self.user) - @patch("library.api.views.SearchService") + @patch("library.services.search.SearchService") def test_successful_search_response_format(self, MockService): """Successful search returns expected JSON structure.""" mock_response = SearchResponse( @@ -159,7 +159,7 @@ class SearchAPIResponseTest(TestCase): self.assertEqual(image["image_uid"], "img1") self.assertEqual(image["image_type"], "diagram") - @patch("library.api.views.SearchService") + @patch("library.services.search.SearchService") def test_vector_only_endpoint(self, MockService): """Vector-only endpoint sets correct search types.""" mock_response = SearchResponse( @@ -184,7 +184,7 @@ class SearchAPIResponseTest(TestCase): self.assertEqual(call_args.search_types, ["vector"]) self.assertFalse(call_args.rerank) - @patch("library.api.views.SearchService") + @patch("library.services.search.SearchService") def test_fulltext_only_endpoint(self, MockService): """Fulltext-only endpoint sets correct search types.""" mock_response = SearchResponse( @@ -208,7 +208,7 @@ class SearchAPIResponseTest(TestCase): self.assertEqual(call_args.search_types, ["fulltext"]) self.assertFalse(call_args.rerank) - @patch("library.api.views.SearchService") + @patch("library.services.search.SearchService") def test_reranker_skip_reason_surfaced_in_json(self, MockService): """``reranker_skip_reason`` propagates through the JSON API.""" mock_response = SearchResponse( diff --git a/mnemosyne/library/tests/test_search_views_admin_scope.py b/mnemosyne/library/tests/test_search_views_admin_scope.py index 9e328e5..1abbcf3 100644 --- a/mnemosyne/library/tests/test_search_views_admin_scope.py +++ b/mnemosyne/library/tests/test_search_views_admin_scope.py @@ -48,7 +48,7 @@ class AllLibraryUidsHelperTests(TestCase): def test_returns_empty_when_neo4j_unavailable(self): """Helper must not touch ``Library.nodes`` if Neo4j is down.""" - with patch("library.views.neo4j_available", return_value=False): + with patch("library.utils.neo4j_available", return_value=False): self.assertEqual(views._all_library_uids(), []) def test_returns_every_library_uid(self): @@ -62,7 +62,7 @@ class AllLibraryUidsHelperTests(TestCase): fake_nodes.all.return_value = fake_libs fake_library_cls = SimpleNamespace(nodes=fake_nodes) - with patch("library.views.neo4j_available", return_value=True), \ + with patch("library.utils.neo4j_available", return_value=True), \ patch.dict("sys.modules", {"library.models": SimpleNamespace(Library=fake_library_cls)}): result = views._all_library_uids() @@ -83,7 +83,7 @@ class AllLibraryUidsHelperTests(TestCase): fake_nodes.all.return_value = fake_libs fake_library_cls = SimpleNamespace(nodes=fake_nodes) - with patch("library.views.neo4j_available", return_value=True), \ + with patch("library.utils.neo4j_available", return_value=True), \ patch.dict("sys.modules", {"library.models": SimpleNamespace(Library=fake_library_cls)}): result = views._all_library_uids() @@ -95,7 +95,7 @@ class AllLibraryUidsHelperTests(TestCase): fake_nodes.all.side_effect = RuntimeError("neo4j blew up") fake_library_cls = SimpleNamespace(nodes=fake_nodes) - with patch("library.views.neo4j_available", return_value=True), \ + with patch("library.utils.neo4j_available", return_value=True), \ patch.dict("sys.modules", {"library.models": SimpleNamespace(Library=fake_library_cls)}): self.assertEqual(views._all_library_uids(), []) diff --git a/mnemosyne/library/tests/test_tasks.py b/mnemosyne/library/tests/test_tasks.py index 702d320..239b6d2 100644 --- a/mnemosyne/library/tests/test_tasks.py +++ b/mnemosyne/library/tests/test_tasks.py @@ -13,7 +13,7 @@ from django.test import TestCase, override_settings class EmbedItemTaskTests(TestCase): """Tests for the embed_item task.""" - @patch("library.tasks.EmbeddingPipeline") + @patch("library.services.pipeline.EmbeddingPipeline") def test_embed_item_success(self, mock_pipeline_cls): from library.tasks import embed_item @@ -31,7 +31,7 @@ class EmbedItemTaskTests(TestCase): self.assertEqual(result["item_uid"], "test-uid-123") mock_pipeline.process_item.assert_called_once() - @patch("library.tasks.EmbeddingPipeline") + @patch("library.services.pipeline.EmbeddingPipeline") def test_embed_item_failure(self, mock_pipeline_cls): from library.tasks import embed_item @@ -49,7 +49,7 @@ class EmbedItemTaskTests(TestCase): class ReembedItemTaskTests(TestCase): """Tests for the reembed_item task.""" - @patch("library.tasks.EmbeddingPipeline") + @patch("library.services.pipeline.EmbeddingPipeline") def test_reembed_item_success(self, mock_pipeline_cls): from library.tasks import reembed_item -- 2.43.0