]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
test: fix error handling with `set -e`
authorFrantisek Sumsal <frantisek@sumsal.cz>
Sat, 18 Sep 2021 19:44:38 +0000 (21:44 +0200)
committerFrantisek Sumsal <frantisek@sumsal.cz>
Sun, 19 Sep 2021 11:46:55 +0000 (13:46 +0200)
Unfortunately, when checking the return/exit code using &&, ||, if,
while, etc., `set -e` is disabled for all nested functions as well,
which leads to incorrectly ignored errors, *sigh*.

Example:

```
set -eu
set -o pipefail

task() {
    echo "task init"
    echo "this should fail"
    false
    nonexistentcommand
    echo "task end (we shouldn't be here)"
}

if ! task; then
    echo >&2 "The task failed"
    exit 1
else
    echo "The task passed"
fi
```

```
$ bash test.sh
task init
this should fail
test.sh: line 10: nonexistentcommand: command not found
task end (we shouldn't be here)
The task passed
$ echo $?
0
```

But without the `if`, everything works "as expected":

```
set -eu
set -o pipefail

task() {
    echo "task init"
    echo "this should fail"
    false
    nonexistentcommand
    echo "task end (we shouldn't be here)"
}

task
```

```
$ bash test.sh
task init
this should fail
$ echo $?
1
```

Wonderful.

test/TEST-64-UDEV-STORAGE/test.sh

index c4119cf9ecd0e446c76ce45f0fe39cb72bc04f09..bcf8d9bb6e7945a2f9a9c1656d76776ba774e357 100755 (executable)
@@ -29,13 +29,13 @@ _host_has_feature() {(
 
     case "${1:?}" in
         multipath)
-            command -v multipath && command -v multipathd
+            command -v multipath && command -v multipathd || return $?
             ;;
         lvm)
-            command -v lvm
+            command -v lvm || return $?
             ;;
         btrfs)
-            modprobe -nv btrfs && command -v mkfs.btrfs && command -v btrfs
+            modprobe -nv btrfs && command -v mkfs.btrfs && command -v btrfs || return $?
             ;;
         *)
             echo >&2 "ERROR: Unknown feature '$1'"
@@ -111,7 +111,17 @@ test_run() {
     for testcase in "${TESTCASES[@]}"; do
         _image_cleanup
         echo "------ $testcase: BEGIN ------"
-        { "$testcase" "$test_id"; ec=$?; } || :
+        # Note for my future frustrated self: `fun && xxx` (as wel as ||, if, while,
+        # until, etc.) _DISABLES_ the `set -e` behavior in _ALL_ nested function
+        # calls made from `fun()`, i.e. the function _CONTINUES_ even when a called
+        # command returned non-zero EC. That may unexpectedly hide failing commands
+        # if not handled properly. See: bash(1) man page, `set -e` section.
+        #
+        # So, be careful when adding clean up snippets in the testcase_*() functions -
+        # if the `test_run_one()` function isn't the last command, you have propagate
+        # the exit code correctly (e.g. `test_run_one() || return $?`, see below).
+        ec=0
+        "$testcase" "$test_id" || ec=$?
         case $ec in
             0)
                 passed+=("$testcase")
@@ -228,7 +238,7 @@ EOF
     KERNEL_APPEND="systemd.setenv=TEST_FUNCTION_NAME=${FUNCNAME[0]} ${USER_KERNEL_APPEND:-}"
     # Limit the number of VCPUs and set a timeout to make sure we trigger the issue
     QEMU_OPTIONS="${qemu_opts[*]} ${USER_QEMU_OPTIONS:-}"
-    QEMU_SMP=1 QEMU_TIMEOUT=60 test_run_one "${1:?}"
+    QEMU_SMP=1 QEMU_TIMEOUT=60 test_run_one "${1:?}" || return $?
 
     rm -f "${TESTDIR:?}"/namedpart*.img
 }
@@ -271,7 +281,7 @@ EOF
 
     KERNEL_APPEND="systemd.setenv=TEST_FUNCTION_NAME=${FUNCNAME[0]} ${USER_KERNEL_APPEND:-}"
     QEMU_OPTIONS="${qemu_opts[*]} ${USER_QEMU_OPTIONS:-}"
-    test_run_one "${1:?}"
+    test_run_one "${1:?}" || return $?
 
     rm -f "$partdisk"
 }
@@ -289,7 +299,7 @@ testcase_simultaneous_events() {
 
     KERNEL_APPEND="systemd.setenv=TEST_FUNCTION_NAME=${FUNCNAME[0]} ${USER_KERNEL_APPEND:-}"
     QEMU_OPTIONS="${qemu_opts[*]} ${USER_QEMU_OPTIONS:-}"
-    test_run_one "${1:?}"
+    test_run_one "${1:?}" || return $?
 
     rm -f "$partdisk"
 }
@@ -316,7 +326,7 @@ testcase_lvm_basic() {
 
     KERNEL_APPEND="systemd.setenv=TEST_FUNCTION_NAME=${FUNCNAME[0]} ${USER_KERNEL_APPEND:-}"
     QEMU_OPTIONS="${qemu_opts[*]} ${USER_QEMU_OPTIONS:-}"
-    test_run_one "${1:?}"
+    test_run_one "${1:?}" || return $?
 
     rm -f "${TESTDIR:?}"/lvmbasic*.img
 }
@@ -344,7 +354,7 @@ testcase_btrfs_basic() {
 
     KERNEL_APPEND="systemd.setenv=TEST_FUNCTION_NAME=${FUNCNAME[0]} ${USER_KERNEL_APPEND:-}"
     QEMU_OPTIONS="${qemu_opts[*]} ${USER_QEMU_OPTIONS:-}"
-    test_run_one "${1:?}"
+    test_run_one "${1:?}" || return $?
 
     rm -f "${TESTDIR:?}"/btrfsbasic*.img
 }