Test infra: switch test DB to local Docker Postgres#14
Conversation
There was a problem hiding this comment.
Clean infrastructure PR — adds local Docker Postgres for tests so you don't need a remote test database. The approach is solid and well-documented.
No security issues, no data isolation concerns (no backend code changed), and the test infrastructure setup follows good practices. The two inline comments are minor suggestions about port conflicts and wait-loop timeouts — neither is a blocker.
| POSTGRES_USER: postgres | ||
| POSTGRES_PASSWORD: postgres | ||
| ports: | ||
| - "5432:5432" |
There was a problem hiding this comment.
Minor: Port conflict consideration
The hardcoded 5432:5432 mapping will conflict if the developer already has a Postgres instance running locally on the default port.
This is acceptable for a simple local dev setup, but you could consider:
- Using a non-standard port like
"5433:5432"to avoid conflicts - Documenting in README.md that port 5432 must be free
- Adding a check in
run-tests.shto fail early with a helpful message if the port is in use
For a portfolio project targeting simplicity, option 2 (just documenting it) is probably sufficient. The current approach is fine — just something to be aware of if you run into "port already allocated" errors.
|
|
||
| echo "Starting local test database..." | ||
| docker compose up -d db | ||
| until docker compose exec -T db pg_isready -U postgres -q; do sleep 1; done |
There was a problem hiding this comment.
Nice pattern: until loop with pg_isready
This is the right way to wait for Postgres — pg_isready checks if the server is actually accepting connections, not just that the container started.
The -T flag on docker compose exec is critical here: without it, Docker expects an interactive terminal and the command fails in non-interactive contexts (scripts, CI). Good catch documenting this in til.md.
One minor enhancement if you want to prevent infinite hangs: add a timeout counter:
WAIT_TIME=0
until docker compose exec -T db pg_isready -U postgres -q; do
sleep 1
WAIT_TIME=$((WAIT_TIME + 1))
if [ $WAIT_TIME -gt 30 ]; then
echo "Error: Database failed to start after 30 seconds"
exit 1
fi
doneBut for local dev, the current infinite loop is fine — if Docker/Postgres is broken, you want to know immediately anyway.
There was a problem hiding this comment.
decided to skip, easy to spot when testing locally
Review SummaryCritical: None — this is a clean infrastructure change with no security or data integrity issues. Minor:
The Docker setup is well-structured, the healthcheck is correct, and the Cost: $0.21 |
Tests previously ran against a remote Neon database (~3 min per run). A local container cuts this significantly.
Decisions worth noting
run-tests.shstarts the container if needed but does not stop it. Stopping on every run would negate the speed gain when running tests repeatedly.docker compose downis documented as a "done for the day" step.dockerpresence check in the script — unlike theuvcheck, Docker's own error output is clear enough if the daemon isn't running. An explicit guard would be noise.TEST_DATABASE_URLonly —DATABASE_URL(production Neon) is untouched. The safety guard inconftest.pythat rejects equal URLs remains the line of defence against accidentally running tests against production.What's next
Auth tests (
test_auth.py) — register, login, bad credentials, duplicate email.