LCORE-574: Unit tests for PostgreSQL configuration + removed unused check #423
Conversation
WalkthroughRemoved CA certificate path validation from PostgreSQL configuration in src/models/config.py. Added/updated unit tests in tests/unit/models/test_config.py that validate defaults, port constraints, and CA certificate file-path behavior, and reference new constants for default SSL/GSS enc modes. Changes
Sequence Diagram(s)Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/unit/models/test_config.py (2)
912-933: Parametrize CA cert path tests for clarity and to drop subtests dependency.This reduces boilerplate and avoids relying on pytest-subtests.
-def test_postgresql_database_configuration_ca_cert_path(subtests) -> None: - """Test the PostgreSQLDatabaseConfiguration model.""" - with subtests.test(msg="Path exists"): - c = PostgreSQLDatabaseConfiguration( - db="db", - user="user", - password="password", - port=1234, - ca_cert_path=Path("tests/configuration/server.crt"), - ) - assert c.ca_cert_path == Path("tests/configuration/server.crt") - - with subtests.test(msg="Path does not exist"): - 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("not a file"), - ) +@pytest.mark.parametrize( + "ca_path,exc,pattern", + [ + (Path("tests/configuration/server.crt"), None, None), + (Path("not a file"), ValidationError, "Path does not point to a file"), + ], +) +def test_postgresql_database_configuration_ca_cert_path(ca_path, exc, pattern) -> None: + """Test the PostgreSQLDatabaseConfiguration ca_cert_path validation.""" + if exc is None: + c = PostgreSQLDatabaseConfiguration( + db="db", user="user", password="password", port=1234, ca_cert_path=ca_path + ) + assert c.ca_cert_path == ca_path + else: + with pytest.raises(exc, match=pattern): + PostgreSQLDatabaseConfiguration( + db="db", user="user", password="password", port=1234, ca_cert_path=ca_path + )If subtests must be kept, please ensure pytest-subtests is declared in your test dependencies (see the script in the previous comment).
890-910: pytest-subtests plugin is already declared; parametrize refactor is optionalThe
pytest-subtestsdependency is present in pyproject.toml (line 95), so you don’t need to add it if you choose to keep using subtests. However, you may still opt for a more idiomatic pytest style by switching to@pytest.mark.parametrize.• Dependency found:
– pyproject.toml, line 95:pytest-subtests>=0.14.2• Optional refactor (no dependency change required):
- def test_postgresql_database_configuration_port_setting(subtests) -> None: - """Test the PostgreSQLDatabaseConfiguration model.""" - with subtests.test(msg="Correct port value"): - c = PostgreSQLDatabaseConfiguration(db="db", user="user", password="password", port=1234) - 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) - - 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) + @pytest.mark.parametrize( + "port,exc,pattern", + [ + (1234, None, None), + (-1, ValidationError, "Input should be greater than 0"), + (100000, ValueError, "Port value should be less than 65536"), + ], + ) + def test_postgresql_database_configuration_port_setting(port, exc, pattern) -> None: + """Test the PostgreSQLDatabaseConfiguration port validation.""" + if exc is None: + c = PostgreSQLDatabaseConfiguration(db="db", user="user", password="password", port=port) + assert c is not None + assert c.port == port + else: + with pytest.raises(exc, match=pattern): + PostgreSQLDatabaseConfiguration(db="db", user="user", password="password", port=port)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/models/config.py(0 hunks)tests/unit/models/test_config.py(3 hunks)
💤 Files with no reviewable changes (1)
- src/models/config.py
⏰ 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). (2)
- GitHub Check: build-pr
- GitHub Check: e2e_tests
🔇 Additional comments (3)
tests/unit/models/test_config.py (3)
14-16: Good call using constants for Postgres defaults.Referencing POSTGRES_DEFAULT_SSL_MODE and POSTGRES_DEFAULT_GSS_ENCMODE keeps tests resilient to value changes.
29-29: Import of PostgreSQLDatabaseConfiguration looks correct.Consistent with other model imports from models.config.
875-888: Solid coverage of PostgreSQL defaults.Asserts for host, port, namespace, and SSL/GSS encmode defaults are clear and aligned with expectations.
1 participant
Description
LCORE-574: Unit tests for PostgreSQL configuration + removed unused check
Type of change
Related Tickets & Documents
Summary by CodeRabbit
New Features
Bug Fixes
Tests