Skip to content
Merged
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
106 changes: 102 additions & 4 deletions tests/unit/models/config/test_postgresql_database_configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,12 @@ def test_postgresql_database_configuration() -> None:
"""Test the PostgreSQLDatabaseConfiguration model."""
# pylint: disable=no-member
c = PostgreSQLDatabaseConfiguration(
db="db", user="user", password="password"
db="db",
user="user",
password="password",
) # pyright: ignore[reportCallIssue]

# most attributes are set to default values
assert c is not None
assert c.host == "localhost"
assert c.port == 5432
Expand All @@ -31,6 +35,52 @@ def test_postgresql_database_configuration() -> None:
assert c.ca_cert_path is None


def test_postgresql_database_configuration_missing_values(subtests: SubTests) -> None:
"""Test the PostgreSQLDatabaseConfiguration model."""
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 | 💤 Low value

Improve docstring specificity.

The docstring is generic and doesn't describe what this test validates. Consider making it more descriptive.

📝 Suggested improvement
-    """Test the PostgreSQLDatabaseConfiguration model."""
+    """Test that PostgreSQLDatabaseConfiguration raises validation errors for missing required fields."""
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"""Test the PostgreSQLDatabaseConfiguration model."""
"""Test that PostgreSQLDatabaseConfiguration raises validation errors for missing required fields."""
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unit/models/config/test_postgresql_database_configuration.py` at line
39, Update the module docstring for the tests in
tests/unit/models/config/test_postgresql_database_configuration.py so it
specifically states what the test suite validates for the
PostgreSQLDatabaseConfiguration model (e.g., that it correctly parses connection
parameters, applies defaults, and raises validation errors for invalid inputs);
replace the generic """Test the PostgreSQLDatabaseConfiguration model.""" with a
brief sentence describing the exact behaviors covered by the tests (mentioning
PostgreSQLDatabaseConfiguration by name to make it clear).

with subtests.test(msg="Missing 'db' attribute"):
with pytest.raises(ValueError, match="Field required"):
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.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check the Pydantic version and verify ValidationError behavior

# Find Pydantic version in requirements or pyproject.toml
rg -n "pydantic" -g "requirements*.txt" -g "pyproject.toml" -g "setup.py" -C2

# Look for other tests that validate Pydantic required field errors to confirm pattern
rg -nP 'pytest\.raises\((ValueError|ValidationError).*match=.*[Ff]ield\s+required' -g 'tests/**/*.py' -C2

Repository: lightspeed-core/lightspeed-stack

Length of output: 5774


🏁 Script executed:

# Find and examine the PostgreSQLDatabaseConfiguration model
find . -type f -name "*.py" -path "*/models/config*" | grep -v __pycache__ | grep -v test

# Check the test file imports
head -20 tests/unit/models/config/test_postgresql_database_configuration.py

Repository: lightspeed-core/lightspeed-stack

Length of output: 628


🏁 Script executed:

# Examine the PostgreSQLDatabaseConfiguration model
grep -n "class PostgreSQLDatabaseConfiguration" src/models/config.py -A 30

Repository: lightspeed-core/lightspeed-stack

Length of output: 1204


🏁 Script executed:

# Check the ConfigurationBase class definition
grep -n "class ConfigurationBase" src/models/config.py -A 5

# Check imports at the top of the config.py file
head -30 src/models/config.py

Repository: lightspeed-core/lightspeed-stack

Length of output: 849


🏁 Script executed:

# Let's check if there's any custom exception handling or if ValueError is somehow mapped
grep -n "ValueError" src/models/config.py | head -20

# Check what the actual error message format is in test_conversation_history.py for comparison
grep -n "ValidationError" tests/unit/models/config/test_conversation_history.py -B2 | head -30

Repository: lightspeed-core/lightspeed-stack

Length of output: 2309


Use ValidationError instead of ValueError for Pydantic validation failures.

Pydantic v2 raises ValidationError for model validation failures (required fields, type validation, etc.), not ValueError. The test file already imports ValidationError from pydantic but incorrectly uses ValueError in pytest.raises(). Change all instances on lines 41, 48, 55, 62, 68, 74, and 80 to use pytest.raises(ValidationError, match="Field required") to match Pydantic v2 behavior and be consistent with other tests like test_conversation_history.py.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unit/models/config/test_postgresql_database_configuration.py` at line
41, Replace the incorrect ValueError usage in the pytest.raises assertions with
Pydantic's ValidationError: update each pytest.raises(ValueError, match="Field
required") call in the test_postgresql_database_configuration tests (the
occurrences at the pytest.raises calls around lines where the assertions run) to
pytest.raises(ValidationError, match="Field required"), ensuring the imported
ValidationError from pydantic is used so the tests reflect Pydantic v2 behavior.

PostgreSQLDatabaseConfiguration(
user="user",
password="password",
) # pyright: ignore[reportCallIssue]

with subtests.test(msg="Missing 'user' attribute"):
with pytest.raises(ValueError, match="Field required"):
PostgreSQLDatabaseConfiguration(
db="db",
password="password",
) # pyright: ignore[reportCallIssue]

