I have a test suite that queries a localhost-based server. The server process needs to be running in order for the server to work. The process may be running already for debugging work, but it might not be. If a server process is running, it's probably safe to assume it's the correct one. If we started the process, we should kill it when we're done, but if it was already running, we should leave it.
I've written a bash script which seems to work, but isn't quite perfect.
if [ -z `pgrep -x server-process` ]; then
mkdir ./data
server-process ./data &
sleep 1; # Wait for server to print output,
# reporting initialization details etc.
fi
# Set up environment variables...
run-test-suite $@
if jobs %1 2>/dev/null; then
kill %1
rm -R ./data;
fi
In particular:
- Is there a more robust way to do this?
- Are there any edge cases I've missed?
- Is there a better way to wait for
server-process
's initial output, rather thansleep 1
(which is often not long enough anyway)?server-process
doesn't close the pipe - it could still report errors - I don't mind if these are interleaved with test suite output. I just want to wait until after the server has printed it's initial config information.
2 Answers 2
Capturing the output of pgrep
and testing whether it's empty doesn't seem the best way to use that utility, given that it returns a true value when it finds one or more processes:
if ! pgrep -x server-process >/dev/null 2>&1
then
mkdir ./data
server-process ./data &
sleep 1; # Wait for server to print output,
# reporting initialization details etc.
fi
Using job specifications with %1
is a convenience for interactive shells. We can write more robust code using $!
:
server_pid=0
if
then
server-process ./data &
server_pid=$!
fi
This has the advantage of being portable - i.e. we can use /bin/sh
as interpreter rather than requiring /bin/bash
.
Actually, instead of remembering the pid and conditionally using it at end of script, we can immediately register the action to be taken:
if
then
server-process ./data &
trap "kill $!; rm -r ./data" EXIT
fi
Unless there's a compelling reason to create the data directory inside the current working directory, it's probably better to use mktemp
to create us a nice empty initial directory.
Putting it all together:
#!/bin/sh
set -eu
if ! pgrep -x server-process >/dev/null 2>&1
then
data_dir=$(mktemp -d)
server-process "$data_dir" &
trap "kill $!; rm -r '$data_dir'" EXIT
sleep 1; # Wait for server to initialise
fi
run-test-suite "$@"
Without knowing the details of server-process
I can only speculate, but perhaps there's a better alternative to the arbitrary sleep
. Is there perhaps a file whose appearance in $data_dir
might indicate that the server can be used? Or a socket which starts responding?
Welcome to Code Review, Ari.
Good
- indentation
- useful comments
Major suggestions
- Regarding your server-process: it would be nice for it to have a proper "init" script to start it. Things have changed in systemd-land and you should follow that style if your system is based on systemd. Either way, having this in a proper init/systemd setup will make life easier for your admins and your developers.
- If you're in systemd-land then your
systemctl start
will take care of waiting for the process to start. If you're in init-land you will need to find some other way to verify that the process is started. Is there a log message you can wait for or a socket you can check? If you are just waiting for output, redirect that to a file and wait for it to contain something. Somehow, find something to verify and loop until that happens. - Once you've got a clean way to start it you can remove the backgrounding on your server-process and the sleep after it. The start command (either style) should not exit until your process is actually started.
- Your start script should also handle stopping so you won't need to check for whether it is running anymore. Just run your script with the stop argument (or run systemctl) and it will go away. If it died earlier, you might get a message. But then you can remove the data tree.
Small nits
- You don't need to end lines with semicolons. The shell takes each line as a command unless there's a backslach at the end of a line. Using the semicolon to put the
then
on the same line as theif
is good. It is one commonly-seen example of putting multiple commands on one line. - Use double square brackets for conditionals to avoid some surprises.
- Try shellcheck. It will have some suggestions I've left out.
- Put the directory into a variable and then refer to that variables instead of
./data
.