Libguestfs - Sep 2018 - [PATCH nbdkit 0/4] tests: Move common functions into tests/functions.sh

Combine much common code into tests/functions.sh.

Patch 1: Preparation for patch 3.

Patch 2: Fix a long-standing bug in how man pages links are generated.

Patch 3: Common code for iterating a test function over every plugin.

Patch 4: Common code for starting nbdkit in a test and waiting for the
         PID file to appear.  This is the largest and most complex of
         the patches but is basically repetitive.

Rich.

It's easier to get it to other places if it starts out in the
configure script.

Also split the list into language and non-language plugins.
---
 common-rules.mk | 44 ----------------------------------------
 configure.ac    | 53 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 44 deletions(-)

diff --git a/common-rules.mk b/common-rules.mk
index 2c5609f..03e01e6 100644
--- a/common-rules.mk
+++ b/common-rules.mk
@@ -30,50 +30,6 @@
 # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 # SUCH DAMAGE.
 
-plugins = \
-	curl \
-	data \
-	example1 \
-	example2 \
-	example3 \
-	example4 \
-	ext2 \
-	file \
-	guestfs \
-	gzip \
-	libvirt \
-	lua \
-	memory \
-	nbd \
-	null \
-	ocaml \
-	pattern \
-	perl \
-	python \
-	random \
-	ruby \
-	sh \
-	split \
-	streaming \
-	tar \
-	tcl \
-	vddk \
-	xz \
-	zero
-
-filters = \
-	blocksize \
-	cache \
-	cow \
-	delay \
-	error \
-	fua \
-	log \
-	nozero \
-	offset \
-	partition \
-	truncate
-
 plugindir = $(libdir)/nbdkit/plugins
 filterdir = $(libdir)/nbdkit/filters
 
diff --git a/configure.ac b/configure.ac
index 4d340d4..3a02c66 100644
--- a/configure.ac
+++ b/configure.ac
@@ -542,6 +542,59 @@ AC_ARG_ENABLE([vddk],[
     [enable_vddk=yes])
 AM_CONDITIONAL([HAVE_VDDK], [test "x$enable_vddk" =
"xyes"])
 
+dnl List of plugins and filters.
+lang_plugins="\
+        lua \
+        ocaml \
+        perl \
+        python \
+        ruby \
+        sh \
+        tcl \
+        "
+non_lang_plugins="\
+        curl \
+        data \
+	example1 \
+	example2 \
+	example3 \
+	example4 \
+	ext2 \
+	file \
+	guestfs \
+	gzip \
+	libvirt \
+	memory \
+	nbd \
+	null \
+	pattern \
+	random \
+	split \
+	streaming \
+	tar \
+	vddk \
+	xz \
+	zero \
+        "
+plugins="$(echo $lang_plugins $non_lang_plugins | xargs -n1 | sort -u |
xargs)"
+filters="\
+	blocksize \
+	cache \
+	cow \
+	delay \
+	error \
+	fua \
+	log \
+	nozero \
+	offset \
+	partition \
+	truncate \
+        "
+AC_SUBST([plugins])
+AC_SUBST([lang_plugins])
+AC_SUBST([non_lang_plugins])
+AC_SUBST([filters])
+
 dnl Produce output files.
 AC_CONFIG_HEADERS([config.h])
 AC_CONFIG_FILES([nbdkit],
-- 
2.19.0.rc0

---
 docs/Makefile.am | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/docs/Makefile.am b/docs/Makefile.am
index ccf0a79..cbe7033 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -104,10 +104,11 @@ nbdkit-filter.3: nbdkit-filter.pod filter-links.pod
 
 endif HAVE_POD
 
-# XXX Language links are broken - they link to section 1 instead of section 3.
 plugin-links.pod: $(top_srcdir)/common-rules.mk
 	rm -f $@ $@-t
-	$(srcdir)/make-links.sh plugin 1 $(plugins) > $@-t
+	$(srcdir)/make-links.sh plugin 1 $(non_lang_plugins) > $@-t
+	echo \; >> $@-t
+	$(srcdir)/make-links.sh plugin 3 $(lang_plugins) >> $@-t
 	mv $@-t $@
 
 filter-links.pod: $(top_srcdir)/common-rules.mk
-- 
2.19.0.rc0

This resurrects the unused tests/functions.sh file (although now we
need to generate it from tests/functions.sh.in).  Put the common code
for running a test against every plugin here.

Because of the previous commits we can now use the plugins list
directly from configure.ac instead of needing to use weird shell
script, although we still need to preserve the test that the plugin
was built so that the tests continue to work if a plugin is disabled
or not built through missing dependencies.
---
 .gitignore                              |  1 +
 configure.ac                            |  1 +
 tests/Makefile.am                       |  1 -
 tests/{functions.sh => functions.sh.in} | 23 +++++++++++++++++++----
 tests/test-dump-plugin.sh               | 19 ++++++++-----------
 tests/test-help.sh                      | 18 ++++++++----------
 tests/test-version.sh                   | 18 ++++++++----------
 7 files changed, 45 insertions(+), 36 deletions(-)

diff --git a/.gitignore b/.gitignore
index a21484e..b8396f1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -55,6 +55,7 @@ Makefile.in
 /tests/disk.xz
 /tests/ext2.img
 /tests/file-data
+/tests/functions.sh
 /tests/keys.psk
 /tests/offset-data
 /tests/partition-disk
diff --git a/configure.ac b/configure.ac
index 3a02c66..518b2bc 100644
--- a/configure.ac
+++ b/configure.ac
@@ -651,6 +651,7 @@ AC_CONFIG_FILES([Makefile
                  filters/truncate/Makefile
                  src/Makefile
                  src/nbdkit.pc
+                 tests/functions.sh
                  tests/Makefile])
 
 AC_OUTPUT
diff --git a/tests/Makefile.am b/tests/Makefile.am
index a7d0c2d..23316ea 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -35,7 +35,6 @@ include $(top_srcdir)/common-rules.mk
 MAINTAINERCLEANFILES  
 EXTRA_DIST = \
-	functions.sh \
 	make-pki.sh \
 	make-psk.sh \
 	python-exception.py \
diff --git a/tests/functions.sh b/tests/functions.sh.in
similarity index 74%
rename from tests/functions.sh
rename to tests/functions.sh.in
index e54bb9d..4071384 100644
--- a/tests/functions.sh
+++ b/tests/functions.sh.in
@@ -1,5 +1,7 @@
 # nbdkit
-# Copyright (C) 2017 Red Hat Inc.
+# Common functions used by the tests.
+# @configure_input@
+# Copyright (C) 2017-2018 Red Hat Inc.
 # All rights reserved.
 #
 # Redistribution and use in source and binary forms, with or without
@@ -30,6 +32,19 @@
 # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 # SUCH DAMAGE.
 
-# Some functions used by the tests.
-
-# (Currently empty)
+# foreach_plugin f
+#
+# For each plugin that was built, call test function f with the plugin
+# name as argument.
+foreach_plugin ()
+{
+    for p in @plugins@; do
+        # Was the plugin built?
+        d=@top_builddir@/plugins/$p
+        if [ -f $d/.libs/nbdkit-$p-plugin.so ] ||
+           [ -f $d/nbdkit-$p-plugin ]; then
+            # Yes so run the test.
+            "$1" "$p"
+        fi
+    done
+}
diff --git a/tests/test-dump-plugin.sh b/tests/test-dump-plugin.sh
index e08db84..77bd995 100755
--- a/tests/test-dump-plugin.sh
+++ b/tests/test-dump-plugin.sh
@@ -31,9 +31,9 @@
 # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 # SUCH DAMAGE.
 
+source functions.sh
 set -e
 set -x
-source ./functions.sh
 
 # Basic check that the name field is present.
 output="$(nbdkit example1 --dump-plugin)"
@@ -51,15 +51,11 @@ if [[ ! ( "$output" =~ example2_extra\=hello ) ]];
then
     exit 1
 fi
 
-# Get a list of all plugins and run --dump-plugin on all of them.
+# Run nbdkit plugin --dump-plugin for each plugin.
 # However some of these tests are expected to fail.
-plugins="$(
-    cd ..;
-    ls -1 plugins/*/.libs/nbdkit-*-plugin.so plugins/*/nbdkit-*-plugin |
-        sed 's,^plugins/\([^/]*\)/.*,\1,'
-)"
-for p in $plugins; do
-    case "$p${NBDKIT_VALGRIND:+-valgrind}" in
+test ()
+{
+    case "$1${NBDKIT_VALGRIND:+-valgrind}" in
         vddk | vddk-valgrind)
             # VDDK won't run without special environment variables
             # being set, so ignore it.
