From 9db5217bdbb779212c5d066ed1a84f7ca6fc0282 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sat, 19 Sep 2026 16:30:15 +0200 Subject: [PATCH] fix(vm): require the completion marker of the full leg only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The verdict guard added a few commits ago demanded `=== All tests passed ===` from every run. Only the full leg prints it. `scripts/test.sh --suites` stops after the suites it was named and says nothing more, because it has no end to reach — so every `win.sh test ` run came back red while passing: === test.sh: ITERATION mode — suites: artifact (incremental build) === === artifact === 23 passed, 1 skipped GUARD: the log has a summary but no completion marker (runner rc=0) That is the same defect as the one the guard exists to prevent, pointing the other way. A guard against false greens is not allowed to invent false reds, and this one broke the documented iteration workflow on its first real use. vm_verdict now takes a mode. The full leg is unchanged: failures, then the marker, then the note about a lost exit status. A named subset is judged on its summaries and, since it has no marker to lean on, on its exit status — it is a developer tool rather than a gate, so there it obeys rc instead of overriding it. The contract test grew four cases, and they are self-proving rather than inert: the SAME markerless log must be green in iteration mode and red for the full leg, so a build that ignored the mode would fail them. That gap is why the original eight passed while the shipped path was broken — they only ever fed it full-leg logs, which is the case I was thinking about rather than the case I shipped. Signed-off-by: Martin Vogel --- test-infrastructure/vm/vm-run-tests.sh | 32 ++++++++++++++++++++------ tests/test_vm_verdict_contract.sh | 21 +++++++++++++---- 2 files changed, 42 insertions(+), 11 deletions(-) diff --git a/test-infrastructure/vm/vm-run-tests.sh b/test-infrastructure/vm/vm-run-tests.sh index 5e5760741..ec17f771d 100644 --- a/test-infrastructure/vm/vm-run-tests.sh +++ b/test-infrastructure/vm/vm-run-tests.sh @@ -49,9 +49,17 @@ esac # logic with synthetic logs instead of a copy of it that can drift # (tests/test_vm_verdict_contract.sh). See the two-channel note at the call # site for why the log outranks the exit status. +# `mode` is "full" for the whole venue leg and "iteration" for a named subset of +# suites. Only the full leg prints the completion marker, because only the full +# leg HAS an end to reach: scripts/test.sh --suites finishes after the suites it +# was given and says nothing more. Requiring the marker in both modes made every +# `win.sh test ` run report failure while passing -- 23 passed, 1 +# skipped, rc=0, called red. A guard against false greens is not allowed to +# invent false reds. vm_verdict() { local log="$1" local rc="$2" + local mode="${3:-full}" if ! grep -Eq '[0-9]+ passed' "$log"; then echo "GUARD: test runner produced no completion summary — the suites did" \ "not validly run; treating as failure (runner rc=$rc)" >&2 @@ -66,11 +74,19 @@ vm_verdict() { echo "GUARD: the log reports $failed_total failed test(s) (runner rc=$rc)" >&2 return 1 fi - if [ "$complete" -eq 0 ]; then + if [ "$mode" = "full" ] && [ "$complete" -eq 0 ]; then echo "GUARD: the log has a summary but no completion marker — the leg" \ "stopped before the end (runner rc=$rc)" >&2 return 1 fi + if [ "$mode" != "full" ] && [ "${rc:-1}" -ne 0 ]; then + # No marker to lean on here, so a non-zero status is the only evidence + # that the run ended badly after its last summary line. Iteration mode + # is a developer tool, not a gate, so it obeys rc rather than overriding + # it the way the full leg does. + echo "GUARD: suites reported no failures but the run exited $rc" >&2 + return 1 + fi if [ "${rc:-1}" -ne 0 ]; then echo "GUARD: every suite passed and the run completed, but the exit status" \ "came back as $rc — the ssh/msys2_shell chain lost it. Reporting the" \ @@ -81,11 +97,11 @@ vm_verdict() { # Verdict-only mode for the contract test: decide a log WITHOUT a VM. if [ "${1:-}" = "--verdict" ]; then - if [ $# -ne 3 ]; then - echo "usage: vm-run-tests.sh --verdict " >&2 + if [ $# -lt 3 ] || [ $# -gt 4 ]; then + echo "usage: vm-run-tests.sh --verdict [full|iteration]" >&2 exit 2 fi - vm_verdict "$2" "$3" + vm_verdict "$2" "$3" "${4:-full}" exit $? fi @@ -227,7 +243,9 @@ fi # # So the LOG decides the test outcome (the runner writes it locally; it cannot # be mangled in transit) and rc decides what the log cannot see. Green requires -# zero reported failures AND the completion marker that scripts/test.sh prints -# as its last statement — a leg that stopped early has a summary but no marker. -vm_verdict "$LOG" "$rc" +# zero reported failures AND, for the full leg, the completion marker that +# scripts/test.sh prints as its last statement — a leg that stopped early has a +# summary but no marker. A named subset of suites prints no marker at all, so it +# is judged on its summaries and its exit status instead. +vm_verdict "$LOG" "$rc" "$([ "$1" = "--par" ] && echo full || echo iteration)" exit $? diff --git a/tests/test_vm_verdict_contract.sh b/tests/test_vm_verdict_contract.sh index 3076eb820..275f865e8 100755 --- a/tests/test_vm_verdict_contract.sh +++ b/tests/test_vm_verdict_contract.sh @@ -36,9 +36,9 @@ printf ' 7925 passed, 0 failed, 74 skipped\n' > "$WORK/no_marker" printf 'configure: something exploded\n' > "$WORK/no_summary" failures=0 -expect() { # description log rc want - local description="$1" log="$2" rc="$3" want="$4" got - bash "$RUNNER" --verdict "$WORK/$log" "$rc" >/dev/null 2>&1 +expect() { # description log rc want [mode] + local description="$1" log="$2" rc="$3" want="$4" mode="${5:-full}" got + bash "$RUNNER" --verdict "$WORK/$log" "$rc" "$mode" >/dev/null 2>&1 got=$? if [ "$got" -ne "$want" ]; then echo "FAIL: $description — verdict $got, expected $want" >&2 @@ -65,8 +65,21 @@ expect "a summary without the completion marker is red" no_marker 0 1 # distinguishable from "ran and failed". expect "a log with no summary is the no-summary guard" no_summary 0 90 +# Iteration mode: `test.sh --suites` finishes after the suites it was named and +# prints no completion marker, because it has no end to reach. Demanding one +# turned a passing subset run into a failure — 23 passed, 1 skipped, rc=0, +# reported red (win.sh test artifact, 2026-09-19). A guard against false greens +# must not invent false reds, so the marker is required of the full leg only. +expect "a subset run with no marker is green in iteration mode" no_marker 0 0 iteration +expect "the same log is still red for the full leg" no_marker 0 1 full +# Without a marker to lean on, rc is the only evidence left that a subset run +# ended badly after its last summary, so iteration mode obeys it. +expect "a non-zero status is red in iteration mode" no_marker 1 1 iteration +# Failures still outrank everything, in either mode. +expect "failures are red in iteration mode too" red 0 1 iteration + if [ "$failures" -gt 0 ]; then echo "VM verdict contract VIOLATED: $failures case(s)" >&2 exit 1 fi -echo "VM verdict contract passed (8 cases: lost exit status, false green, partial runs)" +echo "VM verdict contract passed (12 cases: lost exit status, false green, partial runs, iteration mode)"