#729 and #738 are the same defect twice: a HAS_*-gated test suite whose dependency no CI job installs, reporting as skips that read as passes. #737 and #740 fixed the install lines for DPKT, crypto, cli and NGAP — but nothing prevents the next regression.
tests/test_tier_guard.py already guards the adjacent invariants: it checks the test job's ignore-globs and that fixture_tier_paths() is actually called. It does not check that each HAS_* dependency is installed on a leg whose selection reaches the tests it gates.
So today's placement is correct and entirely undefended. Deleting crypto from :114 would re-dark 14 ESP tests, go green, and nobody would know.
What a guard needs to assert
For each HAS_<DEP> flag, tie three facts together:
- the files gated on it, resolved by AST over class-level and method-level
skipUnless — a grep misses the class-level ones;
- which job selections reach those files, from
tests/_tiers.py::fixture_tier_paths() and the test job's --ignore flags;
- that every reaching job's
pip install -e '.[...]' line carries the extra providing that dep.
Fail if a gated file is reached by a job whose install line lacks the extra.
Deliberate exclusions the guard must tolerate, or it will just be silenced: HAS_PYPCAP and HAS_PCAP_CT are C extensions needing libpcap headers; HAS_PYPCAPFILE is excluded pending #743; HAS_VENDOR_DEPS/HAS_CRAWLER_DEPS are ruled onto a non-blocking leg. An allowlist with a reason per entry is the shape — a bare skip list rots into the same invisibility.
Two traps for whoever writes it
Found while cross-reviewing #740, which is otherwise sound and merge-ready.
#729 and #738 are the same defect twice: a
HAS_*-gated test suite whose dependency no CI job installs, reporting as skips that read as passes. #737 and #740 fixed the install lines forDPKT,crypto,cliandNGAP— but nothing prevents the next regression.tests/test_tier_guard.pyalready guards the adjacent invariants: it checks thetestjob's ignore-globs and thatfixture_tier_paths()is actually called. It does not check that eachHAS_*dependency is installed on a leg whose selection reaches the tests it gates.So today's placement is correct and entirely undefended. Deleting
cryptofrom:114would re-dark 14 ESP tests, go green, and nobody would know.What a guard needs to assert
For each
HAS_<DEP>flag, tie three facts together:skipUnless— a grep misses the class-level ones;tests/_tiers.py::fixture_tier_paths()and thetestjob's--ignoreflags;pip install -e '.[...]'line carries the extra providing that dep.Fail if a gated file is reached by a job whose install line lacks the extra.
Deliberate exclusions the guard must tolerate, or it will just be silenced:
HAS_PYPCAPandHAS_PCAP_CTare C extensions needing libpcap headers;HAS_PYPCAPFILEis excluded pending #743;HAS_VENDOR_DEPS/HAS_CRAWLER_DEPSare ruled onto a non-blocking leg. An allowlist with a reason per entry is the shape — a bare skip list rots into the same invisibility.Two traps for whoever writes it
"0 occurrences of 'X not installed'"proves nothing. CI runspytest -qwith no-r, so skip reasons never print; it reads identically whether the fix worked. Only the skip delta is a signal — ci(unit-tests): install DPKT so the dpkt-gated tests actually run #737 established this.pytest-subtestsmarks a parentpassedwhen only itssubTests fail — this cost a reviewer a wrong number on ci(unit-tests): install crypto, cli and NGAP for their gated tests #740 (5 where the truth was 7). Use plainunittestfor any count the guard depends on.Found while cross-reviewing #740, which is otherwise sound and merge-ready.