fix(ci): a node test step with an unmatched glob is a gate that gates nothing (improvise-vz54)
Under bash an unmatched glob survives as a literal; node 22+ globs the literal itself, matches nothing, reports 0 tests and exits 0. Both CI node steps -- the `js` job and the browser job's bundle suite -- were exposed: rename or move either directory and the step goes green having asserted nothing. Reproduced under `bash -e`, which is the shell CI names; zsh ABORTS on an unmatched glob, so the failure is invisible from a developer's shell. The glob becomes the script's rather than the shell's. scripts/node-test.sh expands it, refuses an empty match, and also refuses a matched file that declares no test -- checked LEXICALLY, because node reports such a file as one passing test named after the file, so the `# tests N` a minimum-count assertion would read is the FILE count and is satisfied by the offender. Measured on node 24.20. scripts/node-test-test.sh proves each refusal by injecting its fault, including that the wrapper does not buy its refusals by swallowing a real failure, and runs in the `js` job: a check wired into nothing is a convention. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
This commit is contained in:
@@ -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
|
||||
|
||||
Executable
+86
@@ -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 ]
|
||||
Executable
+56
@@ -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##*/} '<glob>' ['<glob>'...]" >&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"
|
||||
Reference in New Issue
Block a user