3
\$\begingroup\$

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 than sleep 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.
asked Jun 20, 2021 at 16:52
\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

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?

answered Jun 21, 2021 at 7:36
\$\endgroup\$
2
\$\begingroup\$

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 the if 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.
answered Jun 20, 2021 at 20:47
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.