From 2ad2e943f38f283a8885ad64cb6d46d2f2ce65d4 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sun, 14 Jun 2026 19:19:18 -0400 Subject: [PATCH] test(ci): add Postgres integration lane + real run_maintenance guard The unit suite can't catch sync/async API mismatches against SQLAlchemy (an un-awaited execution_options passed green CI but failed at runtime: VACUUM 0/6). Add a real-Postgres integration lane modelled on the family pattern (rules 6/79-82): a new CI 'integration' job with a postgres:16 service, bridge-IP discovery, busybox-safe readiness wait, and 'alembic upgrade head', running pytest -m integration. Non-gating, like the unit lane. - tests/test_integration_db_maintenance.py: runs run_maintenance() and get_table_health() against real Postgres; asserts all allowlisted tables vacuum OK (the await regression makes this fail) and health reports real stats. - pyproject: register the 'integration' marker. - conftest: integration-marked tests use the real DATABASE_URL, not the stub. - ci.yml: unit 'test' job now runs -m 'not integration'. Co-Authored-By: Claude Opus 4.8 --- .forgejo/workflows/ci.yml | 70 +++++++++++++++++++++++- pyproject.toml | 3 + tests/conftest.py | 10 +++- tests/test_integration_db_maintenance.py | 41 ++++++++++++++ 4 files changed, 121 insertions(+), 3 deletions(-) create mode 100644 tests/test_integration_db_maintenance.py diff --git a/.forgejo/workflows/ci.yml b/.forgejo/workflows/ci.yml index 8180ffb..8a3aa81 100644 --- a/.forgejo/workflows/ci.yml +++ b/.forgejo/workflows/ci.yml @@ -142,7 +142,75 @@ jobs: uv pip install --python /opt/venv/bin/python -e ".[dev]" - name: Run tests - run: /opt/venv/bin/python -m pytest tests/ -q + # Integration tests (real Postgres) run in the `integration` job below. + run: /opt/venv/bin/python -m pytest tests/ -q -m "not integration" + + # Real-Postgres lane (family rule 6). Exercises the async SQLAlchemy connection + # path the unit stubs can't reach — the un-awaited execution_options regression + # that made every VACUUM report 0/6 lived here. Like `test`, it runs for + # visibility and does NOT gate the build. + # + # Job key stays separator-free ("integration"): act_runner derives the service- + # container name from the (truncated) job display name and the discovery step + # filters `docker ps` by it. Service hostnames aren't routable on this runner, + # so the step resolves the Postgres container's bridge IP. No `name:` on purpose. + integration: + if: github.ref == 'refs/heads/dev' || github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/tags/v') + runs-on: python-ci + container: + image: git.fabledsword.com/bvandeusen/ci-python:3.14 + env: + # Config + the module engine read these at import time. DATABASE_URL itself + # is built from the discovered service IP in the run step. + SECRET_KEY: ci_integration_placeholder + services: + postgres: + image: postgres:16-alpine + env: + POSTGRES_USER: scribe + POSTGRES_PASSWORD: ci_integration + POSTGRES_DB: scribe_test + options: >- + --health-cmd "pg_isready -U scribe" + --health-interval 10s + --health-timeout 5s + --health-retries 10 + steps: + - uses: actions/checkout@v6 + - name: Create virtual environment + run: uv venv /opt/venv + - name: Install package with dev deps + run: | + uv pip install --python /opt/venv/bin/python setuptools wheel + uv pip install --python /opt/venv/bin/python --no-build-isolation http-ece + uv pip install --python /opt/venv/bin/python -e ".[dev]" + - name: Integration suite (resolve service IP, migrate, test) + run: | + set -eux + echo "=== container landscape (diagnostic for the name filter) ===" + docker ps -a --format '{{.ID}} {{.Image}} -> {{.Names}}' + PG=$(docker ps --filter "name=integration" --filter "ancestor=postgres:16-alpine" -q | head -n1) + test -n "$PG" + PG_IP=$(docker inspect -f '{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}' "$PG") + test -n "$PG_IP" + export DATABASE_URL="postgresql+asyncpg://scribe:ci_integration@${PG_IP}:5432/scribe_test" + # Wait for Postgres to accept connections (busybox sh — the runner + # default — has no bash /dev/tcp, so use Python). + /opt/venv/bin/python - "$PG_IP" <<'PY' + import socket, sys, time + for _ in range(30): + try: + socket.create_connection((sys.argv[1], 5432), timeout=2).close() + break + except OSError: + time.sleep(1) + else: + sys.exit("postgres did not become reachable") + PY + # Real migrations build the schema; the maintenance tests then run + # VACUUM (ANALYZE) and read pg_stat_user_tables against it. + /opt/venv/bin/alembic upgrade head + /opt/venv/bin/python -m pytest tests/ -v -m integration build: name: Build & push image diff --git a/pyproject.toml b/pyproject.toml index 7c19f4e..be0e53f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -36,6 +36,9 @@ where = ["src"] [tool.pytest.ini_options] asyncio_mode = "auto" testpaths = ["tests"] +markers = [ + "integration: requires a real Postgres database (runs only in the CI integration lane)", +] [tool.ruff] line-length = 120 diff --git a/tests/conftest.py b/tests/conftest.py index 9210dee..459f4ce 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -12,8 +12,14 @@ import pytest @pytest.fixture(autouse=True) -def _isolate_env(monkeypatch): - """Prevent tests from accidentally reading production env vars.""" +def _isolate_env(request, monkeypatch): + """Prevent unit tests from accidentally reading production env vars. + + Integration tests (marked `integration`) are skipped here: they must use the + real DATABASE_URL injected by the CI integration lane, not the fake one. + """ + if request.node.get_closest_marker("integration"): + return monkeypatch.setenv("DATABASE_URL", "postgresql+asyncpg://test:test@localhost/test") monkeypatch.setenv("SECRET_KEY", "test-secret-key") monkeypatch.setenv("OLLAMA_URL", "http://localhost:11434") diff --git a/tests/test_integration_db_maintenance.py b/tests/test_integration_db_maintenance.py new file mode 100644 index 0000000..cff8cf3 --- /dev/null +++ b/tests/test_integration_db_maintenance.py @@ -0,0 +1,41 @@ +"""Real-Postgres integration tests for DB maintenance + health. + +These run only in the CI integration lane (a real Postgres service + schema +built by `alembic upgrade head`). They exercise the actual async SQLAlchemy +connection path that unit mocks cannot: the un-awaited +`AsyncConnection.execution_options` regression (which made every VACUUM raise +AttributeError, reporting 0/6) passes the unit suite but fails here. +""" +import pytest + +from scribe.services.db_maintenance import ( + MAINTENANCE_TABLES, + get_table_health, + run_maintenance, +) + +pytestmark = pytest.mark.integration + + +@pytest.mark.asyncio +async def test_run_maintenance_vacuums_real_tables(): + summary = await run_maintenance() + assert summary["tables"], "no tables were vacuumed" + # Every allowlisted table exists after `alembic upgrade head`, so every + # VACUUM (ANALYZE) must succeed. With the un-awaited execution_options bug + # they would ALL fail with AttributeError — this is the guard. + failed = [t for t in summary["tables"] if not t["ok"]] + assert not failed, f"VACUUM failed for: {failed}" + assert {t["table"] for t in summary["tables"]} <= set(MAINTENANCE_TABLES) + + +@pytest.mark.asyncio +async def test_table_health_reports_real_stats(): + health = await get_table_health() + assert health["db_bytes"] > 0 + names = {t["table"] for t in health["tables"]} + # Core table built by migrations must show up in pg_stat_user_tables. + assert "notes" in names + for t in health["tables"]: + assert t["dead_pct"] >= 0 + assert t["total_bytes"] >= 0