Merge pull request lightspeed-core#930 from tisnik/lcore-1126-updated-docstrings-in-unit-tests

LCORE-1126: updated docstrings in unit tests

Author: tisnik

tests/unit/app/endpoints/README.md

@@ -42,6 +42,9 @@ Unit tests for the /query (v2) REST API endpoint using Responses API.
 ## [test_rags.py](test_rags.py)
 Unit tests for the /rags REST API endpoints.
 
+## [test_rlsapi_v1.py](test_rlsapi_v1.py)
+Unit tests for the rlsapi v1 /infer REST API endpoint.
+
 ## [test_root.py](test_root.py)
 Unit tests for the / endpoint handler.
 

tests/unit/app/endpoints/test_authorized.py

@@ -47,7 +47,19 @@ async def test_authorized_unauthorized() -> None:
 
 @pytest.mark.asyncio
 async def test_authorized_dependency_unauthorized() -> None:
-    """Test that auth dependency raises HTTPException with 403 for unauthorized access."""
+    """Test that auth dependency raises HTTPException with 403 for unauthorized access.
+
+    Verify extract_user_token raises HTTPException with status code 401 and the
+    expected detail for missing or malformed Authorization headers.
+
+    Checks two scenarios:
+    - Missing Authorization header: HTTPException.status_code == 401,
+      detail["response"] == "Missing or invalid credentials provided by
+      client", detail["cause"] == "No Authorization header found".
+    - Invalid Authorization format: HTTPException.status_code == 401,
+      detail["response"] == "Missing or invalid credentials provided by
+      client", detail["cause"] == "No token found in Authorization header".
+    """
     # Test the auth utility function that would be called by auth dependencies
     # This simulates the unauthorized scenario that would prevent the handler from being called
 

tests/unit/app/endpoints/test_conversations.py

@@ -34,7 +34,13 @@
 
 @pytest.fixture
 def dummy_request() -> Request:
