fix(tooling): a barren file is refused by node's own report, not by grepping for test(
Two findings from the PR #69 review, one class: a lexical `test(` scan is satisfied by things that execute no assertion -- a commented-out call, the words in a string, `test.skip`/`test.todo`, and an empty `test.describe`. scripts/node-test.sh drops the scan and reads node's report instead. `# 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 both how a barren file is detected and why `# tests` alone reads as the file count. That naming convention is an assumption about node, so the self-test pins it: a node that stops doing it fails the case rather than letting the guard go quiet. Six cases added, including a barren file hiding among passing ones. --check-evidence had the parallel hole: _JS_TEST accepted `.skip`, `.todo` and `.describe`, so a quoted heading could be satisfied by disabled coverage or by an empty suite. Now runnable leaves only (`.only` included), with three cases asserting that a skipped, todo or suite-only name leaves the heading orphaned while a test nested in a suite still satisfies one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
This commit is contained in:
@@ -254,8 +254,11 @@ 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).
|
||||
_JS_TEST = re.compile(
|
||||
r"""(?:^|[^\w.])(?:test|it)(?:\.(?:describe|skip|only|todo))?\s*\(\s*(['"`])(.+?)\1"""
|
||||
r"""(?:^|[^\w.])(?:test|it)(?:\.only)?\s*\(\s*(['"`])(.+?)\1"""
|
||||
)
|
||||
|
||||
|
||||
@@ -709,6 +712,21 @@ SELF_TEST_EVIDENCE = [
|
||||
"\n#[test]\nfn uncited() {}\n",
|
||||
"", [], []),
|
||||
|
||||
("a heading cannot be satisfied by a skipped or todo test",
|
||||
'#### "a skipped one"\n#### "a todo one"\n', "",
|
||||
'test.skip("a skipped one", () => {});\ntest.todo("a todo one");\n',
|
||||
["a skipped one", "a todo one"], []),
|
||||
|
||||
("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',
|
||||
["a suite with nothing in it"], []),
|
||||
|
||||
("a test nested in a suite still satisfies its heading",
|
||||
'#### "a nested test"\n', "",
|
||||
'test.describe("a group", () => {\n test("a nested test", () => {});\n});\n',
|
||||
[], []),
|
||||
|
||||
("a JS test that cites the evidence file and has no entry is caught",
|
||||
"", "",
|
||||
'// See context/test-evidence/clients.md.\ntest("a cited spec", async () => {});\n',
|
||||
|
||||
+46
-10
@@ -38,21 +38,43 @@ 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.
|
||||
# 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 lexical check must not call a real suite barren. Both spellings the
|
||||
# repo uses, and the indented form, live here.
|
||||
mkdir -p "$work/shapes"
|
||||
# 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("an indented test", () => {});
|
||||
test("a nested test", () => {});
|
||||
});
|
||||
EOF
|
||||
|
||||
@@ -65,11 +87,25 @@ run_case "a matching file with a passing test is accepted" 0 "$work/tests/green.
|
||||
# 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 file that registers no test is refused" 1 "$work/empty/*.test.js"
|
||||
|
||||
run_case "a suite using describe and an indented test is not called barren" 0 \
|
||||
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 barren file hiding among real ones is the shape a refactor leaves: the run
|
||||
# passes, the count is healthy, and one file asserts nothing.
|
||||
run_case "a barren file among passing ones is still named" 1 \
|
||||
"$work/tests/green.test.js" "$work/empty/nothing.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 \
|
||||
|
||||
+48
-15
@@ -6,17 +6,23 @@
|
||||
# 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.
|
||||
# Three things are refused, and all three are "the gate asserted nothing"
|
||||
# wearing an exit 0: a pattern that matches no file, a run in which nothing
|
||||
# passed, and a matched file that registered 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
|
||||
# The last two are read off node's OWN report rather than scanned for lexically.
|
||||
# A `grep` for `test(` is satisfied by a commented-out call, by a string
|
||||
# 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. Measured on node 24.20.
|
||||
#
|
||||
# scripts/node-test-test.sh proves each refusal by injecting its fault.
|
||||
# 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
|
||||
|
||||
usage() {
|
||||
@@ -40,17 +46,44 @@ for pattern in "$@"; do
|
||||
files+=("${matched[@]}")
|
||||
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
|
||||
|
||||
barren=()
|
||||
for f in "${files[@]}"; do
|
||||
grep -Eq '(^|[^A-Za-z0-9_.])(test|it)(\.[A-Za-z]+)?[[:space:]]*\(' "$f" || barren+=("$f")
|
||||
awk -v want="$f" '
|
||||
/^(ok|not ok) [0-9]+ - / {
|
||||
desc = $0
|
||||
sub(/^(ok|not ok) [0-9]+ - /, "", desc)
|
||||
sub(/ # (SKIP|TODO).*$/, "", desc)
|
||||
if (desc == want) { found = 1 }
|
||||
}
|
||||
END { exit found ? 0 : 1 }
|
||||
' "$tap" && barren+=("$f")
|
||||
done
|
||||
if [ "${#barren[@]}" -gt 0 ]; then
|
||||
echo "node-test: these files declare no test, so running them asserts nothing:" >&2
|
||||
echo "node-test: these files registered no test, so running them asserted 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"
|
||||
echo "node-test: $passed test(s) passed over ${#files[@]} file(s)"
|
||||
|
||||
Reference in New Issue
Block a user