LCORE-2264: Unit tests for checking the model for a class PostgreSQLDatabaseConfiguration#1749
Conversation
WalkthroughThis PR expands validation test coverage for the PostgreSQL database configuration model by introducing new required-field validation tests and extending existing port and path validation scenarios with subtests, while reformatting test constructor calls to multi-line argument blocks. ChangesPostgreSQL Database Configuration Validation Tests
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with 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.
Inline comments:
In `@tests/unit/models/config/test_postgresql_database_configuration.py`:
- 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).
- Around line 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.
- 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.
- Around line 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.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 19075a32-fb1b-4afb-ad9d-8c4385a7cb9c
📒 Files selected for processing (1)
tests/unit/models/config/test_postgresql_database_configuration.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: radon
- GitHub Check: unit_tests (3.13)
- GitHub Check: build-pr
- GitHub Check: unit_tests (3.12)
- GitHub Check: spectral
- GitHub Check: integration_tests (3.12)
- GitHub Check: Pylinter
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
- GitHub Check: E2E: library mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 3
- GitHub Check: E2E: library mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 3
- GitHub Check: E2E: server mode / ci / group 2
- GitHub Check: E2E Tests for Lightspeed Evaluation job
🧰 Additional context used
📓 Path-based instructions (1)
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/**/*.py: Use pytest for all unit and integration tests; do not use unittest
Usepytest.mark.asynciomarker for async tests
Files:
tests/unit/models/config/test_postgresql_database_configuration.py
🔇 Additional comments (2)
tests/unit/models/config/test_postgresql_database_configuration.py (2)
20-24: LGTM!
213-221: LGTM!
|
|
||
|
|
||
| def test_postgresql_database_configuration_missing_values(subtests: SubTests) -> None: | ||
| """Test the PostgreSQLDatabaseConfiguration model.""" |
There was a problem hiding this comment.
🧹 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.
| """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).
| def test_postgresql_database_configuration_missing_values(subtests: SubTests) -> None: | ||
| """Test the PostgreSQLDatabaseConfiguration model.""" | ||
| with subtests.test(msg="Missing 'db' attribute"): | ||
| with pytest.raises(ValueError, match="Field required"): |
There was a problem hiding this comment.
🧩 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' -C2Repository: 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.pyRepository: lightspeed-core/lightspeed-stack
Length of output: 628
🏁 Script executed:
# Examine the PostgreSQLDatabaseConfiguration model
grep -n "class PostgreSQLDatabaseConfiguration" src/models/config.py -A 30Repository: 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.pyRepository: 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 -30Repository: 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.
| with subtests.test(msg="Missing 'id' and 'user' attributes"): | ||
| with pytest.raises(ValueError, match="Field required"): | ||
| PostgreSQLDatabaseConfiguration( | ||
| password="password", | ||
| ) # pyright: ignore[reportCallIssue] |
There was a problem hiding this comment.
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.
| 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="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] |
There was a problem hiding this comment.
🧩 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" -C3Repository: 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 50Repository: 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.pyRepository: 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.pyRepository: 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 -20Repository: 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.pyRepository: 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.pyRepository: 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.tomlRepository: 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.
1 participant
Description
LCORE-2264: Unit tests for checking the model for a class
PostgreSQLDatabaseConfigurationType of change
Tools used to create PR
Related Tickets & Documents
Summary by CodeRabbit
Release Notes
This release contains internal quality improvements with no user-visible changes. Test coverage has been expanded for database configuration validation to ensure robust error handling and field requirement verification.