From 5dbbf233327bff1c89088ed0fc45882563240b1a Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 9 Dec 2021 15:16:18 +0000 Subject: [PATCH 1/2] Make init.sh safe to run twice `init.sh` is tested on its own. We used to do that. I deleted it in 4720853129b6866775edd9f90ad6f10701f98a3c but I am not sure why. Better to just restore it; at one point working on this every other test passed, so seems good to check whether `init.sh` can be run twice. We don't *need* to run `init.sh` twice, but I want to try to make our tests as robust as possible so that manual debugging (where tests for better or worse might be run ways that we didn't expect) is less fragile. --- tests/init.sh | 6 ++++++ tests/local.mk | 1 + 2 files changed, 7 insertions(+) mode change 100644 => 100755 tests/init.sh diff --git a/tests/init.sh b/tests/init.sh old mode 100644 new mode 100755 index 3c6d5917d64..da0568ed903 --- a/tests/init.sh +++ b/tests/init.sh @@ -1,8 +1,14 @@ +set -e -o pipefail + source common.sh test -n "$TEST_ROOT" if test -d "$TEST_ROOT"; then chmod -R u+w "$TEST_ROOT" + # We would delete any daemon socket, so let's stop the daemon first. + if [[ -n "${NIX_DAEMON_PACKAGE:-}" ]]; then + killDaemon + fi rm -rf "$TEST_ROOT" fi mkdir "$TEST_ROOT" diff --git a/tests/local.mk b/tests/local.mk index b3135fd4dfb..4ef071f9678 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -1,4 +1,5 @@ nix_tests = \ + init.sh \ flakes/flakes.sh \ flakes/run.sh \ flakes/mercurial.sh \ From 87da941348e1b345d1be348b091d3ae65ac66bb7 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 9 Dec 2021 15:16:18 +0000 Subject: [PATCH 2/2] Clean up daemon handling Split `common.sh` into the vars and functions definitions vs starting the daemon (and possibly other initialization logic). This way, `init.sh` can just `source` the former. Trying to start the daemon before `nix.conf` is written will fail because `nix daemon` requires `--experimental-features 'nix-command'`. `killDaemon` is idempotent, so it's safe to call when no daemon is running. `startDaemon` and `killDaemon` use the PID (which is now exported to subshells) to decide whether there is work to be done, rather than `NIX_REMOTE`, which might conceivably be set differently even if a daemon is running. `startDaemon` and `killDaemon` can save/restore the old `NIX_REMOTE` as `NIX_REMOTE_OLD`. `init.sh` kills daemon before deleting everything (including the daemon socket). --- .gitignore | 2 +- tests/common.sh | 12 ++++++ .../vars-and-functions.sh.in} | 43 +++++++++++-------- tests/db-migration.sh | 1 - tests/init.sh | 9 ++-- tests/local.mk | 4 +- 6 files changed, 44 insertions(+), 27 deletions(-) create mode 100644 tests/common.sh rename tests/{common.sh.in => common/vars-and-functions.sh.in} (81%) diff --git a/.gitignore b/.gitignore index 83313fe0a6d..e326966d646 100644 --- a/.gitignore +++ b/.gitignore @@ -75,7 +75,7 @@ perl/Makefile.config # /tests/ /tests/test-tmp -/tests/common.sh +/tests/common/vars-and-functions.sh /tests/result* /tests/restricted-innocent /tests/shell diff --git a/tests/common.sh b/tests/common.sh new file mode 100644 index 00000000000..68b90a85f67 --- /dev/null +++ b/tests/common.sh @@ -0,0 +1,12 @@ +set -e + +if [[ -z "${COMMON_SH_SOURCED-}" ]]; then + +COMMON_SH_SOURCED=1 + +source "$(readlink -f "$(dirname "${BASH_SOURCE[0]}")")/common/vars-and-functions.sh" +if [[ -n "${NIX_DAEMON_PACKAGE:-}" ]]; then + startDaemon +fi + +fi # COMMON_SH_SOURCED diff --git a/tests/common.sh.in b/tests/common/vars-and-functions.sh.in similarity index 81% rename from tests/common.sh.in rename to tests/common/vars-and-functions.sh.in index 939f2fce1ae..0deef4c1c06 100644 --- a/tests/common.sh.in +++ b/tests/common/vars-and-functions.sh.in @@ -1,8 +1,8 @@ set -e -if [[ -z "$COMMON_SH_SOURCED" ]]; then +if [[ -z "${COMMON_VARS_AND_FUNCTIONS_SH_SOURCED-}" ]]; then -COMMON_SH_SOURCED=1 +COMMON_VARS_AND_FUNCTIONS_SH_SOURCED=1 export PS4='+(${BASH_SOURCE[0]}:$LINENO) ' @@ -25,7 +25,7 @@ if [[ -n $NIX_STORE ]]; then fi export _NIX_IN_TEST=$TEST_ROOT/shared export _NIX_TEST_NO_LSOF=1 -export NIX_REMOTE=$NIX_REMOTE_ +export NIX_REMOTE=${NIX_REMOTE_-} unset NIX_PATH export TEST_HOME=$TEST_ROOT/test-home export HOME=$TEST_HOME @@ -90,13 +90,14 @@ clearCacheCache() { startDaemon() { # Don’t start the daemon twice, as this would just make it loop indefinitely - if [[ "$NIX_REMOTE" == daemon ]]; then - return + if [[ "${_NIX_TEST_DAEMON_PID-}" != '' ]]; then + return fi # Start the daemon, wait for the socket to appear. rm -f $NIX_DAEMON_SOCKET_PATH - PATH=$DAEMON_PATH nix-daemon& - pidDaemon=$! + PATH=$DAEMON_PATH nix-daemon & + _NIX_TEST_DAEMON_PID=$! + export _NIX_TEST_DAEMON_PID for ((i = 0; i < 300; i++)); do if [[ -S $NIX_DAEMON_SOCKET_PATH ]]; then DAEMON_STARTED=1 @@ -108,25 +109,35 @@ startDaemon() { fail "Didn’t manage to start the daemon" fi trap "killDaemon" EXIT + # Save for if daemon is killed + NIX_REMOTE_OLD=$NIX_REMOTE export NIX_REMOTE=daemon } killDaemon() { - kill $pidDaemon + # Don’t fail trying to stop a non-existant daemon twice + if [[ "${_NIX_TEST_DAEMON_PID-}" == '' ]]; then + return + fi + kill $_NIX_TEST_DAEMON_PID for i in {0..100}; do - kill -0 $pidDaemon 2> /dev/null || break + kill -0 $_NIX_TEST_DAEMON_PID 2> /dev/null || break sleep 0.1 done - kill -9 $pidDaemon 2> /dev/null || true - wait $pidDaemon || true + kill -9 $_NIX_TEST_DAEMON_PID 2> /dev/null || true + wait $_NIX_TEST_DAEMON_PID || true + rm -f $NIX_DAEMON_SOCKET_PATH + # Indicate daemon is stopped + unset _NIX_TEST_DAEMON_PID + # Restore old nix remote + NIX_REMOTE=$NIX_REMOTE_OLD trap "" EXIT } restartDaemon() { - [[ -z "${pidDaemon:-}" ]] && return 0 + [[ -z "${_NIX_TEST_DAEMON_PID:-}" ]] && return 0 killDaemon - unset NIX_REMOTE startDaemon } @@ -190,10 +201,6 @@ enableFeatures() { set -x -if [[ -n "${NIX_DAEMON_PACKAGE:-}" ]]; then - startDaemon -fi - onError() { set +x echo "$0: test failed at:" >&2 @@ -205,4 +212,4 @@ onError() { trap onError ERR -fi # COMMON_SH_SOURCED +fi # COMMON_VARS_AND_FUNCTIONS_SH_SOURCED diff --git a/tests/db-migration.sh b/tests/db-migration.sh index 3f9dc89726d..92dd4f3baa7 100644 --- a/tests/db-migration.sh +++ b/tests/db-migration.sh @@ -9,7 +9,6 @@ fi source common.sh killDaemon -unset NIX_REMOTE # Fill the db using the older Nix PATH_WITH_NEW_NIX="$PATH" diff --git a/tests/init.sh b/tests/init.sh index da0568ed903..fea6595163d 100755 --- a/tests/init.sh +++ b/tests/init.sh @@ -1,14 +1,13 @@ -set -e -o pipefail +set -eu -o pipefail -source common.sh +# Don't start the daemon +source common/vars-and-functions.sh test -n "$TEST_ROOT" if test -d "$TEST_ROOT"; then chmod -R u+w "$TEST_ROOT" # We would delete any daemon socket, so let's stop the daemon first. - if [[ -n "${NIX_DAEMON_PACKAGE:-}" ]]; then - killDaemon - fi + killDaemon rm -rf "$TEST_ROOT" fi mkdir "$TEST_ROOT" diff --git a/tests/local.mk b/tests/local.mk index 4ef071f9678..a4537cf09f6 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -129,9 +129,9 @@ endif install-tests += $(foreach x, $(nix_tests), tests/$(x)) -clean-files += $(d)/common.sh $(d)/config.nix $(d)/ca/config.nix +clean-files += $(d)/tests/common/vars-and-functions.sh $(d)/config.nix $(d)/ca/config.nix -test-deps += tests/common.sh tests/config.nix tests/ca/config.nix +test-deps += tests/common/vars-and-functions.sh tests/config.nix tests/ca/config.nix tests/plugins/libplugintest.$(SO_EXT) ifeq ($(BUILD_SHARED_LIBS), 1) test-deps += tests/plugins/libplugintest.$(SO_EXT)