Skip to content

docs(zsh-standard): require a removal's test to be able to fail - #605

Merged
ss-o merged 1 commit into
mainfrom
feature-604
Sep 2, 2026
Merged

docs(zsh-standard): require a removal's test to be able to fail#605
ss-o merged 1 commit into
mainfrom
feature-604

Conversation

@ss-o

@ss-o ss-o commented Sep 2, 2026

Copy link
Copy Markdown
Member

Adds zsh/test/cover-removed-behavior. Documentation and policy only.

Why

zi#488 was not caused by a missing test. It was caused by a test that could not fail.

zi#477 removed a line as dead and added tests/plugin-autoload-fpath-scope.zsh as evidence the removal was safe. Its commit message said so outright:

It passes before and after, which is the point.

That is the defect. The test asserted autoload +X only for functions in the plug-in's own directories, which resolve from $PLUGIN_DIR with or without the removed line. The behaviour the line actually provided, carrying the caller's $fpath so a foreign function could resolve, was never exercised. The test passed on both sides because it was blind to the only thing at issue, and Aloxaf/fzf-tab broke completely.

A test whose result is invariant to the change it accompanies is evidence of nothing, and it is worse than no test: it reads as diligence and closes down review.

It recurred immediately

While fixing #488 I inspected the neighbouring -w branch of the same function, judged its fpath+=( $PLUGIN_DIR ) an unlocalized leak from its shape alone, and wrote the "fix". It broke -w completely: autoload -w <digest> declares functions that resolve lazily at first call, so that append has to outlive the call. The full suite stayed green while -w was broken end to end, because nothing exercised it either. Corrected in zi#493.

Two instances, one function, one release cycle. Both times a line was judged inert on sight, and both times the test set was structurally incapable of disagreeing.

Duplicate search

Per the candidate quality gate in runbooks/learning-capture.md:

Existing rule Governs
zsh/review/report-without-rewrite read-only work, not removals
zsh/change/conform-touched-code scope of cleanup, not its evidence
zsh/test/isolate-environment how a test is built
zsh/test/declare-negative-fixtures how a test is built
zsh/test/match-production-profile how a test is built

Nothing requires that a test cited for a removal be capable of failing without the removed code.

The rule

Scoped to test-fixture, matching the other rules in that section: the obligation lands on the test artifact, not on the profile of the code being removed. My first draft declared all five profiles, which the validator's startup-file membership check correctly rejected.

The check itself is cheap: run the candidate test against the unmodified source. If it passes there, it is not evidence.

Verification

python3 scripts/validate-zsh-standard-policy.py      -> passed
python3 scripts/test_validate_zsh_standard_policy.py -> Ran 99 tests, OK

Frozen consumer-parser golden: block count 65 to 66, digest replaced. startup-file membership unchanged.

Closes #604

z-shell/zi#488 was not caused by a missing test. It was caused by a test that
could not fail.

z-shell/zi#477 removed a line as dead and added a test as evidence the removal
was safe. Its commit message said the test "passes before and after, which is
the point". That is the defect. The test asserted `autoload +X' only for
functions in the plug-in's own directories, which resolve from $PLUGIN_DIR
either way. The behaviour the removed line provided, carrying the caller's
$fpath so a foreign function could resolve, was never exercised, so the test
was structurally incapable of objecting. fzf-tab broke completely.

Not a one-off. While fixing that, the neighbouring `-w' branch was judged an
unlocalized leak by inspection and "fixed"; the whole suite stayed green while
-w was broken end to end, because nothing exercised it either. Corrected in
z-shell/zi#493. Two instances, one function, one release cycle, both a line
judged inert on sight against a test set that could not disagree.

Add zsh/test/cover-removed-behavior: a test offered as evidence for a removal
must exercise the behaviour the removed code provided, observed failing without
it and passing with it. Running the candidate against the unmodified source is
the whole check; if it passes there, it is not evidence.

Scoped to test-fixture, matching the other rules in the section: the obligation
lands on the test artifact, not on the profile of the code being removed.

Searched for duplicates first. zsh/review/report-without-rewrite governs
read-only work, zsh/change/conform-touched-code governs cleanup scope, and the
three existing zsh/test rules govern how a test is built, not what it must be
able to detect.

The frozen consumer-parser golden moves accordingly: block count 65 to 66 and
the digest replaced. startup-file membership is unchanged, as the rule is
test-fixture only.

Closes #604
@ss-o
ss-o requested a review from a team as a code owner September 2, 2026 23:30
@ss-o
ss-o merged commit abac83d into main Sep 2, 2026
11 checks passed
@ss-o
ss-o deleted the feature-604 branch September 2, 2026 23:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Zsh standard: a test justifying a removal must be able to fail without the removed code

1 participant