On Wed, Apr 11, 2018 at 11:16:27AM -0400, John Ferlan wrote:



On 04/04/2018 04:13 AM, Peter Krempa wrote:
Allow testing of XML->argv conversion with using a real capability map
as used in the qemucapabilitiestest. This allows specifying the required
qemu version with the test rather than having to enumerate all the
required capabilities.

Signed-off-by: Peter Krempa <pkre...@redhat.com>
---
 tests/qemuxml2argvtest.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index d79913dd0a..c540ce2f50 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -654,6 +654,37 @@ mymain(void)
     if (VIR_STRDUP_QUIET(driver.config->memoryBackingDir, 
"/var/lib/libvirt/qemu/ram") < 0)
         return EXIT_FAILURE;

+
+# define DO_TEST_CAPS_ARCH(name, migrateFrom, flags, \
+                           parseFlags, gic, arch, ver) \
+    do { \
+        static struct testInfo info = { \
+            name, NULL, migrateFrom, migrateFrom ? 7 : -1, (flags), 
parseFlags, \
+            false, NULL \
+        }; \
+        info.skipLegacyCPUs = skipLegacyCPUs; \
+        if (testInitQEMUCaps(&info, gic) < 0) \
+            return EXIT_FAILURE; \
+        if (!(info.qemuCaps = 
qemuTestParseCapabilitiesArch(virArchFromString(arch), \
+                                          abs_srcdir 
"/qemucapabilitiesdata/caps_" \
+                                          ver "." arch ".xml"))) \
+            return EXIT_FAILURE; \
+        if (virTestRun("QEMU XML-2-ARGV " name, \
+                       testCompareXMLToArgv, &info) < 0) \
+            ret = -1; \
+        if (info.vm && virTestRun("QEMU XML-2-startup-XML " name, \
+                                  testCompareXMLToStartupXML, &info) < 0) \
+            ret = -1; \
+        virObjectUnref(info.qemuCaps); \
+        virObjectUnref(info.vm); \
+    } while (0)
+
+# define DO_TEST_CAPS_FULL(name, flags, parseFlags, ver) \
+    DO_TEST_CAPS_ARCH(name, NULL, flags, parseFlags, GIC_NONE, "x86_64", ver)

Assumes x64_64...

+
+# define DO_TEST_CAPS(name, ver) \
+    DO_TEST_CAPS_FULL(name, 0, 0, ver)
+
 # define DO_TEST_FULL(name, migrateFrom, migrateFd, flags, \
                       parseFlags, gic, ...) \
     do { \


Do you expect to see wide spread and future use of this model as opposed
to the existing DO_TEST model? Shouldn't at least some of the existing
tests be converted, then?

Yes to both.


Perhaps the macro(s) should be DO_TEST_VERS[ION] or DO_TEST_<ARCH>_VERS
instead since DO_TEST runs the test with specific capabilities and this
new model runs the test with a specific version for a specific platform.

How do or should we enforce that when adding a new test from this point
forward that the DO_TEST_CAPS model is chosen over DO_TEST?  I suppose
there is still value in DO_TEST since one can pick and choose which CAP
is needed - although I'll freely admit that's usually a cut-n-paste from
some other test and change the name type activity.

The problem with that approach is that we're testing against
capability combinations that might never occur in the wild.

I actually remember fixing some bug that was supposed to be covered by
a unit test, but it was missing the important capability.

Enforcement by review ought to be enough for everybody.

Though there still might be features that you can compile out even from
QEMU versions that otherwise have the capability, but we don't add
negative tests for that often.

Jano


I think I have too many questions for an R-b at this point.


John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to