Merge pull request #1251 from asamal4/avoid-unittest

[LCORE-1394] chore: enforce avoiding unittest and fix existing test cases

Author: tisnik

pyproject.toml

@@ -221,8 +221,12 @@ disable = ["R0801"]
 [tool.ruff]
 extend-exclude = ["tests/profiles/syntax_error.py"]
 
-[tool.ruff.lint.flake8-tidy-imports]
-banned-api = { "unittest" = { msg = "use pytest instead of unittest" }, "unittest.mock" = { msg = "use pytest-mock instead of unittest.mock" } }
+[tool.ruff.lint]
+extend-select = ["TID251"]
+
+[tool.ruff.lint.flake8-tidy-imports.banned-api]
+unittest = { msg = "use pytest instead of unittest" }
+"unittest.mock" = { msg = "use pytest-mock instead of unittest.mock" }
 
 [tool.mypy]
 explicit_package_bases = true

tests/unit/a2a_storage/test_storage_factory.py

@@ -3,8 +3,7 @@
 # pylint: disable=protected-access
 
 from pathlib import Path
-from typing import Generator
-from unittest.mock import PropertyMock
+from typing import Any, Generator
 
 import pytest
 from pytest_mock import MockerFixture
@@ -17,6 +16,16 @@
 from models.config import A2AStateConfiguration, SQLiteDatabaseConfiguration
 
 
+class _FakeProperty:  # pylint: disable=too-few-public-methods
+    """Descriptor that returns a fixed value (like PropertyMock)."""
+
+    def __init__(self, value: Any) -> None:
+        self._value = value
+
+    def __get__(self, obj: Any, owner: Any = None) -> Any:
+        return self._value
+
+
 class TestA2AStorageFactory:
     """Tests for A2AStorageFactory."""
 
@@ -127,12 +136,11 @@ async def test_invalid_storage_type_raises_error(
         """Test that an invalid storage type raises ValueError."""
         config = A2AStateConfiguration()
 
-        # Mock the storage_type property to return an invalid value
+        # Replace property on class so config.storage_type returns "invalid"
         mocker.patch.object(
             A2AStateConfiguration,
             "storage_type",
-            new_callable=PropertyMock,
-            return_value="invalid",
+            _FakeProperty("invalid"),
         )
         with pytest.raises(ValueError, match="Unknown A2A state type"):
             await A2AStorageFactory.create_task_store(config)
@@ -144,12 +152,11 @@ async def test_sqlite_storage_type_without_config_raises_error(
         """Test that SQLite storage type without config raises ValueError."""
         config = A2AStateConfiguration()
 
-        # Mock to simulate misconfiguration
+        # Replace property on class so config.storage_type returns "sqlite"
         mocker.patch.object(
             A2AStateConfiguration,
             "storage_type",
-            new_callable=PropertyMock,
-            return_value="sqlite",
+            _FakeProperty("sqlite"),
         )
         with pytest.raises(ValueError, match="SQLite configuration required"):
             await A2AStorageFactory.create_task_store(config)
@@ -161,12 +168,11 @@ async def test_postgres_storage_type_without_config_raises_error(
         """Test that PostgreSQL storage type without config raises ValueError."""
         config = A2AStateConfiguration()
 
-        # Mock to simulate misconfiguration
+        # Replace property on class so config.storage_type returns "postgres"
         mocker.patch.object(
             A2AStateConfiguration,
             "storage_type",
-            new_callable=PropertyMock,
-            return_value="postgres",
+            _FakeProperty("postgres"),
         )
         with pytest.raises(ValueError, match="PostgreSQL configuration required"):
             await A2AStorageFactory.create_task_store(config)

tests/unit/app/endpoints/test_a2a.py

@@ -4,7 +4,6 @@
 # pylint: disable=protected-access
 
 from typing import Any
-from unittest.mock import AsyncMock, MagicMock
 
 import httpx
 import pytest
@@ -161,7 +160,7 @@ def test_convert_single_output_item(self, mocker: MockerFixture) -> None:
             "app.endpoints.a2a.extract_text_from_response_item",
             return_value="Hello, world!",
         )
-        mock_output_item = MagicMock()
+        mock_output_item = mocker.MagicMock()
         result = _convert_responses_content_to_a2a_parts([mock_output_item])
         assert len(result) == 1
         assert result[0].root is not None
@@ -175,8 +174,8 @@ def test_convert_multiple_output_items(self, mocker: MockerFixture) -> None:
         )
         extract_mock.side_effect = ["First", "Second"]
 
-        mock_item1 = MagicMock()
-        mock_item2 = MagicMock()
+        mock_item1 = mocker.MagicMock()
+        mock_item2 = mocker.MagicMock()
 
         result = _convert_responses_content_to_a2a_parts([mock_item1, mock_item2])
         assert len(result) == 2
@@ -194,7 +193,7 @@ def test_convert_output_items_with_none_text(self, mocker: MockerFixture) -> Non
         )
         extract_mock.side_effect = ["Valid text", None, "Another valid"]
 
