Skip to content

Clean up daemon handling in the tests #5753

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
roberth merged 2 commits into from
Feb 23, 2023
Merged

Clean up daemon handling in the tests #5753

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5dbbf23
Make init.sh safe to run twice
Ericson2314 Dec 9, 2021
87da941
Clean up daemon handling
Ericson2314 Dec 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions tests/common.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
43 changes: 25 additions & 18 deletions tests/common.sh.in → tests/common/vars-and-functions.sh.in
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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) '

Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
}

Expand Down Expand Up @@ -190,10 +201,6 @@ enableFeatures() {

set -x

if [[ -n "${NIX_DAEMON_PACKAGE:-}" ]]; then
startDaemon
fi

onError() {
set +x
echo "$0: test failed at:" >&2
Expand All @@ -205,4 +212,4 @@ onError() {

trap onError ERR

fi # COMMON_SH_SOURCED
fi # COMMON_VARS_AND_FUNCTIONS_SH_SOURCED
1 change: 0 additions & 1 deletion tests/db-migration.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ fi
source common.sh

killDaemon
unset NIX_REMOTE

# Fill the db using the older Nix
PATH_WITH_NEW_NIX="$PATH"
Expand Down
7 changes: 6 additions & 1 deletion tests/init.sh
100644 → 100755
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
source common.sh
set -eu -o pipefail

# Don't start the daemon
source common/vars-and-functions.sh
Copy link
Member Author

@Ericson2314 Ericson2314 Feb 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thufschmitt to answer the question you asked on IRC a bit better, I didn't put the starting the daemon in init for these reasons:

  1. Running tests without recalling init should still work. if one wants to examine state: the test itself starts and stops the daemon, rather than something else starting and it stopping.
  2. init.sh merely does cleanup and setup of files. Besides removing stuff, it just creates new blank filesystem state. Stopping the daemon before removing the files is just for any rogue daemon, before it becomes harder to stop because the socket file was deleted.

The file is split because init.sh still needs the env vars.

Copy link
Member

@thufschmitt thufschmitt Mar 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yes of course, it’s nice to be able to manually run init.sh once and then the test (I actually do that all the time)


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.
killDaemon
rm -rf "$TEST_ROOT"
fi
mkdir "$TEST_ROOT"
Expand Down
5 changes: 3 additions & 2 deletions tests/local.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
nix_tests = \
init.sh \
flakes/flakes.sh \
flakes/run.sh \
flakes/mercurial.sh \
Expand Down Expand Up @@ -128,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)
Expand Down