From 8ffe191282303b0011a36de833887632f808bd2e Mon Sep 17 00:00:00 2001 From: Debarshi Ray Date: Sun, 5 Jul 2026 14:56:42 +0200 Subject: [PATCH] test/system: Simplify handling Bash's 'set -e' set by Bats 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 54a2ca1ead343c3db2c7443cc58b9d9e896d3c46 [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 https://github.com/containers/toolbox/pull/1816 --- test/system/libs/helpers.bash | 4 ++-- test/system/setup_suite.bash | 20 ++++++++++---------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/test/system/libs/helpers.bash b/test/system/libs/helpers.bash index e26d0bd6a..e47640655 100644 --- a/test/system/libs/helpers.bash +++ b/test/system/libs/helpers.bash @@ -116,11 +116,11 @@ function _pull_and_cache_distro_image() { local -i ret_val for ((j = 0; j < num_of_retries; j++)); do + ret_val=0 error_message="$( (skopeo --command-timeout 10m copy \ --dest-compress \ "docker://${image}" \ - "dir:${IMAGE_CACHE_DIR}/${image_archive}" >/dev/null) 2>&1)" - ret_val="$?" + "dir:${IMAGE_CACHE_DIR}/${image_archive}" >/dev/null) 2>&1)" || ret_val="$?" if [ "$ret_val" -eq 0 ]; then cached=true diff --git a/test/system/setup_suite.bash b/test/system/setup_suite.bash index 216be00a3..9e84439a5 100644 --- a/test/system/setup_suite.bash +++ b/test/system/setup_suite.bash @@ -42,29 +42,29 @@ setup_suite() { _setup_environment if echo "$TOOLBX_TEST_SYSTEM_TAGS" | grep "arch" >/dev/null 2>/dev/null; then - _pull_and_cache_distro_image arch latest || false + _pull_and_cache_distro_image arch latest fi if echo "$TOOLBX_TEST_SYSTEM_TAGS" | grep "fedora" >/dev/null 2>/dev/null; then # Cache the default image for the system - _pull_and_cache_distro_image "$system_id" "$system_version" || false + _pull_and_cache_distro_image "$system_id" "$system_version" # Cache all images that will be needed during the tests - _pull_and_cache_distro_image fedora 34 || false - _pull_and_cache_distro_image rhel 8.10 || false + _pull_and_cache_distro_image fedora 34 + _pull_and_cache_distro_image rhel 8.10 fi if echo "$TOOLBX_TEST_SYSTEM_TAGS" | grep "ubuntu" >/dev/null 2>/dev/null; then - _pull_and_cache_distro_image ubuntu 16.04 || false - _pull_and_cache_distro_image ubuntu 18.04 || false - _pull_and_cache_distro_image ubuntu 20.04 || false + _pull_and_cache_distro_image ubuntu 16.04 + _pull_and_cache_distro_image ubuntu 18.04 + _pull_and_cache_distro_image ubuntu 20.04 fi if echo "$TOOLBX_TEST_SYSTEM_TAGS" | grep "commands-options" >/dev/null 2>/dev/null; then - _pull_and_cache_distro_image busybox || false + _pull_and_cache_distro_image busybox # If run on Fedora Rawhide, cache 2 extra images (previous Fedora versions) if is_fedora_rawhide && (echo "$TOOLBX_TEST_SYSTEM_TAGS" | grep "fedora" >/dev/null 2>/dev/null); then - _pull_and_cache_distro_image fedora "$((system_version-1))" || false - _pull_and_cache_distro_image fedora "$((system_version-2))" || false + _pull_and_cache_distro_image fedora "$((system_version-1))" + _pull_and_cache_distro_image fedora "$((system_version-2))" fi _setup_docker_registry