test/system: Simplify handling Bash's 'set -e' set by Bats#1816
Conversation
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. This means that if the 'skopeo copy' used to download and
cache an OCI image in _pull_and_cache_distro_image() fails, the function
will return immediately without doing anything else, and it will be
impossible to try the 'skopeo copy' again.
So far, this was being solved by creating a situation where Bash ignores
the 'set -e' and doesn't exit [2]. Specifically, by assembling a list
of commands [3] separated by the '||' operator and ensuring that
_pull_and_cache_distro_image() wasn't the last. This ignored 'set -e'
for the whole function.
Instead of expecting callers of _pull_and_cache_distro_image() to
carefully use the function in alignment with its implementation, it will
be better to align the implementation with itself.
So far, a failure to download and cache an image led to:
not ok 1 setup_suite
# (from function `setup_suite' in test file
test/system/setup_suite.bash, line 45)
# `_pull_and_cache_distro_image arch latest || false' failed
# Failed to cache image quay.io/toolbx/arch-toolbox:latest to
/var/tmp/bats-run-KCijAa/suite/image-cache/arch-toolbox-latest
# time="2026-07-05T14:58:26+02:00" level=fatal msg="initializing
source docker://quay.io/toolbx/arch-toolbox:latest-foo: reading
manifest latest-foo in quay.io/toolbx/arch-toolbox: manifest
unknown"
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 136,
# from function `setup_suite' in test file
test/system/setup_suite.bash, line 45)
# `_pull_and_cache_distro_image arch latest' failed with status 2
# Failed to cache image quay.io/toolbx/arch-toolbox:latest to
/var/tmp/bats-run-E2i7Y7/suite/image-cache/arch-toolbox-latest
# time="2026-07-05T14:52:17+02:00" level=fatal msg="initializing
source docker://quay.io/toolbx/arch-toolbox:latest-foo: reading
manifest latest-foo in quay.io/toolbx/arch-toolbox: manifest
unknown"
The differences are minor. The backtrace will now show the exit code of
'skopeo copy' and the return site of _pull_and_cache_distro_image().
Fallout from 54a2ca1
[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] https://www.gnu.org/software/bash/manual/html_node/Lists.html
containers#1816
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the _pull_and_cache_distro_image helper function in test/system/libs/helpers.bash to correctly capture the exit status of the skopeo command substitution by using an OR operator (|| ret_val="$?") and initializing ret_val to 0. Additionally, it simplifies test/system/setup_suite.bash by removing the redundant || false suffixes from the _pull_and_cache_distro_image calls. There are no review comments, and I have no further feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
debarshiray
force-pushed
the
wip/rishi/test-system-simplify-set-e-ignore
branch
from
2c61968 to
8ffe191
Compare
We are debugging this in #1805 |
1 participant
No description provided.