@@ -70,7 +66,8 @@ for p in $plugins; do
             # Plugins written in scripting languages can't run under
valgrind.
             ;;
         *)
-            nbdkit $p --dump-plugin
+            nbdkit $1 --dump-plugin
             ;;
     esac
-done
+}
+foreach_plugin test
diff --git a/tests/test-help.sh b/tests/test-help.sh
index f1a988c..9bda3c2 100755
--- a/tests/test-help.sh
+++ b/tests/test-help.sh
@@ -31,6 +31,7 @@
 # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 # SUCH DAMAGE.
 
+source functions.sh
 set -e
 
 output="$(nbdkit --help)"
@@ -40,15 +41,11 @@ if [[ ! ( "$output" =~ dump-config ) ]]; then
     exit 1
 fi
 
-# Get a list of all plugins and run --help on all of them.
+# Run nbdkit plugin --help for each plugin.
 # However some of these tests are expected to fail.
-plugins="$(
-    cd ..;
-    ls -1 plugins/*/.libs/nbdkit-*-plugin.so plugins/*/nbdkit-*-plugin |
-        sed 's,^plugins/\([^/]*\)/.*,\1,'
-)"
-for p in $plugins; do
-    case "$p${NBDKIT_VALGRIND:+-valgrind}" in
+test ()
+{
+    case "$1${NBDKIT_VALGRIND:+-valgrind}" in
         vddk | vddk-valgrind)
             # VDDK won't run without special environment variables
             # being set, so ignore it.
@@ -59,7 +56,8 @@ for p in $plugins; do
             # Plugins written in scripting languages can't run under
valgrind.
             ;;
         *)
-            nbdkit $p --help
+            nbdkit $1 --help
             ;;
     esac
-done
+}
+foreach_plugin test
diff --git a/tests/test-version.sh b/tests/test-version.sh
index 39fe74a..262e1a2 100755
--- a/tests/test-version.sh
+++ b/tests/test-version.sh
@@ -31,6 +31,7 @@
 # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 # SUCH DAMAGE.
 
+source functions.sh
 set -e
 
 output="$(nbdkit --version)"
@@ -40,15 +41,11 @@ if [[ ! ( "$output" =~ ^nbdkit\ 1\. ) ]]; then
     exit 1
 fi
 
-# Get a list of all plugins and run --version on all of them.
+# Run nbdkit plugin --version for each plugin.
 # However some of these tests are expected to fail.
-plugins="$(
-    cd ..;
-    ls -1 plugins/*/.libs/nbdkit-*-plugin.so plugins/*/nbdkit-*-plugin |
-        sed 's,^plugins/\([^/]*\)/.*,\1,'
-)"
-for p in $plugins; do
-    case "$p${NBDKIT_VALGRIND:+-valgrind}" in
+test ()
+{
+    case "$1${NBDKIT_VALGRIND:+-valgrind}" in
         vddk | vddk-valgrind)
             # VDDK won't run without special environment variables
             # being set, so ignore it.
@@ -59,7 +56,8 @@ for p in $plugins; do
             # Plugins written in scripting languages can't run under
valgrind.
             ;;
         *)
-            nbdkit $p --version
+            nbdkit $1 --version
             ;;
     esac
-done
+}
+foreach_plugin test
-- 
2.19.0.rc0

This assumes bashisms, but bash is required to run the tests.

This is mostly simple refactoring.  Except for the test-memory*.sh
tests where nbdkit used to run in the foreground, but that seems to be
a consequence of some left over debugging.
---
 tests/functions.sh.in                  | 35 ++++++++++++++++++++
 tests/test-blocksize.sh                | 23 +++----------
 tests/test-cache.sh                    | 16 ++-------
 tests/test-cow.sh                      | 16 ++-------
 tests/test-data-7E.sh                  | 16 ++-------
 tests/test-data-base64.sh              | 16 ++-------
 tests/test-data-raw.sh                 | 16 ++-------
 tests/test-fua.sh                      | 45 ++++++++++----------------
 tests/test-ip.sh                       | 17 ++--------
 tests/test-log.sh                      | 16 ++-------
 tests/test-memory-largest-for-qemu.sh  | 19 ++---------
 tests/test-memory-largest.sh           | 20 ++----------
 tests/test-nozero.sh                   | 41 ++++++++---------------
 tests/test-offset2.sh                  | 16 ++-------
 tests/test-parallel-nbd.sh             | 15 ++-------
 tests/test-pattern-largest-for-qemu.sh | 16 ++-------
 tests/test-pattern-largest.sh          | 16 ++-------
 tests/test-pattern.sh                  | 16 ++-------
 tests/test-start.sh                    | 17 ++--------
 tests/test-tls-psk.sh                  | 18 ++---------
 tests/test-tls.sh                      | 17 ++--------
 tests/test-truncate1.sh                | 16 ++-------
 tests/test-truncate2.sh                | 16 ++-------
 23 files changed, 112 insertions(+), 347 deletions(-)

