Merge pull request #239 from tisnik/lcore-303-fixed-pylint-issues-

LCORE-303: fixed pylint issues

Author: tisnik

tests/unit/app/endpoints/test_authorized.py

@@ -1,3 +1,5 @@
+"""Unit tests for the /authorized REST API endpoint."""
+
 from unittest.mock import AsyncMock
 
 import pytest

tests/unit/app/endpoints/test_config.py

@@ -1,3 +1,5 @@
+"""Unit tests for the /config REST API endpoint."""
+
 import pytest
 
 from fastapi import HTTPException, status
@@ -20,7 +22,7 @@ def test_config_endpoint_handler_configuration_not_loaded(mocker):
         assert e.detail["response"] == "Configuration is not loaded"
 
 
-def test_config_endpoint_handler_configuration_loaded(mocker):
+def test_config_endpoint_handler_configuration_loaded():
     """Test the config endpoint handler."""
     config_dict = {
         "name": "foo",

tests/unit/app/endpoints/test_feedback.py

@@ -1,3 +1,5 @@
+"""Unit tests for the /feedback REST API endpoint."""
+
 from fastapi import HTTPException, status
 import pytest
 
@@ -123,7 +125,7 @@ def test_store_feedback(mocker):
     )
 
 
-def test_feedback_status(mocker):
+def test_feedback_status():
     """Test that feedback_status returns the correct status response."""
     configuration.user_data_collection_configuration.feedback_disabled = False
 

tests/unit/app/endpoints/test_health.py

@@ -1,12 +1,15 @@
+"""Unit tests for the /health REST API endpoint."""
+
 from unittest.mock import Mock
 
+from llama_stack.providers.datatypes import HealthStatus
+
 from app.endpoints.health import (
     readiness_probe_get_method,
     liveness_probe_get_method,
     get_providers_health_statuses,
 )
 from models.responses import ProviderHealthStatus, ReadinessResponse
-from llama_stack.providers.datatypes import HealthStatus
 
 
 def test_readiness_probe_fails_due_to_unhealthy_providers(mocker):

tests/unit/app/endpoints/test_info.py

@@ -1,8 +1,10 @@
+"""Unit tests for the /info REST API endpoint."""
+
 from app.endpoints.info import info_endpoint_handler
 from configuration import AppConfig
 
 
