From a5fae489e08f267a7ea44ada5a817f999827425f Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sat, 19 Sep 2026 22:48:20 +0200 Subject: [PATCH] test: a request that raises is rolled back, as production rolls it back MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The shared test session kept whatever a failing handler flushed and never committed, so a later read in the test saw a write production loses when get_db closes the session — and a handler whose failure path must commit (E16: the tracker's issue link) looked right without the commit. --- backend/tests/conftest.py | 10 ++++++-- backend/tests/test_override_dep_convention.py | 24 +++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index d6c37c9a..1c817f11 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -111,8 +111,11 @@ def _request_dep(db, unfinished: list[str]): request that ends normally with a write it did not commit — flushed, or still only staged — is recorded, and the test fails on it at teardown. Only the ROOT transaction's end clears it: a savepoint released inside the handler commits nothing. The test's own staged rows are flushed before the request starts — they are the - test's business, not the handler's. A request that raises is exempt: its writes are meant to - roll back.""" + test's business, not the handler's. A request that raises is rolled back, as production's + `close()` rolls it back: without that, what a failing handler flushed but never committed stayed + readable to the rest of the test — the one state production cannot be in (E16: an issue created + on the tracker before a failed upload was kept only by an explicit commit, and nothing saw it + missing).""" def dep(conn: HTTPConnection): db.flush() @@ -133,6 +136,9 @@ def _request_dep(db, unfinished: list[str]): staged = db.new or db.deleted or any(db.is_modified(o) for o in db.dirty) if state["dirty"] or staged: unfinished.append(f"{conn.scope.get('method', 'WS')} {conn.url.path}") + except Exception: + db.rollback() + raise finally: for name, fn in hooks: event.remove(db, name, fn) diff --git a/backend/tests/test_override_dep_convention.py b/backend/tests/test_override_dep_convention.py index 4f032823..32eeb61a 100644 --- a/backend/tests/test_override_dep_convention.py +++ b/backend/tests/test_override_dep_convention.py @@ -58,3 +58,27 @@ def test_the_commit_check_counts_only_the_root_transaction(db): next(request) assert db.info["unfinished"] == ["POST /probe"] db.info["unfinished"].clear() # recorded as it should be — not this test's failure + + +def test_a_request_that_raises_leaves_nothing_it_did_not_commit(db): + """Production closes a failing request's session, which rolls back whatever it flushed and did + not commit; the shared test session must do the same, or a later read in the test sees a write + production would have lost (E16: the tracker's issue link, kept only by an explicit commit in + the handler's failure path). Driven directly, as a request would drive the dependency.""" + import pytest + from sqlalchemy import select + from starlette.requests import HTTPConnection + + from app.models import User + + conn = HTTPConnection( + {"type": "http", "method": "POST", "path": "/probe", "headers": [], "query_string": b""} + ) + request = db.info["request"](conn) + session = next(request) + session.add(User(email="rolled-back-probe@example.com")) + session.flush() + with pytest.raises(RuntimeError): + request.throw(RuntimeError("the handler failed")) + assert db.scalar(select(User).where(User.email == "rolled-back-probe@example.com")) is None + assert db.info["unfinished"] == [] # a failing request is rolled back, not reported