fix: stop executing remaining commands in set_up/tear_down after first failure

Author: Chemaclass

src/runner.sh

@@ -526,8 +526,9 @@ function runner::cleanup_on_exit() {
   local exit_code="$2"
 
   set +e
-  local teardown_status=0
-  runner::run_tear_down "$test_file" || teardown_status=$?
+  # Don't use || here - it disables ERR trap in the entire call chain
+  runner::run_tear_down "$test_file"
+  local teardown_status=$?
   runner::clear_mocks
   cleanup_testcase_temp_files
 
@@ -743,20 +744,22 @@ function runner::execute_test_hook() {
   local hook_output_file
   hook_output_file=$(temp_file "${hook_name}_output")
 
-  # Enable errexit and errtrace to catch any failing command in the hook
+  # Enable errexit and errtrace to catch any failing command in the hook.
+  # The ERR trap saves the exit status to a global variable (since return value
+  # from trap doesn't propagate properly), disables errexit (to prevent caller
+  # from exiting) and returns from the hook function, preventing subsequent
+  # commands from executing.
+  # Variables set before the failure are preserved since we don't use a subshell.
+  _BASHUNIT_HOOK_ERR_STATUS=0
   set -eE
-  # Set up trap to catch errors and record the exit status
-  trap 'status=$?; set +eE; trap - ERR' ERR
+  trap '_BASHUNIT_HOOK_ERR_STATUS=$?; set +eE; trap - ERR; return $_BASHUNIT_HOOK_ERR_STATUS' ERR
 
   {
     "$hook_name"
   } >"$hook_output_file" 2>&1
 
-  # Capture final status and clean up
-  local block_exit=$?
-  if [[ $status -eq 0 ]]; then
-    status=$block_exit
-  fi
+  # Capture exit status from global variable and clean up
+  status=$_BASHUNIT_HOOK_ERR_STATUS
   trap - ERR
   set +eE
 

tests/acceptance/bashunit_setup_error_test.sh

@@ -86,3 +86,23 @@ function test_bashunit_when_set_up_with_intermediate_failing_command() {
   assert_contains "$assertions_summary" "$actual"
   assert_general_error "$(./bashunit --no-parallel --env "$TEST_ENV_FILE" "$test_file")"
 }
+
+# Issue #517: When set_up fails, remaining commands should not execute
+function test_bashunit_set_up_stops_on_first_failure() {
+  local test_file=./tests/acceptance/fixtures/test_bashunit_setup_stops_on_failure.sh
+  local marker_file="/tmp/bashunit_setup_marker_test"
+
+  # Clean up any existing marker file
+  rm -f "$marker_file"
+
+  set +e
+  ./bashunit --no-parallel --env "$TEST_ENV_FILE" "$test_file" >/dev/null 2>&1
+  set -e
+
+  # The marker file should NOT exist because the touch command
+  # should not have executed after the failing command
+  assert_file_not_exists "$marker_file"
+
+  # Clean up
+  rm -f "$marker_file"
+}

tests/acceptance/fixtures/test_bashunit_setup_stops_on_failure.sh

@@ -0,0 +1,14 @@
+#!/usr/bin/env bash
+
+# Test fixture for issue #517: When set_up fails, remaining commands should not execute
+
+function set_up() {
+  # This command will fail
+  false
+  # This should NOT execute - if it does, the marker file will be created
+  touch "/tmp/bashunit_setup_marker_test"
+}
+
+function test_dummy() {
+  assert_same "foo" "foo"
+}