From 2e12b5812647a97a9ed8b7e6aa48fbe7725491aa Mon Sep 17 00:00:00 2001 From: John Ericson Date: Tue, 28 May 2024 10:10:02 -0400 Subject: [PATCH] Shellcheck some test scripts Progress on #10795 --- .gitignore | 2 +- maintainers/flake-module.nix | 15 +------ mk/common-test.sh | 3 +- .../build-remote-content-addressed-fixed.sh | 2 + ...build-remote-content-addressed-floating.sh | 2 + .../build-remote-input-addressed.sh | 12 ++--- .../build-remote-trustless-after.sh | 9 +++- .../build-remote-trustless-should-fail-0.sh | 6 +++ .../build-remote-trustless-should-pass-0.sh | 2 + .../build-remote-trustless-should-pass-1.sh | 2 + .../build-remote-trustless-should-pass-2.sh | 2 + .../build-remote-trustless-should-pass-3.sh | 2 + tests/functional/build-remote-trustless.sh | 14 ++++-- .../build-remote-with-mounted-ssh-ng.sh | 14 +++--- tests/functional/build-remote.sh | 44 ++++++++++--------- tests/functional/common.sh | 8 ++-- tests/functional/common/init.sh | 2 + tests/functional/common/subst-vars.sh.in | 15 +++++++ ...-functions.sh.in => vars-and-functions.sh} | 19 +++----- tests/functional/local.mk | 2 +- 20 files changed, 106 insertions(+), 71 deletions(-) create mode 100644 tests/functional/common/subst-vars.sh.in rename tests/functional/common/{vars-and-functions.sh.in => vars-and-functions.sh} (96%) diff --git a/.gitignore b/.gitignore index 52aaec23f..28f853715 100644 --- a/.gitignore +++ b/.gitignore @@ -92,7 +92,7 @@ perl/Makefile.config # /tests/functional/ /tests/functional/test-tmp -/tests/functional/common/vars-and-functions.sh +/tests/functional/common/subst-vars.sh /tests/functional/result* /tests/functional/restricted-innocent /tests/functional/shell diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix index 1f19c673a..bc5dde9aa 100644 --- a/maintainers/flake-module.nix +++ b/maintainers/flake-module.nix @@ -514,18 +514,6 @@ ''^tests/functional/brotli\.sh$'' ''^tests/functional/build-delete\.sh$'' ''^tests/functional/build-dry\.sh$'' - ''^tests/functional/build-remote-content-addressed-fixed\.sh$'' - ''^tests/functional/build-remote-content-addressed-floating\.sh$'' - ''^tests/functional/build-remote-input-addressed\.sh$'' - ''^tests/functional/build-remote-trustless-after\.sh$'' - ''^tests/functional/build-remote-trustless-should-fail-0\.sh$'' - ''^tests/functional/build-remote-trustless-should-pass-0\.sh$'' - ''^tests/functional/build-remote-trustless-should-pass-1\.sh$'' - ''^tests/functional/build-remote-trustless-should-pass-2\.sh$'' - ''^tests/functional/build-remote-trustless-should-pass-3\.sh$'' - ''^tests/functional/build-remote-trustless\.sh$'' - ''^tests/functional/build-remote-with-mounted-ssh-ng\.sh$'' - ''^tests/functional/build-remote\.sh$'' ''^tests/functional/build\.sh$'' ''^tests/functional/ca/build-cache\.sh$'' ''^tests/functional/ca/build-dry\.sh$'' @@ -554,8 +542,7 @@ ''^tests/functional/check-reqs\.sh$'' ''^tests/functional/check\.sh$'' ''^tests/functional/chroot-store\.sh$'' - ''^tests/functional/common\.sh$'' - ''^tests/functional/common/init\.sh$'' + ''^tests/functional/common/vars-and-functions\.sh$'' ''^tests/functional/completions\.sh$'' ''^tests/functional/compression-levels\.sh$'' ''^tests/functional/compute-levels\.sh$'' diff --git a/mk/common-test.sh b/mk/common-test.sh index de24b6fcc..c80abd381 100644 --- a/mk/common-test.sh +++ b/mk/common-test.sh @@ -3,8 +3,7 @@ # Remove overall test dir (at most one of the two should match) and # remove file extension. -# shellcheck disable=SC2154 -test_name=$(echo -n "$test" | sed \ +test_name=$(echo -n "${test?must be defined by caller (test runner)}" | sed \ -e "s|^tests/unit/[^/]*/data/||" \ -e "s|^tests/functional/||" \ -e "s|\.sh$||" \ diff --git a/tests/functional/build-remote-content-addressed-fixed.sh b/tests/functional/build-remote-content-addressed-fixed.sh index ae7441591..61a1f4a46 100644 --- a/tests/functional/build-remote-content-addressed-fixed.sh +++ b/tests/functional/build-remote-content-addressed-fixed.sh @@ -1,3 +1,5 @@ +#!/usr/bin/env bash + source common.sh file=build-hook-ca-fixed.nix diff --git a/tests/functional/build-remote-content-addressed-floating.sh b/tests/functional/build-remote-content-addressed-floating.sh index e83b42b41..33d667f92 100644 --- a/tests/functional/build-remote-content-addressed-floating.sh +++ b/tests/functional/build-remote-content-addressed-floating.sh @@ -1,3 +1,5 @@ +#!/usr/bin/env bash + source common.sh file=build-hook-ca-floating.nix diff --git a/tests/functional/build-remote-input-addressed.sh b/tests/functional/build-remote-input-addressed.sh index 49d15c389..986692dbc 100644 --- a/tests/functional/build-remote-input-addressed.sh +++ b/tests/functional/build-remote-input-addressed.sh @@ -1,3 +1,5 @@ +#!/usr/bin/env bash + source common.sh file=build-hook.nix @@ -11,17 +13,17 @@ registerBuildHook () { # Dummy post-build-hook just to ensure that it's executed correctly. # (we can't reuse the one from `$PWD/push-to-store.sh` because of # https://github.com/NixOS/nix/issues/4341) - cat < $TEST_ROOT/post-build-hook.sh + cat < "$TEST_ROOT/post-build-hook.sh" #!/bin/sh echo "Post hook ran successfully" # Add an empty line to a counter file, just to check that this hook ran properly echo "" >> $TEST_ROOT/post-hook-counter EOF - chmod +x $TEST_ROOT/post-build-hook.sh - rm -f $TEST_ROOT/post-hook-counter + chmod +x "$TEST_ROOT/post-build-hook.sh" + rm -f "$TEST_ROOT/post-hook-counter" - echo "post-build-hook = $TEST_ROOT/post-build-hook.sh" >> $NIX_CONF_DIR/nix.conf + echo "post-build-hook = $TEST_ROOT/post-build-hook.sh" >> "$NIX_CONF_DIR/nix.conf" } registerBuildHook @@ -30,4 +32,4 @@ source build-remote.sh # `build-hook.nix` has four derivations to build, and the hook runs twice for # each derivation (once on the builder and once on the host), so the counter # should contain eight lines now -[[ $(cat $TEST_ROOT/post-hook-counter | wc -l) -eq 8 ]] +[[ $(wc -l < "$TEST_ROOT/post-hook-counter") -eq 8 ]] diff --git a/tests/functional/build-remote-trustless-after.sh b/tests/functional/build-remote-trustless-after.sh index 19f59e6ae..2fcdbf10a 100644 --- a/tests/functional/build-remote-trustless-after.sh +++ b/tests/functional/build-remote-trustless-after.sh @@ -1,2 +1,7 @@ -outPath=$(readlink -f $TEST_ROOT/result) -grep 'FOO BAR BAZ' ${remoteDir}/${outPath} +# shellcheck shell=bash + +# Variables must be defined by caller, so +# shellcheck disable=SC2154 + +outPath=$(readlink -f "$TEST_ROOT/result") +grep 'FOO BAR BAZ' "${remoteDir}/${outPath}" diff --git a/tests/functional/build-remote-trustless-should-fail-0.sh b/tests/functional/build-remote-trustless-should-fail-0.sh index 3d4a4b097..269f7f112 100644 --- a/tests/functional/build-remote-trustless-should-fail-0.sh +++ b/tests/functional/build-remote-trustless-should-fail-0.sh @@ -1,3 +1,5 @@ +#!/usr/bin/env bash + source common.sh enableFeatures "daemon-trust-override" @@ -22,8 +24,12 @@ nix-build build-hook.nix -A passthru.input2 \ # copy our already-build `input2` to the remote store. That store object # is input-addressed, so this will fail. +# For script below +# shellcheck disable=SC2034 file=build-hook.nix +# shellcheck disable=SC2034 prog=$(readlink -e ./nix-daemon-untrusting.sh) +# shellcheck disable=SC2034 proto=ssh-ng expectStderr 1 source build-remote-trustless.sh \ diff --git a/tests/functional/build-remote-trustless-should-pass-0.sh b/tests/functional/build-remote-trustless-should-pass-0.sh index 2a7ebd8c6..b81060907 100644 --- a/tests/functional/build-remote-trustless-should-pass-0.sh +++ b/tests/functional/build-remote-trustless-should-pass-0.sh @@ -1,3 +1,5 @@ +#!/usr/bin/env bash + source common.sh # Remote trusts us diff --git a/tests/functional/build-remote-trustless-should-pass-1.sh b/tests/functional/build-remote-trustless-should-pass-1.sh index 516bdf092..b8dc038bf 100644 --- a/tests/functional/build-remote-trustless-should-pass-1.sh +++ b/tests/functional/build-remote-trustless-should-pass-1.sh @@ -1,3 +1,5 @@ +#!/usr/bin/env bash + source common.sh # Remote trusts us diff --git a/tests/functional/build-remote-trustless-should-pass-2.sh b/tests/functional/build-remote-trustless-should-pass-2.sh index b769a88f0..ba5d1ff7a 100644 --- a/tests/functional/build-remote-trustless-should-pass-2.sh +++ b/tests/functional/build-remote-trustless-should-pass-2.sh @@ -1,3 +1,5 @@ +#!/usr/bin/env bash + source common.sh enableFeatures "daemon-trust-override" diff --git a/tests/functional/build-remote-trustless-should-pass-3.sh b/tests/functional/build-remote-trustless-should-pass-3.sh index 40f81da5a..187b89948 100644 --- a/tests/functional/build-remote-trustless-should-pass-3.sh +++ b/tests/functional/build-remote-trustless-should-pass-3.sh @@ -1,3 +1,5 @@ +#!/usr/bin/env bash + source common.sh enableFeatures "daemon-trust-override" diff --git a/tests/functional/build-remote-trustless.sh b/tests/functional/build-remote-trustless.sh index 81e5253bf..c498d46c3 100644 --- a/tests/functional/build-remote-trustless.sh +++ b/tests/functional/build-remote-trustless.sh @@ -1,5 +1,11 @@ +# shellcheck shell=bash + +# All variables should be defined externally by the scripts that source +# this, `set -u` will catch any that are forgotten. +# shellcheck disable=SC2154 + requireSandboxSupport -[[ $busybox =~ busybox ]] || skipTest "no busybox" +[[ "$busybox" =~ busybox ]] || skipTest "no busybox" unset NIX_STORE_DIR unset NIX_STATE_DIR @@ -8,7 +14,7 @@ remoteDir=$TEST_ROOT/remote # Note: ssh{-ng}://localhost bypasses ssh. See tests/functional/build-remote.sh for # more details. -nix-build $file -o $TEST_ROOT/result --max-jobs 0 \ - --arg busybox $busybox \ - --store $TEST_ROOT/local \ +nix-build "$file" -o "$TEST_ROOT/result" --max-jobs 0 \ + --arg busybox "$busybox" \ + --store "$TEST_ROOT/local" \ --builders "$proto://localhost?remote-program=$prog&remote-store=${remoteDir}%3Fsystem-features=foo%20bar%20baz - - 1 1 foo,bar,baz" diff --git a/tests/functional/build-remote-with-mounted-ssh-ng.sh b/tests/functional/build-remote-with-mounted-ssh-ng.sh index 443acb6ca..e2627af39 100644 --- a/tests/functional/build-remote-with-mounted-ssh-ng.sh +++ b/tests/functional/build-remote-with-mounted-ssh-ng.sh @@ -1,3 +1,5 @@ +#!/usr/bin/env bash + source common.sh requireSandboxSupport @@ -6,17 +8,17 @@ requireSandboxSupport enableFeatures mounted-ssh-store nix build -Lvf simple.nix \ - --arg busybox $busybox \ - --out-link $TEST_ROOT/result-from-remote \ + --arg busybox "$busybox" \ + --out-link "$TEST_ROOT/result-from-remote" \ --store mounted-ssh-ng://localhost nix build -Lvf simple.nix \ - --arg busybox $busybox \ - --out-link $TEST_ROOT/result-from-remote-new-cli \ + --arg busybox "$busybox" \ + --out-link "$TEST_ROOT/result-from-remote-new-cli" \ --store 'mounted-ssh-ng://localhost?remote-program=nix daemon' # This verifies that the out link was actually created and valid. The ability # to create out links (permanent gc roots) is the distinguishing feature of # the mounted-ssh-ng store. -cat $TEST_ROOT/result-from-remote/hello | grepQuiet 'Hello World!' -cat $TEST_ROOT/result-from-remote-new-cli/hello | grepQuiet 'Hello World!' +grepQuiet 'Hello World!' < "$TEST_ROOT/result-from-remote/hello" +grepQuiet 'Hello World!' < "$TEST_ROOT/result-from-remote-new-cli/hello" diff --git a/tests/functional/build-remote.sh b/tests/functional/build-remote.sh index d2a2132c1..1a5334577 100644 --- a/tests/functional/build-remote.sh +++ b/tests/functional/build-remote.sh @@ -1,5 +1,9 @@ +# shellcheck shell=bash + +: "${file?must be defined by caller (remote building test case using this)}" + requireSandboxSupport -[[ $busybox =~ busybox ]] || skipTest "no busybox" +[[ "${busybox-}" =~ busybox ]] || skipTest "no busybox" # Avoid store dir being inside sandbox build-dir unset NIX_STORE_DIR @@ -15,50 +19,50 @@ fi builders=( # system-features will automatically be added to the outer URL, but not inner # remote-store URL. - "ssh://localhost?remote-store=$TEST_ROOT/machine1?system-features=$(join_by "%20" foo ${EXTRA_SYSTEM_FEATURES[@]}) - - 1 1 $(join_by "," foo ${EXTRA_SYSTEM_FEATURES[@]})" - "$TEST_ROOT/machine2 - - 1 1 $(join_by "," bar ${EXTRA_SYSTEM_FEATURES[@]})" - "ssh-ng://localhost?remote-store=$TEST_ROOT/machine3?system-features=$(join_by "%20" baz ${EXTRA_SYSTEM_FEATURES[@]}) - - 1 1 $(join_by "," baz ${EXTRA_SYSTEM_FEATURES[@]})" + "ssh://localhost?remote-store=$TEST_ROOT/machine1?system-features=$(join_by "%20" foo "${EXTRA_SYSTEM_FEATURES[@]}") - - 1 1 $(join_by "," foo "${EXTRA_SYSTEM_FEATURES[@]}")" + "$TEST_ROOT/machine2 - - 1 1 $(join_by "," bar "${EXTRA_SYSTEM_FEATURES[@]}")" + "ssh-ng://localhost?remote-store=$TEST_ROOT/machine3?system-features=$(join_by "%20" baz "${EXTRA_SYSTEM_FEATURES[@]}") - - 1 1 $(join_by "," baz "${EXTRA_SYSTEM_FEATURES[@]}")" ) -chmod -R +w $TEST_ROOT/machine* || true -rm -rf $TEST_ROOT/machine* || true +chmod -R +w "$TEST_ROOT/machine"* || true +rm -rf "$TEST_ROOT/machine"* || true # Note: ssh://localhost bypasses ssh, directly invoking nix-store as a # child process. This allows us to test LegacySSHStore::buildDerivation(). # ssh-ng://... likewise allows us to test RemoteStore::buildDerivation(). -nix build -L -v -f $file -o $TEST_ROOT/result --max-jobs 0 \ - --arg busybox $busybox \ - --store $TEST_ROOT/machine0 \ +nix build -L -v -f "$file" -o "$TEST_ROOT/result" --max-jobs 0 \ + --arg busybox "$busybox" \ + --store "$TEST_ROOT/machine0" \ --builders "$(join_by '; ' "${builders[@]}")" -outPath=$(readlink -f $TEST_ROOT/result) +outPath=$(readlink -f "$TEST_ROOT/result") -grep 'FOO BAR BAZ' $TEST_ROOT/machine0/$outPath +grep 'FOO BAR BAZ' "$TEST_ROOT/machine0/$outPath" -testPrintOutPath=$(nix build -L -v -f $file --no-link --print-out-paths --max-jobs 0 \ - --arg busybox $busybox \ - --store $TEST_ROOT/machine0 \ +testPrintOutPath=$(nix build -L -v -f "$file" --no-link --print-out-paths --max-jobs 0 \ + --arg busybox "$busybox" \ + --store "$TEST_ROOT/machine0" \ --builders "$(join_by '; ' "${builders[@]}")" ) [[ $testPrintOutPath =~ store.*build-remote ]] # Ensure that input1 was built on store1 due to the required feature. -output=$(nix path-info --store $TEST_ROOT/machine1 --all) +output=$(nix path-info --store "$TEST_ROOT/machine1" --all) echo "$output" | grepQuiet builder-build-remote-input-1.sh echo "$output" | grepQuietInverse builder-build-remote-input-2.sh echo "$output" | grepQuietInverse builder-build-remote-input-3.sh unset output # Ensure that input2 was built on store2 due to the required feature. -output=$(nix path-info --store $TEST_ROOT/machine2 --all) +output=$(nix path-info --store "$TEST_ROOT/machine2" --all) echo "$output" | grepQuietInverse builder-build-remote-input-1.sh echo "$output" | grepQuiet builder-build-remote-input-2.sh echo "$output" | grepQuietInverse builder-build-remote-input-3.sh unset output # Ensure that input3 was built on store3 due to the required feature. -output=$(nix path-info --store $TEST_ROOT/machine3 --all) +output=$(nix path-info --store "$TEST_ROOT/machine3" --all) echo "$output" | grepQuietInverse builder-build-remote-input-1.sh echo "$output" | grepQuietInverse builder-build-remote-input-2.sh echo "$output" | grepQuiet builder-build-remote-input-3.sh @@ -66,7 +70,7 @@ unset output for i in input1 input3; do -nix log --store $TEST_ROOT/machine0 --file "$file" --arg busybox $busybox passthru."$i" | grep hi-$i +nix log --store "$TEST_ROOT/machine0" --file "$file" --arg busybox "$busybox" "passthru.$i" | grep hi-$i done # Behavior of keep-failed @@ -74,9 +78,9 @@ out="$(nix-build 2>&1 failing.nix \ --no-out-link \ --builders "$(join_by '; ' "${builders[@]}")" \ --keep-failed \ - --store $TEST_ROOT/machine0 \ + --store "$TEST_ROOT/machine0" \ -j0 \ - --arg busybox $busybox)" || true + --arg busybox "$busybox")" || true [[ "$out" =~ .*"note: keeping build directory".* ]] diff --git a/tests/functional/common.sh b/tests/functional/common.sh index 4ec17b706..d038aaf59 100644 --- a/tests/functional/common.sh +++ b/tests/functional/common.sh @@ -1,13 +1,15 @@ +# shellcheck shell=bash + set -eu -o pipefail if [[ -z "${COMMON_SH_SOURCED-}" ]]; then COMMON_SH_SOURCED=1 -dir="$(readlink -f "$(dirname "${BASH_SOURCE[0]-$0}")")" +functionalTestsDir="$(readlink -f "$(dirname "${BASH_SOURCE[0]-$0}")")" -source "$dir"/common/vars-and-functions.sh -source "$dir"/common/init.sh +source "$functionalTestsDir/common/vars-and-functions.sh" +source "$functionalTestsDir/common/init.sh" if [[ -n "${NIX_DAEMON_PACKAGE:-}" ]]; then startDaemon diff --git a/tests/functional/common/init.sh b/tests/functional/common/init.sh index 74da12651..dda1ecd41 100755 --- a/tests/functional/common/init.sh +++ b/tests/functional/common/init.sh @@ -1,3 +1,5 @@ +# shellcheck shell=bash + test -n "$TEST_ROOT" # We would delete any daemon socket, so let's stop the daemon first. killDaemon diff --git a/tests/functional/common/subst-vars.sh.in b/tests/functional/common/subst-vars.sh.in new file mode 100644 index 000000000..4105e9f35 --- /dev/null +++ b/tests/functional/common/subst-vars.sh.in @@ -0,0 +1,15 @@ +# NOTE: instances of @variable@ are substituted as defined in /mk/templates.mk + +export PATH=@bindir@:$PATH +export coreutils=@coreutils@ +#lsof=@lsof@ + +export dot=@dot@ +export SHELL="@bash@" +export PAGER=cat +export busybox="@sandbox_shell@" + +export version=@PACKAGE_VERSION@ +export system=@system@ + +export BUILD_SHARED_LIBS=@BUILD_SHARED_LIBS@ diff --git a/tests/functional/common/vars-and-functions.sh.in b/tests/functional/common/vars-and-functions.sh similarity index 96% rename from tests/functional/common/vars-and-functions.sh.in rename to tests/functional/common/vars-and-functions.sh index cb1f0d566..ad5b29a94 100644 --- a/tests/functional/common/vars-and-functions.sh.in +++ b/tests/functional/common/vars-and-functions.sh @@ -8,6 +8,12 @@ COMMON_VARS_AND_FUNCTIONS_SH_SOURCED=1 set +x +commonDir="$(readlink -f "$(dirname "${BASH_SOURCE[0]-$0}")")" + +source "$commonDir/subst-vars.sh" +# Make sure shellcheck knows all these will be defined by the above generated snippet +: "${PATH?} ${coreutils?} ${dot?} ${SHELL?} ${PAGER?} ${busybox?} ${version?} ${system?} ${BUILD_SHARED_LIBS?}" + export TEST_ROOT=$(realpath ${TMPDIR:-/tmp}/nix-test)/${TEST_NAME:-default/tests\/functional//} export NIX_STORE_DIR if ! NIX_STORE_DIR=$(readlink -f $TEST_ROOT/store 2> /dev/null); then @@ -37,7 +43,6 @@ unset XDG_CONFIG_HOME unset XDG_CONFIG_DIRS unset XDG_CACHE_HOME -export PATH=@bindir@:$PATH if [[ -n "${NIX_CLIENT_PACKAGE:-}" ]]; then export PATH="$NIX_CLIENT_PACKAGE/bin":$PATH fi @@ -45,18 +50,6 @@ DAEMON_PATH="$PATH" if [[ -n "${NIX_DAEMON_PACKAGE:-}" ]]; then DAEMON_PATH="${NIX_DAEMON_PACKAGE}/bin:$DAEMON_PATH" fi -coreutils=@coreutils@ -lsof=@lsof@ - -export dot=@dot@ -export SHELL="@bash@" -export PAGER=cat -export busybox="@sandbox_shell@" - -export version=@PACKAGE_VERSION@ -export system=@system@ - -export BUILD_SHARED_LIBS=@BUILD_SHARED_LIBS@ export IMPURE_VAR1=foo export IMPURE_VAR2=bar diff --git a/tests/functional/local.mk b/tests/functional/local.mk index 18b920f7d..a94c5712c 100644 --- a/tests/functional/local.mk +++ b/tests/functional/local.mk @@ -153,7 +153,7 @@ $(d)/plugins.sh.test $(d)/plugins.sh.test-debug: \ install-tests += $(foreach x, $(nix_tests), $(d)/$(x)) test-clean-files := \ - $(d)/common/vars-and-functions.sh \ + $(d)/common/subst-vars.sh \ $(d)/config.nix clean-files += $(test-clean-files)