6c079f9d

test: hardkill must kill the child, not wait on a wrapper that cannot exit

a73x 2026-08-22 06:07

Commit message

test: hardkill must kill the child, not wait on a wrapper that cannot exit

Review of 96febc7 found the child kill missing. It went out when the
discriminator changed: `pkill -9 -P "$1"` had been doing double duty, both
testing for a wrapper and killing its child, and replacing it with a comm
probe kept the test and dropped the kill.

What was committed could not work. The wrapper only exits once its traced
child dies, nothing killed the child, so the wait could never succeed: it
burned its full second at every traced-daemon site and then SIGKILLed the
wrapper, which discards that daemon's coverage — the exact loss the wait was
added to prevent, at the three legs that tear a daemon down with SIGKILL.
The runs still limped past those legs, but by accident: kcov's leftover
breakpoints kill the detached daemon when it next executes an unhit line.

Also from the review:

- COV_WRAP's validation exited inside the command substitution feeding a
  for-loop's word list, where the failure does not propagate. A typo printed
  "coverage FAIL" and then ran the whole suite and exited 0. Validated in the
  shell that can actually exit, before anything is generated.
- The shim tested for the `start` verb by matching the word anywhere in
  argv; a session named "start" would have run its daemon untraced in
  silence. It tests $1.
- A comment claimed the report's totals were the only unquoted numbers in
  kcov's JSON. They are not, and the awk never depended on it.
- E2E_STOP_AFTER now validates where TIME_SCALE does, so a bad value names
  itself instead of dying in test(1) at the first passing scenario.

Untraced behaviour is unchanged and stays that way: soak is 10/10 green on
the abort legs, and the persistence phase is flat (fds 6->6).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

test/coverage.sh

Old		New
@@ -77,7 +77,9 @@ mkdir -p "$SHIM" "$RAW"
77	# auto-start and `--via` legs wedge on this, and a SIGTERM will not free it.	77	# auto-start and `--via` legs wedge on this, and a SIGTERM will not free it.
78	#	78	#
79	# Excluding the `start` verb (below) does not cover it: the daemon is spawned	79	# Excluding the `start` verb (below) does not cover it: the daemon is spawned
80	# by the client, not named on the shim's command line. --exit-first-process is	80	# by the client, so `start` never appears on the shim's command line at all.
		81	# The verb slot is tested exactly ($1), not matched anywhere in argv — a
		82	# session named "start" would otherwise run its daemon untraced in silence. --exit-first-process is
81	# kcov's answer and does fix this one, but it reparents the tracer, and then	83	# kcov's answer and does fix this one, but it reparents the tracer, and then
82	# the real process is no longer a child of the pid the suite holds — which	84	# the real process is no longer a child of the pid the suite holds — which
83	# breaks hardkill and moves the hang one leg later. Daemons carry server.zig,	85	# breaks hardkill and moves the hang one leg later. Daemons carry server.zig,
@@ -85,29 +87,36 @@ mkdir -p "$SHIM" "$RAW"
85	#	87	#
86	# Set COV_WRAP="mux" to measure the client over a slice that avoids those	88	# Set COV_WRAP="mux" to measure the client over a slice that avoids those
87	# legs (E2E_STOP_AFTER pairs with it), and merge the two runs by hand.	89	# legs (E2E_STOP_AFTER pairs with it), and merge the two runs by hand.
88	for spec in $(echo "${COV_WRAP:-muxd muxa muxweb}" \| tr ' ' '\n' \| while read -r w; do	90	WRAP="${COV_WRAP:-muxd muxa muxweb}"
		91	# Validated in THIS shell, before anything is generated. The obvious spelling
		92	# puts the check inside the loop's word list, where `exit 1` only leaves the
		93	# command substitution: the message prints, the suite runs anyway, and the
		94	# whole thing exits 0 — a FAIL that does not fail.
		95	for w in $WRAP; do
89	case "$w" in	96	case "$w" in
90	muxd) echo "muxd\|$MUXD" ;;	97	muxd\|mux\|muxa\|muxweb) ;;
91	mux) echo "mux\|$MUX" ;;	98	*) echo "coverage FAIL: COV_WRAP names unknown binary '$w'"
92	muxa) echo "muxa\|$MUXA" ;;	99	echo " known: muxd mux muxa muxweb"
93	muxweb) echo "muxweb\|$MUXWEB" ;;	100	exit 1 ;;
94	"") ;;	101	esac
95	*) echo "coverage FAIL: COV_WRAP names unknown binary '$w'" >&2; exit 1 ;;	102	done
		103
		104	for w in $WRAP; do
		105	case "$w" in
		106	muxd) real=$MUXD ;;
		107	mux) real=$MUX ;;
		108	muxa) real=$MUXA ;;
		109	muxweb) real=$MUXWEB ;;
