fix(ci): the coverage-doc guard never ran in CI — only on maintainer laptops - #36
Merged
Merged
Conversation
…laptops `tests/test_coverage_doc.py` re-measures every figure in `tests/README-coverage.md`. It was written after five of that table's rows were found wrong by 16 to 61 points, every one of them understating the truth. It is the only thing keeping those numbers honest. **It never ran in CI.** CI's test step is `coverage run -m pytest tests`, and coverage writes `.coverage` only when it EXITS — so while the suite is running there is no data file, and those three assertions called `pytest.skip`. They executed on maintainer laptops, where `make ci` had already produced the file, and skipped on every CI run. The guard that keeps the coverage doc honest was only ever verified on the machine of the person who might have let it drift. Measured, not inferred: replicating CI's exact invocation locally reproduces `SKIPPED [3]`. This is INV-CI-1's shape a second time — a check that silently no-ops precisely where it matters — which I flagged in #35 as deserving its own change rather than being bolted on. ## The fix is two-part, because part 1 alone would decay 1. `ci.yml` runs the module as a dedicated step AFTER `coverage report`, when the data exists. 2. That step sets `SENTINEL_REQUIRE_COVERAGE_DATA=1`, under which absent or stale data RAISES instead of skipping. Without part 2 a later change to the data-file path would quietly restore the no-op and the new step would still report green — fixing the symptom while leaving the failure mode intact. A dedicated step that is still allowed to skip is the same failure wearing a different hat. Both unavailable-data paths (missing/stale file, and an unresolvable `coverage` launcher) now route through ONE `_unavailable()` helper, and the guard asserts there is at most one bare `pytest.skip(` left in the module. "A fix applied to one call site is not an invariant" — and I had read that launcher skip as a success three times before it became an assertion. Local developer experience is unchanged: without the flag, a missing `.coverage` still gives the friendly skip with instructions. Over-strictness gets routed around. ## A negative result worth recording Before fixing this I asked the broader question — how many guards in this suite pass vacuously? — and mechanised it: an AST scan for tests that discover a collection, assert it is empty, and never assert it is non-empty. First pass flagged 13. **All 13 were false positives**, and inspecting them taught me my scanner did not understand two legitimate control forms: a `@pytest.mark.parametrize` driving the iteration (an empty set collapses to zero collected tests, which is visible), and a sibling assertion pinning the collection's size. After teaching it both, the count was **0**. So I positive-controlled the scanner itself: planted a deliberately vacuous guard in `tests/` and confirmed it was caught, then removed it. The scanner works and the finding is genuinely zero — this category is clean and needs no change. A scan finding nothing is indistinguishable from a broken scan, so the control is what makes the zero worth reporting. ## Mutations 5/5 caught, control valid | mutation | | |---|---| | ci.yml drops the dedicated coverage-doc step | caught | | the step stops setting the require flag | caught | | the flag is set to a falsey value | caught | | the helper stops raising (skip restored) | caught | | a second bare `pytest.skip` reappears | caught | The falsey-value mutation matters: `bool("false")` is True in Python and this repo has recorded that trap three times (INV-COERCE), so the flag's parsing is asserted both ways — truthy spellings raise, falsey spellings still skip. ## Testing - **3830 passed / 6 skipped in BOTH fixed and random order** - **E2E: the full CI three-step sequence replicated locally** — `coverage run -m pytest` → `coverage report --fail-under=88` (92%) → the new step, which reports **6 passed, ZERO skipped**. That is the assertion this whole change is about, verified as CI will run it. - all four flag/data combinations verified: data+flag passes, no-data+flag FAILS, no-data without flag skips, data without flag passes - `make ci` green · both mypy gates clean · ruff clean · secret scan clean - `README-coverage.md` now states the 9-vs-6 skip delta and WHY, instead of quoting a number that only held under one invocation New invariant: **INV-DOC-5**.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(ci): the coverage-doc guard never ran in CI — only on maintainer laptops
tests/test_coverage_doc.pyre-measures every figure intests/README-coverage.md. It waswritten after five of that table's rows were found wrong by 16 to 61 points, every one of
them understating the truth. It is the only thing keeping those numbers honest.
It never ran in CI. CI's test step is
coverage run -m pytest tests, and coverage writes.coverageonly when it EXITS — so while the suite is running there is no data file, andthose three assertions called
pytest.skip. They executed on maintainer laptops, wheremake cihad already produced the file, and skipped on every CI run.The guard that keeps the coverage doc honest was only ever verified on the machine of the
person who might have let it drift.
Measured, not inferred: replicating CI's exact invocation locally reproduces
SKIPPED [3].This is INV-CI-1's shape a second time — a check that silently no-ops precisely where it
matters — which I flagged in #35 as deserving its own change rather than being bolted on.
The fix is two-part, because part 1 alone would decay
ci.ymlruns the module as a dedicated step AFTERcoverage report, when the data exists.SENTINEL_REQUIRE_COVERAGE_DATA=1, under which absent or stale data RAISESinstead of skipping.
Without part 2 a later change to the data-file path would quietly restore the no-op and the
new step would still report green — fixing the symptom while leaving the failure mode intact.
A dedicated step that is still allowed to skip is the same failure wearing a different hat.
Both unavailable-data paths (missing/stale file, and an unresolvable
coveragelauncher) nowroute through ONE
_unavailable()helper, and the guard asserts there is at most one barepytest.skip(left in the module. "A fix applied to one call site is not an invariant" — andI had read that launcher skip as a success three times before it became an assertion.
Local developer experience is unchanged: without the flag, a missing
.coveragestill givesthe friendly skip with instructions. Over-strictness gets routed around.
A negative result worth recording
Before fixing this I asked the broader question — how many guards in this suite pass
vacuously? — and mechanised it: an AST scan for tests that discover a collection, assert it
is empty, and never assert it is non-empty.
First pass flagged 13. All 13 were false positives, and inspecting them taught me my
scanner did not understand two legitimate control forms: a
@pytest.mark.parametrizedrivingthe iteration (an empty set collapses to zero collected tests, which is visible), and a
sibling assertion pinning the collection's size. After teaching it both, the count was 0.
So I positive-controlled the scanner itself: planted a deliberately vacuous guard in
tests/and confirmed it was caught, then removed it. The scanner works and the finding is genuinely
zero — this category is clean and needs no change. A scan finding nothing is indistinguishable
from a broken scan, so the control is what makes the zero worth reporting.
Mutations 5/5 caught, control valid
pytest.skipreappearsThe falsey-value mutation matters:
bool("false")is True in Python and this repo hasrecorded that trap three times (INV-COERCE), so the flag's parsing is asserted both ways —
truthy spellings raise, falsey spellings still skip.
Testing
coverage run -m pytest→coverage report --fail-under=88(92%) → the new step, which reports 6 passed, ZEROskipped. That is the assertion this whole change is about, verified as CI will run it.
without flag skips, data without flag passes
make cigreen · both mypy gates clean · ruff clean · secret scan cleanREADME-coverage.mdnow states the 9-vs-6 skip delta and WHY, instead of quoting a numberthat only held under one invocation
New invariant: INV-DOC-5.