chore(test): disambiguate Postgres skip reason; close 2 idea folders#261
Conversation
Bundles two small ideas into one PR. CHORE_DB_SESSION_SKIP_REASON_DISAMBIGUATION (#4 in /pipeline status): Refactors backend/tests/conftest.py to split the previously-monolithic postgres_reachable() helper: - NEW postgres_skip_reason() -> str | None returns the precise skip-reason string per failure mode (env-var-missing / Settings-construction-failure / TCP-unreachable). Each reason includes operator-actionable remediation (e.g. "missing -e flag", "scripts/ install.sh", "make up"). - postgres_reachable() -> bool kept as a thin wrapper (postgres_skip_reason() is None) so the 100+ existing pytest.mark.skipif(not postgres_reachable(), ...) callsites need no migration. - db_session fixture now emits the precise reason instead of the misleading hardcoded "Postgres not reachable" that lied in 2 of 3 failure modes. New unit tests at backend/tests/unit/test_postgres_skip_reason.py (8 tests, sub-second runtime) lock the precise reason strings against silent regression. BUG_DOCKERFILE_MISSING_SCRIPTS_DIR (#10 in /pipeline status): Paperwork close - the underlying fix shipped in PR #232 (chore_dashboard_pr_extraction_from_idea) and the idea file was already marked "Fixed". Verified Dockerfile still contains the COPY ... scripts/ /app/scripts/ line. Moves the folder to implemented_features/2026_05_24_*/ so it stops appearing in /pipeline status backlog. CI: make test-unit rc=0. Closes 2 of the 17 (now 12) idea-stage items in the MVP1 backlog. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the Postgres reachability check in backend/tests/conftest.py by splitting it into postgres_skip_reason() (which provides precise, operator-actionable skip reasons for different failure modes) and keeping postgres_reachable() as a backward-compatible wrapper. It also adds unit tests to lock in these skip reasons and updates the project dashboards. The reviewer feedback suggests moving the URL parsing and host/port extraction inside the try...except block to prevent potential runtime crashes (such as a ValueError on a malformed port) and ensure robust skip detection.
There was a problem hiding this comment.
The URL parsing and port extraction are currently performed outside of the try...except block. If the database_url contains a malformed port (e.g., postgresql://localhost:invalid_port/db), accessing parsed.port will raise a ValueError at runtime. Since this helper is designed to be a robust, best-effort skip detector, this exception will cause the entire test suite setup to crash instead of gracefully returning a skip reason.
Moving the urlparse and host/port extraction inside the try block ensures that any parsing or value errors are caught and handled as settings construction failures.
1 participant
Summary
Bundles two small ideas into one PR cycle.
chore_db_session_skip_reason_disambiguation(#4 in pipeline status backlog)Refactors
backend/tests/conftest.pyto split the previously-monolithicpostgres_reachable()helper:postgres_skip_reason() -> str | None— returns the precise skip-reason string per failure mode:infra_test_worktree_missing_integration_envsscripts/install.shfor regenerationhost:port+make upfrom the main worktreepostgres_reachable() -> boolkept as a thin wrapper (postgres_skip_reason() is None) so the 100+ existingpytest.mark.skipif(not postgres_reachable(), ...)callsites need no migrationdb_sessionfixture now emits the precise reason instead of the misleading hardcoded"Postgres not reachable"that lied in 2 of 3 failure modesNew unit tests at
backend/tests/unit/test_postgres_skip_reason.py(8 tests, sub-second runtime) lock the precise reason strings against silent regression.bug_dockerfile_missing_scripts_dir(#10 in pipeline status backlog)Paperwork close. The underlying fix shipped in PR #232 and the idea file was already marked
**Fixed**. Verified the Dockerfile still contains theCOPY ... scripts/ /app/scripts/line. Moves the folder toimplemented_features/2026_05_24_*/(using PR #232's merge date) so it stops appearing in the/pipeline statusbacklog.Test plan
make test-unit— exit 0 (8 new tests intest_postgres_skip_reason.pypass + all pre-existing tests still pass)make lint+make typecheckcleanmake fmtran; lockfile drift = 0COPY ... scripts/ /app/scripts/(the PR feat: swap_template followups (Tier B of feat_digest_executable_followups) #232 fix is intact)🤖 Generated with Claude Code