Merge pull request lightspeed-core#1457 from tisnik/lcore-1609-improved-docstrings-in-unit-tests

LCORE-1608: improved docstrings in unit tests

Author: tisnik

tests/unit/models/config/test_authentication_configuration.py

@@ -115,7 +115,17 @@ def test_authentication_configuration_rh_identity_one_entitlement() -> None:
 
 
 def test_authentication_configuration_rh_identity_more_entitlements() -> None:
-    """Test the AuthenticationConfiguration with RH identity token."""
+    """Test the AuthenticationConfiguration with RH identity token.
+
+    Verify AuthenticationConfiguration accepts an RH Identity configuration
+    with multiple required entitlements.
+
+    Asserts that the configuration's module is set to RH Identity, the provided
+    RHIdentityConfiguration is attached and returned by the
+    rh_identity_configuration property, the required_entitlements list is
+    preserved as ["foo", "bar", "baz"], and TLS/Kubernetes-related fields and
+    the skip_for_health_probes flag match the inputs.
+    """
     auth_config = AuthenticationConfiguration(
         module=AUTH_MOD_RH_IDENTITY,
         skip_tls_verification=False,
@@ -369,7 +379,15 @@ def test_authentication_configuration_skip_readiness_probe() -> None:
 
 
 def test_authentication_configuration_in_config_k8s() -> None:
-    """Test the authentication configuration in main config."""
+    """Test the authentication configuration in main config.
+
+    Verify K8S authentication settings are preserved when embedded in the main Configuration.
+
+    Asserts that the configuration's authentication section uses the K8S module
+    and that the following fields match the provided values:
+        `skip_tls_verification` is True, `k8s_ca_cert_path` equals
+        Path("tests/configuration/server.crt"), and `k8s_cluster_api` is None.
+    """
     # pylint: disable=no-member
     cfg = Configuration(
         name="test_name",
@@ -584,6 +602,12 @@ def test_rh_identity_max_header_size_validation(
 
     Verify that PositiveInt accepts valid custom values and rejects zero and
     negative values.
+
+    Parameters:
+        - max_header_size (int): The max header size to validate.
+        - expectation (AbstractContextManager): A context manager that asserts
+          either successful construction or that a ValidationError is raised
+          for invalid values.
     """
     with expectation:
         config = RHIdentityConfiguration(max_header_size=max_header_size)

tests/unit/models/config/test_byok_rag.py

@@ -15,7 +15,15 @@
 
 
 def test_byok_rag_configuration_default_values() -> None:
-    """Test the ByokRag constructor."""
+    """Test the ByokRag constructor.
+
+    Verify that ByokRag initializes correctly when only required fields are provided.
+
+    Asserts that the instance stores the given `rag_id`, `vector_db_id`, and
+    `db_path`, and that unspecified fields use the module's default values for
+    `rag_type`, `embedding_model`, `embedding_dimension`, and
+    `score_multiplier`.
+    """
     byok_rag = ByokRag(  # pyright: ignore[reportCallIssue]
         rag_id="rag_id",
         vector_db_id="vector_db_id",
@@ -58,7 +66,13 @@ def test_byok_rag_configuration_nondefault_values() -> None:
 
 
 def test_byok_rag_configuration_wrong_dimension() -> None:
-    """Test the ByokRag constructor."""
+    """Test the ByokRag constructor.
+
+    Verify constructing ByokRag with embedding_dimension less than or equal to
+    zero raises a ValidationError.
+
+    The raised ValidationError's message must contain "should be greater than 0".
+    """
     with pytest.raises(ValidationError, match="should be greater than 0"):
         _ = ByokRag(
             rag_id="rag_id",
@@ -71,7 +85,13 @@ def test_byok_rag_configuration_wrong_dimension() -> None:
 
 
 def test_byok_rag_configuration_empty_rag_id() -> None:
-    """Test the ByokRag constructor."""
+    """Test the ByokRag constructor.
+
+    Validate that constructing a ByokRag with an empty `rag_id` raises a validation error.
+
+    Expects a `pydantic.ValidationError` whose message contains "String should
+    have at least 1 character".
+    """
     with pytest.raises(
         ValidationError, match="String should have at least 1 character"
     ):
@@ -108,7 +128,13 @@ def test_byok_rag_configuration_empty_rag_type() -> None:
 
 
 def test_byok_rag_configuration_empty_embedding_model() -> None:
-    """Test the ByokRag constructor."""
+    """Test the ByokRag constructor.
+
+    Verify that constructing a ByokRag with an empty `embedding_model` raises a validation error.
+
+    Expects a pydantic.ValidationError whose message contains "String should
+    have at least 1 character".
+    """
     with pytest.raises(
         ValidationError, match="String should have at least 1 character"
     ):
@@ -123,7 +149,13 @@ def test_byok_rag_configuration_empty_embedding_model() -> None:
 
 
 def test_byok_rag_configuration_empty_vector_db_id() -> None:
-    """Test the ByokRag constructor."""
+    """Test the ByokRag constructor.
+
+    Ensure constructing a ByokRag with an empty `vector_db_id` raises a ValidationError.
+
+    Asserts that Pydantic validation fails with a message containing "String
+    should have at least 1 character".
+    """
     with pytest.raises(
         ValidationError, match="String should have at least 1 character"
     ):

tests/unit/models/config/test_conversation_history.py

@@ -120,7 +120,17 @@ def test_conversation_cache_no_type_but_configured(subtests: SubTests) -> None:
 
 
 def test_conversation_cache_multiple_configurations(subtests: SubTests) -> None:
-    """Test how multiple configurations are handled."""
+    """Test how multiple configurations are handled.
+
+    Verify that selecting a cache type while providing multiple backend
+    configurations raises validation errors.
+
+    Asserts that constructing ConversationHistoryConfiguration with more than
+    one backend config fails and produces the specific validation messages:
+    - For memory type: "Only memory cache config must be provided"
+    - For SQLite type: "Only SQLite cache config must be provided"
+    - For PostgreSQL type: "Only PostgreSQL cache config must be provided"
+    """
     d = PostgreSQLDatabaseConfiguration(
         db="db",
         user="user",

tests/unit/models/config/test_customization.py

@@ -58,13 +58,30 @@ def test_service_customization(subtests: SubTests) -> None:
 
 
 def test_service_customization_wrong_system_prompt_path() -> None:
-    """Check the service customization class."""
+    """Check the service customization class.
+
+    Verify that providing a non-existent `system_prompt_path` raises a `ValidationError`.
+
+    Asserts that constructing `Customization` with a path that does not point
+    to a file raises `pydantic.ValidationError` with a message matching "Path
+    does not point to a file".
+    """
     with pytest.raises(ValidationError, match="Path does not point to a file"):
         _ = Customization(system_prompt_path=Path("/path/does/not/exists"))
 
 
 def test_service_customization_correct_system_prompt_path(subtests: SubTests) -> None:
-    """Check the service customization class."""
+    """Check the service customization class.
+
+    Validate that Customization loads system_prompt from a provided file for
+    both single-line and multi-line prompts.
+
+    One subtest verifies that a one-line prompt file yields the exact string
+    "This is system prompt." Another subtest verifies that a multi-line prompt
+    file is loaded and contains the expected substrings: "You are OpenShift
+    Lightspeed", "Here are your instructions", and "Here are some basic facts
+    about OpenShift".
+    """
     with subtests.test(msg="One line system prompt"):
         # pass a file containing system prompt
         c = Customization(

tests/unit/models/config/test_database_configuration.py

@@ -43,7 +43,16 @@ def test_database_configuration(subtests: SubTests) -> None:
 
 
 def test_no_databases_configuration() -> None:
-    """Test if no databases configuration is checked."""
+    """Test if no databases configuration is checked.
+
+    Verify default database selection and error behavior when no database
+    configuration is present.
+
+    Asserts that a newly constructed DatabaseConfiguration defaults to SQLite
+    (db_type == "sqlite"), and that if both `sqlite` and `postgres` are set to
+    None, accessing the `db_type` or `config` properties raises a `ValueError`
+    with message "No database configuration found".
+    """
     d = DatabaseConfiguration()  # pyright: ignore[reportCallIssue]
     assert d is not None
 

tests/unit/models/config/test_inference_configuration.py

@@ -41,7 +41,13 @@ def test_inference_default_model_missing() -> None:
 
 def test_inference_default_provider_missing() -> None:
     """
-    Test case where only default model is set, should fail
+    Test case where only default model is set, should fail.
+
+    Checks that constructing InferenceConfiguration with only `default_model`
+    set raises a ValueError.
+
+    Asserts the error message equals "Default provider must be specified when
+    default model is set".
     """
     with pytest.raises(
         ValueError,

tests/unit/models/config/test_jwt_role_rule.py

@@ -72,7 +72,14 @@ def test_jwt_role_rule_no_roles_specified() -> None:
 
 
 def test_jwt_role_rule_star_role_specified() -> None:
-    """Check the JwtRoleRule config class."""
+    """Check the JwtRoleRule config class.
+
+    Asserts that specifying the wildcard '*' role in JwtRoleRule raises a ValidationError.
+
+    Instantiates JwtRoleRule with roles=['*'] and expects a ValidationError
+    whose message contains "The wildcard role is not allowed in role
+    rules".
+    """
     with pytest.raises(
         ValidationError, match="The wildcard '\\*' role is not allowed in role rules"
     ):

tests/unit/models/config/test_llama_stack_configuration.py

@@ -83,7 +83,16 @@ def test_llama_stack_wrong_configuration_constructor_library_mode_off() -> None:
 
 
 def test_llama_stack_wrong_configuration_no_config_file() -> None:
-    """Test the LlamaStackConfiguration constructor."""
+    """Test the LlamaStackConfiguration constructor.
+
+    Verify that enabling library-client mode without providing a configuration
+    file path raises a ValueError.
+
+    Asserts that constructing LlamaStackConfiguration with
+    use_as_library_client=True and no library_client_config_path raises a
+    ValueError whose message is "Llama stack library client mode is enabled but
+    a configuration file path is not specified".
+    """
     m = "Llama stack library client mode is enabled but a configuration file path is not specified"
     with pytest.raises(ValueError, match=m):
         LlamaStackConfiguration(

tests/unit/models/config/test_model_context_protocol_server.py

@@ -145,7 +145,15 @@ def test_configuration_multiple_mcp_servers() -> None:
 def test_model_context_protocol_server_with_authorization_headers(
     tmp_path: Path,
 ) -> None:
-    """Test ModelContextProtocolServer with authorization headers."""
+    """Test ModelContextProtocolServer with authorization headers.
+
+    Verify that authorization headers supplied as file paths are preserved and resolved.
+
+    Creates temporary files for header secrets, constructs a ModelContextProtocolServer with
+    authorization_headers mapping header names to those file paths, and asserts that:
+    - the model retains the original mapping to file paths, and
+    - resolved_authorization_headers contains the file contents for each header.
+    """
     auth_file = tmp_path / "auth.txt"
     auth_file.write_text("my-secret")
     api_key_file = tmp_path / "api_key.txt"

tests/unit/models/config/test_postgresql_database_configuration.py

@@ -32,7 +32,16 @@ def test_postgresql_database_configuration() -> None:
 
 
 def test_postgresql_database_configuration_namespace_specification() -> None:
-    """Test the PostgreSQLDatabaseConfiguration model."""
+    """Test the PostgreSQLDatabaseConfiguration model.
+
+    Verify that an explicit `namespace` is preserved and other fields use their defaults.
+
+    Asserts that providing `namespace="foo"` results in `namespace` set to
+    "foo", `host` defaulting to "localhost", `port` defaulting to 5432, `db`
+    and `user` preserved, `password` stored as a secret whose
+    `get_secret_value()` returns the original, `ssl_mode` and `gss_encmode`
+    matching their PostgreSQL defaults, and `ca_cert_path` being `None`.
+    """
     # pylint: disable=no-member
     c = PostgreSQLDatabaseConfiguration(
         db="db", user="user", password="password", namespace="foo"