[libvirt] [PATCH 4/4] qemu: add unit tests for the capabilities xml

[Francesco Romani]

----- Original Message -----
> From: "Eric Blake" <eblake at redhat.com>
> To: "Francesco Romani" <fromani at redhat.com>, libvir-list at redhat.com
> Sent: Friday, February 28, 2014 12:49:25 AM
> Subject: Re: [libvirt] [PATCH 4/4] qemu: add unit tests for the capabilities xml
> 
> On 01/17/2014 08:31 AM, Francesco Romani wrote:
> > the test is loosely inspired from qemucapabilitiestest
> > and qemuxml2xmltest.
> > 
> > Added a new test instead of extending an existing one because
> > the feature being tested don't really fits nicely in any
> 
> s/don't/doesn't/ s/fits/fit/

Will fix.
 
> > existing place.
> > ---
> >  .gitignore                                         |   1 +
> >  tests/Makefile.am                                  |  10 +-
> >  tests/qemucaps2xmldata/all_1.6.0-1.caps            | 142 ++++++++++++++
> >  tests/qemucaps2xmldata/all_1.6.0-1.xml             |  51 +++++
> >  tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.caps | 141 +++++++++++++
> >  tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml  |  51 +++++
> >  tests/qemucaps2xmltest.c                           | 217
> >  +++++++++++++++++++++
> >  7 files changed, 612 insertions(+), 1 deletion(-)
> >  create mode 100644 tests/qemucaps2xmldata/all_1.6.0-1.caps
> >  create mode 100644 tests/qemucaps2xmldata/all_1.6.0-1.xml
> >  create mode 100644 tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.caps
> >  create mode 100644 tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml
> >  create mode 100644 tests/qemucaps2xmltest.c
> 
> Fails 'make check' for me:
> 
> $ make -C tests check TESTS=qemucaps2xmltest \
>   VERBOSE=yes VIR_TEST_DEBUG=1
> ...
> 
> TEST: qemucaps2xmltest
>  1) all_1.6.0-1                                                       ...
> Offset 393
> Expect [6</machine>
>         <machine>none</machine>
>         <machine canonical='pc-i440fx-1.6'>pc</machine>
>         <machine>pc-i440fx-1.5</machine>
>         <machine>pc-i440fx-1.4</machine>
> ...
> Actual [7</machine>
>         <machine>none</machine>
>         <machine canonical='pc-i440fx-1.7'>pc</machine>
>         <machine>pc-i440fx-1.6</machine>
>         <machine>pc-i440fx-1.5</machine>
> 
> which means you wrote the test by probing the installed qemu, instead of
> our testsuite helper binary, and my installed qemu 1.7 differs from your
> setup of 1.6.  That has to be fixed before this can be applied.

Yes, I have qemu 1.6.1 in my test box, this is probably the reason why
I haven't noticed this misbehaviour before.
This is a point I wanted to discuss because I'm not sure how to properly
achieve this. Please see below for my questions on this topic.

> 
> > +++ b/tests/qemucaps2xmltest.c
> > @@ -0,0 +1,217 @@
> > +/*
> 
> > +
> > +    virQEMUCapsCacheAdd(cache, "kvm", qemuCaps);
> > +    virQEMUCapsCacheAdd(cache, "qemu", qemuCaps);
> > +    virQEMUCapsCacheAdd(cache, "qemu-system-x86_64", qemuCaps);
> > +    virQEMUCapsCacheAdd(cache, "qemu-system-i386", qemuCaps);
> > +    virQEMUCapsCacheAdd(cache, "/usr/bin/qemu-system-x86_64", qemuCaps);
> > +    virQEMUCapsCacheAdd(cache, "/usr/bin/qemu-system-i386", qemuCaps);
> 
> Is this where the testsuite ended up probing the installed file system
> instead of using only data from the testsuite?

Yes, it is.
What I want to achieve here is to pre-fill the capabilities cache,
by using the function I exported in a previous patch on this series,
with the test data.

The intent is of course to use only the data from the testsuite and to
be independent from the installed QEMU, if any, as you pointed out above.

I tried a few different attempts but looks like I'm still missing the point.

How can I improve this test to achieve this objective?
There is an unit-test I can learn from?

Any advice is welcome, I want to fix this.

Thanks for the review and best regards,

-- 
Francesco Romani
RedHat Engineering Virtualization R & D
Phone: 8261328
IRC: fromani