peter
0ca0cac534
Round 2 of the deep review, on round 1's own fixes:
- `ORDER BY position … FOR UPDATE` sorts before it locks, so the reorder that waited got the
fresh rows in its STALE order and dealt the pins it left out (a tab with Plex off) back to their
old slots. It locks in id order — one global order, so two reorders cannot deadlock — and sorts
the fresh rows itself. The race test now sends that partial list and fails unless tab B really
blocked.
- The request check saw only writes the handler flushed: a `db.delete` left staged, or a flush
followed by a released savepoint, passed. Staged changes count at the end of the request, and
only the ROOT transaction's end clears it (a savepoint test drives the dependency directly).
- The `ok` test exempted every call as a serializer, hiding four domain REPORTS (YouTube sync,
revert, push; Plex sync) without `ok`. They are spread into `{"ok": True, …}`; the exemption is a
named list of real serializers.
61 lines
2.7 KiB
Python
61 lines
2.7 KiB
Python
"""Dependency overrides go through `conftest.override_dep`, never `app.dependency_overrides` directly.
|
|
|
|
The hand-rolled shape — assign, then `app.dependency_overrides.clear()` in a `finally` — wipes EVERY
|
|
override, not the ones the test set, so it stops fixtures composing: one test's teardown silently
|
|
clobbers an override another fixture installed. `override_dep` restores each key's prior value
|
|
instead. 22 files had drifted to the hand-rolled shape (it was the house norm at ~20 files to 3), and
|
|
the sweep that converted them is only as good as the check that keeps it converted — this one.
|
|
"""
|
|
|
|
from pathlib import Path
|
|
|
|
TESTS = Path(__file__).parent
|
|
|
|
|
|
def test_no_test_writes_dependency_overrides_directly():
|
|
offenders = [
|
|
f"{p.name}:{n}"
|
|
for p in sorted(TESTS.rglob("*.py"))
|
|
if p.name not in {"conftest.py", Path(__file__).name}
|
|
for n, line in enumerate(p.read_text(encoding="utf-8").splitlines(), 1)
|
|
if "dependency_overrides" in line
|
|
]
|
|
assert offenders == [], "use the override_dep fixture: " + ", ".join(offenders)
|
|
|
|
|
|
def test_a_request_gets_the_session_that_checks_its_commit():
|
|
"""`get_db` is overridden with `db.info["request"]` — the test session plus the check that the
|
|
handler committed what it flushed (conftest `_request_dep`). A bare `lambda: db` hands the route
|
|
the same session without the check, and a handler that forgot its commit passes again."""
|
|
offenders = [
|
|
f"{p.name}:{n}"
|
|
for p in sorted(TESTS.rglob("*.py"))
|
|
if p.name != Path(__file__).name
|
|
for n, line in enumerate(p.read_text(encoding="utf-8").splitlines(), 1)
|
|
if "override_dep(get_db," in line and 'override_dep(get_db, db.info["request"])' not in line
|
|
]
|
|
assert offenders == [], 'override get_db with db.info["request"]: ' + ", ".join(offenders)
|
|
|
|
|
|
def test_the_commit_check_counts_only_the_root_transaction(db):
|
|
"""A savepoint released inside a handler commits nothing to the database; ending it must not
|
|
clear the check (round 2). Driven directly, as a request would drive the dependency."""
|
|
import pytest
|
|
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="savepoint-probe@example.com"))
|
|
session.flush()
|
|
with session.begin_nested():
|
|
pass
|
|
with pytest.raises(StopIteration):
|
|
next(request)
|
|
assert db.info["unfinished"] == ["POST /probe"]
|
|
db.info["unfinished"].clear() # recorded as it should be — not this test's failure
|