test(648): close the mutation gaps round 5 found — two tests passed for the wrong reason
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 13s
PR Gates / Docs update reminder (pull_request) Successful in 16s
PR Gates / decisions lifecycle (pull_request) Successful in 17s
Review verdict / Set review-verdict status (pull_request) Successful in 31s
PR Gates / Script tests (pytest) (pull_request) Successful in 35s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 5m59s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 8s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 7s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 16m24s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 22m27s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
review-verdict/h10 Review-verdict: MERGEABLE @ b255b7f

Round 5 could not break the predicate itself: 28,930 real runs of the script across
14,465 crafted --version strings on bash 3.2.57 and 5.3.15 produced zero fail-opens, and
`{1,9}` is honoured on bash 3.2, so round 4's bound is not void on the authoring Macs.

What it did find is that two of round 4's changes were unpinned, and the tests that
looked like they covered them did not.

Reverting BOTH the first-line slice and `[[:blank:]]`→`[[:space:]]` together left the whole
suite green. The four filler cases are all killed by the SEPARATOR restriction alone, so
they attributed the fix to the wrong layer. Added three cases carrying the literal word
`version` (`jq\nversion\n9.9` and friends), which satisfy the separator rule and can only be
stopped by confining the parse to line one with a newline-free blank class.

The CR-strip test was worse: vacuous through two independent mechanisms. `str.splitlines()`
also splits on `\r`, so a per-line view dropped the stray CR; and `subprocess.run(text=True)`
translates `\r` to `\n` outright, so even a raw-string check on stdout was unfalsifiable.
The mutant demonstrably emits `... = jq-1.6<CR> (parsed 1.6; ...)` at the byte level while
the test reported green. Added `run_bytes()` and a bytes comparison.

Both gaps are now mutation-verified: reverting either change reddens exactly its own test.

Also records the operational edge this parser acquires in the follow-up: it is strictly
fail-closed by design, so once the floor mode gates the required check, a jq wrapper that
prints a banner line would deadlock merges. The fix there is to widen the accepted forms,
never to relax fail-closed.

