cal/paper-dynasty-database
1
0
Fork 0
Code Issues 7 Pull Requests 4 Actions Packages Projects Releases Wiki Activity

feat: SQL migration for evolution tables and variant/image_url columns (#69) #84

Closed
Claude wants to merge 2 commits from ai/paper-dynasty-database#69 into card-evolution
pull from: ai/paper-dynasty-database#69
merge into: cal:card-evolution
Conversation 4 Commits 2 Files Changed 4 +34
1 changed files with 34 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 094f445fea - Show all commits

34
tests/conftest.py
View File

@ -8,7 +8,41 @@ responsible for binding models to its own in-memory database.
import os
import psycopg2
import pytest
os.environ["DATABASE_TYPE"] = "postgresql"
# Provide dummy credentials so PooledPostgresqlDatabase can be instantiated
# without raising a configuration error (it will not actually be used).
os.environ.setdefault("POSTGRES_PASSWORD", "test-dummy")
@pytest.fixture(scope="session")
def pg_conn():
"""Live PostgreSQL connection for integration tests.
Requires POSTGRES_HOST to be set; runs the migration before yielding the
connection so that all integration tests start from a migrated schema.
The connection is rolled back and closed at session teardown so that the
migration is not persisted to the dev database between test runs.
"""
conn = psycopg2.connect(
host=os.environ["POSTGRES_HOST"],
dbname=os.environ.get("POSTGRES_DB", "pd_master"),
user=os.environ.get("POSTGRES_USER", "pd_admin"),
password=os.environ["POSTGRES_PASSWORD"],
port=int(os.environ.get("POSTGRES_PORT", "5432")),
)
conn.autocommit = False
migration_path = os.path.join(
cal commented 2026-03-13 13:15:01 +00:00
Review
Copy Link

The migration SQL contains BEGIN; ... COMMIT;. When this fixture calls conn.cursor().execute(fh.read()) and then conn.commit(), the COMMIT inside the SQL fires during execute and permanently durably commits the DDL. The conn.rollback() at teardown has no effect on the already-committed schema changes.

The docstring promise — "The connection is rolled back and closed at session teardown so that the migration is not persisted to the dev database between test runs" — is incorrect as written.

Fix options:

  • Strip BEGIN; / COMMIT; from the SQL before executing, so the transaction is under the fixture's control.
  • Or accept permanent migration and update the docstring to reflect that the integration test is run-once against dev (which may be the intended use).
The migration SQL contains `BEGIN; ... COMMIT;`. When this fixture calls `conn.cursor().execute(fh.read())` and then `conn.commit()`, the `COMMIT` inside the SQL fires during execute and permanently durably commits the DDL. The `conn.rollback()` at teardown has no effect on the already-committed schema changes. The docstring promise — *"The connection is rolled back and closed at session teardown so that the migration is not persisted to the dev database between test runs"* — is incorrect as written. Fix options: - Strip `BEGIN;` / `COMMIT;` from the SQL before executing, so the transaction is under the fixture's control. - Or accept permanent migration and update the docstring to reflect that the integration test is run-once against dev (which may be the intended use).
os.path.dirname(__file__),
"..",
"migrations",
"2026-03-12_add_evolution_tables.sql",
)
with open(migration_path) as fh:
conn.cursor().execute(fh.read())
conn.commit()
yield conn
conn.rollback()
conn.close()