On Fri, Apr 09, 2021 at 05:14:41PM +0200, Peter Krempa wrote:
> Hi,
>
> recently I've got very annoyed that we still have a very large amount of
> tests in qemuxml2argvtest which use DO_TEST or some other fake-caps
> test.
>
> While I can see value of fake-caps test for negative cases (but I'd
> prefer actually real-caps with capability masking) I don't think there's
> much value in keeping the DO_TEST fake-caps stuff around.
>
> Namely in most cases it doesn't test anything useful which could be
> encountered in real world.
>
> I propose that we convert all DO_TEST cases to:
>
> - DO_TEST_CAPS_LATEST
> - and possibly a amount of version-bound tests based on:
> - oldest supported version?
> - versions in popular distros?
> - none?
> - any ideas?
>
> For the negative cases we can think of the caps masking or other
> approach but getting rid of DO_TEST would IMO be a great step forward in
> terms of usefullnes of our unit tests.
First off, the goal for the tests is to get the maximum coverage
at the lowest execution time, and lowest number of test data fies.
Much easier said, than done.
We have a massive amount of overlap in tests, as many of the XML
files are very broad. eg almost every test has a <disk>, even if
the test is unrelated to disks.
When considering the capabilities I think we only have a couple
of distinct situations:
1. Capabilities tracking the introduction of a feature.
2. Capabilities tracking changed syntax for configuring a feature
For the first scenario we need a test that is at least as new
as the version where it was introduced.
For the second scenario we need a test that is at least as new
as the version where it was introduced, and also a test that is
at least as old as the version prior to where the syntax changed.
DO_TEST_CAPS_LATEST covers scenario (1) and the first half
of scenario (2). For the second halve of scenario (2) we
also need a DO_TEST_CAPS_VER.
The downside with DO_TEST_CAPS_LATEST though is that any
time the latest caps change, we cause churn over any .args
files that are impacted. This is compounded by the overly
broad XML in many tests - eg watchdog.xml has a <disk>
so its .args file is impacted when disk handling changes,
even though it is a watchdog test.
To get full coverage of the different arg combinations, it
ought to be sufficient to test at the initial transition
point.
eg if we have a disk-cdrom.xml file, we should only need to
test at the oldest caps that we have to cover -drive and
the first caps that introduced -blockdev.
The question is what happens when -blockdev-ng arrives.
We want to have coverage of this new arg.
Using DO_TEST_CAPS_LATEST gets that coverage, but at a quite
large cost. Basically it is a big hammer approach that lets us
ignore what tests are actually needed to get minimal desired
coverage, and instead just test everything.
One thing that is clearly bad with plain DO_TEST() is that
due to the individual listed capability flags, we end up
generating an frankenstein argv that has no relation to a
real world QEMU version.
Also we have _ARCH variants which are arguably redundant.
We can just get the arch from /domain/os/type/@arch after
parsing the XML and use that to find the .args file.
Overall I'd say we should aim to test at the initial
transition points, where a feature is first introduced,
and exclusively use capabilities sets captured from real
QEMU binaries.
Ideally I think we should have a single "DO_TEST" which
just accepts a QEMU version number.
DO_TEST("2.12.0", "disk-cdrom");
DO_TEST("5.0.0", "disk-cdrom");
This gets us coverage of the two different ways disks are
configured, without churn from newer versions. If we did
get a -blockdev-ng, we can add DO_TEST("10.0.0", "disk-cdrom")
and for the few other relevant tests.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
