Shellcheck some test scripts

Progress on #10795

.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

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$''

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$||" \

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

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

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 <<EOF > $TEST_ROOT/post-build-hook.sh
+    cat <<EOF > "$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 ]]

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}"

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 \

tests/functional/build-remote-trustless-should-pass-0.sh

@@ -1,3 +1,5 @@
+#!/usr/bin/env bash
+
 source common.sh
 
 # Remote trusts us

tests/functional/build-remote-trustless-should-pass-1.sh

@@ -1,3 +1,5 @@
+#!/usr/bin/env bash
+
 source common.sh
 
 # Remote trusts us

tests/functional/build-remote-trustless-should-pass-2.sh

@@ -1,3 +1,5 @@
+#!/usr/bin/env bash
+
 source common.sh
 
 enableFeatures "daemon-trust-override"

tests/functional/build-remote-trustless-should-pass-3.sh

@@ -1,3 +1,5 @@
+#!/usr/bin/env bash
+
 source common.sh
 
 enableFeatures "daemon-trust-override"

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"

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"

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".* ]]
 

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

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

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@

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

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)