mirror of
https://github.com/kuhyx/testsAndMisc.git
synced 2026-11-10 20:07:04 +01:00
Make the jailed lib/tests gate able to fail, without breaking measurement
The jail recorded each case's exit code but reported only timeouts, discarding every other non-zero status. A suite with a failing assertion exited 0 through the push gate -- the same blind spot that let an earlier session ship 2 failing assertions in the nc_php suite. Found by negative control, not by reading: a deliberately failing assertion still exited 0. Propagating that status unconditionally was wrong, and caught before landing: the jail's own usage example passes a `bogus` case that exits non-zero to cover an error path, so a clean 5/5 = 100.00% run started failing. That is the mode the 105-lib campaign runs in. So the behaviour sits behind --fail-on-case-error, default off, passed only by ci_mirror.sh and shell-tests.yml. Verified in all four quadrants: measurement + error case = 0, gate + error case = 1, gate + clean suite = 0, gate + broken assertion = 1. Both gates also pass --min 0: with no --measure the jail was measuring run_all.sh itself against its default --min 100 and failing the push at 54.55%, a coverage verdict neither gate asked for. ci_mirror.sh now prints the jail's output on failure, so a red gate can be diagnosed. The added flag pushed the jail over the 250-line cap, so the in-namespace case runner moved to lib/shell_coverage_case_runner.sh, following the existing lib/shell_coverage_jail_setup.sh split. All four quadrants were re-run after the extraction. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016sxxsDHWgdRhmdmzNMA6XM
This commit is contained in:
parent
08b58794e2
commit
170f843481
6
.github/workflows/shell-tests.yml
vendored
6
.github/workflows/shell-tests.yml
vendored
@ -83,8 +83,12 @@ jobs:
|
||||
failed=$((failed + 1))
|
||||
continue
|
||||
fi
|
||||
# --min 0: this step gates on the ASSERTIONS, not on coverage.
|
||||
# The jail's default --min 100 would otherwise measure the
|
||||
# runner itself and fail on a number this step never asked for;
|
||||
# the coverage bar belongs to check_shell_coverage.sh.
|
||||
cmd=(bash meta/scripts/shell_coverage_jail.sh \
|
||||
--subject "$r" "${jail_args[@]}" -- "")
|
||||
--subject "$r" "${jail_args[@]}" --min 0 --fail-on-case-error -- "")
|
||||
else
|
||||
cmd=("$r")
|
||||
fi
|
||||
|
||||
@ -0,0 +1,21 @@
|
||||
{
|
||||
"title": "Make the lib/tests gate run jail-requiring suites, and make it able to fail",
|
||||
"objective": "Unblock the push gate and CI for suites that must execute inside shell_coverage_jail.sh, and close the fail-open hole that let a suite with a failing assertion exit 0 through that gate -- without changing the jail's behaviour in the measurement mode the 105-lib shell-coverage campaign depends on.",
|
||||
"acceptance_criteria": [
|
||||
"A suite declares its jail requirement as data (a jail_args file beside run_all.sh); neither ci_mirror.sh nor shell-tests.yml names any suite path, so a future jail-requiring suite needs no edit to either runner.",
|
||||
"Both runners fail closed on a jail_args file that is present but empty: reported as a configuration error, never silently skipped.",
|
||||
"Gate mode fails on a failing assertion: the features/lib suite with a deliberately broken _t_eq injected before _t_report exits non-zero.",
|
||||
"Gate mode does not false-positive: the unmodified features/lib suite exits 0.",
|
||||
"Measurement mode is unchanged: a subject with an error-path case that exits non-zero still reports its coverage and exits 0, so --fail-on-case-error is off by default.",
|
||||
"The extracted jail_args flags reproduce the independently measured dot_resolver_install.sh 39/39 = 100.00%.",
|
||||
"The five lib/tests suites that do not require the jail still run bare and pass.",
|
||||
"Every touched shell file is within the 250-line cap and shellcheck-clean, with no suppressions added."
|
||||
],
|
||||
"out_of_scope": [
|
||||
"The REFUSING guards in the individual test files: they are what stop a test run from writing the developer's real /etc, and are deliberately untouched.",
|
||||
"Wiring check_shell_coverage.sh into any hook -- that arms only when the allowlist reaches zero.",
|
||||
"The kcov mis-measurement of rpi_nc_install.sh, which is tracked separately as an instrument defect.",
|
||||
"The unrelated python_pkg/token_audit/ files a concurrent session staged into the shared index."
|
||||
],
|
||||
"verifier": "Four quadrants via meta/scripts/shell_coverage_jail.sh: measurement+error-case = 0 with coverage printed; gate+error-case = 1; gate+clean features/lib suite = 0; gate+injected-failing-assertion = 1. Plus bash meta/scripts/check_file_length.sh --all showing no violation, shellcheck on the three shell files, and a successful git push through the ci-mirror pre-push hook."
|
||||
}
|
||||
@ -0,0 +1,70 @@
|
||||
{
|
||||
"intent": "Make the jailed lib/tests gate actually able to fail. The jail recorded each case's exit code but reported only timeouts (124/137), discarding every other non-zero status, so a test suite with a failing assertion exited 0 through the push gate. A gate that cannot fail reports safe while checking nothing.",
|
||||
"scope": [
|
||||
"meta/scripts/shell_coverage_jail.sh: new --fail-on-case-error flag (default OFF); record non-zero case exits and fail only when the flag is set",
|
||||
"meta/scripts/ci_mirror.sh: pass --min 0 to the suite-running invocation",
|
||||
".github/workflows/shell-tests.yml: same --min 0",
|
||||
"Non-goal: the coverage bar stays check_shell_coverage.sh's job, per-lib and --measure'd; this gate asks only 'did the assertions pass?'",
|
||||
"meta/scripts/lib/shell_coverage_case_runner.sh: new; the in-namespace case runner extracted to hold shell_coverage_jail.sh under the 250-line cap"
|
||||
],
|
||||
"changes": [
|
||||
"Found by negative control, not by reading: a deliberately failing assertion injected into test_dot_resolver_install.sh still exited 0. This is the same blind spot that let an earlier session ship 2 failing assertions in the nc_php suite.",
|
||||
"The jail was built as a MEASUREMENT instrument, where a subject exiting non-zero is normal (an installer given a bogus arg still yields coverage). Using it as a GATE requires propagating that status, so the non-zero exit is now recorded rather than swallowed.",
|
||||
"Coverage is still reported before the failure check: the number is wanted even when a case failed, and suppressing it would hide the diagnosis behind the symptom.",
|
||||
"--min 0 on both gate invocations: with no --measure, the jail measured run_all.sh itself against its default --min 100 and failed the push at 54.55% -- a number neither gate asked for.",
|
||||
"Caught in review before landing: propagating the status UNCONDITIONALLY broke measurement mode. The jail's own usage example passes a `bogus` case that exits non-zero to cover an error path; measured on a minimal subject, a clean 5/5 = 100.00% run started exiting 1. The behaviour is now behind --fail-on-case-error, passed only by the two gates. Measurement mode -- what the 105-lib campaign runs -- is unchanged.",
|
||||
"ci_mirror.sh captures the jail's output and prints it on the failure path only. Discarding it left a red gate reporting 'suite failed: <runner>' with no warn: or FAIL: lines to diagnose from.",
|
||||
"The added flag pushed shell_coverage_jail.sh to 285 lines and the file-length-cap hook rejected the commit. The in-namespace case runner (a self-contained heredoc block) was extracted to lib/shell_coverage_case_runner.sh, following the existing lib/shell_coverage_jail_setup.sh split. Jail 240 lines, new lib 64. All four verification quadrants were re-run after the extraction to confirm the refactor changed no behaviour."
|
||||
],
|
||||
"verification": [
|
||||
{
|
||||
"command": "shell_coverage_jail.sh --subject errsubj.sh --min 100 -- \"\" bogus (measurement mode, error-path case)",
|
||||
"result": "pass",
|
||||
"evidence": "exit=0, errsubj.sh: 5/5 lines = 100.00%. Confirms the regression is gone: before the flag this exited 1 on a clean 100%."
|
||||
},
|
||||
{
|
||||
"command": "shell_coverage_jail.sh --subject errsubj.sh --min 100 --fail-on-case-error -- \"\" bogus (gate mode)",
|
||||
"result": "pass",
|
||||
"evidence": "exit=1, 'Error: 1 case(s) exited non-zero'. Same subject, opposite verdict -- the flag is what separates the modes."
|
||||
},
|
||||
{
|
||||
"command": "jailed features/lib run_all.sh --min 0 --fail-on-case-error, unmodified suite",
|
||||
"result": "pass",
|
||||
"evidence": "exit=0. No false positive; a green suite passes the gate. This is also the first time the 'assertions pass' claim has been machine-checked -- the gate could not fail before."
|
||||
},
|
||||
{
|
||||
"command": "same, with `_t_eq \"a\" \"b\"` injected BEFORE _t_report, then restored",
|
||||
"result": "pass",
|
||||
"evidence": "exit=1. The negative control that proves the gate can fail. git diff --stat empty afterwards, confirming the injection was fully reverted."
|
||||
},
|
||||
{
|
||||
"command": "shellcheck meta/scripts/shell_coverage_jail.sh meta/scripts/ci_mirror.sh",
|
||||
"result": "pass",
|
||||
"evidence": "no findings; no suppressions added."
|
||||
},
|
||||
{
|
||||
"command": "git diff --cached --name-only",
|
||||
"result": "pass",
|
||||
"evidence": "only my files. A concurrent session had staged 8 python_pkg/token_audit/ files into the shared index; unstaged with `git restore --staged`, left intact on disk."
|
||||
},
|
||||
{
|
||||
"command": "bash meta/scripts/check_file_length.sh --all",
|
||||
"result": "pass",
|
||||
"evidence": "all checked files are within 250 lines (was: 285 for shell_coverage_jail.sh, +35 over)."
|
||||
},
|
||||
{
|
||||
"command": "all four quadrants re-run AFTER the lib extraction",
|
||||
"result": "pass",
|
||||
"evidence": "measurement+error=0, gate+error=1, gate+clean=0, gate+broken=1 -- identical to pre-refactor. shellcheck clean on all three shell files."
|
||||
}
|
||||
],
|
||||
"risks": [
|
||||
"The two modes now differ by a flag. A future gate that forgets --fail-on-case-error is silently fail-open again -- the exact defect this commit fixes. The negative control in this evidence is the check that catches it; re-run it if either gate is edited.",
|
||||
"CI needs kcov on ubuntu-latest (apt-installable, added to the existing install step); an apt outage turns shell-tests red for an infrastructure reason.",
|
||||
"jail_args is positional shell words, one per line; a flag and its value on one line would be passed as a single argument and rejected by the jail."
|
||||
],
|
||||
"rollback": [
|
||||
"git revert the commit; the jail returns to swallowing non-zero case exits and both gates return to their default --min 100.",
|
||||
"After rollback, re-run the negative control: it should exit 0 again, confirming the fail-open behaviour is back and the revert was complete."
|
||||
]
|
||||
}
|
||||
@ -146,9 +146,20 @@ run_jailed_suite() {
|
||||
fail "jail_args is present but empty: $args_file"
|
||||
fi
|
||||
|
||||
(cd "$CHECKOUT" && bash meta/scripts/shell_coverage_jail.sh \
|
||||
--subject "$runner" "${jail_args[@]}" -- "" >/dev/null) ||
|
||||
# --min 0: this gate asks "did the assertions pass?", not "is the lib at
|
||||
# 100%?". Without it the jail measures the runner itself against its
|
||||
# default --min 100 and fails the push on a number no one asked for.
|
||||
# The coverage bar is check_shell_coverage.sh's job, per-lib and --measure'd.
|
||||
# Output is captured, not discarded: on success it is noise, but on failure
|
||||
# it holds the `warn: case exited` line and the suite's own FAIL: lines.
|
||||
# Without this the gate reports only "suite failed" and cannot be diagnosed.
|
||||
local out
|
||||
if ! out="$(cd "$CHECKOUT" && bash meta/scripts/shell_coverage_jail.sh \
|
||||
--subject "$runner" "${jail_args[@]}" \
|
||||
--min 0 --fail-on-case-error -- "" 2>&1)"; then
|
||||
printf '%s\n' "$out" >&2
|
||||
fail "jailed lib/tests suite failed: $runner"
|
||||
fi
|
||||
}
|
||||
|
||||
run_shell_test_gate() {
|
||||
|
||||
64
meta/scripts/lib/shell_coverage_case_runner.sh
Executable file
64
meta/scripts/lib/shell_coverage_case_runner.sh
Executable file
@ -0,0 +1,64 @@
|
||||
#!/usr/bin/env bash
|
||||
|
||||
# ============================================================================
|
||||
# write_case_runner — emit the script that executes the cases INSIDE the jail.
|
||||
#
|
||||
# Split out of shell_coverage_jail.sh to hold that file under the 250-line cap
|
||||
# after --fail-on-case-error was added. Sourced, never run: it needs $JAIL from
|
||||
# its caller.
|
||||
#
|
||||
# The runner is written to a file rather than inlined into the `unshare`
|
||||
# command because it executes in the child namespace: a quoted inline block
|
||||
# would have to defer every expansion to the child shell, which reads as a
|
||||
# quoting bug and trips SC2016.
|
||||
# ============================================================================
|
||||
|
||||
write_case_runner() {
|
||||
# The runner below executes INSIDE the namespace, so it is written out as
|
||||
# its own file rather than inlined: a quoted inline block would have to
|
||||
# defer every expansion to the child shell, which reads as a quoting bug
|
||||
# and trips SC2016.
|
||||
cat >"$JAIL/run_cases.sh" <<'INNER'
|
||||
#!/usr/bin/env bash
|
||||
set -u
|
||||
jail="$1"
|
||||
subject_dir="$2"
|
||||
subject_base="$3"
|
||||
measure_base="$4"
|
||||
case_timeout="$5"
|
||||
. "$jail/mounts.sh"
|
||||
export PATH="$jail/bin:/usr/bin:/bin"
|
||||
export HOME="$jail/home"
|
||||
USER="${USER:-root}"
|
||||
export USER
|
||||
export SUDO_USER="$USER"
|
||||
cd "$subject_dir" || exit 1
|
||||
while IFS= read -r line; do
|
||||
read -r -a case_args <<<"$line"
|
||||
# Bounded, and with stdin closed. Both matter: a subject that reads stdin
|
||||
# blocks forever on the caller's terminal (a `crontab -` fallback did
|
||||
# exactly this and hung a run for the full two minutes with no output),
|
||||
# and a subject that spins never yields at all. A timed-out case is
|
||||
# reported rather than swallowed, because a silent timeout looks
|
||||
# identical to an unreachable line in the coverage report.
|
||||
timeout --kill-after=10s "$case_timeout" \
|
||||
kcov --include-pattern="$measure_base" "$jail/cov" \
|
||||
"./$subject_base" ${case_args[@]+"${case_args[@]}"} \
|
||||
>/dev/null 2>&1 </dev/null
|
||||
rc=$?
|
||||
if [[ $rc -eq 124 || $rc -eq 137 ]]; then
|
||||
printf 'warn: case timed out after %s: %s\n' "$case_timeout" "$line" >&2
|
||||
printf '%s\n' "$rc" >>"$jail/case_failures"
|
||||
elif [[ $rc -ne 0 ]]; then
|
||||
# A non-zero case is only noise when measuring (an installer that
|
||||
# exits 1 on a bogus arg still yields coverage), but it is the whole
|
||||
# signal when the subject is a TEST SUITE. Record it and let the
|
||||
# caller decide; swallowing it made a deliberately-broken assertion
|
||||
# exit 0 through the push gate.
|
||||
printf 'warn: case exited %d: %s\n' "$rc" "$line" >&2
|
||||
printf '%s\n' "$rc" >>"$jail/case_failures"
|
||||
fi
|
||||
done <"$jail/cases"
|
||||
INNER
|
||||
chmod +x "$JAIL/run_cases.sh"
|
||||
}
|
||||
@ -46,6 +46,11 @@ MIN_PERCENT=100
|
||||
# Per-case wall-clock ceiling. A hung case cannot be distinguished from a slow
|
||||
# one without a bound, and an unbounded run blocks a hook indefinitely.
|
||||
CASE_TIMEOUT="120s"
|
||||
# Off by default: when MEASURING, a case that exits non-zero is normal and
|
||||
# wanted -- the usage example's `bogus` case exists precisely to cover an
|
||||
# error path. Only a GATE, whose subject is a test suite, wants that status
|
||||
# to fail the run. See --fail-on-case-error.
|
||||
FAIL_ON_CASE_ERROR=0
|
||||
BIND_PATHS=()
|
||||
EXTRA_SHIMS=()
|
||||
CASES=()
|
||||
@ -69,6 +74,12 @@ Usage: $SCRIPT_NAME --subject <file> [options] -- <case> [<case>...]
|
||||
--seed-file <path> create an empty file at <path> inside the jail
|
||||
--timeout <dur> per-case wall-clock ceiling (default 120s)
|
||||
--min <percent> fail below this (default 100)
|
||||
--fail-on-case-error
|
||||
exit non-zero if any case exits non-zero. For GATE use,
|
||||
where the subject is a test suite and a failing
|
||||
assertion must fail the run. Off by default: when
|
||||
measuring, an error-path case exiting non-zero is the
|
||||
point of running it.
|
||||
-- <case>... one invocation each; "" means "no arguments"
|
||||
|
||||
Example:
|
||||
@ -113,6 +124,10 @@ while [[ $# -gt 0 ]]; do
|
||||
MIN_PERCENT="$2"
|
||||
shift 2
|
||||
;;
|
||||
--fail-on-case-error)
|
||||
FAIL_ON_CASE_ERROR=1
|
||||
shift
|
||||
;;
|
||||
-h | --help) usage ;;
|
||||
--)
|
||||
shift
|
||||
@ -161,6 +176,10 @@ trap cleanup EXIT
|
||||
# shellcheck source=./lib/shell_coverage_jail_setup.sh
|
||||
. "$(dirname "${BASH_SOURCE[0]}")/lib/shell_coverage_jail_setup.sh"
|
||||
|
||||
# The in-namespace case runner, likewise split for the 250-line cap.
|
||||
# shellcheck source=./lib/shell_coverage_case_runner.sh
|
||||
. "$(dirname "${BASH_SOURCE[0]}")/lib/shell_coverage_case_runner.sh"
|
||||
|
||||
main() {
|
||||
validate_requirements
|
||||
build_jail
|
||||
@ -180,44 +199,7 @@ main() {
|
||||
local tree_before
|
||||
tree_before="$(cd "$REPO_ROOT" && git status --porcelain)"
|
||||
|
||||
# The runner below executes INSIDE the namespace, so it is written out as
|
||||
# its own file rather than inlined: a quoted inline block would have to
|
||||
# defer every expansion to the child shell, which reads as a quoting bug
|
||||
# and trips SC2016.
|
||||
cat >"$JAIL/run_cases.sh" <<'INNER'
|
||||
#!/usr/bin/env bash
|
||||
set -u
|
||||
jail="$1"
|
||||
subject_dir="$2"
|
||||
subject_base="$3"
|
||||
measure_base="$4"
|
||||
case_timeout="$5"
|
||||
. "$jail/mounts.sh"
|
||||
export PATH="$jail/bin:/usr/bin:/bin"
|
||||
export HOME="$jail/home"
|
||||
USER="${USER:-root}"
|
||||
export USER
|
||||
export SUDO_USER="$USER"
|
||||
cd "$subject_dir" || exit 1
|
||||
while IFS= read -r line; do
|
||||
read -r -a case_args <<<"$line"
|
||||
# Bounded, and with stdin closed. Both matter: a subject that reads stdin
|
||||
# blocks forever on the caller's terminal (a `crontab -` fallback did
|
||||
# exactly this and hung a run for the full two minutes with no output),
|
||||
# and a subject that spins never yields at all. A timed-out case is
|
||||
# reported rather than swallowed, because a silent timeout looks
|
||||
# identical to an unreachable line in the coverage report.
|
||||
timeout --kill-after=10s "$case_timeout" \
|
||||
kcov --include-pattern="$measure_base" "$jail/cov" \
|
||||
"./$subject_base" ${case_args[@]+"${case_args[@]}"} \
|
||||
>/dev/null 2>&1 </dev/null
|
||||
rc=$?
|
||||
if [[ $rc -eq 124 || $rc -eq 137 ]]; then
|
||||
printf 'warn: case timed out after %s: %s\n' "$case_timeout" "$line" >&2
|
||||
fi
|
||||
done <"$jail/cases"
|
||||
INNER
|
||||
chmod +x "$JAIL/run_cases.sh"
|
||||
write_case_runner
|
||||
|
||||
# Seeds are created after the mounts, from inside the namespace: a path
|
||||
# under a bound target does not exist until that target is mounted.
|
||||
@ -243,8 +225,19 @@ INNER
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Report coverage first -- the number is wanted even when a case failed,
|
||||
# and suppressing it would hide the diagnosis behind the symptom.
|
||||
local report_rc=0
|
||||
python3 "$REPO_ROOT/meta/scripts/shell_coverage_report.py" \
|
||||
"$JAIL/cov" "$measure_base" "$MIN_PERCENT"
|
||||
"$JAIL/cov" "$measure_base" "$MIN_PERCENT" || report_rc=$?
|
||||
|
||||
if [[ $FAIL_ON_CASE_ERROR -eq 1 && -s "$JAIL/case_failures" ]]; then
|
||||
printf 'Error: %d case(s) exited non-zero; see the warn: lines above\n' \
|
||||
"$(wc -l <"$JAIL/case_failures")" >&2
|
||||
return 1
|
||||
fi
|
||||
|
||||
return "$report_rc"
|
||||
}
|
||||
|
||||
main "$@"
|
||||
|
||||
Loading…
Reference in New Issue
Block a user