refactor: extract test-transaction seam from production session#53
Merged
Conversation
CustomAsyncSession existed only so the per-test rollback worked: its close() became a no-op (expunge_all) when bound to a connection — a test concern leaking into the production data-access module. Remove it. Test-transaction ownership now lives in the db_session fixture via SQLAlchemy 2.0's native join_transaction_mode="create_savepoint". Each session owns its own savepoint; advanced-alchemy's auto_commit releases it while the fixture's outer transaction survives and is rolled back per test. This obsoletes both the close() override and the begin_nested() the old conditional_savepoint default 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). 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".Ports modern-python/litestar-sqlalchemy-template#28 to this template.
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 session takes the same mode.CLAUDE.md— update the Database-layer and Tests notes that documentedCustomAsyncSession/expunge_all/the nested savepoint.Verification
just test→ 19 passed, 100% coverage.just lint(ruff format/ruff check/ty check) clean.🤖 Generated with Claude Code