diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5896edab..e97e0ff8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -218,10 +218,17 @@ jobs: - uses: actions/setup-node@v4 with: node-version: 22 - # The glob is the SHELL's, not node's: `web/tests/bundle/` needs a built + # The glob is the SCRIPT's, not the shell's and not node's. Under bash an + # unmatched glob survives as a literal, node globs it, matches nothing and + # exits 0 having run no test -- so this step gated nothing the moment + # `web/tests/` moved (improvise-vz54). `web/tests/bundle/` needs a built # wasm bundle and runs in the browser job instead. - name: What the page decides between its boundaries - run: node --test web/tests/*.test.js + run: ./scripts/node-test.sh 'web/tests/*.test.js' + # The guard above is only worth its line if its refusals are themselves + # checked -- the step it replaces was green while asserting nothing. + - name: The node test runner refuses what it claims to + run: ./scripts/node-test-test.sh kb: name: Knowledge base consistency @@ -349,7 +356,7 @@ jobs: # runs here rather than in the `js` job, and before Playwright — a # renamed export is cheaper to read here than as a blank page. - name: The page's calls match the bundle it calls - run: node --test web/tests/bundle/*.test.js + run: ./scripts/node-test.sh 'web/tests/bundle/*.test.js' - name: Run the harness working-directory: harness run: npx playwright test diff --git a/scripts/node-test-test.sh b/scripts/node-test-test.sh new file mode 100755 index 00000000..c8e69c9f --- /dev/null +++ b/scripts/node-test-test.sh @@ -0,0 +1,86 @@ +#!/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 declares 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 its own count cannot see it and a +# minimum-count assertion over `# tests` is satisfied by the offender. +cat > "$work/empty/nothing.test.js" <<'EOF' +module.exports = { helper: () => 1 }; +EOF + +# The lexical check must not call a real suite barren. Both spellings the +# repo uses, and the indented form, live here. +mkdir -p "$work/shapes" +cat > "$work/shapes/shapes.test.js" <<'EOF' +const { test } = require("node:test"); +test.describe("a group", () => { + test("an indented 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 declares no test is refused" 1 "$work/empty/*.test.js" + +run_case "a suite using describe and an indented test is not called barren" 0 \ + "$work/shapes/*.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. +run_case "already-expanded filenames still run" 0 \ + "$work/tests/green.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 new file mode 100755 index 00000000..058dca88 --- /dev/null +++ b/scripts/node-test.sh @@ -0,0 +1,56 @@ +#!/usr/bin/env bash +# `node --test web/tests/*.test.js` is a gate that stops gating the moment the +# directory moves: bash leaves an unmatched glob as a literal, node 22+ treats +# the literal as a glob of its own, matches nothing, reports 0 tests and exits +# 0 (improvise-vz54). zsh aborts on an unmatched glob, so the failure is +# invisible from a developer's shell and visible only under CI's. +# +# So the glob is the SCRIPT's, not the shell's -- quote it at the call site. +# Two things are refused, and both are "the gate asserted nothing" wearing an +# exit 0: a pattern that matches no file, and a matched file that declares no +# test. +# +# The second is checked LEXICALLY, and that is not laziness. node's own count +# cannot see it: a file registering no test is reported as one passing test +# named after the file, so `# tests N` reads as the FILE count and a +# minimum-count assertion over it is satisfied by the very file it should +# reject. Measured on node 24.20. +# +# scripts/node-test-test.sh proves each refusal by injecting its fault. +set -uo pipefail + +usage() { + echo "usage: ${0##*/} '' [''...]" >&2 + echo " quote each glob: this script expands it, not the calling shell" >&2 + exit 2 +} + +[ "$#" -gt 0 ] || usage + +shopt -s nullglob + +files=() +for pattern in "$@"; do + matched=($pattern) + if [ "${#matched[@]}" -eq 0 ]; then + echo "node-test: no file matches '$pattern'" >&2 + echo "node-test: refusing to run, because node would report success here" >&2 + exit 1 + fi + files+=("${matched[@]}") +done + +barren=() +for f in "${files[@]}"; do + grep -Eq '(^|[^A-Za-z0-9_.])(test|it)(\.[A-Za-z]+)?[[:space:]]*\(' "$f" || barren+=("$f") +done +if [ "${#barren[@]}" -gt 0 ]; then + echo "node-test: these files declare no test, so running them asserts nothing:" >&2 + printf ' %s\n' "${barren[@]}" >&2 + exit 1 +fi + +node --test "${files[@]}" +status=$? +[ "$status" -eq 0 ] && echo "node-test: ${#files[@]} file(s) ran" +exit "$status"