From 2183e022551a080fea80a4b208c51147115e60fa Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Tue, 22 Jul 2025 07:55:33 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=93=9D=20Add=20docstrings=20to=20`lcore-3?= =?UTF-8?q?24-add-check-for-config-file`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Docstrings generation was requested by @tisnik. * https://github.com/lightspeed-core/lightspeed-stack/pull/266#issuecomment-3101460024 The following files were modified: * `src/models/config.py` * `tests/unit/models/test_config.py` * `tests/unit/utils/test_common.py` --- src/models/config.py | 9 +++++- tests/unit/models/test_config.py | 47 +++++++++++++++++++++++++------- tests/unit/utils/test_common.py | 6 +++- 3 files changed, 50 insertions(+), 12 deletions(-) diff --git a/src/models/config.py b/src/models/config.py index 31cd1551..08049523 100644 --- a/src/models/config.py +++ b/src/models/config.py @@ -65,7 +65,14 @@ class LlamaStackConfiguration(BaseModel): @model_validator(mode="after") def check_llama_stack_model(self) -> Self: - """Check Llama stack configuration.""" + """ + Validates the Llama stack configuration after model initialization. + + Ensures that either a URL is provided or library client mode is explicitly enabled. If library client mode is enabled, verifies that a configuration file path is specified and points to an existing, readable file. Raises a ValueError if any required condition is not met. + + Returns: + Self: The validated LlamaStackConfiguration instance. + """ if self.url is None: if self.use_as_library_client is None: raise ValueError( diff --git a/tests/unit/models/test_config.py b/tests/unit/models/test_config.py index 1f25d38f..eb090489 100644 --- a/tests/unit/models/test_config.py +++ b/tests/unit/models/test_config.py @@ -29,7 +29,9 @@ def test_service_configuration_constructor() -> None: - """Test the ServiceConfiguration constructor.""" + """ + Verify that the ServiceConfiguration constructor sets default values for all fields. + """ s = ServiceConfiguration() assert s is not None @@ -58,7 +60,9 @@ def test_service_configuration_workers_value() -> None: def test_llama_stack_configuration_constructor() -> None: - """Test the LLamaStackConfiguration constructor.""" + """ + Verify that the LlamaStackConfiguration constructor accepts valid combinations of parameters and creates instances successfully. + """ llama_stack_configuration = LlamaStackConfiguration( use_as_library_client=True, library_client_config_path="tests/configuration/run.yaml", @@ -80,7 +84,9 @@ def test_llama_stack_configuration_constructor() -> None: def test_llama_stack_configuration_no_run_yaml() -> None: - """Test the LLamaStackConfiguration constructor when run.yaml file is not a file.""" + """ + Verify that constructing a LlamaStackConfiguration with a non-existent or invalid library_client_config_path raises InvalidConfigurationError. + """ with pytest.raises( InvalidConfigurationError, match="Llama Stack configuration file 'not a file' is not a file", @@ -92,7 +98,9 @@ def test_llama_stack_configuration_no_run_yaml() -> None: def test_llama_stack_wrong_configuration_constructor_no_url() -> None: - """Test the LLamaStackConfiguration constructor.""" + """ + Verify that constructing a LlamaStackConfiguration without specifying either a URL or enabling library client mode raises a ValueError. + """ with pytest.raises( ValueError, match="LLama stack URL is not specified and library client mode is not specified", @@ -308,7 +316,11 @@ def test_model_context_protocol_server_required_fields() -> None: def test_configuration_empty_mcp_servers() -> None: - """Test Configuration with empty MCP servers list.""" + """ + Test that a Configuration object can be created with an empty list of MCP servers. + + Verifies that the Configuration instance is constructed successfully and that the mcp_servers attribute is empty. + """ cfg = Configuration( name="test_name", service=ServiceConfiguration(), @@ -327,7 +339,9 @@ def test_configuration_empty_mcp_servers() -> None: def test_configuration_single_mcp_server() -> None: - """Test Configuration with a single MCP server.""" + """ + Test that a Configuration object can be created with a single MCP server and verifies its properties. + """ mcp_server = ModelContextProtocolServer( name="test-server", url="http://localhost:8080" ) @@ -351,7 +365,9 @@ def test_configuration_single_mcp_server() -> None: def test_configuration_multiple_mcp_servers() -> None: - """Test Configuration with multiple MCP servers.""" + """ + Verify that the Configuration object correctly handles multiple ModelContextProtocolServer instances in its mcp_servers list, including custom provider IDs. + """ mcp_servers = [ ModelContextProtocolServer(name="server1", url="http://localhost:8080"), ModelContextProtocolServer( @@ -381,7 +397,9 @@ def test_configuration_multiple_mcp_servers() -> None: def test_dump_configuration(tmp_path) -> None: - """Test the ability to dump configuration.""" + """ + Test that the Configuration object can be serialized to a JSON file and that the resulting file contains all expected sections and values. + """ cfg = Configuration( name="test_name", service=ServiceConfiguration(), @@ -461,7 +479,12 @@ def test_dump_configuration(tmp_path) -> None: def test_dump_configuration_with_one_mcp_server(tmp_path) -> None: - """Test the ability to dump configuration with one MCP server configured.""" + """ + Verify that a configuration with a single MCP server can be serialized to JSON and that all expected fields and values are present in the output. + + Parameters: + tmp_path: Temporary directory path provided by pytest for file output. + """ mcp_servers = [ ModelContextProtocolServer(name="test-server", url="http://localhost:8080"), ] @@ -545,7 +568,11 @@ def test_dump_configuration_with_one_mcp_server(tmp_path) -> None: def test_dump_configuration_with_more_mcp_servers(tmp_path) -> None: - """Test the ability to dump configuration with more MCP servers configured.""" + """ + Test that a configuration with multiple MCP servers can be serialized to JSON and that all server entries are correctly included in the output. + + Verifies that the dumped configuration file contains all expected fields and that each MCP server is present with the correct name, URL, and provider ID. + """ mcp_servers = [ ModelContextProtocolServer(name="test-server-1", url="http://localhost:8081"), ModelContextProtocolServer(name="test-server-2", url="http://localhost:8082"), diff --git a/tests/unit/utils/test_common.py b/tests/unit/utils/test_common.py index 145e4314..cbb436ba 100644 --- a/tests/unit/utils/test_common.py +++ b/tests/unit/utils/test_common.py @@ -244,7 +244,11 @@ async def test_register_mcp_servers_with_custom_provider(mocker): @pytest.mark.asyncio async def test_register_mcp_servers_async_with_library_client(mocker): - """Test register_mcp_servers_async with library client configuration.""" + """ + Test that `register_mcp_servers_async` correctly registers MCP servers when using the library client configuration. + + This test verifies that the function initializes the async client, checks for existing toolgroups, and registers new MCP servers as needed when the configuration specifies the use of a library client. + """ # Mock the logger mock_logger = Mock(spec=Logger)