From 35997c8ae5673ed666332cdca1f5f7a58c8af424 Mon Sep 17 00:00:00 2001
From: A <258483684+la14-1@users.noreply.github.com>
Date: Thu, 12 Feb 2026 15:01:49 -0800
Subject: [PATCH] refactor: extract helpers from run_test() in test/mock.sh
 (#713)

Break down the 150-line run_test() function into focused helpers:
- run_script_with_timeout(): script execution with env vars and timeout
- show_failure_output(): display last 20 lines on failure
- assert_error_scenario(): handle error scenario assertions
- assert_cloud_api_calls(): cloud-specific API call assertions
- record_test_result(): write pass/fail to RESULTS_FILE

run_test() is now 57 lines (62% reduction), each helper is under 35 lines.

Agent: complexity-hunter

Co-authored-by: A <6723574+louisgv@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
 test/mock.sh | 172 ++++++++++++++++++++++++++++++---------------------
 1 file changed, 100 insertions(+), 72 deletions(-)

diff --git a/test/mock.sh b/test/mock.sh
index ed24188d..3c74e5af 100644
--- a/test/mock.sh
+++ b/test/mock.sh
@@ -603,41 +603,19 @@ discover_agents() {
 }
 
 # ============================================================
-# Test runner
+# Test runner helpers
 # ============================================================
 
-run_test() {
-    local cloud="$1"
-    local agent="$2"
-    local script_path="${REPO_ROOT}/${cloud}/${agent}.sh"
+# Run a script in a sandboxed environment with a 4-second timeout.
+# Sets exit_code variable in the caller's scope.
+# Args: script_path cloud state_file fake_home
+run_script_with_timeout() {
+    local script_path="$1"
+    local cloud="$2"
+    local state_file="$3"
+    local fake_home="$4"
 
-    if [[ ! -f "$script_path" ]]; then
-        printf '%b\n' "  ${YELLOW}skip${NC} ${cloud}/${agent}.sh — file not found"
-        SKIPPED=$((SKIPPED + 1))
-        return 0
-    fi
-
-    printf '%b\n' "  ${CYAN}test${NC} ${cloud}/${agent}.sh"
-
-    # Snapshot failure count before this test's assertions
-    local _pre_failed="${FAILED}"
-
-    # Reset mock log
-    : > "${MOCK_LOG}"
-
-    # Set up environment
-    setup_env_for_cloud "$cloud"
-
-    # Fake HOME to avoid polluting real home
-    local fake_home
-    fake_home=$(setup_fake_home)
-
-    # Set up state file for state tracking
-    local state_file="${TEST_DIR}/state_${cloud}_${agent}.log"
-    : > "${state_file}"
-
-    # Run the script with mocked PATH + HOME (10s timeout — all calls are fake)
-    local exit_code=0
+    exit_code=0
 
     MOCK_LOG="${MOCK_LOG}" \
     MOCK_FIXTURE_DIR="${FIXTURES_DIR}/${cloud}" \
@@ -657,16 +635,18 @@ run_test() {
             kill -9 "$pid" 2>/dev/null
             wait "$pid" 2>/dev/null || true
             exit_code=124
-            break
+            return
         fi
         sleep 1
         i=$((i + 1))
     done
-    if [[ "$exit_code" -ne 124 ]]; then
-        wait "$pid" 2>/dev/null || exit_code=$?
-    fi
+    wait "$pid" 2>/dev/null || exit_code=$?
+}
 
-    # Show last lines of output on failure
+# Print last 20 lines of output on script failure.
+# Args: exit_code
+show_failure_output() {
+    local exit_code="$1"
     if [[ "${exit_code}" -ne 0 ]]; then
         printf '%b\n' "    ${RED}--- output (last 20 lines) ---${NC}"
         tail -20 "${TEST_DIR}/output.log" 2>/dev/null | while IFS= read -r line; do
@@ -674,30 +654,34 @@ run_test() {
         done
         printf '%b\n' "    ${RED}--- end output ---${NC}"
     fi
+}
 
-    # --- Assertions ---
-    if [[ -n "${MOCK_ERROR_SCENARIO:-}" ]]; then
-        # Error scenarios: expect non-zero exit
-        if [[ "${exit_code}" -ne 0 ]]; then
-            printf '%b\n' "    ${GREEN}✓${NC} fails on ${MOCK_ERROR_SCENARIO} (exit code ${exit_code})"
-            PASSED=$((PASSED + 1))
-            if [[ -n "${RESULTS_FILE:-}" ]]; then
-                printf '%s/%s:pass\n' "${cloud}" "${agent}" >> "${RESULTS_FILE}"
-            fi
-        else
-            printf '%b\n' "    ${RED}✗${NC} should fail on ${MOCK_ERROR_SCENARIO} but exited 0"
-            FAILED=$((FAILED + 1))
-            if [[ -n "${RESULTS_FILE:-}" ]]; then
-                printf '%s/%s:fail\n' "${cloud}" "${agent}" >> "${RESULTS_FILE}"
-            fi
-        fi
-        printf '\n'
-        return 0
+# Assert that the script failed when an error scenario was injected.
+# Returns 0 (with result recorded) if an error scenario is active, 1 otherwise.
+# Args: exit_code cloud agent
+assert_error_scenario() {
+    local exit_code="$1"
+    local cloud="$2"
+    local agent="$3"
+
+    [[ -n "${MOCK_ERROR_SCENARIO:-}" ]] || return 1
+
+    if [[ "${exit_code}" -ne 0 ]]; then
+        printf '%b\n' "    ${GREEN}✓${NC} fails on ${MOCK_ERROR_SCENARIO} (exit code ${exit_code})"
+        PASSED=$((PASSED + 1))
+        record_test_result "${cloud}" "${agent}" "pass"
+    else
+        printf '%b\n' "    ${RED}✗${NC} should fail on ${MOCK_ERROR_SCENARIO} but exited 0"
+        FAILED=$((FAILED + 1))
+        record_test_result "${cloud}" "${agent}" "fail"
     fi
+    return 0
+}
 
-    assert_exit_code "${exit_code}" 0 "exits successfully"
-
-    # Cloud-specific API call assertions
+# Assert that the expected cloud-specific API calls were made.
+# Args: cloud
+assert_cloud_api_calls() {
+    local cloud="$1"
     case "$cloud" in
         hetzner)
             assert_api_called "GET" "/ssh_keys" "fetches SSH keys"
@@ -727,31 +711,75 @@ run_test() {
             assert_log_contains "curl (GET|POST) https://" "makes API calls"
             ;;
     esac
+}
 
-    # Check that SSH was used (for remote execution)
+# Write pass/fail result to RESULTS_FILE if set.
+# Args: cloud agent result ("pass" or "fail", or "auto" to compute from _pre_failed)
+record_test_result() {
+    local cloud="$1"
+    local agent="$2"
+    local result="$3"
+    [[ -n "${RESULTS_FILE:-}" ]] || return 0
+    printf '%s/%s:%s\n' "${cloud}" "${agent}" "${result}" >> "${RESULTS_FILE}"
+}
+
+# ============================================================
+# Test runner
+# ============================================================
+
+run_test() {
+    local cloud="$1"
+    local agent="$2"
+    local script_path="${REPO_ROOT}/${cloud}/${agent}.sh"
+
+    if [[ ! -f "$script_path" ]]; then
+        printf '%b\n' "  ${YELLOW}skip${NC} ${cloud}/${agent}.sh — file not found"
+        SKIPPED=$((SKIPPED + 1))
+        return 0
+    fi
+
+    printf '%b\n' "  ${CYAN}test${NC} ${cloud}/${agent}.sh"
+
+    local _pre_failed="${FAILED}"
+
+    : > "${MOCK_LOG}"
+    setup_env_for_cloud "$cloud"
+
+    local fake_home
+    fake_home=$(setup_fake_home)
+
+    local state_file="${TEST_DIR}/state_${cloud}_${agent}.log"
+    : > "${state_file}"
+
+    local exit_code
+    run_script_with_timeout "${script_path}" "${cloud}" "${state_file}" "${fake_home}"
+    show_failure_output "${exit_code}"
+
+    # Error scenario mode: just check that script failed, then return
+    if assert_error_scenario "${exit_code}" "${cloud}" "${agent}"; then
+        printf '\n'
+        return 0
+    fi
+
+    # Normal mode: run standard assertions
+    assert_exit_code "${exit_code}" 0 "exits successfully"
+    assert_cloud_api_calls "$cloud"
     assert_log_contains "ssh " "uses SSH"
-
-    # Check OpenRouter API key injection
     assert_env_injected "OPENROUTER_API_KEY"
 
-    # Body validation (when enabled)
     if [[ "${MOCK_VALIDATE_BODY:-}" == "1" ]]; then
         assert_no_body_errors
     fi
-
-    # State tracking (when enabled)
     if [[ "${MOCK_TRACK_STATE:-}" == "1" ]]; then
         assert_server_cleaned_up "${state_file}"
     fi
 
-    # Write per-test result to RESULTS_FILE (used by qa-dry-run.sh / qa-cycle.sh)
-    if [[ -n "${RESULTS_FILE:-}" ]]; then
-        local pre_fail=$((FAILED - _pre_failed))
-        if [[ "$pre_fail" -gt 0 ]]; then
-            printf '%s/%s:fail\n' "${cloud}" "${agent}" >> "${RESULTS_FILE}"
-        else
-            printf '%s/%s:pass\n' "${cloud}" "${agent}" >> "${RESULTS_FILE}"
-        fi
+    # Record result
+    local pre_fail=$((FAILED - _pre_failed))
+    if [[ "$pre_fail" -gt 0 ]]; then
+        record_test_result "${cloud}" "${agent}" "fail"
+    else
+        record_test_result "${cloud}" "${agent}" "pass"
     fi
 
     printf '\n'