Libguestfs - Jan 2019 - Re: [PATCH v2 nbdkit] tests: Add generic ‘requires’ function for test prerequisites.

On 1/23/19 9:36 AM, Richard W.M. Jones wrote:
> +++ b/tests/functions.sh.in
> @@ -1,7 +1,7 @@
>  # nbdkit
>  # Common functions used by the tests.
>  # @configure_input@
> -# Copyright (C) 2017-2018 Red Hat Inc.
> +# Copyright (C) 2017-2019 Red Hat Inc.
>  # All rights reserved.
>  #
>  # Redistribution and use in source and binary forms, with or without
> @@ -57,6 +57,21 @@ _run_cleanup_hooks ()
>  }
>  trap _run_cleanup_hooks INT QUIT TERM EXIT ERR
>  
> +# requires program [args]
> +#
> +# Check that ‘program [args]’ works.  If not, skip the test.
> +# For example to check that qemu-img is available, do:
> +#
> +# requires qemu-img --version
> +requires ()
> +{
> +    ( "$@" ) </dev/null >/dev/null 2>&1 || {
> +        echo "$0: ‘$*’ failed with error code $?"
> +        echo "$0: test prerequisite is missing or not working"
> +        exit 77
> +    }
> +}
Updated function looks good.
> +++ b/tests/test-ip.sh
> @@ -37,26 +37,17 @@
>  source ./functions.sh
>  set -e
>  
> -rm -f ip.pid ipv4.out ipv6.out
> -cleanup_fn rm -f ip.pid ipv4.out ipv6.out
> +requires ss --version
> +requires ip -V
> +requires qemu-img --version
>  
> -# Don't fail if certain commands aren't available.
> -if ! ss --version; then
> -    echo "$0: 'ss' command not available"
> -    exit 77
> -fi
> -if ! command -v qemu-img; then
> -    echo "$0: 'qemu-img' command not available"
> -    exit 77
> -fi
>  if ! qemu-img --help | grep -- --image-opts; then
>      echo "$0: 'qemu-img' command does not have the
--image-opts option"
>      exit 77
The old test insists on --image-opts,
>  fi
> -if ! ip -V; then
> -    echo "$0: 'ip' command not available"
> -    exit 77
> -fi
> +
> +rm -f ip.pid ipv4.out ipv6.out
> +cleanup_fn rm -f ip.pid ipv4.out ipv6.out
...but the rewrite does not.  Could be solved with:

requires qemu-img info --image-opts driver=file,filename=functions.sh
> +++ b/tests/test-partitioning2.sh
> @@ -40,16 +40,12 @@ source ./functions.sh
>  set -e
>  set -x
>  
> +requires mke2fs -V
> +
>  files="partitioning2.pid partitioning2.sock partitioning2.fs
partitioning2.p1 partitioning2.p3"
>  rm -f $files
>  cleanup_fn rm -f $files
>  
> -# Test that mke2fs works
> -if ! mke2fs -V; then
> -    echo "$0: missing or broken mke2fs"
> -    exit 77
> -fi
> -
>  # Create partitions before and after.
>  truncate -s 1 partitioning2.p1
'truncate' is not universally available; that may be something for a
future patch to add a requires line for (but not this patch, which
should be just the mechanical rewrites).
> +++ b/tests/test-tls-psk.sh
> @@ -35,15 +35,9 @@ source ./functions.sh
>  set -e
>  set -x
>  
> -# Don't fail if certain commands aren't available.
> -if ! ss --version; then
> -    echo "$0: 'ss' command not available"
> -    exit 77
> -fi
> -if ! command -v qemu-img > /dev/null; then
> -    echo "$0: 'qemu-img' command not available"
> -    exit 77
> -fi
> +requires ss --version
> +requires qemu-img --version
> +
>  if ! qemu-img --help | grep -- --object; then
There might be a way to express this via requires (similar to the way I
checked for --image-opts support above), via a harmless probe of a
known-existing file. But I didn't spend time coming up with such a
working command line probe (I got distracted by the fact that 'qemu-img
info --object nosuch file' has status 1 but no error message on stderr,
oops).

Looks like you're ready to go.

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

