Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the test database setup to programmatically create test databases and schemas instead of relying on Docker initialization scripts. Despite the title "Switch logger to rotoger," the actual changes focus entirely on test infrastructure improvements.
Key Changes:
- Introduces a separate test database engine and session factory for isolation between test and production databases
- Implements programmatic database and schema creation in test fixtures rather than using Docker entrypoint scripts
- Simplifies the Docker setup by removing test-compose.yml overlay and database initialization scripts
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/conftest.py | Adds functions to programmatically create test database and schemas, updates fixtures to use new test engine |
| app/database.py | Introduces test_engine and TestAsyncSessionFactory for test database connections, adds get_test_db dependency |
| app/config.py | Adds POSTGRES_TEST_USER and POSTGRES_TEST_DB environment variables, implements test_asyncpg_url computed field |
| test-compose.yml | Removes Docker Compose test overlay file (no longer needed) |
| db/Dockerfile | Removes database initialization script reference |
| Makefile | Updates docker-test command to remove test-compose.yml overlay |
| .env | Adds POSTGRES_TEST_DB and POSTGRES_TEST_USER environment variables |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tests/conftest.py
Outdated
| @@ -1,10 +1,13 @@ | |||
| from collections.abc import AsyncGenerator | |||
| from types import SimpleNamespace | |||
There was a problem hiding this comment.
The import SimpleNamespace from the types module is not used anywhere in this file. This unused import should be removed to keep the code clean.
| from types import SimpleNamespace |
| POSTGRES_PASSWORD: str | ||
| POSTGRES_HOST: str | ||
| POSTGRES_DB: str | ||
| POSTGRES_TEST_USER: str |
There was a problem hiding this comment.
The environment variable POSTGRES_TEST_USER is defined in the Settings class but is never used. The test_asyncpg_url method uses POSTGRES_USER instead of POSTGRES_TEST_USER for the username. Either POSTGRES_TEST_USER should be used in test_asyncpg_url, or if it's not needed, it should be removed from the Settings class.
| POSTGRES_TEST_USER: str |
…test database URL
No description provided.