100% coverage didn't prove our fixes were tested
keel's CI holds line and branch coverage at 100 % (fail_under = 100). It is a good bar, and it produced a bad habit. Pull requests that fixed a bug kept offering one sentence as evidence: "Maintained 100% line + branch test coverage across the repository." With the bar enforced, that sentence is true of every merged pull request before anyone writes it. It describes the repository, not the test.
Coverage says a line ran. It cannot say that any test would fail if the line were different. Those are different questions, and the gap between them is where a fix can go missing.
What an audit found
In September we reverted each of fourteen closed fixes and re-ran the tests that were supposed to guard them (#1289). Today all fourteen fail without their fix. But at the time their issues were closed, three had survived:
- #877: the fix was never written. A genuine refactor removed two redundant frozensets and closed a bug it never touched. The issue was reopened five days later.
- Half of #871: its guarded and unguarded arms sat in one hunk. The closing pull request could have stated a true "the test fails with the whole fix reverted" result while the unguarded half sat untested behind it.
- Half of #879: likewise, a true whole-fix result was available while half the fix was not pinned.
All three closing pull requests offered the coverage sentence as evidence. The coverage was real. It just was not evidence of anything.
The first response, in keel 1.24.0, was a rule: a fix names, for each behaviour it changes, a test that fails as an assertion when that one change is reverted. A rule only written down is the weakest option a project has, so keel 1.25.0 adds a gate that asks the question mechanically.
What the revert-check gate does
When a project lists revert-check in gates:, keel diffs the branch against its base with no context lines, so every contiguous edit is its own change. It checks out the committed HEAD as a scratch worktree under the OS temp directory; your checkout is never touched. There it runs the test command once as a baseline, then once per production change with that change alone undone by git apply -R, resetting the tree before each run.
The unit is a hunk by default, or a whole file with unit: file. Added Python code is split further, one top-level definition at a time, so a test that calls one of three new functions does not vouch for the other two.
A change passes only when a test fails as an assertion. keel reads unittest's summary, where failures count and errors do not, and pytest's short summary, where a FAILED line counts when its reason is assert …, AssertionError or Failed:, except Failed: Timeout, which pytest-timeout writes when a test runs out of time and which says nothing about what the test checked. The distinction matters. If reverting a fix makes a test raise NameError or fail to import, the test reached the code; nothing shows it checked what the code does. That blocks too. The one narrow exception is a change that only adds lines (or only changes one-line Python imports) and whose run reports nothing but missing names, with every failure the run counts explained by one of them (a summary of two failures that describes only one NameError still blocks): no assertion can fail against code that is not there, so that result is a nit, not a pass.
A suite that stays green with a change reverted is a blocking finding that names the hunk. So is a run that only errors, times out, or prints no summary keel can read. A check that cannot judge at all, because there is no test command, no declared test paths, or a baseline that is already red, fails. It never passes.
A real run
A scratch repository with one function and one fix: cap the discount at 100 %, and round the result to cents. The branch adds a test for the cap. Coverage on pkg/ is 100 % line and branch: the tests run every line. The configuration:
gates: [build, revert-check]
knobs:
build_gate_cmd: "python3 -m unittest discover -s tests -t ."
revert_check:
paths: ["pkg/**"]
policy_pack:
name: shop
test_groups:
unit:
command: "python3 -m unittest discover -s tests -t ."
paths: ["pkg/**", "tests/**"]
test_paths: ["tests/**"]
The fix, as the gate sees it:
@@ -1,0 +2 @@ def discount(total, percent):
+ percent = min(max(percent, 0), 100)
@@ -3 +4 @@ def discount(total, percent):
- return total - saving
+ return round(total - saving, 2)
And keel 1.25.0's output, unedited:
$ keel run-gates .keel/project.yaml --root .
ok build
FAIL revert-check
[major] revert-check: pkg/prices.py @@ -3 +4 @@: no test notices this change — the test command passed with the change reverted
[nit] revert-check: 1 of 2 production change(s) made a test fail as an assertion when reverted alone
BLOCKED — merge is gated by the findings above
The rounding ran under test, so coverage counted it. No test would notice it gone. After adding a test that asserts discount(19.99, 15) == 16.99, the same command prints ok revert-check and 2 of 2 production change(s) made a test fail as an assertion when reverted alone.
Why it is off by default
It costs one full test run per production change, plus the baseline. The bounds are knobs.revert_check.max_changes (default 10), budget_s for the whole check (default 1800 seconds), and gate_timeout_s for each run. A change the bounds leave out is reported not checked and blocks; the gate never certifies what it did not run. For a slow suite, point revert_check.cmd at a faster subset rather than raising the budget.
It is a pre-merge gate, so an implementation loop that runs only the guard and test phases reports it NOT-RUN instead of paying for it on every iteration. It is not a test gate: list build beside it, or the run blocks as unconfigured. And it stays off unless listed, including in keel's own configuration, which lists build and lint.
What it does not check
A passing check says a test failed with the change undone. It does not say the test is about that change. The configuration reference lists what stays a reviewer's question:
- The unit is a hunk, not a behaviour. Two arms of one conditional edited in one block are one change, and a test of either passes it. That is exactly #871's shape.
- Added code in other languages is not split, nor are added Python lines that do not parse on their own.
- A test can fail without the fix and still be blind to what the fix broke. #873's fix passes a revert check and still shipped the regression filed as #1268: its fixture made the fix and the bug agree.
- A flaky test that fails on a revert run, an
assertin the code under test, and a test that pins source text all count as an assertion failure. - Files outside the production scope, and anything under the test paths, are never reverted.
- Only unittest and pytest output is read. Any other runner's failures are unreadable, and block.
The short version: coverage tells you what ran. A revert tells you what a test would miss. Neither tells you the test is right, so the reviewers still ask.
keel is open source; install.md covers setup, and the revert_check reference covers every knob.