On 9/13/18 11:09 AM, Richard W.M. Jones wrote:
This assumes bashisms, but bash is required to run the tests.
This is mostly refactoring. However the changes (simplifications) are quite substantial:
- Since the new start_nbdkit helper function cleans up nbdkit on exit, most scripts no longer need to deal with the pid or kill the pid in the cleanup function.
- As a result, cleanup functions are radically simpler, and often disappear completely.
- Also removed the comment "# The cleanup() function is called implicitly on exit" passim partly because the cleanup functions have mostly been removed and partly because it should be clear from looking at functions.sh.
There is one additional change: In the test-memory*.sh tests, nbdkit used to run in the foreground, but that seems to be a consequence of some left over debugging. I changed it so they work like the other tests. ---
+# start_nbdkit -P pidfile args... +# +# Run nbdkit with args and wait for it to start up. If it fails to +# start up, exit with an error message. Also a cleanup handler is +# installed automatically which kills nbdkit on exit. +start_nbdkit () +{ + # -P <pidfile> must be the first two parameters. + if [ "$1" != "-P" ]; then + echo "$0: start_nbdkit: -P <pidfile> option must be first" + exit 1 + fi + pidfile="$2"
Although none of the tests passes a space within $2, it's better to consistently quote...
+ + # Run nbdkit. + nbdkit "$@" + + # Wait for the pidfile to appear. + for i in {1..10}; do + if test -f "$pidfile"; then + break + fi + sleep 1 + done + if ! test -f "$pidfile"; then + echo "$0: start_nbdkit: PID file $pidfile was not created" + exit 1 + fi + + # Kill nbdkit on exit. + cleanup_fn kill "$(cat $pidfile)"
...and thus this should be "$(cat "$pidfile")"
+++ b/tests/test-ip.sh
-if ! test -f ip.pid; then - echo "$0: PID file was not created" - exit 1 -fi - +start_nbdkit -P ip.pid -p $port example1 pid="$(cat ip.pid)"
# Check the process exists. kill -s 0 $pid
Hmm. Should the 'kill -s 0 $pid' be moved into the common code for all tests to benefit from (detecting early crashes where nbdkit started and created the pidfile, but then died)? If so, you don't need $pid in this file any longer.
+++ b/tests/test-start.sh
-if ! test -f start.pid; then - echo "$0: PID file was not created" - exit 1 -fi - +start_nbdkit -P start.pid -U start.sock example1 pid="$(cat start.pid)"
# Check the process exists.
and another one. The stuff I found is minor enough that I don't see a problem with you fixing and committing the series, rather than going through a v3. And I love the overall diffstat :) -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org