Merge pull request #904 from debarshiray/wip/rishi/unbreak-suite-load…

…-readonly

Unbreak test suites with multiple files loading common constants

.github/workflows/tests.yml

@@ -89,14 +89,15 @@ jobs:
     steps:
       - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
       - uses: actions/setup-node@60edb5dd545a775178f52524783378180af0d1f8 # v4.0.2
+        with:
+          node-version: v18.20.2 
+      - run: npm pack ./
+      - run: npm install -g ./bats-*.tgz
       - name: Run test on OS ${{ matrix.os }}
         shell: 'script -q -e -c "bash {0}"' # work around tty issues
         env:
           TERM: linux # fix tput for tty issue work around
-        run: |
-          npm pack ./
-          sudo npm install -g ./bats-*.tgz
-          bats test --print-output-on-failure
+        run: bats test --print-output-on-failure
 
   windows:
     runs-on: windows-2019
@@ -116,6 +117,8 @@ jobs:
     steps:
       - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
       - uses: actions/setup-node@60edb5dd545a775178f52524783378180af0d1f8 # v4.0.2
+        with:
+          node-version: v18.20.2 
       - run: npm pack ./
       - run: npm install -g (get-item .\bats-*.tgz).FullName
       - run: bats  -T --print-output-on-failure test
@@ -161,6 +164,8 @@ jobs:
     steps:
       - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
       - uses: actions/setup-node@60edb5dd545a775178f52524783378180af0d1f8 # v4.0.2
+        with:
+          node-version: v18.20.2 
       - name: Install unbuffer via expect
         run: brew install expect
       - name: Run test on OS ${{ matrix.os }}

docs/CHANGELOG.md

@@ -19,6 +19,8 @@ The format is based on [Keep a Changelog][kac] and this project adheres to
 
 ### Fixed
 
