[Libguestfs] [PATCH nbdkit 3/4] tests: Move common code for testing every plugin to tests/functions.sh.

[Eric Blake]

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