Skip to content

Conversation

Peter-Darton-i2
Copy link

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:

  1. Adds a unit-test to test for the usecase.
  2. Attempts a fix for the JUnit formatter, catching the "we failed before we really got started" scenario and putting the blame on the setup_suite code.

@Peter-Darton-i2 Peter-Darton-i2 requested a review from a team as a code owner August 11, 2025 12:55
Comment on lines +219 to +223
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
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After some debugging, I found that the JUnit formatter got little in the way of information when setup_suite fails.

  1. bats_tap_stream_plan gets called
  2. bats_tap_stream_not_ok gets called with args 1 and setup_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.

Comment on lines +174 to +175
class=''
name=''
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +161 to +170

@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'* ]]
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I would describe my proposed changes to the formatter itself as "speculative", I am more confident in the changes to the unit-test.

  1. it's using a pre-existing test scenario (suite_setup_teardown/error_in_setup_suite/test.bats)
  2. 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant