diff --git a/.github/scripts/unrunnable-arm-names.py b/.github/scripts/unrunnable-arm-names.py new file mode 100644 index 00000000..4ee76937 --- /dev/null +++ b/.github/scripts/unrunnable-arm-names.py @@ -0,0 +1,306 @@ +#!/usr/bin/env python3 +"""Every unrunnable record must name the check it stands in for. + +#1040. `check_unrunnable NAME REASON DETAIL` gives one check the honesty +`pgc_skip` gives a whole suite: the check did not run, and the reader is told +WHICH. That only works if the record carries the name the check uses when it DOES +run. Where the two spellings differ the property has two ledger keys, and which +one appears depends on runtime state -- on `hilbert_locality` it depended on +whether that box's two partitions came out different that day. + +This is part 470's property one helper over. 470 asks whether a skip LOOP still +names the arms its sibling branch emits; this asks whether an unrunnable record +names a check the same file asserts anywhere. + +`check_skip` IS NOT SWEPT, and that is a finding rather than an omission. A +skipped arm has no runnable counterpart by construction -- the name IS the arm -- +so 21 of its 23 call sites have no twin and always will. Sweeping it would report +21 mismatches that are all correct code. For the same reason a SKIP record is not +a twin: neither side ran. + +WHAT IS AND IS NOT COMPARED, printed rather than assumed: + + sites every name passed to a refusal emitter + dynamic the whole name is one shell expansion, so there is no literal to + compare (`arms_unrunnable` reads its names from a list) + compared the rest, where a missing twin is a real finding + +`dynamic` is printed for the reason 470 prints `interpolated`: a comparison +nobody makes and a comparison that passes are indistinguishable in a total. + +THREE THINGS THIS TOOL DERIVES RATHER THAN LISTS, each because a list of it was +wrong first: + +1. WHICH FUNCTIONS RECORD, from the `pgc_record` call in their body. Two + hand-written lists gave two wrong answers on this question. One omitted + `pgc_pass`, and `projection_rewrite.sh` then reported a false orphan because + its runnable twin records through `pgc_pass` and not a `check_*` helper. + +2. WHICH ARGUMENT IS THE NAME. `pgc_skip ` records + `pgc_record FAIL "$2"`: the first quoted argument is the CAPABILITY, not the + name. A reader assuming argument one takes `arrow` where the check is called + `arrow support is present` -- the bash-side mirror of the name-position defect + #1036 and #1038 closed on the python side. 22 call sites. + `pgc_require_tools` records a FIXED name and takes no name argument at all. + +3. WHAT COUNTS AS A REFUSAL, from the verdict recorded (`UNRUN`), not from the + helper's spelling. + +THE TWO SETS ARE ASYMMETRIC ON PURPOSE, because their failure directions are. +An over-broad REFUSAL set sweeps a site that did not need sweeping: extra work, +visible. An over-broad TWIN set supplies a name nothing records and turns a real +mismatch green: silent. So refusal wrappers are followed transitively, while the +twin set is restricted to `lib.sh` and to suite-local definitions spelled +`check` or `check_`. `check[a-z_]*` is NOT that spelling: it also matches +`checks_in` in `decode_interrupts.sh`, a counting utility that records nothing +(@jdatcmd). A population named after a prefix is not a population named after a +behaviour. + +AND THE SUBJECT MUST NOT BE IN THE REFERENCE SET. The first sweep of this +property subtracted the unrunnable names from the runnable set -- removing the +very names it was looking up -- so every name could only report absent and a +nine-for-nine file read as nine orphans. `--show-emitters` prints both sets so +the separation is inspectable rather than asserted. +""" + +import pathlib +import re +import sys + +_HELPER_DEF = re.compile(r"^([a-z_][a-z0-9_]*)\(\)\s*\{", re.M) +_SUITE_LOCAL = re.compile(r"^check(?:_[a-z_]+)?$") +_HEREDOC = re.compile(r"<<-?\s*'?\"?([A-Za-z_][A-Za-z0-9_]*)'?\"?\s*$") +_BARE_VAR = re.compile(r"^\$\{?([A-Za-z_][A-Za-z0-9_]*|[0-9]+)\}?$") + + +def blank_heredocs(lines): + """-> the lines with heredoc BODIES blanked out. + + A fixture written into a heredoc is text, not shell. Kept because + `skip-loop-arms.py` needs it on this corpus; `--keep-heredocs` runs without + it so the part can show the answer does not depend on it. + """ + out, term = [], None + for l in lines: + if term is None: + m = _HEREDOC.search(l) + out.append(l) + if m: + term = m.group(1) + else: + out.append("") + if l.strip() == term: + term = None + return out + + +def bodies(text): + """-> [(name, body)] with the body ended by ITS OWN closing brace. + + `text.find("\n}")` is wrong for a one-line definition: `q() { psql ...; }` in + `audit.sh` closes on its own line, so a search for a brace at column zero ran + on to the NEXT function's and swallowed every `check` call in between. `q` + then appeared in the twin set -- a psql wrapper offering SQL text as check + names. Depth is counted per line from the opening brace. + """ + out = [] + lines = text.splitlines() + for i, l in enumerate(lines): + m = _HELPER_DEF.match(l) + if not m: + continue + depth = l.count("{") - l.count("}") + body = [l[m.end():]] + j = i + 1 + while j < len(lines) and depth > 0: + depth += lines[j].count("{") - lines[j].count("}") + body.append(lines[j]) + j += 1 + out.append((m.group(1), "\n".join(body))) + return out + + +def split_args(line, fn): + """-> the arguments of `fn` on this line, as written. + + A quoted argument yields its contents; an unquoted one yields the word. The + POSITION is what matters, so counting only quoted strings would misread + `pgc_skip $cap "the message"`. + """ + m = re.match(r"^(?:.*?(?:;|&&|\|\||\||&))?\s*%s\s+(.*)$" % re.escape(fn), line) + if not m: + return None + rest, args, buf, q, esc = m.group(1), [], "", None, False + for ch in rest: + if esc: + buf += ch; esc = False; continue + if ch == "\\": + esc = True; continue + if q: + if ch == q: + q = None + else: + buf += ch + continue + if ch in "\"'": + q = ch; continue + if ch.isspace(): + if buf: + args.append(buf); buf = "" + continue + if ch in "#;&|" and not buf: + break + buf += ch + if buf: + args.append(buf) + return args + + +def _resolve(expr, body): + """-> the 1-based argument position the name comes from, or None. + + `pgc_record PASS "$1"` is position 1. `check()` writes `local name="$1"` and + then records `"$name"`, so a one-step assignment is followed. + """ + m = _BARE_VAR.match(expr.strip()) + if not m: + return None + tok = m.group(1) + if tok.isdigit(): + return int(tok) + a = re.search(r"\b%s=\"\$\{?([0-9]+)\}?\"" % re.escape(tok), body) + return int(a.group(1)) if a else None + + +def recorders(text, lib): + """-> (refusal, twins): {fn: name-position-or-None}, by the verdict recorded. + + None as a position means the function records a FIXED name, which still + contributes that literal (`pgc_require_tools`) but takes no name argument. + """ + defs, in_lib = [], set() + for src, is_lib in ((lib, True), (text, False)): + for name, body in bodies(src): + defs.append((name, body)) + if is_lib: + in_lib.add(name) + + refusal, twins, fixed = {"check_unrunnable": 1}, {}, {} + for _ in range(2): + for name, body in defs: + eligible = name in in_lib or _SUITE_LOCAL.match(name) + for verdict, expr in re.findall(r"pgc_record\s+(\w+)\s+\"([^\"]*)\"", body): + pos = _resolve(expr, body) + if verdict == "UNRUN": + refusal.setdefault(name, pos) + elif verdict == "SKIP": + pass # neither side ran; not a twin + elif eligible and name not in refusal: + # A REFUSAL EMITTER IS NEVER ITS OWN TWIN. `check_unrunnable` + # records FAIL when the reason code is not one of the closed + # list -- a harness-misuse record, not the check running. Left + # in the twin set it also blocked wrapper detection, because a + # wrapper's body naming `check_unrunnable` then looked like a + # body naming a runnable helper. + if pos is None and "$" not in expr: + fixed.setdefault(name, expr) + twins.setdefault(name, pos) + # A wrapper that reaches a refusal emitter and no recorder is itself a + # refusal: over-broad on the safe side. + if name not in refusal and not re.search(r"\bpgc_record\b", body): + if any(re.search(r"\b%s\b" % re.escape(r), body) for r in refusal): + if not any(re.search(r"\b%s\b" % re.escape(t), body) for t in twins): + refusal[name] = None + # A helper that DELEGATES its name reaches the twin set through the + # delegate, whether or not it also records directly: `check_timing` + # records SKIP itself and passes the same name on to `check`. + if name not in refusal and name not in twins and eligible: + if True: + for t, tpos in list(twins.items()): + c = re.search(r"\b%s\s+(.*)" % re.escape(t), body) + if c and tpos: + args = split_args("\t" + t + " " + c.group(1), t) or [] + # `check_ratio "$@"` forwards every argument, so the + # delegate's name position IS this one's. Without this + # `check_ratio_needs_quiet_machine` was absent from the + # twin set for no reason a reader could see, and an + # unexplained hole in a guard's own population is the + # thing the guard is supposed to be better than. + if args[:1] == ["$@"]: + twins.setdefault(name, tpos) + elif len(args) >= tpos: + p = _resolve(args[tpos - 1], body) + if p: + twins.setdefault(name, p) + break + for k in refusal: + twins.pop(k, None) + return refusal, twins, fixed + + +def names_for(lines, emitters): + """-> [(name, lineno)] taking each emitter's OWN name argument, not argument one.""" + out = [] + for i, l in enumerate(lines): + for fn, pos in emitters.items(): + if pos is None: + continue + args = split_args(l, fn) + if args and len(args) >= pos: + out.append((args[pos - 1], i + 1)) + break + return out + + +def main(testdir, show_emitters=False, keep_heredocs=False): + root = pathlib.Path(testdir) + files = sorted(root.glob("*.sh")) + lib = (root / "lib.sh").read_text(errors="replace") if (root / "lib.sh").exists() else "" + + sites = dynamic = compared = 0 + bad = [] + all_ref, all_twin, all_fixed = {}, {}, {} + for f in files: + text = f.read_text(errors="replace") + lines = text.splitlines() + if not keep_heredocs: + lines = blank_heredocs(lines) + refusal, twins, fixed = recorders(text, lib) + all_ref.update(refusal); all_twin.update(twins); all_fixed.update(fixed) + unrun = names_for(lines, refusal) + if not unrun: + continue + seen = {n for n, _ in names_for(lines, twins)} | set(fixed.values()) + for name, lineno in unrun: + sites += 1 + if _BARE_VAR.match(name): + dynamic += 1 + continue + compared += 1 + if name not in seen: + bad.append(f"{f.name}:{lineno} {name}") + + print(f"files {len(files)}") + print(f"sites {sites}") + print(f"dynamic {dynamic}") + print(f"compared {compared}") + for b in bad: + print(f"MISMATCH {b}") + if show_emitters: + print("refusal " + " ".join(f"{k}:{v}" for k, v in sorted(all_ref.items()))) + print("twins " + " ".join(f"{k}:{v}" for k, v in sorted(all_twin.items()))) + print("fixed " + " ".join(sorted(all_fixed))) + # THE EXIT CODE IS THE VERDICT. It was a flat 0 in the first version, so the + # tool printed two MISMATCH lines and reported success. Part 480 gates on the + # parsed output and was never fooled, but the next caller is the hazard: anyone + # wiring this into CI and trusting `$?` would get a gate that cannot fail. + # Reported by @jdatcmd, who re-measured it without a pipe before believing it, + # because the first reading was `tail`'s status and not this program's. + return 1 if bad else 0 + + +if __name__ == "__main__": + args = [a for a in sys.argv[1:] if not a.startswith("--")] + sys.exit(main(args[0] if args else "test", + show_emitters="--show-emitters" in sys.argv, + keep_heredocs="--keep-heredocs" in sys.argv)) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ed4c46c..bfc30042 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,71 @@ true until the next version shipped. ### Added +- An unrunnable record names the check it stands in for, so a refused check keeps + one ledger key instead of two (#1040). + + `check_unrunnable NAME REASON DETAIL` gives one check the honesty `pgc_skip` gives a + whole suite: the check did not run, and the reader is told which. That only works if + the record carries the name the check uses when it DOES run. Two of the four sites in + one loop in `hilbert_locality.sh` carried a shortened name: + + :574 check_unrunnable "box $box: groups read, Z-order" + :597 check_num "box $box: groups read over $PLACEMENTS placements, Z-order" + + while the other two matched their runnable twins exactly. So the property had two + ledger keys and which one appeared depended on whether that box's two partitions came + out different that day -- the key was a function of the data. `hilbert_locality` is not + a ledger-covered suite, so this was a wrong key waiting to be seeded rather than a + wrong number in `check_ledger.tsv`. + + The convention is already near-universal, and that is what made the lapse invisible: + 23 of the 25 `check_unrunnable` call sites in `test/*.sh` carry the runnable name + (`hilbert_cluster` 9 of 9, `projection_rewrite` 11 of 11), so a shorter name in a new + refusal branch reads as ordinary. Both halves of a two-branch site are rarely read + together. + + A new selftest part asserts the property over the whole corpus, driving + `.github/scripts/unrunnable-arm-names.py`. The tool DERIVES three things a list of it + got wrong first: which functions record, from the `pgc_record` call in their body; + which argument is the name, because `pgc_skip` records `"$2"` and reading argument one + takes the capability where the check is called `arrow support is present`; and what + counts as a refusal, from the verdict rather than the helper's spelling. `check_skip` + is deliberately not swept -- a skipped arm has no runnable counterpart by construction, + so 21 of its 23 call sites have no twin and always will. + + The part carries a positive control because the guard's steady state is zero and a + broken sweep reports zero too: it drives the real tool over a fixture whose refusal + branch names something the file never records, and over a control where the names + agree. + + This also closes the only pair `compare_to_bash.py` fails once its extractor is + widened to all eight `lib.sh` helpers (#1040): with the two names fixed, all seven + ported suites grade one-for-one under the widened extractor, with no change to the + port and none to `pgc_vacuity.py`. + + The sweep's exit code is the verdict. It was a flat zero in the first version, so the + tool printed two `MISMATCH` lines and reported success. The part gates on the parsed + output and was never fooled, which is exactly why the exit code needed its own arms + rather than an observation: the next caller is the one that trusts `$?`, and a gate that + cannot fail is a trap whether or not today's only caller steps in it. Three arms, because + pinning the non-zero side alone passes on a tool that always exits 1, and the clean side + alone passes on a corpus with nothing to find -- so the clean run uses a second fixture + directory that HAS a refusal site, with a premise asserting it. Reported by @jdatcmd. + +- The census re-derivation printed in `check_ledger_budget.txt` reads the wrong field + and returns zero (#1040). + + awk -F'\t' '$4=="never"' test/check_ledger.tsv | wc -l -> 0 + awk -F'\t' '$5=="never"' test/check_ledger.tsv | wc -l -> 1189 on db74d9e + + #1010 inserted the majors a row claims as field 4, moving the last-red to field 5, and + the recipe stayed on field 4. The entry for #1010 in this file states that + `check_ledger_budget.txt` carries the corrected form; it did not. The gate is + unaffected -- it computes the census itself and never runs this command -- so the harm + is a reviewer re-deriving the number by the printed method, getting 0, and correcting a + budget that was right. Fixed here rather than filed because this change moves that very + number, and a wrong recipe beside a number nobody can check is worse than no recipe. + - `allow_empty` documented a rule the code did not enforce (#1031). `Expect.rows` documents the argument as taking *"a REASON, not a flag"*. The sentence even diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index de82e5c4..6d62fbbb 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -1136,6 +1136,18 @@ harness_selftest 470-a-skipped-arm-records-under-its-own-name every comparable s harness_selftest 470-a-skipped-arm-records-under-its-own-name premise: and it actually compared some of them 15;16;17;18;19 never - harness_selftest 470-a-skipped-arm-records-under-its-own-name premise: the sweep found skip loops at all 15;16;17;18;19 never - harness_selftest 470-a-skipped-arm-records-under-its-own-name premise: the sweep tool is present 15;16;17;18;19 never - +harness_selftest 480-an-unrunnable-record-names-its-check and exits zero on a corpus that has refusal sites and no mismatch 15;16;17;18;19 never - +harness_selftest 480-an-unrunnable-record-names-its-check and it stays silent on the same file with the names in agreement 15;16;17;18;19 never - +harness_selftest 480-an-unrunnable-record-names-its-check every unrunnable record names a check its own suite asserts (#1040) 15;16;17;18;19 never - +harness_selftest 480-an-unrunnable-record-names-its-check pgc_pass and pgc_fail are twins, because a check can be recorded through them 15;16;17;18;19 never - +harness_selftest 480-an-unrunnable-record-names-its-check pgc_skip's name is read from argument TWO, not argument one 15;16;17;18;19 never - +harness_selftest 480-an-unrunnable-record-names-its-check premise: and it compared them rather than writing them all off as dynamic 15;16;17;18;19 never - +harness_selftest 480-an-unrunnable-record-names-its-check premise: that clean run had a refusal site to be silent ABOUT 15;16;17;18;19 never - +harness_selftest 480-an-unrunnable-record-names-its-check premise: the sweep found unrunnable sites at all 15;16;17;18;19 never - +harness_selftest 480-an-unrunnable-record-names-its-check premise: the sweep tool is present 15;16;17;18;19 never - +harness_selftest 480-an-unrunnable-record-names-its-check the refusal emitters are the two the tree defines 15;16;17;18;19 never - +harness_selftest 480-an-unrunnable-record-names-its-check the sweep EXITS non-zero when it reports a mismatch 15;16;17;18;19 never - +harness_selftest 480-an-unrunnable-record-names-its-check the sweep REPORTS a name the file records only in its refusal branch 15;16;17;18;19 never - native_join_runtime_filter native_join_runtime_filter 3-table answer equals filter-off 15;16;17;18;19 never - native_join_runtime_filter native_join_runtime_filter 3-table join order matches filter-off 15;16;17;18;19 never - native_join_runtime_filter native_join_runtime_filter 3-table plan has coordinator 15;16;17;18;19 never - diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index 4e54a8ad..ff21e21a 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -38,11 +38,20 @@ suites_not_covered 249 # HOW TO RE-DERIVE IT, written here because a changelog entry got it wrong and a # derivation is only useful where the number is: # -# awk -F'\t' '$4=="never"' test/check_ledger.tsv | wc -l +# awk -F'\t' '$5=="never"' test/check_ledger.tsv | wc -l +# +# FIELD FIVE, and it said four until #1040. The command is only useful if it +# runs, and `$4` returned 0 on the shipped tree: #1010 inserted the majors a row +# claims as field 4, so the last-red moved to field 5 and the old form read a +# major where it expected a date. The CHANGELOG entry for #1010 says this file +# carries the corrected form; it did not, which is why the claim is worth as +# little as the recipe. Corrected here rather than filed because this change +# moves the number below, and a reviewer re-deriving it with the printed command +# would have got 0 against a stated 1198. # # Rows whose LAST-RED is `never`, not the row count. The two agree only while # nothing has ever been observed red, so a plain `grep -c` overcounts by exactly # the number of attacked checks -- and it overcounts from the first moment this # ledger does the job it exists for. The gate prints both quantities side by side # (`rows=N | never observed red=M`) because they are different questions. -checks_never_observed_red 1189 +checks_never_observed_red 1201 diff --git a/test/hilbert_locality.sh b/test/hilbert_locality.sh index dde3d51a..3c511d31 100755 --- a/test/hilbert_locality.sh +++ b/test/hilbert_locality.sh @@ -571,9 +571,9 @@ for pin in $PINS; do IFS=: read -r box want_z want_h floor <<<"$pin" if [ "$CURVE_DIFFERS" != different ]; then - check_unrunnable "box $box: groups read, Z-order" UNMET_PRECONDITION \ + check_unrunnable "box $box: groups read over $PLACEMENTS placements, Z-order" UNMET_PRECONDITION \ "the two partitions are not different ($CURVE_DIFFERS), so a ratio between them is not about the curve" - check_unrunnable "box $box: groups read, Hilbert" UNMET_PRECONDITION \ + check_unrunnable "box $box: groups read over $PLACEMENTS placements, Hilbert" UNMET_PRECONDITION \ "the two partitions are not different ($CURVE_DIFFERS), so a ratio between them is not about the curve" check_unrunnable "box $box: Hilbert reads fewer groups than Z-order" UNMET_PRECONDITION \ "the two partitions are not different ($CURVE_DIFFERS)" diff --git a/test/selftest/480-an-unrunnable-record-names-its-check.sh b/test/selftest/480-an-unrunnable-record-names-its-check.sh new file mode 100644 index 00000000..244352da --- /dev/null +++ b/test/selftest/480-an-unrunnable-record-names-its-check.sh @@ -0,0 +1,131 @@ +# ---- an unrunnable record must name the check it stands in for -------------- +# +# #1040. `check_unrunnable NAME REASON DETAIL` gives one check the honesty +# `pgc_skip` gives a whole suite: the check did not run, and the reader is told +# WHICH. That only works if the record carries the name the check uses when it +# DOES run. Where the two spellings differ the property has TWO ledger keys and +# which one appears depends on runtime state -- in `hilbert_locality` it depended +# on whether that box's two partitions came out different that day, so the key +# was a function of the data. +# +# THIS IS PART 470's PROPERTY ONE HELPER OVER. 470 asks whether a skip LOOP still +# names the arms its sibling branch emits. This asks whether an unrunnable record +# names a check the same file asserts anywhere. +# +# WHY A GUARD AND NOT JUST THE RENAME. The convention is already near-universal +# and that is exactly what makes a lapse invisible: 23 of 25 call sites carry the +# runnable name (`hilbert_cluster` 9 of 9, `projection_rewrite` 11 of 11), so a +# reader skimming a new refusal branch sees nothing unusual in a shorter name. +# Both halves of a two-branch site are rarely read together. +# +# `check_skip` IS NOT SWEPT and that is a finding rather than an omission. A +# skipped arm has no runnable counterpart by construction -- the name IS the arm +# -- so 21 of its 23 call sites have no twin and always will. Sweeping it would +# report 21 mismatches that are all correct code. +# +# THE POSITIVE CONTROL BELOW IS THE POINT OF THE PART. A sweep that matched +# nothing reports the same zero as a corpus in agreement, and this guard's steady +# state is zero, so nothing else would ever distinguish the two. The fixture arms +# drive the real tool over a file whose answer is known. + +_un_py="$TESTDIR/../.github/scripts/unrunnable-arm-names.py" + +check "premise: the sweep tool is present" \ + "$([ -r "$_un_py" ] && echo yes || echo no)" "yes" + +_un_out="$(python3 "$_un_py" "$TESTDIR" --show-emitters 2>&1)" || _un_out="TOOL FAILED: $_un_out" + +_un_sites=$(printf '%s\n' "$_un_out" | sed -n 's/^sites \([0-9]*\)$/\1/p') +_un_dyn=$(printf '%s\n' "$_un_out" | sed -n 's/^dynamic \([0-9]*\)$/\1/p') +_un_cmp=$(printf '%s\n' "$_un_out" | sed -n 's/^compared \([0-9]*\)$/\1/p') +_un_ref=$(printf '%s\n' "$_un_out" | sed -n 's/^refusal //p') +_un_twins=$(printf '%s\n' "$_un_out" | sed -n 's/^twins //p') +_un_bad="$(printf '%s\n' "$_un_out" | sed -n 's/^MISMATCH //p' | tr '\n' ' ' | sed 's/ $//')" + +echo "-- unrunnable names: $_un_sites sites, $_un_cmp compared, $_un_dyn dynamic" + +# THE POPULATION IS THE PREMISE, twice. A sweep that found nothing and a corpus in +# agreement both print zero mismatches, and so does a sweep that found sites and +# compared none of them. +check "premise: the sweep found unrunnable sites at all" \ + "$([ "${_un_sites:-0}" -ge 20 ] && echo yes || echo "only ${_un_sites:-none}")" "yes" +check "premise: and it compared them rather than writing them all off as dynamic" \ + "$([ "${_un_cmp:-0}" -ge 20 ] && echo yes || echo "only ${_un_cmp:-none}")" "yes" + +# THREE PINS, each on a value that was WRONG in a draft of the tool, so each is a +# regression test rather than a restatement of the code. +# +# The refusal set decides what gets swept. A new refusal emitter must be looked at +# rather than silently sweep or silently not sweep; `arms_unrunnable` is here +# because it wraps `check_unrunnable` and reads its names from a list, so it must +# be recognised as a refusal and must NOT be offered as a twin. +check_text "the refusal emitters are the two the tree defines" \ + "$_un_ref" "arms_unrunnable:None check_unrunnable:1" + +# pgc_skip records `pgc_record FAIL "$2"`: its first quoted argument is the +# CAPABILITY, not the name. Reading argument one takes `arrow` where the check is +# called `arrow support is present` -- the bash-side mirror of the name-position +# defect #1036 and #1038 closed on the python side, across 22 call sites. +check "pgc_skip's name is read from argument TWO, not argument one" \ + "$(printf '%s\n' "$_un_twins" | tr ' ' '\n' | grep -c '^pgc_skip:2$')" "1" + +# pgc_pass was missing from two hand-written runnable lists, and its absence made +# projection_rewrite.sh report a false orphan: the twin of its unrunnable name is +# recorded by pgc_pass and not by any check_* helper. +check "pgc_pass and pgc_fail are twins, because a check can be recorded through them" \ + "$(printf '%s\n' "$_un_twins" | tr ' ' '\n' | grep -c '^pgc_\(pass\|fail\):1$')" "2" + +# ---- the positive control --------------------------------------------------- +# +# Driven over a fixture whose answer is known, because this guard's steady state +# is zero mismatches and a broken sweep reports zero too. +_un_fx="$(mktemp -d)" +cp "$TESTDIR/lib.sh" "$_un_fx/lib.sh" + +cat > "$_un_fx/offender.sh" <<'PGC_FX_BAD' +check_num "the property, said one way" "$a" "$b" +check_unrunnable "the property, said another way" UNMET_PRECONDITION "no fixture" +PGC_FX_BAD + +cat > "$_un_fx/control.sh" <<'PGC_FX_GOOD' +check_num "the property, said one way" "$a" "$b" +check_unrunnable "the property, said one way" UNMET_PRECONDITION "no fixture" +PGC_FX_GOOD + +# A SECOND FIXTURE DIRECTORY holding only the agreeing file, so the clean exit code +# is measured on a corpus that HAS a refusal site rather than on one with nothing to +# find -- those two report the same 0 and only one of them is evidence. +_un_fxok="$(mktemp -d)" +cp "$TESTDIR/lib.sh" "$_un_fxok/lib.sh" +cp "$_un_fx/control.sh" "$_un_fxok/control.sh" + +# NO PIPE ON EITHER RUN. `$?` after a pipeline is the LAST stage's, which is how the +# missing exit code first read as present (@jdatcmd). +_un_fxout="$(python3 "$_un_py" "$_un_fx" 2>&1)"; _un_fxrc=$? +_un_okout="$(python3 "$_un_py" "$_un_fxok" 2>&1)"; _un_okrc=$? + +check "the sweep REPORTS a name the file records only in its refusal branch" \ + "$(printf '%s\n' "$_un_fxout" | grep -c '^MISMATCH offender.sh:2 the property, said another way$')" "1" +check "and it stays silent on the same file with the names in agreement" \ + "$(printf '%s\n' "$_un_fxout" | grep -c '^MISMATCH control.sh')" "0" + +# THE EXIT CODE IS THE VERDICT, pinned in both directions. It was a flat 0 in the +# first version of the tool: two MISMATCH lines printed and success reported. This +# part gates on the parsed output and so was never fooled, which is exactly why the +# hazard needs its own arm -- the next caller is the one that trusts `$?`. +check "the sweep EXITS non-zero when it reports a mismatch" \ + "$([ "$_un_fxrc" -ne 0 ] && echo yes || echo "no, rc=$_un_fxrc")" "yes" +check "and exits zero on a corpus that has refusal sites and no mismatch" \ + "$_un_okrc" "0" +check "premise: that clean run had a refusal site to be silent ABOUT" \ + "$(printf '%s\n' "$_un_okout" | sed -n 's/^compared \([0-9]*\)$/\1/p')" "1" + +rm -rf "$_un_fx" "$_un_fxok" + +# ---- the tree --------------------------------------------------------------- + +check "every unrunnable record names a check its own suite asserts (#1040)" \ + "$_un_bad" "" + +unset _un_py _un_out _un_sites _un_dyn _un_cmp _un_ref _un_twins _un_bad +unset _un_fx _un_fxok _un_fxout _un_okout _un_fxrc _un_okrc diff --git a/test/selftest/parts.manifest b/test/selftest/parts.manifest index b3a184ad..13653435 100644 --- a/test/selftest/parts.manifest +++ b/test/selftest/parts.manifest @@ -44,3 +44,4 @@ 450-a-red-nightly-must-be-findable.sh 460-the-controller-must-record-the-binary.sh 470-a-skipped-arm-records-under-its-own-name.sh +480-an-unrunnable-record-names-its-check.sh