Commit 70209824 by PLN (Algolia)

test(hygiene): a guard against suites that are green because they run nothing

Four suites in this workspace were found hollow in one week, each hollow in a
different way, and every one of them had read as fine for months. The pattern
is not a bug in any of them — it is that a test suite reports on the code and
nothing reports on the suite.

So: tools/tests/test_suite_hygiene.py walks every test_*.py by AST (no imports,
so it works with or without any project venv) and asserts three things.

  * No file collects zero tests. A test_*.py with no collectable function is
    green and hollow.
  * No test requests a fixture nothing declares. `def test_x(suite)` makes
    pytest error on a missing fixture, and twelve such errors read as a config
    nit for months while twelve real audio cases went unrun.
  * No module-level sys.exit(). It fires during COLLECTION and takes the whole
    directory down instead of failing one test — which is how `pytest
    tools/tests/` once ran zero tests while 273 passed individually.

It also guards itself: if the glob ever stops finding files, every rule above
passes vacuously, so the file count has a floor of its own.

Verified by mutation, three planted defects, one per rule — a hollow file, a
`def test_thing(suite)`, and a trailing sys.exit(0). Each is caught by name and
the tree comes back clean.

Also splits tools/tests/test_setlist.py, whose 22 checks reported as one
aggregate assertion that named the file and not the fault. check() now records
each outcome and pytest replays them one named test per check; the aggregate
stays as the belt to those braces, and the file still runs as a script.