with subtests.test(msg="Missing 'password' attribute"):
with pytest.raises(ValueError, match="Field required"):
PostgreSQLDatabaseConfiguration(
db="db",
user="user",
) # pyright: ignore[reportCallIssue]

with subtests.test(msg="Missing 'db' and 'user' attributes"):
with pytest.raises(ValueError, match="Field required"):
PostgreSQLDatabaseConfiguration(
password="password",
) # pyright: ignore[reportCallIssue]

with subtests.test(msg="Missing 'id' and 'user' attributes"):
with pytest.raises(ValueError, match="Field required"):
PostgreSQLDatabaseConfiguration(
password="password",
) # pyright: ignore[reportCallIssue]
Comment on lines +67 to +71
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.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Duplicate test case with incorrect message.

This subtest is identical to lines 61-65 (both only pass password), and the message incorrectly references a non-existent 'id' field. This appears to be a copy-paste error.

🔧 Proposed fix: Remove duplicate or correct to test missing 'db' and 'password'

If you intended to test missing 'db' and 'password' (passing only user):

-    with subtests.test(msg="Missing 'id' and 'user' attributes"):
+    with subtests.test(msg="Missing 'db' and 'password' attributes"):
         with pytest.raises(ValueError, match="Field required"):
             PostgreSQLDatabaseConfiguration(
-                password="password",
+                user="user",
             )  # pyright: ignore[reportCallIssue]