-    """Mock request object for testing."""
+    """Mock request object for testing.
+
+    Create a mock FastAPI Request configured for tests with full authorization.
+
+    The returned Request has state.authorized_actions set to a set containing
+    every member of Action.
+    """
     request = Request(
         scope={
             "type": "http",
@@ -56,7 +62,35 @@ def create_mock_conversation(
     last_used_provider: str,
     topic_summary: Optional[str] = None,
 ) -> MockType:
-    """Helper function to create a mock conversation object with all required attributes."""
+    """Helper function to create a mock conversation object with all required attributes.
+
+    Create a mock conversation object with the attributes used by the
+    conversations list and detail tests.
+
+    The returned mock has the following attributes:
+    - id: the conversation identifier (string)
+    - created_at.isoformat(): returns the provided created_at string
+    - last_message_at.isoformat(): returns the provided last_message_at string
+    - message_count: number of messages in the conversation
+    - last_used_model: model identifier last used in the conversation
+    - last_used_provider: provider identifier last used in the conversation
+    - topic_summary: optional topic summary (may be None or empty string)
+
+    Parameters:
+        mocker (MockerFixture): pytest mocker fixture used to build the mock object.
+        conversation_id (str): Conversation identifier to assign to the mock.
+        created_at (str): ISO-formatted created-at timestamp to be returned by
+        created_at.isoformat().
+        last_message_at (str): ISO-formatted last-message timestamp to be
+        returned by last_message_at.isoformat().
+        message_count (int): Message count to assign to the mock.
+        last_used_model (str): Last used model string to assign to the mock.
+        last_used_provider (str): Last used provider string to assign to the mock.
+        topic_summary (Optional[str]): Optional topic summary to assign to the mock.
+
+    Returns:
+        mock_conversation: A mock object configured with the above attributes.
+    """
     mock_conversation = mocker.Mock()
     mock_conversation.id = conversation_id
     mock_conversation.created_at = mocker.Mock()
@@ -73,7 +107,20 @@ def create_mock_conversation(
 def mock_database_session(
     mocker: MockerFixture, query_result: Optional[list[MockType]] = None
 ) -> MockType:
-    """Helper function to mock get_session with proper context manager support."""
+    """Helper function to mock get_session with proper context manager support.
+
+    Create and patch a mocked database session and a context-manager-compatible get_session.
+
+    Parameters:
+        mocker (pytest.MockerFixture): Fixture used to create and patch mocks.
+        query_result (Optional[list]): If provided, configures the
+        session.query().all() and session.query().filter_by().all() to return
+        this list.
+
+    Returns:
+        Mock: The mocked session object that will be yielded by the patched
+        get_session context manager.
+    """
     mock_session = mocker.Mock()
     if query_result is not None:
         # Mock both the filtered and unfiltered query paths
@@ -94,7 +141,16 @@ def mock_database_session(
 
 @pytest.fixture(name="setup_configuration")
 def setup_configuration_fixture() -> AppConfig:
-    """Set up configuration for tests."""
+    """Set up configuration for tests.
+
+    Create an AppConfig prepopulated with test-friendly default settings.
+
+    Returns:
+        AppConfig: An AppConfig instance initialized from a dictionary
+        containing defaults suitable for tests (local service host/port,
+        disabled auth and user-data collection, test Llama Stack API key and
+        URL, and single worker).
+    """
     config_dict: dict[str, Any] = {
         "name": "test",
         "service": {
@@ -123,7 +179,29 @@ def setup_configuration_fixture() -> AppConfig:
 
 @pytest.fixture(name="mock_session_data")
 def mock_session_data_fixture() -> dict[str, Any]:
-    """Create mock session data for testing."""
+    """Create mock session data for testing.
+
+    Provide a representative mock session data payload used by tests to
+    simulate a conversation session.
+
+    The returned dictionary contains:
+    - session_id: conversation identifier.
+    - session_name: human-readable session name.
+    - started_at: ISO 8601 timestamp when the session started.
+    - turns: list of turn objects; each turn includes:
+        - turn_id: identifier for the turn.
+        - input_messages: list of input message objects with `content`, `role`,
+          and optional `context`.
+        - output_message: assistant response object with `content`, `role`, and
+          auxiliary fields (e.g., `stop_reason`, `tool_calls`) that tests
+          expect to be filtered by simplification logic.
+        - started_at / completed_at: ISO 8601 timestamps for the turn.
+        - steps: detailed internal steps included to verify they are removed by simplification.
+
+    Returns:
+        dict: A mock session data structure matching the shape produced by the
+        Llama Stack client for use in unit tests.
+    """
     return {
         "session_id": VALID_CONVERSATION_ID,
         "session_name": "test-session",
@@ -165,7 +243,17 @@ def mock_session_data_fixture() -> dict[str, Any]:
 
 @pytest.fixture(name="expected_chat_history")
 def expected_chat_history_fixture() -> list[dict[str, Any]]:
-    """Create expected simplified chat history for testing."""
+    """Create expected simplified chat history for testing.
+
+    Expected simplified chat history used by tests.
+
+    Returns:
+        list[dict[str, Any]]: A list of conversation turns. Each turn contains:
+            - messages: list of message dicts with `content` (str) and `type`
+              (`"user"` or `"assistant"`)
+            - started_at: ISO 8601 UTC timestamp string for the turn start
+            - completed_at: ISO 8601 UTC timestamp string for the turn end
+    """
     return [
         {
             "messages": [
@@ -188,7 +276,14 @@ def expected_chat_history_fixture() -> list[dict[str, Any]]:
 
 @pytest.fixture(name="mock_conversation")
 def mock_conversation_fixture() -> UserConversation:
-    """Create a mock UserConversation object for testing."""
+    """Create a mock UserConversation object for testing.
+
+    Returns:
+        mock_conv (UserConversation): A UserConversation initialized with
+        VALID_CONVERSATION_ID, user_id set to "another_user", message_count 2,
+        last_used_model "mock-model", last_used_provider "mock-provider", and
+        topic_summary "Mock topic".
+    """
     mock_conv = UserConversation()
     mock_conv.id = VALID_CONVERSATION_ID
     mock_conv.user_id = "another_user"  # Different from test auth
@@ -366,7 +461,15 @@ async def test_llama_stack_not_found_error(
         setup_configuration: AppConfig,
         dummy_request: Request,
     ) -> None:
-        """Test the endpoint when LlamaStack returns NotFoundError."""
+        """Test the endpoint when LlamaStack returns NotFoundError.
+
+        Verify the GET /conversations/{conversation_id} handler raises an HTTP
+        404 when the Llama Stack client reports the session as not found.
+
+        Asserts that the raised HTTPException contains a response message
+        indicating the conversation was not found and a cause that includes
+        "does not exist" and the conversation ID.
+        """
         mock_authorization_resolvers(mocker)
         mocker.patch("app.endpoints.conversations.configuration", setup_configuration)
         mocker.patch("app.endpoints.conversations.check_suid", return_value=True)

tests/unit/app/endpoints/test_conversations_v2.py

@@ -135,7 +135,16 @@ def test_transform_message_with_empty_referenced_documents(self) -> None:
 
 @pytest.fixture
 def mock_configuration(mocker: MockerFixture) -> MockType:
-    """Mock configuration with conversation cache."""
+    """Mock configuration with conversation cache.
+
+    Create a mocked configuration object with a mocked `conversation_cache` attribute.
+
+    Parameters:
+        mocker (pytest.MockFixture): The pytest-mock fixture used to create mocks.
+
+    Returns:
+        Mock: A mock configuration object whose `conversation_cache` attribute is a mock.
+    """
     mock_config = mocker.Mock()
     mock_cache = mocker.Mock()
     mock_config.conversation_cache = mock_cache
@@ -303,7 +312,15 @@ async def test_successful_retrieval_empty_list(
     async def test_with_skip_userid_check(
         self, mocker: MockerFixture, mock_configuration: MockType
     ) -> None:
-        """Test the endpoint with skip_userid_check flag."""
+        """Test the endpoint with skip_userid_check flag.
+
+        Verify the conversations list handler forwards the skip_userid_check
+        flag from the auth tuple to the conversation cache.
+
+        Sets up a mocked configuration and auth tuple with the skip flag set to
+        True, invokes the handler, and asserts that `conversation_cache.list`
+        is called with the user ID and `True`.
+        """
         mock_authorization_resolvers(mocker)
         mocker.patch("app.endpoints.conversations_v2.configuration", mock_configuration)
         mock_configuration.conversation_cache.list.return_value = []
@@ -375,7 +392,18 @@ async def test_invalid_conversation_id_format(
     async def test_conversation_cache_not_configured(
         self, mocker: MockerFixture
     ) -> None:
-        """Test the endpoint when conversation cache is not configured."""
+        """Test the endpoint when conversation cache is not configured.
+
+        Verify the conversation GET endpoint raises an HTTP 500 error when the
+        conversation cache is not configured.
+
+        Patches the application configuration so
+        `conversation_cache_configuration.type` is None and ensures
+        `check_suid` returns True, then calls
+        `get_conversation_endpoint_handler` and asserts that it raises an
+        `HTTPException` with status code 500 and a response detail containing
+        "Conversation cache not configured".
+        """
         mock_authorization_resolvers(mocker)
         mock_config = mocker.Mock()
         mock_config.conversation_cache_configuration = mocker.Mock()
@@ -632,7 +660,17 @@ async def test_unsuccessful_deletion(
     async def test_with_skip_userid_check(
         self, mocker: MockerFixture, mock_configuration: MockType
     ) -> None:
-        """Test the endpoint with skip_userid_check flag."""
+        """Test the endpoint with skip_userid_check flag.
+
+        Verifies that providing an auth tuple with the skip-userid flag set
+        causes the conversation delete handler to call the cache delete method
+        with the skip flag.
+
+        This test patches configuration and SUID validation, supplies an auth
+        tuple where the third element is True, invokes
+        delete_conversation_endpoint_handler, and asserts the cache.delete was
+        called with (user_id, conversation_id, True).
+        """
         mock_authorization_resolvers(mocker)
         mocker.patch("app.endpoints.conversations_v2.configuration", mock_configuration)
         mocker.patch("app.endpoints.conversations_v2.check_suid", return_value=True)

tests/unit/app/endpoints/test_health.py

@@ -126,7 +126,18 @@ class TestGetProvidersHealthStatuses:
     """Test cases for the get_providers_health_statuses function."""
 
     async def test_get_providers_health_statuses(self, mocker: MockerFixture) -> None:
-        """Test get_providers_health_statuses with healthy providers."""
+        """Test get_providers_health_statuses with healthy providers.
+
+        Verify get_providers_health_statuses returns a ProviderHealthStatus
+        entry for each provider reported by the client.
+
+        Mocks an AsyncLlamaStack client whose providers.list() returns three
+        providers with distinct health dicts, then asserts the function
+        produces three results with:
+        - provider1: status OK, message "All good"
+        - provider2: status NOT_IMPLEMENTED, message "Provider does not implement health check"
+        - unhealthy_provider: status ERROR, message "Connection failed"
+        """
         # Mock the imports
         mock_lsc = mocker.patch("client.AsyncLlamaStackClientHolder.get_client")
 

tests/unit/app/endpoints/test_info.py

@@ -77,7 +77,18 @@ async def test_info_endpoint(mocker: MockerFixture) -> None:
 
 @pytest.mark.asyncio
 async def test_info_endpoint_connection_error(mocker: MockerFixture) -> None:
-    """Test the info endpoint handler."""
+    """Test the info endpoint handler.
+
+    Verify that info_endpoint_handler raises an HTTPException with
+    status 503 when the LlamaStack client cannot connect.
+
+    Sets up application configuration and patches the LlamaStack
+    client so that calling its version inspection raises an
+    APIConnectionError, then asserts the raised HTTPException has
+    status code 503 and a detail payload containing a "response" of
+    "Service unavailable" and a "cause" that includes "Unable to
+    connect to Llama Stack".
+    """
     mock_authorization_resolvers(mocker)
 
     # configuration for tests

tests/unit/app/endpoints/test_models.py

@@ -44,7 +44,17 @@ async def test_models_endpoint_handler_configuration_not_loaded(
 async def test_models_endpoint_handler_configuration_loaded(
     mocker: MockerFixture,
 ) -> None:
-    """Test the models endpoint handler if configuration is loaded."""
+    """Test the models endpoint handler if configuration is loaded.
+
+    Verify the models endpoint raises HTTP 503 when configuration is loaded but
+    the Llama Stack client cannot connect.
+
+    Loads an AppConfig from a test dictionary, patches the endpoint's
+    configuration and AsyncLlamaStackClientHolder so that get_client raises
+    APIConnectionError, issues a request with an authorization header, and
+    asserts that calling the handler raises an HTTPException with status 503
+    and a detail response of "Unable to connect to Llama Stack".
+    """
     mock_authorization_resolvers(mocker)
 
     # configuration for tests