aboutsummaryrefslogtreecommitdiff
path: root/scripts/testing/cleanup-tests.sh
diff options
context:
space:
mode:
authorCraig Jennings <c@cjennings.net>2026-05-17 14:38:40 -0500
committerCraig Jennings <c@cjennings.net>2026-05-17 14:38:40 -0500
commit663cec6520a72680609c0d803494fb0bde4ce765 (patch)
tree17ecf5a8493f561e762826db82e93928ca5d5049 /scripts/testing/cleanup-tests.sh
parent79f027d4fa8a0e5abb32c6cf8c21586b73c77a39 (diff)
downloadarchsetup-663cec6520a72680609c0d803494fb0bde4ce765.tar.gz
archsetup-663cec6520a72680609c0d803494fb0bde4ce765.zip
fix(testing): cleanup traps, arg validation, and two real bugs
Two real bugs and a sweep of hygiene across the harness. `make test` passed cleanly on this branch with the same 52/0/5 profile as the 2026-05-11 run, so the wiring is verified end-to-end. Real bugs: - `lib/vm-utils.sh` `snapshot_exists` was running `qemu-img snapshot -l | grep -q "$snapshot_name"`, which matches the name as a substring anywhere in the output — including inside dates or filenames in other fields. Replaced with an awk field extraction on the TAG column plus `grep -Fxq` for a whole-line literal match. - `run-test-baremetal.sh` was setting `VALIDATION_PASSED=true|false` after validation, but `validation.sh` already uses `VALIDATION_PASSED` as a pass counter. The test report then referenced `$VALIDATION_PASSED_COUNT`, which is defined nowhere. Renamed the boolean to `TEST_PASSED` (matching run-test.sh's pattern) and report the actual counter. Cleanup traps and arg validation: - `run-test.sh` now installs a top-level EXIT trap that, on abort, kills QEMU and restores the clean-install snapshot. A `CLEANUP_DONE=1` sentinel keeps the existing normal-path cleanup from double-firing. This is the recurring pain from 2026-05-11 where two failed runs left orphaned QEMU processes and dirty base disks behind. - `create-base-vm.sh` and `debug-vm.sh` got the same kind of trap, plus `debug-vm.sh` now rejects non-`.qcow2` paths up front instead of letting QEMU fail later. - `run-test.sh`, `run-test-baremetal.sh`, and `cleanup-tests.sh` now validate that options with required values actually receive one (`${var:?msg}` for `--script`/`--snapshot`/`--host`/`--password`, numeric check for `--keep`). - `run-test-baremetal.sh` traps the temp git bundle for cleanup if the script aborts before its explicit `rm`. The ZFS rollback loop now uses `while IFS= read -r ds` and quotes `$ds` inside the ssh_cmd so dataset names with whitespace wouldn't break it. Smaller hygiene: - `vm-utils.sh` `check_ovmf` also checks `OVMF_VARS_TEMPLATE`; `start_qemu` validates disk and ISO paths before building the QEMU command; numeric tests quoted. - `cleanup-tests.sh` find expression for temp disks wrapped in `\( ... -o ... \)`, all `while read` loops use `IFS= read -r`, orphaned QEMU cleanup tries SIGTERM with a 2s sleep before SIGKILL. - `create-base-vm.sh` moved the "Copy an archangel-*.iso" info line before its `fatal` instead of after (unreachable), and added the serial-log path to the final summary. - `lib/logging.sh` `stop_timer` no longer produces `$((end - ))` when the named timer was never started. - `lib/network-diagnostics.sh` `read` → `IFS= read -r`. - `setup-testing-env.sh` now installs all missing pacman packages in one transaction instead of one-at-a-time (avoids half-installed state if package N fails). KVM check also verifies the user has read+write on `/dev/kvm` and prints the `gpasswd -a $(id -un) kvm` fix if not. A few items from the review I deliberately skipped: replacing the codebase-wide unquoted `$SSH_OPTS` string with an array (cosmetic, would need to be done everywhere at once), `set -e` adds where the existing fall-through-on-failure is intentional, and a `--force` gate on `create-base-vm.sh` (would break the expected workflow).
Diffstat (limited to 'scripts/testing/cleanup-tests.sh')
-rwxr-xr-xscripts/testing/cleanup-tests.sh24
1 files changed, 17 insertions, 7 deletions
diff --git a/scripts/testing/cleanup-tests.sh b/scripts/testing/cleanup-tests.sh
index fd2f8de..5c0153b 100755
--- a/scripts/testing/cleanup-tests.sh
+++ b/scripts/testing/cleanup-tests.sh
@@ -20,6 +20,12 @@ FORCE=false
while [[ $# -gt 0 ]]; do
case $1 in
--keep)
+ case "${2:-}" in
+ ''|*[!0-9]*)
+ echo "Error: --keep requires a non-negative integer (got: '${2:-}')" >&2
+ exit 1
+ ;;
+ esac
KEEP_LAST="$2"
shift 2
;;
@@ -68,13 +74,17 @@ QEMU_PIDS=$(pgrep -f "qemu-system.*archsetup-test" 2>/dev/null || true)
if [ -n "$QEMU_PIDS" ]; then
info "Found orphaned QEMU processes: $QEMU_PIDS"
if $FORCE; then
- echo "$QEMU_PIDS" | xargs kill -9 2>/dev/null || true
+ echo "$QEMU_PIDS" | xargs -r kill 2>/dev/null || true
+ sleep 2
+ echo "$QEMU_PIDS" | xargs -r kill -9 2>/dev/null || true
success "Orphaned processes killed"
else
read -p "Kill orphaned QEMU processes? [y/N] " -n 1 -r
echo ""
if [[ $REPLY =~ ^[Yy]$ ]]; then
- echo "$QEMU_PIDS" | xargs kill -9 2>/dev/null || true
+ echo "$QEMU_PIDS" | xargs -r kill 2>/dev/null || true
+ sleep 2
+ echo "$QEMU_PIDS" | xargs -r kill -9 2>/dev/null || true
success "Orphaned processes killed"
fi
fi
@@ -88,7 +98,7 @@ section "Cleaning Up Disk Images"
step "Finding temporary disk images"
if [ -d "$PROJECT_ROOT/vm-images" ]; then
- TEMP_DISKS=$(find "$PROJECT_ROOT/vm-images" -name "debug-overlay-*.qcow2" -o -name "archsetup-test-*.qcow2" 2>/dev/null || true)
+ TEMP_DISKS=$(find "$PROJECT_ROOT/vm-images" \( -name "debug-overlay-*.qcow2" -o -name "archsetup-test-*.qcow2" \) 2>/dev/null || true)
if [ -z "$TEMP_DISKS" ]; then
info "No temporary disk images found"
@@ -98,7 +108,7 @@ if [ -d "$PROJECT_ROOT/vm-images" ]; then
info "Found $DISK_COUNT temporary disk image(s) totaling $DISK_SIZE"
if $FORCE; then
- echo "$TEMP_DISKS" | while read disk; do
+ echo "$TEMP_DISKS" | while IFS= read -r disk; do
rm -f "$disk"
done
success "Temporary disk images deleted"
@@ -109,7 +119,7 @@ if [ -d "$PROJECT_ROOT/vm-images" ]; then
read -p "Delete these disk images? [y/N] " -n 1 -r
echo ""
if [[ $REPLY =~ ^[Yy]$ ]]; then
- echo "$TEMP_DISKS" | while read disk; do
+ echo "$TEMP_DISKS" | while IFS= read -r disk; do
rm -f "$disk"
done
success "Temporary disk images deleted"
@@ -143,7 +153,7 @@ else
info "Keeping last $KEEP_LAST, deleting $DELETE_COUNT old result(s)"
if $FORCE; then
- echo "$TO_DELETE" | while read dir; do
+ echo "$TO_DELETE" | while IFS= read -r dir; do
rm -rf "$dir"
done
success "Old test results deleted"
@@ -155,7 +165,7 @@ else
read -p "Delete these test results? [y/N] " -n 1 -r
echo ""
if [[ $REPLY =~ ^[Yy]$ ]]; then
- echo "$TO_DELETE" | while read dir; do
+ echo "$TO_DELETE" | while IFS= read -r dir; do
rm -rf "$dir"
done
success "Old test results deleted"