tests: add extra protection against unexpected exits

Many tests use `program ... && fail=1` to ensure expected
error situations are indicated.  However that would mask
an unexpected exit (like a crash).  Therefore explicitly
check the expected exit code.
Note where error messages are also verified, the extra
protection is not added.

* tests/init.sh (returns_): A new helper function to
check the return code of a command, and used
throughout the tests.
* cfg.mk (sc_prohibit_and_fail_1): Add a syntax check
to avoid new instances of this issue.

6 files changed, 11 insertions, 15 deletions

diff --git a/tests/rm/i-no-r.sh b/tests/rm/i-no-r.sh
index e7f7196aa..22819ab6f 100755
--- a/tests/rm/i-no-r.sh
+++ b/tests/rm/i-no-r.sh
@@ -26,7 +26,7 @@ echo y > y || framework_failure_
 
 
 # This must fail.
-rm -i dir < y > /dev/null 2>&1 && fail=1
+returns_ 1 rm -i dir < y > /dev/null 2>&1 || fail=1
 
 # The directory must remain.
 test -d dir || fail=1
diff --git a/tests/rm/no-give-up.sh b/tests/rm/no-give-up.sh
index b7b5b0eda..a5134d760 100755
--- a/tests/rm/no-give-up.sh
+++ b/tests/rm/no-give-up.sh
@@ -30,8 +30,8 @@ chmod go=x . || framework_failure_
 
 
 # This must fail, since '.' is not writable by $NON_ROOT_USERNAME.
-chroot --skip-chdir --user=$NON_ROOT_USERNAME / env PATH="$PATH" \
-  rm -rf d 2>/dev/null && fail=1
+returns_ 1 chroot --skip-chdir --user=$NON_ROOT_USERNAME / env PATH="$PATH" \
+  rm -rf d 2>/dev/null || fail=1
 
 # d must remain.
 test -d d || fail=1
diff --git a/tests/rm/r-4.sh b/tests/rm/r-4.sh
index de3da0cf7..91a8b58aa 100755
--- a/tests/rm/r-4.sh
+++ b/tests/rm/r-4.sh
@@ -43,8 +43,8 @@ rmtest 'd/..'    || fail=1
 rmtest 'd/../'   || fail=1
 
 
-# This test is too dangerous -- if there's a bug you're wiped out!
-# rm -fr / 2>/dev/null && fail=1
+# This test is handled more carefully in r-root.sh
+# returns_ 1 rm -fr / 2>/dev/null || fail=1
 
 test -f d/a || fail=1
 
diff --git a/tests/rm/r-root.sh b/tests/rm/r-root.sh
index 179edf6ff..e8fe714af 100755
--- a/tests/rm/r-root.sh
+++ b/tests/rm/r-root.sh
@@ -143,8 +143,7 @@ for opts in           \
   'rootlink2/'        \
   'rootlink3/'        ; do
 
-  exercise_rm_r_root $opts \
-    && fail=1
+  returns_ 1 exercise_rm_r_root $opts || fail=1
 
   # For some of the synonyms, the error diagnostic slightly differs from that
   # of the basic "/" case (see gnulib's fts_open' and ROOT_DEV_INO_WARN):
@@ -178,8 +177,7 @@ done
 # by setting the following variable.
 CU_TEST_SKIP_EXIT=1
 
-exercise_rm_r_root --preserve-root file1 '/' file2 \
-  && fail=1
+returns_ 1 exercise_rm_r_root --preserve-root file1 '/' file2 || fail=1
 
 unset CU_TEST_SKIP_EXIT
 
@@ -219,8 +217,7 @@ for file in      \
 
   test -d "$file" || continue   # if e.g. /etc does not exist.
 
-  exercise_rm_r_root --preserve-root "$file" \
-    && fail=1
+  returns_ 1 exercise_rm_r_root --preserve-root "$file" || fail=1
 
   grep "^rm: refusing to remove '\.' or '\.\.' directory: skipping" err \
     || fail=1
@@ -244,8 +241,7 @@ exercise_rm_r_root  --interactive=never --no-preserve-root '/' \
   || fail=1
 
 # The 'err' file should not contain the above error diagnostic.
-grep "^rm: it is dangerous to operate recursively on '/'" err \
-  && fail=1
+grep "^rm: it is dangerous to operate recursively on '/'" err && fail=1
 
 # Instead, rm(1) should have called the intercepted unlinkat() function,
 # i.e. the evidence file "x" should exist.
diff --git a/tests/rm/rm4.sh b/tests/rm/rm4.sh
index 28ff20bdf..e0f141fd4 100755
--- a/tests/rm/rm4.sh
+++ b/tests/rm/rm4.sh
@@ -24,7 +24,7 @@ mkdir dir || framework_failure_
 
 
 # This should fail.
-rm dir > /dev/null 2>&1 && fail=1
+returns_ 1 rm dir > /dev/null 2>&1 || fail=1
 
 test -d dir || fail=1
 
diff --git a/tests/rm/sunos-1.sh b/tests/rm/sunos-1.sh
index d2468217b..db4221f03 100755
--- a/tests/rm/sunos-1.sh
+++ b/tests/rm/sunos-1.sh
@@ -24,6 +24,6 @@
 . "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
 print_ver_ rm
 
-rm -r '' > /dev/null 2>&1 && fail=1
+returns_ 1 rm -r '' > /dev/null 2>&1 || fail=1
 
 Exit $fail