Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 26 additions & 2 deletions tests/unit/models/config/test_authentication_configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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)
Expand Down
42 changes: 37 additions & 5 deletions tests/unit/models/config/test_byok_rag.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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".
"""
Comment on lines +69 to +75
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider standardizing exception documentation style.

The docstrings use varying phrasings and formatting for expected exceptions:

  • Line 69-75: "The raised ValidationError's message must contain"
  • Line 88-94: "Expects a pydantic.ValidationError" (with backticks)
  • Line 131-137: "Expects a pydantic.ValidationError" (no backticks)
  • Line 152-158: "Asserts that Pydantic validation fails"
  • Line 108-116 (existing): Uses structured "Raises:" section

For consistency and alignment with Python docstring conventions, consider adopting the structured "Raises:" format used in test_byok_rag_configuration_empty_rag_type (lines 108-116) across all validation tests.

📝 Example standardized format
 def test_byok_rag_configuration_wrong_dimension() -> None:
-    """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".
-    """
+    """Test the ByokRag constructor.
+
+    Verify constructing ByokRag with embedding_dimension less than or equal to
+    zero raises a ValidationError.
+
+    Raises:
+        ValidationError: if `embedding_dimension` is <= 0; error message
+        includes "should be greater than 0".
+    """

Also applies to: 88-94, 131-137, 152-158

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/unit/models/config/test_byok_rag.py` around lines 69 - 75, Standardize
the exception documentation in the ByokRag validation tests by replacing
informal phrasings with a structured "Raises:" section like the one in
test_byok_rag_configuration_empty_rag_type: for the ByokRag constructor test and
the other validation tests (the docstrings currently phrased as "The raised
ValidationError's message must contain", "Expects a `pydantic.ValidationError`",
"Expects a pydantic.ValidationError", and "Asserts that Pydantic validation
fails"), add a Raises: block that names pydantic.ValidationError and describes
the expected message substring (e.g., "should be greater than 0"); update the
docstrings for those test functions to match the same format and wording used in
test_byok_rag_configuration_empty_rag_type.

with pytest.raises(ValidationError, match="should be greater than 0"):
_ = ByokRag(
rag_id="rag_id",
Expand All @@ -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"
):
Expand Down Expand Up @@ -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"
):
Expand All @@ -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"
):
Expand Down
12 changes: 11 additions & 1 deletion tests/unit/models/config/test_conversation_history.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
21 changes: 19 additions & 2 deletions tests/unit/models/config/test_customization.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
11 changes: 10 additions & 1 deletion tests/unit/models/config/test_database_configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
8 changes: 7 additions & 1 deletion tests/unit/models/config/test_inference_configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
9 changes: 8 additions & 1 deletion tests/unit/models/config/test_jwt_role_rule.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
):
Expand Down
11 changes: 10 additions & 1 deletion tests/unit/models/config/test_llama_stack_configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
10 changes: 9 additions & 1 deletion tests/unit/models/config/test_model_context_protocol_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
16 changes: 14 additions & 2 deletions tests/unit/models/config/test_splunk_configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@

@pytest.fixture(name="token_file")
def token_file_fixture(tmp_path: Path) -> Path:
"""Create a temporary token file for testing."""
"""Create a temporary token file for testing.

Returns:
Path: Path to the created token file.
"""
token_file = tmp_path / "token"
token_file.write_text("test-token")
return token_file
Expand Down Expand Up @@ -70,7 +74,15 @@ def test_enabled_missing_required_fields(


def test_valid_enabled_configuration(token_file: Path) -> None:
"""Test valid enabled Splunk configuration passes validation."""
"""Test valid enabled Splunk configuration passes validation.

Verify that an enabled SplunkConfiguration with all required fields is
accepted and preserves the provided values.

