Re: [libvirt] PATCH: Fix libvirtd test cases
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(-)