LCORE-519: Ensure unit-tests cover database.py
#640
Conversation
WalkthroughAdds a new unit test module for app.database that validates engine/session initialization, table creation, SQLite/Postgres engine construction (including CA cert and schema handling), and the initialize_database control flow using fixtures and mocks. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor TestRunner
participant Config
participant DB as app.database
participant FS as Filesystem
participant SA as SQLAlchemy
participant PG as Postgres
TestRunner->>DB: initialize_database(config)
DB->>Config: inspect db.type, url, options
alt sqlite
DB->>FS: ensure DB file directory exists
FS-->>DB: ok / error
DB->>SA: create_engine(sqlite://..., connect_args)
SA-->>DB: Engine / error
else postgres
DB->>SA: create_engine(postgres://..., ssl options)
SA-->>DB: Engine / error
DB->>SA: engine.connect()
SA-->>DB: Connection / error
DB->>PG: set search_path / create schema (if requested)
PG-->>DB: ok / error
end
DB->>SA: sessionmaker(bind=engine)
SA-->>DB: SessionFactory
DB-->>TestRunner: (engine, SessionLocal) or raise on failure
note over TestRunner,DB: Tests assert calls, return values, call counts, and error handling
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/unit/app/test_database.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: All modules start with descriptive module-level docstrings explaining purpose
Use logger = logging.getLogger(name) for module logging after import logging
Define type aliases at module level for clarity
All functions require docstrings with brief descriptions
Provide complete type annotations for all function parameters and return types
Use typing_extensions.Self in model validators where appropriate
Use modern union syntax (str | int) and Optional[T] or T | None consistently
Function names use snake_case with descriptive, action-oriented prefixes (get_, validate_, check_)
Avoid in-place parameter modification; return new data structures instead of mutating arguments
Use appropriate logging levels: debug, info, warning, error with clear messages
All classes require descriptive docstrings explaining purpose
Class names use PascalCase with conventional suffixes (Configuration, Error/Exception, Resolver, Interface)
Abstract base classes should use abc.ABC and @AbstractMethod for interfaces
Provide complete type annotations for all class attributes
Follow Google Python docstring style for modules, classes, and functions, including Args, Returns, Raises, Attributes sections as needed
Files:
tests/unit/app/test_database.py
tests/{unit,integration}/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/{unit,integration}/**/*.py: Use pytest for all unit and integration tests
Do not use unittest in tests; pytest is the standard
Files:
tests/unit/app/test_database.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use pytest-mock to create AsyncMock objects for async interactions in tests
Use the shared auth mock constant: MOCK_AUTH = ("mock_user_id", "mock_username", False, "mock_token") in tests
Files:
tests/unit/app/test_database.py
🧬 Code graph analysis (1)
tests/unit/app/test_database.py (3)
src/models/config.py (4)
config(139-145)SQLiteDatabaseConfiguration(74-77)PostgreSQLDatabaseConfiguration(86-104)db_type(130-136)src/app/database.py (6)
get_engine(20-26)get_session(34-40)create_tables(29-31)_create_sqlite_engine(43-54)_create_postgres_engine(57-98)initialize_database(101-129)src/configuration.py (1)
database_configuration(140-144)
⏰ 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: e2e_tests (ci)
- GitHub Check: build-pr
🔇 Additional comments (7)
tests/unit/app/test_database.py (7)
1-14: LGTM!The module docstring is clear, and the imports are appropriate for comprehensive unit testing. The pylint directive appropriately allows testing of protected methods.
46-64: LGTM!The test cases properly cover both the initialized and uninitialized states with clear assertions.
66-87: LGTM!The test cases correctly handle both states, and the mock setup properly simulates
SessionLocalas a session factory.
89-111: LGTM!The test cases properly mock dependencies and verify both successful table creation and error handling when the engine is not initialized.
113-146: LGTM!The test suite comprehensively covers the SQLite engine creation: successful creation with a real engine, directory validation, and error handling. The use of temporary directories is appropriate.
148-237: LGTM!Comprehensive test coverage for PostgreSQL engine creation, including default/custom schemas, CA certificate handling, and all error paths. The tests properly verify URL construction, connection arguments, and schema creation logic.
239-337: LGTM!The test suite effectively covers database initialization for both SQLite and PostgreSQL. The helper methods reduce duplication, and the tests properly verify debug mode behavior (affecting the
echoparameter), engine creation calls, and sessionmaker configuration.
tisnik
left a comment
tisnik
left a comment
There was a problem hiding this comment.
please use pytest Mocker, not unittests.mock
See https://issues.redhat.com/browse/LCORE-417 that should fix all existing tests.
TY in advance!
The coverage on this file was poor, adding some unit-tests to improve it.
2 participants
Description
The coverage on this file was poor, adding some unit-tests to improve it.
Type of change
Related Tickets & Documents
LCORE-519
Checklist before requesting a review
Testing
Summary by CodeRabbit