-def test_info_endpoint(mocker):
+def test_info_endpoint():
     """Test the info endpoint handler."""
     config_dict = {
         "name": "foo",

tests/unit/app/endpoints/test_models.py

@@ -1,6 +1,9 @@
-import pytest
+"""Unit tests for the /models REST API endpoint."""
 
 from unittest.mock import Mock
+
+import pytest
+
 from fastapi import HTTPException, status
 
 from app.endpoints.models import models_endpoint_handler
@@ -62,7 +65,7 @@ def test_models_endpoint_handler_improper_llama_stack_configuration(mocker):
         assert e.detail["response"] == "LLama stack is not configured"
 
 
-def test_models_endpoint_handler_configuration_loaded(mocker):
+def test_models_endpoint_handler_configuration_loaded():
     """Test the models endpoint handler if configuration is loaded."""
     # configuration for tests
     config_dict = {

tests/unit/app/endpoints/test_query.py

@@ -1,7 +1,14 @@
+"""Unit tests for the /query REST API endpoint."""
+
+# pylint: disable=too-many-lines
+
 import json
 from fastapi import HTTPException, status
 import pytest
 
+from llama_stack_client import APIConnectionError
+from llama_stack_client.types import UserMessage  # type: ignore
+
 from configuration import AppConfig
 from app.endpoints.query import (
     query_endpoint_handler,
@@ -15,16 +22,15 @@
     get_agent,
     _agent_cache,
 )
-from llama_stack_client import APIConnectionError
+
 from models.requests import QueryRequest, Attachment
 from models.config import ModelContextProtocolServer
-from llama_stack_client.types import UserMessage  # type: ignore
 
 MOCK_AUTH = ("mock_user_id", "mock_username", "mock_token")
 
 
-@pytest.fixture(autouse=True)
-def setup_configuration():
+@pytest.fixture(name="setup_configuration")
+def setup_configuration_fixture():
     """Set up configuration for tests."""
     config_dict = {
         "name": "test",
@@ -52,12 +58,13 @@ def setup_configuration():
     return cfg
 
 
-@pytest.fixture(autouse=True)
-def prepare_agent_mocks(mocker):
+@pytest.fixture(autouse=True, name="prepare_agent_mocks")
+def prepare_agent_mocks_fixture(mocker):
+    """Fixture that yields mock agent when called."""
     mock_client = mocker.Mock()
     mock_agent = mocker.Mock()
-    """Cleanup agent cache after tests."""
     yield mock_client, mock_agent
+    # cleanup agent cache after tests
     _agent_cache.clear()
 
 
@@ -98,7 +105,7 @@ def test_is_transcripts_disabled(setup_configuration, mocker):
     assert is_transcripts_enabled() is False, "Transcripts should be disabled"
 
 
-def _test_query_endpoint_handler(mocker, store_transcript=False):
+def _test_query_endpoint_handler(mocker, store_transcript_to_file=False):
     """Test the query endpoint handler."""
     mock_client = mocker.Mock()
     mock_lsc = mocker.patch("client.LlamaStackClientHolder.get_client")
@@ -110,7 +117,7 @@ def _test_query_endpoint_handler(mocker, store_transcript=False):
 
     mock_config = mocker.Mock()
     mock_config.user_data_collection_configuration.transcripts_disabled = (
-        not store_transcript
+        not store_transcript_to_file
     )
     mocker.patch("app.endpoints.query.configuration", mock_config)
 
@@ -124,7 +131,8 @@ def _test_query_endpoint_handler(mocker, store_transcript=False):
     )
     mocker.patch("app.endpoints.query.select_model_id", return_value="fake_model_id")
     mocker.patch(
-        "app.endpoints.query.is_transcripts_enabled", return_value=store_transcript
+        "app.endpoints.query.is_transcripts_enabled",
+        return_value=store_transcript_to_file,
     )
     mock_transcript = mocker.patch("app.endpoints.query.store_transcript")
 
@@ -137,7 +145,7 @@ def _test_query_endpoint_handler(mocker, store_transcript=False):
     assert response.conversation_id == conversation_id
 
     # Assert the store_transcript function is called if transcripts are enabled
-    if store_transcript:
+    if store_transcript_to_file:
         mock_transcript.assert_called_once_with(
             user_id="user_id_placeholder",
             conversation_id=conversation_id,
@@ -155,12 +163,12 @@ def _test_query_endpoint_handler(mocker, store_transcript=False):
 
 def test_query_endpoint_handler_transcript_storage_disabled(mocker):
     """Test the query endpoint handler with transcript storage disabled."""
-    _test_query_endpoint_handler(mocker, store_transcript=False)
+    _test_query_endpoint_handler(mocker, store_transcript_to_file=False)
 
 
 def test_query_endpoint_handler_store_transcript(mocker):
     """Test the query endpoint handler with transcript storage enabled."""
-    _test_query_endpoint_handler(mocker, store_transcript=True)
+    _test_query_endpoint_handler(mocker, store_transcript_to_file=True)
 
 
 def test_select_model_id(mocker):
@@ -368,9 +376,17 @@ def test_retrieve_response_one_available_shield(prepare_agent_mocks, mocker):
     """Test the retrieve_response function."""
 
     class MockShield:
+        """Mock for Llama Stack shield to be used."""
+
         def __init__(self, identifier):
             self.identifier = identifier
 
+        def __str__(self):
+            return "MockShield"
+
+        def __repr__(self):
+            return "MockShield"
+
     mock_client, mock_agent = prepare_agent_mocks
     mock_agent.create_turn.return_value.output_message.content = "LLM answer"
     mock_client.shields.list.return_value = [MockShield("shield1")]
@@ -407,9 +423,17 @@ def test_retrieve_response_two_available_shields(prepare_agent_mocks, mocker):
     """Test the retrieve_response function."""
 
     class MockShield:
+        """Mock for Llama Stack shield to be used."""
+
         def __init__(self, identifier):
             self.identifier = identifier
 
+        def __str__(self):
+            return "MockShield"
+
+        def __repr__(self):
+            return "MockShield"
+
     mock_client, mock_agent = prepare_agent_mocks
     mock_agent.create_turn.return_value.output_message.content = "LLM answer"
     mock_client.shields.list.return_value = [
@@ -832,7 +856,7 @@ def test_store_transcript(mocker):
     )
 
 
-def test_get_rag_toolgroups(mocker):
+def test_get_rag_toolgroups():
     """Test get_rag_toolgroups function."""
     vector_db_ids = []
     result = get_rag_toolgroups(vector_db_ids)
@@ -864,7 +888,7 @@ def test_query_endpoint_handler_on_connection_error(mocker):
         query_endpoint_handler(query_request)
 
 
-def test_get_agent_cache_hit(prepare_agent_mocks, mocker):
+def test_get_agent_cache_hit(prepare_agent_mocks):
     """Test get_agent function when agent exists in cache."""
     mock_client, mock_agent = prepare_agent_mocks
 

tests/unit/app/endpoints/test_root.py

@@ -1,7 +1,9 @@
+"""Unit tests for the / endpoint handler."""
+
 from app.endpoints.root import root_endpoint_handler
 
 
-def test_root_endpoint(mocker):
+def test_root_endpoint():
     """Test the root endpoint handler."""
     request = None
     response = root_endpoint_handler(request)

tests/unit/app/endpoints/test_streaming_query.py

@@ -1,7 +1,16 @@
-import pytest
+"""Unit tests for the /streaming-query REST API endpoint."""
+
+# pylint: disable=too-many-lines
+
 import json
 
+import pytest
+
 from fastapi import HTTPException, status
+from fastapi.responses import StreamingResponse
+
+from llama_stack_client import APIConnectionError
+from llama_stack_client.types import UserMessage  # type: ignore
 from llama_stack_client.types.shared.interleaved_content_item import TextContentItem
 
 from configuration import AppConfig
@@ -13,10 +22,8 @@
     get_agent,
     _agent_cache,
 )
-from llama_stack_client import APIConnectionError
 from models.requests import QueryRequest, Attachment
 from models.config import ModelContextProtocolServer
-from llama_stack_client.types import UserMessage  # type: ignore
 
 MOCK_AUTH = ("mock_user_id", "mock_username", "mock_token")
 
@@ -47,8 +54,8 @@
 ]
 
 
-@pytest.fixture(autouse=True)
-def setup_configuration():
+@pytest.fixture(autouse=True, name="setup_configuration")
+def setup_configuration_fixture():
     """Set up configuration for tests."""
     config_dict = {
         "name": "test",
@@ -75,12 +82,13 @@ def setup_configuration():
     return cfg
 
 
-@pytest.fixture(autouse=True)
-def prepare_agent_mocks(mocker):
+@pytest.fixture(autouse=True, name="prepare_agent_mocks")
+def prepare_agent_mocks_fixture(mocker):
+    """Preparation for mock for the LLM agent."""
     mock_client = mocker.AsyncMock()
     mock_agent = mocker.AsyncMock()
-    """Cleanup agent cache after tests."""
     yield mock_client, mock_agent
+    # cleanup agent cache after tests
     _agent_cache.clear()
 
 
@@ -205,9 +213,7 @@ async def _test_streaming_query_endpoint_handler(mocker, store_transcript=False)
         None, query_request, auth=MOCK_AUTH
     )
 
-    # Assert the response is a StreamingResponse
-    from fastapi.responses import StreamingResponse
-
+    # assert the response is a StreamingResponse
     assert isinstance(response, StreamingResponse)
 
     # Collect the streaming response content
@@ -288,7 +294,8 @@ async def test_retrieve_response_vector_db_available(prepare_agent_mocks, mocker
         mock_client, model_id, query_request, token
     )
 
-    # For streaming, the response should be the streaming object and conversation_id should be returned
+    # For streaming, the response should be the streaming object and
+    # conversation_id should be returned
     assert response is not None
     assert conversation_id == "test_conversation_id"
     mock_agent.create_turn.assert_called_once_with(
@@ -324,7 +331,8 @@ async def test_retrieve_response_no_available_shields(prepare_agent_mocks, mocke
         mock_client, model_id, query_request, token
     )
 
-    # For streaming, the response should be the streaming object and conversation_id should be returned
+    # For streaming, the response should be the streaming object and
+    # conversation_id should be returned
     assert response is not None
     assert conversation_id == "test_conversation_id"
     mock_agent.create_turn.assert_called_once_with(
@@ -340,11 +348,16 @@ async def test_retrieve_response_one_available_shield(prepare_agent_mocks, mocke
     """Test the retrieve_response function."""
 
     class MockShield:
+        """Mock for Llama Stack shield to be used."""
+
         def __init__(self, identifier):
             self.identifier = identifier
 
-        def identifier(self):
-            return self.identifier
+        def __str__(self):
+            return "MockShield"
+
+        def __repr__(self):
+            return "MockShield"
 
     mock_client, mock_agent = prepare_agent_mocks
     mock_agent.create_turn.return_value.output_message.content = "LLM answer"
@@ -383,11 +396,16 @@ async def test_retrieve_response_two_available_shields(prepare_agent_mocks, mock
     """Test the retrieve_response function."""
 
     class MockShield:
+        """Mock for Llama Stack shield to be used."""
+
         def __init__(self, identifier):
             self.identifier = identifier
 
-        def identifier(self):
-            return self.identifier
+        def __str__(self):
+            return "MockShield"
+
+        def __repr__(self):
+            return "MockShield"
 
     mock_client, mock_agent = prepare_agent_mocks
     mock_agent.create_turn.return_value.output_message.content = "LLM answer"

tests/unit/app/test_routers.py

@@ -29,9 +29,11 @@ def include_router(self, router: Any, prefix: Optional[str] = None) -> None:
         self.routers.append((router, prefix))
 
     def get_routers(self) -> list[Any]:
+        """Retrieve all routers defined in mocked REST API."""
         return [r[0] for r in self.routers]
 
     def get_router_prefix(self, router: Any) -> Optional[str]:
+        """Retrieve router prefix configured for mocked REST API."""
         return list(filter(lambda r: r[0] == router, self.routers))[0][1]