From 0ab64e14eb78b8a3f956f056d91b79e2a2fa2f75 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 --- maintainers/flake-module.nix | 14 ------ maintainers/local.mk | 7 ++- 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 | 2 + tests/functional/common/init.sh | 2 + 17 files changed, 85 insertions(+), 54 deletions(-) diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix index 1f19c673a175..150e83be164a 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,6 @@ ''^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/completions\.sh$'' ''^tests/functional/compression-levels\.sh$'' ''^tests/functional/compute-levels\.sh$'' diff --git a/maintainers/local.mk b/maintainers/local.mk index 88d594d67d87..b1aeb363757e 100644 --- a/maintainers/local.mk +++ b/maintainers/local.mk @@ -2,8 +2,13 @@ .PHONY: format print-top-help += echo ' format: Format source code' +# These generated files are needed for linters to work more reliably. +# Generating them is cheap, so let's just do it. +generated-code = \ + tests/functional/common/vars-and-functions.sh + # This uses the cached .pre-commit-hooks.yaml file -format: +format: $(generated-code) @if ! type -p pre-commit &>/dev/null; then \ echo "make format: pre-commit not found. Please use \`nix develop\`."; \ exit 1; \ diff --git a/mk/common-test.sh b/mk/common-test.sh index de24b6fccb60..c80abd3813ac 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 ae744159113a..61a1f4a464bb 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 e83b42b4150f..33d667f92113 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 49d15c38963b..986692dbc01b 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 19f59e6aea89..2fcdbf10a016 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 3d4a4b0979ed..269f7f1129eb 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 2a7ebd8c69c1..b810609074db 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 516bdf092dfd..b8dc038bf365 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 b769a88f0cf1..ba5d1ff7a452 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 40f81da5a5ee..187b899487ca 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 81e5253bf298..c498d46c301d 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 443acb6ca788..e2627af394cb 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 d2a2132c1a41..1a53345778d0 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 4ec17b70664f..36ca9f153196 100644 --- a/tests/functional/common.sh +++ b/tests/functional/common.sh @@ -1,3 +1,5 @@ +# shellcheck shell=bash + set -eu -o pipefail if [[ -z "${COMMON_SH_SOURCED-}" ]]; then diff --git a/tests/functional/common/init.sh b/tests/functional/common/init.sh index 74da126517ad..dda1ecd41fa2 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