Merge pull request #621 from martin-schulze-vireso/bugfix/fix_all_nou…

…nset_problems

Global nounset test

.github/workflows/set_nounset.bash

@@ -0,0 +1 @@
+set -u

.github/workflows/tests.yml

@@ -32,6 +32,7 @@ jobs:
           - ''
           # allow for some parallelity without GNU parallel, since it is not installed by default
           - 'BATS_NO_PARALLELIZE_ACROSS_FILES=1 BATS_NUMBER_OF_PARALLEL_JOBS=2'
+          - 'BASH_ENV=${GITHUB_WORKSPACE}/.github/workflows/set_nounset.bash'
     runs-on: ${{ matrix.os }}
     steps:
       - uses: actions/checkout@v2

docs/CHANGELOG.md

@@ -33,6 +33,7 @@ The format is based on [Keep a Changelog][kac] and this project adheres to
   * also add `--clean-and-gather-test-outputs-in <directory>` for improved UX
 * double slashes in paths derived from TMPDIR on MacOS (#607)
 * fix `load` in `teardown` marking failed tests as not run (#612)
+* fix unset variable errors (with set -u) and add regression test (#621)
 
 #### Documentation
 

lib/bats-core/common.bash

@@ -65,13 +65,13 @@ bats_require_minimum_version() { # <required version>
 }
 
 bats_binary_search() { # <search-value> <array-name>
-  local -r search_value=$1 array_name=$2
-
   if [[ $# -ne 2 ]]; then
     printf "ERROR: bats_binary_search requires exactly 2 arguments: <search value> <array name>\n" >&2
     return 2
   fi
 
+  local -r search_value=$1 array_name=$2
+
   # we'd like to test if array is set but we cannot distinguish unset from empty arrays, so we need to skip that
 
   local start=0 mid end mid_value 

lib/bats-core/tracing.bash

@@ -30,7 +30,7 @@ bats_capture_stack_trace() {
 bats_get_failure_stack_trace() {
 	local stack_trace_var
 	# See bats_debug_trap for details.
-	if [[ -n "${BATS_DEBUG_LAST_STACK_TRACE_IS_VALID}" ]]; then
+	if [[ -n "${BATS_DEBUG_LAST_STACK_TRACE_IS_VALID:-}" ]]; then
 		stack_trace_var=BATS_DEBUG_LAST_STACK_TRACE
 	else
 		stack_trace_var=BATS_DEBUG_LASTLAST_STACK_TRACE
@@ -275,7 +275,7 @@ bats_debug_trap() {
 # isn't set properly during `teardown()` errors.
 bats_check_status_from_trap() {
 	local status="$?"
-	if [[ -z "$BATS_TEST_COMPLETED" ]]; then
+	if [[ -z "${BATS_TEST_COMPLETED:-}" ]]; then
 		BATS_ERROR_STATUS="${BATS_ERROR_STATUS:-$status}"
 		if [[ "$BATS_ERROR_STATUS" -eq 0 ]]; then
 			BATS_ERROR_STATUS=1
@@ -299,6 +299,26 @@ bats_add_debug_exclude_path() { # <path>
 }
 
 bats_setup_tracing() {
+	# Variables for capturing accurate stack traces. See bats_debug_trap for
+	# details.
+	#
+	# BATS_DEBUG_LAST_LINENO, BATS_DEBUG_LAST_SOURCE, and
+	# BATS_DEBUG_LAST_STACK_TRACE hold data from the most recent call to
+	# bats_debug_trap.
+	#
+	# BATS_DEBUG_LASTLAST_STACK_TRACE holds data from two bats_debug_trap calls
+	# ago.
+	#
+	# BATS_DEBUG_LAST_STACK_TRACE_IS_VALID indicates that
+	# BATS_DEBUG_LAST_STACK_TRACE contains the stack trace of the test's error. If
+	# unset, BATS_DEBUG_LAST_STACK_TRACE is unreliable and
+	# BATS_DEBUG_LASTLAST_STACK_TRACE should be used instead.
+	BATS_DEBUG_LASTLAST_STACK_TRACE=()
+	BATS_DEBUG_LAST_LINENO=()
+	BATS_DEBUG_LAST_SOURCE=()
+	BATS_DEBUG_LAST_STACK_TRACE=()
+	BATS_DEBUG_LAST_STACK_TRACE_IS_VALID=
+	BATS_ERROR_SUFFIX=
 	BATS_DEBUG_EXCLUDE_PATHS=()
 	# exclude some paths by default
 	bats_add_debug_exclude_path "$BATS_ROOT/lib/"

libexec/bats-core/bats

@@ -282,13 +282,14 @@ export BATS_WARNING_FILE="${BATS_RUN_TMPDIR}/warnings.log"
 
 bats_exit_trap() {
   if [[ -s "$BATS_WARNING_FILE" ]]; then
+    local pre_cat='' post_cat=''
     if [[ $formatter == pretty ]]; then
-      PRE_CAT=$'\x1B[31m'
-      POST_CAT=$'\x1B[0m'
+      pre_cat=$'\x1B[31m'
+      post_cat=$'\x1B[0m'
     fi
-    printf "\nThe following warnings were encountered during tests:\n%s" "$PRE_CAT"
+    printf "\nThe following warnings were encountered during tests:\n%s" "$pre_cat"
     cat "$BATS_WARNING_FILE"
-    printf "%s" "$POST_CAT"
+    printf "%s" "$post_cat"
   fi >&2
 
   if [[ -n "$BATS_TEMPDIR_CLEANUP" ]]; then

libexec/bats-core/bats-exec-suite

@@ -124,7 +124,8 @@ bats_gather_tests() {
   test_count="${#all_tests[@]}"
 }
 
-TEST_ROOT=${1%/*}
+TEST_ROOT=${1-}
+TEST_ROOT=${TEST_ROOT%/*}
 BATS_RUN_LOGS_DIRECTORY="$TEST_ROOT/.bats/run-logs"
 if [[ ! -d "$BATS_RUN_LOGS_DIRECTORY" ]]; then
   if [[ -n "$filter_status" ]]; then
@@ -218,7 +219,7 @@ if [[ -n "$filter_status" ]]; then
     done >> "$BATS_RUNLOG_FILE"
 
     test_count="${#filtered_tests[@]}"
-    if [[ ${#failed_tests} -eq 0 && ${#filtered_tests[@]} -eq 0 ]]; then
+    if [[ ${#failed_tests[@]} -eq 0 && ${#filtered_tests[@]} -eq 0 ]]; then
       printf "There where no failed tests in the last recorded run.\n" >&2
     fi
   else
@@ -287,7 +288,7 @@ bats_suite_exit_trap() {
     printf "\n# Received SIGINT, aborting ...\n\n"
   fi
 
-  if [[ -d "$BATS_RUN_LOGS_DIRECTORY" && -n "${BATS_INTERRUPTED}" ]]; then
+  if [[ -d "$BATS_RUN_LOGS_DIRECTORY" && -n "${BATS_INTERRUPTED:-}" ]]; then
     # aborting a test run with CTRL+C does not save the runlog file
     rm "$BATS_RUNLOG_FILE"
   fi

libexec/bats-core/bats-exec-test

@@ -202,31 +202,10 @@ bats_perform_test() {
     exit 1
   fi
 
-  # Variables for capturing accurate stack traces. See bats_debug_trap for
-  # details.
-  #
-  # BATS_DEBUG_LAST_LINENO, BATS_DEBUG_LAST_SOURCE, and
-  # BATS_DEBUG_LAST_STACK_TRACE hold data from the most recent call to
-  # bats_debug_trap.
-  #
-  # BATS_DEBUG_LASTLAST_STACK_TRACE holds data from two bats_debug_trap calls
-  # ago.
-  #
-  # BATS_DEBUG_LAST_STACK_TRACE_IS_VALID indicates that
-  # BATS_DEBUG_LAST_STACK_TRACE contains the stack trace of the test's error. If
-  # unset, BATS_DEBUG_LAST_STACK_TRACE is unreliable and
-  # BATS_DEBUG_LASTLAST_STACK_TRACE should be used instead.
-  BATS_DEBUG_LASTLAST_STACK_TRACE=()
-  BATS_DEBUG_LAST_LINENO=()
-  BATS_DEBUG_LAST_SOURCE=()
-  BATS_DEBUG_LAST_STACK_TRACE=()
-  BATS_DEBUG_LAST_STACK_TRACE_IS_VALID=
-
   BATS_TEST_COMPLETED=
   BATS_TEST_SKIPPED=
   BATS_TEARDOWN_COMPLETED=
   BATS_ERROR_STATUS=
-  BATS_ERROR_SUFFIX=
   bats_setup_tracing
   # mark this call as trap call
   trap 'bats_teardown_trap as-exit-trap' EXIT

libexec/bats-core/bats-preprocess

@@ -42,7 +42,7 @@ tests=()
     if [[ "$line" =~ $BATS_TEST_PATTERN ]] || [[ "$line" =~ $BATS_TEST_PATTERN_COMMENT ]]; then
       name="${BASH_REMATCH[1]#[\'\"]}"
       name="${name%[\'\"]}"
-      body="${BASH_REMATCH[2]}"
+      body="${BASH_REMATCH[2]:-}"
       bats_encode_test_name "$name" 'encoded_name'
       printf '%s() { bats_test_begin "%s"; %s\n' "${encoded_name:?}" "$name" "$body" || :
 

test/bats.bats

@@ -326,8 +326,8 @@ setup() {
 @test 'ensure compatibility with unofficial Bash strict mode' {
   local expected='ok 1 unofficial Bash strict mode conditions met'
 
-  if [[ -n "$BATS_NUMBER_OF_PARALLEL_JOBS" ]]; then
-    if [[ -z "$BATS_NO_PARALLELIZE_ACROSS_FILES" ]]; then
+  if [[ -n "${BATS_NUMBER_OF_PARALLEL_JOBS:-}" ]]; then
+    if [[ -z "${BATS_NO_PARALLELIZE_ACROSS_FILES:-}" ]]; then
       type -p parallel &>/dev/null || skip "Don't check file parallelized without GNU parallel"
     fi
     (type -p flock &>/dev/null || type -p shlock &>/dev/null) || skip "Don't check parallelized without flock/shlock "
@@ -340,8 +340,8 @@ setup() {
   run env - \
           "PATH=$PATH" \
           "HOME=$HOME" \
-          "BATS_NO_PARALLELIZE_ACROSS_FILES=$BATS_NO_PARALLELIZE_ACROSS_FILES" \
-          "BATS_NUMBER_OF_PARALLEL_JOBS=$BATS_NUMBER_OF_PARALLEL_JOBS" \
+          "BATS_NO_PARALLELIZE_ACROSS_FILES=${BATS_NO_PARALLELIZE_ACROSS_FILES:-}" \
+          "BATS_NUMBER_OF_PARALLEL_JOBS=${BATS_NUMBER_OF_PARALLEL_JOBS:-}" \
           SHELLOPTS=nounset \
       "${BATS_ROOT}/bin/bats" "$FIXTURE_ROOT/unofficial_bash_strict_mode.bats"
   if [[ "$status" -ne 0 || "${lines[1]}" != "$expected" ]]; then
@@ -602,7 +602,7 @@ END_OF_ERR_MSG
 }
 
 @test "Don't hang on CTRL-C (issue #353)" {
-  if [[ "$BATS_NUMBER_OF_PARALLEL_JOBS" -gt 1 ]]; then
+  if [[ "${BATS_NUMBER_OF_PARALLEL_JOBS:-1}" -gt 1 ]]; then
     skip "Aborts don't work in parallel mode"
   fi
   load 'concurrent-coordination'
@@ -731,7 +731,7 @@ END_OF_ERR_MSG
 }
 
 @test "CTRL-C aborts and fails the current test" {
-  if [[ "$BATS_NUMBER_OF_PARALLEL_JOBS" -gt 1 ]]; then
+  if [[ "${BATS_NUMBER_OF_PARALLEL_JOBS:-1}" -gt 1 ]]; then
     skip "Aborts don't work in parallel mode"
   fi
 
@@ -767,7 +767,7 @@ END_OF_ERR_MSG
 }
 
 @test "CTRL-C aborts and fails the current run" {
-  if [[ "$BATS_NUMBER_OF_PARALLEL_JOBS" -gt 1 ]]; then
+  if [[ "${BATS_NUMBER_OF_PARALLEL_JOBS:-1}" -gt 1 ]]; then
     skip "Aborts don't work in parallel mode"
   fi
 
@@ -804,7 +804,7 @@ END_OF_ERR_MSG
 @test "CTRL-C aborts and fails after run" {
   # shellcheck disable=SC2034
   BATS_TEST_RETRIES=2
-  if [[ "$BATS_NUMBER_OF_PARALLEL_JOBS" -gt 1 ]]; then
+  if [[ "${BATS_NUMBER_OF_PARALLEL_JOBS:-1}" -gt 1 ]]; then
     skip "Aborts don't work in parallel mode"
   fi
 
@@ -839,7 +839,7 @@ END_OF_ERR_MSG
 }
 
 @test "CTRL-C aborts and fails the current teardown" {
-  if [[ "$BATS_NUMBER_OF_PARALLEL_JOBS" -gt 1 ]]; then
+  if [[ "${BATS_NUMBER_OF_PARALLEL_JOBS:-1}" -gt 1 ]]; then
     skip "Aborts don't work in parallel mode"
   fi
 
@@ -875,7 +875,7 @@ END_OF_ERR_MSG
 }
 
 @test "CTRL-C aborts and fails the current setup_file" {
-  if [[ "$BATS_NUMBER_OF_PARALLEL_JOBS" -gt 1 ]]; then
+  if [[ "${BATS_NUMBER_OF_PARALLEL_JOBS:-1}" -gt 1 ]]; then
     skip "Aborts don't work in parallel mode"
   fi
 
@@ -911,7 +911,7 @@ END_OF_ERR_MSG
 }
 
 @test "CTRL-C aborts and fails the current teardown_file" {
-  if [[ "$BATS_NUMBER_OF_PARALLEL_JOBS" -gt 1 ]]; then
+  if [[ "${BATS_NUMBER_OF_PARALLEL_JOBS:-1}" -gt 1 ]]; then
     skip "Aborts don't work in parallel mode"
   fi
   # shellcheck disable=SC2031
@@ -1338,7 +1338,7 @@ enforce_own_process_group() {
   }
 
 @test "--filter-status failed does not update list when run is aborted" {
-  if [[ "$BATS_NUMBER_OF_PARALLEL_JOBS" -gt 1 ]]; then
+  if [[ "${BATS_NUMBER_OF_PARALLEL_JOBS:-1}" -gt 1 ]]; then
     skip "Aborts don't work in parallel mode"
   fi
 

test/file_setup_teardown.bats

@@ -96,7 +96,7 @@ not ok 1 failing test
 }
 
 @test "teardown_file should run even after user abort via CTRL-C" {
-  if [[ "$BATS_NUMBER_OF_PARALLEL_JOBS" -gt 1 ]]; then
+  if [[ "${BATS_NUMBER_OF_PARALLEL_JOBS:-1}" -gt 1 ]]; then
     skip "Aborts don't work in parallel mode"
   fi
   # shellcheck disable=SC2031,SC2030

test/fixtures/bats/environment.bats

@@ -6,5 +6,5 @@
 
 @test "variables do not persist across tests" {
   # shellcheck disable=SC2031
-  [ -z "$variable" ]
+  [ -z "${variable:-}" ]
 }

test/fixtures/bats/retry.bats

@@ -1,7 +1,7 @@
 BATS_TEST_RETRIES=2 # means three tries per test
 
 log_caller() {
-    printf "%s %s %s\n" "$BATS_TEST_NAME" "${FUNCNAME[1]}" "$BATS_TEST_TRY_NUMBER"  >> "${LOG?}"
+    printf "%s %s %s\n" "${BATS_TEST_NAME:-}" "${FUNCNAME[1]}" "${BATS_TEST_TRY_NUMBER:-}"  >> "${LOG?}"
 }
 
 setup_file() {

test/fixtures/file_setup_teardown/teardown_file_does_not_leak2.bats

@@ -1,3 +1,3 @@
 @test "must not see variable from first run" {
-    [[ -z "$POTENTIALLY_LEAKING_VARIABLE" ]]
+    [[ -z "${POTENTIALLY_LEAKING_VARIABLE:-}" ]]
 }

test/fixtures/load/bats_load_library.bats

@@ -1,4 +1,4 @@
-[ -n "$HELPER_NAME" ] || HELPER_NAME="test_helper"
+[ -n "${HELPER_NAME:-}" ] || HELPER_NAME="test_helper"
 bats_load_library "$HELPER_NAME"
 
 @test "calling a loaded helper" {

test/fixtures/load/load.bats

@@ -1,4 +1,4 @@
-[ -n "$HELPER_NAME" ] || HELPER_NAME="test_helper"
+[ -n "${HELPER_NAME:-}" ] || HELPER_NAME="test_helper"
 load "$HELPER_NAME"
 
 @test "calling a loaded helper" {

test/fixtures/parallel/must_not_parallelize_within_file.bats

@@ -1,13 +1,13 @@
 setup_file() {
     export FILE_MARKER
     FILE_MARKER=$(mktemp "${BATS_RUN_TMPDIR}/file_marker.XXXXXX")
-    if [[ -n "${DISABLE_IN_SETUP_FILE_FUNCTION}" ]]; then
+    if [[ -n "${DISABLE_IN_SETUP_FILE_FUNCTION:-}" ]]; then
         export BATS_NO_PARALLELIZE_WITHIN_FILE=true
         echo "setup_file() sets BATS_NO_PARALLELIZE_WITHIN_FILE=true" >&2
     fi
 }
 
-if [[ -n "${DISABLE_OUTSIDE_ALL_FUNCTIONS}" ]]; then
+if [[ -n "${DISABLE_OUTSIDE_ALL_FUNCTIONS:-}" ]]; then
     export BATS_NO_PARALLELIZE_WITHIN_FILE=true
     echo "File sets BATS_NO_PARALLELIZE_WITHIN_FILE=true" >&2
 fi
@@ -17,7 +17,7 @@ teardown_file() {
 }
 
 setup() {
-    if [[ -n "${DISABLE_IN_SETUP_FUNCTION}" ]]; then
+    if [[ -n "${DISABLE_IN_SETUP_FUNCTION:-}" ]]; then
         export BATS_NO_PARALLELIZE_WITHIN_FILE=true
         echo "setup() sets BATS_NO_PARALLELIZE_WITHIN_FILE=true" >&3
     fi
@@ -29,7 +29,7 @@ teardown() {
 }
 
 @test "test 1" {
-    if [[ -n "${DISABLE_IN_TEST_FUNCTION}" ]]; then
+    if [[ -n "${DISABLE_IN_TEST_FUNCTION:-}" ]]; then
         export BATS_NO_PARALLELIZE_WITHIN_FILE=true
         echo "Test function sets BATS_NO_PARALLELIZE_WITHIN_FILE=true" >&3
     fi

test/fixtures/suite/multiple/b.bats

@@ -3,5 +3,5 @@
 }
 
 @test "quasi-truth" {
-  [ -z "$FLUNK" ]
+  [ -z "${FLUNK:-}" ]
 }

test/formatter.bats

@@ -64,7 +64,7 @@ setup() {
     # the replay should be possible without errors
     bash -u "$formatter" >/dev/null <<EOF
 1..1
-suite "$BATS_FIXTURE_ROOT/failing.bats"
+suite "$FIXTURE_ROOT/failing.bats"
 # output from setup_file
 begin 1 test_a_failing_test
 # fd3 output from test

test/test_helper.bash

@@ -13,7 +13,9 @@ fixtures() {
 }
 
 filter_control_sequences() {
-  "$@" | sed $'s,\x1b\\[[0-9;]*[a-zA-Z],,g'
+  local status=0
+  "$@" | sed $'s,\x1b\\[[0-9;]*[a-zA-Z],,g' || status=$?
+  return "$status"
 }
 
 if ! command -v tput >/dev/null; then