96	esac	110	esac
97	done); do
98	name=${spec%%\|*}
99	real=${spec#*\|}
100	real=$(cd "$(dirname "$real")" && pwd)/$(basename "$real")	111	real=$(cd "$(dirname "$real")" && pwd)/$(basename "$real")
101	cat > "$SHIM/$name" <<SHIM_EOF	112	cat > "$SHIM/$w" <<SHIM_EOF
102	#!/bin/sh	113	#!/bin/sh
103	[ -n "\$MUX_KCOV_ACTIVE" ] && exec "$real" "\$@"	114	[ -n "\$MUX_KCOV_ACTIVE" ] && exec "$real" "\$@"
104	MUX_KCOV_ACTIVE=1; export MUX_KCOV_ACTIVE	115	MUX_KCOV_ACTIVE=1; export MUX_KCOV_ACTIVE
105	case " \$* " in	116	[ "\${1:-}" = start ] && exec "$real" "\$@"
106	" start ") exec "$real" "\$@" ;;	117	exec kcov --collect-only "--include-path=$SRC" "$RAW/$w.\$\$" "$real" "\$@"
107	esac
108	exec kcov --collect-only --include-path=$SRC "$RAW/$name.\$\$" "$real" "\$@"
109	SHIM_EOF	118	SHIM_EOF
110	chmod +x "$SHIM/$name"	119	chmod +x "$SHIM/$w"
111	done	120	done
112		121
113	# e2e.sh prepends `dirname "$MUXD"` to PATH so an auto-started daemon is the	122	# e2e.sh prepends `dirname "$MUXD"` to PATH so an auto-started daemon is the
@@ -160,8 +169,8 @@ awk -F'"' '/"file":/ {
160	n = split($4, p, "/")	169	n = split($4, p, "/")
161	printf "%-22s %7s%% %6d/%-6d %10d\n", p[n], $8, $12, $16, $16 - $12	170	printf "%-22s %7s%% %6d/%-6d %10d\n", p[n], $8, $12, $16, $16 - $12
162	}' "$JSON" \| sort -k4 -rn	171	}' "$JSON" \| sort -k4 -rn
163	# The totals live at the top level of the same file, past the files array,	172	# The totals live at the top level of the same file, past the files array;
164	# and are the only bare (unquoted) numbers in it.	173	# anchored on the key names, so quoting of the values does not matter.
165	awk -F'[:,]' '	174	awk -F'[:,]' '
166	/^ "percent_covered"/ { gsub(/[ "]/, "", $2); pct = $2 }	175	/^ "percent_covered"/ { gsub(/[ "]/, "", $2); pct = $2 }
167	/^ "covered_lines"/ { gsub(/[ "]/, "", $2); cov = $2 }	176	/^ "covered_lines"/ { gsub(/[ "]/, "", $2); cov = $2 }

test/e2e.sh

Old		New
@@ -524,6 +524,13 @@ case "$TIME_SCALE" in
524	esac	524	esac
525	[ "$TIME_SCALE" -ge 1 ] \|\| { echo "e2e FAIL: E2E_TIME_SCALE must be >= 1"; exit 1; }	525	[ "$TIME_SCALE" -ge 1 ] \|\| { echo "e2e FAIL: E2E_TIME_SCALE must be >= 1"; exit 1; }
526		526
		527	# Validated here rather than at its use in ok(), for the reason above: a bad
		528	# value would otherwise surface as a bare test(1) error at the first passing
		529	# scenario, with no e2e FAIL line to say what was wrong.
		530	case "${E2E_STOP_AFTER:-1}" in
		531	''\|[!0-9]) echo "e2e FAIL: E2E_STOP_AFTER must be a positive integer"; exit 1 ;;
		532	esac
		533
527	wait_for() {	534	wait_for() {
528	_file="$1"; _pat="$2"; _ticks=$(( ${3:-15} * 10 * TIME_SCALE )); _i=0	535	_file="$1"; _pat="$2"; _ticks=$(( ${3:-15} * 10 * TIME_SCALE )); _i=0
529	while [ "$_i" -lt "$_ticks" ]; do	536	while [ "$_i" -lt "$_ticks" ]; do
@@ -554,13 +561,17 @@ hardkill() {
554	# session exit takes a different path out of the client (measured: exit	561	# session exit takes a different path out of the client (measured: exit
555	# 128 where the leg wants 0). Only a wrapper gets the two-step treatment.	562	# 128 where the leg wants 0). Only a wrapper gets the two-step treatment.
556	if [ "$(ps -o comm= -p "$1" 2>/dev/null)" = kcov ]; then	563	if [ "$(ps -o comm= -p "$1" 2>/dev/null)" = kcov ]; then
557	# A wrapper writes its coverage when it sees its child die, so give it	564	# The CHILD is the daemon the suite means to kill; the pid it holds is
558	# that moment. Killing both at once loses the database entirely —	565	# only the tracer. Kill the child first and the wrapper exits on its
559	# measured: SIGKILLing kcov yields no coverage.db at any	566	# own, writing the coverage it has collected — measured: a SIGKILLed
560	# --output-interval, so the write only ever happens on a clean exit.	567	# wrapper writes no coverage.db at any --output-interval, so a clean
561	# Bounded at 1s, and it costs nothing when the wrapper exits at once.	568	# exit is the only exit that keeps the data. Then wait for it, because
		569	# killing both at once is the same as never killing the child at all.
		570	for _c in $(ps -o pid= --ppid "$1" 2>/dev/null); do
		571	kill -9 "$_c" 2>/dev/null \|\| true
		572	done
562	_i=0	573	_i=0
563	while kill -0 "$1" 2>/dev/null && [ "$_i" -lt 20 ]; do	574	while kill -0 "$1" 2>/dev/null && [ "$_i" -lt $(( 20 * TIME_SCALE )) ]; do
564	sleep 0.05; _i=$((_i + 1))	575	sleep 0.05; _i=$((_i + 1))
565	done	576	done
566	fi	577	fi