Merge pull request #752 from tisnik/lcore-740-type-hints-for-endpoint-handlers

LCORE-740: type hints for endpoint handlers

Author: tisnik

tests/unit/app/endpoints/test_conversations.py

@@ -2,7 +2,7 @@
 
 """Unit tests for the /conversations REST API endpoints."""
 
-from pytest_mock import MockerFixture
+from pytest_mock import MockerFixture, MockType
 import pytest
 from fastapi import HTTPException, status
 
@@ -122,7 +122,7 @@ def test_transform_message_with_empty_referenced_documents(self) -> None:
 
 
 @pytest.fixture
-def mock_configuration(mocker: MockerFixture):
+def mock_configuration(mocker: MockerFixture) -> MockType:
     """Mock configuration with conversation cache."""
     mock_config = mocker.Mock()
     mock_cache = mocker.Mock()
@@ -133,27 +133,31 @@ def mock_configuration(mocker: MockerFixture):
 class TestCheckValidConversationId:
     """Test cases for the check_valid_conversation_id function."""
 
-    def test_valid_conversation_id(self, mocker: MockerFixture):
+    def test_valid_conversation_id(self, mocker: MockerFixture) -> None:
         """Test with a valid conversation ID."""
         mocker.patch("app.endpoints.conversations_v2.check_suid", return_value=True)
         # Should not raise an exception
         check_valid_conversation_id(VALID_CONVERSATION_ID)
 
-    def test_invalid_conversation_id(self, mocker: MockerFixture):
+    def test_invalid_conversation_id(self, mocker: MockerFixture) -> None:
         """Test with an invalid conversation ID."""
         mocker.patch("app.endpoints.conversations_v2.check_suid", return_value=False)
 
         with pytest.raises(HTTPException) as exc_info:
             check_valid_conversation_id(INVALID_CONVERSATION_ID)
 
         assert exc_info.value.status_code == status.HTTP_400_BAD_REQUEST
-        assert "Invalid conversation ID format" in exc_info.value.detail["response"]
+        detail = exc_info.value.detail
+        assert isinstance(detail, dict)
+        assert "Invalid conversation ID format" in detail["response"]
 
 
 class TestCheckConversationExistence:
     """Test cases for the check_conversation_existence function."""
 
-    def test_conversation_exists(self, mocker, mock_configuration):
+    def test_conversation_exists(
+        self, mocker: MockerFixture, mock_configuration: MockType
+    ) -> None:
         """Test when conversation exists."""
         mock_configuration.conversation_cache.list.return_value = [
             mocker.Mock(conversation_id=VALID_CONVERSATION_ID)
@@ -163,7 +167,9 @@ def test_conversation_exists(self, mocker, mock_configuration):
         # Should not raise an exception
         check_conversation_existence("user_id", VALID_CONVERSATION_ID)
 
-    def test_conversation_not_exists(self, mocker, mock_configuration):
+    def test_conversation_not_exists(
+        self, mocker: MockerFixture, mock_configuration: MockType
+    ) -> None:
         """Test when conversation does not exist."""
         mock_configuration.conversation_cache.list.return_value = []
         mocker.patch("app.endpoints.conversations_v2.configuration", mock_configuration)
@@ -172,14 +178,16 @@ def test_conversation_not_exists(self, mocker, mock_configuration):
             check_conversation_existence("user_id", VALID_CONVERSATION_ID)
 
         assert exc_info.value.status_code == status.HTTP_404_NOT_FOUND
-        assert "Conversation not found" in exc_info.value.detail["response"]
+        detail = exc_info.value.detail
+        assert isinstance(detail, dict)
+        assert "Conversation not found" in detail["response"]
 
 
 class TestUpdateConversationEndpoint:
     """Test cases for the PUT /conversations/{conversation_id} endpoint."""
 
     @pytest.mark.asyncio
-    async def test_configuration_not_loaded(self, mocker: MockerFixture):
+    async def test_configuration_not_loaded(self, mocker: MockerFixture) -> None:
         """Test the endpoint when configuration is not loaded."""
         mock_authorization_resolvers(mocker)
         mocker.patch("app.endpoints.conversations_v2.configuration", None)
@@ -197,8 +205,8 @@ async def test_configuration_not_loaded(self, mocker: MockerFixture):
 
     @pytest.mark.asyncio
     async def test_invalid_conversation_id_format(
-        self, mocker: MockerFixture, mock_configuration
-    ):
+        self, mocker: MockerFixture, mock_configuration: MockType
+    ) -> None:
         """Test the endpoint with an invalid conversation ID format."""
         mock_authorization_resolvers(mocker)
         mocker.patch("app.endpoints.conversations_v2.configuration", mock_configuration)
