diff options
| author | Craig Jennings <c@cjennings.net> | 2026-05-17 14:38:40 -0500 |
|---|---|---|
| committer | Craig Jennings <c@cjennings.net> | 2026-05-17 14:38:40 -0500 |
| commit | 663cec6520a72680609c0d803494fb0bde4ce765 (patch) | |
| tree | 17ecf5a8493f561e762826db82e93928ca5d5049 /scripts/testing/setup-testing-env.sh | |
| parent | 79f027d4fa8a0e5abb32c6cf8c21586b73c77a39 (diff) | |
| download | archsetup-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/setup-testing-env.sh')
| -rwxr-xr-x | scripts/testing/setup-testing-env.sh | 26 |
1 files changed, 18 insertions, 8 deletions
diff --git a/scripts/testing/setup-testing-env.sh b/scripts/testing/setup-testing-env.sh index f0e63aa..fb0628b 100755 --- a/scripts/testing/setup-testing-env.sh +++ b/scripts/testing/setup-testing-env.sh @@ -45,25 +45,35 @@ PACKAGES=( socat ) +to_install=() for pkg in "${PACKAGES[@]}"; do if pacman -Qi "$pkg" &>/dev/null; then info "$pkg is already installed" else - step "Installing $pkg" - if sudo pacman -S --noconfirm "$pkg" >> "$LOGFILE" 2>&1; then - success "$pkg installed" - else - error "Failed to install $pkg" - fatal "Package installation failed" - fi + to_install+=("$pkg") fi done +if [ "${#to_install[@]}" -gt 0 ]; then + step "Installing in one transaction: ${to_install[*]}" + if sudo pacman -S --needed --noconfirm "${to_install[@]}" >> "$LOGFILE" 2>&1; then + success "All required packages installed" + else + fatal "Package installation failed" + fi +fi + # Verify KVM support section "Verifying KVM Support" if [ -e /dev/kvm ]; then - success "KVM is available" + if [ -r /dev/kvm ] && [ -w /dev/kvm ]; then + success "KVM is available and accessible" + else + warn "KVM exists but is not readable/writable by user $(id -un)" + info "Add the user to the kvm group: sudo gpasswd -a $(id -un) kvm" + info "Then log out and back in so the new group takes effect" + fi else error "KVM is not available" info "Check if virtualization is enabled in BIOS" |
