refactor: extract test-transaction seam from production session#28
Merged
Conversation
CustomAsyncSession existed only so the per-test rollback worked: its close() became a no-op when bound to a connection. That is a test concern living in the production data-access module. Replace the hack with SQLAlchemy 2.0's join_transaction_mode="create_savepoint". Each session owns its savepoint, so auto_commit releases a savepoint while the fixture's outer transaction survives and is rolled back per test. The kwarg is inert in production (the session binds to an engine, never an in-transaction connection). Production session is now a plain AsyncSession with no test branch; the conftest fixture drops begin_nested() and owns the savepoint lifecycle via the same mode. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
CustomAsyncSessionexisted for one reason: so the per-test rollback worked. Itsclose()became a no-op (expunge_all) when the session was bound to a connection — i.e. a test concern leaking into the production data-access module (app/resources/db.py).This removes it. Test-transaction ownership now lives where it belongs (the
db_sessionfixture), using SQLAlchemy 2.0's nativejoin_transaction_mode="create_savepoint".How
With
create_savepoint, each session owns its own savepoint. Advanced-alchemy'sauto_commitreleases that savepoint while the fixture's outer real transaction survives and is rolled back at the end of each test. This obsoletes both theclose()override and thebegin_nested()that the oldconditional_savepointdefault required.The kwarg is inert in production: it only takes effect when the session is bound to a connection already in a transaction, which production never does (it binds to an engine).
Changes
app/resources/db.py— deleteCustomAsyncSession;create_sessionreturns a plainAsyncSession(..., join_transaction_mode="create_savepoint")(commented to explain the prod-inert rationale).tests/conftest.py— dropconnection.begin_nested(); the fixture's polyfactory session takes the same mode.Production session has no test branch any more; per-request-session fidelity is preserved (the fixture still overrides
database_engine→ connection, so each request builds its own session).Verification
just test→ 19 passed, 100% coverage. Isolation is exercised:test_get_decks_empty/test_get_cards_emptyassert emptiness and would fail if a prior test's writes leaked.ruff format/ruff check/ty checkclean.🤖 Generated with Claude Code