Otherwise, remove lines 67-71 entirely as they duplicate lines 61-65.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
with subtests.test(msg="Missing 'id' and 'user' attributes"):
with pytest.raises(ValueError, match="Field required"):
PostgreSQLDatabaseConfiguration(
password="password",
) # pyright: ignore[reportCallIssue]
with subtests.test(msg="Missing 'db' and 'password' attributes"):
with pytest.raises(ValueError, match="Field required"):
PostgreSQLDatabaseConfiguration(
user="user",
) # pyright: ignore[reportCallIssue]
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unit/models/config/test_postgresql_database_configuration.py` around
lines 67 - 71, The subtest that instantiates PostgreSQLDatabaseConfiguration
with only password is a duplicate and has the wrong message; either remove this
duplicate subtest block (the one with msg "Missing 'id' and 'user' attributes")
or change it to test the intended case by passing only user (e.g.,
PostgreSQLDatabaseConfiguration(user="user")) and update the subtest message to
"Missing 'db' and 'password' attributes" and the pytest.raises assertion
accordingly so the test verifies the correct missing-field behavior for
PostgreSQLDatabaseConfiguration.


with subtests.test(msg="Missing 'user' and 'password' attributes"):
with pytest.raises(ValueError, match="Field required"):
PostgreSQLDatabaseConfiguration(
db="db",
) # pyright: ignore[reportCallIssue]

with subtests.test(msg="Missing all required attributes"):
with pytest.raises(ValueError, match="Field required"):
PostgreSQLDatabaseConfiguration() # pyright: ignore[reportCallIssue]


def test_postgresql_database_configuration_namespace_specification() -> None:
"""Test the PostgreSQLDatabaseConfiguration model.

Expand All @@ -46,6 +96,8 @@ def test_postgresql_database_configuration_namespace_specification() -> None:
c = PostgreSQLDatabaseConfiguration(
db="db", user="user", password="password", namespace="foo"
) # pyright: ignore[reportCallIssue]

# most attributes are set to default values
assert c is not None
assert c.host == "localhost"
assert c.port == 5432
Expand All @@ -70,21 +122,57 @@ def test_postgresql_database_configuration_port_setting(subtests: SubTests) -> N
"""
with subtests.test(msg="Correct port value"):
c = PostgreSQLDatabaseConfiguration(
db="db", user="user", password="password", port=1234
db="db",
user="user",
password="password",
port=1234,
) # pyright: ignore[reportCallIssue]
assert c is not None
assert c.port == 1234

with subtests.test(msg="Negative port value"):
with pytest.raises(ValidationError, match="Input should be greater than 0"):
PostgreSQLDatabaseConfiguration(
db="db", user="user", password="password", port=-1
db="db",
user="user",
password="password",
port=-1,
) # pyright: ignore[reportCallIssue]

with subtests.test(msg="Zero port value"):
with pytest.raises(ValidationError, match="Input should be greater than 0"):
PostgreSQLDatabaseConfiguration(
db="db",
user="user",
password="password",
port=0,
) # pyright: ignore[reportCallIssue]
Comment on lines +142 to 149
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.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Verify Pydantic validation error types for port field

# Search for the PostgreSQLDatabaseConfiguration model definition to check field validators
rg -n "class PostgreSQLDatabaseConfiguration" -A 30 -g "*.py"

# Look for field_validator or validator decorators on port field
rg -n "port.*=.*Field|@.*validator.*port|`@field_validator`.*port" -g "*.py" -C3

Repository: lightspeed-core/lightspeed-stack

Length of output: 2540


🏁 Script executed:

# Read the test file to see the complete test cases for port validation
head -n 180 tests/unit/models/config/test_postgresql_database_configuration.py | tail -n 50

Repository: lightspeed-core/lightspeed-stack

Length of output: 1985


🏁 Script executed:

# Also check if there are imports related to validation errors
rg -n "import.*ValidationError|from.*ValidationError" tests/unit/models/config/test_postgresql_database_configuration.py

Repository: lightspeed-core/lightspeed-stack

Length of output: 115


🏁 Script executed:

# Check the PositiveInt type and how Pydantic v2 handles validation
rg -n "PositiveInt" -B2 -A2 src/models/config.py

Repository: lightspeed-core/lightspeed-stack

Length of output: 1934


🏁 Script executed:

# Search for custom validators on port field in the config model
rg -n "`@field_validator`|`@validator`" src/models/config.py -A 10 | grep -A 10 "port"

Repository: lightspeed-core/lightspeed-stack

Length of output: 58


🏁 Script executed:

# Check if there are field validators defined for the PostgreSQL config class
ast-grep --pattern 'class PostgreSQLDatabaseConfiguration {
  $$$
  `@field_validator`($_)
  $$$
}'

Repository: lightspeed-core/lightspeed-stack

Length of output: 58


🏁 Script executed:

# Look for any validation customization in the entire config module
rg -n "field_validator|model_validator" src/models/config.py | head -20

Repository: lightspeed-core/lightspeed-stack

Length of output: 830


🏁 Script executed:

# Get the full model_validator at line 283 area to see if it's for PostgreSQL config
sed -n '250,290p' src/models/config.py

Repository: lightspeed-core/lightspeed-stack

Length of output: 1430


🏁 Script executed:

# Search for port-specific validation logic after the PostgreSQL config class definition
sed -n '181,250p' src/models/config.py

Repository: lightspeed-core/lightspeed-stack

Length of output: 2069


🏁 Script executed:

# Verify Pydantic v2 behavior by checking what version is being used
rg -n "pydantic" pyproject.toml

Repository: lightspeed-core/lightspeed-stack

Length of output: 104


🏁 Script executed:

# Check the actual error message patterns expected in test to understand if they match Pydantic v2
cat tests/unit/models/config/test_postgresql_database_configuration.py | grep -A 8 "Non integer port value\|Null port value"

Repository: lightspeed-core/lightspeed-stack

Length of output: 770


Fix error type expectations for port type validation failures.

The subtests for port="xyzzy" and port=None expect ValueError, but Pydantic v2 raises ValidationError for type coercion failures on PositiveInt fields. Only the custom validator (port > 65535) should raise ValueError. Update lines 160-176 to expect ValidationError instead:

Error type corrections needed
with subtests.test(msg="Non integer port value"):
    with pytest.raises(ValidationError, match="Input should be a valid integer"):
        PostgreSQLDatabaseConfiguration(
            db="db",
            user="user",
            password="password",
            port="xyzzy",
        )

with subtests.test(msg="Null port value"):
    with pytest.raises(ValidationError, match="Input should be a valid integer"):
        PostgreSQLDatabaseConfiguration(
            db="db",
            user="user",
            password="password",
            port=None,
        )
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unit/models/config/test_postgresql_database_configuration.py` around
lines 142 - 149, The tests currently expect ValueError for non-integer and null
port inputs but Pydantic v2 raises ValidationError for type/coercion failures on
the PositiveInt field; update the two subtests that construct
PostgreSQLDatabaseConfiguration with port="xyzzy" and port=None to assert
pytest.raises(ValidationError, match="Input should be a valid integer") instead
of expecting ValueError so the tests check for ValidationError produced by the
model validation rather than the custom port range validator.


with subtests.test(msg="Too big port value"):
with pytest.raises(ValueError, match="Port value should be less than 65536"):
PostgreSQLDatabaseConfiguration(
db="db", user="user", password="password", port=100000
db="db",
user="user",
password="password",
port=100000,
) # pyright: ignore[reportCallIssue]

with subtests.test(msg="Non integer port value"):
with pytest.raises(ValueError, match="Input should be a valid integer"):
PostgreSQLDatabaseConfiguration(
db="db",
user="user",
password="password",
port="xyzzy",
) # pyright: ignore[reportCallIssue]

with subtests.test(msg="Null port value"):
with pytest.raises(ValueError, match="Input should be a valid integer"):
PostgreSQLDatabaseConfiguration(
db="db",
user="user",
password="password",
port=None,
) # pyright: ignore[reportCallIssue]


Expand Down Expand Up @@ -121,3 +209,13 @@ def test_postgresql_database_configuration_ca_cert_path(subtests: SubTests) -> N
port=1234,
ca_cert_path=Path("not a file"),
) # pyright: ignore[reportCallIssue]

with subtests.test(msg="Path is empty"):
with pytest.raises(ValidationError, match="Path does not point to a file"):
PostgreSQLDatabaseConfiguration(
db="db",
user="user",
password="password",
port=1234,
ca_cert_path=Path(""),
) # pyright: ignore[reportCallIssue]
Loading