[Libguestfs] [PATCH v2 5/5] v2v: update tests to match changes in OVA import

[Tomáš Golembiovský]

On Mon, 21 Nov 2016 16:41:49 +0100
Pino Toscano <ptoscano at redhat.com> wrote:

> On Saturday, 12 November 2016 16:37:53 CET Tomáš Golembiovský wrote:
> > The virt-v2v behaviour for OVA input now depends on QEMU version
> > available. The tests affected by this now have two *.expect files and
> > the expected result now also depends on the QEMU used.
> > 
> > Signed-off-by: Tomáš Golembiovský <tgolembi at redhat.com>
> > ---  
> 
> This IMHO should be part of patch #4 as well, otherwise applying only
> patch #4 without this could lead to test failures (e.g. when bisecting).
> 
> >  test-data/utils.sh                      |  21 +++++  
> 
> qemu-utils.sh?
> Also, it must be added to EXTRA_DIST in test-data/Makefile.am.
 
I wanted to make the name generic enough in case we want to add some
other functions in the future. If you prefer qemu-utils.sh I will rename
it. 


> >  v2v/Makefile.am                         |   1 +
> >  v2v/test-v2v-i-ova-formats.sh           |   5 +-
> >  v2v/test-v2v-i-ova-subfolders.expected2 |  18 +++++
> >  v2v/test-v2v-i-ova-subfolders.sh        |  12 ++-
> >  v2v/test-v2v-i-ova-tar.expected         |  18 +++++
> >  v2v/test-v2v-i-ova-tar.expected2        |  18 +++++
> >  v2v/test-v2v-i-ova-tar.ovf              | 138 ++++++++++++++++++++++++++++++++  
> 
> The ovf and expected files are basically copies of the -formats
> versions -- would it be possible to reuse them?

Wouldn't that be confusing?

Maybe I could rename it to test-v2v-i-ova.ovf and
test-v2v-i-ova.expected and then use it in both tests.


> > diff --git a/test-data/utils.sh b/test-data/utils.sh
> > new file mode 100755
> > index 0000000..49e173f
> > --- /dev/null
> > +++ b/test-data/utils.sh
> > @@ -0,0 +1,21 @@
> > +#!/bin/bash -
> > +
> > +# Returns 0 if QEMU version is greater or equal to the arguments
> > +qemu_version() {  
> 
> This name would suggest it prints the version of qemu, while it really
> checks against a wanted version -- I'd suggest qemu_is_version, just
> like the simple tool we have for shell tests involving libvirt,
> libvirt-is-version (see src/libvirt-is-version.c).

Good point.


> > diff --git a/v2v/test-v2v-i-ova-subfolders.sh b/v2v/test-v2v-i-ova-subfolders.sh
> > index c49f7cb..89a9a3e 100755
> > --- a/v2v/test-v2v-i-ova-subfolders.sh
> > +++ b/v2v/test-v2v-i-ova-subfolders.sh
> > @@ -35,6 +35,7 @@ fi
> >  export VIRT_TOOLS_DATA_DIR="$srcdir/../test-data/fake-virt-tools"
> >  
> >  . $srcdir/../test-data/guestfs-hashsums.sh
> > +. $srcdir/../test-data/utils.sh
> >  
> >  d=test-v2v-i-ova-subfolders.d
> >  rm -rf $d
> > @@ -56,10 +57,15 @@ popd
> >  # normalize the output.
> >  $VG virt-v2v --debug-gc --quiet \
> >      -i ova $d/test.ova \
> > -    --print-source |
> > -sed 's,[^ \t]*\(subfolder/disk.*\.vmdk\),\1,' > $d/source
> > +    --print-source > $d/source
> >  
> >  # Check the parsed source is what we expect.
> > -diff -u test-v2v-i-ova-subfolders.expected $d/source
> > +if qemu_version 2 8 ; then  
> 
> Here I'd normalize the output, by removing "$d/" (i.e. the path of the
> subdirectory plus the trailing slash):
> 
>   sed -i -e "s,$d/,." $d/source
> 

Any specific reason for that? Here $d is specific to the test. It does
not change between single runs of the test.


-- 
Tomáš Golembiovský <tgolembi at redhat.com>