182 lines
7.5 KiB
Markdown
182 lines
7.5 KiB
Markdown
# Guards — how to write a check that actually checks
|
|
|
|
```
|
|
Status: Current
|
|
Owner: _null
|
|
Last reviewed: 2026-08-18
|
|
Governs: structural tests, source-grep assertions, probes, and any check whose
|
|
passing is taken as evidence
|
|
Review trigger: A guard is found to have been passing while the thing it guards
|
|
was broken; a new class of check is added to the suite.
|
|
```
|
|
|
|
A guard that cannot fail is worse than no guard, because it is trusted. Every
|
|
rule here was learned by finding one that had been green for months over
|
|
something broken.
|
|
|
|
## 1. Prove the guard fails before you believe it passes
|
|
|
|
The one discipline that matters most, and it takes thirty seconds:
|
|
|
|
```bash
|
|
cp src/lib/thing.ts /tmp/thing.bak
|
|
# break exactly the thing the test protects
|
|
sed -i 's/if (body.error)/if (false)/' src/lib/thing.ts
|
|
npx vitest run tests/thing.test.ts # expect: exactly one failure
|
|
cp /tmp/thing.bak src/lib/thing.ts
|
|
npx vitest run tests/thing.test.ts # expect: green again
|
|
```
|
|
|
|
**Exactly one** is the part people skip. If breaking the guard's target fails
|
|
three tests, two of them are coincidental and will mask a real regression later.
|
|
If it fails none, the guard is decoration — and you have just learned that for
|
|
the price of one `sed`.
|
|
|
|
`scripts/prove-guard.sh` performs exactly this, which removes the two ways it
|
|
gets skipped: the restore is a `trap`, so an interrupted run cannot leave the
|
|
code broken, and the failure count comes from the runner's own summary rather
|
|
than from eyeballing red — one failing test is routinely reported on half a
|
|
dozen lines, and counting those calls a clean result six coincidental
|
|
failures.
|
|
|
|
Do this when you write a guard, and again when you change what it guards. A
|
|
test written alongside the code it tests has never been observed failing.
|
|
|
|
## 2. A source-grep guard must tell code from the comment about code
|
|
|
|
Structural tests that assert a file does *not* contain some pattern will match
|
|
the docblock explaining why that pattern is forbidden. So the clearest possible
|
|
comment breaks the test, and the obvious fix is to delete the explanation.
|
|
|
|
Strip comments first:
|
|
|
|
```ts
|
|
const codeOf = (path: string) =>
|
|
readFileSync(path, "utf8")
|
|
.split("\n")
|
|
.filter((line) => !/^\s*(\*|\/\/|\{\/\*)/.test(line))
|
|
.join("\n");
|
|
|
|
expect(codeOf("src/lib/thing.ts")).not.toContain("dangerouslySetInnerHTML");
|
|
```
|
|
|
|
Otherwise the guard quietly punishes documenting the rule it exists to enforce —
|
|
which is exactly backwards, because the comment is how the next person learns
|
|
the rule at all.
|
|
|
|
## 3. Pin the behaviour, not the spelling
|
|
|
|
A guard should fail when the protected behaviour breaks and stay quiet
|
|
otherwise. One that asserts on a variable name fails on a rename that changed
|
|
nothing.
|
|
|
|
```ts
|
|
// Brittle: breaks when the variable is renamed, while the fallback it protects
|
|
// is untouched.
|
|
expect(route).toContain("readAsset(project.forgejoRepo");
|
|
|
|
// Pins the behaviour: the route fetches through the wrapper that tries both
|
|
// spellings, and never through the raw reader.
|
|
expect(route).toMatch(/readAsset\(\s*\w+,\s*ASSETS\[which\]\s*\)/);
|
|
expect(body).not.toContain("readFileBytes(");
|
|
```
|
|
|
|
A guard that fails on changes it does not care about is one people learn to edit
|
|
rather than heed, and the edit is usually deletion.
|
|
|
|
## 4. A negative result is only as good as the probe that produced it
|
|
|
|
"The check found nothing" and "the check did not run" are different facts, and
|
|
they look identical from the outside. Before reporting an absence, prove the
|
|
instrument worked:
|
|
|
|
```bash
|
|
# Not this alone — an unreadable file produces the same silence as an unset key
|
|
grep -c '^WANTED=' /proc/$PID/environ
|
|
|
|
# Establish the read succeeded first
|
|
tr '\0' '\n' < /proc/$PID/environ | grep -c . # 0 here means "could not read"
|
|
```
|
|
|
|
This is the confident-absence failure one level up: the same trap as a screen
|
|
rendering a failed query as a count of zero, applied to your own diagnosis.
|
|
|
|
## 5. A guard that is often wrong is worse than none
|
|
|
|
A check with a high false-positive rate trains everybody to skip its output,
|
|
including on the day it is right.
|
|
|
|
One written for this template flagged **684 of 1142** candidates on its first
|
|
run. That was not 684 findings, it was a broken heuristic — and shipping it
|
|
would have taught its readers that the check is noise. Two rounds of narrowing
|
|
brought it to 17 of 363, all of them real.
|
|
|
|
If a new guard's first run is loud, tune it until it is quiet before anybody
|
|
relies on it. Report the false-positive rate you settled at, so the next person
|
|
knows what silence is worth.
|
|
|
|
## 6. Guards belong before the artifact exists
|
|
|
|
A check that runs after publication catches the problem once it is somewhere it
|
|
cannot be taken back from: the tag is in the registry, and refusing the commit
|
|
afterwards leaves git with no record of it.
|
|
|
|
Order the gates so the expensive, irreversible step is last — preconditions,
|
|
guards, build, verify the built thing is what was asked for, publish, and record
|
|
it last of all.
|
|
|
|
## 7. When the gate finds something that invalidates the operation, stop
|
|
|
|
Printing a warning and continuing produces the worst outcome available: the bad
|
|
thing happens *and* a reassuring summary appears above it.
|
|
|
|
The question is not how bad the finding is. It is **whether it invalidates what
|
|
the operation claims**:
|
|
|
|
- A release whose test gate skipped half the suite — a release claims to be
|
|
tested. **Refuse.**
|
|
- A backup written to a group-readable directory — the backup is still a
|
|
backup. **Warn.**
|
|
|
|
Escape hatches are fine, and they have to be asked for by name, never be the
|
|
default, and say plainly what is being given up.
|
|
|
|
## 8. A red is not a proof — read what went red
|
|
|
|
`scripts/prove-guard.sh` decides the guard caught the mutation from the runner's
|
|
**exit code**. It cannot tell *"the mutation broke the test"* from *"the command
|
|
was malformed and nothing ran"*, and both are non-zero.
|
|
|
|
Found by using it on the fix for the missing application-scope exception
|
|
handler. This looked like a clean proof and was not one:
|
|
|
|
```bash
|
|
bash scripts/prove-guard.sh app/.../PeriodApplication.kt \
|
|
' + CoroutineExceptionHandler { _, _ -> }' '' \
|
|
./gradlew :app:test --tests '*ApplicationScopeTest*'
|
|
```
|
|
|
|
`:app:test` is AGP's lifecycle task and takes no `--tests` option, so Gradle
|
|
failed with `Unknown command-line option '--tests'` in 544 ms. The mutation was
|
|
never compiled and the test never ran — and `prove-guard` reported *"the guard
|
|
caught it"*. The concrete task is `:app:testDebugUnitTest`; against that, the
|
|
same mutation failed exactly one test and the proof was real.
|
|
|
|
Two rules follow, and the first is the general one:
|
|
|
|
- **Read the `--- what failed ---` block, every time.** A proof is a proof only
|
|
when the *named test* is what went red. An exit code alone cannot distinguish
|
|
a caught regression from a typo, and this script is most likely to be run at
|
|
the moment you least want to read output — after the code already works.
|
|
- **Give it a fail pattern when the runner prints no summary.** The fallback
|
|
counts matching log lines, and the default pattern also matches Gradle's
|
|
`FAILURE:` and `BUILD FAILED` banners, so one caught violation reads as three
|
|
and the script exits 3. Exit 3 is not a pass. The three boundary-guard proofs
|
|
in [`README.md`](README.md) carry a `PROVE_GUARD_FAIL_PATTERN` for exactly
|
|
this reason; without it they exit 3 while the guard is behaving perfectly,
|
|
which is the failure this document exists to prevent — a check whose red you
|
|
have learned to ignore.
|
|
|
|
The uncomfortable part: the documented proofs had been exiting 3 rather than 0
|
|
since they were written. Nobody had run them and read the last line.
|