-
Notifications
You must be signed in to change notification settings - Fork 4
Description
Problem
Tests connect to the dev database (ocotilloapi_dev) instead of the test database (ocotilloapi_test) because multiple load_dotenv(override=True) calls stomp env vars set by the test framework. There are 7 load_dotenv calls across the codebase with inconsistent override behavior:
| File | Call | Override |
|---|---|---|
tests/__init__.py:23 |
load_dotenv(override=True) |
Yes |
tests/conftest.py:7 |
load_dotenv(override=True) |
Yes |
db/engine.py:35 |
load_dotenv(override=False) |
No |
alembic/env.py:47 |
load_dotenv() |
No (default) |
main.py:7 |
load_dotenv() |
No (default) |
cli/cli.py:28 |
load_dotenv(override=True) |
Yes |
transfers/transfer.py:39 |
load_dotenv(override=False) |
No |
The override=True calls re-read .env (which has POSTGRES_DB=ocotilloapi_dev) and overwrite the test framework's POSTGRES_DB=ocotilloapi_test.
Proposed fix: follow standard Python convention
The standard convention in the Python/FastAPI ecosystem:
-
load_dotenv()(no override) everywhere —.envprovides defaults, not overrides. Env vars already set by the runtime, test framework, or CI take precedence. This is the default behavior ofpython-dotenvfor a reason. -
Set test env vars before
load_dotenv()— intests/__init__.py, setPOSTGRES_DB=ocotilloapi_testbefore callingload_dotenv(). Since the default is no-override,.env's value won't stomp it. -
One
load_dotenv()per entry point —main.py,cli/cli.py,tests/__init__.py,transfers/transfer.pyeach call it once. Library code (db/engine.py) and Alembic (env.py) should NOT callload_dotenv()— they rely on the entry point having loaded it.
Concrete changes
tests/__init__.py: setPOSTGRES_DB=ocotilloapi_testbeforeload_dotenv(), removeoverride=Truetests/conftest.py: removeload_dotenvcall entirely, removeTEST_DATABASE_NAME/_test_database_url()db/engine.py: removeload_dotenv(override=False)(entry points handle it)alembic/env.py: removeload_dotenv()(entry points handle it)cli/cli.py: changeoverride=Trueto plainload_dotenv()transfers/transfer.py: simplifyoverride=Falseto plainload_dotenv()main.py: already correct, no change