[libvirt] PATCH: Fix libvirtd test cases

[Daniel P. Berrange]

On Tue, Mar 03, 2009 at 09:20:03PM +0100, Jim Meyering wrote:
> Daniel P. Berrange wrote:
> > The libvirtd tests have a number of bugs causing them to fail & generally
> > do bad things. They all currently fail on RHEL5 hosts.
> 
> Odd that they'd all fail for you.
> Only one fails for me: libvirtd-pool
> and daemon-conf is mistakenly skipped due to use of ancient
> automake-1.9.
> 
> >  - daemon-conf - the abs_topbuild_dir env var was not being set correctly
> 
> As you know, that's because you're building with RHEL5's ancient
> version of automake.
> That is a no-no (for many reasons) but easy to work around.
> IMHO, no one should never build using such old autotools,
> but I do know how we're currently constrained.  no choice.

That is not a no-no. It is fundamentally a requirement for libvirt 
that is not going to change.

> >    so it failed to find config.h. It also broken by changes in stderr
> >    debug output from libvirtd. This patch fixes the env var, and changes
> >    it to it just looks for the desired error message, not doing a diff
> >    across entire of stdout/err.
> 
> Once I worked around automake-1.9's lack of abs_top_builddir,
> daemon-conf passed.
> 
> What error did you see?
> 
> I want to keep the diffs strict.  While sometimes more work,

This has been a total PITA for the entire time this test has existed
constantly breaking whenever something appears on stdout for reasons
totally unrelated to the test case. It is just not maintainable as
it is.

> >  - libvirtd-fail - again fails because it is diffing the whole of stdout/err
> >    and coming across warning messages its not expecting. Change it to look
> >    for daemon error exit status because that reliably indicates whether it
> >    quit as expected on bogus configs
> 
> Didn't fail for me.
> Provide more detail (e.g., actual warnings) and I'll be happy
> to find a solution we can both accept.



> 
> >  - libvirtd-pool - running the QEMU driver which does not exist, just to
> >    test virsh's XML generation capabilities. This adds a --print-xml arg
> >    to virsh and uses the test:///default driver for testing, so we avoid
> >    the QEMU driver & daemon during tests
> 
> --- pool-list-exp       2009-03-03 14:54:35.000000000 -0500
> +++ out 2009-03-03 14:54:35.000000000 -0500
> @@ -12,8 +12,8 @@
>      <path>/target-path</path>
>      <permissions>
>        <mode>0700</mode>
> -      <owner>500</owner>
> -      <group>500</group>
> +      <owner>508</owner>
> +      <group>508</group>
>      </permissions>
>    </target>
>  </pool>
> FAIL: libvirtd-pool
> 
> It's easy to avoid that difference.
> With the following, it passes for me:

This is still missing the point - the test is overengineered &
unmaintainable. There is absolutely no need to run the daemon
or run the QEMU driver in order to test the XML generation.
This test should never have been committed since you didn't
even wait for review or comments before committing. 

> While I like the idea of your new print-xml option,
> I don't want to decreasing coverage.

It is not decreasing the test coverage - it is focusing the test
case on the thing that is actually being tested - the XML generation
of the virsh command to produce a more reliable & maintainable
test case which doesn't randomly fail & need disabling based on
configure options.

> >  - libvirt-net-persist - again trying to rnu the QEMU driver which does
> >    not exist, and its writing config files into the user's home directory.
> >    There's no easy fix for this, so I'm killing it off. It can be tested
> >    in the separate integration test suite where you can be sure to arrange
> >    for correct pre-requisites and safe working environment
> 
> NACK to removing this test.
> 
> When built without qemu, this test should simply be skipped.
> Removing test coverage (no matter how much you dislike the
> current form) should be done only as a last resort.
> Just do what daemon-conf does:

That is not sufficient. Even if the QEMU driver is compiled into
libvirt you cannot expect that you will be able run the QEMU
driver. We fundamentally *DO NOT* run the real hypervisor drivers
in the test suite.  This is what the "test:///default' driver was
written to do.


Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|