From 7e6f78cd4d12b8bf64dd4b522dcf21bcfc05e810 Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Mon, 14 Jul 2025 07:59:48 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=93=9D=20Add=20docstrings=20to=20`lcore-3?= =?UTF-8?q?03-fixed-pylint-issues`?= 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/233#issuecomment-3068221248 The following files were modified: * `tests/unit/auth/test_noop_with_token.py` * `tests/unit/test_client.py` * `tests/unit/test_configuration.py` --- tests/unit/auth/test_noop_with_token.py | 16 +++- tests/unit/test_client.py | 29 +++++- tests/unit/test_configuration.py | 113 +++++++++++++++++++----- 3 files changed, 132 insertions(+), 26 deletions(-) diff --git a/tests/unit/auth/test_noop_with_token.py b/tests/unit/auth/test_noop_with_token.py index 784bdf03..3a97834a 100644 --- a/tests/unit/auth/test_noop_with_token.py +++ b/tests/unit/auth/test_noop_with_token.py @@ -55,7 +55,11 @@ async def test_noop_with_token_auth_dependency_custom_user_id(): async def test_noop_with_token_auth_dependency_no_token(): - """Test the NoopWithTokenAuthDependency class with no token.""" + """ + Test that NoopWithTokenAuthDependency raises an HTTPException when no Authorization header is present in the request. + + Asserts that the exception has a status code of 400 and the detail message "No Authorization header found". + """ dependency = NoopWithTokenAuthDependency() # Create a mock request without token @@ -69,14 +73,18 @@ async def test_noop_with_token_auth_dependency_no_token(): # Assert that an HTTPException is raised when no Authorization header is found with pytest.raises(HTTPException) as exc_info: - user_id, username, user_token = await dependency(request) + await dependency(request) assert exc_info.value.status_code == 400 assert exc_info.value.detail == "No Authorization header found" async def test_noop_with_token_auth_dependency_no_bearer(): - """Test the NoopWithTokenAuthDependency class with no token.""" + """ + Test that NoopWithTokenAuthDependency raises an HTTPException when the Authorization header does not contain a Bearer token. + + Asserts that the exception has a 400 status code and the detail message "No token found in Authorization header". + """ dependency = NoopWithTokenAuthDependency() # Create a mock request without token @@ -90,7 +98,7 @@ async def test_noop_with_token_auth_dependency_no_bearer(): # Assert that an HTTPException is raised when no Authorization header is found with pytest.raises(HTTPException) as exc_info: - user_id, username, user_token = await dependency(request) + await dependency(request) assert exc_info.value.status_code == 400 assert exc_info.value.detail == "No token found in Authorization header" diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py index 773b108e..23dab511 100644 --- a/tests/unit/test_client.py +++ b/tests/unit/test_client.py @@ -6,8 +6,10 @@ from models.config import LLamaStackConfiguration -# [tisnik] Need to resolve dependencies on CI to be able to run this tests def test_get_llama_stack_library_client() -> None: + """ + Tests initialization of the Llama Stack client in library client mode using a valid configuration file. + """ cfg = LLamaStackConfiguration( url=None, api_key=None, @@ -20,6 +22,11 @@ def test_get_llama_stack_library_client() -> None: def test_get_llama_stack_remote_client() -> None: + """ + Test initialization of Llama Stack in remote (server) client mode. + + Verifies that a Llama Stack client can be successfully created using a configuration with `use_as_library_client` set to False and a valid server URL. + """ cfg = LLamaStackConfiguration( url="http://localhost:8321", api_key=None, @@ -32,6 +39,11 @@ def test_get_llama_stack_remote_client() -> None: def test_get_llama_stack_wrong_configuration() -> None: + """ + Test that initializing a Llama Stack client in library mode without a configuration path raises an exception. + + Verifies that an exception with the expected error message is raised if `library_client_config_path` is not set when `use_as_library_client` is True. + """ cfg = LLamaStackConfiguration( url=None, api_key=None, @@ -48,6 +60,11 @@ def test_get_llama_stack_wrong_configuration() -> None: async def test_get_async_llama_stack_library_client() -> None: + """ + Test asynchronous initialization of a Llama Stack client in library mode. + + Verifies that the AsyncLlamaStackClientHolder can be successfully loaded with a configuration specifying library client mode and a valid configuration path. + """ cfg = LLamaStackConfiguration( url=None, api_key=None, @@ -60,6 +77,11 @@ async def test_get_async_llama_stack_library_client() -> None: async def test_get_async_llama_stack_remote_client() -> None: + """ + Test asynchronous initialization of a Llama Stack client in remote (server) mode. + + Verifies that the AsyncLlamaStackClientHolder can be successfully loaded with a configuration specifying remote server mode. + """ cfg = LLamaStackConfiguration( url="http://localhost:8321", api_key=None, @@ -72,6 +94,11 @@ async def test_get_async_llama_stack_remote_client() -> None: async def test_get_async_llama_stack_wrong_configuration() -> None: + """ + Test that initializing AsyncLlamaStackClientHolder with missing library_client_config_path raises an exception. + + Verifies that an exception with the expected error message is raised when attempting to load the client in library client mode without specifying the required configuration path. + """ cfg = LLamaStackConfiguration( url=None, api_key=None, diff --git a/tests/unit/test_configuration.py b/tests/unit/test_configuration.py index 55f6892b..249a8f08 100644 --- a/tests/unit/test_configuration.py +++ b/tests/unit/test_configuration.py @@ -6,30 +6,55 @@ def test_default_configuration() -> None: + """ + Verify that accessing any configuration-related property on an uninitialized AppConfig instance raises an exception indicating the configuration is not loaded. + """ cfg = AppConfig() assert cfg is not None # configuration is not loaded with pytest.raises(Exception, match="logic error: configuration is not loaded"): # try to read property - cfg.configuration + cfg.configuration # pylint: disable=pointless-statement with pytest.raises(Exception, match="logic error: configuration is not loaded"): # try to read property - cfg.llama_stack_configuration + cfg.service_configuration # pylint: disable=pointless-statement with pytest.raises(Exception, match="logic error: configuration is not loaded"): # try to read property - cfg.user_data_collection_configuration + cfg.llama_stack_configuration # pylint: disable=pointless-statement + + with pytest.raises(Exception, match="logic error: configuration is not loaded"): + # try to read property + cfg.user_data_collection_configuration # pylint: disable=pointless-statement + + with pytest.raises(Exception, match="logic error: configuration is not loaded"): + # try to read property + cfg.mcp_servers # pylint: disable=pointless-statement + + with pytest.raises(Exception, match="logic error: configuration is not loaded"): + # try to read property + cfg.authentication_configuration # pylint: disable=pointless-statement + + with pytest.raises(Exception, match="logic error: configuration is not loaded"): + # try to read property + cfg.customization # pylint: disable=pointless-statement def test_configuration_is_singleton() -> None: + """ + Verify that multiple instances of AppConfig refer to the same singleton configuration object. + """ cfg1 = AppConfig() cfg2 = AppConfig() assert cfg1 == cfg2 def test_init_from_dict() -> None: + """ + Verify that initializing AppConfig from a dictionary correctly sets all configuration subsections and their attributes. + """ config_dict = { "name": "foo", "service": { @@ -126,8 +151,13 @@ def test_init_from_dict_with_mcp_servers() -> None: def test_load_proper_configuration(tmpdir) -> None: + """ + Test that a valid YAML configuration file can be loaded and all configuration subsections are accessible. + + Creates a sample configuration file, loads it using `AppConfig`, and asserts that the main configuration and its subsections are properly initialized. + """ cfg_filename = tmpdir / "config.yaml" - with open(cfg_filename, "w") as fout: + with open(cfg_filename, "w", encoding="utf-8") as fout: fout.write( """ name: foo bar baz @@ -157,9 +187,11 @@ def test_load_proper_configuration(tmpdir) -> None: def test_load_configuration_with_mcp_servers(tmpdir) -> None: - """Test loading configuration from YAML file with MCP servers.""" + """ + Test that loading a YAML configuration file with multiple MCP servers correctly initializes the `mcp_servers` property, including handling of default and custom `provider_id` values. + """ cfg_filename = tmpdir / "config.yaml" - with open(cfg_filename, "w") as fout: + with open(cfg_filename, "w", encoding="utf-8") as fout: fout.write( """ name: test service @@ -268,58 +300,95 @@ def test_mcp_servers_property_with_servers() -> None: def test_configuration_not_loaded(): - """Test that accessing configuration before loading raises an error.""" + """ + Test that accessing the `configuration` property on an uninitialized `AppConfig` instance raises an AssertionError with the expected message. + """ cfg = AppConfig() with pytest.raises( AssertionError, match="logic error: configuration is not loaded" ): - cfg.configuration + cfg.configuration # pylint: disable=pointless-statement def test_service_configuration_not_loaded(): - """Test that accessing service_configuration before loading raises an error.""" + """ + Test that accessing the service_configuration property on an uninitialized AppConfig instance raises an AssertionError with the expected message. + """ cfg = AppConfig() with pytest.raises( AssertionError, match="logic error: configuration is not loaded" ): - cfg.service_configuration + cfg.service_configuration # pylint: disable=pointless-statement def test_llama_stack_configuration_not_loaded(): - """Test that accessing llama_stack_configuration before loading raises an error.""" + """ + Test that accessing the llama_stack_configuration property on an uninitialized AppConfig instance raises an AssertionError with the expected message. + """ cfg = AppConfig() with pytest.raises( AssertionError, match="logic error: configuration is not loaded" ): - cfg.llama_stack_configuration + cfg.llama_stack_configuration # pylint: disable=pointless-statement def test_user_data_collection_configuration_not_loaded(): - """Test that accessing user_data_collection_configuration before loading raises an error.""" + """ + Test that accessing the user data collection configuration property before loading raises an AssertionError with the expected message. + """ cfg = AppConfig() with pytest.raises( AssertionError, match="logic error: configuration is not loaded" ): - cfg.user_data_collection_configuration + cfg.user_data_collection_configuration # pylint: disable=pointless-statement def test_mcp_servers_not_loaded(): - """Test that accessing mcp_servers before loading raises an error.""" + """ + Test that accessing the `mcp_servers` property on an uninitialized `AppConfig` instance raises an AssertionError with the expected message. + """ + cfg = AppConfig() + with pytest.raises( + AssertionError, match="logic error: configuration is not loaded" + ): + cfg.mcp_servers # pylint: disable=pointless-statement + + +def test_authentication_configuration_not_loaded(): + """ + Test that accessing the authentication_configuration property on an uninitialized AppConfig instance raises an AssertionError with the expected message. + """ + cfg = AppConfig() + with pytest.raises( + AssertionError, match="logic error: configuration is not loaded" + ): + cfg.authentication_configuration # pylint: disable=pointless-statement + + +def test_customization_not_loaded(): + """ + Test that accessing the `customization` property on an uninitialized `AppConfig` instance raises an AssertionError with the expected message. + """ cfg = AppConfig() with pytest.raises( AssertionError, match="logic error: configuration is not loaded" ): - cfg.mcp_servers + cfg.customization # pylint: disable=pointless-statement def test_load_configuration_with_customization_system_prompt_path(tmpdir) -> None: - """Test loading configuration from YAML file with customization.""" + """ + Test that loading a YAML configuration file with a customization section specifying a system prompt file path correctly loads the system prompt content from the referenced file. + + Parameters: + tmpdir: Temporary directory fixture provided by pytest for file operations. + """ system_prompt_filename = tmpdir / "system_prompt.txt" - with open(system_prompt_filename, "w") as fout: + with open(system_prompt_filename, "w", encoding="utf-8") as fout: fout.write("this is system prompt") cfg_filename = tmpdir / "config.yaml" - with open(cfg_filename, "w") as fout: + with open(cfg_filename, "w", encoding="utf-8") as fout: fout.write( f""" name: test service @@ -357,9 +426,11 @@ def test_load_configuration_with_customization_system_prompt_path(tmpdir) -> Non def test_load_configuration_with_customization_system_prompt(tmpdir) -> None: - """Test loading configuration from YAML file with system_prompt in the customization.""" + """ + Test that loading a YAML configuration file with an inline `system_prompt` in the customization section correctly sets the `system_prompt` property of the loaded configuration. + """ cfg_filename = tmpdir / "config.yaml" - with open(cfg_filename, "w") as fout: + with open(cfg_filename, "w", encoding="utf-8") as fout: fout.write( """ name: test service