peter
4aaaaa7860
The code cited its plan instead of saying why: `E28 review J4`, `(R6 S3b)`, `S1 justified…`,
`deep-review round 5`. That plan lives in the private ops repo, so for every reader of this one the
citation was noise, and a sentence whose subject was the plan step ("round 2 showed…") hid its
lesson behind an id. Each one is gone and the "why" kept: a bare citation is deleted, a sentence
carrying a lesson is reworded around it ("a later attempt showed…").
Measured (`scripts/audit_metrics.py`, product code): 652 lines in 270 files carried an epic code by
the definition the next commit holds (507 / 241 by the old, two-digit-only one) -> 0. Across all
code, tests, scripts and configs: ~1100 epic-code lines plus ~320 sprint/round ids.
Behaviour unchanged, checked mechanically: every changed .py file has the same AST once docstrings
are blanked; css/sh/toml/yml/json files are identical once comments are stripped; the only
non-comment changes in TS/JS are test titles, one description string in a contract table, and five
ESLint messages, whose text is all the gates snapshot diff holds. Not touched: Markdown (docs and
the changelog keep their history), Alembic migrations, TV episode notation (`S2 · E5`), DESIGN.md
ids (`C1`, `B4`, `S3h`), tracker ids (`SEC-16`, `DX-17`).
One stale claim fixed on the way: test_hls_reaper's docstring said the player checkpoints "every
10 s whether or not it is playing", which plex/stream.py calls wrong; it now says what the code does.
85 lines
3.8 KiB
Python
85 lines
3.8 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. 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
|
|
|
|
|
|
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 (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
|