Every count is pinned, because the failure being guarded against is not a wrong
answer but an empty one, and an empty one is invisible without a number to
compare against. That guard earned itself immediately: I had counted 28 checks
by eye and the truth was 22.
parent be61d56c
......@@ -19,6 +19,8 @@ from __future__ import annotations
import importlib.util
import pathlib
import sys
import pytest
import tempfile
ROOT = pathlib.Path(__file__).resolve().parent.parent.parent
......@@ -31,10 +33,12 @@ sys.modules["setlist"] = S
spec.loader.exec_module(S)
FAILED: list[str] = []
CHECKS: list[tuple[str, bool, str]] = [] # replayed below as one test each
def check(name: str, cond: bool, detail: str = "") -> None:
print(f" {'ok ' if cond else 'FAIL'} {name}" + (f" {detail}" if detail else ""))
CHECKS.append((name, bool(cond), detail))
if not cond:
FAILED.append(name)
......@@ -160,6 +164,24 @@ check("the generated .txt round-trips through the parser",
print(f"\n{len(FAILED)} failed" if FAILED else "\nall passed")
@pytest.mark.parametrize(
"name,passed,detail", CHECKS, ids=[c[0] for c in CHECKS],
)
def test_backlog_setlist_check(name, passed, detail):
"""One named test per check, so a failure says WHICH check.
The aggregate below stays as the belt to this file's braces, but 22 checks
reporting as a single test named the file and not the fault.
"""
assert passed, detail or name
def test_every_check_was_recorded():
"""If the import-time block stops running, every test above vanishes
silently and this file goes green while asserting nothing."""
assert len(CHECKS) == 22, f"{len(CHECKS)} checks ran, expected 22"
def test_backlog_setlist_checks_all_pass():
"""Expose this plain-script suite to pytest.
......
#!/usr/bin/env python3
"""Guard against test suites that are green because they run nothing.
Four suites in this workspace were found hollow in one week, each hollow in a
different way, and each one had looked fine for months:
* `test_*(suite)` — pytest reads the parameter as a fixture request, finds no
such fixture, and reports an ERROR. Twelve of them read as a config nit,
so twelve real audio cases went unrun (tidal-ears/tests/test_adversarial).
* checks that run at import via a `check()` helper, with no `def test_*` at
all — pytest collects zero and reports success (test_gain_for_stem, and
tools/tests/test_setlist before it was split).
* a module-level `sys.exit()` in such a file — the first genuine regression
aborts collection of the whole directory instead of failing one test.
* dozens of checks behind ONE aggregate assertion, so a failure names the
file and not the fault.
The common thread is that a test suite reports on the code but nothing reports
on the suite. Green is not evidence; a number you can compare is. So this file
counts.
Run standalone: python3 tools/tests/test_suite_hygiene.py
"""
from __future__ import annotations
import ast
import sys
from pathlib import Path
REPO = Path(__file__).resolve().parents[2]
SKIP_PARTS = {".venv", "node_modules", ".git", "worktrees", "site-packages"}
# Fixtures pytest itself provides; no conftest declares these.
BUILTIN_FIXTURES = {
"tmp_path", "tmp_path_factory", "tmpdir", "tmpdir_factory", "capsys",
"capsysbinary", "capfd", "capfdbinary", "monkeypatch", "request", "caplog",
"recwarn", "pytestconfig", "cache", "doctest_namespace", "record_property",
"record_testsuite_property", "subtests", "event_loop",
}
def _skip(path: Path) -> bool:
return any(part in SKIP_PARTS for part in path.parts)
def test_files() -> list[Path]:
return sorted(p for p in REPO.rglob("test_*.py") if not _skip(p))
def _parse(path: Path) -> ast.Module | None:
try:
return ast.parse(path.read_text(encoding="utf-8"))
except (SyntaxError, UnicodeDecodeError):
return None
def _fixtures(tree: ast.Module) -> set[str]:
return {
node.name
for node in ast.walk(tree)
if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef))
and any("fixture" in ast.unparse(d) for d in node.decorator_list)
}
def _conftest_fixtures() -> set[str]:
out = set(BUILTIN_FIXTURES)
for c in REPO.rglob("conftest.py"):
if _skip(c):
continue
tree = _parse(c)
if tree is not None:
out |= _fixtures(tree)
return out
def _test_functions(tree: ast.Module) -> list[ast.FunctionDef]:
"""Everything pytest would collect: module functions and Test* methods."""
found = []
for node in tree.body:
if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)) and node.name.startswith("test"):
found.append(node)
elif isinstance(node, ast.ClassDef) and node.name.startswith("Test"):
if any("__test__" in ast.unparse(s) for s in node.body):
continue # opted out of collection on purpose
for sub in node.body:
if isinstance(sub, (ast.FunctionDef, ast.AsyncFunctionDef)) and sub.name.startswith("test"):
found.append(sub)
return found
def _audit() -> tuple[list[str], list[str], list[str]]:
conf = _conftest_fixtures()
hollow, uncallable_, module_exit = [], [], []
for path in test_files():
rel = path.relative_to(REPO)
tree = _parse(path)
if tree is None:
continue
tests = _test_functions(tree)
if not tests:
hollow.append(str(rel))
known = _fixtures(tree) | conf
for fn in tests:
if any("parametrize" in ast.unparse(d) for d in fn.decorator_list):
continue
args = [a.arg for a in fn.args.args if a.arg != "self"]
missing = [a for a in args if a not in known]
if missing:
uncallable_.append(f"{rel}::{fn.name} needs {missing}")
# A sys.exit() reachable at import time aborts collection.
for node in tree.body:
if isinstance(node, ast.Expr) and isinstance(node.value, ast.Call):
if ast.unparse(node.value).startswith(("sys.exit", "exit(")):
module_exit.append(f"{rel}:{node.lineno}")
return hollow, uncallable_, module_exit
HOLLOW, UNCALLABLE, MODULE_EXIT = _audit()
def test_the_audit_actually_scanned_something():
"""The guard's own hollowness check. If the glob breaks, every test below
passes vacuously — which is the exact failure being guarded against."""
files = test_files()
assert len(files) >= 40, f"only {len(files)} test files found under {REPO}"
def test_no_suite_collects_zero_tests():
"""A file named test_*.py with no collectable test is green and hollow."""
assert not HOLLOW, (
"these files collect zero tests, so they assert nothing:\n "
+ "\n ".join(HOLLOW)
)
def test_no_test_takes_an_undeclared_fixture():
"""`def test_x(suite)` errors on a missing fixture; twelve such errors read
as a config nit for months."""
assert not UNCALLABLE, (
"these tests request fixtures nothing declares, so pytest errors "
"instead of running them:\n " + "\n ".join(UNCALLABLE)
)
def test_no_suite_exits_at_import():
"""A module-level sys.exit() fires during COLLECTION, taking the whole
directory down instead of failing one test. Guard it under __main__."""
assert not MODULE_EXIT, (
"module-level sys.exit() aborts collection; put it under "
"`if __name__ == \"__main__\":`\n " + "\n ".join(MODULE_EXIT)
)
if __name__ == "__main__":
print(f"scanned {len(test_files())} test files under {REPO}")
for label, rows in (("HOLLOW", HOLLOW), ("UNCALLABLE", UNCALLABLE),
("EXITS AT IMPORT", MODULE_EXIT)):
print(f"\n{label} ({len(rows)}):")
for r in rows:
print(f" {r}")
sys.exit(1 if (HOLLOW or UNCALLABLE or MODULE_EXIT) else 0)
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment