test(ci): sweep every module for dangling imports (t41)
Two defects shipped with a green suite: button.py imported a helper that had been deleted (no harness ever loaded the file), and the drift sweep only judged `homeassistant.*`, so a relative import of a name that no longer exists was invisible to it. Both are now impossible to ship silently. * tests/test_module_coverage.py loads EVERY *.py file of the package, in a clean interpreter, under the stub harness — exhaustive by construction (a new platform is covered the moment the file exists; there is no list and no special case). It asserts the reported set equals the files on disk, so a silently skipped module fails too; * entity_stublib.load_entity_modules() now also loads button.py (the named gap); the sweep is what keeps that list from needing to be complete; * ha_api_drift gained pass 5: the package's own relative imports. Pure AST, no homeassistant required, with its own entry point `scan_package_imports()` for the fast suite — it checks the target file exists and defines every imported name (submodule imports still resolve), instead of trusting that the modules a harness happened to load are the ones that matter; * the file walk in both AST passes is recursive, and the non-vacuity floors moved with it (modules 10 → 20, relative_imports ≥ 100 — a shallow-glob regression trips the floor instead of looking green); * two permanent red proofs, both on a copy of the tree with the reverted `from .helpers import discover_adb_host` reference in button.py: the stub-harness sweep fails with "FAIL button.py", and the static pass fails with "button.py:1: broken-relative-import: … not defined in helpers.py". Manual runs confirm the engine's other direction on the real pre-fix tree (c588a7c: exit 1 with the t40 entity-contract finding) and exit 0 on HEAD. No Home-Assistant API drift (23 modules, 58 imports, 148 relative imports, 36 call arguments, 6 awaited HA calls). Verified: 334 passed / 2 skipped, 335 / 1 with androidtvremote2, ruff clean. Note (pre-existing, not introduced here): running a SUBSET of test files in an order the suite was not written for (e.g. test_init.py alone after test_standalone_setup.py) fails 3 import-step tests with "ModuleNotFoundError: No module named 'voluptuous'" — the per-file stub scoping assumes the suite's order. Verified identical on the pre-t41 tree (60d884b), so it is left alone here.
This commit is contained in:
1 parent
151c49a0da
commit
130dee4bc6
4 files changed
+449
-10
No files matched your search
@@ -0,0 +1,198 @@
|
||||
"""tests/test_module_coverage.py — nothing in the package may dangle (t41).
|
||||
|
||||
Two defects motivated this file, and both shipped with a green suite:
|
||||
|
||||
1. ``button.py`` imported a helper that had been deleted. No harness ever loaded
|
||||
that file (``entity_stublib.load_entity_modules`` lists the platforms it
|
||||
knows), so the ImportError would have appeared only in a user's log. The fix
|
||||
is exhaustive by construction: the sweep walks the package for ``*.py`` files
|
||||
in a clean interpreter, under the stub harness, and loads every one of them.
|
||||
A new platform is covered the moment the file exists — there is no list to
|
||||
update and no special case.
|
||||
|
||||
2. The drift sweep resolved ``homeassistant.*`` imports only, so it could not
|
||||
see a relative import of a name that no longer exists — the same defect one
|
||||
layer up. ``ha_api_drift.scan_package_imports`` now resolves the package's own
|
||||
imports from the AST, with no Home Assistant and therefore no stub needed.
|
||||
|
||||
Each sweep carries its own red proof: a copy of the tree with the reverted
|
||||
``discover_adb_host`` reference must fail it, or the check is decoration.
|
||||
"""
|
||||
|
||||
import shutil
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
import ha_api_drift
|
||||
import pytest
|
||||
from conftest import _SHIELD_DIR
|
||||
|
||||
TESTS_DIR = Path(__file__).resolve().parent
|
||||
REPO_ROOT = _SHIELD_DIR.parent.parent
|
||||
|
||||
#: Loads every module of a package under the stub harness, in a clean process.
|
||||
#: ``argv``: 1 = this tests directory, 2 = the package directory to sweep.
|
||||
_DRIVER = r"""
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
tests_dir, package_root = Path(sys.argv[1]), Path(sys.argv[2])
|
||||
sys.path.insert(0, str(tests_dir))
|
||||
sys.path.insert(0, str(package_root))
|
||||
|
||||
import entity_stublib # noqa: E402
|
||||
from conftest import _load_module # noqa: E402
|
||||
|
||||
# conftest eagerly loads a few stdlib-only modules from the REAL tree (it cannot
|
||||
# know about a copied one): point the synthetic package at the tree under test
|
||||
# and drop what it imported, so every relative import resolves inside the copy.
|
||||
sys.modules["nsh"].__path__ = [str(package_root)]
|
||||
for name in [n for n in list(sys.modules) if n.startswith("nsh.")]:
|
||||
del sys.modules[name]
|
||||
|
||||
entity_stublib.install()
|
||||
|
||||
loaded, failures = [], []
|
||||
for path in sorted(package_root.rglob("*.py")):
|
||||
if "__pycache__" in path.parts:
|
||||
continue
|
||||
rel = path.relative_to(package_root)
|
||||
if rel.name == "__init__.py":
|
||||
dotted = ".".join(rel.parent.parts)
|
||||
name = "nsh.init" if dotted in ("", ".") else f"nsh.{dotted}"
|
||||
else:
|
||||
name = "nsh." + ".".join(rel.with_suffix("").parts)
|
||||
search = [str(path.parent)] if rel.name == "__init__.py" else None
|
||||
try:
|
||||
_load_module(name, path, search)
|
||||
except Exception as exc: # noqa: BLE001 - the error IS the finding
|
||||
failures.append(f"{rel}: {type(exc).__name__}: {exc}")
|
||||
else:
|
||||
loaded.append(str(rel))
|
||||
|
||||
for rel in loaded:
|
||||
print(f"OK {rel}")
|
||||
for failure in failures:
|
||||
print(f"FAIL {failure}")
|
||||
print(f"SUMMARY loaded={len(loaded)} failed={len(failures)} total={len(loaded) + len(failures)}")
|
||||
sys.exit(1 if failures else 0)
|
||||
"""
|
||||
|
||||
|
||||
def _package_modules(root: Path) -> set[str]:
|
||||
"""Every Python file of a package, as the sweep reports them."""
|
||||
return {
|
||||
str(path.relative_to(root))
|
||||
for path in root.rglob("*.py")
|
||||
if "__pycache__" not in path.parts
|
||||
}
|
||||
|
||||
|
||||
def _sweep(root: Path) -> subprocess.CompletedProcess:
|
||||
return subprocess.run(
|
||||
[sys.executable, "-c", _DRIVER, str(TESTS_DIR), str(root)],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
cwd=str(REPO_ROOT),
|
||||
check=False,
|
||||
)
|
||||
|
||||
|
||||
def _reverted_copy(tmp_path: Path) -> Path:
|
||||
"""A copy of the package where button.py imports the deleted helper again."""
|
||||
target = tmp_path / "nvidia_shield"
|
||||
shutil.copytree(
|
||||
_SHIELD_DIR,
|
||||
target,
|
||||
ignore=shutil.ignore_patterns("__pycache__", "*.json", "*.png"),
|
||||
)
|
||||
button = target / "button.py"
|
||||
button.write_text(
|
||||
"from .helpers import discover_adb_host # the reverted reference\n"
|
||||
+ button.read_text(encoding="utf-8"),
|
||||
encoding="utf-8",
|
||||
)
|
||||
return target
|
||||
|
||||
|
||||
def test_every_module_imports_under_the_stub_harness():
|
||||
"""The whole package loads in a clean interpreter — no module is skipped."""
|
||||
result = _sweep(_SHIELD_DIR)
|
||||
output = result.stdout + result.stderr
|
||||
|
||||
assert result.returncode == 0, output
|
||||
reported = {
|
||||
line.split(" ", 1)[1]
|
||||
for line in result.stdout.splitlines()
|
||||
if line.startswith(("OK ", "FAIL "))
|
||||
}
|
||||
assert reported == _package_modules(_SHIELD_DIR), "the sweep skipped or invented a file"
|
||||
assert "failed=0" in result.stdout, output
|
||||
|
||||
|
||||
def test_the_sweep_fails_on_a_reverted_helper_reference(tmp_path):
|
||||
"""Red proof 1: the exact defect that motivated this check.
|
||||
|
||||
button.py imported ``discover_adb_host`` from helpers after that helper was
|
||||
deleted (t41 removed it with the entity path). The sweep must fail loudly.
|
||||
"""
|
||||
result = _sweep(_reverted_copy(tmp_path))
|
||||
output = result.stdout + result.stderr
|
||||
|
||||
assert result.returncode != 0, output
|
||||
assert "discover_adb_host" in output
|
||||
assert "FAIL button.py" in result.stdout
|
||||
|
||||
|
||||
def test_every_relative_import_resolves():
|
||||
"""The same contract, statically: the package's own imports all resolve."""
|
||||
findings = ha_api_drift.scan_package_imports()
|
||||
|
||||
assert findings == [], "\n".join(str(finding) for finding in findings)
|
||||
|
||||
|
||||
def test_the_relative_import_pass_is_not_vacuous():
|
||||
"""A pass that resolved nothing would be green for the wrong reason."""
|
||||
report = ha_api_drift.Report()
|
||||
|
||||
ha_api_drift._scan_package_imports([_SHIELD_DIR], report)
|
||||
|
||||
assert report.relative_imports >= ha_api_drift.FLOORS["relative_imports"]
|
||||
# and the walk itself reached every module, not only the top level (the
|
||||
# floor sits above the top-level-only count, so a shallow glob trips it)
|
||||
assert len(_package_modules(_SHIELD_DIR)) >= ha_api_drift.FLOORS["modules"]
|
||||
|
||||
|
||||
def test_the_static_sweep_fails_on_a_reverted_helper_reference(tmp_path):
|
||||
"""Red proof 2: the AST pass sees the same defect, with no Home Assistant."""
|
||||
findings = ha_api_drift.scan_package_imports(_reverted_copy(tmp_path))
|
||||
|
||||
assert [finding.kind for finding in findings] == ["broken-relative-import"]
|
||||
assert "discover_adb_host" in findings[0].detail
|
||||
assert findings[0].path.endswith("button.py")
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("source", "expected"),
|
||||
[
|
||||
# a name nobody defines
|
||||
("from .helpers import no_such_helper\n", "broken-relative-import"),
|
||||
# a relative import that leaves the package
|
||||
("from ..sibling import thing\n", "broken-relative-import"),
|
||||
# a submodule import is valid without a binding in __init__
|
||||
("from .ha_adb_common import adb_client\n", None),
|
||||
],
|
||||
)
|
||||
def test_the_pass_judges_the_cases_that_matter(tmp_path, source, expected):
|
||||
package = tmp_path / "nvidia_shield"
|
||||
(package / "ha_adb_common").mkdir(parents=True)
|
||||
(package / "__init__.py").write_text("", encoding="utf-8")
|
||||
(package / "helpers.py").write_text("def entry_host(entry):\n return ''\n", encoding="utf-8")
|
||||
(package / "ha_adb_common" / "__init__.py").write_text("", encoding="utf-8")
|
||||
(package / "ha_adb_common" / "adb_client.py").write_text("", encoding="utf-8")
|
||||
(package / "module.py").write_text(source, encoding="utf-8")
|
||||
|
||||
findings = ha_api_drift.scan_package_imports(package)
|
||||
|
||||
assert [finding.kind for finding in findings] == ([expected] if expected else [])
|
||||
Reference in new issue
Block a user