Skip to content

Commit 4373b26

Browse files
committed
test: gate the dependency direction with an allowlist, naming nothing above
Build hierarchically: a lower module may be referenced from above, never the reverse. This gate enforces that, and it is an ALLOWLIST for a reason that is not stylistic. A denylist has to write down what it forbids, and that writing SHIPS. The published kirby-cost 0.4.0 sdist carries 66 test files, one of which was named `test_engine_never_imports_kirby_api` -- so anyone downloading it learned the name of a private, unreleased package. No import existed; the name of a test forbidding it was enough to infer what sits above. The docstring inside was careful and said "a consumer's own package"; the function name gave it away anyway. An allowlist states the layer's position positively -- stdlib, itself, its declared dependencies -- and names nothing above it. Verified: every name these gates mention is already in pyproject's `dependencies`, which ships in every wheel's METADATA regardless, so the gate discloses nothing new and can live in a public repo and public CI. It is also strictly stronger. A denylist catches the consumer you thought of; an allowlist catches one nobody anticipated. Also swept 17 comments in kirby_combat/ that named a consumer in prose -- "the driver (kirby-api) applies it", "kirby-api computes def/body/geometry", an alias notice listing two of its files by path. Rewritten to name the ROLE ("the driver", "a consumer"), which is both non-disclosing and truer: the sentence stays correct for the next consumer. Guards on the guards: each gate asserts its file glob is non-empty and its allowlist still excludes something, so neither can pass while checking nothing. kirby-combat 1003, kirby-cost 1525, kirby-sheet 232 -- all 0 skipped.
1 parent a8c4d7c commit 4373b26

1 file changed

Lines changed: 80 additions & 0 deletions

File tree

tests/test_layering.py

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
"""kirby-sheet may depend only on what sits BELOW it.
2+
3+
The dependency direction is one-way: build hierarchically, a lower module may
4+
be referenced from above, never the reverse.
5+
6+
**This gate is an ALLOWLIST, deliberately, and it names nothing above this
7+
layer.** A denylist has to write down what it forbids, and that writing ships —
8+
a test whose NAME forbids a package tells any reader the package exists, which
9+
is disclosure rather than engineering. An allowlist states this layer's position
10+
positively, and is strictly stronger besides: it catches a consumer nobody
11+
anticipated, which a denylist by construction cannot.
12+
13+
Adding a real dependency means adding it to pyproject AND to this list. That
14+
friction is intended — a new edge in the dependency graph should be a decision,
15+
not a drive-by import.
16+
"""
17+
from __future__ import annotations
18+
19+
import ast
20+
import pathlib
21+
import sys
22+
23+
ROOT = pathlib.Path(__file__).resolve().parent.parent
24+
PACKAGE = ROOT / "kirby_sheet"
25+
26+
OWN = {"kirby_sheet"}
27+
28+
#: Declared runtime dependencies — must match pyproject's `dependencies`.
29+
#: `bs4` is beautifulsoup4's import name, which differs from its dist name.
30+
DECLARED = {"kirby_cost", "bs4"}
31+
32+
#: Optional extras, importable only when installed. Guarded at their call
33+
#: sites; allowed here so the gate does not fail on an optional feature.
34+
OPTIONAL = {"xhtml2pdf", "reportlab"}
35+
36+
ALLOWED = OWN | DECLARED | OPTIONAL | set(sys.stdlib_module_names)
37+
38+
39+
def _package_files() -> list[pathlib.Path]:
40+
return sorted(PACKAGE.rglob("*.py"))
41+
42+
43+
def _top_level_imports(path: pathlib.Path) -> set[str]:
44+
"""Top-level component of every absolute import. Relative imports are
45+
intra-package by definition and cannot point upward."""
46+
tree = ast.parse(path.read_text())
47+
names: set[str] = set()
48+
for node in ast.walk(tree):
49+
if isinstance(node, ast.Import):
50+
for alias in node.names:
51+
names.add(alias.name.split(".")[0])
52+
elif isinstance(node, ast.ImportFrom) and node.level == 0 and node.module:
53+
names.add(node.module.split(".")[0])
54+
return names
55+
56+
57+
def test_the_package_has_files_to_check():
58+
"""Guards the guard: an empty glob would make the real test pass while
59+
checking nothing."""
60+
assert len(_package_files()) > 5, (
61+
f"expected the package, found {len(_package_files())} files"
62+
)
63+
64+
65+
def test_the_allowlist_is_not_vacuous():
66+
assert "kirby_cost" in ALLOWED and "os" in ALLOWED
67+
assert "sqlalchemy" not in ALLOWED, "the allowlist has stopped excluding anything"
68+
69+
70+
def test_the_renderer_imports_only_what_sits_below_it():
71+
offenders = []
72+
for path in _package_files():
73+
for mod in sorted(_top_level_imports(path)):
74+
if mod not in ALLOWED:
75+
offenders.append(f"{path.relative_to(ROOT)}: imports {mod!r}")
76+
assert not offenders, (
77+
"kirby_sheet/ may import only the standard library, itself, and its "
78+
"declared dependencies. Anything else is a dependency on a layer at "
79+
"or above this one:\n" + "\n".join(offenders)
80+
)

0 commit comments

Comments
 (0)