On Wed, Jan 23, 2019 at 10:02:18AM -0600, Eric Blake
wrote:
> > +++ b/tests/test-ip.sh
> > @@ -37,26 +37,17 @@
> >  source ./functions.sh
> >  set -e
> >  
> > -rm -f ip.pid ipv4.out ipv6.out
> > -cleanup_fn rm -f ip.pid ipv4.out ipv6.out
> > +requires ss --version
> > +requires ip -V
> > +requires qemu-img --version
> >  
> > -# Don't fail if certain commands aren't available.
> > -if ! ss --version; then
> > -    echo "$0: 'ss' command not available"
> > -    exit 77
> > -fi
> > -if ! command -v qemu-img; then
> > -    echo "$0: 'qemu-img' command not available"
> > -    exit 77
> > -fi
> >  if ! qemu-img --help | grep -- --image-opts; then
> >      echo "$0: 'qemu-img' command does not have the
--image-opts option"
> >      exit 77
> 
> The old test insists on --image-opts,
> 
> >  fi
> > -if ! ip -V; then
> > -    echo "$0: 'ip' command not available"
> > -    exit 77
> > -fi
> > +
> > +rm -f ip.pid ipv4.out ipv6.out
> > +cleanup_fn rm -f ip.pid ipv4.out ipv6.out
> 
> ...but the rewrite does not.  Could be solved with:
> 
> requires qemu-img info --image-opts driver=file,filename=functions.sh
Yes - as you can see I didn't convert the old test, it's still there.
However I will try your variation to see if that works instead.
> > +++ b/tests/test-partitioning2.sh
> > @@ -40,16 +40,12 @@ source ./functions.sh
> >  set -e
> >  set -x
> >  
> > +requires mke2fs -V
> > +
> >  files="partitioning2.pid partitioning2.sock partitioning2.fs
partitioning2.p1 partitioning2.p3"
> >  rm -f $files
> >  cleanup_fn rm -f $files
> >  
> > -# Test that mke2fs works
> > -if ! mke2fs -V; then
> > -    echo "$0: missing or broken mke2fs"
> > -    exit 77
> > -fi
> > -
> >  # Create partitions before and after.
> >  truncate -s 1 partitioning2.p1
> 
> 'truncate' is not universally available; that may be something for
a
> future patch to add a requires line for (but not this patch, which
> should be just the mechanical rewrites).
We had a problem on FreeBSD where truncate --size does not work (their
binary only understands truncate -s).  That was fixed by commit
f62e3712428.  Do you know what platforms would lack truncate entirely?

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

On 1/23/19 10:26 AM, Richard W.M. Jones wrote:
>>> -if ! command -v qemu-img; then
>>> -    echo "$0: 'qemu-img' command not available"
>>> -    exit 77
>>> -fi
>>>  if ! qemu-img --help | grep -- --image-opts; then
>>>      echo "$0: 'qemu-img' command does not have the
--image-opts option"
>>>      exit 77
>>
>> The old test insists on --image-opts,
>>
>>>  fi
>>> -if ! ip -V; then
>>> -    echo "$0: 'ip' command not available"
>>> -    exit 77
>>> -fi
>>> +
>>> +rm -f ip.pid ipv4.out ipv6.out
>>> +cleanup_fn rm -f ip.pid ipv4.out ipv6.out
>>
>> ...but the rewrite does not.  Could be solved with:
>>
>> requires qemu-img info --image-opts driver=file,filename=functions.sh
> 
> Yes - as you can see I didn't convert the old test, it's still
there.
Ah, my fault for not noticing the - lines, and thinking one entire block
was replaced by another.
> However I will try your variation to see if that works instead.
Can be a separate patch on top, as needed (in general, it's always nice
to do feature tests for features that are not universally available),
the real trick is coming up with harmless command line probes for those
features where the answer can be given via $?.

>>> -
>>>  # Create partitions before and after.
>>>  truncate -s 1 partitioning2.p1
>>
>> 'truncate' is not universally available; that may be something
for a
>> future patch to add a requires line for (but not this patch, which
>> should be just the mechanical rewrites).
> 
> We had a problem on FreeBSD where truncate --size does not work (their
> binary only understands truncate -s).  That was fixed by commit
> f62e3712428.  Do you know what platforms would lack truncate entirely?
POSIX doesn't require it to exist, but yeah, these days, most platforms
include BSD or GNU truncate by default.  I also note that 'requires od'
in test-eflags.sh is probably pointless, since POSIX does require od to
exist.

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