@@ -214,10 +222,14 @@ async def test_invalid_conversation_id_format(
             )
 
         assert exc_info.value.status_code == status.HTTP_400_BAD_REQUEST
-        assert "Invalid conversation ID format" in exc_info.value.detail["response"]
+        detail = exc_info.value.detail
+        assert isinstance(detail, dict)
+        assert "Invalid conversation ID format" in detail["response"]
 
     @pytest.mark.asyncio
-    async def test_conversation_cache_not_configured(self, mocker: MockerFixture):
+    async def test_conversation_cache_not_configured(
+        self, mocker: MockerFixture
+    ) -> None:
         """Test the endpoint when conversation cache is not configured."""
         mock_authorization_resolvers(mocker)
         mock_config = mocker.Mock()
@@ -235,14 +247,14 @@ async def test_conversation_cache_not_configured(self, mocker: MockerFixture):
             )
 
         assert exc_info.value.status_code == status.HTTP_404_NOT_FOUND
-        assert (
-            "Conversation cache is not configured" in exc_info.value.detail["response"]
-        )
+        detail = exc_info.value.detail
+        assert isinstance(detail, dict)
+        assert "Conversation cache is not configured" in detail["response"]
 
     @pytest.mark.asyncio
     async def test_conversation_not_found(
-        self, mocker: MockerFixture, mock_configuration
-    ):
+        self, mocker: MockerFixture, mock_configuration: MockType
+    ) -> None:
         """Test the endpoint when conversation does not exist."""
         mock_authorization_resolvers(mocker)
         mocker.patch("app.endpoints.conversations_v2.configuration", mock_configuration)
@@ -259,10 +271,14 @@ async def test_conversation_not_found(
             )
 
         assert exc_info.value.status_code == status.HTTP_404_NOT_FOUND
-        assert "Conversation not found" in exc_info.value.detail["response"]
+        detail = exc_info.value.detail
+        assert isinstance(detail, dict)
+        assert "Conversation not found" in detail["response"]
 
     @pytest.mark.asyncio
-    async def test_successful_update(self, mocker: MockerFixture, mock_configuration):
+    async def test_successful_update(
+        self, mocker: MockerFixture, mock_configuration: MockType
+    ) -> None:
         """Test successful topic summary update."""
         mock_authorization_resolvers(mocker)
         mocker.patch("app.endpoints.conversations_v2.configuration", mock_configuration)

tests/unit/app/endpoints/test_conversations_v2.py

@@ -1,14 +1,16 @@
 """Unit tests for the /metrics REST API endpoint."""
 
 import pytest
+from pytest_mock import MockerFixture
 from fastapi import Request
 
+from authentication.interface import AuthTuple
 from app.endpoints.metrics import metrics_endpoint_handler
 from tests.unit.utils.auth_helpers import mock_authorization_resolvers
 
 
 @pytest.mark.asyncio
-async def test_metrics_endpoint(mocker):
+async def test_metrics_endpoint(mocker: MockerFixture) -> None:
     """Test the metrics endpoint handler."""
     mock_authorization_resolvers(mocker)
 
@@ -20,7 +22,7 @@ async def test_metrics_endpoint(mocker):
             "type": "http",
         }
     )
-    auth = ("test_user", "token", {})
+    auth: AuthTuple = ("test_user", "token", {})
     response = await metrics_endpoint_handler(auth=auth, request=request)
     assert response is not None
     assert response.status_code == 200

tests/unit/app/endpoints/test_metrics.py

@@ -1,18 +1,23 @@
 """Unit tests for the /models REST API endpoint."""
 
+from typing import Any
 import pytest
 
 from fastapi import HTTPException, Request, status
+from pytest_mock import MockerFixture
 
 from llama_stack_client import APIConnectionError
 
+from authentication.interface import AuthTuple
 from app.endpoints.models import models_endpoint_handler
 from configuration import AppConfig
 from tests.unit.utils.auth_helpers import mock_authorization_resolvers
 
 
 @pytest.mark.asyncio
-async def test_models_endpoint_handler_configuration_not_loaded(mocker):
+async def test_models_endpoint_handler_configuration_not_loaded(
+    mocker: MockerFixture,
+) -> None:
     """Test the models endpoint handler if configuration is not loaded."""
     mock_authorization_resolvers(mocker)
 
@@ -29,7 +34,7 @@ async def test_models_endpoint_handler_configuration_not_loaded(mocker):
             "headers": [(b"authorization", b"Bearer invalid-token")],
         }
     )
