diff --git a/CHANGELOG.md b/CHANGELOG.md index e6c4af67..327e0219 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -863,6 +863,37 @@ true until the next version shipped. deleted fourth; the end state is one. which never started. +- One matrix report no longer gives two answers to "how many suites accounted for their + checks" (#928). + + A full PG 17 report printed both, three lines apart: + + population reconciliation: registered=251 | accounted=237, ... | sum=251 + of those, 235 accounted for their checks and 7 did not + + 237 and 235, both describing suites that accounted for their checks, differing by + exactly 2. The population line counted with the WIDE reader, + `pgc_log_shows_any_accounting`; the breakdown line derived from the NARROW one, + `pgc_log_shows_accounting`. The figure a reader acts on is the second, because it is the + one phrased as a problem, and it overstated the debt by 2. + + That matters more than a mismatch: the breakdown line exists to stop an overcount, and + deriving it from the narrower reader reintroduced a smaller version of the same + overcount in the line added to close it. + + THE GAP IS A DIFFERENT MECHANISM, NOT A DEBT. The wide reader also accepts a suite that + prints its own `checks run:` line. Measured on this tree: of the twelve registered + suites that never call `pgc_summary`, exactly two -- `bench_guards` and `docs_style` -- + emit a tally of their own, and the other ten keep none. Those two are the 2. + + The headline now comes from the same file the population line counts, and the two + mechanisms are broken out beneath it with the own-mechanism suites NAMED rather than + counted, so a third adopting its own tally appears without anyone editing a number. + + The comment above the line said "Ten registered suites exit 0 having never called + pgc_summary", which conflated the two populations: twelve never call it, and ten of + those keep no tally. Both halves were true of something; neither was true of what it + said. - A test helper no longer takes a tree it ignores (#933). `_sh(srcdir, expr)` in `test/pytest/test_build_refusal.py` read as "evaluate one diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index 5b80283c..26563e13 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -479,9 +479,12 @@ harness_selftest 390-a-registered-suite-must-account and it is NAMED, so the rea harness_selftest 390-a-registered-suite-must-account and it is named as that fault, not as one of the other two never - harness_selftest 390-a-registered-suite-must-account and it is named as the opposite fault, not the same one never - harness_selftest 390-a-registered-suite-must-account and it is named, which the symmetry check could never do never - +harness_selftest 390-a-registered-suite-must-account and names nothing when the two readers agree never - harness_selftest 390-a-registered-suite-must-account and prose containing the word does not count as the line never - harness_selftest 390-a-registered-suite-must-account and so does a failing one, which is the point never - +harness_selftest 390-a-registered-suite-must-account and sorts its inputs, because the runner appends them in SUITES order never - harness_selftest 390-a-registered-suite-must-account and the excused one is not named as a failure never - +harness_selftest 390-a-registered-suite-must-account and the population reconciliation counts that same file never - harness_selftest 390-a-registered-suite-must-account and the reader answers no on it, which is the wrong answer the arm catches never - harness_selftest 390-a-registered-suite-must-account and the real function reconciles the same input, so the arm is not noise never - harness_selftest 390-a-registered-suite-must-account and the reconciliation is given that record never - @@ -491,10 +494,17 @@ harness_selftest 390-a-registered-suite-must-account equal sets reconcile never harness_selftest 390-a-registered-suite-must-account every registered suite has a file never - harness_selftest 390-a-registered-suite-must-account nor no for every one of them never - harness_selftest 390-a-registered-suite-must-account opposite errors do not cancel: both directions are reported never - +harness_selftest 390-a-registered-suite-must-account premise: a lib.sh accounting line is seen by the narrow reader never - +harness_selftest 390-a-registered-suite-must-account premise: a private tally is NOT seen by the narrow reader never - +harness_selftest 390-a-registered-suite-must-account premise: and a log with neither is seen by neither never - harness_selftest 390-a-registered-suite-must-account premise: and produced exactly one accounting line to be read never - +harness_selftest 390-a-registered-suite-must-account premise: and so is the wide reader it is paired with never - harness_selftest 390-a-registered-suite-must-account premise: and that count excludes the definition line, which mentions it never - harness_selftest 390-a-registered-suite-must-account premise: and the accounted reader that feeds it never - +harness_selftest 390-a-registered-suite-must-account premise: and the narrow count is still printed, as the lib.sh half never - harness_selftest 390-a-registered-suite-must-account premise: and the real function still does never - +harness_selftest 390-a-registered-suite-must-account premise: but IS seen by the wide one, which is where the 2 came from never - +harness_selftest 390-a-registered-suite-must-account premise: it is callable never - harness_selftest 390-a-registered-suite-must-account premise: pipefail is on, which is the condition the bug needs never - harness_selftest 390-a-registered-suite-must-account premise: the declaration reader evalled out of the runner is callable never - harness_selftest 390-a-registered-suite-must-account premise: the drift changed the line the reader looks for never - @@ -509,13 +519,16 @@ harness_selftest 390-a-registered-suite-must-account premise: the reconciliation harness_selftest 390-a-registered-suite-must-account premise: the registered list is not empty, so the partition means something never - harness_selftest 390-a-registered-suite-must-account premise: the runner defines the declaration reader this part evals never - harness_selftest 390-a-registered-suite-must-account premise: the runner defines the observation reader this part evals never - +harness_selftest 390-a-registered-suite-must-account premise: the runner defines the own-mechanism difference this part evals never - harness_selftest 390-a-registered-suite-must-account premise: the runner defines the population reconciliation never - harness_selftest 390-a-registered-suite-must-account premise: the runner defines the reconciliation this part evals never - harness_selftest 390-a-registered-suite-must-account premise: the twin script was written and is runnable never - harness_selftest 390-a-registered-suite-must-account premise: the unsorted twin is callable never - +harness_selftest 390-a-registered-suite-must-account the breakdown headline counts the wide set, not the narrow one never - harness_selftest 390-a-registered-suite-must-account the debt file is in the tree never - harness_selftest 390-a-registered-suite-must-account the grep -q shape is the one that gets this wrong under pipefail never - harness_selftest 390-a-registered-suite-must-account the identity catches comm reading unsorted input never - +harness_selftest 390-a-registered-suite-must-account the own-mechanism difference names the suite the readers disagree about never - harness_selftest 390-a-registered-suite-must-account the partition over the real suite list adds up never - harness_selftest 390-a-registered-suite-must-account the population partitions, and prints inputs == sum(buckets) never - harness_selftest 390-a-registered-suite-must-account the reader accepts the line the producer actually emits never - diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index d4e6df49..913055e2 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -34,4 +34,4 @@ suites_not_covered 250 # Without that it is a hand-maintained count that drifts, which is the failure # this repository has spent a day proving. It is not a ceiling; it is a # measurement that must be true. -checks_never_observed_red 806 +checks_never_observed_red 819 diff --git a/test/run_all_versions.sh b/test/run_all_versions.sh index 5d02cef9..74ef7d0e 100755 --- a/test/run_all_versions.sh +++ b/test/run_all_versions.sh @@ -1015,6 +1015,21 @@ pgc_reconcile_records() { # pgc_reconcile_records LOGFILE -> 0 ok, 1 mismatch return 0 } +# Which suites accounted by a mechanism of their own rather than by lib.sh's. +# +# A FUNCTION so an arm can drive it, for the reason `pgc_reconcile_records` is one: a +# set difference computed inline can only be tested by re-implementing it, and a test +# that re-implements its subject agrees with it by construction. +# +# The NARROW file holds the suites whose log carries lib.sh's `accounting:` line; the +# WIDE file holds those accounted by any mechanism, which also accepts a suite printing +# its own `checks run:`. The difference is therefore the suites with a private tally -- +# a different mechanism, not a debt, and the thing that made one report give two answers +# to the same question (#928). +pgc_own_mechanism_suites() { # pgc_own_mechanism_suites NARROWFILE WIDEFILE -> names + comm -13 <(sort "$1") <(sort "$2") +} + pgc_log_shows_any_accounting() { # pgc_log_shows_any_accounting LOGFILE -> yes|no # Did this suite count its checks AT RUNTIME, by any mechanism the log shows? # @@ -1428,14 +1443,38 @@ pgc_tally_suite() { # pgc_tally_suite NAME VERDICT LOGFILE fi # How many of the suites counted as having RUN actually accounted for their - # checks (#916). Ten registered suites exit 0 having never called pgc_summary; - # counting them among the suites that ran is the overcount #447 added this - # line to stop, one level further down, and printing the total without this - # breakdown leaves it exactly where it was. Raised by OffgridwithJD, who was - # right that the drift detector alone does not close it. + # checks (#916). Counting a suite that never accounted among the suites that ran + # is the overcount #447 added this line to stop, one level further down, and + # printing the total without this breakdown leaves it exactly where it was. + # Raised by OffgridwithJD, who was right that the drift detector alone does not + # close it. + # + # ONE READER FOR THE HEADLINE, because two readers gave one report two answers. + # This line derived from `_acc_observed`, built with the NARROW reader, while the + # population reconciliation three lines above counts `_acc_accounted`, built with + # the WIDE one -- so a single PG 17 matrix report said `accounted=237` and then + # `of those, 235 accounted for their checks and 7 did not`, three lines apart, + # differing by exactly 2 (#928). The figure a reader acts on is the second, + # because it is the one phrased as a problem, and it overstated the debt. + # + # THE GAP IS A DIFFERENT MECHANISM, NOT A DEBT. The wide reader also accepts a + # suite that prints its own `checks run:` line. Measured on this tree: twelve + # registered suites never call `pgc_summary`, and of those exactly two -- + # `bench_guards` and `docs_style` -- emit a tally of their own, which is the 2. + # The other ten keep no tally at all and are the real debt. + # + # THE NAMES ARE PRINTED, NOT COUNTED, so nothing here can go stale: the two are + # named by the run rather than by this comment, and a third adopting its own + # mechanism appears without anyone editing a number. That is the rule this + # directory learned from nine collisions on one written count in a day. _acc_ran="$(grep -c . "$_acc_observed" 2>/dev/null || true)" + _acc_any="$(grep -c . "$_acc_accounted" 2>/dev/null || true)" + _acc_own="$(pgc_own_mechanism_suites "$_acc_observed" "$_acc_accounted" | tr '\n' ' ')" echo " suites that ran: $suites_ran of ${#SUITES[@]} (skipped: $suites_skipped, incomplete: $suites_incomplete)" - echo " of those, $_acc_ran accounted for their checks and $((suites_ran - _acc_ran)) did not" + echo " of those, $_acc_any accounted for their checks and $((suites_ran - _acc_any)) did not" + if [ -n "${_acc_own// /}" ]; then + echo " $_acc_ran via lib.sh's accounting; by their own mechanism:${_acc_own% }" + fi if [ "$suites_skipped" != 0 ]; then echo " skipped:${skipped_names}" fi diff --git a/test/selftest/390-a-registered-suite-must-account.sh b/test/selftest/390-a-registered-suite-must-account.sh index 90f35a9b..c8228077 100644 --- a/test/selftest/390-a-registered-suite-must-account.sh +++ b/test/selftest/390-a-registered-suite-must-account.sh @@ -623,3 +623,77 @@ check "and a failed population reconciliation fails the major" \ # is the whole reason it is a file and not a number in the environment. check "the debt file is in the tree" \ "$([ -f "$PGC_TESTDIR/suites_without_accounting.txt" ] && echo yes || echo no)" "yes" + +# ---- one report, one answer to "how many accounted" (#928) --------------------- +# +# A single PG 17 matrix report printed two different answers three lines apart: +# +# population reconciliation: registered=251 | accounted=237, ... | sum=251 +# of those, 235 accounted for their checks and 7 did not +# +# 237 and 235, both describing suites that accounted for their checks, differing by +# exactly 2. The population line counted with the WIDE reader and the breakdown line +# derived from the NARROW one, so the figure phrased as a problem -- the second -- +# overstated the debt. The breakdown exists to stop an overcount, and deriving it from +# the narrower reader reintroduced a smaller version of the same overcount in the line +# added to close it. +# +# THE GAP IS A DIFFERENT MECHANISM, NOT A DEBT. The wide reader also accepts a suite +# that prints its own `checks run:` line. Measured on this tree: of the twelve +# registered suites that never call `pgc_summary`, exactly two emit a tally of their own +# -- `bench_guards` and `docs_style` -- and the other ten keep none. The two are the 2. +# +# THE NAMES ARE PRINTED BY THE RUN, not counted here: a third suite adopting its own +# mechanism appears without anyone editing a number. + +check "premise: the runner defines the own-mechanism difference this part evals" \ + "$(grep -c '^pgc_own_mechanism_suites()' "$_rv")" "1" +eval "$(sed -n '/^pgc_own_mechanism_suites()/,/^}/p' "$_rv")" +eval "$(sed -n '/^pgc_log_shows_any_accounting()/,/^}/p' "$_rv")" +check "premise: it is callable" "$(type -t pgc_own_mechanism_suites)" "function" +check "premise: and so is the wide reader it is paired with" \ + "$(type -t pgc_log_shows_any_accounting)" "function" + +# THE TWO READERS MUST DISAGREE ON EXACTLY ONE SHAPE, which is the whole premise. A log +# carrying only `checks run:` is accounted by the wide reader and not by the narrow one. +_o28="$PGC_WORKDIR/acc928"; mkdir -p "$_o28" +# THE FIXTURE CARRIES THE SHAPE THE NARROW READER ACTUALLY WANTS, and that shape +# moved under this branch: lib.sh's accounting line gained a `skipped` term, so the +# four-term form this fixture first wrote stopped being accepted. The premise arm +# below is what said so -- it went red on the rebase with `got [no] want [yes]`, +# which is precisely the job of a premise that asserts a fixture really is in the +# state the test needs. Without it the two readers would have agreed on this log for +# the wrong reason and the arm about their disagreement would have been vacuous. +printf 'accounting: 3 passed + 0 failed + 0 unrunnable + 0 skipped = 3\nx.sh: PASSED\n' > "$_o28/libsh.log" +printf 'checks run: 9\nowntally.sh: PASSED\n' > "$_o28/own.log" +printf 'some output\nPASSED\n' > "$_o28/neither.log" +check "premise: a lib.sh accounting line is seen by the narrow reader" \ + "$(pgc_log_shows_accounting "$_o28/libsh.log")" "yes" +check "premise: a private tally is NOT seen by the narrow reader" \ + "$(pgc_log_shows_accounting "$_o28/own.log")" "no" +check "premise: but IS seen by the wide one, which is where the 2 came from" \ + "$(pgc_log_shows_any_accounting "$_o28/own.log")" "yes" +check "premise: and a log with neither is seen by neither" \ + "$(pgc_log_shows_any_accounting "$_o28/neither.log")$(pgc_log_shows_accounting "$_o28/neither.log")" "nono" + +# The difference names the private-tally suite and nothing else. +printf 'alpha\ngamma\n' > "$_o28/narrow" +printf 'alpha\nbeta\ngamma\n' > "$_o28/wide" +check "the own-mechanism difference names the suite the readers disagree about" \ + "$(pgc_own_mechanism_suites "$_o28/narrow" "$_o28/wide" | tr '\n' ' ')" "beta " +check "and names nothing when the two readers agree" \ + "$(pgc_own_mechanism_suites "$_o28/wide" "$_o28/wide" | grep -c . || true)" "0" +# UNSORTED INPUT, because the real files are appended in SUITES order and `comm` on +# unsorted input answers wrongly without saying so. +printf 'gamma\nalpha\n' > "$_o28/narrow_unsorted" +printf 'gamma\nbeta\nalpha\n' > "$_o28/wide_unsorted" +check "and sorts its inputs, because the runner appends them in SUITES order" \ + "$(pgc_own_mechanism_suites "$_o28/narrow_unsorted" "$_o28/wide_unsorted" | tr '\n' ' ')" "beta " + +# And the headline must come from the SAME file the population line counts. +check "the breakdown headline counts the wide set, not the narrow one" \ + "$(grep -c 'of those, \$_acc_any accounted for their checks' "$_rv")" "1" +check "and the population reconciliation counts that same file" \ + "$(grep -c '_acc_any="\$(grep -c \. "\$_acc_accounted"' "$_rv")" "1" +check "premise: and the narrow count is still printed, as the lib.sh half" \ + "$(grep -c 'via lib.sh.s accounting; by their own mechanism' "$_rv")" "1"