Files
improvise/scripts/node-test-test.sh
T
Edward Langley cd094b6dfb fix(kb): the dj1l cases discriminate, and a replaced rule cannot silently become an add
Five findings from the second review round, and the first is this PR's own
standard applied to its own P1.

**The two fixes improvise-dj1l names had no discriminating test.** Reverting
`has_key` to unification, or dropping `canon_key` from the duplicate gate, left
kb-test at 27 passed. Case 11 was carried by `check_total` refusing with a
DIFFERENT message, and case 10 by the other half of the fix. Both now assert
which refusal fired (`said()`), plus two cases aimed at each half: a rm key with
a variable in a non-final position must be refused as ABSENT rather than
matching everything, and a keyed FACT whose key holds a variable must ADD
without excluding the siblings it would unify with. Each proved by reverting
exactly one fix and watching exactly that case fail.

**Keying a rule on its whole clause made `replace` on an edited rule silently
become `add`** -- both arms live, the stale one still firing, and `check_total`
cannot see it because `NO + NF` is correct arithmetic for an add. Reachable on
the 15 `kb_error/1` arms and the `kb_text/2` rules, so editing the check
improvise-2f97 adds is the first thing to hit it. `check_rule_replace` refuses
and names the route (rm, then add); the negative arm -- replace of a rule
present verbatim -- is tested, or the refusal would just be "replace never
works on rules".

**The `decision(` arm was dead code.** `cited_name` took the first field, and
the repo spells `decision(<ticket>, <name>)`, whose ticket id fails
`name_shaped/1` on the hyphen -- so that marker could never fire for a real
citation. A second `cited_name` clause takes the second field. All three
markers are now exercised.

Minor, from the same round: `_JS_TEST` no longer accepts `test.only`, which
disables every other test in its file, with a case asserting a heading cannot
be satisfied by one; and `node-test-test.sh`'s last case no longer duplicates
an earlier one.

Filed rather than fixed: improvise-w8ih, extracting `web/assemble.sh`'s gate so
its own absence is visible.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
2026-09-20 17:01:33 -07:00

132 lines
5.1 KiB
Bash
Executable File

#!/usr/bin/env bash
# check_the_checker for scripts/node-test.sh. Every case is a way a node test
# step can exit 0 having asserted nothing -- which is what the step it replaces
# did, on both CI jobs, for the whole life of web/tests/ (improvise-vz54).
set -uo pipefail
here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
RUN="$here/node-test.sh"
work="$(mktemp -d)"
trap 'rm -rf "$work"' EXIT
pass=0; fail=0
ok() { printf ' ok %s\n' "$1"; pass=$((pass+1)); }
bad() { printf ' FAIL %s (exit %s, wanted %s)\n' "$1" "$2" "$3"; fail=$((fail+1)); }
# $1 name, $2 expected exit, rest: patterns
run_case() {
local name="$1" want="$2"; shift 2
"$RUN" "$@" >/dev/null 2>&1
local got=$?
if [ "$want" = "nonzero" ]; then
[ "$got" -ne 0 ] && ok "$name" || bad "$name" "$got" "$want"
else
[ "$got" = "$want" ] && ok "$name" || bad "$name" "$got" "$want"
fi
}
mkdir -p "$work/tests" "$work/empty"
cat > "$work/tests/green.test.js" <<'EOF'
const { test } = require("node:test");
test("a test that passes", () => {});
EOF
cat > "$work/tests/red.test.js" <<'EOF'
const { test } = require("node:test");
const assert = require("node:assert");
test("a test that fails", () => { assert.equal(1, 2); });
EOF
# A file that matches the glob and registers no test -- a helper left in the
# directory, or a suite emptied by a refactor. node reports it as ONE PASSING
# TEST NAMED AFTER THE FILE, so `# tests` reads as the file count and a
# minimum-count assertion over it is satisfied by the offender. This case also
# PINS that naming convention: if node stops doing it, this fails rather than
# the guard going quiet.
cat > "$work/empty/nothing.test.js" <<'EOF'
module.exports = { helper: () => 1 };
EOF
# The four shapes a lexical `grep` for `test(` cannot tell from a test, each
# of which executes no assertion (found by review on PR #69).
mkdir -p "$work/commented" "$work/skipped" "$work/todo" "$work/suite" "$work/shapes"
cat > "$work/commented/commented.test.js" <<'EOF'
const { test } = require("node:test");
// test("this one was removed", () => {});
const note = 'the word test( appears in this string';
EOF
cat > "$work/skipped/skipped.test.js" <<'EOF'
const { test } = require("node:test");
test.skip("skipped, so nothing ran", () => {});
EOF
cat > "$work/todo/todo.test.js" <<'EOF'
const { test } = require("node:test");
test.todo("not written yet");
EOF
cat > "$work/suite/suite.test.js" <<'EOF'
const { test } = require("node:test");
test.describe("a suite with nothing in it", () => {});
EOF
# The guard must not call a real suite barren: a describe holding a test is
# coverage, and its tests are nested rather than top-level.
cat > "$work/shapes/shapes.test.js" <<'EOF'
const { test } = require("node:test");
test.describe("a group", () => {
test("a nested test", () => {});
});
EOF
# The bug itself. Under bash the literal survives, node globs it, matches
# nothing, and the step goes green. Demonstrated at improvise-vz54.
run_case "a pattern that matches nothing is refused" 1 "$work/gone/*.test.js"
run_case "a matching file with a passing test is accepted" 0 "$work/tests/green.test.js"
# The refusal must not be bought by swallowing real failures.
run_case "a failing test still fails" nonzero "$work/tests/red.test.js"
run_case "a file that registers no test is refused" 1 "$work/empty/*.test.js"
run_case "a commented-out test, and the words test( in a string, are not a test" 1 \
"$work/commented/*.test.js"
run_case "a file whose only test is skipped is refused" 1 "$work/skipped/*.test.js"
run_case "a file whose only test is a todo is refused" 1 "$work/todo/*.test.js"
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 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 \
"$work/tests/green.test.js" "$work/gone/*.test.js"
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 -- 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/shapes/shapes.test.js"
printf '\nnode-test-test: %d passed, %d failed\n' "$pass" "$fail"
[ "$fail" -eq 0 ]