✅ test: teach the dist_* gates to tell code from comments - #328
Merged
Conversation
`mutant_rejected()` counted ANY nonzero exit as "the gate rejected this mutation", so the self-test proved nothing about the gate: an empty file, an unparseable file and a nonexistent path all exit nonzero and each printed `ok - self-test: gate rejects ...`. A self-test that passes on garbage is not evidence. Each mutant must now be non-empty and byte-different from the real workflow, and the rejection has to carry the message of the assertion the mutation was built to trip -- `cmp` alone only rules out no-op mutations, it says nothing about WHICH check fired, so the expected-message substring is the load-bearing half. Adds `self_test_guard_holds()`, which re-runs the four degenerate "rejections" (empty, unparseable, nonexistent, unmutated copy) through `mutant_rejected` every CI run and fails if any of them is still accepted as proof. Refs: GATE-COMMENT-01 (F-4)
The step anchors ran `contains()` over the RAW `run:` body, which cannot
distinguish a command from a comment. Two mutations exploited that and left the
gate fully green while producing exactly the end state it exists to prevent:
- `run: "# bash hack/install-operator-sdk.sh ./bin"` still satisfied the
install anchor, so `lint` ran the dist_* glob with no operator-sdk;
- commenting out `bash "${script}"` inside the glob step's body still matched,
because the surrounding `scripts=(hack/test/dist_*_test.sh)` line carries the
literal -- the step expanded the glob and executed nothing.
Every anchor now matches a comment-stripped view of the body (`RUN_CODE`), which
closes the first shape. The second needs more than stripping, since the pattern
appears on lines other than the invocation, so the glob step must additionally be
shown to contain the `bash "${script}"` call; the failure names the exact line to
restore.
Both shapes are added to the built-in yq self-test, so they stay closed.
Refs: GATE-COMMENT-01 (F-1)
The gate asserted step ORDERING but never that the ordered steps are reachable and enforcing. Four mutations satisfied every existing assertion while making the dist_* meta-tests incapable of failing anything: `continue-on-error: true` or `if: "false"` on the glob step, and the same two on `.jobs.lint` itself. Adds those four assertions with distinct failure messages, and the matching mutants to the built-in self-test. yq's `//` alternative operator cannot be used for the defaulting here -- it treats a literal `false` as absent, which is exactly the value being guarded against -- so the raw `yq eval` output is compared instead. `continue-on-error` on the operator-sdk install step is deliberately NOT asserted: that is a different class. A soft-failed install still reds the build, because the glob step it feeds is not soft-failed and dies without the binary. Refs: GATE-COMMENT-01 (F-5)
The workflow reference checks grepped the raw release.yaml, on a committed
rationale that the file inverts. Measured with a strip-every-executable-line
sweep against release.yaml:
- OPERATORHUB_PAT vacuous — comments at :478 and :481 kept it GREEN
- hack/operatorhub-pr.sh vacuous — the comment at :499 kept it GREEN
- operatorhub-pr:, continue-on-error: true, hack/install-operator-sdk.sh
already redded; no comment carries those literals
The old comment claimed the opposite: that install-operator-sdk was the risky
one, and that a raw grep for it "would keep passing if the `run:` line were ever
commented out or deleted". The deletion half was false. That comment is corrected
here rather than left to mislead the next reader.
All five checks now run over a comment-stripped view, and a built-in self-test
re-measures the property every CI run: for each literal it rebuilds release.yaml
with the executable occurrences removed and asserts the check reds. A `cmp` guard
fails loudly if a literal has no executable occurrence at all, so an assertion
that comments alone satisfy can no longer hide as a passing check.
Refs: GATE-COMMENT-01 (F-2, F-3)
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
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
1 participant
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.
Problem
The two
dist_*gates could not tell code from comments, so the states they exist to prevent were reachable while both stayed green:operator-sdkinstall body inci.yamlleft the wiring gate green.bash "${script}"line inside thedist_*glob step left it green — the glob would expand and match scripts that were then never run.release.yaml:478,481,499).Change
Ten mutation shapes are now permanent, built-in self-test assertions:
hack/test/dist_ci_wiring_test.shhack/test/dist_operatorhub_pr_test.shmutant_rejectednow refuses to count an empty file, an unparseable file, a nonexistent path, or an unmutated copy of the real workflow as a rejection — closing the tautology in the self-test itself.Note on the prescribed remedy
The reviewer-prescribed remedy in the story turned out to be insufficient. Comment-stripping alone does not close the
bash "${script}"shape, because the glob step's stripped body still carries the matching literal on itsscripts=(…)assignment. The author caught this and added a separate invocation assertion rather than relying on the strip.Scope
Two files, zero Go. No workflow file is modified at any point in this branch's history.