fix(tooling): --check-evidence sees #### headings, JS tests, and the reverse direction (improvise-5ray, improvise-lyyc)
Two blind spots with one consequence: an evidence file could promise a guard
that was not there, and a test could point at prose that did not exist.
5ray: headings were collected with `startswith("### ")` and resolved against
Rust fn names only. context/test-evidence/clients.md uses `#### "name"` for
every browser test precisely BECAUSE a `###` naming a Playwright test would
have been reported as an orphan -- so the convention that kept the checker
quiet was the same one that made it blind. Now `###` and `####` alike, decided
by SHAPE: a bare identifier is a Rust test, a quoted string is a JS test
(harness/tests and web/tests), anything else is prose naming nothing.
lyyc: the check was one-directional. A test whose doc comment names
context/test-evidence must now appear there. Deliberately only that narrower
obligation -- most tests have no evidence and should not be forced to. Neither
direction can see a STALE entry; that stays discipline.
Each half found a real instance on its first run. A `#### "? opens help..."`
heading outlived the test 36ade35 deleted, and is demoted to prose with the
coverage filed as improvise-v5pc. `an_entry_that_holds_no_document_is_seeded_over`
cited the file and had no entry; one is written from the test.
The checker's --self-test covered the comment scanner and nothing else, which
is how both survived. It grows ten evidence cases.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
This commit is contained in:
Edward Langley
+241
-55
@@ -249,76 +249,188 @@ def _is_test_attribute(line):
|
||||
|
||||
|
||||
EVIDENCE_DIR = os.path.join("context", "test-evidence")
|
||||
EVIDENCE_CITE = "test-evidence"
|
||||
JS_ROOTS = [os.path.join("harness", "tests"), os.path.join("web", "tests")]
|
||||
|
||||
_RUST_NAME = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$")
|
||||
_QUOTED_NAME = re.compile(r'^"(.+)"$')
|
||||
_JS_TEST = re.compile(
|
||||
r"""(?:^|[^\w.])(?:test|it)(?:\.(?:describe|skip|only|todo))?\s*\(\s*(['"`])(.+?)\1"""
|
||||
)
|
||||
|
||||
|
||||
def evidence_claims(text):
|
||||
"""Yield (kind, name, lineno) for every heading in an evidence file.
|
||||
|
||||
`### ` and `#### ` alike, because the browser entries have to use `####`
|
||||
and were invisible to a check that collected only `### ` (improvise-5ray).
|
||||
A heading is a CLAIM when it is shaped like a test name -- a bare
|
||||
identifier (Rust) or a quoted string (JS). Anything else is prose about
|
||||
the section: `Helper - x`, `Note - y`, a file's own scope. That shape rule
|
||||
is the whole discrimination, so see context/code/tooling.md before loosening
|
||||
it: a heading that is neither shape names nothing and is checked by nobody.
|
||||
"""
|
||||
for n, raw in enumerate(text.split("\n"), 1):
|
||||
s = raw.strip()
|
||||
if not (s.startswith("### ") or s.startswith("#### ")):
|
||||
continue
|
||||
body = s.lstrip("#").strip().strip("`").strip()
|
||||
m = _QUOTED_NAME.match(body)
|
||||
if m:
|
||||
yield "js", m.group(1), n
|
||||
elif _RUST_NAME.match(body):
|
||||
yield "rust", body, n
|
||||
|
||||
|
||||
def rust_tests(text):
|
||||
"""(tests defined here, tests whose doc comment cites the evidence dir).
|
||||
|
||||
A heading must name a TEST, not merely a function. Accepting any `fn` let a
|
||||
`###` naming a fixture or a helper pass, so an evidence file could report
|
||||
itself satisfied while naming nothing that runs (found on PR #39).
|
||||
"""
|
||||
defined, citing = set(), set()
|
||||
annotated = False
|
||||
doc = []
|
||||
# A `proptest!` block accepts its bodies as tests -- but only INSIDE the
|
||||
# block. A bare flag was never cleared, so the first proptest! in a file
|
||||
# made every later fn, helpers included, count as a test: a silent
|
||||
# success in the gate itself (found by review on PR #40). Track the
|
||||
# block's braces and leave when they balance.
|
||||
proptest_depth = None
|
||||
for raw in text.split("\n"):
|
||||
s = raw.strip()
|
||||
if proptest_depth is not None:
|
||||
proptest_depth += raw.count("{") - raw.count("}")
|
||||
if proptest_depth <= 0:
|
||||
proptest_depth = None
|
||||
elif s.startswith("proptest!"):
|
||||
proptest_depth = raw.count("{") - raw.count("}")
|
||||
if proptest_depth <= 0:
|
||||
proptest_depth = None if "{" in raw else 0
|
||||
in_proptest = proptest_depth is not None
|
||||
if s.startswith("//"):
|
||||
doc.append(s)
|
||||
continue
|
||||
# ANCHOR the attribute. `"test" in s` also matched `#[cfg(test)]`,
|
||||
# so every cfg-gated fixture cleared a gate written to reject
|
||||
# exactly those -- the third instance of the unanchored-substring
|
||||
# class in this campaign, after the bd audit's exclusion list.
|
||||
if s.startswith("#[") and _is_test_attribute(s):
|
||||
annotated = True
|
||||
continue
|
||||
if s.startswith("fn ") or s.startswith("pub fn ") or s.startswith("async fn "):
|
||||
if annotated or in_proptest:
|
||||
name = s.split("fn ", 1)[1].split("(")[0].split("<")[0].strip()
|
||||
defined.add(name)
|
||||
if any(EVIDENCE_CITE in d for d in doc):
|
||||
citing.add(name)
|
||||
annotated = False
|
||||
doc = []
|
||||
elif s and not s.startswith("#["):
|
||||
annotated = False
|
||||
doc = []
|
||||
return defined, citing
|
||||
|
||||
|
||||
def js_tests(text):
|
||||
"""(tests defined here, tests whose preceding comment cites the evidence dir).
|
||||
|
||||
The JS half exists because `defined` held only Rust `fn` names, so a
|
||||
`####` naming a Playwright or node test could never have resolved even if
|
||||
the heading had been collected -- the convention that kept the checker
|
||||
quiet was the same one that made it blind (improvise-5ray).
|
||||
"""
|
||||
defined, citing = set(), set()
|
||||
doc = []
|
||||
for raw in text.split("\n"):
|
||||
s = raw.strip()
|
||||
m = _JS_TEST.search(raw)
|
||||
if m:
|
||||
defined.add(m.group(2))
|
||||
if any(EVIDENCE_CITE in d for d in doc):
|
||||
citing.add(m.group(2))
|
||||
doc = []
|
||||
continue
|
||||
if s.startswith("//") or s.startswith("*") or s.startswith("/*"):
|
||||
doc.append(s)
|
||||
elif s:
|
||||
doc = []
|
||||
return defined, citing
|
||||
|
||||
|
||||
def js_files(roots):
|
||||
for root in roots:
|
||||
for dirpath, dirnames, filenames in os.walk(root):
|
||||
dirnames[:] = [d for d in dirnames if d != "node_modules"]
|
||||
for f in filenames:
|
||||
if f.endswith(".spec.js") or f.endswith(".test.js"):
|
||||
yield os.path.join(dirpath, f)
|
||||
|
||||
|
||||
def evidence_verdict(claims, defined, citing):
|
||||
"""(orphans, unstated) -- the two directions, keyed by ("rust"|"js", name).
|
||||
|
||||
`orphans` is evidence naming a test nobody can find. `unstated` is the
|
||||
reverse obligation (improvise-lyyc): a test that POINTS AT the evidence
|
||||
file must appear in it, or a reader following the pointer arrives nowhere.
|
||||
Only that narrower obligation, because most tests have no evidence and
|
||||
should not be forced to. Note what neither direction can see: an entry can
|
||||
exist and still describe its subject backwards, which is discipline rather
|
||||
than a gate.
|
||||
"""
|
||||
orphans = {k: v for k, v in claims.items() if k[1] not in defined[k[0]]}
|
||||
unstated = {}
|
||||
for kind, names in citing.items():
|
||||
for name in names:
|
||||
if (kind, name) not in claims:
|
||||
unstated.setdefault((kind, name), []).extend(sorted(names[name]))
|
||||
return orphans, unstated
|
||||
|
||||
|
||||
def check_evidence(roots):
|
||||
"""Every `### name` in context/test-evidence/ must name a test that exists.
|
||||
|
||||
One direction only, deliberately: a test may have no evidence (most do
|
||||
not), but evidence naming a test nobody can find is an orphan -- which is
|
||||
the cost design-principles names for splitting the narrative from the test.
|
||||
"""
|
||||
"""Both directions over context/test-evidence/. See evidence_verdict."""
|
||||
if not os.path.isdir(EVIDENCE_DIR):
|
||||
print("no context/test-evidence/ yet: nothing to check")
|
||||
return 0
|
||||
|
||||
claimed = {}
|
||||
claims = {}
|
||||
for dirpath, _, filenames in os.walk(EVIDENCE_DIR):
|
||||
for f in sorted(filenames):
|
||||
if not f.endswith(".md"):
|
||||
continue
|
||||
path = os.path.join(dirpath, f)
|
||||
for n, raw in enumerate(open(path, encoding="utf8", errors="replace"), 1):
|
||||
s = raw.strip()
|
||||
if s.startswith("### "):
|
||||
claimed.setdefault(s[4:].strip().strip("`"), []).append(f"{path}:{n}")
|
||||
text = open(path, encoding="utf8", errors="replace").read()
|
||||
for kind, name, n in evidence_claims(text):
|
||||
claims.setdefault((kind, name), []).append(f"{path}:{n}")
|
||||
|
||||
# A heading must name a TEST, not merely a function. Accepting any `fn`
|
||||
# let a `###` naming a fixture or a helper pass, so an evidence file could
|
||||
# report itself satisfied while naming nothing that runs (found on PR #39).
|
||||
defined = set()
|
||||
defined = {"rust": set(), "js": set()}
|
||||
citing = {"rust": {}, "js": {}}
|
||||
for path in rust_files(roots):
|
||||
lines = open(path, encoding="utf8", errors="replace").read().split("\n")
|
||||
annotated = False
|
||||
# A `proptest!` block accepts its bodies as tests -- but only INSIDE the
|
||||
# block. A bare flag was never cleared, so the first proptest! in a file
|
||||
# made every later fn, helpers included, count as a test: a silent
|
||||
# success in the gate itself (found by review on PR #40). Track the
|
||||
# block's braces and leave when they balance.
|
||||
proptest_depth = None
|
||||
for raw in lines:
|
||||
s = raw.strip()
|
||||
if proptest_depth is not None:
|
||||
proptest_depth += raw.count("{") - raw.count("}")
|
||||
if proptest_depth <= 0:
|
||||
proptest_depth = None
|
||||
elif s.startswith("proptest!"):
|
||||
proptest_depth = raw.count("{") - raw.count("}")
|
||||
if proptest_depth <= 0:
|
||||
proptest_depth = None if "{" in raw else 0
|
||||
in_proptest = proptest_depth is not None
|
||||
# ANCHOR the attribute. `"test" in s` also matched `#[cfg(test)]`,
|
||||
# so every cfg-gated fixture cleared a gate written to reject
|
||||
# exactly those -- the third instance of the unanchored-substring
|
||||
# class in this campaign, after the bd audit's exclusion list.
|
||||
if s.startswith("#[") and _is_test_attribute(s):
|
||||
annotated = True
|
||||
continue
|
||||
if s.startswith("fn ") or s.startswith("pub fn ") or s.startswith("async fn "):
|
||||
if annotated or in_proptest:
|
||||
name = s.split("fn ", 1)[1].split("(")[0].split("<")[0].strip()
|
||||
defined.add(name)
|
||||
annotated = False
|
||||
elif s and not s.startswith("#[") and not s.startswith("//"):
|
||||
annotated = False
|
||||
d, c = rust_tests(open(path, encoding="utf8", errors="replace").read())
|
||||
defined["rust"] |= d
|
||||
for name in c:
|
||||
citing["rust"].setdefault(name, []).append(path)
|
||||
for path in js_files(JS_ROOTS):
|
||||
d, c = js_tests(open(path, encoding="utf8", errors="replace").read())
|
||||
defined["js"] |= d
|
||||
for name in c:
|
||||
citing["js"].setdefault(name, []).append(path)
|
||||
|
||||
orphans = {k: v for k, v in claimed.items() if k not in defined}
|
||||
if not orphans:
|
||||
print(f"test-evidence: {len(claimed)} entries, all name a test that exists")
|
||||
orphans, unstated = evidence_verdict(claims, defined, citing)
|
||||
if not orphans and not unstated:
|
||||
print(f"test-evidence: {len(claims)} entries, all name a test that exists; "
|
||||
f"{sum(len(v) for v in citing.values())} tests cite the file and all have one")
|
||||
return 0
|
||||
print(f"{len(orphans)} test-evidence entries name no test:\n", file=sys.stderr)
|
||||
for name, where in sorted(orphans.items()):
|
||||
print(f" {name} ({', '.join(where)})", file=sys.stderr)
|
||||
if orphans:
|
||||
print(f"{len(orphans)} test-evidence entries name no test:\n", file=sys.stderr)
|
||||
for (kind, name), where in sorted(orphans.items()):
|
||||
print(f" [{kind}] {name} ({', '.join(where)})", file=sys.stderr)
|
||||
if unstated:
|
||||
print(f"\n{len(unstated)} tests cite {EVIDENCE_DIR} and have no entry there:\n",
|
||||
file=sys.stderr)
|
||||
for (kind, name), where in sorted(unstated.items()):
|
||||
print(f" [{kind}] {name} ({', '.join(where)})", file=sys.stderr)
|
||||
return 1
|
||||
|
||||
|
||||
@@ -553,6 +665,78 @@ fn g() {}
|
||||
]
|
||||
|
||||
|
||||
# (name, evidence markdown, rust source, js source, orphans, unstated).
|
||||
#
|
||||
# The evidence checker had no self-test at all, which is how both halves of
|
||||
# improvise-5ray and improvise-lyyc survived: the shape that silenced the
|
||||
# checker was the shape it could not see. Each case below is an injection.
|
||||
SELF_TEST_EVIDENCE = [
|
||||
("a #### heading naming no test is an orphan",
|
||||
'#### "a test that does not exist anywhere"\n',
|
||||
"", "", ["a test that does not exist anywhere"], []),
|
||||
|
||||
("a #### heading naming a JS test that exists is satisfied",
|
||||
'#### "a peer that cannot be reached is named"\n',
|
||||
"", 'test("a peer that cannot be reached is named", async () => {});\n', [], []),
|
||||
|
||||
("a quoted heading can never be satisfied by a Rust fn of that name",
|
||||
'#### "a_rust_name"\n',
|
||||
"#[test]\nfn a_rust_name() {}\n", "", ["a_rust_name"], []),
|
||||
|
||||
("a prose #### heading claims nothing",
|
||||
"#### Helper — `addressed`\n#### Note — the basis_check module\n"
|
||||
"#### harness/tests/shell.spec.js — the file's own scope\n",
|
||||
"", "", [], []),
|
||||
|
||||
("a ### naming a helper rather than a test is still an orphan",
|
||||
"### a_helper\n", "fn a_helper() {}\n", "", ["a_helper"], []),
|
||||
|
||||
("a test that cites the evidence file and has no entry is caught",
|
||||
"", "/// See `context/test-evidence/clients.md`.\n#[test]\nfn a_cited_test() {}\n",
|
||||
"", [], ["a_cited_test"]),
|
||||
|
||||
("the same test with an entry is satisfied",
|
||||
"### a_cited_test\n",
|
||||
"/// See `context/test-evidence/clients.md`.\n#[test]\nfn a_cited_test() {}\n",
|
||||
"", [], []),
|
||||
|
||||
("a NON-test fn citing the file is under no obligation",
|
||||
"", "/// See `context/test-evidence/clients.md`.\nfn a_helper() {}\n", "", [], []),
|
||||
|
||||
("a citation belonging to the fn ABOVE does not attach to the next one",
|
||||
"### cited\n",
|
||||
"/// See `context/test-evidence/clients.md`.\n#[test]\nfn cited() {}\n"
|
||||
"\n#[test]\nfn uncited() {}\n",
|
||||
"", [], []),
|
||||
|
||||
("a JS test that cites the evidence file and has no entry is caught",
|
||||
"", "",
|
||||
'// See context/test-evidence/clients.md.\ntest("a cited spec", async () => {});\n',
|
||||
[], ["a cited spec"]),
|
||||
]
|
||||
|
||||
|
||||
def _evidence_self_test():
|
||||
failures = []
|
||||
for name, md, rs, js, want_orphans, want_unstated in SELF_TEST_EVIDENCE:
|
||||
claims = {}
|
||||
for kind, cname, n in evidence_claims(md):
|
||||
claims.setdefault((kind, cname), []).append(f"fixture:{n}")
|
||||
rd, rc = rust_tests(rs)
|
||||
jd, jc = js_tests(js)
|
||||
defined = {"rust": rd, "js": jd}
|
||||
citing = {"rust": {k: ["fixture.rs"] for k in rc},
|
||||
"js": {k: ["fixture.js"] for k in jc}}
|
||||
orphans, unstated = evidence_verdict(claims, defined, citing)
|
||||
got_o = sorted(k[1] for k in orphans)
|
||||
got_u = sorted(k[1] for k in unstated)
|
||||
if got_o != sorted(want_orphans):
|
||||
failures.append(f" ORPHANS: {name}: expected {sorted(want_orphans)}, saw {got_o}")
|
||||
if got_u != sorted(want_unstated):
|
||||
failures.append(f" UNSTATED: {name}: expected {sorted(want_unstated)}, saw {got_u}")
|
||||
return failures
|
||||
|
||||
|
||||
def self_test():
|
||||
"""Check the checker against every bypass it has already been fixed for.
|
||||
|
||||
@@ -569,12 +753,14 @@ def self_test():
|
||||
runs = [n for n in comment_runs(src.split("\n")) if n[1] > LIMIT]
|
||||
if runs:
|
||||
failures.append(f" FALSE POSITIVE: {name}: flagged {runs}")
|
||||
failures += _evidence_self_test()
|
||||
if failures:
|
||||
print("checker self-test FAILED:", file=sys.stderr)
|
||||
print("\n".join(failures), file=sys.stderr)
|
||||
return 1
|
||||
print(f"checker self-test: {len(SELF_TEST_CASES)} violations caught, "
|
||||
f"{len(SELF_TEST_CLEAN)} legitimate shapes allowed")
|
||||
f"{len(SELF_TEST_CLEAN)} legitimate shapes allowed, "
|
||||
f"{len(SELF_TEST_EVIDENCE)} evidence verdicts")
|
||||
return 0
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user