-    auth = ("user_id", "user_name", "token")
+    auth: AuthTuple = ("user_id", "user_name", "token")
 
     with pytest.raises(HTTPException) as e:
         await models_endpoint_handler(request=request, auth=auth)
@@ -38,12 +43,14 @@ async def test_models_endpoint_handler_configuration_not_loaded(mocker):
 
 
 @pytest.mark.asyncio
-async def test_models_endpoint_handler_improper_llama_stack_configuration(mocker):
+async def test_models_endpoint_handler_improper_llama_stack_configuration(
+    mocker: MockerFixture,
+) -> None:
     """Test the models endpoint handler if Llama Stack configuration is not proper."""
     mock_authorization_resolvers(mocker)
 
     # configuration for tests
-    config_dict = {
+    config_dict: dict[str, Any] = {
         "name": "test",
         "service": {
             "host": "localhost",
@@ -80,20 +87,22 @@ async def test_models_endpoint_handler_improper_llama_stack_configuration(mocker
             "headers": [(b"authorization", b"Bearer invalid-token")],
         }
     )
-    auth = ("test_user", "token", {})
+    auth: AuthTuple = ("test_user", "token", {})
     with pytest.raises(HTTPException) as e:
         await models_endpoint_handler(request=request, auth=auth)
         assert e.value.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR
         assert e.detail["response"] == "Llama stack is not configured"
 
 
 @pytest.mark.asyncio
-async def test_models_endpoint_handler_configuration_loaded(mocker):
+async def test_models_endpoint_handler_configuration_loaded(
+    mocker: MockerFixture,
+) -> None:
     """Test the models endpoint handler if configuration is loaded."""
     mock_authorization_resolvers(mocker)
 
     # configuration for tests
-    config_dict = {
+    config_dict: dict[str, Any] = {
         "name": "foo",
         "service": {
             "host": "localhost",
@@ -124,7 +133,7 @@ async def test_models_endpoint_handler_configuration_loaded(mocker):
             "headers": [(b"authorization", b"Bearer invalid-token")],
         }
     )
-    auth = ("test_user", "token", {})
+    auth: AuthTuple = ("test_user", "token", {})
 
     with pytest.raises(HTTPException) as e:
         await models_endpoint_handler(request=request, auth=auth)
@@ -133,12 +142,14 @@ async def test_models_endpoint_handler_configuration_loaded(mocker):
 
 
 @pytest.mark.asyncio
-async def test_models_endpoint_handler_unable_to_retrieve_models_list(mocker):
+async def test_models_endpoint_handler_unable_to_retrieve_models_list(
+    mocker: MockerFixture,
+) -> None:
     """Test the models endpoint handler if configuration is loaded."""
     mock_authorization_resolvers(mocker)
 
     # configuration for tests
-    config_dict = {
+    config_dict: dict[str, Any] = {
         "name": "foo",
         "service": {
             "host": "localhost",
@@ -177,18 +188,20 @@ async def test_models_endpoint_handler_unable_to_retrieve_models_list(mocker):
             "headers": [(b"authorization", b"Bearer invalid-token")],
         }
     )
-    auth = ("test_user", "token", {})
+    auth: AuthTuple = ("test_user", "token", {})
     response = await models_endpoint_handler(request=request, auth=auth)
     assert response is not None
 
 
 @pytest.mark.asyncio
-async def test_models_endpoint_llama_stack_connection_error(mocker):
+async def test_models_endpoint_llama_stack_connection_error(
+    mocker: MockerFixture,
+) -> None:
     """Test the model endpoint when LlamaStack connection fails."""
     mock_authorization_resolvers(mocker)
 
     # configuration for tests
-    config_dict = {
+    config_dict: dict[str, Any] = {
         "name": "foo",
         "service": {
             "host": "localhost",
@@ -214,7 +227,7 @@ async def test_models_endpoint_llama_stack_connection_error(mocker):
     # mock AsyncLlamaStackClientHolder to raise APIConnectionError
     # when models.list() method is called
     mock_client = mocker.AsyncMock()
-    mock_client.models.list.side_effect = APIConnectionError(request=None)
+    mock_client.models.list.side_effect = APIConnectionError(request=None)  # type: ignore
     mock_client_holder = mocker.patch(
         "app.endpoints.models.AsyncLlamaStackClientHolder"
     )
@@ -229,7 +242,7 @@ async def test_models_endpoint_llama_stack_connection_error(mocker):
             "headers": [(b"authorization", b"Bearer invalid-token")],
         }
     )
-    auth = ("test_user", "token", {})
+    auth: AuthTuple = ("test_user", "token", {})
 
     with pytest.raises(HTTPException) as e:
         await models_endpoint_handler(request=request, auth=auth)