fix(ci): close the RUN_TESTS_STRICT leak at its source, not at each caller
7607522 fixed the symptom in the wrong place. It made `test-run-tests.sh`'s
`run_fake()` spawn fixtures via `env -u RUN_TESTS_STRICT`, which stops that one
suite inheriting strictness — and leaves every future suite to defend itself the
same way. The variable's only job is done the moment `run-tests.sh` latches it
into the `STRICT` shell local, so it is unset there now and the leak is gone for
every child. The `env -u` stays as this suite's own defence in depth rather than
as the fix.
Two corrections to that commit's account of the bug, both overstated and both
cheap to have checked:
- The blast radius was two assertions, cases 10c and 10g, not six. Nothing else
in the repo reads `RUN_TESTS_STRICT`.
- The pre-push gate was never red. It invokes `bash tests/run-tests.sh --strict`,
and the flag sets a shell local that is never exported, so the flag spelling
never leaked at all. Only the env-var spelling did.
That asymmetry between the two documented spellings is the real finding, and
nothing asserted against it. Case 10b compared the parent's verdict, which is the
half that already matched; the halves that differed were the environments the two
spellings handed every dispatched suite. New case 10i asks a child directly —
`${VAR+set}`, so an exported empty value still counts as a leak — and asserts the
two observations equal each other rather than a hardcoded expectation, so they
cannot drift apart in a direction the case did not anticipate.This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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=<absent>"
|
||||
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:-<none>}', env: '${ENV_CHILD:-<none>}')"
|
||||
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=<absent>" ]]; 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 ]]
|
||||
|
||||
Reference in New Issue
Block a user