+* unbreak test suites with multiple files loading common constants (#904), introduced in v1.11.0
+
 #### Documentation
 
 * fix hard-coded link to readthedocs (#901)

lib/bats-core/tracing.bash

@@ -277,19 +277,17 @@ bats_debug_trap() {
   # We need to normalize them to a common format!
   local NORMALIZED_INPUT
   bats_normalize_windows_dir_path NORMALIZED_INPUT "${1%/*}"
-  local file_excluded='' path
+  local path
   for path in "${BATS_DEBUG_EXCLUDE_PATHS[@]}"; do
     if [[ "$NORMALIZED_INPUT" == "$path"* ]]; then
-      file_excluded=1
-      break
+      return # skip this call
     fi
   done
 
   # don't update the trace within library functions or we get backtraces from inside traps
   # also don't record new stack traces while handling interruptions, to avoid overriding the interrupted command
-  if [[ -z "$file_excluded" &&
-    "${BATS_INTERRUPTED-NOTSET}" == NOTSET &&
-    "${BATS_TIMED_OUT-NOTSET}" == NOTSET ]]; then
+  if [[ "${BATS_INTERRUPTED-NOTSET}" == NOTSET &&
+        "${BATS_TIMED_OUT-NOTSET}" == NOTSET ]]; then
     BATS_DEBUG_LASTLAST_STACK_TRACE=(
       ${BATS_DEBUG_LAST_STACK_TRACE[@]+"${BATS_DEBUG_LAST_STACK_TRACE[@]}"}
     )

libexec/bats-core/bats-gather-tests

@@ -3,8 +3,6 @@ set -eET
 
 args=("$@")
 filter_tags_list=()
-included_tests=()
-excluded_tests=()
 
 # shellcheck source=lib/bats-core/common.bash disable=SC2153 
 source "$BATS_ROOT/lib/bats-core/common.bash"
@@ -250,61 +248,129 @@ if [[ -n "${filter_status-}" ]]; then
   else
     printf "No recording of previous runs found. Running all tests!\n" >&2
   fi
+else
+  : #printf "Not filtering by status!\n" >&2
 fi
 
 # shellcheck source=lib/bats-core/tracing.bash
 source "$BATS_ROOT/lib/bats-core/tracing.bash"
 
+BATS_OUT="$BATS_RUN_TMPDIR/gather-tests.out"
+touch "$BATS_OUT"
+
 bats_gather_tests_exit_trap() {
   local bats_gather_tests_exit_status=$?
   trap - ERR EXIT DEBUG
-  if [[ ${BATS_ERROR_STATUS:-0} != 0 ]]; then
-    bats_gather_tests_exit_status=$BATS_ERROR_STATUS
+  if (( bats_gather_tests_exit_status != 0)); then
     printf "1..1\nnot ok 1 bats-gather-tests\n"
-    bats_get_failure_stack_trace stack_trace
-    bats_print_stack_trace "${stack_trace[@]}"
-    bats_print_failed_command "${stack_trace[@]}" 
+    # play back traces from test file evaluation
+    bats_replace_filename < "$BATS_TRACE"
+    bats_replace_filename <"$BATS_OUT" | bats_prefix_lines_for_tap_output
   fi >&2
   exit "$bats_gather_tests_exit_status"
 }
+
 trap bats_gather_tests_exit_trap EXIT
 
-bats_set_stacktrace_limit
+# prepare tracing for errors during test file evaluation
+BATS_TRACE="$BATS_RUN_TMPDIR/bats-gather-tests.trace"
+touch "$BATS_TRACE"
+
+bats_gather_tests_source_exit_trap() {
+  local bats_gather_tests_source_exit_status=$?
+  trap - ERR EXIT DEBUG
+  if (( bats_gather_tests_source_exit_status != 0)); then
+    bats_get_failure_stack_trace stack_trace
+    bats_print_stack_trace "${stack_trace[@]}"
+    # TODO: why doesn't this work via ERR trap?
+    BATS_ERROR_STATUS=$bats_gather_tests_source_exit_status
+    bats_print_failed_command "${stack_trace[@]}"
+  fi  >>"$BATS_TRACE"
+  exit "$bats_gather_tests_source_exit_status"
+}
+
+bats_gather_tests_for_file() {
+  local test_names=() test_dupes=() included_tests=() excluded_tests=()
+
+  trap bats_gather_tests_source_exit_trap EXIT
+  bats_setup_tracing
+  bats_set_stacktrace_limit
+
+  # do the actual evaluation for gathering the tests
+  # shellcheck disable=SC1090
+  BATS_TEST_DIRNAME="${filename%/*}" source "$BATS_TEST_SOURCE" 1>"$BATS_OUT" 2>&1
+
+  if [[ "${#test_dupes[@]}" -ne 0 ]]; then
+    printf 'file_duplicate_test_names="%q"\n' "${test_dupes[*]#$filename$'\t'}"
+  fi
+
+  if [[ -n "$filter_status" ]]; then
+    # save filtered tests to exclude them again in next round
+    for test_line in "${excluded_tests[@]}"; do
+      printf "status-filtered %s %s\n" "$filter_status" "$test_line"
+    done >>"$BATS_RUNLOG_FILE"
+  fi
+
+
+  printf "file_test_count=%d\n" "${#test_names[@]}"
+  printf "file_included_test_count=%d\n" "${#included_tests[@]}"
+  printf "focus_mode=%d\n" "$focus_mode"
+}
 
-bats_setup_tracing
 focus_mode=0
+total_test_count=0
+total_included_test_count=0
 for filename in "$@"; do
   if [[ ! -f "$filename" ]]; then
     abort 'Test file "%s" does not exist.\n' "${filename}"
   fi
 
-  test_names=()
-  test_dupes=()
-
   BATS_TEST_FILENAME="$filename"
   _bats_test_functions_setup -1 # invalid TEST_NUMBER, as this is not a test
 
   BATS_TEST_NAME=source
   BATS_TEST_FILTER="$BATS_TEST_FILTER" bats_preprocess_source # uses BATS_TEST_FILENAME
-  # shellcheck disable=SC1090
-  BATS_TEST_DIRNAME="${filename%/*}" source "$BATS_TEST_SOURCE"
 
-  if [[ "${#test_dupes[@]}" -ne 0 ]]; then
-    abort 'Duplicate test name(s) in file "%s": %s' "${filename}" "${test_dupes[*]#$filename$'\t'}"
+  file_duplicate_test_names=""
+  file_test_count=0
+  file_included_test_count=0
+  saved_focus_mode=$focus_mode
+
+  # get new values for the variables above
+  if [[ $BASH_VERSION == 4.3.*  ]]; then
+    # Bash 4.3 has function scoping issues when this is run in $() -> work around via file
+    bats_gather_tests_var_transfer_file=$BATS_RUN_TMPDIR/gather-tests-var-transfer
+    (set -eET; bats_gather_tests_for_file >"$bats_gather_tests_var_transfer_file")
+    result=$(<"$bats_gather_tests_var_transfer_file")
+  else
+    # separate retrieval from eval to avoid hiding the exit code
+    result="$(set -eET; bats_gather_tests_for_file)"
+  fi
+
+  eval "$result"
+
+  if [[ -n "$file_duplicate_test_names" ]]; then
+    trap - EXIT # prevent 1..1 from being printed
+    abort 'Duplicate test name(s) in file "%s": %s' "$filename" "$file_duplicate_test_names"
+  fi
+
+  total_test_count=$((total_test_count + file_test_count))
+
+  # did focus mode turn on in this file? (cannot turn off afterwards)
+  if (( saved_focus_mode != focus_mode)); then # -> only count new tests
+    total_included_test_count=$file_included_test_count
+  else # -> count previous tests as well
+    total_included_test_count=$((total_included_test_count + file_included_test_count))
   fi
 done
 
 if [[ -n "$filter_status" ]]; then
-  # save filtered tests to exclude them again in next round
-  for test_line in "${excluded_tests[@]}"; do
-    printf "status-filtered %s %s\n" "$filter_status" "$test_line"
-  done >>"$BATS_RUNLOG_FILE"
-
-  if [[ ${#test_names[@]} -eq 0 && ${#included_tests[@]} -eq 0 ]]; then
+  if (( total_test_count == 0 && total_included_test_count == 0 )); then
     printf "There were no tests of status '%s' in the last recorded run.\n" "$filter_status" >&2
   fi
 fi
 
+# communicate to the caller that we are running in focus mode
 if (( focus_mode )); then
   printf "focus_mode\n"
-fi
+fi

test/fixtures/suite/errors_in_multiple_load/a.bats

@@ -0,0 +1,5 @@
+load test_helper
+
+@test "truth" {
+  true
+}

test/fixtures/suite/errors_in_multiple_load/b.bats

@@ -0,0 +1,5 @@
+load test_helper
+
+@test "more truth" {
+  true
+}

test/fixtures/suite/errors_in_multiple_load/c.bats

@@ -0,0 +1,5 @@
+load nonexistent
+
+@test "yet more truth" {
+  true
+}

test/fixtures/suite/errors_in_multiple_load/test_helper.bash

@@ -0,0 +1 @@
+call-to-undefined-command

test/fixtures/suite/multiple_load_constants/a.bats

@@ -0,0 +1,5 @@
+load test_helper
+
+@test "constant" {
+  [ "$A_CONSTANT" = "value" ]
+}

test/fixtures/suite/multiple_load_constants/b.bats

@@ -0,0 +1,5 @@
+load test_helper
+
+@test "constant (again)" {
+  [ "$A_CONSTANT" = "value" ]
+}

test/fixtures/suite/multiple_load_constants/test_helper.bash

@@ -0,0 +1,2 @@
+# shellcheck disable=SC2034
+readonly A_CONSTANT="value"

test/suite.bats

@@ -36,13 +36,56 @@ setup() {
   echo "$output" | grep "^ok . quasi-truth"
 }
 
+@test "aggregated output of multiple tests in a suite loading common constants" {
+  reentrant_run bats "$FIXTURE_ROOT/multiple_load_constants"
+  [ $status -eq 0 ]
+  [ "${lines[0]}" = "1..2" ]
+  [ "${lines[1]}" = "ok 1 constant" ]
+  [ "${lines[2]}" = "ok 2 constant (again)" ]
+}
+
 @test "a failing test in a suite results in an error exit code" {
   FLUNK=1 reentrant_run bats "$FIXTURE_ROOT/multiple"
   [ $status -eq 1 ]
   [ "${lines[0]}" = "1..3" ]
   echo "$output" | grep "^not ok . quasi-truth"
 }
 
+@test "errors when loading common helper from multiple tests in a suite" {
+  reentrant_run bats "$FIXTURE_ROOT/errors_in_multiple_load"
+  [ $status -eq 1 ]
+  [ "${lines[0]}" = "1..1" ]
+  [ "${lines[1]}" = "not ok 1 bats-gather-tests" ]
+
+  # bash > 4.0 returns error codes from source
+  # bash < 4.0 does not handle the status on source, it fails through the ERREXIT instead, which creates another trace
+  # bash == 4.0 seems to be sonwhere in between
+  if (( BASH_VERSINFO[0] > 4 )) || (( BASH_VERSINFO[0] == 4 && BASH_VERSINFO[1] > 0 )); then
+    [ "${lines[2]}" = "# (in test file $RELATIVE_FIXTURE_ROOT/errors_in_multiple_load/a.bats, line 1)" ]
+    [ "${lines[3]}" = "#   \`load test_helper' failed" ]
+    [ "${lines[4]}" = "# $FIXTURE_ROOT/errors_in_multiple_load/test_helper.bash: line 1: call-to-undefined-command: command not found" ]
+    [ "${lines[5]}" = "# Error while sourcing library loader at '$FIXTURE_ROOT/errors_in_multiple_load/test_helper.bash'" ]
+    [ "${#lines[@]}" -eq 6 ]
+  else
+    [ "${lines[2]}" = "# (in file $RELATIVE_FIXTURE_ROOT/errors_in_multiple_load/test_helper.bash, line 1," ]
+    [ "${lines[3]}" = "#  from function \`bats_internal_load' in file ${RELATIVE_BATS_ROOT}lib/bats-core/test_functions.bash, line 69," ]
+    [ "${lines[4]}" = "#  from function \`bats_load_safe' in file ${RELATIVE_BATS_ROOT}lib/bats-core/test_functions.bash, line 106," ]
+    [ "${lines[5]}" = "#  from function \`load' in file ${RELATIVE_BATS_ROOT}lib/bats-core/test_functions.bash, line 156," ]
+    [ "${lines[6]}" = "#  in test file $RELATIVE_FIXTURE_ROOT/errors_in_multiple_load/a.bats, line 1)" ]
+    if (( BASH_VERSINFO[0] == 4)); then
+      [ "${lines[7]}" = "#   \`load test_helper' failed" ]
+      [ "${lines[8]}" = "# $FIXTURE_ROOT/errors_in_multiple_load/test_helper.bash: line 1: call-to-undefined-command: command not found" ]
+      [ "${lines[9]}" = "# Error while sourcing library loader at '$FIXTURE_ROOT/errors_in_multiple_load/test_helper.bash'" ]
+      [ "${#lines[@]}" -eq 10 ]
+    else
+      [ "${lines[7]}" = "#   \`load test_helper' failed with status 127" ]
+      [ "${lines[8]}" = "# $FIXTURE_ROOT/errors_in_multiple_load/test_helper.bash: line 1: call-to-undefined-command: command not found" ]
+      [ "${#lines[@]}" -eq 9 ]
+    fi
+  fi
+
+}
+
 @test "running an ad-hoc suite by specifying multiple test files" {
   reentrant_run bats "$FIXTURE_ROOT/multiple/a.bats" "$FIXTURE_ROOT/multiple/b.bats"
   [ $status -eq 0 ]

test/test_helper.bash

@@ -15,6 +15,14 @@ fixtures() {
   FIXTURE_ROOT="$BATS_TEST_DIRNAME/fixtures/$1"
   # shellcheck disable=SC2034
   RELATIVE_FIXTURE_ROOT="${FIXTURE_ROOT#"$BATS_CWD"/}"
+  if [[ $BATS_ROOT == "$BATS_CWD" ]]; then
+    RELATIVE_BATS_ROOT=''
+  else
+    RELATIVE_BATS_ROOT=${BATS_ROOT#"$BATS_CWD"/}
+  fi
+  if [[ -n "$RELATIVE_BATS_ROOT" && "$RELATIVE_BATS_ROOT" != */ ]]; then
+    RELATIVE_BATS_ROOT+=/
+  fi
 }
 
 filter_control_sequences() {

test/warnings.bats

@@ -5,15 +5,6 @@ bats_require_minimum_version 1.5.0
 setup() {
   load test_helper
   fixtures warnings
-  if [[ $BATS_ROOT == "$BATS_CWD" ]]; then
-    RELATIVE_BATS_ROOT=''
-  else
-    RELATIVE_BATS_ROOT=${BATS_ROOT#"$BATS_CWD"/}
-  fi
-  if [[ -n "$RELATIVE_BATS_ROOT" && "$RELATIVE_BATS_ROOT" != */ ]]; then
-    RELATIVE_BATS_ROOT+=/
-  fi
-  echo "RELATIVE_BATS_ROOT=$RELATIVE_BATS_ROOT" "BATS_ROOT=$BATS_ROOT" "BATS_CWD=$BATS_CWD"
 }
 
 @test "invalid warning is an error" {