test: fail the gate when a suite is skipped or never reports
Three gaps left by the previous round, all the same shape: a gate reporting success having verified less than it appears to. run_bats() hard-failed on a missing or non-executable runner but never checked that the runner produced anything. An empty, executable run-bats.sh exits 0, and the dispatcher printed a green summary with 166 bats tests silently absent. It now requires an "N tests, M failures" line with a non-zero count. run-tests.sh's skip listing is swallowed by pre-commit on a pass, so on a machine without vale three suites exited 77 and the pre-push gate went green having run 14 of 17. The hook now invokes it as --strict, where a skip fails and the error names each suite and the reason it skipped. An ad-hoc local run still skips gracefully -- at pre-push a skip means a documented dependency is missing, which is a setup error, not a legitimate state. Deliberately not wired to the vale downgrade's env var: one flag must not disarm two gates. BATS_FILE_FLOOR is replaced by an expectation derived from git ls-files. A floor of 8 against a real count of 10 let two files and eleven tests disappear green, and the number needed an edit whenever a plugin was added. The derived set needs no number, and catches an addition as well as a removal -- a .bats file staged into the index and deleted from disk is now demanded back. The vale opt-out announced its downgrade to nobody: pre-commit prints nothing for a passing hook, so the summary line AGENTS.md tells the reader to check was unreachable in exactly the situation it exists for. The hook is now verbose. Also corrects the PROBES_CHECKED guard, whose commit message described a state that cannot occur -- the .vale.ini loop errs first. Its two reachable triggers, a gutted probe heredoc and a probe row naming a missing directory, had no test; they do now, each asserting the guard is the sole cause. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT
This commit is contained in:
@@ -1,22 +1,63 @@
|
||||
#!/usr/bin/env bash
|
||||
# Run all test-*.sh files in the repo (including plugins) and the bats suite.
|
||||
# Usage: bash tests/run-tests.sh [--bats-only]
|
||||
# Usage: bash tests/run-tests.sh [--bats-only] [--strict]
|
||||
#
|
||||
# A script exiting 77 (the automake convention) is reported as SKIPPED, not
|
||||
# passed — a suite that can't run for lack of a binary must not read as green.
|
||||
#
|
||||
# --strict (or RUN_TESTS_STRICT=1) additionally makes any skip FAIL the run. Two
|
||||
# different readings of a skip are both correct, and which one applies depends on
|
||||
# who is running:
|
||||
#
|
||||
# * ad-hoc, on a laptop: skipping gracefully is the point. You are missing a
|
||||
# dev binary, the other 15 suites still tell you something, and turning that
|
||||
# into a red run would just train people to ignore red.
|
||||
# * as a GATE (the run-tests pre-push hook): a skip is a SETUP ERROR, not a
|
||||
# legitimate state. AGENTS.md documents vale, apm and jq as required pre-push
|
||||
# dependencies, so a suite that cannot run on the machine doing the pushing
|
||||
# means the machine is misconfigured -- and pre-commit prints NOTHING for a
|
||||
# passing hook, so the skip list below is swallowed entirely. On a vale-less
|
||||
# PATH that silently shipped a green gate having verified 15 of 17 suites.
|
||||
# Exactly the vacuous-pass class the rest of this file exists to close.
|
||||
#
|
||||
# Deliberately its own switch, NOT folded into
|
||||
# CHECK_VALE_STYLE_SYNC_ALLOW_MISSING_VALE. That one governs whether
|
||||
# check-vale-style-sync may downgrade itself; this one governs whether the test
|
||||
# dispatcher tolerates an unrunnable suite. They are separate decisions and one
|
||||
# flag disarming both gates is how an opt-out quietly grows blast radius.
|
||||
#
|
||||
# TEST_DIR — override root to search for test-*.sh (default: REPO_ROOT); used by tests.
|
||||
set -euo pipefail
|
||||
|
||||
REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
|
||||
BATS="$REPO_ROOT/tests/run-bats.sh"
|
||||
BATS_ONLY=false
|
||||
[[ "${1:-}" == "--bats-only" ]] && BATS_ONLY=true
|
||||
STRICT=false
|
||||
if [[ "${RUN_TESTS_STRICT:-}" == "1" ]]; then
|
||||
STRICT=true
|
||||
fi
|
||||
# A loop rather than the `[[ "${1:-}" == --bats-only ]]` test this used to be, so
|
||||
# the two flags compose and an unknown flag is rejected instead of ignored. A
|
||||
# silently-ignored `--strict` is the one typo that would turn the gate back off.
|
||||
for arg in ${@+"$@"}; do
|
||||
case "$arg" in
|
||||
--bats-only) BATS_ONLY=true ;;
|
||||
--strict) STRICT=true ;;
|
||||
*)
|
||||
echo "Usage: $0 [--bats-only] [--strict]" >&2
|
||||
exit 2
|
||||
;;
|
||||
esac
|
||||
done
|
||||
|
||||
SEARCH_ROOT="${TEST_DIR:-$REPO_ROOT}"
|
||||
|
||||
FAILED=()
|
||||
SKIPPED=()
|
||||
# Parallel array, index-matched to SKIPPED. Not an associative array: bash 3.2
|
||||
# (macOS) has none, and tests/test-vale-wrap.sh's bash-3.2 scan rejects
|
||||
# `declare -A` outright.
|
||||
SKIP_REASONS=()
|
||||
PASSED=0
|
||||
SKIP_EXIT=77
|
||||
|
||||
@@ -28,14 +69,43 @@ SKIP_EXIT=77
|
||||
# degraded to a no-op that printed nothing and exited 0. That is the same
|
||||
# green-either-way hole run-bats.sh's own zero-count guard closes one level down;
|
||||
# this closes it in the dispatcher that pre-push actually invokes.
|
||||
#
|
||||
# Present and executable is still not "it ran". `bash "$BATS"` on an EMPTY
|
||||
# run-bats.sh exits 0 having printed nothing, and the dispatcher printed
|
||||
# `=== bats ===`, a blank line, and a green summary -- the same green-either-way
|
||||
# defect one spelling over. Truncation, a partial write, an editor saving an
|
||||
# empty buffer, and a `set -e` abort in a future run-bats.sh preamble all land
|
||||
# there. So the runner's own summary line is required, and its count must be
|
||||
# non-zero: that line is run-bats.sh's contract with this script, and it is only
|
||||
# emitted after run-bats.sh's own zero-count guard has passed.
|
||||
#
|
||||
# Stdout is captured (the summary is on stdout) while stderr passes straight
|
||||
# through, so a failing runner's diagnostics still reach the terminal live. The
|
||||
# capture costs no streaming that was not already lost: run-bats.sh buffers its
|
||||
# per-file output and flushes it at the end regardless.
|
||||
run_bats() {
|
||||
if [[ ! -x "$BATS" ]]; then
|
||||
echo "Error: bats runner not found or not executable at $BATS — the bats suite cannot be skipped silently" >&2
|
||||
exit 1
|
||||
fi
|
||||
echo "=== bats ==="
|
||||
bash "$BATS"
|
||||
local out rc=0 summary count
|
||||
out="$(bash "$BATS")" || rc=$?
|
||||
[[ -z "$out" ]] || printf '%s\n' "$out"
|
||||
echo ""
|
||||
if [[ $rc -ne 0 ]]; then
|
||||
exit "$rc"
|
||||
fi
|
||||
summary="$(printf '%s\n' "$out" | grep -E '^[0-9]+ tests, [0-9]+ failures$' | tail -n 1 || true)"
|
||||
if [[ -z "$summary" ]]; then
|
||||
echo "Error: $BATS exited 0 without reporting an 'N tests, M failures' summary — it ran but produced nothing, so the bats suite was not verified" >&2
|
||||
exit 1
|
||||
fi
|
||||
count="${summary%% *}"
|
||||
if [[ "$count" -eq 0 ]]; then
|
||||
echo "Error: $BATS reported 0 tests — the bats suite executed nothing" >&2
|
||||
exit 1
|
||||
fi
|
||||
}
|
||||
|
||||
if $BATS_ONLY; then
|
||||
@@ -110,6 +180,28 @@ for script in ${SCRIPTS[@]+"${SCRIPTS[@]}"}; do
|
||||
PASSED=$((PASSED + 1))
|
||||
elif [[ "$rc" == "$SKIP_EXIT" ]]; then
|
||||
SKIPPED+=("$rel")
|
||||
# Capture WHY, not just that. The reason is printed by the suite itself and
|
||||
# is otherwise swallowed with the rest of its log, which leaves the reader
|
||||
# knowing something was skipped but not which binary to install. There is no
|
||||
# single house format for it -- three suites print `SKIP: <reason>` on stdout
|
||||
# and one prints `apm not installed -- skipping (...)` on stderr -- so this
|
||||
# tries the shapes in decreasing order of confidence and falls back to the
|
||||
# last thing the suite said before exiting 77, which for a guard that exits
|
||||
# immediately is the reason by construction. batch-run.sh folds stderr into
|
||||
# the same log, so the stderr spelling is reachable here.
|
||||
reason="$(grep -E '^[[:space:]]*SKIP' "$SCRATCH_ROOT/$idx.log" 2>/dev/null | head -n 1 || true)"
|
||||
if [[ -z "$reason" ]]; then
|
||||
reason="$(grep -iE 'skip' "$SCRATCH_ROOT/$idx.log" 2>/dev/null | head -n 1 || true)"
|
||||
fi
|
||||
if [[ -z "$reason" ]]; then
|
||||
reason="$(grep -vE '^[[:space:]]*$' "$SCRATCH_ROOT/$idx.log" 2>/dev/null | tail -n 1 || true)"
|
||||
fi
|
||||
if [[ -z "$reason" ]]; then
|
||||
reason="(exited $SKIP_EXIT without printing a reason)"
|
||||
fi
|
||||
# Trimmed of leading whitespace so the reasons line up under their suite
|
||||
# names regardless of how each suite indents its own message.
|
||||
SKIP_REASONS+=("${reason#"${reason%%[![:space:]]*}"}")
|
||||
else
|
||||
FAILED+=("$rel")
|
||||
fi
|
||||
@@ -117,16 +209,42 @@ for script in ${SCRIPTS[@]+"${SCRIPTS[@]}"}; do
|
||||
done
|
||||
|
||||
echo "=== Summary: $PASSED passed, ${#SKIPPED[@]} skipped, ${#FAILED[@]} failed ==="
|
||||
if [[ ${#SKIPPED[@]} -gt 0 ]]; then
|
||||
# Suppressed under --strict: the strict block below reports the same suites with
|
||||
# the same reasons, and printing both left the reader scrolling past one list to
|
||||
# reach an identical one. Under strict the failure block IS the list.
|
||||
if [[ ${#SKIPPED[@]} -gt 0 && "$STRICT" != true ]]; then
|
||||
echo "Skipped scripts:"
|
||||
sidx=0
|
||||
for s in ${SKIPPED[@]+"${SKIPPED[@]}"}; do
|
||||
echo " $s"
|
||||
echo " ${SKIP_REASONS[$sidx]}"
|
||||
sidx=$((sidx + 1))
|
||||
done
|
||||
fi
|
||||
|
||||
RC=0
|
||||
if [[ ${#FAILED[@]} -gt 0 ]]; then
|
||||
echo "Failed scripts:"
|
||||
for s in ${FAILED[@]+"${FAILED[@]}"}; do
|
||||
echo " $s"
|
||||
done
|
||||
exit 1
|
||||
RC=1
|
||||
fi
|
||||
|
||||
# Strict mode turns every skip into a failure. Reported separately from FAILED
|
||||
# above rather than folded into it: a skipped suite did not fail, the machine
|
||||
# did, and a message that says so points at the fix. Named with reasons again
|
||||
# here (not just referenced) because this block goes to stderr and is what a
|
||||
# pre-push reader actually gets handed.
|
||||
if [[ "$STRICT" == true && ${#SKIPPED[@]} -gt 0 ]]; then
|
||||
echo "Error: --strict and ${#SKIPPED[@]} suite(s) skipped. Run as a gate, a skip is a SETUP ERROR on this machine, not a legitimate state: AGENTS.md documents vale, apm and jq as required pre-push dependencies, so every suite is expected to be runnable here. Install what each suite names below and re-run; do not skip the hook." >&2
|
||||
sidx=0
|
||||
for s in ${SKIPPED[@]+"${SKIPPED[@]}"}; do
|
||||
echo " $s" >&2
|
||||
echo " ${SKIP_REASONS[$sidx]}" >&2
|
||||
sidx=$((sidx + 1))
|
||||
done
|
||||
RC=1
|
||||
fi
|
||||
|
||||
exit "$RC"
|
||||
|
||||
Reference in New Issue
Block a user