diff --git a/prolog-sessions/kb.pl b/prolog-sessions/kb.pl index 4f6ee6ac..24ff75bb 100644 --- a/prolog-sessions/kb.pl +++ b/prolog-sessions/kb.pl @@ -221,6 +221,7 @@ doctrine(design_against_the_model_before_implementing,'AGENTS.md has always said doctrine(a_key_rule_must_ask_whether_the_relation_is_multi_valued,'scripts/kb keyed every clause on the functor plus all arguments but the last, which is right for doctrine(Name,Text) and wrong for any RELATION: crate_depends_on(A,B) then keyed on A, so a crate depending on two crates read as one key twice and was unaddable. Found by trying to model a proposed crate, not by any test -- the tool had only ever been pointed at single-valued facts. The fix cannot read the KB own functional/1, because that check keys an arity-3 predicate on its FIRST argument, which is right for crate/3 and wrong for decision/3 where one ticket holds several decisions under different names. So key_arity/2 is a hand-maintained table and anything unlisted keys on the WHOLE head, which is the safe direction: an unlisted relation accepts many clauses and only an exact repeat is a duplicate.'). doctrine(a_local_shell_is_not_the_ci_shell,'zsh ABORTS on an unmatched glob and bash leaves it as a literal, so a CI step written and probed in zsh can fail in a way no developer shell can reproduce. node --test web/tests/*.test.js under bash therefore globbed the literal itself, matched nothing, reported 0 tests and exited 0 -- both node steps gated nothing the moment their directory moved (improvise-vz54). Local shell vs CI shell is a fault-injection axis in its own right: probe a CI line under the shell the workflow names, not the one you are typing in. The fix moves the glob into scripts/node-test.sh so the expansion is the script\'s and can be refused.'). doctrine(a_convention_that_keeps_a_checker_quiet_is_the_one_that_blinds_it,'context/test-evidence used #### for browser tests precisely BECAUSE --check-evidence resolved names against Rust fn names only, so a ### heading naming a Playwright test would be reported as an orphan. The workaround made every browser entry invisible, and a deleted test left its heading promising coverage for a whole release (improvise-5ray). Cure: collect ### and #### alike and decide by SHAPE -- a bare identifier is a Rust test, a quoted string is a JS test, anything else is prose naming nothing. When a checker forces a convention on the thing it checks, ask what that convention hides.'). +doctrine(a_refusal_is_evidence_only_when_it_is_the_one_you_named,'Two kb-test cases passed against a tree with the fix they guarded reverted, because a DIFFERENT guard refused the same delta and the assertion was only \"fails, and the kb is unchanged\" (review of PR #69). A tool with several guards will usually refuse a broken delta somehow, so \"it was refused\" is nearly free and says nothing about the fix under test. Assert the REFUSAL MESSAGE, or construct a case only the fix can catch -- and prove it by reverting that one fix and watching the case fail. The same trap in reverse: a case asserting an OUTCOME (the add lands and every sibling survives) can discriminate where a message cannot, because what a wrong key destroys is the outcome.'). work_done('improvise-8tk',bug_fix,'ZZ/:wq quit even when the save failed, silently discarding the model. Save now aborts the effect batch on failure; save_to_current_path/1 returns Result; three tests in ui/effect.rs.'). work_done('improvise-907',bug_fix,'Import wizard step machine redesign: validation_error() gate at ReviewProposals (no-category and over-limit), history-stack previous() bound to Left on every step, and Done made a real confirmation step holding the built workbook. Superseded improvise-gfq, improvise-o1c, improvise-c6j.'). work_done('improvise-6sq',wont_do,'Merging EditOrDrill and EnterEditAtCursorCmd into one EnterEdit{mode} is not safe: they differ in WHEN the cell value is read, not in a mode parameter. Added ui/app.rs::tests::i_and_o_diverge_on_an_empty_records_grid as executable evidence.'). @@ -560,6 +561,10 @@ name_char('_'). name_shaped(A):-atom_chars(A,B),B=[C|D],lower_or_digit(C),forall(member(E,B),name_char(E)),memberchk('_',D). citation_token(A,B):-(sub_atom(A,C,1,D,')')->true;C=999),(sub_atom(A,E,1,F,',')->true;E=999),G is min(C,E),G>0,G<120,sub_atom(A,0,G,H,B). cited_name(A,B,C):-citation_marker(D,B),sub_atom(A,E,F,G,D),sub_atom(A,H,G,0,I),citation_token(I,C),name_shaped(C). +cited_name(A,decision,B):-sub_atom(A,C,D,E,'decision('),sub_atom(A,F,E,0,G),decision_name(G,B),name_shaped(B). citation_resolves(doctrine,A):-doctrine(A,B). citation_resolves(invariant,A):-invariant(A,B). citation_resolves(decision,A):-decision(B,A,C). +citation_body(A,B):-sub_atom(A,C,1,D,')'),!,C>0,C<200,sub_atom(A,0,C,E,B). +trim_left(A,B):-sub_atom(A,0,1,C,' ')->sub_atom(A,1,D,0,E),trim_left(E,B);B=A. +decision_name(A,B):-citation_body(A,C),sub_atom(C,D,1,E,','),!,F is D+1,sub_atom(C,F,G,0,H),trim_left(H,B). diff --git a/scripts/check-comment-runs.py b/scripts/check-comment-runs.py index cdb0cc88..fa719f96 100755 --- a/scripts/check-comment-runs.py +++ b/scripts/check-comment-runs.py @@ -254,11 +254,12 @@ JS_ROOTS = [os.path.join("harness", "tests"), os.path.join("web", "tests")] _RUST_NAME = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$") _QUOTED_NAME = re.compile(r'^"(.+)"$') -# Only a RUNNABLE LEAF test. `.skip`, `.todo` and `.describe` are excluded -- -# a quoted heading must not be satisfiable by disabled coverage or by an empty -# suite, which is the same hole the node wrapper had (review, PR #69). +# Only a RUNNABLE LEAF test. `.skip`, `.todo`, `.describe` and `.only` are all +# excluded: the first three name no executed assertion, and `.only` disables +# every OTHER test in its file, so a heading satisfied by one is a heading +# satisfied by a file that stopped running (review, PR #69). _JS_TEST = re.compile( - r"""(?:^|[^\w.])(?:test|it)(?:\.only)?\s*\(\s*(['"`])(.+?)\1""" + r"""(?:^|[^\w.])(?:test|it)\s*\(\s*(['"`])(.+?)\1""" ) @@ -717,6 +718,11 @@ SELF_TEST_EVIDENCE = [ 'test.skip("a skipped one", () => {});\ntest.todo("a todo one");\n', ["a skipped one", "a todo one"], []), + ("a heading cannot be satisfied by a test.only, which disables its file", + '#### "the only one left running"\n', "", + 'test.only("the only one left running", () => {});\n', + ["the only one left running"], []), + ("a heading cannot be satisfied by an empty suite's name", '#### "a suite with nothing in it"\n', "", 'test.describe("a suite with nothing in it", () => {});\n', diff --git a/scripts/kb-merge.pl b/scripts/kb-merge.pl index 75a3fad5..c9aef2c8 100644 --- a/scripts/kb-merge.pl +++ b/scripts/kb-merge.pl @@ -49,6 +49,7 @@ run(Mode, KbFile, DeltaFile, OutFile) :- check_self_consistent(Funcs, New), conflicts(Funcs, Old, New, Clashes), gate(Mode, Clashes), + check_rule_replace(Funcs, Mode, Old, New), foldl_merge(Funcs, New, Mode, Old, Merged), expected_total(Funcs, Mode, Old, New, Expect), check_total(Merged, Expect), @@ -197,6 +198,26 @@ conflicts(Funcs, Old, New, Clashes) :- member(O, Old), clause_key(Funcs, O, K2), same_key(K, K2) ), Clashes). +% A rule keys on its whole clause, so an EDITED rule matches nothing and +% `replace` silently becomes `add` -- both arms then live, and the stale one +% goes on firing beside the new one. check_total cannot see it: NO + NF is +% correct arithmetic for an add. So replace refuses a rule it cannot find and +% names the route, which is rm then add (improvise-dj1l, review of PR #69). +check_rule_replace(Funcs, replace, Old, New) :- + !, + findall(T, + ( member(T, New), T = (_ :- _), + clause_key(Funcs, T, K), + \+ ( member(O, Old), clause_key(Funcs, O, K2), same_key(K, K2) ) ), + Fresh), + ( Fresh == [] + -> true + ; format("kb-merge: REFUSED -- replace cannot find these rules, so it would ADD them beside the ones already there:~n", []), + forall(member(T, Fresh), format(" ~q~n", [T])), + die('a rule is keyed on its whole clause, so an edited body is a NEW clause: kb rm the old one, then kb add') + ). +check_rule_replace(_, _, _, _). + gate(add, []) :- !. gate(add, Clashes) :- !, diff --git a/scripts/kb-test.sh b/scripts/kb-test.sh index c2b0b569..8039ecb0 100755 --- a/scripts/kb-test.sh +++ b/scripts/kb-test.sh @@ -18,6 +18,11 @@ bad() { printf ' FAIL %s\n' "$1"; fail=$((fail+1)); } fresh() { cp "$KB" "$work/kb.pl"; } kb() { KB_FILE="$work/kb.pl" "$here/kb" "$@"; } unchanged() { diff -q "$KB" "$work/kb.pl" >/dev/null; } +# A refusal is only evidence when it is the refusal you NAMED: two of these +# cases passed against a tree with the fix reverted, because a DIFFERENT guard +# caught the same delta and the assertion was only "fails and kb unchanged" +# (review of PR #69). said() asserts which one fired. +said() { grep -qF "$2" "$1"; } echo "kb-test: the refusals" @@ -313,7 +318,11 @@ fresh printf 'probe_fact(a).\nprobe_fact(b).\n' > "$work/facts.pl" kb add "$work/facts.pl" >/dev/null 2>&1 printf 'probe_fact(X) :- doctrine(X,_).\n' > "$work/rule.pl" -if kb replace "$work/rule.pl" >/dev/null 2>&1; then +# `add`, not `replace`: the rule is new, and a rule that is not already there +# is what case 15 makes replace refuse. This is also the discriminating door -- +# under unification `conflicts` sees the non-ground key match both facts and +# refuses the add outright, so the reverted fix fails HERE. +if kb add "$work/rule.pl" >/dev/null 2>&1; then kept=0 grep -q '^probe_fact(a)\.$' "$work/kb.pl" && grep -q '^probe_fact(b)\.$' "$work/kb.pl" && kept=1 if [ "$kept" = 1 ]; then @@ -329,19 +338,21 @@ if kb replace "$work/rule.pl" >/dev/null 2>&1; then bad "the rule was not written at all" fi else - bad "replace refused a rule over a predicate holding facts" + bad "add refused a rule over a predicate holding facts: a variable key matched them" fi # 11. The duplicate gate must still fire once keys are compared as variants: # two clauses that really are the same key are still two clauses for one key. fresh printf 'probe_rule(X) :- doctrine(X,_).\nprobe_rule(Z) :- doctrine(Z,_).\n' > "$work/same.pl" -if kb add "$work/same.pl" >/dev/null 2>&1; then +if kb add "$work/same.pl" >"$work/out" 2>&1; then bad "add accepted the same clause twice in one delta" -elif unchanged; then +elif ! unchanged; then + bad "add refused a duplicate but MODIFIED the kb" +elif said "$work/out" "names one key twice"; then ok "add still refuses one key named twice, compared as variants" else - bad "add refused a duplicate but MODIFIED the kb" + bad "the delta was refused, but not BY THE DUPLICATE GATE (canon_key is unreached)" fi # 12. The arithmetic check, proved by injecting the fault it exists for: a @@ -400,5 +411,92 @@ else bad "the citation check refuses a name that DOES resolve" fi +# 14. The case only same_key/2 can catch. A rm key with a variable in a +# NON-final position -- work_done keys on two arguments -- matches nothing +# as a variant and everything by unification. Under unification it removed +# an arbitrary bug_fix entry and reported success; here it must be refused +# as ABSENT, by that message and no other. +fresh +printf 'work_done(_,bug_fix,_).\n' > "$work/wild.pl" +if kb rm "$work/wild.pl" >"$work/out" 2>&1; then + bad "rm removed clauses by unifying a variable key against them" +elif ! unchanged; then + bad "rm refused a wildcard key but MODIFIED the kb" +elif said "$work/out" "no such clause to remove"; then + ok "a key with a variable in it matches nothing, rather than everything" +else + bad "the wildcard rm was refused, but not as ABSENT (same_key is unreached)" +fi + +# 15. A rule keys on its whole clause, so an EDITED rule matches nothing and +# replace would silently become add -- leaving the stale arm firing beside +# the new one, with correct arithmetic (found by review on PR #69). +fresh +printf 'probe_rule(X) :- doctrine(X,_).\n' > "$work/rule1.pl" +printf 'probe_rule(X) :- doctrine(X,_), X \\== b.\n' > "$work/rule2.pl" +kb add "$work/rule1.pl" >/dev/null 2>&1 +before="$(kb count | grep '^total=' | cut -d= -f2)" +if kb replace "$work/rule2.pl" >"$work/out" 2>&1; then + after="$(kb count | grep '^total=' | cut -d= -f2)" + bad "replace ADDED an edited rule beside the old one ($before -> $after)" +elif said "$work/out" "kb rm the old one"; then + ok "replace refuses a rule it cannot find, and names the route" +else + bad "the edited rule was refused, but without naming rm/add" +fi + +# The negative arm: replace of a rule that IS there must still work, or the +# refusal above would just be "replace never works on rules". +if kb replace "$work/rule1.pl" >/dev/null 2>&1; then + ok "replace of an unchanged rule is still accepted" +else + bad "replace refuses a rule that is present verbatim" +fi + +# 16. All three citation markers must FIRE. doctrine( and invariant( take the +# name as the first field; decision( takes the SECOND, because the repo +# spells decision(, ) and a ticket id is not name-shaped -- +# so a single-field rule left that marker unreachable (review, PR #69). +cat > "$work/cite-d.pl" <<'EOF' +doctrine(a_probe_citing_a_doctrine,'cites doctrine(a_name_that_was_retired), which does not exist'). +EOF +cat > "$work/cite-i.pl" <<'EOF' +doctrine(a_probe_citing_an_invariant,'cites invariant(a_rule_that_was_never_written), which does not exist'). +EOF +cat > "$work/cite-x.pl" <<'EOF' +doctrine(a_probe_citing_a_decision,'cites decision(improvise-nope, a_name_that_was_never_decided), which does not exist'). +EOF +fresh +markers=0 +for m in d i x; do + kb check --with "$work/cite-$m.pl" >/dev/null 2>&1 || markers=$((markers+1)) +done +if [ "$markers" -eq 3 ]; then + ok "all three citation markers are reachable, not just the first" +else + bad "only $markers of 3 citation markers fire" +fi + +# 17. The case aimed at has_key/2 itself. A keyed FACT with a variable in a +# key position -- work_done keys on (id, kind) -- is a key no existing +# clause is a variant of, so the add must land and every sibling survive. +# Under unification the fold excludes the clauses the key happens to match +# and the add is refused instead (by the total, which is the backstop, not +# the fix). Asserted as the OUTCOME, because that is what a wrong has_key +# destroys. +fresh +before="$(kb count | grep '^work_done/3=' | cut -d= -f2)" +printf "work_done(_,maintenance,'a probe clause with a variable id').\n" > "$work/wildfact.pl" +if kb add "$work/wildfact.pl" >"$work/out" 2>&1; then + after="$(kb count | grep '^work_done/3=' | cut -d= -f2)" + if [ "$((before + 1))" = "$after" ]; then + ok "a fact whose key holds a variable adds without excluding its siblings" + else + bad "adding one clause moved work_done/3 from $before to $after" + fi +else + bad "add refused a key holding a variable: the fold matched by unification" +fi + printf '\nkb-test: %d passed, %d failed\n' "$pass" "$fail" [ "$fail" -eq 0 ] diff --git a/scripts/node-test-test.sh b/scripts/node-test-test.sh index 4a64d94b..17ab03ed 100755 --- a/scripts/node-test-test.sh +++ b/scripts/node-test-test.sh @@ -101,11 +101,19 @@ run_case "an empty suite is refused" 1 "$work/suite/*.test.js" run_case "a describe holding a real test is not called barren" 0 \ "$work/shapes/*.test.js" -# A barren file hiding among real ones is the shape a refactor leaves: the run -# passes, the count is healthy, and one file asserts nothing. +# A degenerate file hiding among real ones is the shape a refactor leaves: the +# run passes, the count is healthy, and one file asserts nothing. node's TAP +# does not attribute a passing test to its file, so a whole-run count cannot +# see either of these -- which is why the guard runs one file at a time. run_case "a barren file among passing ones is still named" 1 \ "$work/tests/green.test.js" "$work/empty/nothing.test.js" +run_case "a skip-only file among passing ones is still named" 1 \ + "$work/tests/green.test.js" "$work/skipped/skipped.test.js" + +run_case "a todo-only file among passing ones is still named" 1 \ + "$work/tests/green.test.js" "$work/todo/todo.test.js" + # Two globs where the SECOND dies is the shape a directory move makes: the # first still matches, so a check over the union would pass. run_case "one dead pattern among several is refused" 1 \ @@ -114,9 +122,10 @@ run_case "one dead pattern among several is refused" 1 \ run_case "no argument at all is a usage error, not a pass" 2 # The literal the caller would pass if they forgot to quote AND the glob -# matched: ordinary filenames, which expand to themselves. +# matched: ordinary filenames, which expand to themselves -- several of them, +# since one is what the accepted case above already passes. run_case "already-expanded filenames still run" 0 \ - "$work/tests/green.test.js" + "$work/tests/green.test.js" "$work/shapes/shapes.test.js" printf '\nnode-test-test: %d passed, %d failed\n' "$pass" "$fail" [ "$fail" -eq 0 ] diff --git a/scripts/node-test.sh b/scripts/node-test.sh index 123605f7..4b310671 100755 --- a/scripts/node-test.sh +++ b/scripts/node-test.sh @@ -15,12 +15,13 @@ # mentioning one, by `test.skip`/`test.todo` and by an empty `test.describe` -- # every one of which executes no assertion (found by review on PR #69). # -# Two measurements make the report usable, both on node 24.20. `# pass` is 0 -# for a skip-only file, a todo-only file and an empty suite. And a file that -# registered NO test is reported as one passing test WHOSE NAME IS THE FILE -# PATH, which is why `# tests` alone reads as the file count and a -# minimum-count assertion over it is satisfied by the very file it should -# reject. That naming convention is an assumption about node, so +# Two measurements make the report usable, both on node 24.20, and both are +# taken PER FILE. `# pass` is 0 for a skip-only file, a todo-only file and an +# empty suite -- read over a whole run it would be masked by a passing +# neighbour. And a file that registered NO test is reported as one passing test +# WHOSE NAME IS THE FILE PATH, which is why `# tests` alone reads as the file +# count and a minimum-count assertion over it is satisfied by the very file it +# should reject. That naming convention is an assumption about node, so # scripts/node-test-test.sh pins it: if a future node stops doing it, the # self-test fails rather than the guard silently going quiet. set -uo pipefail @@ -49,27 +50,39 @@ done tap="$(mktemp)" trap 'rm -f "$tap"' EXIT -# Two reporters: the readable one to the terminal, TAP to a file, so the run -# can be read back without downgrading what a human sees. -node --test \ - --test-reporter=spec --test-reporter-destination=stdout \ - --test-reporter=tap --test-reporter-destination="$tap" \ - "${files[@]}" -status=$? -[ "$status" -eq 0 ] || exit "$status" - -passed="$(sed -n 's/^# pass \([0-9][0-9]*\)$/\1/p' "$tap" | tail -1)" -if [ -z "$passed" ]; then - echo "node-test: could not read a pass count out of node's TAP output" >&2 - exit 1 -fi -if [ "$passed" -eq 0 ]; then - echo "node-test: the run exited 0 with nothing passing (skipped, todo or an empty suite)" >&2 - exit 1 -fi - +# ONE FILE PER RUN. node's TAP does not attribute a passing test to the file it +# came from -- three files produce three flat `ok` lines named after the tests +# -- so a whole-run `# pass` count cannot tell a fully-skipped file from its +# passing neighbours, and a skip-only file rode in on a sibling (found by +# review on PR #69). node already forks a child per file, so this changes what +# is OBSERVABLE, not how the tests run. +status=0 +total=0 +silent=() barren=() for f in "${files[@]}"; do + node --test \ + --test-reporter=spec --test-reporter-destination=stdout \ + --test-reporter=tap --test-reporter-destination="$tap" \ + "$f" + ran=$? + if [ "$ran" -ne 0 ]; then + status="$ran" + continue + fi + passed="$(sed -n 's/^# pass \([0-9][0-9]*\)$/\1/p' "$tap" | tail -1)" + if [ -z "$passed" ]; then + echo "node-test: could not read a pass count out of node's TAP output for $f" >&2 + status=1 + continue + fi + total=$((total + passed)) + if [ "$passed" -eq 0 ]; then + silent+=("$f") + continue + fi + # A file that registered NO test is reported as one passing test whose name + # is the FILE PATH, which is why `# tests` reads as the file count. awk -v want="$f" ' /^(ok|not ok) [0-9]+ - / { desc = $0 @@ -80,10 +93,20 @@ for f in "${files[@]}"; do END { exit found ? 0 : 1 } ' "$tap" && barren+=("$f") done + +if [ "${#silent[@]}" -gt 0 ]; then + echo "node-test: these files passed nothing (skipped, todo or an empty suite):" >&2 + printf ' %s\n' "${silent[@]}" >&2 +fi if [ "${#barren[@]}" -gt 0 ]; then echo "node-test: these files registered no test, so running them asserted nothing:" >&2 printf ' %s\n' "${barren[@]}" >&2 +fi +if [ "$status" -ne 0 ]; then + exit "$status" +fi +if [ "${#silent[@]}" -gt 0 ] || [ "${#barren[@]}" -gt 0 ]; then exit 1 fi -echo "node-test: $passed test(s) passed over ${#files[@]} file(s)" +echo "node-test: $total test(s) passed over ${#files[@]} file(s)"