[libvirt] [PATCH 1/2] tests: Enable failure testing with CompareDomXML2XML
This allows tests to check for specific failure scenarios --- tests/bhyvexml2xmltest.c | 4 ++-- tests/genericxml2xmltest.c | 4 ++-- tests/lxcxml2xmltest.c | 3 ++- tests/qemuxml2xmltest.c | 6 ++++-- tests/testutils.c | 47 ++++++++++++++++++++++++++++++++++------------ tests/testutils.h | 12 +++++++++++- 6 files changed, 56 insertions(+), 20 deletions(-) diff --git a/tests/bhyvexml2xmltest.c b/tests/bhyvexml2xmltest.c index 8f556ee..c8c6c6e 100644 --- a/tests/bhyvexml2xmltest.c +++ b/tests/bhyvexml2xmltest.c @@ -32,8 +32,8 @@ testCompareXMLToXMLHelper(const void *data) ret = testCompareDomXML2XMLFiles(driver.caps, driver.xmlopt, xml_in, info->different ? xml_out : xml_in, - false, - NULL, NULL, 0); + false, NULL, NULL, 0, + TEST_COMPARE_DOM_XML2XML_RESULT_SUCCESS); cleanup: VIR_FREE(xml_in); diff --git a/tests/genericxml2xmltest.c b/tests/genericxml2xmltest.c index bf9b11d..7d504db 100644 --- a/tests/genericxml2xmltest.c +++ b/tests/genericxml2xmltest.c @@ -39,8 +39,8 @@ testCompareXMLToXMLHelper(const void *data) ret = testCompareDomXML2XMLFiles(caps, xmlopt, xml_in, info->different ? xml_out : xml_in, - !info->inactive_only, - NULL, NULL, 0); + !info->inactive_only, NULL, NULL, 0, + TEST_COMPARE_DOM_XML2XML_RESULT_SUCCESS); cleanup: VIR_FREE(xml_in); VIR_FREE(xml_out); diff --git a/tests/lxcxml2xmltest.c b/tests/lxcxml2xmltest.c index 0b51340..fec0142 100644 --- a/tests/lxcxml2xmltest.c +++ b/tests/lxcxml2xmltest.c @@ -46,7 +46,8 @@ testCompareXMLToXMLHelper(const void *data) ret = testCompareDomXML2XMLFiles(caps, xmlopt, xml_in, info->different ? xml_out : xml_in, !info->inactive_only, - NULL, NULL, info->parse_flags); + NULL, NULL, info->parse_flags, + TEST_COMPARE_DOM_XML2XML_RESULT_SUCCESS); cleanup: VIR_FREE(xml_in); VIR_FREE(xml_out); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 0735677..b0f298c 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -54,7 +54,8 @@ testXML2XMLActive(const void *opaque) return testCompareDomXML2XMLFiles(driver.caps, driver.xmlopt, info->inName, info->outActiveName, true, - qemuXML2XMLPreFormatCallback, opaque, 0); + qemuXML2XMLPreFormatCallback, opaque, 0, + TEST_COMPARE_DOM_XML2XML_RESULT_SUCCESS); } @@ -65,7 +66,8 @@ testXML2XMLInactive(const void *opaque) return testCompareDomXML2XMLFiles(driver.caps, driver.xmlopt, info->inName, info->outInactiveName, false, - qemuXML2XMLPreFormatCallback, opaque, 0); + qemuXML2XMLPreFormatCallback, opaque, 0, + TEST_COMPARE_DOM_XML2XML_RESULT_SUCCESS); } diff --git a/tests/testutils.c b/tests/testutils.c index fc4c339..a0ce4b6 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -1079,10 +1079,12 @@ int testCompareDomXML2XMLFiles(virCapsPtr caps, virDomainXMLOptionPtr xmlopt, const char *infile, const char *outfile, bool live, testCompareDomXML2XMLPreFormatCallback cb, - const void *opaque, unsigned int parseFlags) + const void *opaque, unsigned int parseFlags, + testCompareDomXML2XMLResult expectResult) { char *actual = NULL; int ret = -1; + testCompareDomXML2XMLResult result; virDomainDefPtr def = NULL; unsigned int parse_flags = live ? 0 : VIR_DOMAIN_DEF_PARSE_INACTIVE; unsigned int format_flags = VIR_DOMAIN_DEF_FORMAT_SECURE; @@ -1092,25 +1094,46 @@ testCompareDomXML2XMLFiles(virCapsPtr caps, virDomainXMLOptionPtr xmlopt, if (!live) format_flags |= VIR_DOMAIN_DEF_FORMAT_INACTIVE; - if (!(def = virDomainDefParseFile(infile, caps, xmlopt, parse_flags))) - goto fail; + if (!(def = virDomainDefParseFile(infile, caps, xmlopt, parse_flags))) { + result = TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE; + goto out; + } if (!virDomainDefCheckABIStability(def, def)) { VIR_TEST_DEBUG("ABI stability check failed on %s", infile); - goto fail; + result = TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_STABILITY; + goto out; } - if (cb && cb(def, opaque) < 0) - goto fail; + if (cb && cb(def, opaque) < 0) { + result = TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_CB; + goto out; + } - if (!(actual = virDomainDefFormat(def, caps, format_flags))) - goto fail; + if (!(actual = virDomainDefFormat(def, caps, format_flags))) { + result = TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_FORMAT; + goto out; + } - if (virtTestCompareToFile(actual, outfile) < 0) - goto fail; + if (virtTestCompareToFile(actual, outfile) < 0) { + result = TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_COMPARE; + goto out; + } + + result = TEST_COMPARE_DOM_XML2XML_RESULT_SUCCESS; + out: + if (result == expectResult) { + ret = 0; + if (expectResult != TEST_COMPARE_DOM_XML2XML_RESULT_SUCCESS) { + VIR_TEST_DEBUG("Got expected failure code=%d msg=%s", + result, virGetLastErrorMessage()); + } + } else { + ret = -1; + VIR_TEST_DEBUG("Expected failure code=%d but received code=%d", + expectResult, result); + } - ret = 0; - fail: VIR_FREE(actual); virDomainDefFree(def); return ret; diff --git a/tests/testutils.h b/tests/testutils.h index 058be55..0417a0b 100644 --- a/tests/testutils.h +++ b/tests/testutils.h @@ -134,6 +134,15 @@ int virtTestMain(int argc, virCapsPtr virTestGenericCapsInit(void); virDomainXMLOptionPtr virTestGenericDomainXMLConfInit(void); +typedef enum { + TEST_COMPARE_DOM_XML2XML_RESULT_SUCCESS, + TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE, + TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_STABILITY, + TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_CB, + TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_FORMAT, + TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_COMPARE, +} testCompareDomXML2XMLResult; + typedef int (*testCompareDomXML2XMLPreFormatCallback)(virDomainDefPtr def, const void *opaque); int testCompareDomXML2XMLFiles(virCapsPtr caps, @@ -143,6 +152,7 @@ int testCompareDomXML2XMLFiles(virCapsPtr caps, bool live, testCompareDomXML2XMLPreFormatCallback cb, const void *opaque, - unsigned int parseFlags); + unsigned int parseFlags, + testCompareDomXML2XMLResult expectResult); #endif /* __VIR_TEST_UTILS_H__ */ -- 2.5.5
* Add a test for listen=XXX and <listen address=YYY/> collision error * Add an explicit test for listen=XXX duplicated to <listen address=XXX/> We implicitly test it elsewhere but I figure it's better to be explicit, and this test case can be extended in the future for additional listen back compat if/when we support <listen type='socket'/> syntax --- The latter test case I added to try and exercise the potential crash that Jan pointed out here: https://www.redhat.com/archives/libvir-list/2016-April/msg00241.html That patch was fixed before committing but I was still curious... the test case doesn't tickle the crash? STREQ_NULLABLE seems to avoid it somehow for address=NULL, yet if I try to printf address->address it understandably crashes. Oh well :) ...eneric-graphics-listen-back-compat-mismatch.xml | 30 ++++++++++++++++++++++ .../generic-graphics-listen-back-compat.xml | 28 ++++++++++++++++++++ .../generic-graphics-listen-back-compat.xml | 30 ++++++++++++++++++++++ tests/genericxml2xmltest.c | 16 ++++++++---- 4 files changed, 99 insertions(+), 5 deletions(-) create mode 100644 tests/genericxml2xmlindata/generic-graphics-listen-back-compat-mismatch.xml create mode 100644 tests/genericxml2xmlindata/generic-graphics-listen-back-compat.xml create mode 100644 tests/genericxml2xmloutdata/generic-graphics-listen-back-compat.xml diff --git a/tests/genericxml2xmlindata/generic-graphics-listen-back-compat-mismatch.xml b/tests/genericxml2xmlindata/generic-graphics-listen-back-compat-mismatch.xml new file mode 100644 index 0000000..eee3fed --- /dev/null +++ b/tests/genericxml2xmlindata/generic-graphics-listen-back-compat-mismatch.xml @@ -0,0 +1,30 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='vnc' listen='192.168.123.123'> + <listen type='address' address='1.2.3.4'/> + </graphics> + <video> + <model type='cirrus' vram='16384' heads='1'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/genericxml2xmlindata/generic-graphics-listen-back-compat.xml b/tests/genericxml2xmlindata/generic-graphics-listen-back-compat.xml new file mode 100644 index 0000000..c60fb6e --- /dev/null +++ b/tests/genericxml2xmlindata/generic-graphics-listen-back-compat.xml @@ -0,0 +1,28 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='vnc' listen='192.168.123.123'/> + <video> + <model type='cirrus' vram='16384' heads='1'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/genericxml2xmloutdata/generic-graphics-listen-back-compat.xml b/tests/genericxml2xmloutdata/generic-graphics-listen-back-compat.xml new file mode 100644 index 0000000..aa00d34 --- /dev/null +++ b/tests/genericxml2xmloutdata/generic-graphics-listen-back-compat.xml @@ -0,0 +1,30 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='vnc' port='-1' autoport='yes' listen='192.168.123.123'> + <listen type='address' address='192.168.123.123'/> + </graphics> + <video> + <model type='cirrus' vram='16384' heads='1' primary='yes'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/genericxml2xmltest.c b/tests/genericxml2xmltest.c index 7d504db..05563fb 100644 --- a/tests/genericxml2xmltest.c +++ b/tests/genericxml2xmltest.c @@ -21,6 +21,7 @@ struct testInfo { const char *name; int different; bool inactive_only; + testCompareDomXML2XMLResult expectResult; }; static int @@ -40,7 +41,7 @@ testCompareXMLToXMLHelper(const void *data) ret = testCompareDomXML2XMLFiles(caps, xmlopt, xml_in, info->different ? xml_out : xml_in, !info->inactive_only, NULL, NULL, 0, - TEST_COMPARE_DOM_XML2XML_RESULT_SUCCESS); + info->expectResult); cleanup: VIR_FREE(xml_in); VIR_FREE(xml_out); @@ -59,22 +60,27 @@ mymain(void) if (!(xmlopt = virTestGenericDomainXMLConfInit())) return EXIT_FAILURE; -#define DO_TEST_FULL(name, is_different, inactive) \ +#define DO_TEST_FULL(name, is_different, inactive, expectResult) \ do { \ - const struct testInfo info = {name, is_different, inactive}; \ + const struct testInfo info = {name, is_different, inactive, \ + expectResult}; \ if (virtTestRun("GENERIC XML-2-XML " name, \ testCompareXMLToXMLHelper, &info) < 0) \ ret = -1; \ } while (0) #define DO_TEST(name) \ - DO_TEST_FULL(name, 0, false) + DO_TEST_FULL(name, 0, false, TEST_COMPARE_DOM_XML2XML_RESULT_SUCCESS) #define DO_TEST_DIFFERENT(name) \ - DO_TEST_FULL(name, 1, false) + DO_TEST_FULL(name, 1, false, TEST_COMPARE_DOM_XML2XML_RESULT_SUCCESS) DO_TEST_DIFFERENT("disk-virtio"); + DO_TEST_DIFFERENT("graphics-listen-back-compat"); + DO_TEST_FULL("graphics-listen-back-compat-mismatch", 0, false, + TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); + virObjectUnref(caps); virObjectUnref(xmlopt); -- 2.5.5
On Fri, 2016-04-08 at 13:49 -0400, Cole Robinson wrote:
* Add a test for listen=XXX and <listen address=YYY/> collision error * Add an explicit test for listen=XXX duplicated to <listen address=XXX/> We implicitly test it elsewhere but I figure it's better to be explicit, and this test case can be extended in the future for additional listen back compat if/when we support <listen type='socket'/> syntax --- The latter test case I added to try and exercise the potential crash that Jan pointed out here:
https://www.redhat.com/archives/libvir-list/2016-April/msg00241.html
That patch was fixed before committing but I was still curious... the test case doesn't tickle the crash? STREQ_NULLABLE seems to avoid it somehow for address=NULL, yet if I try to printf address->address it understandably crashes. Oh well :)
...eneric-graphics-listen-back-compat-mismatch.xml | 30 ++++++++++++++++++++++ .../generic-graphics-listen-back-compat.xml | 28 ++++++++++++++++++++ .../generic-graphics-listen-back-compat.xml | 30 ++++++++++++++++++++++ tests/genericxml2xmltest.c | 16 ++++++++---- 4 files changed, 99 insertions(+), 5 deletions(-) create mode 100644 tests/genericxml2xmlindata/generic-graphics-listen-back-compat-mismatch.xml create mode 100644 tests/genericxml2xmlindata/generic-graphics-listen-back-compat.xml create mode 100644 tests/genericxml2xmloutdata/generic-graphics-listen-back-compat.xml
ACK -- Andrea Bolognani Software Engineer - Virtualization Team
On Fri, 2016-04-08 at 13:48 -0400, Cole Robinson wrote:
This allows tests to check for specific failure scenarios --- tests/bhyvexml2xmltest.c | 4 ++-- tests/genericxml2xmltest.c | 4 ++-- tests/lxcxml2xmltest.c | 3 ++- tests/qemuxml2xmltest.c | 6 ++++-- tests/testutils.c | 47 ++++++++++++++++++++++++++++++++++------------ tests/testutils.h | 12 +++++++++++- 6 files changed, 56 insertions(+), 20 deletions(-)
[...]
- if (virtTestCompareToFile(actual, outfile) < 0) - goto fail; + if (virtTestCompareToFile(actual, outfile) < 0) { + result = TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_COMPARE; + goto out; + } + + result = TEST_COMPARE_DOM_XML2XML_RESULT_SUCCESS;
Would look nicer with an empty line before the label.
+ out: + if (result == expectResult) { + ret = 0; + if (expectResult != TEST_COMPARE_DOM_XML2XML_RESULT_SUCCESS) { + VIR_TEST_DEBUG("Got expected failure code=%d msg=%s", + result, virGetLastErrorMessage()); + } + } else { + ret = -1; + VIR_TEST_DEBUG("Expected failure code=%d but received code=%d", + expectResult, result);
s/failure/result/ in the second message above... If expectResult == TEST_COMPARE_DOM_XML2XML_RESULT_SUCCESS we don't want to call "success" a "failure code" :) ACK with the above taken care of. Cheers. -- Andrea Bolognani Software Engineer - Virtualization Team
On 04/14/2016 10:00 AM, Andrea Bolognani wrote:
On Fri, 2016-04-08 at 13:48 -0400, Cole Robinson wrote:
This allows tests to check for specific failure scenarios --- tests/bhyvexml2xmltest.c | 4 ++-- tests/genericxml2xmltest.c | 4 ++-- tests/lxcxml2xmltest.c | 3 ++- tests/qemuxml2xmltest.c | 6 ++++-- tests/testutils.c | 47 ++++++++++++++++++++++++++++++++++------------ tests/testutils.h | 12 +++++++++++- 6 files changed, 56 insertions(+), 20 deletions(-)
[...]
- if (virtTestCompareToFile(actual, outfile) < 0) - goto fail; + if (virtTestCompareToFile(actual, outfile) < 0) { + result = TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_COMPARE; + goto out; + } + + result = TEST_COMPARE_DOM_XML2XML_RESULT_SUCCESS;
Would look nicer with an empty line before the label.
+ out: + if (result == expectResult) { + ret = 0; + if (expectResult != TEST_COMPARE_DOM_XML2XML_RESULT_SUCCESS) { + VIR_TEST_DEBUG("Got expected failure code=%d msg=%s", + result, virGetLastErrorMessage()); + } + } else { + ret = -1; + VIR_TEST_DEBUG("Expected failure code=%d but received code=%d", + expectResult, result);
s/failure/result/ in the second message above... If expectResult == TEST_COMPARE_DOM_XML2XML_RESULT_SUCCESS we don't want to call "success" a "failure code" :)
ACK with the above taken care of.
Thanks, made those changes and pushed - Cole
participants (2)
-
Andrea Bolognani -
Cole Robinson