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.
\$\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?

\$\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.
\$\endgroup\$

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.