test(archsetup): make Testinfra the authoritative validator (P3 cutover)

run-test.sh no longer runs the shell run_all_validations sweep; the Testinfra
pytest sweep now drives the run's pass/fail. run_testinfra_validation returns
pytest's exit code (and treats "could not run" as a failure, not a silent
pass), surfaces the pass/skip/fail counts through the shared VALIDATION_*
counters, and parses the attribution file so generate_issue_report still
buckets failures into archsetup / base_install / unknown.

The shell-sweep functions stay in validation.sh for now because
run-test-baremetal.sh still calls them; removing them (after migrating the
bare-metal runner) is filed as a follow-up.

2 files changed, 66 insertions, 39 deletions

diff --git a/scripts/testing/lib/testinfra.sh b/scripts/testing/lib/testinfra.sh
index bfcd43a..0822a9f 100644
--- a/scripts/testing/lib/testinfra.sh
+++ b/scripts/testing/lib/testinfra.sh
@@ -3,19 +3,40 @@
 #
 # Testinfra post-install validation sweep (runs on the host, over SSH).
 #
-# P1 status: advisory. This runs alongside the shell sweep (run_all_validations)
-# so a real VM run can diff the two and prove parity before pytest becomes the
-# primary validator (P3 cutover). It never sets the run's pass/fail here.
+# This is the primary post-install validator (it replaced the shell
+# run_all_validations sweep). It connects to the freshly-installed VM over SSH
+# and runs the pytest suite under scripts/testing/tests/. Its result drives the
+# run's pass/fail, and per-test failures are bucketed (archsetup / base_install
+# / unknown) into the same issue-attribution report the shell sweep produced.
 #
-# Auth: a throwaway ed25519 keypair is generated per run, its pubkey authorized
-# in the VM over the existing sshpass channel, and pytest/testinfra connects
-# key-only via a generated ssh-config. The keypair lives in the results dir and
-# is discarded with it.
+# Auth: reuse the root key the harness already authorized (inject_root_key),
+# which survives the sshd prohibit-password hardening; mint our own only if the
+# harness didn't (standalone use). pytest connects key-only via a generated
+# ssh-config. Key + config live in the results dir and are discarded with it.
 #
 # Uses globals from run-test.sh / vm-utils.sh: SCRIPT_DIR, VM_IP, SSH_PORT,
-# ROOT_PASSWORD, ARCHSETUP_VM_CONF. Toggle with RUN_TESTINFRA=false.
+# ROOT_PASSWORD, ROOT_SSH_KEY, ARCHSETUP_VM_CONF, plus the validation.sh
+# helpers attribute_issue / VALIDATION_*. Toggle with RUN_TESTINFRA=false.
+
+# Record each pytest failure from the attribution file into the issue arrays
+# (validation.sh's attribute_issue), so generate_issue_report covers them.
+_testinfra_record_attribution() {
+    local file="$1" bucket=""
+    [ -f "$file" ] || return 0
+    while IFS= read -r line; do
+        case "$line" in
+            "[archsetup]")    bucket=archsetup ;;
+            "[base_install]") bucket=base ;;
+            "[unknown]")      bucket=unknown ;;
+            "  "*)            attribute_issue "testinfra: ${line#  }" "$bucket" ;;
+        esac
+    done < "$file"
+}
 
 # run_testinfra_validation <results_dir>
