scope frontend contracts to the feature, not to one file
93 test files read a single frontend source and asserted identifiers in it. The
MapWorkspace split showed what that costs: 24 tests went red for a move that
changed no behaviour at all. A contract belongs to the feature — a container,
its hooks, its domain layer — not to whichever file currently holds it.
232 read sites now resolve through read_feature(). The distinction that makes
this safe is direction: a *positive* contract ("this is wired") may widen,
because the identifier must still exist somewhere in the feature; a *negative*
one ("this component performs no transport") is a statement about one file, and
widening it would quietly weaken the check. The 73 single-file reads that
remain are exactly those, and a guard now enforces the rule for new tests.
Verified rather than assumed: of the 732 migrated positive assertions, 644 still
match exactly one module — as specific as before — and the other 86 already
spanned a container and its hook by nature. Two apparent misses are an artefact
of the checking regex reading an escaped newline literally.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -73,3 +73,48 @@ def test_frontend_contract_helpers_are_available() -> None:
|
||||
assert_wired(source, "analyzeSelection")
|
||||
assert_calls(source, "analyzeSelection", first_argument="bbox")
|
||||
assert_mentions(source, "AREAIDFORSELECTION")
|
||||
|
||||
|
||||
FRONTEND_READ = re.compile(
|
||||
r'(?P<var>\w+)\s*=\s*\(?\s*(?:ROOT|root|REPO_ROOT)\s*/\s*'
|
||||
r'(?P<path>"[^"]+"(?:\s*/\s*"[^"]+")*)\s*\)?\.read_text\('
|
||||
)
|
||||
NEGATIVE_ASSERT = re.compile(r"assert\s+[^\n]*not in\s+(\w+)")
|
||||
|
||||
|
||||
def _feature_owner() -> dict[str, str]:
|
||||
from tests.frontend_contract import FEATURE_SOURCES
|
||||
|
||||
return {source: feature for feature, sources in FEATURE_SOURCES.items() for source in sources}
|
||||
|
||||
|
||||
def test_a_positive_contract_reads_the_feature_not_one_file() -> None:
|
||||
"""Moving code between sibling modules must not red the suite.
|
||||
|
||||
A single-file read is right for a *negative* contract — "this component
|
||||
performs no transport" is a statement about that file, and widening it
|
||||
would quietly weaken the check. For a positive contract it pins the
|
||||
contract to whichever file happens to hold it today.
|
||||
"""
|
||||
|
||||
owner = _feature_owner()
|
||||
offenders: list[str] = []
|
||||
|
||||
for path, source in _test_sources():
|
||||
if "frontend" not in source:
|
||||
continue
|
||||
negatives = set(NEGATIVE_ASSERT.findall(source))
|
||||
for match in FRONTEND_READ.finditer(source):
|
||||
if match.group("var") in negatives:
|
||||
continue
|
||||
joined = re.sub(r'["\s/]+', "/", match.group("path")).strip("/")
|
||||
if "frontend/src/" not in joined:
|
||||
continue
|
||||
relative = joined.split("frontend/src/", 1)[1]
|
||||
if relative in owner:
|
||||
offenders.append(f"{path.name}: {match.group('var')} -> {relative}")
|
||||
|
||||
assert not offenders, (
|
||||
"These read one file of a multi-module feature for a positive contract. "
|
||||
f"Use read_feature() from tests/frontend_contract.py instead: {sorted(offenders)}"
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user