On Wednesday, 23 November 2016 16:40:59 CET Tomáš Golembiovský wrote:
> On Mon, 21 Nov 2016 16:41:49 +0100
> Pino Toscano <ptosc...@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ý <tgole...@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.

In the past I've added test-data/guestfs-hashsums.sh -- maybe that one
could be renamed to test-utils.sh, and then add the new qemu_is_version
there.

> > >  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.

Good idea.

> > > 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.

Just like we normalize the line in the output containing the vmdk path,
IMHO the reference test output should not contain any path to the test
itself. Easier to read and maintain IMHO.

Thanks,
-- 
Pino Toscano

Attachment: signature.asc
Description: This is a digitally signed message part.

_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Reply via email to