test/system: Don't use Bats' 'run' to only check if a command succeeded#1825
Merged
debarshiray merged 2 commits intoJul 25, 2026
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
The long options are easier to grep(1) for in the sources than their shorter aliases. Fallout from ecd1ced and 54a2ca1 containers#1825
debarshiray
force-pushed
the
wip/rishi/test-system-run-helper-is-not-necessary-to-merely-check
branch
from
July 25, 2026 00:01
084d171 to
de8ef82
Compare
Member
Author
|
Member
Author
|
recheck |
debarshiray
added a commit
to debarshiray/toolbox
that referenced
this pull request
Jul 25, 2026
Bats' 'run' helper is not necessary to merely check if a command
succeeded or not [1].
Bats sets Bash's 'set -e' [1,2] to fail and exit immediately if a
pipeline, list of commands or compound command returns with a non-zero
exit code. Therefore, directly invoking the command without 'run' is
enough to stop execution in case of an error.
Perhaps more importantly, until Bats 1.14.0 that came out four days ago,
'run --separate-stderr' didn't work inside setup_suite() because
BATS_TEST_TMPDIR wasn't defined [3].
So far, a failure led to:
not ok 1 setup_suite
# (from function `assert_success' in file
test/system/libs/bats-assert/src/assert.bash, line 114,
# from function `_pull_and_cache_distro_image' in file
test/system/libs/helpers.bash, line 110,
# from function `setup_suite' in test file
test/system/setup_suite.bash, line 52)
# `_pull_and_cache_distro_image fedora 34' failed
#
# -- command failed --
# status : 1
# output : mkdir: cannot create directory
‘/var/tmp/bats-run-KCijAa/suite/image-cache’: Permission denied
# --
#
Instead, from now on, it will be:
not ok 1 setup_suite
# (from function `_pull_and_cache_distro_image' in file
test/system/libs/helpers.bash, line 109,
# from function `setup_suite' in test file
test/system/setup_suite.bash, line 52)
# `_pull_and_cache_distro_image fedora 34' failed
# mkdir: cannot create directory
‘/var/tmp/bats-run-KCijAa/suite/image-cache’: Permission denied
Fallout from 8f6dead and
ecd1ced
[1] https://bats-core.readthedocs.io/en/stable/writing-tests.html
[2] https://www.gnu.org/software/bash/manual/html_node/The-Set-Builtin.html
[3] Bats commit 33ffbedd2760e51c
bats-core/bats-core@33ffbedd2760e51c
bats-core/bats-core#1118
containers#1825
debarshiray
added a commit
to debarshiray/toolbox
that referenced
this pull request
Jul 25, 2026
Bats' 'run' helper is not necessary to merely check if a command
succeeded or not [1].
Bats sets Bash's 'set -e' [1,2] to fail and exit immediately if a
pipeline, list of commands or compound command returns with a non-zero
exit code. Therefore, directly invoking the command without 'run' is
enough to stop execution in case of an error.
Perhaps more importantly, until Bats 1.14.0 that came out four days ago,
'run --separate-stderr' didn't work inside setup_suite() because
BATS_TEST_TMPDIR wasn't defined [3]. Hence, using 'run' in these
functions used in setup_suite() exposed it to the possibility that it
would be used in ways that don't work.
So far, a failure led to:
not ok 1 setup_suite
# (from function `assert_success' in file
test/system/libs/bats-assert/src/assert.bash, line 114,
# from function `_pull_and_cache_distro_image' in file
test/system/libs/helpers.bash, line 110,
# from function `setup_suite' in test file
test/system/setup_suite.bash, line 52)
# `_pull_and_cache_distro_image fedora 34' failed
#
# -- command failed --
# status : 1
# output : mkdir: cannot create directory
‘/var/tmp/bats-run-KCijAa/suite/image-cache’: Permission denied
# --
#
Instead, from now on, it will be:
not ok 1 setup_suite
# (from function `_pull_and_cache_distro_image' in file
test/system/libs/helpers.bash, line 109,
# from function `setup_suite' in test file
test/system/setup_suite.bash, line 52)
# `_pull_and_cache_distro_image fedora 34' failed
# mkdir: cannot create directory
‘/var/tmp/bats-run-KCijAa/suite/image-cache’: Permission denied
Fallout from 8f6dead and
ecd1ced
[1] https://bats-core.readthedocs.io/en/stable/writing-tests.html
[2] https://www.gnu.org/software/bash/manual/html_node/The-Set-Builtin.html
[3] Bats commit 33ffbedd2760e51c
bats-core/bats-core@33ffbedd2760e51c
bats-core/bats-core#1118
containers#1825
debarshiray
force-pushed
the
wip/rishi/test-system-run-helper-is-not-necessary-to-merely-check
branch
from
July 25, 2026 12:50
ce688ce to
9f52e10
Compare
Bats' 'run' helper is not necessary to merely check if a command
succeeded or not [1].
Bats sets Bash's 'set -e' [1,2] to fail and exit immediately if a
pipeline, list of commands or compound command returns with a non-zero
exit code. Therefore, directly invoking the command without 'run' is
enough to stop execution in case of an error.
Perhaps more importantly, until Bats 1.14.0 that came out four days ago,
'run --separate-stderr' didn't work inside setup_suite() because
BATS_TEST_TMPDIR wasn't defined [3]. Hence, using 'run' in these
functions used in setup_suite() exposed them to the possibility that it
would be used in ways that don't work.
So far, a failure led to:
not ok 1 setup_suite
# (from function `assert_success' in file
test/system/libs/bats-assert/src/assert.bash, line 114,
# from function `_pull_and_cache_distro_image' in file
test/system/libs/helpers.bash, line 110,
# from function `setup_suite' in test file
test/system/setup_suite.bash, line 52)
# `_pull_and_cache_distro_image fedora 34' failed
#
# -- command failed --
# status : 1
# output : mkdir: cannot create directory
‘/var/tmp/bats-run-KCijAa/suite/image-cache’: Permission denied
# --
#
Instead, from now on, it will be:
not ok 1 setup_suite
# (from function `_pull_and_cache_distro_image' in file
test/system/libs/helpers.bash, line 109,
# from function `setup_suite' in test file
test/system/setup_suite.bash, line 52)
# `_pull_and_cache_distro_image fedora 34' failed
# mkdir: cannot create directory
‘/var/tmp/bats-run-KCijAa/suite/image-cache’: Permission denied
Fallout from 8f6dead and
ecd1ced
[1] https://bats-core.readthedocs.io/en/stable/writing-tests.html
[2] https://www.gnu.org/software/bash/manual/html_node/The-Set-Builtin.html
[3] Bats commit 33ffbedd2760e51c
bats-core/bats-core@33ffbedd2760e51c
bats-core/bats-core#1118
containers#1825
debarshiray
force-pushed
the
wip/rishi/test-system-run-helper-is-not-necessary-to-merely-check
branch
from
July 25, 2026 17:08
9f52e10 to
8422277
Compare
debarshiray
deleted the
wip/rishi/test-system-run-helper-is-not-necessary-to-merely-check
branch
July 25, 2026 22:41
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bats'
runhelper is not necessary to merely check if a commandsucceeded or not [1].
Bats sets Bash's
set -e[1,2] to fail and exit immediately if apipeline, list of commands or compound command returns with a non-zero
exit code. Therefore, directly invoking the command without
runisenough to stop execution in case of an error.
[1] https://bats-core.readthedocs.io/en/stable/writing-tests.html
[2] https://www.gnu.org/software/bash/manual/html_node/The-Set-Builtin.html