diff --git a/tests/functions.sh.in b/tests/functions.sh.in
index 4071384..01f3a60 100644
--- a/tests/functions.sh.in
+++ b/tests/functions.sh.in
@@ -32,6 +32,41 @@
 # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 # SUCH DAMAGE.
 
+# start_nbdkit args...
+#
+# Run nbdkit with args and wait for it to start up.  If it fails
+# to start up, exit with an error message.
+start_nbdkit ()
+{
+    # Find the -P <pidfile> argument.
+    pidfile+    for i in `seq 1 $#`; do
+        if [ "${!i}" = "-P" ]; then
+            j=$((i+1))
+            pidfile="${!j}"
+        fi
+    done
+    if [ "$pidfile" = "" ]; then
+        echo "$0: start_nbdkit: -P option not present on nbdkit command
line"
+        exit 1
+    fi
+
+    # Run nbdkit.
+    nbdkit "$@"
+
+    # Wait for the pidfile to appear.
+    for i in `seq 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
+}
+
 # foreach_plugin f
 #
 # For each plugin that was built, call test function f with the plugin
diff --git a/tests/test-blocksize.sh b/tests/test-blocksize.sh
index cb9b09f..3fe3723 100755
--- a/tests/test-blocksize.sh
+++ b/tests/test-blocksize.sh
@@ -31,6 +31,7 @@
 # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 # SUCH DAMAGE.
 
+source functions.sh
 set -e
 
 files="blocksize1.img blocksize1.log blocksize1.sock blocksize1.pid
@@ -72,27 +73,13 @@ cleanup ()
 trap cleanup INT QUIT TERM EXIT ERR
 
 # Run two parallel nbdkit; to compare the logs and see what changes.
-nbdkit -P blocksize1.pid -U blocksize1.sock \
+start_nbdkit -P blocksize1.pid -U blocksize1.sock \
        --filter=log file logfile=blocksize1.log blocksize1.img
-nbdkit -P blocksize2.pid -U blocksize2.sock --filter=blocksize \
+start_nbdkit -P blocksize2.pid -U blocksize2.sock --filter=blocksize \
        --filter=log file logfile=blocksize2.log blocksize2.img \
        minblock=1024 maxdata=512k maxlen=1M
-
-# We may have to wait a short time for the pid files to appear.
-for i in `seq 1 10`; do
-    if test -f blocksize1.pid && test -f blocksize2.pid; then
-        break
-    fi
-    sleep 1
-done
-
-pid1="$(cat blocksize1.pid)" || :
-pid2="$(cat blocksize2.pid)" || :
-
-if ! test -f blocksize1.pid || ! test -f blocksize2.pid; then
-    echo "$0: PID files were not created"
-    exit 1
-fi
+pid1="$(cat blocksize1.pid)"
+pid2="$(cat blocksize2.pid)"
 
 # Test behavior on short accesses.
 qemu-io -f raw -c 'r 1 1' -c 'w 10001 1' -c 'w -z 20001
1' \
diff --git a/tests/test-cache.sh b/tests/test-cache.sh
index 215bab1..81c85a3 100755
--- a/tests/test-cache.sh
+++ b/tests/test-cache.sh
@@ -31,6 +31,7 @@
 # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 # SUCH DAMAGE.
 
+source functions.sh
 set -e
 set -x
 
@@ -41,20 +42,7 @@ rm -f $files
 truncate -s 1G cache.img
 
 # Run nbdkit with the caching filter.
-nbdkit -P cache.pid -U cache.sock --filter=cache file cache.img
-
-# We may have to wait a short time for the pid file to appear.
-for i in `seq 1 10`; do
-    if test -f cache.pid; then
-        break
-    fi
-    sleep 1
-done
-if ! test -f cache.pid; then
-    echo "$0: PID file was not created"
-    exit 1
-fi
-
+start_nbdkit -P cache.pid -U cache.sock --filter=cache file cache.img
 pid="$(cat cache.pid)"
 
 # Kill the nbdkit process on exit.
diff --git a/tests/test-cow.sh b/tests/test-cow.sh
index f9b0649..abe6dbd 100755
--- a/tests/test-cow.sh
+++ b/tests/test-cow.sh
@@ -31,6 +31,7 @@
 # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 # SUCH DAMAGE.
 
+source functions.sh
 set -e
 set -x
 
@@ -42,20 +43,7 @@ guestfish -N cow-base.img=fs exit
 lastmod="$(stat -c "%y" cow-base.img)"
 
 # Run nbdkit with a COW overlay.
-nbdkit -P cow.pid -U cow.sock --filter=cow file cow-base.img
-
-# We may have to wait a short time for the pid file to appear.
-for i in `seq 1 10`; do
-    if test -f cow.pid; then
-        break
-    fi
-    sleep 1
-done
-if ! test -f cow.pid; then
-    echo "$0: PID file was not created"
-    exit 1
-fi
-
+start_nbdkit -P cow.pid -U cow.sock --filter=cow file cow-base.img
 pid="$(cat cow.pid)"
 
 # Kill the nbdkit process on exit.
diff --git a/tests/test-data-7E.sh b/tests/test-data-7E.sh
index 8327511..1ffc668 100755
--- a/tests/test-data-7E.sh
+++ b/tests/test-data-7E.sh
@@ -34,6 +34,7 @@
 # Test the data plugin creating a 7 EB partitioned disk, and
 # the partition filter on top.
 
+source functions.sh
 set -e
 set -x
 
@@ -47,7 +48,7 @@ if ! qemu-io --help >/dev/null; then
 fi
 
 # Run nbdkit.
-nbdkit -P data-7E.pid -U data-7E.sock \
+start_nbdkit -P data-7E.pid -U data-7E.sock \
        --filter=partition \
        data size=7E partition=1 \
        data="
@@ -86,19 +87,6 @@ nbdkit -P data-7E.pid -U data-7E.sock \
                  0xdf 0xff 0xff 0xff 0xff 0xff 0x37 0
           0x80 0 0 0 0x80 0 0 0  0x79 0x8a 0xd0 0x7e 0 0 0 0
    "
-
-# We may have to wait a short time for the pid file to appear.
-for i in `seq 1 10`; do
-    if test -f data-7E.pid; then
-        break
-    fi
-    sleep 1
-done
-if ! test -f data-7E.pid; then
-    echo "$0: PID file was not created"
-    exit 1
-fi
-
 pid="$(cat data-7E.pid)"
 
 # Kill the nbdkit process on exit.
diff --git a/tests/test-data-base64.sh b/tests/test-data-base64.sh
index 6ff324b..be9f25a 100755
--- a/tests/test-data-base64.sh
+++ b/tests/test-data-base64.sh
@@ -33,6 +33,7 @@
 
 # Test the data plugin with base64= parameter.
 
+source functions.sh
 set -e
 set -x
 
@@ -52,21 +53,8 @@ if ! qemu-io --help >/dev/null; then
 fi
 
 # Run nbdkit.
-nbdkit -P data-base64.pid -U data-base64.sock \
+start_nbdkit -P data-base64.pid -U data-base64.sock \
        data base64=MTIz size=512
-
-# We may have to wait a short time for the pid file to appear.
-for i in `seq 1 10`; do
-    if test -f data-base64.pid; then
-        break
-    fi
-    sleep 1
-done
-if ! test -f data-base64.pid; then
-    echo "$0: PID file was not created"
-    exit 1
-fi
-
 pid="$(cat data-base64.pid)"
 
 # Kill the nbdkit process on exit.
diff --git a/tests/test-data-raw.sh b/tests/test-data-raw.sh
index c6652b7..d875777 100755
--- a/tests/test-data-raw.sh
+++ b/tests/test-data-raw.sh
@@ -33,6 +33,7 @@
 
 # Test the data plugin with raw= parameter.
 
+source functions.sh
 set -e
 set -x
 
@@ -46,21 +47,8 @@ if ! qemu-io --help >/dev/null; then
 fi
 
 # Run nbdkit.
-nbdkit -P data-raw.pid -U data-raw.sock \
+start_nbdkit -P data-raw.pid -U data-raw.sock \
        data raw=123 size=512
-
-# We may have to wait a short time for the pid file to appear.
-for i in `seq 1 10`; do
-    if test -f data-raw.pid; then
-        break
-    fi
-    sleep 1
-done
-if ! test -f data-raw.pid; then
-    echo "$0: PID file was not created"
-    exit 1
-fi
-
 pid="$(cat data-raw.pid)"
 
 # Kill the nbdkit process on exit.
diff --git a/tests/test-fua.sh b/tests/test-fua.sh
index f95aa18..88c14b5 100755
--- a/tests/test-fua.sh
+++ b/tests/test-fua.sh
@@ -31,6 +31,7 @@
 # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 # SUCH DAMAGE.
 
+source functions.sh
 set -e
 set -x
 
@@ -83,34 +84,22 @@ trap cleanup INT QUIT TERM EXIT ERR
 # 2: fuamode=emulate: log shows that blocksize optimizes fua to flush
 # 3: fuamode=native: log shows that blocksize preserves fua
 # 4: fuamode=force: log shows that fua is always enabled
-nbdkit -P fua1.pid -U fua1.sock --filter=log --filter=fua \
-       file logfile=fua1.log fua.img
-nbdkit -P fua2.pid -U fua2.sock --filter=blocksize --filter=log --filter=fua \
-       file logfile=fua2.log fua.img fuamode=emulate maxdata=4k maxlen=4k
-nbdkit -P fua3.pid -U fua3.sock --filter=blocksize --filter=log --filter=fua \
-       file logfile=fua3.log fua.img fuamode=native maxdata=4k maxlen=4k
-nbdkit -P fua4.pid -U fua4.sock --filter=fua --filter=log \
-       file logfile=fua4.log fua.img fuamode=force
-
-# We may have to wait a short time for the pid files to appear.
-for i in `seq 1 10`; do
-    if test -f fua1.pid && test -f fua2.pid && test -f fua3.pid
&&
-       test -f fua4.pid; then
-        break
-    fi
-    sleep 1
-done
-
-pid1="$(cat fua1.pid)" || :
-pid2="$(cat fua2.pid)" || :
-pid3="$(cat fua3.pid)" || :
-pid4="$(cat fua4.pid)" || :
-
-if ! test -f fua1.pid || ! test -f fua2.pid || ! test -f fua3.pid ||
-   ! test -f fua4.pid; then
-    echo "$0: PID files were not created"
-    exit 1
-fi
+start_nbdkit -P fua1.pid -U fua1.sock \
+             --filter=log --filter=fua \
+             file logfile=fua1.log fua.img
+start_nbdkit -P fua2.pid -U fua2.sock \
+             --filter=blocksize --filter=log --filter=fua \
+             file logfile=fua2.log fua.img fuamode=emulate maxdata=4k maxlen=4k
+start_nbdkit -P fua3.pid -U fua3.sock \
+             --filter=blocksize --filter=log --filter=fua \
+             file logfile=fua3.log fua.img fuamode=native maxdata=4k maxlen=4k
+start_nbdkit -P fua4.pid -U fua4.sock \
+             --filter=fua --filter=log \
+             file logfile=fua4.log fua.img fuamode=force
+pid1="$(cat fua1.pid)"
+pid2="$(cat fua2.pid)"
+pid3="$(cat fua3.pid)"
+pid4="$(cat fua4.pid)"
 
 # Perform a flush, write, and zero, first without then with FUA
 for f in '' -f; do
diff --git a/tests/test-ip.sh b/tests/test-ip.sh
index 12448c4..10c6f88 100755
--- a/tests/test-ip.sh
+++ b/tests/test-ip.sh
@@ -34,8 +34,8 @@
 # Every other test uses a Unix domain socket.  This tests nbdkit over
 # IPv4 and IPv6 localhost connections.
 
+source functions.sh
 set -e
-source ./functions.sh
 
 rm -f ip.pid ipv4.out ipv6.out
 
@@ -65,20 +65,7 @@ echo picked unused port $port
 
 # By default nbdkit will listen on all available interfaces, ie.
 # IPv4 and IPv6.
-nbdkit -P ip.pid -p $port example1
-
-# We may have to wait a short time for the pid file to appear.
-for i in `seq 1 10`; do
-    if test -f ip.pid; then
-        break
-    fi
-    sleep 1
-done
-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.
diff --git a/tests/test-log.sh b/tests/test-log.sh
index f811de4..1cddb7b 100755
--- a/tests/test-log.sh
+++ b/tests/test-log.sh
@@ -31,6 +31,7 @@
 # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 # SUCH DAMAGE.
 
+source functions.sh
 set -e
 
 files="log.img log.log log.sock log.pid"
@@ -44,20 +45,7 @@ if ! qemu-io -f raw -c 'w 1M 2M' log.img; then
 fi
 
 # Run nbdkit with logging enabled to file.
-nbdkit -P log.pid -U log.sock --filter=log file log.img logfile=log.log
-
-# We may have to wait a short time for the pid file to appear.
-for i in `seq 1 10`; do
-    if test -f log.pid; then
-        break
-    fi
-    sleep 1
-done
-if ! test -f log.pid; then
-    echo "$0: PID file was not created"
-    exit 1
-fi
-
+start_nbdkit -P log.pid -U log.sock --filter=log file log.img logfile=log.log
 pid="$(cat log.pid)"
 
 # Kill the nbdkit process on exit.
diff --git a/tests/test-memory-largest-for-qemu.sh
b/tests/test-memory-largest-for-qemu.sh
index 9bc498d..325919d 100755
--- a/tests/test-memory-largest-for-qemu.sh
+++ b/tests/test-memory-largest-for-qemu.sh
@@ -34,6 +34,7 @@
 # Test the memory plugin with the largest possible size supported
 # by qemu and nbdkit.
 
+source functions.sh
 set -e
 
 files="memory-largest-for-qemu.out memory-largest-for-qemu.pid
memory-largest-for-qemu.sock"
@@ -47,22 +48,8 @@ fi
 
 # Run nbdkit with memory plugin.
 # size = (2^63-1) & ~511 which is the largest supported by qemu.
-nbdkit -f -v -D memory.dir=1 \
-       -P memory-largest-for-qemu.pid -U memory-largest-for-qemu.sock \
-       memory size=9223372036854775296 &
-
-# We may have to wait a short time for the pid file to appear.
-for i in `seq 1 10`; do
-    if test -f memory-largest-for-qemu.pid; then
-        break
-    fi
-    sleep 1
-done
-if ! test -f memory-largest-for-qemu.pid; then
-    echo "$0: PID file was not created"
-    exit 1
-fi
-
+start_nbdkit -P memory-largest-for-qemu.pid -U memory-largest-for-qemu.sock \
+       memory size=9223372036854775296
 pid="$(cat memory-largest-for-qemu.pid)"
 
 # Kill the nbdkit process on exit.
diff --git a/tests/test-memory-largest.sh b/tests/test-memory-largest.sh
index d0a6c77..b2115f7 100755
--- a/tests/test-memory-largest.sh
+++ b/tests/test-memory-largest.sh
@@ -34,6 +34,7 @@
 # Test the memory plugin with the largest possible size supported
 # by nbdkit.
 
+source functions.sh
 set -e
 
 files="memory-largest.out memory-largest.pid memory-largest.sock"
@@ -47,23 +48,8 @@ fi
 
 # Run nbdkit with memory plugin.
 # size = 2^63-1
-nbdkit -f -v \
-       -D memory.dir=1 \
-       -P memory-largest.pid -U memory-largest.sock \
-       memory size=9223372036854775807 &
-
-# We may have to wait a short time for the pid file to appear.
-for i in `seq 1 10`; do
-    if test -f memory-largest.pid; then
-        break
-    fi
-    sleep 1
-done
-if ! test -f memory-largest.pid; then
-    echo "$0: PID file was not created"
-    exit 1
-fi
-
+start_nbdkit -P memory-largest.pid -U memory-largest.sock \
+       memory size=9223372036854775807
 pid="$(cat memory-largest.pid)"
 
 # Kill the nbdkit process on exit.
diff --git a/tests/test-nozero.sh b/tests/test-nozero.sh
index 904d822..33456e1 100755
--- a/tests/test-nozero.sh
+++ b/tests/test-nozero.sh
@@ -31,6 +31,7 @@
 # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 # SUCH DAMAGE.
 
+source functions.sh
 set -e
 
 files="nozero1.img nozero1.log nozero1.sock nozero1.pid
@@ -101,40 +102,24 @@ trap cleanup INT QUIT TERM EXIT ERR
 # 4: log after filter with zeromode=emulate, to ensure no ZERO to plugin
 # 5a/b: both sides of nbd plugin: even though server side does not advertise
 # ZERO, the client side still exposes it, and just skips calling nbd's
.zero
-nbdkit -P nozero1.pid -U nozero1.sock --filter=log \
+start_nbdkit -P nozero1.pid -U nozero1.sock --filter=log \
        file logfile=nozero1.log nozero1.img
-nbdkit -P nozero2.pid -U nozero2.sock --filter=log --filter=nozero \
+start_nbdkit -P nozero2.pid -U nozero2.sock --filter=log --filter=nozero \
        file logfile=nozero2.log nozero2.img
-nbdkit -P nozero3.pid -U nozero3.sock --filter=log --filter=nozero \
+start_nbdkit -P nozero3.pid -U nozero3.sock --filter=log --filter=nozero \
        file logfile=nozero3.log nozero3.img zeromode=emulate
-nbdkit -P nozero4.pid -U nozero4.sock --filter=nozero --filter=log \
+start_nbdkit -P nozero4.pid -U nozero4.sock --filter=nozero --filter=log \
        file logfile=nozero4.log nozero4.img zeromode=emulate
-nbdkit -P nozero5a.pid -U nozero5a.sock --filter=log --filter=nozero \
+start_nbdkit -P nozero5a.pid -U nozero5a.sock --filter=log --filter=nozero \
        file logfile=nozero5a.log nozero5.img
-nbdkit -P nozero5b.pid -U nozero5b.sock --filter=log \
+start_nbdkit -P nozero5b.pid -U nozero5b.sock --filter=log \
        nbd logfile=nozero5b.log socket=nozero5a.sock
-
-# We may have to wait a short time for the pid files to appear.
-for i in `seq 1 10`; do
-    if test -f nozero1.pid && test -f nozero2.pid && test -f
nozero3.pid &&
-       test -f nozero4.pid && test -f nozero5a.pid && test -f
nozero5b.pid; then
-        break
-    fi
-    sleep 1
-done
-
-pid1="$(cat nozero1.pid)" || :
-pid2="$(cat nozero2.pid)" || :
-pid3="$(cat nozero3.pid)" || :
-pid4="$(cat nozero4.pid)" || :
-pid5a="$(cat nozero5a.pid)" || :
-pid5b="$(cat nozero5b.pid)" || :
-
-if ! test -f nozero1.pid || ! test -f nozero2.pid || ! test -f nozero3.pid ||
-   ! test -f nozero4.pid || ! test -f nozero5a.pid || ! test -f nozero5b.pid;
then
-    echo "$0: PID files were not created"
-    exit 1
-fi
+pid1="$(cat nozero1.pid)"
+pid2="$(cat nozero2.pid)"
+pid3="$(cat nozero3.pid)"
+pid4="$(cat nozero4.pid)"
+pid5a="$(cat nozero5a.pid)"
+pid5b="$(cat nozero5b.pid)"
 
 # Perform the zero write.
 qemu-io -f raw -c 'w -z -u 0 1M'
'nbd+unix://?socket=nozero1.sock'
diff --git a/tests/test-offset2.sh b/tests/test-offset2.sh
index c37e060..ec1ad91 100755
--- a/tests/test-offset2.sh
+++ b/tests/test-offset2.sh
@@ -33,6 +33,7 @@
 
 # Additional test of the offset filter using the pattern plugin.
 
+source functions.sh
 set -e
 set -x
 
@@ -47,23 +48,10 @@ fi
 
 # Run nbdkit with pattern plugin and offset filter in front.
 # 8070450532247927809 = 7E - 1023
-nbdkit -P offset2.pid -U offset2.sock \
+start_nbdkit -P offset2.pid -U offset2.sock \
        --filter=offset \
        pattern size=7E \
        offset=8070450532247927809 range=512
-
-# We may have to wait a short time for the pid file to appear.
-for i in `seq 1 10`; do
-    if test -f offset2.pid; then
-        break
-    fi
-    sleep 1
-done
-if ! test -f offset2.pid; then
-    echo "$0: PID file was not created"
-    exit 1
-fi
-
 pid="$(cat offset2.pid)"
 
 # Kill the nbdkit process on exit.
diff --git a/tests/test-parallel-nbd.sh b/tests/test-parallel-nbd.sh
index cb70f46..223d953 100755
--- a/tests/test-parallel-nbd.sh
+++ b/tests/test-parallel-nbd.sh
@@ -31,6 +31,8 @@
 # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 # SUCH DAMAGE.
 
+source functions.sh
+
 # Check file-data was created by Makefile and qemu-io exists.
 if ! test -f file-data; then
     echo "$0: missing file-data"
@@ -61,21 +63,10 @@ qemu-io -f raw -c "aio_write -P 1 0 512" -c
"aio_write -P 2 512 512" \
 # may have spurious failures under heavy loads on the test machine, where
 # tuning the delays may help.
 
-nbdkit --exit-with-parent -v \
+start_nbdkit --exit-with-parent -v \
     -U test-parallel-nbd.sock -P test-parallel-nbd.pid \
     --filter=delay \
     file test-parallel-nbd.data wdelay=2 rdelay=1 &
-# We may have to wait a short time for the pid file to appear.
-for i in `seq 1 10`; do
-    if test -f test-parallel-nbd.pid; then
-        break
-    fi
-    sleep 1
-done
-if ! test -f test-parallel-nbd.pid; then
-    echo "$0: PID file was not created"
-    exit 1
-fi
 
 # With --threads=1, the write should complete first because it was issued first
 nbdkit -v -t 1 -U - nbd socket=test-parallel-nbd.sock --run '
diff --git a/tests/test-pattern-largest-for-qemu.sh
b/tests/test-pattern-largest-for-qemu.sh
index cf70354..0900f94 100755
--- a/tests/test-pattern-largest-for-qemu.sh
+++ b/tests/test-pattern-largest-for-qemu.sh
@@ -34,6 +34,7 @@
 # Test the pattern plugin with the largest possible size supported
 # by qemu and nbdkit.
 
+source functions.sh
 set -e
 
 files="pattern-largest-for-qemu.out pattern-largest-for-qemu.pid
pattern-largest-for-qemu.sock"
@@ -47,21 +48,8 @@ fi
 
 # Run nbdkit with pattern plugin.
 # size = (2^63-1) & ~511 which is the largest supported by qemu.
-nbdkit -P pattern-largest-for-qemu.pid -U pattern-largest-for-qemu.sock \
+start_nbdkit -P pattern-largest-for-qemu.pid -U pattern-largest-for-qemu.sock \
        pattern size=9223372036854775296
-
-# We may have to wait a short time for the pid file to appear.
-for i in `seq 1 10`; do
-    if test -f pattern-largest-for-qemu.pid; then
-        break
-    fi
-    sleep 1
-done
-if ! test -f pattern-largest-for-qemu.pid; then
-    echo "$0: PID file was not created"
-    exit 1
-fi
-
 pid="$(cat pattern-largest-for-qemu.pid)"
 
 # Kill the nbdkit process on exit.
diff --git a/tests/test-pattern-largest.sh b/tests/test-pattern-largest.sh
index 2a903bf..8accc14 100755
--- a/tests/test-pattern-largest.sh
+++ b/tests/test-pattern-largest.sh
@@ -34,6 +34,7 @@
 # Test the pattern plugin with the largest possible size supported
 # by nbdkit.
 
+source functions.sh
 set -e
 
 files="pattern-largest.out pattern-largest.pid pattern-largest.sock"
@@ -47,21 +48,8 @@ fi
 
 # Run nbdkit with pattern plugin.
 # size = 2^63-1
-nbdkit -P pattern-largest.pid -U pattern-largest.sock \
+start_nbdkit -P pattern-largest.pid -U pattern-largest.sock \
        pattern size=9223372036854775807
-
-# We may have to wait a short time for the pid file to appear.
-for i in `seq 1 10`; do
-    if test -f pattern-largest.pid; then
-        break
-    fi
-    sleep 1
-done
-if ! test -f pattern-largest.pid; then
-    echo "$0: PID file was not created"
-    exit 1
-fi
-
 pid="$(cat pattern-largest.pid)"
 
 # Kill the nbdkit process on exit.
diff --git a/tests/test-pattern.sh b/tests/test-pattern.sh
index 68022c9..4ff2996 100755
--- a/tests/test-pattern.sh
+++ b/tests/test-pattern.sh
@@ -38,6 +38,7 @@
 # what read parameters we give it.  Hence these tests are rather
 # limited. (XXX)
 
+source functions.sh
 set -e
 
 files="pattern.out pattern.pid pattern.sock"
@@ -50,20 +51,7 @@ if ! qemu-io --help >/dev/null; then
 fi
 
 # Run nbdkit with pattern plugin.
-nbdkit -P pattern.pid -U pattern.sock pattern size=1G
-
-# We may have to wait a short time for the pid file to appear.
-for i in `seq 1 10`; do
-    if test -f pattern.pid; then
-        break
-    fi
-    sleep 1
-done
-if ! test -f pattern.pid; then
-    echo "$0: PID file was not created"
-    exit 1
-fi
-
+start_nbdkit -P pattern.pid -U pattern.sock pattern size=1G
 pid="$(cat pattern.pid)"
 
 # Kill the nbdkit process on exit.
diff --git a/tests/test-start.sh b/tests/test-start.sh
index dc578e7..e866688 100755
--- a/tests/test-start.sh
+++ b/tests/test-start.sh
@@ -31,29 +31,16 @@
 # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 # SUCH DAMAGE.
 
+source functions.sh
 set -e
 set -x
-source ./functions.sh
 
 # Test nbdkit starts up, forks in the background, writes a PID file,
 # and can be killed.
 
 rm -f start.pid start.sock
 
-nbdkit -P start.pid -U start.sock example1
-
-# We may have to wait a short time for the pid file to appear.
-for i in `seq 1 10`; do
-    if test -f start.pid; then
-        break
-    fi
-    sleep 1
-done
-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.
diff --git a/tests/test-tls-psk.sh b/tests/test-tls-psk.sh
index 99c5945..f561588 100755
--- a/tests/test-tls-psk.sh
+++ b/tests/test-tls-psk.sh
@@ -31,9 +31,9 @@
 # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 # SUCH DAMAGE.
 
+source functions.sh
 set -e
 set -x
-source ./functions.sh
 
 # Don't fail if certain commands aren't available.
 if ! ss --version; then
@@ -78,20 +78,8 @@ for port in `seq 51000 65535`; do
 done
 echo picked unused port $port
 
-nbdkit -P tls-psk.pid -p $port -n --tls=require --tls-psk=keys.psk example1
-
-# We may have to wait a short time for the pid file to appear.
-for i in `seq 1 10`; do
-    if test -f tls-psk.pid; then
-        break
-    fi
-    sleep 1
-done
-if ! test -f tls-psk.pid; then
-    echo "$0: PID file was not created"
-    exit 1
-fi
-
+start_nbdkit -P tls-psk.pid -p $port -n \
+             --tls=require --tls-psk=keys.psk example1
 pid="$(cat tls-psk.pid)"
 
 # Kill the process on exit.
diff --git a/tests/test-tls.sh b/tests/test-tls.sh
index 71bd8a4..79f08d3 100755
--- a/tests/test-tls.sh
+++ b/tests/test-tls.sh
@@ -31,9 +31,9 @@
 # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 # SUCH DAMAGE.
 
+source functions.sh
 set -e
 set -x
-source ./functions.sh
 
 # Don't fail if certain commands aren't available.
 if ! ss --version; then
@@ -71,21 +71,8 @@ for port in `seq 50000 65535`; do
 done
 echo picked unused port $port
 
-nbdkit -P tls.pid -p $port -n --tls=require \
+start_nbdkit -P tls.pid -p $port -n --tls=require \
        --tls-certificates="$pkidir" example1
-
-# We may have to wait a short time for the pid file to appear.
-for i in `seq 1 10`; do
-    if test -f tls.pid; then
-        break
-    fi
-    sleep 1
-done
-if ! test -f tls.pid; then
-    echo "$0: PID file was not created"
-    exit 1
-fi
-
 pid="$(cat tls.pid)"
 
 # Kill the process on exit.
diff --git a/tests/test-truncate1.sh b/tests/test-truncate1.sh
index 719f63b..fb5ef65 100755
--- a/tests/test-truncate1.sh
+++ b/tests/test-truncate1.sh
@@ -33,6 +33,7 @@
 
 # Test the truncate filter using the pattern plugin.
 
+source functions.sh
 set -e
 set -x
 
@@ -46,23 +47,10 @@ if ! qemu-io --help >/dev/null; then
 fi
 
 # Run nbdkit with pattern plugin and truncate filter in front.
-nbdkit -P truncate1.pid -U truncate1.sock \
+start_nbdkit -P truncate1.pid -U truncate1.sock \
        --filter=truncate \
        pattern size=503 \
        truncate=512
-
-# We may have to wait a short time for the pid file to appear.
-for i in `seq 1 10`; do
-    if test -f truncate1.pid; then
-        break
-    fi
-    sleep 1
-done
-if ! test -f truncate1.pid; then
-    echo "$0: PID file was not created"
-    exit 1
-fi
-
 pid="$(cat truncate1.pid)"
 
 # Kill the nbdkit process on exit.
diff --git a/tests/test-truncate2.sh b/tests/test-truncate2.sh
index 0295b96..6dd7ba0 100755
--- a/tests/test-truncate2.sh
+++ b/tests/test-truncate2.sh
@@ -33,6 +33,7 @@
 
 # Test the truncate filter using the pattern plugin.
 
+source functions.sh
 set -e
 set -x
 
@@ -46,23 +47,10 @@ if ! qemu-io --help >/dev/null; then
 fi
 
 # Run nbdkit with pattern plugin and truncate filter in front.
-nbdkit -P truncate2.pid -U truncate2.sock \
+start_nbdkit -P truncate2.pid -U truncate2.sock \
        --filter=truncate \
        pattern size=503 \
        round-up=512
-
-# We may have to wait a short time for the pid file to appear.
-for i in `seq 1 10`; do
-    if test -f truncate2.pid; then
-        break
-    fi
-    sleep 1
-done
-if ! test -f truncate2.pid; then
-    echo "$0: PID file was not created"
-    exit 1
-fi
-
 pid="$(cat truncate2.pid)"
 
 # Kill the nbdkit process on exit.
-- 
2.19.0.rc0

On 9/11/18 1:47 PM, Richard W.M. Jones wrote:
> It's easier to get it to other places if it starts out in the
> configure script.
> 
> Also split the list into language and non-language plugins.
> ---
>   common-rules.mk | 44 ----------------------------------------
>   configure.ac    | 53 +++++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 53 insertions(+), 44 deletions(-)
> 
> +++ b/configure.ac
> @@ -542,6 +542,59 @@ AC_ARG_ENABLE([vddk],[
>       [enable_vddk=yes])
>   AM_CONDITIONAL([HAVE_VDDK], [test "x$enable_vddk" =
"xyes"])
>   
> +dnl List of plugins and filters.
> +lang_plugins="\
> +        lua \
> +        ocaml \
> +        perl \
> +        python \
> +        ruby \
> +        sh \
> +        tcl \
> +        "
> +non_lang_plugins="\
> +        curl \
> +        data \
> +	example1 \
> +	example2 \
Inconsistent spacing (space-vs-tab doesn't matter in this particular 
context, but it looks weird to use both)
> +        "
> +plugins="$(echo $lang_plugins $non_lang_plugins | xargs -n1 | sort -u
| xargs)"
Your use of xargs as a reformatter is interesting ;) Could also be spelled:

$(echo $lang_plugins $non_lang_plugins | tr -s ' \t\n' ' ' |
sort -u |
tr ' ' '\n')"

but that's longer to type, so your version is fine.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

On 9/11/18 1:47 PM, Richard W.M. Jones wrote:
> This resurrects the unused tests/functions.sh file (although now we
> need to generate it from tests/functions.sh.in).  Put the common code
> for running a test against every plugin here.
> 
> Because of the previous commits we can now use the plugins list
> directly from configure.ac instead of needing to use weird shell
> script, although we still need to preserve the test that the plugin
> was built so that the tests continue to work if a plugin is disabled
> or not built through missing dependencies.
> ---
>   .gitignore                              |  1 +
>   configure.ac                            |  1 +
>   tests/Makefile.am                       |  1 -
>   tests/{functions.sh => functions.sh.in} | 23 +++++++++++++++++++----
>   tests/test-dump-plugin.sh               | 19 ++++++++-----------
>   tests/test-help.sh                      | 18 ++++++++----------
>   tests/test-version.sh                   | 18 ++++++++----------
>   7 files changed, 45 insertions(+), 36 deletions(-)
> 
> +# foreach_plugin f
> +#
> +# For each plugin that was built, call test function f with the plugin
> +# name as argument.
> +foreach_plugin ()
> +{
> +    for p in @plugins@; do
> +        # Was the plugin built?
> +        d=@top_builddir@/plugins/$p
> +        if [ -f $d/.libs/nbdkit-$p-plugin.so ] ||
> +           [ -f $d/nbdkit-$p-plugin ]; then
A bit cavalier on not quoting arguments to [; but works since neither $d 
nor $p should contain spaces.
> +            # Yes so run the test.
> +            "$1" "$p"
Looks funny to quote $p here but not above.
> +++ b/tests/test-dump-plugin.sh
> @@ -31,9 +31,9 @@
>   # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
>   # SUCH DAMAGE.
>   
> +source functions.sh
>   set -e
>   set -x
> -source ./functions.sh
>   
You NEED to use ./ with source (otherwise, you perform a PATH search, 
which fails if ./ is not in PATH); this change is a regression.

(Whether you use the shorter '.' instead of the longer bashism
'source'
is up to you, since the tests all use bash)

> +++ b/tests/test-help.sh
> @@ -31,6 +31,7 @@
>   # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
>   # SUCH DAMAGE.
>   
> +source functions.sh
>   set -e
Multiple spots in your patch need ./

Otherwise the refactoring looks sane.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

On 9/11/18 1:47 PM, Richard W.M. Jones wrote:
> This assumes bashisms, but bash is required to run the tests.
> 
> This is mostly simple refactoring.  Except for the test-memory*.sh
> tests where nbdkit used to run in the foreground, but that seems to be
> a consequence of some left over debugging.
> ---
> +++ b/tests/functions.sh.in
> @@ -32,6 +32,41 @@
>   # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
>   # SUCH DAMAGE.
>   
> +# start_nbdkit args...
> +#
> +# Run nbdkit with args and wait for it to start up.  If it fails
> +# to start up, exit with an error message.
> +start_nbdkit ()
> +{
> +    # Find the -P <pidfile> argument.
> +    pidfile> +    for i in `seq 1 $#`; do
Is seq universally available, even on BSD?  It's not in POSIX.  Since 
you're already using bashisms, you could instead do

for (( i=1; i <= $#; i++)); do

or even:

for i in {1..$#}; do

and drop the need for seq or even a fork (the former works regardless of 
$#, the latter has poor scaling effects if $# is large since it is 
expanded in advance - but then again, none of our tests pass that large 
of $#)
> +        if [ "${!i}" = "-P" ]; then
Unspecified behavior if ${!i} evaluates to "!" (which none of the
tests
do).  Since you're already using bashisms, you could instead do

if [[ ${!i} == -P ]]; then
> +            j=$((i+1))
> +            pidfile="${!j}"
> +        fi
> +    done
This parse loop requires tests to spell it "-P /path/to/file" as two 
separate args, rather than also permitting "-P/path/to/file".  A 
reasonable restriction but might be worth a comment.
> +    if [ "$pidfile" = "" ]; then
> +        echo "$0: start_nbdkit: -P option not present on nbdkit
command line"
> +        exit 1
> +    fi
> +
> +    # Run nbdkit.
> +    nbdkit "$@"
You could also change the contract of this function to REQUIRE the pid 
filename be first, so that you don't have to hunt over $# where -P 
lives, something like:

start_nbdkit()
{
   pidfile=$1
   shift
   nbdkit -P $pidfile "$@"

and change callers to look like:

start_nbdkit foo.pid --myoption1 myplugin ...
> +
> +    # Wait for the pidfile to appear.
> +    for i in `seq 1 10`; do
I guess we were already relying on seq.  Could also be spelled {1..10}
> +        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
> +}
> +
> +++ b/tests/test-blocksize.sh
> @@ -31,6 +31,7 @@
>   # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
>   # SUCH DAMAGE.
>   
> +source functions.sh
>   set -e
More need for source ./functions.sh throughout the patch
>   
>   files="blocksize1.img blocksize1.log blocksize1.sock blocksize1.pid
> @@ -72,27 +73,13 @@ cleanup ()
>   trap cleanup INT QUIT TERM EXIT ERR
>   
>   # Run two parallel nbdkit; to compare the logs and see what changes.
> -nbdkit -P blocksize1.pid -U blocksize1.sock \
> +start_nbdkit -P blocksize1.pid -U blocksize1.sock \
>          --filter=log file logfile=blocksize1.log blocksize1.img
> -nbdkit -P blocksize2.pid -U blocksize2.sock --filter=blocksize \
> +start_nbdkit -P blocksize2.pid -U blocksize2.sock --filter=blocksize \
>          --filter=log file logfile=blocksize2.log blocksize2.img \
>          minblock=1024 maxdata=512k maxlen=1M
If the first process starts but the second fails, then you have not set 
pid1=, and leak the first process.  This is a regression from the old 
code, which managed to capture pid1 before triggering the trap to 
cleanup(), and thus killed the successful nbdkit.  Several tests are 
impacted.

Less importantly, the old code performed startup in parallel (kick off 
both nbdkit processes, then start the loop; if a pid file was not 
produced by either one of the nbdkit processes, the test determined that 
within 10 seconds); now startup is serial (you don't start a second 
nbdkit until the first one has succeeded). For a test with two nbdkit 
processes, that could expand typical test time from 1 second to 2 (if 
the pid file creation is not instantaneous, but the first sleep 1 was 
sufficient for both processes to be ready); and expand worst-case 
failure detection under heavy load from 10 into 20 seconds (if the first 
succeeded on the last iteration, but the second timed out).  I don't see 
anything wrong with the change, although it might be worth a mention in 
the commit message as intentional.

I'm also seeing a common pattern of assigning pid=$(cat file.pid) right 
after calling start_nbdkt.  Would it be worth changing the signature of 
start_nbdkit() to also populate a pid variable on success, as in:

start_nbdkit blocksize1.pid pid1 myarg1

start_nbdkit()
{
   pidfile=$1
   pidvar=$2
   shift 2
   ...
   pid=$(cat $pidfile)
   eval $pidvar=$pid

which would also have the added benefit of avoiding the regression of 
pid1 not being set if the second nbdkit process doesn't start?

But overall, I like the idea of refactoring the common code for less 
repetition.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org