Merge pull request #652 from martin-schulze-vireso/fix/issue_635_lack…

…ing_documentation_of_setup_suite

Fix lacking documentation of setup_suite

docs/CHANGELOG.md

@@ -25,12 +25,14 @@ The format is based on [Keep a Changelog][kac] and this project adheres to
   `--print-output-on-failure` (#631)
 * `# bats test_tags=<tag list>`/`# bats file_tags=<tag list>` and
   `--filter-tags <tag list>` for tagging tests for execution filters (#642)
+* warning BW03: inform about `setup_file` in wrong file (`.bats` instead of `setup_suite.bash`) (#652)
 
 #### Documentation
 
 * update gotcha about negated statements: Recommend using `run !` on Bats
   versions >=1.5.0 (#593)
 * add documentation for `bats_require_minimum_version` (#595)
+* improve documentation about `setup_file` (#652)
 
 ### Fixed
 

docs/source/warnings/BW03.rst

@@ -0,0 +1,15 @@
+BW03: `setup_suite` is visible to test file '<path>', but was not executed. It belongs into 'setup_suite.bash' to be picked up automatically.
+=============================================================================================================================================
+
+In contrast to the other setup functions, `setup_suite` must not be defined in `*.bats` files but in `setup_suite.bash`.
+When a file is executed and sees `setup_suite` defined but not run before the tests, this warning will be printed.
+
+How to fix
+----------
+
+The fix depends on your actual intention. There are basically two cases:
+
+1. You want a setup before all tests and accidentally put `setup_suite` into a test file instead of `setup_suite.bash`.
+   Simply move `setup_suite` (and `teardown_suite`!) into `setup_suite.bash`.
+2. You did not mean to run a setup before any test but need to defined a function named `setup_suite` in your test file.
+   In this case, you can silence this warning by assigning `BATS_SETUP_SUITE_COMPLETED='suppress BW03'`.

docs/source/warnings/index.rst

@@ -24,4 +24,5 @@ Currently, Bats emits the following warnings:
 .. toctree::
 
     BW01
-    BW02
+    BW02
+    BW03

docs/source/writing-tests.md

@@ -320,6 +320,13 @@ functions (`setup`, the test itself, `teardown`, `teardown_file`).
 Similarly, there is `setup_suite` (and `teardown_suite`) which run once before (and
 after) all tests of the test run.
 
+__Note:__ As `setup_suite` and `teardown_suite` are intended for all files in a suite,
+they must be defined in a separate `setup_suite.bash` file. Automatic discovery works
+by searching for `setup_suite.bash` in the folder of the first `*.bats` file of the suite.
+If this automatism does not work for your usecase, you can work around by specifying
+`--setup-suite-file` on the `bats` command. If you have a `setup_suite.bash`, it must define
+`setup_suite`! However, defining `teardown_suite` is optional.
+
 <!-- markdownlint-disable  MD033 -->
 <details>
   <summary>Example of setup/{,_file,_suite} (and teardown{,_file,_suite}) call order</summary>

lib/bats-core/warnings.bash

@@ -3,23 +3,22 @@
 # shellcheck source=lib/bats-core/tracing.bash
 source "$BATS_ROOT/lib/bats-core/tracing.bash"
 
-BATS_WARNING_SHORT_DESCS=(
-  # to start with 1
-  'PADDING'
-  # see issue #578 for context
-  "\`run\`'s command \`%s\` exited with code 127, indicating 'Command not found'. Use run's return code checks, e.g. \`run -127\`, to fix this message."
-  "%s requires at least BATS_VERSION=%s. Use \`bats_require_minimum_version %s\` to fix this message."
-)
-
 # generate a warning report for the parent call's call site
-bats_generate_warning() { # <warning number> [<printf args for warning string>...]
-  local warning_number="$1" padding="00"
+bats_generate_warning() { # <warning number> [--no-stacktrace] [<printf args for warning string>...]
+  local warning_number="${1-}" padding="00"
   shift
+  local no_stacktrace=
+  if [[ ${1-} == --no-stacktrace ]]; then
+    no_stacktrace=1
+    shift
+  fi
   if [[ $warning_number =~ [0-9]+ ]] && ((warning_number < ${#BATS_WARNING_SHORT_DESCS[@]} )); then
     {
         printf "BW%s: ${BATS_WARNING_SHORT_DESCS[$warning_number]}\n" "${padding:${#warning_number}}${warning_number}" "$@"
-        bats_capture_stack_trace
-        BATS_STACK_TRACE_PREFIX='      ' bats_print_stack_trace "${BATS_DEBUG_LAST_STACK_TRACE[@]}"
+        if [[ -z "$no_stacktrace" ]]; then
+          bats_capture_stack_trace
+          BATS_STACK_TRACE_PREFIX='      ' bats_print_stack_trace "${BATS_DEBUG_LAST_STACK_TRACE[@]}"
+        fi
     } >> "$BATS_WARNING_FILE" 2>&3
   else
     printf "Invalid Bats warning number '%s'. It must be an integer between 1 and %d." "$warning_number" "$((${#BATS_WARNING_SHORT_DESCS[@]} - 1))" >&2
@@ -33,3 +32,13 @@ bats_warn_minimum_guaranteed_version() { # <feature> <minimum required version>
     bats_generate_warning 2 "$1" "$2" "$2"
   fi
 }
+
+# put after functions to avoid line changes in tests when new ones get added
+BATS_WARNING_SHORT_DESCS=(
+  # to start with 1
+  'PADDING'
+  # see issue #578 for context
+  "\`run\`'s command \`%s\` exited with code 127, indicating 'Command not found'. Use run's return code checks, e.g. \`run -127\`, to fix this message."
+  "%s requires at least BATS_VERSION=%s. Use \`bats_require_minimum_version %s\` to fix this message."
+  "\`setup_suite\` is visible to test file '%s', but was not executed. It belongs into 'setup_suite.bash' to be picked up automatically."
+)

libexec/bats-core/bats-exec-file

@@ -154,6 +154,12 @@ bats_file_exit_trap() {
     rm -rf "$BATS_OUT"
     bats_exec_file_status=1
   fi
+
+  # setup_file not executed but defined in this test file? -> might be defined in the wrong file
+  if [[ -z "${BATS_SETUP_SUITE_COMPLETED-}" ]] && declare -F setup_suite >/dev/null; then
+    bats_generate_warning 3 --no-stacktrace "$BATS_TEST_FILENAME"
+  fi
+
   exit $bats_exec_file_status
 }
 

libexec/bats-core/bats-exec-suite

@@ -380,16 +380,13 @@ bats_teardown_suite_trap() {
   bats_suite_exit_trap
 }
 
-setup_suite() {
-  :
-}
-
 teardown_suite() {
   : 
 }
 
 trap bats_teardown_suite_trap EXIT
 
+BATS_OUT="$BATS_RUN_TMPDIR/suite.out"
 if [[ -n "$setup_suite_file" ]]; then
   setup_suite() {
     printf "%s does not define \`setup_suite()\`\n" "$setup_suite_file" >&2
@@ -403,15 +400,18 @@ if [[ -n "$setup_suite_file" ]]; then
 
   # shellcheck disable=SC1090
   source "$setup_suite_file"
+
+  set -eET
+  export BATS_SETUP_SUITE_COMPLETED=
+  setup_suite >>"$BATS_OUT" 2>&1
+  BATS_SETUP_SUITE_COMPLETED=1
+  set +ET
+else
+  # prevent exit trap from printing an error because of incomplete setup_suite, 
+  # when there was none to execute
+  BATS_SETUP_SUITE_COMPLETED=1
 fi
 
-BATS_OUT="$BATS_RUN_TMPDIR/suite.out"
-
-set -eET
-BATS_SETUP_SUITE_COMPLETED=
-setup_suite >>"$BATS_OUT" 2>&1
-BATS_SETUP_SUITE_COMPLETED=1
-set +ET
 
 if [[ "$num_jobs" -gt 1 ]] && [[ -z "$bats_no_parallelize_across_files" ]]; then
   # run files in parallel to get the maximum pool of parallel tasks

test/fixtures/warnings/BW03/define_setup_suite_in_wrong_file.bats

@@ -0,0 +1,7 @@
+setup_suite() {
+    :
+}
+
+@test test {
+    :
+}

test/fixtures/warnings/BW03/non_default_setup_suite.bash

@@ -0,0 +1,3 @@
+setup_suite() {
+    :
+}

test/fixtures/warnings/BW03/suppress_warning.bats

@@ -0,0 +1,10 @@
+setup_suite() {
+    :
+}
+
+# shellcheck disable=SC2034
+BATS_SETUP_SUITE_COMPLETED='suppress BW03'
+
+@test test {
+    :
+}

test/warnings.bats

@@ -52,7 +52,30 @@ setup() {
     [ "${lines[1]}" == "ok 1 Trigger BW02" ]
     [ "${lines[2]}" == "The following warnings were encountered during tests:" ]
     [ "${lines[3]}" == "BW02: Using flags on \`run\` requires at least BATS_VERSION=1.5.0. Use \`bats_require_minimum_version 1.5.0\` to fix this message." ]
-    [[ "${lines[4]}" == "      (from function \`bats_warn_minimum_guaranteed_version' in file ${RELATIVE_BATS_ROOT}lib/bats-core/warnings.bash, line 33,"* ]]
+    [[ "${lines[4]}" == "      (from function \`bats_warn_minimum_guaranteed_version' in file ${RELATIVE_BATS_ROOT}lib/bats-core/warnings.bash, line "* ]]
     [[ "${lines[5]}" == "       from function \`run' in file ${RELATIVE_BATS_ROOT}lib/bats-core/test_functions.bash, line"* ]]
     [ "${lines[6]}" ==  "       in test file $RELATIVE_FIXTURE_ROOT/BW02.bats, line 2)" ]
+}
+
+@test "BW03 is printed when a test file defines setup_suite and setup_suite is not defined" {
+    reentrant_run -0 bats "$FIXTURE_ROOT/BW03/define_setup_suite_in_wrong_file.bats"
+    [ "${lines[0]}" == "1..1" ]
+    [ "${lines[1]}" == "ok 1 test" ]
+    [ "${lines[2]}" == "The following warnings were encountered during tests:" ]
+    [ "${lines[3]}" == "BW03: \`setup_suite\` is visible to test file '${FIXTURE_ROOT}/BW03/define_setup_suite_in_wrong_file.bats', but was not executed. It belongs into 'setup_suite.bash' to be picked up automatically." ]
+    [ "${#lines[@]}" -eq 4 ]
+}
+
+@test "BW03 is not printed when a test file defines setup_suite but setup_suite was completed" {
+    reentrant_run -0 bats "$FIXTURE_ROOT/BW03/define_setup_suite_in_wrong_file.bats" --setup-suite-file "$FIXTURE_ROOT/BW03/non_default_setup_suite.bash"
+    [ "${lines[0]}" == "1..1" ]
+    [ "${lines[1]}" == "ok 1 test" ]
+    [ "${#lines[@]}" -eq 2 ]
+}
+
+@test "BW03 can be suppressed by setting BATS_SETUP_SUITE_COMPLETED" {
+    reentrant_run -0 bats "$FIXTURE_ROOT/BW03/suppress_warning.bats"
+    [ "${lines[0]}" == "1..1" ]
+    [ "${lines[1]}" == "ok 1 test" ]
+    [ "${#lines[@]}" -eq 2 ]
 }