From fcda0311173af3c529980efa699d71fe9298844f Mon Sep 17 00:00:00 2001
From: Ben Duncan <5178243+benduncan@users.noreply.github.com>
Date: Sun, 27 Sep 2026 10:37:19 -0500
Subject: [PATCH] test(e2e): make the workbook teardown a gate, not a warning
 (#1110)

* test(e2e): make the workbook teardown a gate, not a warning

A customer who cannot remove what they created is as stuck as one who could
not create it, so the teardown is part of what a workbook proves. It logged
WARN and passed the suite regardless, and a bucket that could never be emptied
went unnoticed for weeks behind exactly that line.

The failure it hides is a wedge rather than an error: the AWS provider retries
DeleteBucket against a bucket it cannot empty indefinitely. So the run is
bounded by DESTROY_TIMEOUT, defaulting to 900s, and the last forty lines of its
output are printed when it fails. The cleanup after a failed apply is bounded
the same way for the same reason.

* test(e2e): print a workbook's PASS after its teardown, not before

The verdict now depends on the teardown, so a workbook that built everything
correctly and then could not delete it printed PASS and FAIL for the same run.
---
 tests/e2e/run-tofu-examples-e2e.sh | 32 +++++++++++++++++++++++-------
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/tests/e2e/run-tofu-examples-e2e.sh b/tests/e2e/run-tofu-examples-e2e.sh
index 60175d10b..8e9f76979 100755
--- a/tests/e2e/run-tofu-examples-e2e.sh
+++ b/tests/e2e/run-tofu-examples-e2e.sh
@@ -616,14 +616,15 @@ run_workbook() {
 
     if ! tofu apply -auto-approve "${apply_args[@]}"; then
         log "  FAIL ${example}: tofu apply"
-        tofu destroy -auto-approve "${apply_args[@]}" >/dev/null 2>&1 || true
+        # Bounded like the teardown below: this one only tidies up after a
+        # workbook that has already failed, so it must not hold the suite open.
+        timeout "${DESTROY_TIMEOUT:-900}" tofu destroy -auto-approve "${apply_args[@]}" >/dev/null 2>&1 || true
         return 1
     fi
 
-    local rc=0
-    if assert_"${example//-/_}" && assert_clean_plan "$example" "${apply_args[@]}"; then
-        log "  PASS ${example}"
-    else
+    local rc=0 destroy_log
+    destroy_log=$(mktemp)
+    if ! assert_"${example//-/_}" || ! assert_clean_plan "$example" "${apply_args[@]}"; then
         log "  FAIL ${example}: assertion"
         # Capture OVN state while VMs still exist — the EXIT trap fires after
         # destroy, by which point port groups, address sets, and ACLs are gone.
@@ -631,8 +632,25 @@ run_workbook() {
         rc=1
     fi
 
-    tofu destroy -auto-approve "${apply_args[@]}" >/dev/null 2>&1 || \
-        log "  WARN ${example}: tofu destroy failed"
+    # The teardown is part of what the workbook proves: a customer who cannot
+    # remove what they created is as stuck as one who could not create it. It
+    # was a WARN, and a bucket that could never be emptied passed for weeks.
+    #
+    # Bounded because the failure mode is a wedge rather than an error — the
+    # provider retries DeleteBucket against a bucket it cannot empty forever.
+    if ! timeout "${DESTROY_TIMEOUT:-900}" tofu destroy -auto-approve "${apply_args[@]}" > "$destroy_log" 2>&1; then
+        log "  FAIL ${example}: tofu destroy"
+        sed 's|^|    |' "$destroy_log" | tail -40
+        rc=1
+    fi
+    rm -f "$destroy_log"
+
+    # After the teardown, not before it. The verdict now depends on it, so a
+    # workbook that printed PASS and then failed to delete what it built would
+    # have reported both.
+    if [ "$rc" -eq 0 ]; then
+        log "  PASS ${example}"
+    fi
 
     cd "$SCRIPT_DIR"
     return "$rc"