+# Returns 0 only when the pytest sweep ran and passed. Returns non-zero when it
+# failed OR could not run (missing tooling / SSH setup) — a sweep that can't run
+# is not a pass. RUN_TESTINFRA=false is the one explicit opt-out (returns 0).
 run_testinfra_validation() {
     local results_dir="$1"
     local tests_dir="$SCRIPT_DIR/tests"
@@ -23,34 +44,33 @@ run_testinfra_validation() {
     local sshcfg="$results_dir/testinfra_ssh_config"
 
     if [ "${RUN_TESTINFRA:-true}" != "true" ]; then
+        warn "RUN_TESTINFRA=false - skipping the Testinfra validation sweep"
         return 0
     fi
     if ! command -v pytest >/dev/null 2>&1 || ! python3 -c 'import testinfra' >/dev/null 2>&1; then
-        warn "Testinfra/pytest not installed on host - skipping pytest sweep (run: make deps)"
-        return 0
+        error "Testinfra/pytest not installed on host - cannot validate (run: make deps)"
+        return 1
     fi
 
-    step "Running Testinfra validation sweep (advisory)"
+    section "Running Validation Checks (Testinfra)"
 
-    # Prefer the root key the harness already authorized (inject_root_key). It
-    # survives the sshd prohibit-password hardening, so reuse it rather than
-    # authorizing a second key. Fall back to minting our own for standalone use.
+    # Prefer the harness's already-authorized root key; mint one if absent.
     if [ -n "${ROOT_SSH_KEY:-}" ] && [ -f "${ROOT_SSH_KEY}" ]; then
         key="$ROOT_SSH_KEY"
     else
         rm -f "$key" "$key.pub"
         if ! ssh-keygen -t ed25519 -N "" -q -f "$key"; then
-            warn "testinfra: ssh-keygen failed - skipping"
-            return 0
+            error "testinfra: ssh-keygen failed"
+            return 1
         fi
         if ! copy_to_vm "$key.pub" "/tmp/testinfra_key.pub" "$ROOT_PASSWORD"; then
-            warn "testinfra: pubkey copy failed - skipping"
-            return 0
+            error "testinfra: pubkey copy failed"
+            return 1
         fi
         if ! vm_exec "$ROOT_PASSWORD" \
             "mkdir -p /root/.ssh && chmod 700 /root/.ssh && cat /tmp/testinfra_key.pub >> /root/.ssh/authorized_keys && chmod 600 /root/.ssh/authorized_keys"; then
-            warn "testinfra: authorizing key in VM failed - skipping"
-            return 0
+            error "testinfra: authorizing key in VM failed"
+            return 1
         fi
     fi
 
@@ -71,17 +91,30 @@ EOF
     test_user=$(sed -n 's/^USERNAME=//p' "$ARCHSETUP_VM_CONF" 2>/dev/null | head -n1)
     : "${test_user:=cjennings}"
 
+    local logf="$results_dir/testinfra.log"
     ARCHSETUP_TEST_USER="$test_user" pytest "$tests_dir" \
         --hosts="ssh://testinfra-target" \
         --ssh-config="$sshcfg" \
         --attribution-file="$results_dir/testinfra-attribution.txt" \
-        -v >> "$results_dir/testinfra.log" 2>&1
+        -v >> "$logf" 2>&1
     local rc=$?
 
+    # Surface pytest's counts through the shared validation counters so the
+    # issue report summary is meaningful (the shell sweep no longer runs).
+    local summary
+    summary=$(grep -oE '[0-9]+ (passed|failed|error|errors|skipped)' "$logf" | tail -10)
+    VALIDATION_PASSED=$(echo "$summary" | awk '/passed/{print $1}' | tail -1); VALIDATION_PASSED=${VALIDATION_PASSED:-0}
+    VALIDATION_WARNINGS=$(echo "$summary" | awk '/skipped/{print $1}' | tail -1); VALIDATION_WARNINGS=${VALIDATION_WARNINGS:-0}
+    local nfail nerr
+    nfail=$(echo "$summary" | awk '/failed/{print $1}' | tail -1); nfail=${nfail:-0}
+    nerr=$(echo "$summary" | awk '/error/{print $1}' | tail -1); nerr=${nerr:-0}
+    VALIDATION_FAILED=$((nfail + nerr))
+
     if [ "$rc" -eq 0 ]; then
-        success "Testinfra sweep passed (advisory; see testinfra.log)"
+        success "Testinfra validation passed ($VALIDATION_PASSED passed, $VALIDATION_WARNINGS skipped)"
     else
-        warn "Testinfra sweep reported failures (advisory; see testinfra.log + testinfra-attribution.txt)"
+        error "Testinfra validation failed ($VALIDATION_FAILED failed/error; see testinfra.log)"
+        _testinfra_record_attribution "$results_dir/testinfra-attribution.txt"
     fi
-    return 0
+    return "$rc"
 }
diff --git a/scripts/testing/run-test.sh b/scripts/testing/run-test.sh
index 90022d3..6e51fc2 100755
--- a/scripts/testing/run-test.sh
+++ b/scripts/testing/run-test.sh
@@ -316,23 +316,17 @@ copy_from_vm "/var/log/archsetup-installed-packages.txt" "$TEST_RESULTS_DIR/" "$
 # Capture post-install state
 capture_post_install_state "$TEST_RESULTS_DIR"
 
-# Run comprehensive validation
-# This uses the validation.sh library for all checks.
+# Run comprehensive validation (Testinfra/pytest is the primary validator; the
+# old shell run_all_validations sweep was retired once pytest reached parity).
 #
 # From here to the end of the script, errexit is disabled on purpose: the
-# validation functions are designed to fail-and-count (see VALIDATION_FAILED)
-# rather than abort, and the analysis/report-generation steps below can also
-# legitimately return non-zero. With `set -e` active, a single failed check
-# would kill the run before the test report is written or the VM is cleaned
-# up. Pass/fail is signalled explicitly by the exit code at the bottom.
+# analysis/report-generation steps below can legitimately return non-zero, and
+# with `set -e` active a single failed check would kill the run before the test
+# report is written or the VM is cleaned up. Pass/fail is signalled explicitly
+# by the exit code at the bottom.
 set +e
-run_all_validations
-validate_all_services
-
-# Advisory Testinfra sweep alongside the shell sweep (P1). Compare the two on a
-# real run to confirm parity before pytest becomes primary. Does not affect
-# pass/fail yet.
 run_testinfra_validation "$TEST_RESULTS_DIR"
+testinfra_rc=$?
 
 # Analyze log differences (pre vs post install)
 analyze_log_diff "$TEST_RESULTS_DIR"
@@ -341,8 +335,8 @@ analyze_log_diff "$TEST_RESULTS_DIR"
 # If base install issues found and archzfs inbox exists, create issue file
 generate_issue_report "$TEST_RESULTS_DIR" "$ARCHZFS_INBOX"
 
-# Set validation result based on failure count
-if [ "$VALIDATION_FAILED" -eq 0 ]; then
+# The run passes only if the Testinfra sweep passed.
+if [ "$testinfra_rc" -eq 0 ]; then
     TEST_PASSED=true
 else
     TEST_PASSED=false