Fix issue 1106 - JUnit formatter crashes if test errors in setup_suite #1125
Conversation
| if [[ -z "${name:-}" ]]; then | ||
| # Can be called (after bats_tap_stream_plan) before anything else if the test fails in setup_suite | ||
| bats_tap_stream_suite "setup_suite" | ||
| bats_tap_stream_begin "$1" "$2" | ||
| fi |
There was a problem hiding this comment.
After some debugging, I found that the JUnit formatter got little in the way of information when setup_suite fails.
bats_tap_stream_plangets calledbats_tap_stream_not_okgets called with args1andsetup_suite
...which then lead to the "increment file_count" line failing because file_count hadn't been set to zero by then.
I initially tried initialising every variable that was (later) needed in here, before later refactoring that to just "faking" these two calls, as that initialised the same variables.
I guess it's possible that a "better" fix would be to alter how all formatters are driven in this "setup_suite failed" scenario but I haven't investigated any other formatters.
There was a problem hiding this comment.
I concur that it might be good to be more explicit in the communication about what happened. So far, the junit-formatter is the only one that stumbled (it did not test this case).
I think in the long run, we should probably have a suite of internal tap logs to replay that can be tested against all formatters. (Added that to the description of #541)
| class='' | ||
| name='' |
There was a problem hiding this comment.
I figure that, if we're now putting quite a lot of emphasis on "is it empty?", we ought to empty it after we think we're done, just in case we need to start everything all over again.
|
|
||
| @test "don't choke on setup_suite errors (issue #1106)" { | ||
| bats_require_minimum_version 1.5.0 | ||
| local stderr='' # silence shellcheck | ||
| reentrant_run -1 --separate-stderr bats --formatter junit "$FIXTURE_ROOT/../suite_setup_teardown/error_in_setup_suite/test.bats" | ||
| [ "${stderr}" == "" ] | ||
| [[ "${lines[2]}" == '<testsuite name="setup_suite" '*'>' ]] | ||
| [[ "${lines[3]}" == ' <testcase classname="setup_suite" '*'>' ]] | ||
| [[ "${lines[5]}" == *'call-to-undefined-command'* ]] | ||
| } |
There was a problem hiding this comment.
While I would describe my proposed changes to the formatter itself as "speculative", I am more confident in the changes to the unit-test.
- it's using a pre-existing test scenario (
suite_setup_teardown/error_in_setup_suite/test.bats) - it's following the practises employed in earlier tests
I am a puzzled that the previous test does [ "${stderr}" == "" ] instead of using [[ ... ]] - I'd be happy to change both to [[ ... ]] if that would be preferred.
| if [[ -z "${name:-}" ]]; then | ||
| # Can be called (after bats_tap_stream_plan) before anything else if the test fails in setup_suite | ||
| bats_tap_stream_suite "setup_suite" | ||
| bats_tap_stream_begin "$1" "$2" | ||
| fi |
There was a problem hiding this comment.
I concur that it might be good to be more explicit in the communication about what happened. So far, the junit-formatter is the only one that stumbled (it did not test this case).
I think in the long run, we should probably have a suite of internal tap logs to replay that can be tested against all formatters. (Added that to the description of #541)
9bc2b0a to
1af5d49
Compare
|
Thanks for your contribution. |
|
@martin-schulze-vireso I'm looking into why this test is failing on us when we update bats in nixpkgs to 1.13.0. I want to show my math below, so the TL;DR is that a Hoping to see if you think the failure case looks ~real before I open a report (I guess it may just a byproduct of how we package bats, or the way I'm holding bats to debug this)? Here's a run of just junit-formatter.bats: $ bats /nix/store/cf5a80j8hv5bm4ryyrvymj4bvfh7n81q-source/test/junit-formatter.bats --print-output-on-failure --trace --verbose-run --formatter tap
WARNING: Cannot write in /nix/store/cf5a80j8hv5bm4ryyrvymj4bvfh7n81q-source/test/.bats/run-logs. This run will not write a log!
1..11
ok 1 junit formatter with skipped test does not fail
ok 2 junit formatter: escapes xml special chars
ok 3 junit formatter: test suites
ok 4 junit formatter: test suites relative path
ok 5 junit formatter: files with the same name are distinguishable
ok 6 junit formatter as report formatter creates report.xml
ok 7 junit does not mark tests with FD 3 output as failed (issue #360)
ok 8 junit does not mark tests with FD 3 output in teardown_file as failed (issue #531)
ok 9 don't choke on setup_file errors
ok 10 junit outputs status of last completed test when a test is retried (issue #1149)
not ok 11 don't choke on setup_suite errors (issue #1106)
# (in test file /nix/store/cf5a80j8hv5bm4ryyrvymj4bvfh7n81q-source/test/junit-formatter.bats, line 179)
# `[ "${stderr}" == "" ]' failed
# $ [junit-formatter.bats, line 176]
# $ bats_require_minimum_version 1.5.0
# $ local stderr=''
# $ reentrant_run -1 --separate-stderr bats --formatter junit "$FIXTURE_ROOT/../suite_setup_teardown/error_in_setup_suite/test.bats"
# $$ [test_helper.bash, line 64]
# $$ local -a pre_command_args=()
# $$ [[ $1 == -* || $1 == ! ]]
# $$ pre_command_args+=("$1")
# $$ [[ "$1" == -- ]]
# $$ shift
# $$ [[ $1 == -* || $1 == ! ]]
# $$ pre_command_args+=("$1")
# $$ [[ "$1" == -- ]]
# $$ shift
# $$ [[ $1 == -* || $1 == ! ]]
# $$ pre_command_args+=(execute_with_unset_bats_vars)
# $$ run "${pre_command_args[@]}" "$@"
# <?xml version="1.0" encoding="UTF-8"?>
# <testsuites time="0">
# </testsuites>
# stderr:
# /nix/store/x5x5gfzmx8dssqhhvw8byrypnfbx92ak-bats-1.13.0/libexec/bats-core/bats-format-junit: line 226: file_count: unbound variable
# $ [junit-formatter.bats, line 179]
# $ [ "${stderr}" == "" ]
# Last output:
# <?xml version="1.0" encoding="UTF-8"?>
# <testsuites time="0">
# </testsuites>Looking at the change here and reasoning through the code suggests that this outcome means I confirmed this by manually invoking something similar to what the test itself does, but with xtrace on. I can attach a log if needed, but here's how I ran it and my best guess at what's ~enough context: $ env SHELLOPTS=xtrace bats /nix/store/cf5a80j8hv5bm4ryyrvymj4bvfh7n81q-source/test/fixtures/suite_setup_teardown/error_in_setup_suite/test.bats --formatter junit
...
+ setup_suite
+ bats_teardown_suite_trap
+ bats_run_teardown_suite
+ local bats_teardown_suite_status=0
+ trap bats_suite_exit_trap EXIT
+ bats_set_stacktrace_limit
+ BATS_STACK_TRACE_LIMIT=3
+ BATS_TEARDOWN_SUITE_COMPLETED=
+ teardown_suite
+ (( bats_teardown_suite_status == 0 ))
+ BATS_TEARDOWN_SUITE_COMPLETED=1
+ bats_suite_exit_trap
+ local print_bats_out=
+ [[ -z '' ]]
+ [[ -z '' ]]
+ printf 'not ok 1 setup_suite\n'
+ local stack_trace
+ bats_get_failure_stack_trace stack_trace
+ local stack_trace_var
+ [[ -n 1 ]]
+ stack_trace_var=BATS_DEBUG_LAST_STACK_TRACE
+ printf '%s\n' 'not ok 1 setup_suite'
+ case "$line" in
+ (( ++actual_number_of_tests ))
+ IFS=
+ read -r line
+ unset BATS_FORMATTER_TEST_DURATION BATS_FORMATTER_TEST_TIMEOUT
+ case "$line" in
+ (( ++index ))
+ scope=not_ok
+ [[ not ok 1 setup_suite =~ not ok ([0-9]+) (.*) ]]
+ not_ok_index=1
+ test_name=setup_suite
+ [[ not ok 1 setup_suite =~ not ok ([0-9]+) (.*) # timeout after ([0-9]+)s$ ]]
+ [[ setup_suite =~ in ([0-9]+)ms$ ]]
+ bats_tap_stream_not_ok 1 setup_suite
+ [[ -z shell ]]
+ test_exec_time=0
+ (( file_count += 1 ))
/nix/store/x5x5gfzmx8dssqhhvw8byrypnfbx92ak-bats-1.13.0/libexec/bats-core/bats-format-junit: line 226: file_count: unbound variable
++ finish_suite
++ flush_log
++ [[ -n '' ]]
++ _buffer_log=
++ _system_out_log=
++ test_result_state=
++ suite_header
...The line immediately after the This value doesn't surprise me (nix sets I confirmed the thesis by setting an empty $ env name= bats /nix/store/cf5a80j8hv5bm4ryyrvymj4bvfh7n81q-source/test/fixtures/suite_setup_teardown/error_in_setup_suite/test.bats --formatter junit
<?xml version="1.0" encoding="UTF-8"?>
<testsuites time="0">
<testsuite name="setup_suite" tests="1" failures="1" errors="0" skipped="0" time="0" timestamp="2025-11-23T20:55:38" hostname="8d8d141a">
<testcase classname="setup_suite" name="setup_suite" time="0">
<failure type="failure">(from function `setup_suite' in test file /nix/store/cf5a80j8hv5bm4ryyrvymj4bvfh7n81q-source/test/fixtures/suite_setup_teardown/error_in_setup_suite/setup_suite.bash, line 2)
`call-to-undefined-command' failed with status 127
/nix/store/cf5a80j8hv5bm4ryyrvymj4bvfh7n81q-source/test/fixtures/suite_setup_teardown/error_in_setup_suite/setup_suite.bash: line 2: call-to-undefined-command: command not found</failure>
</testcase>
</testsuite>
</testsuites>
$ env name= bats /nix/store/cf5a80j8hv5bm4ryyrvymj4bvfh7n81q-source/test/junit-formatter.bats --print-output-on-failure --trace --verbose-run --formatter tap
WARNING: Cannot write in /nix/store/cf5a80j8hv5bm4ryyrvymj4bvfh7n81q-source/test/.bats/run-logs. This run will not write a log!
1..11
ok 1 junit formatter with skipped test does not fail
ok 2 junit formatter: escapes xml special chars
ok 3 junit formatter: test suites
ok 4 junit formatter: test suites relative path
ok 5 junit formatter: files with the same name are distinguishable
ok 6 junit formatter as report formatter creates report.xml
ok 7 junit does not mark tests with FD 3 output as failed (issue #360)
ok 8 junit does not mark tests with FD 3 output in teardown_file as failed (issue #531)
ok 9 don't choke on setup_file errors
ok 10 junit outputs status of last completed test when a test is retried (issue #1149)
ok 11 don't choke on setup_suite errors (issue #1106)Is an exported |
Fixes issue #1106
If a bats test runs setup_suite.bash and errors, the JUnit formatter would crash out with an undefined variable error and fail to produce a useful JUnit report.
This PR: