From 60afbf084ce4436010e691d071ae9629d47ba227 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Tue, 1 Sep 2026 18:50:27 -0600 Subject: [PATCH 1/3] fix: the incomplete count is per major, not per matrix (#858) `suites_incomplete=${suites_incomplete:-0}` is a `set -u` guard, not an initialiser. It keeps whatever the previous major left, while `suites_ran` and `suites_skipped` on the two lines above it are zeroed unconditionally, and `verfail` is reset per major as well. So on the five-major matrix the count would accumulate: PG16 would report PG15's incomplete suites in its own summary line, and still print PASS, because the only per-run thing in a per-major report was the number that exists to say a check could not be evaluated. Latent today. `check_unrunnable` has no production call site, so no real suite can reach the INCOMPLETE state in a matrix run yet. It stops being latent the moment one does, which is what phase 2 of #858 is for. Found behaviourally rather than by reading: driving the runner's own collect loop twice counted 2 incompletes where 1 had occurred. The arm that does the driving lands in the next commit, along with the extraction that makes the loop drivable at all; the removal proof for this line is the mutation that puts `${suites_incomplete:-0}` back and reddens two named checks. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01WDbfRym2V1sYFmMZ5gnsQL --- test/run_all_versions.sh | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/run_all_versions.sh b/test/run_all_versions.sh index 023cebcd..f79aeb2c 100755 --- a/test/run_all_versions.sh +++ b/test/run_all_versions.sh @@ -802,7 +802,12 @@ pgc_classify_suite_rc() { # pgc_classify_suite_rc RC LOGFILE -> PASS|SKIP|INCOMP } skipped_names="" - suites_incomplete=${suites_incomplete:-0} + # Reset, not a `set -u` guard. suites_ran and suites_skipped are zeroed + # unconditionally above; this line used to read ${suites_incomplete:-0}, + # which KEEPS whatever the previous major left. On a five-major matrix + # PG16 would report PG15's incomplete suites in its own summary line and + # still print PASS, because verfail is per major and this count was not. + suites_incomplete=0 for s in "${SUITES[@]}"; do _rc="$(cat "$builddir/${s}.rc" 2>/dev/null)" _verdict="$(pgc_classify_suite_rc "$_rc" "$builddir/${s}.log")" From 3aa5670f22c222ed7eeb712906b4f8da89a002e1 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Tue, 1 Sep 2026 18:51:01 -0600 Subject: [PATCH 2/3] test: the INCOMPLETE path runs whole, and the dispatch is what is tested (#858) #859's regression was not in `pgc_classify_suite_rc`. The classifier returned INCOMPLETE correctly and the CALLER threw the answer away into a write-only flag. Its fifteen arms could not have caught that, because the caller was four branches in the middle of the per-major loop, and a loop that needs a suite list and a populated build directory is not something a selftest can drive. So the untestable thing is extracted. `pgc_tally_suite NAME VERDICT LOGFILE` takes the four branches out of the loop, which becomes three lines and a call. No behaviour changes: the same counters, the same stdout, the same statement the mapping is called by. `$s` becomes argument 1 and `$builddir/${s}.log` becomes argument 3, because the selftest has neither. The function must never declare the six caller counters `local`, and says so in its own comment. The mutation that adds `local` reddens thirteen checks. Selftest 330 then runs the chain in five links, none of them stubbed and none of them re-derived. Every function and every block is lifted out of run_all_versions.sh by text, and every extraction is premised on being non-empty before it is evalled, because `eval ""` succeeds silently. Link 4 is the one that earns the file. Everything else in 330 stays green if the runner defines `pgc_tally_suite` and never calls it, so link 4 evals the loop itself and reads its own text for the delegation. A first draft of this file had links 1 to 3 and would have merged a test that passes over dead code -- the defect class the file exists to prevent, committed by the file itself. Four weaknesses in that draft are fixed here. The last arm re-derived the runner's PASS/FAIL rule inside the test instead of evalling the runner's own branch. The control asserted three values it had itself just assigned. The skip arm could not tell "left alone" from "zeroed", so it now starts at 5. And the verdict handed to the tally was the string INCOMPLETE retyped, which cut the chain at the exact joint the file exists to test; it is now the classifier's own output. One stale comment corrected in passing: the SKIP branch said "Exit 2" where the classifier tests 66. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01WDbfRym2V1sYFmMZ5gnsQL --- test/run_all_versions.sh | 118 ++++---- .../330-the-incomplete-path-must-run-whole.sh | 261 ++++++++++++++++++ 2 files changed, 331 insertions(+), 48 deletions(-) create mode 100644 test/selftest/330-the-incomplete-path-must-run-whole.sh diff --git a/test/run_all_versions.sh b/test/run_all_versions.sh index f79aeb2c..86743d3d 100755 --- a/test/run_all_versions.sh +++ b/test/run_all_versions.sh @@ -801,6 +801,75 @@ pgc_classify_suite_rc() { # pgc_classify_suite_rc RC LOGFILE -> PASS|SKIP|INCOMP fi } +# Tally one suite's verdict into the per-major counters (#858). +# +# A function, not four branches in the middle of a loop, for the same reason +# the classifier is one: the selftest evals THIS TEXT and drives it. #859's +# regression was not in the classifier. The classifier returned INCOMPLETE +# correctly and the CALLER threw the answer away into a write-only flag -- and +# nothing could reach the caller, because the caller was a branch buried in a +# loop that needs a suite list and a populated build directory to run at all. +# Extracted, the whole chain is drivable end to end, which is what selftest 330 +# does. +# +# It writes the CALLER'S counters on purpose, and must never declare them +# local. verfail, suites_ran, suites_skipped, suites_incomplete, results and +# skipped_names belong to the per-major scope; a `local` on any of them here +# would leave every count at zero while every arm that drives this function +# still passed -- the same shape of defect as the write-only flag, and just as +# invisible to a green run. +pgc_tally_suite() { # pgc_tally_suite NAME VERDICT LOGFILE + local _name="$1" _verdict="$2" _log="$3" + if [ "$_verdict" = PASS ]; then + echo " PASS $_name" + results+="$_name=PASS " + suites_ran=$((suites_ran + 1)) + elif [ "$_verdict" = INCOMPLETE ]; then + echo " INCOMPLETE $_name (a check could not be evaluated)" + grep -E '^UNRUN' "$_log" | sed 's/^/ >> /' + results+="$_name=INCOMPLETE " + suites_ran=$((suites_ran + 1)) + suites_incomplete=$((suites_incomplete + 1)) + [ "$(pgc_verdict_fails_major "$_verdict")" = yes ] && verfail=1 + elif [ "$_verdict" = SKIP ]; then + # Exit 66 is pgc_summary's third state: the suite ran no checks (#447). + # Not a pass, because it asserted nothing. Not a failure, because a + # major without the feature and a box without an optional dependency + # are both supported. Counted, so the total below can say so. + echo " SKIP $_name (ran no checks)" + results+="$_name=SKIP " + suites_skipped=$((suites_skipped + 1)) + skipped_names="$skipped_names $_name" + else + echo " FAIL $_name" + # The failing check first, then the tail. A suite that prints a + # diagnostic and a server-log dump on failure pushes its own FAIL + # lines out of a 20-line tail, which is how an intermittent + # replication failure stayed unreadable across many matrices: the + # evidence was in the log and the summary showed everything but. + if grep -qE '^FAIL' "$_log"; then + grep -E '^FAIL' "$_log" | sed 's/^/ >> /' + fi + # 60, not 20: a suite that prints a failure diagnostic and a + # server-log dump needs more room than 20 lines, and truncating it + # is how the replication failures stayed unreadable. + tail -60 "$_log" | sed 's/^/ /' + results+="$_name=FAIL " + # A failed suite RAN. Counting only passes here made the tally + # contradict itself in the one case that matters. The five-major + # matrix reported + # + # PG19 suites that ran: 121 of 122 (skipped: 0) + # + # with temporal failing: 121 + 0 is not 122, and the failing suite was + # in neither bucket of the count that exists to say what ran. Four + # majors hid it, because a tally only disagrees with itself once + # something actually fails. + suites_ran=$((suites_ran + 1)) + verfail=1 + fi +} + skipped_names="" # Reset, not a `set -u` guard. suites_ran and suites_skipped are zeroed # unconditionally above; this line used to read ${suites_incomplete:-0}, @@ -811,54 +880,7 @@ pgc_classify_suite_rc() { # pgc_classify_suite_rc RC LOGFILE -> PASS|SKIP|INCOMP for s in "${SUITES[@]}"; do _rc="$(cat "$builddir/${s}.rc" 2>/dev/null)" _verdict="$(pgc_classify_suite_rc "$_rc" "$builddir/${s}.log")" - if [ "$_verdict" = PASS ]; then - echo " PASS $s" - results+="$s=PASS " - suites_ran=$((suites_ran + 1)) - elif [ "$_verdict" = INCOMPLETE ]; then - echo " INCOMPLETE $s (a check could not be evaluated)" - grep -E '^UNRUN' "$builddir/${s}.log" | sed 's/^/ >> /' - results+="$s=INCOMPLETE " - suites_ran=$((suites_ran + 1)) - suites_incomplete=$((suites_incomplete + 1)) - [ "$(pgc_verdict_fails_major "$_verdict")" = yes ] && verfail=1 - elif [ "$_verdict" = SKIP ]; then - # Exit 2 is pgc_summary's third state: the suite ran no checks (#447). - # Not a pass, because it asserted nothing. Not a failure, because a - # major without the feature and a box without an optional dependency - # are both supported. Counted, so the total below can say so. - echo " SKIP $s (ran no checks)" - results+="$s=SKIP " - suites_skipped=$((suites_skipped + 1)) - skipped_names="$skipped_names $s" - else - echo " FAIL $s" - # The failing check first, then the tail. A suite that prints a - # diagnostic and a server-log dump on failure pushes its own FAIL - # lines out of a 20-line tail, which is how an intermittent - # replication failure stayed unreadable across many matrices: the - # evidence was in the log and the summary showed everything but. - if grep -qE '^FAIL' "$builddir/${s}.log"; then - grep -E '^FAIL' "$builddir/${s}.log" | sed 's/^/ >> /' - fi - # 60, not 20: a suite that prints a failure diagnostic and a - # server-log dump needs more room than 20 lines, and truncating it - # is how the replication failures stayed unreadable. - tail -60 "$builddir/${s}.log" | sed 's/^/ /' - results+="$s=FAIL " - # A failed suite RAN. Counting only passes here made the tally - # contradict itself in the one case that matters. The five-major - # matrix reported - # - # PG19 suites that ran: 121 of 122 (skipped: 0) - # - # with temporal failing: 121 + 0 is not 122, and the failing suite was - # in neither bucket of the count that exists to say what ran. Four - # majors hid it, because a tally only disagrees with itself once - # something actually fails. - suites_ran=$((suites_ran + 1)) - verfail=1 - fi + pgc_tally_suite "$s" "$_verdict" "$builddir/${s}.log" done # How many suites actually asserted something, said out loud (#447). diff --git a/test/selftest/330-the-incomplete-path-must-run-whole.sh b/test/selftest/330-the-incomplete-path-must-run-whole.sh new file mode 100644 index 00000000..ea51103f --- /dev/null +++ b/test/selftest/330-the-incomplete-path-must-run-whole.sh @@ -0,0 +1,261 @@ +# ---- the INCOMPLETE path must run WHOLE, not one link at a time ------------- +# +# #859 shipped fifteen arms that drive pgc_classify_suite_rc and +# pgc_verdict_fails_major directly, and a reviewer verified the dispatch by +# lifting one line out of the runner and running it. Both prove LINKS. Neither +# proves the CHAIN, and the regression that got through #859 was a chain defect: +# the classifier returned INCOMPLETE correctly and the caller threw the answer +# away into a variable nothing read. +# +# Testing a function and not its caller is how a correct answer gets computed +# and discarded. So this part runs the whole thing, in five links: +# +# 1. a real suite calls check_unrunnable and pgc_summary +# -> exits 67, writes an INCOMPLETE line and an UNRUN line +# 2. the runner's classifier reads the .log and .rc THAT SUITE produced +# -> INCOMPLETE +# 3. the runner's tally consumes the classifier's own verdict +# -> verfail=1, suites_ran=1, suites_incomplete=1 +# 4. the runner's OWN collect loop, evalled out of run_all_versions.sh, runs +# over both fixtures +# -> this is the wiring: a tally the loop does not call fails HERE +# 5. the runner's OWN major-verdict branch, evalled the same way, consumes +# the loop's outcome +# -> SUMMARY says FAIL and overall=1 +# +# Nothing is stubbed and nothing is re-derived: every function and every block +# is lifted out of run_all_versions.sh by text, and the .rc and .log are +# produced by running a suite rather than written by hand. +# +# Link 4 is the arm that did not exist in the first draft of this file. Without +# it, an extraction that defines pgc_tally_suite and leaves the old inline +# branch in the loop leaves every other arm here green over dead code -- which +# is the defect class this file exists to prevent, committed by the file itself. + +_e2e_rv="$PGC_TESTDIR/run_all_versions.sh" +_e2e_dir="$PGC_WORKDIR/e2e"; mkdir -p "$_e2e_dir/test" + +# PGC_WORKDIR comes from pgc_setup, which part 020 called. This part inherits it +# from an unrelated part, so it says out loud that it got one: a fixture written +# into an empty path would make every arm below test nothing. +check "premise: the selftest has a workdir to build fixtures in" \ + "$([ -n "${PGC_WORKDIR:-}" ] && [ -d "$_e2e_dir" ] && echo yes || echo no)" "yes" + +# ---- link 1: two real suites, one that cannot evaluate a check and one that can + +cat > "$_e2e_dir/test/e2e_incomplete.sh" < "$_e2e_dir/test/e2e_pass.sh" <"$_e2e_dir/${_e2e_s}.log" 2>&1 + echo $? >"$_e2e_dir/${_e2e_s}.rc" +done + +check "a suite with an unrunnable check exits 67" \ + "$(cat "$_e2e_dir/e2e_incomplete.rc")" "67" + +check "and its log carries the INCOMPLETE line the classifier needs" \ + "$(grep -c ': INCOMPLETE$' "$_e2e_dir/e2e_incomplete.log")" "1" + +check "and the UNRUN line the runner prints into the matrix output" \ + "$(grep -c '^UNRUN something it could not: ABSENT_FIXTURE:' "$_e2e_dir/e2e_incomplete.log")" "1" + +# The control fixture must be a genuine pass, or link 4's discrimination is +# between two identical things. +check "control fixture: a suite whose checks all ran exits 0" \ + "$(cat "$_e2e_dir/e2e_pass.rc")" "0" + +# ---- link 2: the runner's own functions, over those real files -------------- +# +# Lifted by text out of run_all_versions.sh. Each extraction is premised on +# being non-empty BEFORE it is evalled, because `eval ""` succeeds silently: an +# extraction that drifted off its anchor would otherwise leave these arms +# testing whatever else happened to define the name. + +_e2e_txt_classify="$(sed -n '/^pgc_classify_suite_rc()/,/^}/p' "$_e2e_rv")" +_e2e_txt_fails="$(sed -n '/^pgc_verdict_fails_major()/,/^}/p' "$_e2e_rv")" +_e2e_txt_tally="$(sed -n '/^pgc_tally_suite()/,/^}/p' "$_e2e_rv")" + +check "premise: all three runner functions were extracted, not empty ranges" \ + "$([ -n "$_e2e_txt_classify" ] && echo y || echo n)$([ -n "$_e2e_txt_fails" ] && echo y || echo n)$([ -n "$_e2e_txt_tally" ] && echo y || echo n)" \ + "yyy" + +# A truncated extraction evals to a syntax error, not to nothing, so the closing +# brace is asserted rather than assumed: the sed range stops at the first line +# beginning with `}`, and a body containing one would yield a fragment. +check "premise: and each extraction ends at its own closing brace" \ + "$(printf '%s\n%s\n%s\n' "$_e2e_txt_classify" "$_e2e_txt_fails" "$_e2e_txt_tally" | grep -c '^}$')" \ + "3" + +eval "$_e2e_txt_classify" +eval "$_e2e_txt_fails" +eval "$_e2e_txt_tally" + +check "premise: and all three are callable" \ + "$(type -t pgc_classify_suite_rc)/$(type -t pgc_verdict_fails_major)/$(type -t pgc_tally_suite)" \ + "function/function/function" + +check "the runner classifies the file that suite actually produced" \ + "$(pgc_classify_suite_rc "$(cat "$_e2e_dir/e2e_incomplete.rc")" "$_e2e_dir/e2e_incomplete.log")" \ + "INCOMPLETE" + +# ---- link 3: the tally, which is where the regression lived ---------------- +# +# The verdict is the CLASSIFIER'S OUTPUT, not the string INCOMPLETE retyped +# here. Retyping it cuts the chain at exactly the joint this file exists to +# test, and would leave these arms green against a classifier that had stopped +# returning INCOMPLETE at all. +# +# suites_skipped starts at 5, not 0. The arm below says an INCOMPLETE suite is +# not counted as a skip; initialised to 0 that arm cannot tell "the tally left +# it alone" from "the tally zeroed it", and passes for a tally that does +# nothing whatever. + +verfail=0; suites_ran=0; suites_skipped=5; suites_incomplete=0 +results=""; skipped_names="" +pgc_tally_suite e2e_incomplete \ + "$(pgc_classify_suite_rc "$(cat "$_e2e_dir/e2e_incomplete.rc")" "$_e2e_dir/e2e_incomplete.log")" \ + "$_e2e_dir/e2e_incomplete.log" >"$_e2e_dir/tally_incomplete.out" + +check "an INCOMPLETE suite sets the flag the major verdict actually reads" \ + "$verfail" "1" + +check "and is counted as having run" "$suites_ran" "1" + +check "and counted as incomplete, so the tally can say so" "$suites_incomplete" "1" + +check "and is not counted as skipped, nor is the skip count disturbed" \ + "$suites_skipped" "5" + +check "and appears in the results string as INCOMPLETE" \ + "$(printf '%s' "$results" | grep -c 'e2e_incomplete=INCOMPLETE ')" "1" + +# The tally's stdout is what a human reads off a matrix run. A tally that +# records the verdict silently is a tally nobody can act on. +check "and the tally announces it, with the reason lifted from the log" \ + "$(grep -c '^ INCOMPLETE e2e_incomplete (a check could not be evaluated)$' "$_e2e_dir/tally_incomplete.out")" "1" + +check "and reprints the suite's own UNRUN line beneath it" \ + "$(grep -c '^ >> UNRUN something it could not: ABSENT_FIXTURE:' "$_e2e_dir/tally_incomplete.out")" "1" + +# ---- link 4: THE WIRING. the runner's own collect loop, not a copy of it ---- +# +# Everything above would still pass if run_all_versions.sh defined +# pgc_tally_suite and never called it. This link evals the loop itself, out of +# the runner, and hands it the same build directory the fixtures were captured +# into. If the loop does not call the tally, verfail stays 0 here and nowhere +# else. + +_e2e_txt_loop="$(awk '/^\tsuites_incomplete=/{f=1} f{print} f&&/^\tdone$/{exit}' "$_e2e_rv")" + +check "premise: the runner's collect loop was extracted, not an empty range" \ + "$([ -n "$_e2e_txt_loop" ] && echo yes || echo no)" "yes" + +# Membership, not adjacency, and counted from the extracted text rather than +# from the file: a reflow moves lines, and the question is whether the loop +# delegates, not where. +check "the loop delegates each verdict to pgc_tally_suite" \ + "$(printf '%s\n' "$_e2e_txt_loop" | grep -c 'pgc_tally_suite')" "1" + +check "and no longer counts incompletes inline beside it" \ + "$(printf '%s\n' "$_e2e_txt_loop" | grep -c 'suites_incomplete=\$((')" "0" + +SUITES=(e2e_incomplete e2e_pass) +builddir="$_e2e_dir" +verfail=0; suites_ran=0; suites_skipped=0 +results=""; skipped_names="" +# 99, not 0: the loop must zero this itself. suites_ran and suites_skipped are +# reset per major by lines above the extracted chunk, so this test sets them; +# suites_incomplete is reset INSIDE the chunk, so seeding it with a value no +# correct run can produce is what makes the arm below able to see the reset. +suites_incomplete=99 +eval "$_e2e_txt_loop" >"$_e2e_dir/loop.out" + +check "running the real loop over both fixtures fails the major" "$verfail" "1" + +check "and counts both suites as having run" "$suites_ran" "2" + +check "and exactly one of them as incomplete" "$suites_incomplete" "1" + +check "and neither as skipped" "$suites_skipped" "0" + +check "and records each suite's own verdict in the results string" \ + "$(printf '%s' "$results" | grep -c 'e2e_incomplete=INCOMPLETE e2e_pass=PASS ')" "1" + +# ---- link 5: the major's verdict, which is the end of the chain ------------- +# +# The first draft of this file ended by re-deriving the runner's rule inside the +# test -- `[ "$verfail" = 0 ] && echo PASS || echo FAIL` -- which asserts +# nothing about run_all_versions.sh and is strictly weaker than the arm three +# lines above it. This evals the runner's actual branch instead, so a change to +# what the matrix PRINTS, or a dropped `overall=1`, reddens here. + +_e2e_txt_verdict="$(awk '/^\tif \[ "\$verfail" = 0 \]; then/{f=1} f{print} f&&/^\tfi$/{exit}' "$_e2e_rv")" + +check "premise: the major-verdict branch was extracted, not an empty range" \ + "$(printf '%s\n' "$_e2e_txt_verdict" | grep -c 'SUMMARY+=')" "2" + +major=17; overall=0; SUMMARY=() +eval "$_e2e_txt_verdict" + +check "with an incomplete suite in the tally the major reports FAIL" \ + "$(printf '%s\n' "${SUMMARY[0]:-}" | cut -c1-11)" "FAIL PG17" + +check "and the run's overall status is failure" "$overall" "1" + +check "and the summary line carries the incomplete count a reader needs" \ + "$(printf '%s\n' "${SUMMARY[0]:-}" | grep -c '(2 ran, 0 skipped, 1 incomplete)')" "1" + +# ---- control: the same five links, on a tree where nothing is incomplete ---- +# +# Without this the arms above are satisfied by a tally and a verdict branch that +# say FAIL to everything. The control drives the SAME code over only the passing +# fixture and requires the opposite answer at every link -- including the +# results string and the tally's stdout, because a tally that records nothing at +# all also leaves verfail at 0. + +SUITES=(e2e_pass) +verfail=0; suites_ran=0; suites_skipped=0; suites_incomplete=0 +results=""; skipped_names="" +eval "$_e2e_txt_loop" >"$_e2e_dir/loop_pass.out" + +check "control: the same loop leaves a passing suite passing" \ + "$verfail/$suites_ran/$suites_incomplete/$suites_skipped" "0/1/0/0" + +check "control: and still records that it ran, and how" \ + "$(printf '%s' "$results" | grep -c 'e2e_pass=PASS ')" "1" + +check "control: and still announces it" \ + "$(grep -c '^ PASS e2e_pass$' "$_e2e_dir/loop_pass.out")" "1" + +major=17; overall=0; SUMMARY=() +eval "$_e2e_txt_verdict" + +check "control: and the major reports PASS" \ + "$(printf '%s\n' "${SUMMARY[0]:-}" | cut -c1-11)" "PASS PG17" + +check "control: and leaves the run's overall status alone" "$overall" "0" + +unset _e2e_rv _e2e_dir _e2e_s +unset _e2e_txt_classify _e2e_txt_fails _e2e_txt_tally _e2e_txt_loop _e2e_txt_verdict +unset verfail suites_ran suites_skipped suites_incomplete results skipped_names +unset builddir major overall _rc _verdict s +unset SUITES SUMMARY +unset -f pgc_classify_suite_rc pgc_verdict_fails_major pgc_tally_suite From 57d8db785bf9ac515392f7bd56ee1f5d627e2ee0 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Tue, 1 Sep 2026 20:36:18 -0600 Subject: [PATCH 3/3] docs: the incomplete-count fix and the extraction get a CHANGELOG entry (#858) This repo gives test-only fixes their own entries -- #852, #854 and #856 all have one -- and the per-major reset is a real runner bug even though it is latent while check_unrunnable has no production call site. The entry states the failure a reader would have seen (PG16 reporting PG15's incomplete suites and still printing PASS) rather than the line that changed, and says why the tally became a function: nothing could reach the caller while it was four branches inside a loop. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01WDbfRym2V1sYFmMZ5gnsQL --- CHANGELOG.md | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7feec7a9..60dbb45c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,36 @@ true until the next version shipped. ### Fixed +- The matrix runner counts incomplete suites per major, not per run, and the + INCOMPLETE dispatch is now testable end to end (#858). + + **`suites_incomplete=${suites_incomplete:-0}` is a `set -u` guard, not an + initialiser.** It keeps whatever the previous major left, while `suites_ran` + and `suites_skipped` on the lines above it are zeroed unconditionally, and + `verfail` is reset per major too. On the five-major matrix the count + accumulated: PG16 would report PG15's incomplete suites in its own summary + line and still print `PASS`, because the one number in a per-major report + belonged to the whole run. + + Latent today. `check_unrunnable` has no production call site, so no real suite + can reach the INCOMPLETE state in a matrix run yet. + + **The tally is now a function, `pgc_tally_suite`, rather than four branches in + the middle of the per-major loop.** The regression #859 shipped was not in the + classifier: it returned INCOMPLETE correctly and the caller threw the answer + away into a write-only flag. Nothing could reach the caller, because a loop + that needs a suite list and a populated build directory is not something a + selftest can drive. Extracted, the whole chain is drivable, and + `test/selftest/330-the-incomplete-path-must-run-whole.sh` drives it: a real + suite exits 67, the runner's own classifier reads the files that suite wrote, + the runner's own tally consumes the classifier's verdict, the runner's own + collect loop runs over both fixtures, and the runner's own major-verdict + branch decides PASS or FAIL. + + The arm that earns the file reads the loop's own text. Every behavioural arm + stays green if the runner defines the tally and never calls it, which is the + defect class the file exists to prevent. + - The planner estimate counts live rows, and reads the delete count in one catalog scan rather than one per row group.