From c11836126b5148b6796c2470404a0bdf25cdfbe3 Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Thu, 9 Dec 2021 15:26:46 +0000
Subject: Harden tests' bash

Use `set -u` and `set -o pipefail` to catch accidental mistakes and
failures more strongly.

 - `set -u` catches the use of undefined variables
 - `set -o pipefail` catches failures (like `set -e`) earlier in the
   pipeline.

This makes the tests a bit more robust. It is nice to read code not
worrying about these spurious success paths (via uncaught) errors
undermining the tests. Indeed, I caught some bugs doing this.

There are a few tests where we run a command that should fail, and then
search its output to make sure the failure message is one that we
expect. Before, since the `grep` was the last command in the pipeline
the exit code of those failing programs was silently ignored. Now with
`set -o pipefail` it won't be, and we have to do something so the
expected failure doesn't accidentally fail the test.

To do that we use `expect` and a new `expectStderr` to check for the
exact failing exit code. See the comments on each for why.

`grep -q` is replaced with `grepQuiet`, see the comments on that
function for why.

`grep -v` when we just want the exit code is replaced with `grepInverse,
see the comments on that function for why.

`grep -q -v` together is, surprise surprise, replaced with
`grepQuietInverse`, which is both combined.

Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
---
 tests/test-infra.sh | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 85 insertions(+)
 create mode 100644 tests/test-infra.sh

(limited to 'tests/test-infra.sh')

diff --git a/tests/test-infra.sh b/tests/test-infra.sh
new file mode 100644
index 000000000..54ae120e7
--- /dev/null
+++ b/tests/test-infra.sh
@@ -0,0 +1,85 @@
+# Test the functions for testing themselves!
+# Also test some assumptions on how bash works that they rely on.
+source common.sh
+
+# `true` should exit with 0
+expect 0 true
+
+# `false` should exit with 1
+expect 1 false
+
+# `expect` will fail when we get it wrong
+expect 1 expect 0 false
+
+noisyTrue () {
+    echo YAY! >&2
+    true
+}
+
+noisyFalse () {
+    echo NAY! >&2
+    false
+}
+
+# These should redirect standard error to standard output
+expectStderr 0 noisyTrue | grepQuiet YAY
+expectStderr 1 noisyFalse | grepQuiet NAY
+
+# `set -o pipefile` is enabled
+
+pipefailure () {
+    # shellcheck disable=SC2216
+    true | false | true
+}
+expect 1 pipefailure
+unset pipefailure
+
+pipefailure () {
+    # shellcheck disable=SC2216
+    false | true | true
+}
+expect 1 pipefailure
+unset pipefailure
+
+commandSubstitutionPipeFailure () {
+    # shellcheck disable=SC2216
+    res=$(set -eu -o pipefail; false | true | echo 0)
+}
+expect 1 commandSubstitutionPipeFailure
+
+# `set -u` is enabled
+
+# note (...), making function use subshell, as unbound variable errors
+# in the outer shell are *rightly* not recoverable.
+useUnbound () (
+    set -eu
+    # shellcheck disable=SC2154
+    echo "$thisVariableIsNotBound"
+)
+expect 1 useUnbound
+
+# ! alone unfortunately negates `set -e`, but it works in functions:
+# shellcheck disable=SC2251
+! true
+funBang () {
+    ! true
+}
+expect 1 funBang
+unset funBang
+
+# `grep -v -q` is not what we want for exit codes, but `grepInverse` is
+# Avoid `grep -v -q`. The following line proves the point, and if it fails,
+# we'll know that `grep` had a breaking change or `-v -q` may not be portable.
+{ echo foo; echo bar; } | grep -v -q foo
+{ echo foo; echo bar; } | expect 1 grepInverse foo
+
+# `grepQuiet` is quiet
+res=$(set -eu -o pipefail; echo foo | grepQuiet foo | wc -c)
+(( res == 0 ))
+unset res
+
+# `greqQietInverse` is both
+{ echo foo; echo bar; } | expect 1 grepQuietInverse foo
+res=$(set -eu -o pipefail; echo foo | expect 1 grepQuietInverse foo | wc -c)
+(( res == 0 ))
+unset res
-- 
cgit v1.2.3