From 586302f49a2e06a67c634dee3fbff08e9e269658 Mon Sep 17 00:00:00 2001 From: Lucas Tonon Date: Sat, 18 Jul 2026 23:35:37 -0300 Subject: [PATCH 1/2] ci: run backend tests against Postgres in CI, not only in-memory SQLite backend-test now spins up a postgres:16-alpine service container and points TEST_DATABASE_URL at it; conftest.py respects TEST_DATABASE_URL (falling back to DATABASE_URL, then the sqlite default for local dev) instead of unconditionally forcing sqlite+aiosqlite. This exercises the Column(JSON) fields, FK ON DELETE CASCADE behavior, and the hand-rolled db._run_migrations against the engine we actually ship. Also pins asyncio_default_fixture_loop_scope/asyncio_default_test_loop_scope to "session" in a new backend/pytest.ini. Without it, pytest-asyncio 1.x's per-test event loop (fine for aiosqlite) breaks the module-level asyncpg engine with "Future attached to a different loop" once a second test needs the DB. Pinning both to session fixes this for both SQLite and Postgres. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Wp66uSq9saSpRq9Bbmjxj4 --- .github/workflows/ci.yml | 16 ++++++++++++++++ backend/pytest.ini | 18 ++++++++++++++++++ backend/tests/conftest.py | 12 ++++++++++-- 3 files changed, 44 insertions(+), 2 deletions(-) create mode 100644 backend/pytest.ini diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cdcc835..09cc77c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -33,6 +33,22 @@ jobs: defaults: run: working-directory: backend + services: + postgres: + image: postgres:16-alpine + env: + POSTGRES_USER: postgres + POSTGRES_PASSWORD: postgres + POSTGRES_DB: trainable_test + ports: + - 5432:5432 + options: >- + --health-cmd "pg_isready -U postgres" + --health-interval 5s + --health-timeout 5s + --health-retries 10 + env: + TEST_DATABASE_URL: postgresql+asyncpg://postgres:postgres@localhost:5432/trainable_test steps: - uses: actions/checkout@v4 - uses: actions/setup-python@v5 diff --git a/backend/pytest.ini b/backend/pytest.ini new file mode 100644 index 0000000..4abf6a6 --- /dev/null +++ b/backend/pytest.ini @@ -0,0 +1,18 @@ +[pytest] +# Explicit + pinned pytest-asyncio config (see conftest.py for fixture setup). +# +# asyncio_default_fixture_loop_scope / asyncio_default_test_loop_scope = +# "session" gives the whole test session ONE event loop instead of a fresh +# one per test (the pytest-asyncio 1.x default is "function"). This matters +# because db.py's `engine` (the SQLAlchemy async engine / connection pool) +# is created once at import time. Against SQLite/aiosqlite that mismatch is +# silently tolerated, but asyncpg's connections are strictly bound to the +# event loop that created them — reusing the pooled engine from a second, +# different per-test event loop raises +# "got Future attached to a different loop". Pinning both +# scopes to "session" keeps the engine and every test on the same loop, so +# the suite runs unchanged against Postgres in CI (see +# .github/workflows/ci.yml) as well as the SQLite default. +asyncio_mode = strict +asyncio_default_fixture_loop_scope = session +asyncio_default_test_loop_scope = session diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index f3e953e..e0b0281 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -22,8 +22,16 @@ def pytest_configure(config): ) -# Use in-memory SQLite for tests (no Postgres needed) -os.environ["DATABASE_URL"] = "sqlite+aiosqlite://" +# Use in-memory SQLite for tests by default (no Postgres needed). CI's +# backend-test job runs a Postgres service container and sets +# TEST_DATABASE_URL (or DATABASE_URL directly) so the suite exercises real +# Postgres semantics — Column(JSON) storage, FK ON DELETE CASCADE, and the +# hand-rolled db._run_migrations — instead of only ever validating against +# an engine we don't ship. See .github/workflows/ci.yml. +if os.environ.get("TEST_DATABASE_URL"): + os.environ["DATABASE_URL"] = os.environ["TEST_DATABASE_URL"] +elif not os.environ.get("DATABASE_URL"): + os.environ["DATABASE_URL"] = "sqlite+aiosqlite://" # Mock claude_agent_sdk if it's not installed (it's a private package) if "claude_agent_sdk" not in sys.modules: From de197b8e14a2fe05608b11a2c613ac7d4578e73f Mon Sep 17 00:00:00 2001 From: Lucas Tonon Date: Mon, 20 Jul 2026 10:36:37 -0300 Subject: [PATCH 2/2] fix(review): address Greptile findings on #154 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P1 (conftest.py:34): the middle `elif` branch honored an ambient DATABASE_URL from the developer's shell; combined with setup_db's drop_all teardown that could destroy a real dev database on `pytest`. Now only the explicit TEST_DATABASE_URL opt-in overrides; everything else falls back to in-memory SQLite (as before this PR). CI is unaffected — it sets TEST_DATABASE_URL. Adds tests/test_conftest_db_url.py pinning that guarantee. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Wp66uSq9saSpRq9Bbmjxj4 --- backend/tests/conftest.py | 15 ++++++++++----- backend/tests/test_conftest_db_url.py | 18 ++++++++++++++++++ 2 files changed, 28 insertions(+), 5 deletions(-) create mode 100644 backend/tests/test_conftest_db_url.py diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index e0b0281..b417c64 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -24,13 +24,18 @@ def pytest_configure(config): # Use in-memory SQLite for tests by default (no Postgres needed). CI's # backend-test job runs a Postgres service container and sets -# TEST_DATABASE_URL (or DATABASE_URL directly) so the suite exercises real -# Postgres semantics — Column(JSON) storage, FK ON DELETE CASCADE, and the -# hand-rolled db._run_migrations — instead of only ever validating against -# an engine we don't ship. See .github/workflows/ci.yml. +# TEST_DATABASE_URL so the suite exercises real Postgres semantics — +# Column(JSON) storage, FK ON DELETE CASCADE, and the hand-rolled +# db._run_migrations — instead of only ever validating against an engine +# we don't ship. See .github/workflows/ci.yml. +# +# Only the explicit TEST_DATABASE_URL opt-in is honored: an ambient +# DATABASE_URL (e.g. pointing at a dev database in a developer's shell) is +# deliberately overwritten, because the setup_db fixture drops all tables +# after every test. if os.environ.get("TEST_DATABASE_URL"): os.environ["DATABASE_URL"] = os.environ["TEST_DATABASE_URL"] -elif not os.environ.get("DATABASE_URL"): +else: os.environ["DATABASE_URL"] = "sqlite+aiosqlite://" # Mock claude_agent_sdk if it's not installed (it's a private package) diff --git a/backend/tests/test_conftest_db_url.py b/backend/tests/test_conftest_db_url.py new file mode 100644 index 0000000..4bed76a --- /dev/null +++ b/backend/tests/test_conftest_db_url.py @@ -0,0 +1,18 @@ +"""Regression test for conftest.py's DATABASE_URL handling. + +The setup_db fixture drops all tables after every test, so the suite must +never run against an ambient DATABASE_URL from a developer's shell (which +could point at a real dev/prod database). conftest.py must force in-memory +SQLite unless the explicit TEST_DATABASE_URL opt-in is set (as CI does for +its Postgres service container). +""" + +import os + + +def test_database_url_is_test_url_or_sqlite(): + """After conftest import, DATABASE_URL is either the explicit + TEST_DATABASE_URL opt-in or the in-memory SQLite default — never an + ambient value inherited from the shell.""" + expected = os.environ.get("TEST_DATABASE_URL") or "sqlite+aiosqlite://" + assert os.environ["DATABASE_URL"] == expected