diff --git a/tests/run-tests.sh b/tests/run-tests.sh index 9aa3152..f6ea424 100755 --- a/tests/run-tests.sh +++ b/tests/run-tests.sh @@ -36,6 +36,22 @@ STRICT=false if [[ "${RUN_TESTS_STRICT:-}" == "1" ]]; then STRICT=true fi +# Latched, then REMOVED from the environment. The value has done its only job by +# this line -- it is now held in the STRICT shell local -- and leaving it exported +# makes strictness leak down the whole process tree: every test-*.sh dispatched +# through batch_run below inherits it, and any of them that itself invokes +# run-tests.sh (tests/test-run-tests.sh drives a copy of this script over fixture +# trees) silently turns a deliberately non-strict fixture strict. +# +# That is not symmetric with `--strict`, which never leaked: the flag only ever +# sets the shell local above, so `bash tests/run-tests.sh --strict` (the spelling +# the run-tests pre-push hook uses) always gave children a clean environment. Only +# the env-var spelling leaked, and it broke exactly two assertions in +# tests/test-run-tests.sh -- its cases 10c and 10g. Unsetting here makes the two +# documented invocations equivalent in what a CHILD sees, not just in the parent's +# verdict, so no future suite has to defend itself the way test-run-tests.sh's +# run_fake() does with `env -u`. +unset RUN_TESTS_STRICT # 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. diff --git a/tests/test-run-tests.sh b/tests/test-run-tests.sh index 39c2cf1..803424f 100644 --- a/tests/test-run-tests.sh +++ b/tests/test-run-tests.sh @@ -538,13 +538,24 @@ else fi # --- 10g. An ambient RUN_TESTS_STRICT=1 must not reach a fixture that did not ask -# for it. This suite is discovered and run by run-tests.sh itself, so under the -# gate the variable is exported into every child here. That is not a hypothetical: -# `bash tests/run-tests.sh` reported 19 passed while -# `RUN_TESTS_STRICT=1 bash tests/run-tests.sh` reported this file as the one -# failure, because case 10c's deliberately-non-strict run inherited strict and -# went red. The gate was therefore RED for everyone, and the only way to see it -# was to run the gate. +# for it. This suite is discovered and run by run-tests.sh itself, so when the +# outer run is launched with the env-var spelling the variable used to be exported +# into every child here. That was not a hypothetical: `bash tests/run-tests.sh` +# was green while `RUN_TESTS_STRICT=1 bash tests/run-tests.sh` reported this file +# as the one failure, because case 10c's deliberately-non-strict run inherited +# strict and went red. +# +# Two corrections to the record, because both were overstated before: +# * The blast radius was TWO assertions, not six -- cases 10c and 10g here, and +# nothing else in the repo reads RUN_TESTS_STRICT. +# * The pre-push GATE was never red. It runs `bash tests/run-tests.sh --strict` +# (see .pre-commit-config.yaml), and the flag sets a shell local that is never +# exported, so the flag spelling never leaked. Only the env-var spelling did. +# +# run-tests.sh now `unset`s the variable immediately after latching it, so the +# leak is closed at its source and the two spellings hand children an identical +# environment (case 10i pins that directly). The `env -u` in run_fake() is kept as +# this suite's own defence-in-depth rather than as the fix. # # The variable is exported here rather than passed as a prefix on purpose: a # prefix (`RUN_TESTS_STRICT=1 run_fake ...`) applies to the function call, and @@ -604,6 +615,60 @@ else pass "--strict reports skipped suites on stderr and suppresses the duplicate stdout list" fi +# --- 10i. The two documented invocations are equivalent in what a CHILD sees --- +# `bash tests/run-tests.sh --strict` and `RUN_TESTS_STRICT=1 bash +# tests/run-tests.sh` are documented as the same switch, and case 10b already +# asserts they produce the same PARENT verdict. That is the weaker half: the two +# differed in the ENVIRONMENT they handed every dispatched test-*.sh, because the +# flag sets a shell local while the env var stayed exported down the whole process +# tree. A dispatched suite could therefore behave differently depending on which +# spelling launched the run above it -- which is how case 10c went red under one +# invocation and green under the other. +# +# So this asks the children directly rather than reading the parent's summary. The +# case script reports whether RUN_TESTS_STRICT is present in its own environment +# AT ALL (`${VAR+set}`, not `${VAR:-}` -- an exported empty value is still a leak), +# and both spellings must report it absent. Equivalence is asserted between the two +# observations, not just against a hardcoded expectation, so the two cannot drift +# apart in some future direction neither case anticipated. +echo "" +echo "--- --strict and RUN_TESTS_STRICT=1 hand children the same environment ---" +DIR10I="$(make_fake_repo)" +FIXTURES+=("$DIR10I") +install_healthy_bats_runner "$DIR10I" +add_case "$DIR10I" test-reports-its-env.sh <<'EOF' +#!/usr/bin/env bash +if [[ -n "${RUN_TESTS_STRICT+set}" ]]; then + echo "CHILD-SAW-STRICT=[${RUN_TESTS_STRICT}]" +else + echo "CHILD-SAW-STRICT=" +fi +EOF +# Flag spelling: run_fake scrubs the ambient variable first, so what the child +# sees here is purely a function of what run-tests.sh itself exports. +run_fake "$DIR10I" --strict +FLAG_CHILD="$(echo "$FAKE_OUT" | grep -o 'CHILD-SAW-STRICT=.*' | head -n 1 || true)" +FLAG_RC=$FAKE_RC +# Env spelling: invoked directly, NOT through run_fake, because run_fake's `env -u` +# would strip the very variable under test. +I_PRIV="$(mktemp -d)" +FIXTURES+=("$I_PRIV") +ENV_RC=0 +ENV_OUT="$(TMPDIR="$I_PRIV" TEST_DIR="$DIR10I/cases" RUN_TESTS_STRICT=1 \ + bash "$DIR10I/tests/run-tests.sh" 2>&1)" || ENV_RC=$? +ENV_CHILD="$(echo "$ENV_OUT" | grep -o 'CHILD-SAW-STRICT=.*' | head -n 1 || true)" +if [[ -z "$FLAG_CHILD" || -z "$ENV_CHILD" ]]; then + fail "the reporting case script never ran under one of the two invocations (flag: '${FLAG_CHILD:-}', env: '${ENV_CHILD:-}')" +elif [[ "$FLAG_CHILD" != "$ENV_CHILD" ]]; then + fail "the two documented invocations hand children different environments — flag: $FLAG_CHILD, env: $ENV_CHILD" +elif [[ "$ENV_CHILD" != "CHILD-SAW-STRICT=" ]]; then + fail "RUN_TESTS_STRICT is still exported to dispatched suites ($ENV_CHILD) — a suite that itself runs run-tests.sh inherits strictness it never asked for" +elif [[ $FLAG_RC -ne 0 || $ENV_RC -ne 0 ]]; then + fail "a clean fixture failed under one of the two invocations (flag rc=$FLAG_RC, env rc=$ENV_RC): $FAKE_OUT / $ENV_OUT" +else + pass "--strict and RUN_TESTS_STRICT=1 both dispatch children with RUN_TESTS_STRICT absent" +fi + echo "" echo "Results: $PASS passed, $FAIL failed" [[ $FAIL -eq 0 ]]