Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't let teardown_file errors eat setup_file errors #623

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions docs/CHANGELOG.md
Expand Up @@ -37,6 +37,9 @@ The format is based on [Keep a Changelog][kac] and this project adheres to
* 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)
* `teardown_file` errors don't swallow `setup_file` errors anymore, the behavior
is more like `teardown`'s now (only `return`/last command can trigger `teardown`
errors) (#623)

#### Documentation

Expand Down
20 changes: 16 additions & 4 deletions libexec/bats-core/bats-exec-file
Expand Up @@ -102,23 +102,35 @@ bats_run_setup_file() {
}

bats_run_teardown_file() {
local bats_teardown_file_status=0
# avoid running the therdown trap due to errors in teardown_file
trap 'bats_file_exit_trap' EXIT
# rely on bats_error_trap to catch failures
teardown_file >>"$BATS_OUT" 2>&1

BATS_TEARDOWN_FILE_COMPLETED=1
# rely on bats_error_trap to catch failures
teardown_file >>"$BATS_OUT" 2>&1 || bats_teardown_file_status=$?

if (( bats_teardown_file_status == 0 )); then
BATS_TEARDOWN_FILE_COMPLETED=1
elif [[ -n "${BATS_SETUP_FILE_COMPLETED:-}" ]]; then
BATS_DEBUG_LAST_STACK_TRACE_IS_VALID=1
BATS_ERROR_STATUS=$bats_teardown_file_status
return $BATS_ERROR_STATUS
fi
}

bats_file_teardown_trap() {
bats_run_teardown_file
bats_file_exit_trap
bats_file_exit_trap in-teardown_trap
}

# shellcheck source=lib/bats-core/common.bash
source "$BATS_ROOT/lib/bats-core/common.bash"

bats_file_exit_trap() {
local -r last_return_code=$?
if [[ ${1:-} != in-teardown_trap ]]; then
BATS_ERROR_STATUS=$last_return_code
fi
trap - ERR EXIT
local failure_reason
local -i failure_test_index=$(( BATS_FILE_FIRST_TEST_NUMBER_IN_SUITE + 1 ))
Expand Down
25 changes: 15 additions & 10 deletions test/file_setup_teardown.bats
Expand Up @@ -166,16 +166,12 @@ ok 2 must not see variable from first run" ]]
[ "${lines[3]}" == "# \`false' failed" ]
}

@test "halfway teardown_file errors are caught and reported" {
run bats "$FIXTURE_ROOT/teardown_file_halfway_error.bats"
echo "$output"
[[ $status -ne 0 ]]
[[ "${lines[0]}" == "1..1" ]]
[[ "${lines[1]}" == "ok 1 empty" ]]
[[ "${lines[2]}" == "not ok 2 teardown_file failed" ]]
[[ "${lines[3]}" == "# (from function \`teardown_file' in test file $RELATIVE_FIXTURE_ROOT/teardown_file_halfway_error.bats, line 3)" ]]
[[ "${lines[4]}" == "# \`false' failed" ]]
[[ "${lines[5]}" == "# bats warning: Executed 2 instead of expected 1 tests" ]] # for now this warning is expected
@test "halfway teardown_file errors are ignored" {
run -0 bats "$FIXTURE_ROOT/teardown_file_halfway_error.bats"
[ "${lines[0]}" == "1..1" ]
[ "${lines[1]}" == "ok 1 empty" ]
[ "${#lines[@]}" -eq 2 ]

}

@test "variables exported in setup_file are visible in tests" {
Expand All @@ -191,3 +187,12 @@ ok 2 must not see variable from first run" ]]
[ ! -f "$LOG" ] # setup_file must not have been executed!
[ "${lines[0]}" == '1..1' ] # but at least one test should have been run
}

@test "Failure in setup_file and teardown_file still prints error message" {
run ! bats "$FIXTURE_ROOT/error_in_setup_and_teardown_file.bats"
[ "${lines[0]}" == '1..1' ]
[ "${lines[1]}" == 'not ok 1 setup_file failed' ]
[ "${lines[2]}" == "# (from function \`setup_file' in test file $RELATIVE_FIXTURE_ROOT/error_in_setup_and_teardown_file.bats, line 2)" ]
[ "${lines[3]}" == "# \`false' failed" ]
[ "${#lines[@]}" -eq 4 ]
}
@@ -0,0 +1,11 @@
setup_file() {
false
}

teardown_file() {
false
}

@test dummy {
:
}