Merge pull request #11770 from Jaculabilis/shellcheck

Expand shellcheck coverage in functional tests

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$''
           ];
         };

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,21 +170,21 @@ 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'"
 <<<"$out" grepQuiet -E "error: build of '.*-x[1-3]\\.drv\\^out', '.*-x[1-3]\\.drv\\^out', '.*-x[1-3]\\.drv\\^out' failed"
 
 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'"

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,83 +28,87 @@ 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"
 
   # 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 [[ "${#buildDir[@]}" -ne 1 ]]; then
+    echo "expected one nix-build-* directory, got: ${buildDir[*]}" >&2
+    exit 1
   fi
-  grep $checkBuildId $buildDir/checkBuildId
+  if [[ -e ${buildDir[*]}/build ]]; then
+      buildDir[0]="${buildDir[*]}/build"
+  fi
+  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
 
 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" ]

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