-        mock_items = [MagicMock(), MagicMock(), MagicMock()]
+        mock_items = [mocker.MagicMock(), mocker.MagicMock(), mocker.MagicMock()]
 
         result = _convert_responses_content_to_a2a_parts(mock_items)
         assert len(result) == 2
@@ -470,14 +469,16 @@ def test_executor_initialization_default_mcp_headers(self) -> None:
         assert executor.mcp_headers == {}
 
     @pytest.mark.asyncio
-    async def test_execute_without_message_raises_error(self) -> None:
+    async def test_execute_without_message_raises_error(
+        self, mocker: MockerFixture
+    ) -> None:
         """Test that execute raises error when message is missing."""
         executor = A2AAgentExecutor(auth_token="test-token")
 
-        context = MagicMock(spec=RequestContext)
+        context = mocker.MagicMock(spec=RequestContext)
         context.message = None
 
-        event_queue = AsyncMock(spec=EventQueue)
+        event_queue = mocker.AsyncMock(spec=EventQueue)
 
         with pytest.raises(ValueError, match="A2A request must have a message"):
             await executor.execute(context, event_queue)
@@ -492,23 +493,23 @@ async def test_execute_creates_new_task(
         executor = A2AAgentExecutor(auth_token="test-token")
 
         # Mock the context with a mock message
-        mock_message = MagicMock()
+        mock_message = mocker.MagicMock()
         mock_message.role = "user"
         mock_message.parts = [Part(root=TextPart(text="Hello"))]
         mock_message.metadata = {}
 
-        context = MagicMock(spec=RequestContext)
+        context = mocker.MagicMock(spec=RequestContext)
         context.message = mock_message
         context.current_task = None
         context.task_id = None
         context.context_id = None
         context.get_user_input.return_value = "Hello"
 
         # Mock event queue
-        event_queue = AsyncMock(spec=EventQueue)
+        event_queue = mocker.AsyncMock(spec=EventQueue)
 
         # Mock new_task to return a mock Task
-        mock_task = MagicMock()
+        mock_task = mocker.MagicMock()
         mock_task.id = "test-task-id"
         mock_task.context_id = "test-context-id"
         mocker.patch("app.endpoints.a2a.new_task", return_value=mock_task)
@@ -517,7 +518,7 @@ async def test_execute_creates_new_task(
         mocker.patch.object(
             executor,
             "_process_task_streaming",
-            new_callable=AsyncMock,
+            new_callable=mocker.AsyncMock,
         )
 
         await executor.execute(context, event_queue)
@@ -540,23 +541,23 @@ async def test_execute_passes_task_ids_to_streaming(
         executor = A2AAgentExecutor(auth_token="test-token")
 
         # Mock the context with empty task_id and context_id (first-turn scenario)
-        mock_message = MagicMock()
+        mock_message = mocker.MagicMock()
         mock_message.role = "user"
         mock_message.parts = [Part(root=TextPart(text="Hello"))]
         mock_message.metadata = {}
 
-        context = MagicMock(spec=RequestContext)
+        context = mocker.MagicMock(spec=RequestContext)
         context.message = mock_message
         context.current_task = None
         context.task_id = None  # Empty in context object
         context.context_id = None  # Empty in context object
         context.get_user_input.return_value = "Hello"
 
         # Mock event queue
-        event_queue = AsyncMock(spec=EventQueue)
+        event_queue = mocker.AsyncMock(spec=EventQueue)
 
         # Mock new_task to return a task with specific IDs
-        mock_task = MagicMock()
+        mock_task = mocker.MagicMock()
         mock_task.id = "computed-task-id-123"
         mock_task.context_id = "computed-context-id-456"
         mocker.patch("app.endpoints.a2a.new_task", return_value=mock_task)
@@ -565,7 +566,7 @@ async def test_execute_passes_task_ids_to_streaming(
         mock_process_streaming = mocker.patch.object(
             executor,
             "_process_task_streaming",
-            new_callable=AsyncMock,
+            new_callable=mocker.AsyncMock,
         )
 
         await executor.execute(context, event_queue)
@@ -591,20 +592,20 @@ async def test_execute_handles_errors_gracefully(
         executor = A2AAgentExecutor(auth_token="test-token")
 
         # Mock the context with a mock message
-        mock_message = MagicMock()
+        mock_message = mocker.MagicMock()
         mock_message.role = "user"
         mock_message.parts = [Part(root=TextPart(text="Hello"))]
         mock_message.metadata = {}
 
-        context = MagicMock(spec=RequestContext)
+        context = mocker.MagicMock(spec=RequestContext)
         context.message = mock_message
-        context.current_task = MagicMock()
+        context.current_task = mocker.MagicMock()
         context.task_id = "task-123"
         context.context_id = "ctx-456"
         context.get_user_input.return_value = "Hello"
 
         # Mock event queue
-        event_queue = AsyncMock(spec=EventQueue)
+        event_queue = mocker.AsyncMock(spec=EventQueue)
 
         # Mock the streaming process to raise an error
         mocker.patch.object(
@@ -637,23 +638,23 @@ async def test_process_task_streaming_no_input(
         executor = A2AAgentExecutor(auth_token="test-token")
 
         # Mock the context with no input
-        mock_message = MagicMock()
+        mock_message = mocker.MagicMock()
         mock_message.role = "user"
         mock_message.parts = []
         mock_message.metadata = {}
 
-        context = MagicMock(spec=RequestContext)
+        context = mocker.MagicMock(spec=RequestContext)
         context.task_id = "task-123"
         context.context_id = "ctx-456"
         context.message = mock_message
         context.get_user_input.return_value = ""
 
         # Mock event queue
-        event_queue = AsyncMock(spec=EventQueue)
+        event_queue = mocker.AsyncMock(spec=EventQueue)
 
         # Create task updater mock
-        task_updater = MagicMock()
-        task_updater.update_status = AsyncMock()
+        task_updater = mocker.MagicMock()
+        task_updater.update_status = mocker.AsyncMock()
         task_updater.event_queue = event_queue
 
         await executor._process_task_streaming(
@@ -675,34 +676,34 @@ async def test_process_task_streaming_handles_api_connection_error_on_models_lis
         executor = A2AAgentExecutor(auth_token="test-token")
 
         # Mock the context with valid input
-        mock_message = MagicMock()
+        mock_message = mocker.MagicMock()
         mock_message.role = "user"
         mock_message.parts = [Part(root=TextPart(text="Hello"))]
         mock_message.metadata = {}
 
-        context = MagicMock(spec=RequestContext)
+        context = mocker.MagicMock(spec=RequestContext)
         context.task_id = "task-123"
         context.context_id = "ctx-456"
         context.message = mock_message
         context.get_user_input.return_value = "Hello"
 
         # Mock event queue
-        event_queue = AsyncMock(spec=EventQueue)
+        event_queue = mocker.AsyncMock(spec=EventQueue)
 
         # Create task updater mock
-        task_updater = MagicMock()
-        task_updater.update_status = AsyncMock()
+        task_updater = mocker.MagicMock()
+        task_updater.update_status = mocker.AsyncMock()
         task_updater.event_queue = event_queue
 
         # Mock the context store
-        mock_context_store = AsyncMock()
+        mock_context_store = mocker.AsyncMock()
         mock_context_store.get.return_value = None
         mocker.patch(
             "app.endpoints.a2a._get_context_store", return_value=mock_context_store
         )
 
         # Mock the client to raise APIConnectionError on models.list()
-        mock_client = AsyncMock()
+        mock_client = mocker.AsyncMock()
         # Create a mock httpx.Request for APIConnectionError
         mock_request = httpx.Request("GET", "http://test-llama-stack/models")
         mock_client.models.list.side_effect = APIConnectionError(
@@ -733,35 +734,35 @@ async def test_process_task_streaming_handles_api_connection_error_on_retrieve_r
         executor = A2AAgentExecutor(auth_token="test-token")
 
         # Mock the context with valid input
-        mock_message = MagicMock()
+        mock_message = mocker.MagicMock()
         mock_message.role = "user"
         mock_message.parts = [Part(root=TextPart(text="Hello"))]
         mock_message.metadata = {}
 
-        context = MagicMock(spec=RequestContext)
+        context = mocker.MagicMock(spec=RequestContext)
         context.task_id = "task-123"
         context.context_id = "ctx-456"
         context.message = mock_message
         context.get_user_input.return_value = "Hello"
 
         # Mock event queue
-        event_queue = AsyncMock(spec=EventQueue)
+        event_queue = mocker.AsyncMock(spec=EventQueue)
 
         # Create task updater mock
-        task_updater = MagicMock()
-        task_updater.update_status = AsyncMock()
+        task_updater = mocker.MagicMock()
+        task_updater.update_status = mocker.AsyncMock()
         task_updater.event_queue = event_queue
 
         # Mock the context store
-        mock_context_store = AsyncMock()
+        mock_context_store = mocker.AsyncMock()
         mock_context_store.get.return_value = None
         mocker.patch(
             "app.endpoints.a2a._get_context_store", return_value=mock_context_store
         )
 
         # Mock the client to succeed on models.list()
-        mock_client = AsyncMock()
-        mock_models = [MagicMock()]  # Return a list of models
+        mock_client = mocker.AsyncMock()
+        mock_models = [mocker.MagicMock()]  # Return a list of models
         mock_client.models.list = mocker.AsyncMock(return_value=mock_models)
 
         # Mock responses.create to raise APIConnectionError
@@ -801,12 +802,12 @@ async def test_process_task_streaming_handles_api_connection_error_on_retrieve_r
         assert "Unable to connect to Llama Stack backend service" in str(error_message)
 
     @pytest.mark.asyncio
-    async def test_cancel_raises_not_implemented(self) -> None:
+    async def test_cancel_raises_not_implemented(self, mocker: MockerFixture) -> None:
         """Test that cancel raises NotImplementedError."""
         executor = A2AAgentExecutor(auth_token="test-token")
 
-        context = MagicMock(spec=RequestContext)
-        event_queue = AsyncMock(spec=EventQueue)
+        context = mocker.MagicMock(spec=RequestContext)
+        event_queue = mocker.AsyncMock(spec=EventQueue)
 
         with pytest.raises(NotImplementedError):
             await executor.cancel(context, event_queue)