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.
59 lines
2.4 KiB
Python
59 lines
2.4 KiB
Python
"""No test sends a request and throws the response away.
|
|
|
|
A discarded `client.post(...)` passes whatever it got back. As a SETUP step, a failure leaves the
|
|
test asserting on a state it never reached; as the step UNDER test, the test proves nothing. We
|
|
found `test_youtube_sources_never_consult_the_guard` passing on a 400 that stopped the request
|
|
before the code it meant to exercise, and 53 more discarded responses across the suite. Each one
|
|
now states the status it expects; this keeps it that way.
|
|
"""
|
|
|
|
import ast
|
|
from pathlib import Path
|
|
|
|
TESTS = Path(__file__).resolve().parent
|
|
VERBS = {"get", "post", "put", "patch", "delete", "head", "options", "request"}
|
|
|
|
|
|
def discarded_requests(source: str) -> list[int]:
|
|
"""Line numbers of the statements that are a bare request call on an app path."""
|
|
hits = []
|
|
for node in ast.walk(ast.parse(source)):
|
|
if not (isinstance(node, ast.Expr) and isinstance(node.value, ast.Call)):
|
|
continue
|
|
func = node.value.func
|
|
if not (isinstance(func, ast.Attribute) and func.attr in VERBS):
|
|
continue
|
|
path = node.value.args[0] if node.value.args else None
|
|
literal = path.value if isinstance(path, ast.Constant) else None
|
|
if isinstance(path, ast.JoinedStr):
|
|
first = path.values[0] if path.values else None
|
|
literal = first.value if isinstance(first, ast.Constant) else None
|
|
if isinstance(literal, str) and literal.startswith(("/api", "/auth")):
|
|
hits.append(node.lineno)
|
|
return hits
|
|
|
|
|
|
def test_the_detector_sees_a_discarded_request_and_passes_a_checked_one():
|
|
src = (
|
|
"client.post('/api/x')\n"
|
|
"client.get(f'/auth/{u}')\n"
|
|
"assert client.post('/api/x').status_code == 200\n"
|
|
"r = client.get('/api/y')\n"
|
|
"items.get('/api/not-a-request-but-a-key')\n"
|
|
)
|
|
# The last line is a false positive by design: a dict keyed by an app path is not a thing the
|
|
# suite does, and a narrower match would miss a client held under any other name.
|
|
assert discarded_requests(src) == [1, 2, 5]
|
|
|
|
|
|
def test_no_test_discards_a_response():
|
|
offenders = [
|
|
f"{p.name}:{line}"
|
|
for p in sorted(TESTS.rglob("test_*.py"))
|
|
for line in discarded_requests(p.read_text(encoding="utf-8"))
|
|
]
|
|
assert offenders == [], (
|
|
"assert the status these requests expect (`assert client.post(...).status_code == 200`), "
|
|
f"or keep the response and check it: {offenders}"
|
|
)
|