diff --git a/.github/workflows/python-ci.yml b/.github/workflows/python-ci.yml index 5dd9bce6d..048cd0cc7 100644 --- a/.github/workflows/python-ci.yml +++ b/.github/workflows/python-ci.yml @@ -83,6 +83,13 @@ jobs: bash -c 'until pg_isready -U $POSTGRES_USER; do sleep 5; done' echo "Postgres is ready." + # The opt-in Postgres integration tests (require_polis_postgres) run here + # against the compose `postgres` service, whose image bakes the polis + # migrations (server/postgres/migrations/*.sql via docker-entrypoint-initdb.d), + # so the votes / votes_latest_unique schema + on_vote_insert_update_unique_table + # rule are already applied. The pytest step exports POLIS_TEST_POSTGRES_URL; + # the cold-start generator under test is already baked into the delphi image + # (Dockerfile `COPY scripts/ ./scripts/`), built from this checkout. - name: 6. Run Delphi Pytest run: | echo "Copying test files into container..." @@ -112,6 +119,7 @@ jobs: python create_dynamodb_tables.py --region us-east-1; \ echo '--- Running Pytest ---'; \ export PYTHONPATH=\$PYTHONPATH:/app; \ + export POLIS_TEST_POSTGRES_URL=\"postgresql://\$DATABASE_USER:\$DATABASE_PASSWORD@\$DATABASE_HOST/\$DATABASE_NAME\"; \ pytest --cov=polismath --cov=run_math_pipeline --cov=./umap_narrative --cov-report=xml:/app/coverage.xml /app/tests --ignore=/app/tests/test_pakistan_conversation.py echo '--- Generating Coverage Comment Text ---'; \ python /app/generate_coverage_md.py > /app/coverage-comment.md \ diff --git a/delphi/tests/poller/conftest.py b/delphi/tests/poller/conftest.py new file mode 100644 index 000000000..a245fb6a1 --- /dev/null +++ b/delphi/tests/poller/conftest.py @@ -0,0 +1,30 @@ +"""Shared fixtures for the poller test suite. + +Test-isolation guard: `MathPollerService.apply_engine_mode()` writes +`POLISMATH_ENGINE_MODE` into `os.environ` for the life of the process (the +service is a long-running daemon in production, so it has no reason to restore +it). Under pytest's single serial process, any poller test that constructs a +service with `engine_mode="clojure-legacy"` would otherwise leak legacy mode +into every later-collected test — flipping e.g. the in-conv greedy floor on +tests that assume the default 'improved' mode (this exact leak broke +TestD2cVoteCountSource in CI once #2637 made the postgres integration test run +there instead of skipping). +""" + +import os + +import pytest + +from polismath.utils.engine_mode import ENGINE_MODE_ENV_VAR + + +@pytest.fixture(autouse=True) +def _restore_engine_mode_env(): + """Snapshot and restore POLISMATH_ENGINE_MODE around every poller test.""" + was_set = ENGINE_MODE_ENV_VAR in os.environ + saved = os.environ.get(ENGINE_MODE_ENV_VAR, "") + yield + if was_set: + os.environ[ENGINE_MODE_ENV_VAR] = saved + else: + os.environ.pop(ENGINE_MODE_ENV_VAR, None) diff --git a/delphi/tests/poller/test_integration_postgres.py b/delphi/tests/poller/test_integration_postgres.py index 5ea26e2b0..5155b211f 100644 --- a/delphi/tests/poller/test_integration_postgres.py +++ b/delphi/tests/poller/test_integration_postgres.py @@ -1,89 +1,34 @@ """End-to-end integration test for the math poller against a real Postgres. -OPT-IN and self-skipping (like tests/test_postgres_real_data.py): it provisions -a THROWAWAY postgres:17 container on port 5435 (NEVER the host's live 5432), -applies server/postgres/migrations/000000_initial.sql, seeds one conversation, -and drives poll -> compute -> write, asserting: +OPT-IN and self-skipping (like tests/test_postgres_real_data.py): it obtains a +Postgres with the polis votes schema via the shared ``require_polis_postgres`` +helper — the CI ``postgres`` service (POLIS_TEST_POSTGRES_URL) when present, else +a THROWAWAY postgres:17 on an EPHEMERAL port (NEVER the host's live 5432) with +000000_initial.sql + 000006_update_votes_rule.sql applied — seeds one +conversation, and drives poll -> compute -> write, asserting: * a math_main row appears under the poller's math_env (shadow isolation), * math_bidtopid + math_ptptstats share the cycle's math_tick, * caching_tick / math_tick behave per the Clojure-exact SQL, * a fresh service instance resumes and advances the tick (restart-resumes). -If docker is unavailable or port 5435 is busy, the whole module is skipped with -a clear reason. +If neither a CI service nor docker is available, the whole module is skipped +with a clear reason. """ -import os -import shutil -import subprocess import time -import uuid import pytest -pytestmark = pytest.mark.integration - -MIGRATION = os.path.join( - os.path.dirname(__file__), - "..", "..", "..", "server", "postgres", "migrations", "000000_initial.sql", -) -PORT = 5435 -DB_URL = f"postgresql://postgres:test@localhost:{PORT}/postgres" - - -def _docker() -> str: - exe = shutil.which("docker") - if not exe: - pytest.skip("docker not available") - return exe - +from tests.conftest import require_polis_postgres -def _run(*args, **kwargs): - return subprocess.run(args, capture_output=True, text=True, **kwargs) +pytestmark = pytest.mark.integration @pytest.fixture(scope="module") def pg_url(): - docker = _docker() - migration = os.path.abspath(MIGRATION) - if not os.path.exists(migration): - pytest.skip(f"migration not found: {migration}") - - name = f"delphi-poller-it-{uuid.uuid4().hex[:8]}" - started = _run( - docker, "run", "--rm", "-d", "--name", name, - "-p", f"{PORT}:5432", "-e", "POSTGRES_PASSWORD=test", "postgres:17", - ) - if started.returncode != 0: - pytest.skip(f"could not start postgres container (port {PORT} busy?): " - f"{started.stderr.strip()}") - cid = started.stdout.strip() - try: - # Wait for readiness. - deadline = time.time() + 40 - ready = False - while time.time() < deadline: - if _run(docker, "exec", cid, "pg_isready", "-U", "postgres").returncode == 0: - ready = True - break - time.sleep(1) - if not ready: - pytest.skip("postgres container did not become ready in time") - - # Apply the full initial migration. - with open(migration, "rb") as fh: - applied = subprocess.run( - [docker, "exec", "-i", cid, "psql", "-v", "ON_ERROR_STOP=1", - "-U", "postgres", "-d", "postgres"], - stdin=fh, capture_output=True, text=True, - ) - if applied.returncode != 0: - pytest.skip(f"migration failed to apply: {applied.stderr[-500:]}") - - yield DB_URL - finally: - _run(docker, "stop", cid) + with require_polis_postgres() as url: + yield url def _seed_conversation(engine, zid=1, n_ptpts=8, n_cmts=5):