From c11836126b5148b6796c2470404a0bdf25cdfbe3 Mon Sep 17 00:00:00 2001 From: John Ericson 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 --- tests/linux-sandbox.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'tests/linux-sandbox.sh') diff --git a/tests/linux-sandbox.sh b/tests/linux-sandbox.sh index e62039567..5667000d9 100644 --- a/tests/linux-sandbox.sh +++ b/tests/linux-sandbox.sh @@ -35,8 +35,8 @@ nix-build dependencies.nix --no-out-link --check --sandbox-paths /nix/store nix-build check.nix -A nondeterministic --sandbox-paths /nix/store --no-out-link (! nix-build check.nix -A nondeterministic --sandbox-paths /nix/store --no-out-link --check -K 2> $TEST_ROOT/log) -if grep -q 'error: renaming' $TEST_ROOT/log; then false; fi -grep -q 'may not be deterministic' $TEST_ROOT/log +if grepQuiet 'error: renaming' $TEST_ROOT/log; then false; fi +grepQuiet 'may not be deterministic' $TEST_ROOT/log # Test that sandboxed builds cannot write to /etc easily (! nix-build -E 'with import ./config.nix; mkDerivation { name = "etc-write"; buildCommand = "echo > /etc/test"; }' --no-out-link --sandbox-paths /nix/store) -- cgit v1.2.3 From bfb9eb87fe98f96969188df9df866e15800bd55b Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 16 Mar 2023 16:00:20 -0400 Subject: Cleanup test skipping - Try not to put cryptic "99" in many places Factor out `exit 99` into `skipTest` function - Alows make sure skipping a test is done with a reason `skipTest` takes a mandatory argument - Separate pure conditionals vs side-effectful test skipping. "require daemon" already had this, but "sandbox support" did not. --- tests/linux-sandbox.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'tests/linux-sandbox.sh') diff --git a/tests/linux-sandbox.sh b/tests/linux-sandbox.sh index 5667000d9..5a2cf7abd 100644 --- a/tests/linux-sandbox.sh +++ b/tests/linux-sandbox.sh @@ -4,13 +4,13 @@ needLocalStore "the sandbox only runs on the builder side, so it makes no sense clearStore -if ! canUseSandbox; then exit 99; fi +requireSandboxSupport # Note: we need to bind-mount $SHELL into the chroot. Currently we # only support the case where $SHELL is in the Nix store, because # otherwise things get complicated (e.g. if it's in /bin, do we need # /lib as well?). -if [[ ! $SHELL =~ /nix/store ]]; then exit 99; fi +if [[ ! $SHELL =~ /nix/store ]]; then skipTest "Shell is not from Nix store"; fi chmod -R u+w $TEST_ROOT/store0 || true rm -rf $TEST_ROOT/store0 -- cgit v1.2.3