From a75b082a284c09b63b52952f2d4a360a3cadfb71 Mon Sep 17 00:00:00 2001
From: Tim Van Baak <tim.vanbaak@gmail.com>
Date: Tue, 29 Oct 2024 21:33:28 -0700
Subject: [PATCH 1/3] Expand shellcheck coverage in functional tests

Ref NixOS/nix#10795
---
 maintainers/flake-module.nix      | 15 +-----
 tests/functional/build.sh         |  9 ++--
 tests/functional/check.sh         | 80 +++++++++++++++----------------
 tests/functional/eval.sh          | 12 ++---
 tests/functional/fetchurl.sh      | 64 ++++++++++++-------------
 tests/functional/flakes/common.sh | 18 ++++---
 tests/functional/gc.sh            | 40 ++++++++--------
 tests/functional/help.sh          |  1 +
 tests/functional/lang.sh          |  3 ++
 tests/functional/nar-access.sh    | 44 ++++++++---------
 tests/functional/shell.sh         | 24 +++++-----
 tests/functional/zstd.sh          | 12 ++---
 12 files changed, 159 insertions(+), 163 deletions(-)

diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix
index 78c36d6b6..225b0b300 100644
--- a/maintainers/flake-module.nix
+++ b/maintainers/flake-module.nix
@@ -7,7 +7,7 @@
 
   perSystem = { config, pkgs, ... }: {
 
-    # https://flake.parts/options/pre-commit-hooks-nix.html#options
+    # https://flake.parts/options/git-hooks-nix#options
     pre-commit.settings = {
       hooks = {
         clang-format = {
@@ -501,7 +501,6 @@
             ''^scripts/install-nix-from-closure\.sh$''
             ''^scripts/install-systemd-multi-user\.sh$''
             ''^src/nix/get-env\.sh$''
-            ''^tests/functional/build\.sh$''
             ''^tests/functional/ca/build-dry\.sh$''
             ''^tests/functional/ca/build-with-garbage-path\.sh$''
             ''^tests/functional/ca/common\.sh$''
@@ -517,7 +516,6 @@
             ''^tests/functional/ca/selfref-gc\.sh$''
             ''^tests/functional/ca/why-depends\.sh$''
             ''^tests/functional/characterisation-test-infra\.sh$''
-            ''^tests/functional/check\.sh$''
             ''^tests/functional/common/vars-and-functions\.sh$''
             ''^tests/functional/completions\.sh$''
             ''^tests/functional/compute-levels\.sh$''
@@ -534,7 +532,6 @@
             ''^tests/functional/dyn-drv/old-daemon-error-hack\.sh$''
             ''^tests/functional/dyn-drv/recursive-mod-json\.sh$''
             ''^tests/functional/eval-store\.sh$''
-            ''^tests/functional/eval\.sh$''
             ''^tests/functional/export-graph\.sh$''
             ''^tests/functional/export\.sh$''
             ''^tests/functional/extra-sandbox-profile\.sh$''
@@ -544,13 +541,11 @@
             ''^tests/functional/fetchGitSubmodules\.sh$''
             ''^tests/functional/fetchGitVerification\.sh$''
             ''^tests/functional/fetchMercurial\.sh$''
-            ''^tests/functional/fetchurl\.sh$''
             ''^tests/functional/fixed\.builder1\.sh$''
             ''^tests/functional/fixed\.builder2\.sh$''
             ''^tests/functional/fixed\.sh$''
             ''^tests/functional/flakes/absolute-paths\.sh$''
             ''^tests/functional/flakes/check\.sh$''
-            ''^tests/functional/flakes/common\.sh$''
             ''^tests/functional/flakes/config\.sh$''
             ''^tests/functional/flakes/develop\.sh$''
             ''^tests/functional/flakes/flakes\.sh$''
@@ -565,16 +560,12 @@
             ''^tests/functional/gc-concurrent\.sh$''
             ''^tests/functional/gc-concurrent2\.builder\.sh$''
             ''^tests/functional/gc-non-blocking\.sh$''
-            ''^tests/functional/gc\.sh$''
             ''^tests/functional/git-hashing/common\.sh$''
             ''^tests/functional/git-hashing/simple\.sh$''
             ''^tests/functional/hash-convert\.sh$''
-            ''^tests/functional/help\.sh$''
             ''^tests/functional/impure-derivations\.sh$''
-            ''^tests/functional/impure-env\.sh$''
             ''^tests/functional/impure-eval\.sh$''
             ''^tests/functional/install-darwin\.sh$''
-            ''^tests/functional/lang\.sh$''
             ''^tests/functional/legacy-ssh-store\.sh$''
             ''^tests/functional/linux-sandbox\.sh$''
             ''^tests/functional/local-overlay-store/add-lower-inner\.sh$''
@@ -603,7 +594,6 @@
             ''^tests/functional/logging\.sh$''
             ''^tests/functional/misc\.sh$''
             ''^tests/functional/multiple-outputs\.sh$''
-            ''^tests/functional/nar-access\.sh$''
             ''^tests/functional/nested-sandboxing\.sh$''
             ''^tests/functional/nested-sandboxing/command\.sh$''
             ''^tests/functional/nix-build\.sh$''
@@ -624,7 +614,6 @@
             ''^tests/functional/path-from-hash-part\.sh$''
             ''^tests/functional/path-info\.sh$''
             ''^tests/functional/placeholders\.sh$''
-            ''^tests/functional/plugins\.sh$''
             ''^tests/functional/post-hook\.sh$''
             ''^tests/functional/pure-eval\.sh$''
             ''^tests/functional/push-to-store-old\.sh$''
@@ -639,7 +628,6 @@
             ''^tests/functional/search\.sh$''
             ''^tests/functional/secure-drv-outputs\.sh$''
             ''^tests/functional/selfref-gc\.sh$''
-            ''^tests/functional/shell\.sh$''
             ''^tests/functional/shell\.shebang\.sh$''
             ''^tests/functional/simple\.builder\.sh$''
             ''^tests/functional/supplementary-groups\.sh$''
@@ -649,7 +637,6 @@
             ''^tests/functional/user-envs\.builder\.sh$''
             ''^tests/functional/user-envs\.sh$''
             ''^tests/functional/why-depends\.sh$''
-            ''^tests/functional/zstd\.sh$''
             ''^src/libutil-tests/data/git/check-data\.sh$''
           ];
         };
diff --git a/tests/functional/build.sh b/tests/functional/build.sh
index 5396a465f..3f65a7c2c 100755
--- a/tests/functional/build.sh
+++ b/tests/functional/build.sh
@@ -84,6 +84,7 @@ expectStderr 1 nix build --expr '""' --no-link \
   | grepQuiet "has 0 entries in its context. It should only have exactly one entry"
 
 # Too much string context
+# shellcheck disable=SC2016 # The ${} in this is Nix, not shell
 expectStderr 1 nix build --impure --expr 'with (import ./multiple-outputs.nix).e.a_a; "${drvPath}${outPath}"' --no-link \
   | grepQuiet "has 2 entries in its context. It should only have exactly one entry"
 
@@ -160,7 +161,7 @@ printf "%s\n" "$drv^*" | nix build --no-link --stdin --json | jq --exit-status '
 out="$(nix build -f fod-failing.nix -L 2>&1)" && status=0 || status=$?
 test "$status" = 1
 # one "hash mismatch" error, one "build of ... failed"
-test "$(<<<"$out" grep -E '^error:' | wc -l)" = 2
+test "$(<<<"$out" grep -cE '^error:')" = 2
 <<<"$out" grepQuiet -E "hash mismatch in fixed-output derivation '.*-x1\\.drv'"
 <<<"$out" grepQuiet -vE "hash mismatch in fixed-output derivation '.*-x3\\.drv'"
 <<<"$out" grepQuiet -vE "hash mismatch in fixed-output derivation '.*-x2\\.drv'"
@@ -169,7 +170,7 @@ test "$(<<<"$out" grep -E '^error:' | wc -l)" = 2
 out="$(nix build -f fod-failing.nix -L x1 x2 x3 --keep-going 2>&1)" && status=0 || status=$?
 test "$status" = 1
 # three "hash mismatch" errors - for each failing fod, one "build of ... failed"
-test "$(<<<"$out" grep -E '^error:' | wc -l)" = 4
+test "$(<<<"$out" grep -cE '^error:')" = 4
 <<<"$out" grepQuiet -E "hash mismatch in fixed-output derivation '.*-x1\\.drv'"
 <<<"$out" grepQuiet -E "hash mismatch in fixed-output derivation '.*-x3\\.drv'"
 <<<"$out" grepQuiet -E "hash mismatch in fixed-output derivation '.*-x2\\.drv'"
@@ -177,13 +178,13 @@ test "$(<<<"$out" grep -E '^error:' | wc -l)" = 4
 
 out="$(nix build -f fod-failing.nix -L x4 2>&1)" && status=0 || status=$?
 test "$status" = 1
-test "$(<<<"$out" grep -E '^error:' | wc -l)" = 2
+test "$(<<<"$out" grep -cE '^error:')" = 2
 <<<"$out" grepQuiet -E "error: 1 dependencies of derivation '.*-x4\\.drv' failed to build"
 <<<"$out" grepQuiet -E "hash mismatch in fixed-output derivation '.*-x2\\.drv'"
 
 out="$(nix build -f fod-failing.nix -L x4 --keep-going 2>&1)" && status=0 || status=$?
 test "$status" = 1
-test "$(<<<"$out" grep -E '^error:' | wc -l)" = 3
+test "$(<<<"$out" grep -cE '^error:')" = 3
 <<<"$out" grepQuiet -E "error: 2 dependencies of derivation '.*-x4\\.drv' failed to build"
 <<<"$out" grepQuiet -vE "hash mismatch in fixed-output derivation '.*-x3\\.drv'"
 <<<"$out" grepQuiet -vE "hash mismatch in fixed-output derivation '.*-x2\\.drv'"
diff --git a/tests/functional/check.sh b/tests/functional/check.sh
index 23e3d2ff0..bff6b2ec8 100755
--- a/tests/functional/check.sh
+++ b/tests/functional/check.sh
@@ -7,9 +7,9 @@ buggyNeedLocalStore "see #4813"
 
 checkBuildTempDirRemoved ()
 {
-    buildDir=$(sed -n 's/CHECK_TMPDIR=//p' $1 | head -1)
+    buildDir=$(sed -n 's/CHECK_TMPDIR=//p' "$1" | head -1)
     checkBuildIdFile=${buildDir}/checkBuildId
-    [[ ! -f $checkBuildIdFile ]] || ! grep $checkBuildId $checkBuildIdFile
+    [[ ! -f $checkBuildIdFile ]] || ! grep "$checkBuildId" "$checkBuildIdFile"
 }
 
 # written to build temp directories to verify created by this instance
@@ -28,15 +28,15 @@ nix-build dependencies.nix --no-out-link --check
 # check for dangling temporary build directories
 # only retain if build fails and --keep-failed is specified, or...
 # ...build is non-deterministic and --check and --keep-failed are both specified
-nix-build check.nix -A failed --argstr checkBuildId $checkBuildId \
-    --no-out-link 2> $TEST_ROOT/log || status=$?
+nix-build check.nix -A failed --argstr checkBuildId "$checkBuildId" \
+    --no-out-link 2> "$TEST_ROOT/log" || status=$?
 [ "$status" = "100" ]
-checkBuildTempDirRemoved $TEST_ROOT/log
+checkBuildTempDirRemoved "$TEST_ROOT/log"
 
-nix-build check.nix -A failed --argstr checkBuildId $checkBuildId \
-    --no-out-link --keep-failed 2> $TEST_ROOT/log || status=$?
+nix-build check.nix -A failed --argstr checkBuildId "$checkBuildId" \
+    --no-out-link --keep-failed 2> "$TEST_ROOT/log" || status=$?
 [ "$status" = "100" ]
-if checkBuildTempDirRemoved $TEST_ROOT/log; then false; fi
+if checkBuildTempDirRemoved "$TEST_ROOT/log"; then false; fi
 
 test_custom_build_dir() {
   local customBuildDir="$TEST_ROOT/custom-build-dir"
@@ -44,42 +44,42 @@ test_custom_build_dir() {
   # Nix does not create the parent directories, and perhaps it shouldn't try to
   # decide the permissions of build-dir.
   mkdir "$customBuildDir"
-  nix-build check.nix -A failed --argstr checkBuildId $checkBuildId \
-      --no-out-link --keep-failed --option build-dir "$TEST_ROOT/custom-build-dir" 2> $TEST_ROOT/log || status=$?
+  nix-build check.nix -A failed --argstr checkBuildId "$checkBuildId" \
+      --no-out-link --keep-failed --option build-dir "$TEST_ROOT/custom-build-dir" 2> "$TEST_ROOT/log" || status=$?
   [ "$status" = "100" ]
   [[ 1 == "$(count "$customBuildDir/nix-build-"*)" ]]
-  local buildDir="$customBuildDir/nix-build-"*""
-  if [[ -e $buildDir/build ]]; then
-      buildDir=$buildDir/build
+  local buildDir=("$customBuildDir/nix-build-"*)
+  if [[ -e ${buildDir[*]}/build ]]; then
+      buildDir[0]="${buildDir[*]}/build"
   fi
-  grep $checkBuildId $buildDir/checkBuildId
+  grep "$checkBuildId" "${buildDir[*]}/checkBuildId"
 }
 test_custom_build_dir
 
-nix-build check.nix -A deterministic --argstr checkBuildId $checkBuildId \
-    --no-out-link 2> $TEST_ROOT/log
-checkBuildTempDirRemoved $TEST_ROOT/log
+nix-build check.nix -A deterministic --argstr checkBuildId "$checkBuildId" \
+    --no-out-link 2> "$TEST_ROOT/log"
+checkBuildTempDirRemoved "$TEST_ROOT/log"
 
-nix-build check.nix -A deterministic --argstr checkBuildId $checkBuildId \
-    --no-out-link --check --keep-failed 2> $TEST_ROOT/log
-if grepQuiet 'may not be deterministic' $TEST_ROOT/log; then false; fi
-checkBuildTempDirRemoved $TEST_ROOT/log
+nix-build check.nix -A deterministic --argstr checkBuildId "$checkBuildId" \
+    --no-out-link --check --keep-failed 2> "$TEST_ROOT/log"
+if grepQuiet 'may not be deterministic' "$TEST_ROOT/log"; then false; fi
+checkBuildTempDirRemoved "$TEST_ROOT/log"
 
-nix-build check.nix -A nondeterministic --argstr checkBuildId $checkBuildId \
-    --no-out-link 2> $TEST_ROOT/log
-checkBuildTempDirRemoved $TEST_ROOT/log
+nix-build check.nix -A nondeterministic --argstr checkBuildId "$checkBuildId" \
+    --no-out-link 2> "$TEST_ROOT/log"
+checkBuildTempDirRemoved "$TEST_ROOT/log"
 
-nix-build check.nix -A nondeterministic --argstr checkBuildId $checkBuildId \
-    --no-out-link --check 2> $TEST_ROOT/log || status=$?
-grep 'may not be deterministic' $TEST_ROOT/log
+nix-build check.nix -A nondeterministic --argstr checkBuildId "$checkBuildId" \
+    --no-out-link --check 2> "$TEST_ROOT/log" || status=$?
+grep 'may not be deterministic' "$TEST_ROOT/log"
 [ "$status" = "104" ]
-checkBuildTempDirRemoved $TEST_ROOT/log
+checkBuildTempDirRemoved "$TEST_ROOT/log"
 
-nix-build check.nix -A nondeterministic --argstr checkBuildId $checkBuildId \
-    --no-out-link --check --keep-failed 2> $TEST_ROOT/log || status=$?
-grep 'may not be deterministic' $TEST_ROOT/log
+nix-build check.nix -A nondeterministic --argstr checkBuildId "$checkBuildId" \
+    --no-out-link --check --keep-failed 2> "$TEST_ROOT/log" || status=$?
+grep 'may not be deterministic' "$TEST_ROOT/log"
 [ "$status" = "104" ]
-if checkBuildTempDirRemoved $TEST_ROOT/log; then false; fi
+if checkBuildTempDirRemoved "$TEST_ROOT/log"; then false; fi
 
 TODO_NixOS
 
@@ -87,24 +87,24 @@ clearStore
 
 path=$(nix-build check.nix -A fetchurl --no-out-link)
 
-chmod +w $path
-echo foo > $path
-chmod -w $path
+chmod +w "$path"
+echo foo > "$path"
+chmod -w "$path"
 
 nix-build check.nix -A fetchurl --no-out-link --check
 # Note: "check" doesn't repair anything, it just compares to the hash stored in the database.
-[[ $(cat $path) = foo ]]
+[[ $(cat "$path") = foo ]]
 
 nix-build check.nix -A fetchurl --no-out-link --repair
-[[ $(cat $path) != foo ]]
+[[ $(cat "$path") != foo ]]
 
-echo 'Hello World' > $TEST_ROOT/dummy
+echo 'Hello World' > "$TEST_ROOT/dummy"
 nix-build check.nix -A hashmismatch --no-out-link || status=$?
 [ "$status" = "102" ]
 
-echo -n > $TEST_ROOT/dummy
+echo -n > "$TEST_ROOT/dummy"
 nix-build check.nix -A hashmismatch --no-out-link
-echo 'Hello World' > $TEST_ROOT/dummy
+echo 'Hello World' > "$TEST_ROOT/dummy"
 
 nix-build check.nix -A hashmismatch --no-out-link --check || status=$?
 [ "$status" = "102" ]
diff --git a/tests/functional/eval.sh b/tests/functional/eval.sh
index 22d2d02a2..e2ced41c2 100755
--- a/tests/functional/eval.sh
+++ b/tests/functional/eval.sh
@@ -35,13 +35,13 @@ nix-instantiate --eval -E 'assert 1 + 2 == 3; true'
 [[ "$(nix-instantiate --eval -E '{"assert"=1;bar=2;}')" == '{ "assert" = 1; bar = 2; }' ]]
 
 # Check that symlink cycles don't cause a hang.
-ln -sfn cycle.nix $TEST_ROOT/cycle.nix
-(! nix eval --file $TEST_ROOT/cycle.nix)
+ln -sfn cycle.nix "$TEST_ROOT/cycle.nix"
+(! nix eval --file "$TEST_ROOT/cycle.nix")
 
 # Check that relative symlinks are resolved correctly.
-mkdir -p $TEST_ROOT/xyzzy $TEST_ROOT/foo
-ln -sfn ../xyzzy $TEST_ROOT/foo/bar
-printf 123 > $TEST_ROOT/xyzzy/default.nix
+mkdir -p "$TEST_ROOT/xyzzy" "$TEST_ROOT/foo"
+ln -sfn ../xyzzy "$TEST_ROOT/foo/bar"
+printf 123 > "$TEST_ROOT/xyzzy/default.nix"
 [[ $(nix eval --impure --expr "import $TEST_ROOT/foo/bar") = 123 ]]
 
 # Test --arg-from-file.
@@ -57,7 +57,7 @@ fi
 
 # Test that unknown settings are warned about
 out="$(expectStderr 0 nix eval --option foobar baz --expr '""' --raw)"
-[[ "$(echo "$out" | grep foobar | wc -l)" = 1 ]]
+[[ "$(echo "$out" | grep -c foobar)" = 1 ]]
 
 # Test flag alias
 out="$(nix eval --expr '{}' --build-cores 1)"
diff --git a/tests/functional/fetchurl.sh b/tests/functional/fetchurl.sh
index 5af44fcf2..c25ac3216 100755
--- a/tests/functional/fetchurl.sh
+++ b/tests/functional/fetchurl.sh
@@ -9,12 +9,12 @@ clearStore
 # Test fetching a flat file.
 hash=$(nix-hash --flat --type sha256 ./fetchurl.sh)
 
-outPath=$(nix-build -vvvvv --expr 'import <nix/fetchurl.nix>' --argstr url file://$(pwd)/fetchurl.sh --argstr sha256 $hash --no-out-link)
+outPath=$(nix-build -vvvvv --expr 'import <nix/fetchurl.nix>' --argstr url "file://$(pwd)/fetchurl.sh" --argstr sha256 "$hash" --no-out-link)
 
-cmp $outPath fetchurl.sh
+cmp "$outPath" fetchurl.sh
 
 # Do not re-fetch paths already present.
-outPath2=$(nix-build -vvvvv --expr 'import <nix/fetchurl.nix>' --argstr url file:///does-not-exist/must-remain-unused/fetchurl.sh --argstr sha256 $hash --no-out-link)
+outPath2=$(nix-build -vvvvv --expr 'import <nix/fetchurl.nix>' --argstr url file:///does-not-exist/must-remain-unused/fetchurl.sh --argstr sha256 "$hash" --no-out-link)
 test "$outPath" == "$outPath2"
 
 # Now using a base-64 hash.
@@ -22,9 +22,9 @@ clearStore
 
 hash=$(nix hash file --type sha512 --base64 ./fetchurl.sh)
 
-outPath=$(nix-build -vvvvv --expr 'import <nix/fetchurl.nix>' --argstr url file://$(pwd)/fetchurl.sh --argstr sha512 $hash --no-out-link)
+outPath=$(nix-build -vvvvv --expr 'import <nix/fetchurl.nix>' --argstr url "file://$(pwd)/fetchurl.sh" --argstr sha512 "$hash" --no-out-link)
 
-cmp $outPath fetchurl.sh
+cmp "$outPath" fetchurl.sh
 
 # Now using an SRI hash.
 clearStore
@@ -33,58 +33,58 @@ hash=$(nix hash file ./fetchurl.sh)
 
 [[ $hash =~ ^sha256- ]]
 
-outPath=$(nix-build -vvvvv --expr 'import <nix/fetchurl.nix>' --argstr url file://$(pwd)/fetchurl.sh --argstr hash $hash --no-out-link)
+outPath=$(nix-build -vvvvv --expr 'import <nix/fetchurl.nix>' --argstr url "file://$(pwd)/fetchurl.sh" --argstr hash "$hash" --no-out-link)
 
-cmp $outPath fetchurl.sh
+cmp "$outPath" fetchurl.sh
 
 # Test that we can substitute from a different store dir.
 clearStore
 
-other_store=file://$TEST_ROOT/other_store?store=/fnord/store
+other_store="file://$TEST_ROOT/other_store?store=/fnord/store"
 
 hash=$(nix hash file --type sha256 --base16 ./fetchurl.sh)
 
-storePath=$(nix --store $other_store store add-file ./fetchurl.sh)
+nix --store "$other_store" store add-file ./fetchurl.sh
 
-outPath=$(nix-build -vvvvv --expr 'import <nix/fetchurl.nix>' --argstr url file:///no-such-dir/fetchurl.sh --argstr sha256 $hash --no-out-link --substituters $other_store)
+outPath=$(nix-build -vvvvv --expr 'import <nix/fetchurl.nix>' --argstr url file:///no-such-dir/fetchurl.sh --argstr sha256 "$hash" --no-out-link --substituters "$other_store")
 
 # Test hashed mirrors with an SRI hash.
-nix-build -vvvvv --expr 'import <nix/fetchurl.nix>' --argstr url file:///no-such-dir/fetchurl.sh --argstr hash $(nix hash to-sri --type sha256 $hash) \
-          --no-out-link --substituters $other_store
+nix-build -vvvvv --expr 'import <nix/fetchurl.nix>' --argstr url file:///no-such-dir/fetchurl.sh --argstr hash "$(nix hash to-sri --type sha256 "$hash")" \
+          --no-out-link --substituters "$other_store"
 
 # Test unpacking a NAR.
-rm -rf $TEST_ROOT/archive
-mkdir -p $TEST_ROOT/archive
-cp ./fetchurl.sh $TEST_ROOT/archive
-chmod +x $TEST_ROOT/archive/fetchurl.sh
-ln -s foo $TEST_ROOT/archive/symlink
-nar=$TEST_ROOT/archive.nar
-nix-store --dump $TEST_ROOT/archive > $nar
+rm -rf "$TEST_ROOT/archive"
+mkdir -p "$TEST_ROOT/archive"
+cp ./fetchurl.sh "$TEST_ROOT/archive"
+chmod +x "$TEST_ROOT/archive/fetchurl.sh"
+ln -s foo "$TEST_ROOT/archive/symlink"
+nar="$TEST_ROOT/archive.nar"
+nix-store --dump "$TEST_ROOT/archive" > "$nar"
 
-hash=$(nix-hash --flat --type sha256 $nar)
+hash=$(nix-hash --flat --type sha256 "$nar")
 
-outPath=$(nix-build -vvvvv --expr 'import <nix/fetchurl.nix>' --argstr url file://$nar --argstr sha256 $hash \
+outPath=$(nix-build -vvvvv --expr 'import <nix/fetchurl.nix>' --argstr url "file://$nar" --argstr sha256 "$hash" \
           --arg unpack true --argstr name xyzzy --no-out-link)
 
-echo $outPath | grepQuiet 'xyzzy'
+echo "$outPath" | grepQuiet 'xyzzy'
 
-test -x $outPath/fetchurl.sh
-test -L $outPath/symlink
+test -x "$outPath/fetchurl.sh"
+test -L "$outPath/symlink"
 
-nix-store --delete $outPath
+nix-store --delete "$outPath"
 
 # Test unpacking a compressed NAR.
-narxz=$TEST_ROOT/archive.nar.xz
-rm -f $narxz
-xz --keep $nar
-outPath=$(nix-build -vvvvv --expr 'import <nix/fetchurl.nix>' --argstr url file://$narxz --argstr sha256 $hash \
+narxz="$TEST_ROOT/archive.nar.xz"
+rm -f "$narxz"
+xz --keep "$nar"
+outPath=$(nix-build -vvvvv --expr 'import <nix/fetchurl.nix>' --argstr url "file://$narxz" --argstr sha256 "$hash" \
           --arg unpack true --argstr name xyzzy --no-out-link)
 
-test -x $outPath/fetchurl.sh
-test -L $outPath/symlink
+test -x "$outPath/fetchurl.sh"
+test -L "$outPath/symlink"
 
 # Make sure that *not* passing a outputHash fails.
 requireDaemonNewerThan "2.20"
 expected=100
 if [[ -v NIX_DAEMON_PACKAGE ]]; then expected=1; fi # work around the daemon not returning a 100 status correctly
-expectStderr $expected nix-build --expr '{ url }: builtins.derivation { name = "nix-cache-info"; system = "x86_64-linux"; builder = "builtin:fetchurl"; inherit url; outputHashMode = "flat"; }' --argstr url file://$narxz 2>&1 | grep 'must be a fixed-output or impure derivation'
+expectStderr $expected nix-build --expr '{ url }: builtins.derivation { name = "nix-cache-info"; system = "x86_64-linux"; builder = "builtin:fetchurl"; inherit url; outputHashMode = "flat"; }' --argstr url "file://$narxz" 2>&1 | grep 'must be a fixed-output or impure derivation'
diff --git a/tests/functional/flakes/common.sh b/tests/functional/flakes/common.sh
index f83a02aba..8ec70d703 100644
--- a/tests/functional/flakes/common.sh
+++ b/tests/functional/flakes/common.sh
@@ -1,10 +1,13 @@
+#!/usr/bin/env bash
+
 source ../common.sh
 
+# shellcheck disable=SC2034 # this variable is used by tests that source this file
 registry=$TEST_ROOT/registry.json
 
 writeSimpleFlake() {
     local flakeDir="$1"
-    cat > $flakeDir/flake.nix <<EOF
+    cat > "$flakeDir/flake.nix" <<EOF
 {
   description = "Bla bla";
 
@@ -31,19 +34,19 @@ writeSimpleFlake() {
 }
 EOF
 
-    cp ../simple.nix ../shell.nix ../simple.builder.sh ../config.nix $flakeDir/
+    cp ../simple.nix ../shell.nix ../simple.builder.sh ../config.nix "$flakeDir/"
 }
 
 createSimpleGitFlake() {
     local flakeDir="$1"
-    writeSimpleFlake $flakeDir
-    git -C $flakeDir add flake.nix simple.nix shell.nix simple.builder.sh config.nix
-    git -C $flakeDir commit -m 'Initial'
+    writeSimpleFlake "$flakeDir"
+    git -C "$flakeDir" add flake.nix simple.nix shell.nix simple.builder.sh config.nix
+    git -C "$flakeDir" commit -m 'Initial'
 }
 
 writeDependentFlake() {
     local flakeDir="$1"
-    cat > $flakeDir/flake.nix <<EOF
+    cat > "$flakeDir/flake.nix" <<EOF
 {
   outputs = { self, flake1 }: {
     packages.$system.default = flake1.packages.$system.default;
@@ -55,7 +58,7 @@ EOF
 
 writeTrivialFlake() {
     local flakeDir="$1"
-    cat > $flakeDir/flake.nix <<EOF
+    cat > "$flakeDir/flake.nix" <<EOF
 {
   outputs = { self }: {
     expr = 123;
@@ -71,6 +74,7 @@ createGitRepo() {
     rm -rf "$repo" "$repo".tmp
     mkdir -p "$repo"
 
+    # shellcheck disable=SC2086 # word splitting of extraArgs is intended
     git -C "$repo" init $extraArgs
     git -C "$repo" config user.email "foobar@example.com"
     git -C "$repo" config user.name "Foobar"
diff --git a/tests/functional/gc.sh b/tests/functional/gc.sh
index 7707a7e27..c58f47021 100755
--- a/tests/functional/gc.sh
+++ b/tests/functional/gc.sh
@@ -11,46 +11,46 @@ outPath=$(nix-store -rvv "$drvPath")
 
 # Set a GC root.
 rm -f "$NIX_STATE_DIR/gcroots/foo"
-ln -sf $outPath "$NIX_STATE_DIR/gcroots/foo"
+ln -sf "$outPath" "$NIX_STATE_DIR/gcroots/foo"
 
-[ "$(nix-store -q --roots $outPath)" = "$NIX_STATE_DIR/gcroots/foo -> $outPath" ]
+[ "$(nix-store -q --roots "$outPath")" = "$NIX_STATE_DIR/gcroots/foo -> $outPath" ]
 
-nix-store --gc --print-roots | grep $outPath
-nix-store --gc --print-live | grep $outPath
-nix-store --gc --print-dead | grep $drvPath
-if nix-store --gc --print-dead | grep -E $outPath$; then false; fi
+nix-store --gc --print-roots | grep "$outPath"
+nix-store --gc --print-live | grep "$outPath"
+nix-store --gc --print-dead | grep "$drvPath"
+if nix-store --gc --print-dead | grep -E "$outPath"$; then false; fi
 
 nix-store --gc --print-dead
 
-inUse=$(readLink $outPath/reference-to-input-2)
-if nix-store --delete $inUse; then false; fi
-test -e $inUse
+inUse=$(readLink "$outPath/reference-to-input-2")
+if nix-store --delete "$inUse"; then false; fi
+test -e "$inUse"
 
-if nix-store --delete $outPath; then false; fi
-test -e $outPath
+if nix-store --delete "$outPath"; then false; fi
+test -e "$outPath"
 
-for i in $NIX_STORE_DIR/*; do
+for i in "$NIX_STORE_DIR"/*; do
     if [[ $i =~ /trash ]]; then continue; fi # compat with old daemon
-    touch $i.lock
-    touch $i.chroot
+    touch "$i.lock"
+    touch "$i.chroot"
 done
 
 nix-collect-garbage
 
 # Check that the root and its dependencies haven't been deleted.
-cat $outPath/foobar
-cat $outPath/reference-to-input-2/bar
+cat "$outPath/foobar"
+cat "$outPath/reference-to-input-2/bar"
 
 # Check that the derivation has been GC'd.
-if test -e $drvPath; then false; fi
+if test -e "$drvPath"; then false; fi
 
 rm "$NIX_STATE_DIR/gcroots/foo"
 
 nix-collect-garbage
 
 # Check that the output has been GC'd.
-if test -e $outPath/foobar; then false; fi
+if test -e "$outPath/foobar"; then false; fi
 
 # Check that the store is empty.
-rmdir $NIX_STORE_DIR/.links
-rmdir $NIX_STORE_DIR
+rmdir "$NIX_STORE_DIR/.links"
+rmdir "$NIX_STORE_DIR"
diff --git a/tests/functional/help.sh b/tests/functional/help.sh
index 61efc1cb2..127cc455b 100755
--- a/tests/functional/help.sh
+++ b/tests/functional/help.sh
@@ -65,5 +65,6 @@ def recurse($prefix):
 }
 
 nix __dump-cli | subcommands | while IFS= read -r cmd; do
+    # shellcheck disable=SC2086 # word splitting of cmd is intended
     nix $cmd --help
 done
diff --git a/tests/functional/lang.sh b/tests/functional/lang.sh
index 46cf3f1fe..e64663d30 100755
--- a/tests/functional/lang.sh
+++ b/tests/functional/lang.sh
@@ -37,12 +37,14 @@ nix-instantiate --eval -E 'let x = { repeating = x; tracing = builtins.trace x t
   2>&1 | grepQuiet -F 'trace: { repeating = «repeated»; tracing = «potential infinite recursion»; }'
 
 nix-instantiate --eval -E 'builtins.warn "Hello" 123' 2>&1 | grepQuiet 'warning: Hello'
+# shellcheck disable=SC2016 # The ${} in this is Nix, not shell
 nix-instantiate --eval -E 'builtins.addErrorContext "while doing ${"something"} interesting" (builtins.warn "Hello" 123)' 2>/dev/null | grepQuiet 123
 
 # warn does not accept non-strings for now
 expectStderr 1 nix-instantiate --eval -E 'let x = builtins.warn { x = x; } true; in x' \
   | grepQuiet "expected a string but found a set"
 expectStderr 1 nix-instantiate --eval --abort-on-warn -E 'builtins.warn "Hello" 123' | grepQuiet Hello
+# shellcheck disable=SC2016 # The ${} in this is Nix, not shell
 NIX_ABORT_ON_WARN=1 expectStderr 1 nix-instantiate --eval -E 'builtins.addErrorContext "while doing ${"something"} interesting" (builtins.warn "Hello" 123)' | grepQuiet "while doing something interesting"
 
 set +x
@@ -106,6 +108,7 @@ for i in lang/eval-fail-*.nix; do
         fi
     )"
     if
+        # shellcheck disable=SC2086 # word splitting of flags is intended
         expectStderr 1 nix-instantiate $flags "lang/$i.nix" \
             | sed "s!$(pwd)!/pwd!g" > "lang/$i.err"
     then
diff --git a/tests/functional/nar-access.sh b/tests/functional/nar-access.sh
index b254081cf..2b0a6a329 100755
--- a/tests/functional/nar-access.sh
+++ b/tests/functional/nar-access.sh
@@ -9,57 +9,57 @@ cd "$TEST_ROOT"
 
 # Dump path to nar.
 narFile="$TEST_ROOT/path.nar"
-nix-store --dump $storePath > $narFile
+nix-store --dump "$storePath" > "$narFile"
 
 # Check that find and nar ls match.
-( cd $storePath; find . | sort ) > files.find
-nix nar ls -R -d $narFile "" | sort > files.ls-nar
+( cd "$storePath"; find . | sort ) > files.find
+nix nar ls -R -d "$narFile" "" | sort > files.ls-nar
 diff -u files.find files.ls-nar
 
 # Check that file contents of data match.
-nix nar cat $narFile /foo/data > data.cat-nar
-diff -u data.cat-nar $storePath/foo/data
+nix nar cat "$narFile" /foo/data > data.cat-nar
+diff -u data.cat-nar "$storePath/foo/data"
 
 # Check that file contents of baz match.
-nix nar cat $narFile /foo/baz > baz.cat-nar
-diff -u baz.cat-nar $storePath/foo/baz
+nix nar cat "$narFile" /foo/baz > baz.cat-nar
+diff -u baz.cat-nar "$storePath/foo/baz"
 
-nix store cat $storePath/foo/baz > baz.cat-nar
-diff -u baz.cat-nar $storePath/foo/baz
+nix store cat "$storePath/foo/baz" > baz.cat-nar
+diff -u baz.cat-nar "$storePath/foo/baz"
 
 TODO_NixOS
 
 # Check that 'nix store cat' fails on invalid store paths.
-invalidPath="$(dirname $storePath)/99999999999999999999999999999999-foo"
-cp -r $storePath $invalidPath
-expect 1 nix store cat $invalidPath/foo/baz
+invalidPath="$(dirname "$storePath")/99999999999999999999999999999999-foo"
+cp -r "$storePath" "$invalidPath"
+expect 1 nix store cat "$invalidPath/foo/baz"
 
 # Test --json.
 diff -u \
-    <(nix nar ls --json $narFile / | jq -S) \
+    <(nix nar ls --json "$narFile" / | jq -S) \
     <(echo '{"type":"directory","entries":{"foo":{},"foo-x":{},"qux":{},"zyx":{}}}' | jq -S)
 diff -u \
-    <(nix nar ls --json -R $narFile /foo | jq -S) \
+    <(nix nar ls --json -R "$narFile" /foo | jq -S) \
     <(echo '{"type":"directory","entries":{"bar":{"type":"regular","size":0,"narOffset":368},"baz":{"type":"regular","size":0,"narOffset":552},"data":{"type":"regular","size":58,"narOffset":736}}}' | jq -S)
 diff -u \
-    <(nix nar ls --json -R $narFile /foo/bar | jq -S) \
+    <(nix nar ls --json -R "$narFile" /foo/bar | jq -S) \
     <(echo '{"type":"regular","size":0,"narOffset":368}' | jq -S)
 diff -u \
-    <(nix store ls --json $storePath | jq -S) \
+    <(nix store ls --json "$storePath" | jq -S) \
     <(echo '{"type":"directory","entries":{"foo":{},"foo-x":{},"qux":{},"zyx":{}}}' | jq -S)
 diff -u \
-    <(nix store ls --json -R $storePath/foo | jq -S) \
+    <(nix store ls --json -R "$storePath/foo" | jq -S) \
     <(echo '{"type":"directory","entries":{"bar":{"type":"regular","size":0},"baz":{"type":"regular","size":0},"data":{"type":"regular","size":58}}}' | jq -S)
 diff -u \
-    <(nix store ls --json -R $storePath/foo/bar| jq -S) \
+    <(nix store ls --json -R "$storePath/foo/bar"| jq -S) \
     <(echo '{"type":"regular","size":0}' | jq -S)
 
 # Test missing files.
-expect 1 nix store ls --json -R $storePath/xyzzy 2>&1 | grep 'does not exist'
-expect 1 nix store ls $storePath/xyzzy 2>&1 | grep 'does not exist'
+expect 1 nix store ls --json -R "$storePath/xyzzy" 2>&1 | grep 'does not exist'
+expect 1 nix store ls "$storePath/xyzzy" 2>&1 | grep 'does not exist'
 
 # Test failure to dump.
-if nix-store --dump $storePath >/dev/full ; then
+if nix-store --dump "$storePath" >/dev/full ; then
     echo "dumping to /dev/full should fail"
-    exit -1
+    exit 1
 fi
diff --git a/tests/functional/shell.sh b/tests/functional/shell.sh
index 04a03eef5..cfc8e4102 100755
--- a/tests/functional/shell.sh
+++ b/tests/functional/shell.sh
@@ -27,8 +27,8 @@ expect 1 nix shell -f shell-hello.nix forbidden-symlink -c hello 2>&1 | grepQuie
 # For instance, we might set an environment variable temporarily to affect some
 # initialization or whatnot, but this must not leak into the environment of the
 # command being run.
-env > $TEST_ROOT/expected-env
-nix shell -f shell-hello.nix hello -c env > $TEST_ROOT/actual-env
+env > "$TEST_ROOT/expected-env"
+nix shell -f shell-hello.nix hello -c env > "$TEST_ROOT/actual-env"
 # Remove/reset variables we expect to be different.
 # - PATH is modified by nix shell
 # - we unset TMPDIR on macOS if it contains /var/folders
@@ -39,10 +39,10 @@ sed -i \
   -e 's/_=.*/_=.../' \
   -e '/^TMPDIR=\/var\/folders\/.*/d' \
   -e '/^__CF_USER_TEXT_ENCODING=.*$/d' \
-  $TEST_ROOT/expected-env $TEST_ROOT/actual-env
-sort $TEST_ROOT/expected-env > $TEST_ROOT/expected-env.sorted
-sort $TEST_ROOT/actual-env > $TEST_ROOT/actual-env.sorted
-diff $TEST_ROOT/expected-env.sorted $TEST_ROOT/actual-env.sorted
+  "$TEST_ROOT/expected-env" "$TEST_ROOT/actual-env"
+sort "$TEST_ROOT/expected-env" > "$TEST_ROOT/expected-env.sorted"
+sort "$TEST_ROOT/actual-env" > "$TEST_ROOT/actual-env.sorted"
+diff "$TEST_ROOT/expected-env.sorted" "$TEST_ROOT/actual-env.sorted"
 
 if isDaemonNewer "2.20.0pre20231220"; then
     # Test that command line attribute ordering is reflected in the PATH
@@ -53,8 +53,8 @@ fi
 
 requireSandboxSupport
 
-chmod -R u+w $TEST_ROOT/store0 || true
-rm -rf $TEST_ROOT/store0
+chmod -R u+w "$TEST_ROOT/store0" || true
+rm -rf "$TEST_ROOT/store0"
 
 clearStore
 
@@ -64,10 +64,10 @@ path=$(nix eval --raw -f shell-hello.nix hello)
 # visible in the sandbox.
 nix shell --sandbox-build-dir /build-tmp \
     --sandbox-paths '/nix? /bin? /lib? /lib64? /usr?' \
-    --store $TEST_ROOT/store0 -f shell-hello.nix hello -c hello | grep 'Hello World'
+    --store "$TEST_ROOT/store0" -f shell-hello.nix hello -c hello | grep 'Hello World'
 
-path2=$(nix shell --sandbox-paths '/nix? /bin? /lib? /lib64? /usr?' --store $TEST_ROOT/store0 -f shell-hello.nix hello -c $SHELL -c 'type -p hello')
+path2=$(nix shell --sandbox-paths '/nix? /bin? /lib? /lib64? /usr?' --store "$TEST_ROOT/store0" -f shell-hello.nix hello -c "$SHELL" -c 'type -p hello')
 
-[[ $path/bin/hello = $path2 ]]
+[[ "$path/bin/hello" = "$path2" ]]
 
-[[ -e $TEST_ROOT/store0/nix/store/$(basename $path)/bin/hello ]]
+[[ -e $TEST_ROOT/store0/nix/store/$(basename "$path")/bin/hello ]]
diff --git a/tests/functional/zstd.sh b/tests/functional/zstd.sh
index 90fe58539..450fb7d35 100755
--- a/tests/functional/zstd.sh
+++ b/tests/functional/zstd.sh
@@ -11,22 +11,22 @@ cacheURI="file://$cacheDir?compression=zstd"
 
 outPath=$(nix-build dependencies.nix --no-out-link)
 
-nix copy --to $cacheURI $outPath
+nix copy --to "$cacheURI" "$outPath"
 
-HASH=$(nix hash path $outPath)
+HASH=$(nix hash path "$outPath")
 
 clearStore
 clearCacheCache
 
-nix copy --from $cacheURI $outPath --no-check-sigs
+nix copy --from "$cacheURI" "$outPath" --no-check-sigs
 
-if ls $cacheDir/nar/*.zst &> /dev/null; then
+if ls "$cacheDir/nar/"*.zst &> /dev/null; then
     echo "files do exist"
 else
     echo "nars do not exist"
     exit 1
 fi
 
-HASH2=$(nix hash path $outPath)
+HASH2=$(nix hash path "$outPath")
 
-[[ $HASH = $HASH2 ]]
+[[ "$HASH" = "$HASH2" ]]

From a530939fe40f788d070e5593c884381758cf4192 Mon Sep 17 00:00:00 2001
From: Tim Van Baak <tim.vanbaak@gmail.com>
Date: Thu, 31 Oct 2024 06:46:33 -0700
Subject: [PATCH 2/3] Add check for one nix-build-* directory

---
 tests/functional/check.sh | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/functional/check.sh b/tests/functional/check.sh
index bff6b2ec8..b21349288 100755
--- a/tests/functional/check.sh
+++ b/tests/functional/check.sh
@@ -49,6 +49,10 @@ test_custom_build_dir() {
   [ "$status" = "100" ]
   [[ 1 == "$(count "$customBuildDir/nix-build-"*)" ]]
   local buildDir=("$customBuildDir/nix-build-"*)
+  if [[ "${#buildDir[@]}" -ne 1 ]]; then
+    echo "expected one nix-build-* directory, got: ${buildDir[*]}" >&2
+    exit 1
+  fi
   if [[ -e ${buildDir[*]}/build ]]; then
       buildDir[0]="${buildDir[*]}/build"
   fi

From 39fe52a126a7cba609016f0035e1a85001001acf Mon Sep 17 00:00:00 2001
From: Tim Van Baak <tim.vanbaak@gmail.com>
Date: Thu, 31 Oct 2024 06:46:58 -0700
Subject: [PATCH 3/3] Replace shebang with shellcheck directive

---
 tests/functional/flakes/common.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/functional/flakes/common.sh b/tests/functional/flakes/common.sh
index 8ec70d703..e43e180e3 100644
--- a/tests/functional/flakes/common.sh
+++ b/tests/functional/flakes/common.sh
@@ -1,4 +1,4 @@
-#!/usr/bin/env bash
+# shellcheck shell=bash
 
 source ../common.sh