Decisions-Edit: yes
This commit is contained in:
2026-07-26 22:21:07 +02:00
parent 807ebbd38e
commit b255b7ffdc
2 changed files with 66 additions and 6 deletions
@@ -74,6 +74,20 @@ assumed. So the fix has to hold at 1.6, in every gate, regardless of which lane
required per-sha and a jq bump would deadlock merges with no in-repo remedy at all, whereas here a
human can unblock the repo in one commit.
**The parse is strictly fail-closed, and that has an operational edge once it gates merges.**
`scripts/jq-preflight.sh` accepts only a FIRST line of the form `jq-<X>.<Y>` or `jq version <X>.<Y>`;
anything else — a leading blank line, a wrapper that prints a warning first, a version reported only
on stderr — exits 1 rather than guess. That is the right default for a guard whose whole purpose is
refusing to certify a version it did not parse, and it was arrived at over four revisions in which
every *permissive* variant turned out to be fail-OPEN.
But when the follow-up wires the floor-only mode into `review-verdict.yml`, that strictness sits in
the branch-protection-**required** check. A jq wrapper that starts printing a banner line would then
deadlock merges repo-wide — the very failure the pin/floor asymmetry exists to avoid, arriving
through the parser instead of the pin. If that ever happens the fix is to widen the accepted forms in
`jq-preflight.sh`, **not** to relax the fail-closed behaviour: an unparsed version must never be
treated as satisfying the floor.
**The three constructs to avoid, and their version-stable replacements:**
- Never infer "empty input" from a jq exit status — check the string in shell before invoking jq.
+52 -6
View File
@@ -76,6 +76,20 @@ def preflight(tmp_path):
return subprocess.run([BASH, str(SCRIPT), *args],
env=env, capture_output=True, text=True)
def run_bytes(self, *args):
"""Same, but WITHOUT text mode.
`text=True` enables universal-newlines translation, which rewrites `\\r` to `\\n` in the
captured output — so any assertion about a stray carriage return is unfalsifiable through
`run()`. That is not hypothetical: the CR test passed identically with the strip removed
until this was noticed, while the mutant demonstrably emits
`... = jq-1.6\\r (parsed 1.6; ...)` at the byte level.
"""
env = dict(os.environ)
env["PATH"] = str(bindir)
return subprocess.run([BASH, str(SCRIPT), *args],
env=env, capture_output=True)
return Handle()
@@ -234,21 +248,53 @@ def test_an_OUT_OF_RANGE_digit_run_fails_closed(preflight, version_line):
@pytest.mark.parametrize("version_line", [
"jq\n2.34: cannot load shared library", # NEWLINE crossed by a [[:space:]] separator
# Killed by the SEPARATOR restriction (a blank separator must be followed by `version`).
"jq\n2.34: cannot load shared library",
"jq\n\n\n99.9",
"jq -- 2.34 (real jq-1.6)", # filler walked by a permissive separator class
"jq -- 2.34 (real jq-1.6)",
"jq\t\t9.9",
# Killed ONLY by the first-line slice + `[[:blank:]]`. These carry the literal word `version`,
# so the separator restriction is satisfied and cannot save us — the newline must be excluded
# from the separator class AND the parse confined to line one.
#
# Without these, a round-5 mutation check found that reverting BOTH of those changes together
# (`[[:blank:]]`→`[[:space:]]` and parsing `$raw` instead of `$first`) left the whole suite
# GREEN: the four cases above are all killed by the separator alone, so they attributed the fix
# to the wrong layer. A test that passes for the wrong reason is how the previous three rounds
# each shipped a defect.
"jq\nversion\n9.9",
"jq\nversion 9.9",
"jq \n version \n 9.9",
])
def test_a_number_AFTER_the_jq_token_is_not_reachable_across_filler(preflight, version_line):
"""`[[:space:]]` matches NEWLINES, so the round-3 'anchor' still scanned the whole output —
`jq\\n2.34: cannot load` matched `jq`, crossed the newline as separator, and parsed 2.34.
Parsing the first line only, and requiring one of the two separator forms real jq emits
(`jq-1.6` or `jq version 1.6`), confines the match to a real version token."""
"""Two independent layers keep a stray number from being read as the version, and both are
pinned here: the separator must be one of the forms real jq emits (`jq-1.6` / `jq version 1.6`),
AND the match is confined to the first line with `[[:blank:]]` (which, unlike `[[:space:]]`,
does not match a newline). Round 3's 'anchor' had neither and parsed `jq\\n2.34: cannot load`
as 2.34."""
preflight.with_jq(version_line)
r = preflight.run()
assert r.returncode == 1, f"{version_line!r} was accepted as a version"
def test_a_CRLF_version_line_parses_and_logs_without_the_carriage_return(preflight):
"""The trailing `\\r` strip was unpinned — the commit claimed CRLF was verified, but nothing in
the suite contained one. Harmless today (a `\\r` satisfies the trailing non-digit boundary, so
the version still parses) but the log line would carry a stray CR."""
preflight.with_jq("jq-1.6\r")
r = preflight.run_bytes()
assert r.returncode == 0, r.stderr
assert b"parsed 1.6" in r.stdout
# Two separate traps had to be cleared for this assertion to mean anything:
# 1. `str.splitlines()` also splits on `\r`, so inspecting the "version in use" line would drop
# the stray CR before the assertion could see it;
# 2. `subprocess.run(text=True)` translates `\r` to `\n` outright, so even raw-string checks on
# `r.stdout` were unfalsifiable.
# Both made the test pass identically with the strip removed. Hence `run_bytes()` and a bytes
# comparison — verified by mutation, not by reading the code.
assert b"\r" not in r.stdout, "the carriage return leaked into the log line"
def test_the_observability_line_stays_on_ONE_line(preflight):
"""The no-arg mode exists to put a single grep-able version line in the job log; interpolating a
multi-line `--version` would split it."""