1
0
Fork 0
mirror of https://github.com/NixOS/nix synced 2025-07-12 19:05:08 +02:00

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>
This commit is contained in:
John Ericson 2021-12-09 15:26:46 +00:00
parent 0159dfad3f
commit c11836126b
51 changed files with 300 additions and 177 deletions

View file

@ -1,4 +1,4 @@
set -e
set -eu -o pipefail
if [[ -z "${COMMON_VARS_AND_FUNCTIONS_SH_SOURCED-}" ]]; then
@ -157,7 +157,7 @@ requireDaemonNewerThan () {
}
canUseSandbox() {
if [[ ! $_canUseSandbox ]]; then
if [[ ! ${_canUseSandbox-} ]]; then
echo "Sandboxing not supported, skipping this test..."
return 1
fi
@ -170,13 +170,38 @@ fail() {
exit 1
}
# Run a command failing if it didn't exit with the expected exit code.
#
# Has two advantages over the built-in `!`:
#
# 1. `!` conflates all non-0 codes. `expect` allows testing for an exact
# code.
#
# 2. `!` unexpectedly negates `set -e`, and cannot be used on individual
# pipeline stages with `set -o pipefail`. It only works on the entire
# pipeline, which is useless if we want, say, `nix ...` invocation to
# *fail*, but a grep on the error message it outputs to *succeed*.
expect() {
local expected res
expected="$1"
shift
"$@" || res="$?"
"$@" && res=0 || res="$?"
if [[ $res -ne $expected ]]; then
echo "Expected '$expected' but got '$res' while running '$*'"
echo "Expected '$expected' but got '$res' while running '${*@Q}'" >&2
return 1
fi
return 0
}
# Better than just doing `expect ... >&2` because the "Expected..."
# message below will *not* be redirected.
expectStderr() {
local expected res
expected="$1"
shift
"$@" 2>&1 && res=0 || res="$?"
if [[ $res -ne $expected ]]; then
echo "Expected '$expected' but got '$res' while running '${*@Q}'" >&2
return 1
fi
return 0
@ -191,7 +216,7 @@ needLocalStore() {
# Just to make it easy to find which tests should be fixed
buggyNeedLocalStore() {
needLocalStore
needLocalStore "$1"
}
enableFeatures() {
@ -210,6 +235,35 @@ onError() {
done
}
# `grep -v` doesn't work well for exit codes. We want `!(exist line l. l
# matches)`. It gives us `exist line l. !(l matches)`.
#
# `!` normally doesn't work well with `set -e`, but when we wrap in a
# function it *does*.
grepInverse() {
! grep "$@"
}
# A shorthand, `> /dev/null` is a bit noisy.
#
# `grep -q` would seem to do this, no function necessary, but it is a
# bad fit with pipes and `set -o pipefail`: `-q` will exit after the
# first match, and then subsequent writes will result in broken pipes.
#
# Note that reproducing the above is a bit tricky as it depends on
# non-deterministic properties such as the timing between the match and
# the closing of the pipe, the buffering of the pipe, and the speed of
# the producer into the pipe. But rest assured we've seen it happen in
# CI reliably.
grepQuiet() {
grep "$@" > /dev/null
}
# The previous two, combined
grepQuietInverse() {
! grep "$@" > /dev/null
}
trap onError ERR
fi # COMMON_VARS_AND_FUNCTIONS_SH_SOURCED