Asserts that each field (enabled, url, token_path, index, source, timeout,
verify_ssl) on the created configuration equals the value passed to the
constructor.
"""
Comment on lines +77 to +85
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider maintaining consistent docstring verbosity within the file.

The expanded docstring provides detailed information but creates a notable verbosity inconsistency compared to other test functions in this file (lines 24, 36, 66, 106, 114, 122), which all use concise one-line docstrings. For better maintainability and readability, consider either:

  1. Condensing this to match the existing style: """Test valid enabled Splunk configuration passes validation and preserves provided values."""
  2. Expanding other test docstrings to a similar level of detail

Additionally, enumerating every assertion in the docstring may duplicate information already clear from the test code itself, potentially adding maintenance overhead when assertions change.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/unit/models/config/test_splunk_configuration.py` around lines 77 - 85,
The docstring for the test that verifies a valid enabled Splunk configuration is
overly verbose and inconsistent with other tests; replace it with a concise
one-line docstring (e.g., """Test valid enabled Splunk configuration passes
validation and preserves provided values.""") to match the file's style,
removing the enumerated assertions and extra detail so the test name and code
express the behavior instead; update the docstring in the test function that
constructs and asserts the SplunkConfiguration fields (the test that creates an
enabled SplunkConfiguration with url, token_path, index, source, timeout,
verify_ssl) accordingly.

cfg = SplunkConfiguration(
enabled=True,
url="https://splunk.example.com:8088",
Expand Down
10 changes: 9 additions & 1 deletion tests/unit/models/config/test_tls_configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,15 @@ def test_tls_configuration_wrong_key_path() -> None:


def test_tls_configuration_wrong_password_path() -> None:
"""Test the TLS configuration loading when some path is broken."""
"""Test the TLS configuration loading when some path is broken.

Verify that constructing a TLSConfiguration raises a ValueError when the
tls_key_password path does not point to a file.

Raises:
ValueError: with message "Path does not point to a file" if
`tls_key_password` is not a file.
"""
with pytest.raises(ValueError, match="Path does not point to a file"):
TLSConfiguration(
tls_certificate_path=Path("tests/configurationserver.crt"),
Expand Down
13 changes: 12 additions & 1 deletion tests/unit/models/config/test_user_data_collection.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,18 @@ def test_user_data_collection_feedback_enabled() -> None:


def test_user_data_collection_feedback_disabled() -> None:
"""Test the UserDataCollection constructor for feedback."""
"""Test the UserDataCollection constructor for feedback.

Verify the constructor raises a ValueError when feedback is enabled but no
storage is provided.

This test constructs UserDataCollection with feedback_enabled=True and
feedback_storage=None and asserts that a ValueError is raised with the
message "feedback_storage is required when feedback is enabled".

Raises:
ValueError: if feedback is enabled while feedback_storage is None.
"""
# incorrect configuration
with pytest.raises(
ValueError,
Expand Down
10 changes: 9 additions & 1 deletion tests/unit/models/requests/test_attachment.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,15 @@ def test_constructor(self) -> None:
assert a.content == "kind: Pod\n metadata:\n name: private-reg"

def test_constructor_unknown_attachment_type(self) -> None:
"""Test the Attachment with custom values."""
"""Test the Attachment with custom values.

Verify that an Attachment retains the provided attachment_type,
content_type, and content when given an unrecognized content type.

Asserts that `attachment_type` is "configuration", `content_type` is
"unknown/type", and `content` matches the supplied multi-line YAML
string.
"""
# for now we allow any content type
a = Attachment(
attachment_type="configuration",
Expand Down
7 changes: 6 additions & 1 deletion tests/unit/models/requests/test_feedback_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,12 @@ def test_constructor(self) -> None:
assert fr.user_feedback == "This is a great response!"

def test_check_invalid_uuid_format(self) -> None:
"""Test the UUID format check."""
"""Test the UUID format check.

Asserts that constructing a FeedbackRequest with a non-UUID
conversation_id raises a ValueError with message "Improper conversation
ID invalid-uuid".
"""
with pytest.raises(ValueError, match="Improper conversation ID invalid-uuid"):
FeedbackRequest(
conversation_id="invalid-uuid",
Expand Down
Loading
Loading