[libvirt] PATCH: Fix libvirtd test cases

[Jim Meyering]

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.

Your change to tests/Makefile.am is fine.

>    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,
I've found that it's worthwhile because it helps detect unrelated bugs.

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

diff --git a/tests/libvirtd-pool b/tests/libvirtd-pool
index 370f3b1..bf838b0 100755
--- a/tests/libvirtd-pool
+++ b/tests/libvirtd-pool
@@ -31,6 +31,9 @@ virsh --connect "$url" pool-dumpxml P >> out 2>&1 || fail=1

 # remove random uuid
 sed 's/<uuid>.*/-/' out > k && mv k out || fail=1
+# filter out actual owner and group numbers
+sed 's/<owner>.*/-/' out > k && mv k out || fail=1
+sed 's/<group>.*/-/' out > k && mv k out || fail=1

 kill $pid

@@ -49,8 +52,8 @@ Pool P defined
     <path>/target-path</path>
     <permissions>
       <mode>0700</mode>
-      <owner>500</owner>
-      <group>500</group>
+      -
+      -
     </permissions>
   </target>
 </pool>

While I like the idea of your new print-xml option,
I don't want to decreasing coverage.
If lack of QEMU is the problem, the solution is simply to skip the test.

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

grep '^#define WITH_QEMU 1' $CONFIG_HEADER > /dev/null ||
  skip_test_ "configured without QEMU support"

>  src/virsh.c                |   51 ++++++++++++++++++++++++---------------
>  tests/Makefile.am          |    3 --
>  tests/daemon-conf          |   13 +++-------
>  tests/libvirtd-fail        |    9 ++----
>  tests/libvirtd-net-persist |   58 ---------------------------------------------
>  tests/libvirtd-pool        |   41 ++++++-------------------------
>  6 files changed, 48 insertions(+), 127 deletions(-)