[PATCH 0/2] genericxml2xmltest: Refactor test calls

Kristina Hanicova (2): genericxml2xmltest: Substitute 'inactive' variable with 'active' genericxml2xmltest: Refactor DO_TEST_FULL() calls tests/genericxml2xmltest.c | 71 ++++++++++++++++---------------------- 1 file changed, 29 insertions(+), 42 deletions(-) -- 2.31.1

Having negation in a name of a bool variable seems a bit confusing to me. I think the substitution makes the code much more readable. Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- tests/genericxml2xmltest.c | 46 +++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/tests/genericxml2xmltest.c b/tests/genericxml2xmltest.c index ac89422a32..3d5534ab54 100644 --- a/tests/genericxml2xmltest.c +++ b/tests/genericxml2xmltest.c @@ -138,9 +138,9 @@ mymain(void) if (!(xmlopt = virTestGenericDomainXMLConfInit())) return EXIT_FAILURE; -#define DO_TEST_FULL(name, is_different, inactive, expectResult) \ +#define DO_TEST_FULL(name, is_different, active, expectResult) \ do { \ - const struct testInfo info = {name, is_different, inactive, \ + const struct testInfo info = {name, is_different, active, \ expectResult}; \ if (virTestRun("GENERIC XML-2-XML " name, \ testCompareXMLToXMLHelper, &info) < 0) \ @@ -148,10 +148,10 @@ mymain(void) } while (0) #define DO_TEST(name) \ - DO_TEST_FULL(name, 0, false, TEST_COMPARE_DOM_XML2XML_RESULT_SUCCESS) + DO_TEST_FULL(name, 0, true, TEST_COMPARE_DOM_XML2XML_RESULT_SUCCESS) #define DO_TEST_DIFFERENT(name) \ - DO_TEST_FULL(name, 1, false, TEST_COMPARE_DOM_XML2XML_RESULT_SUCCESS) + DO_TEST_FULL(name, 1, true, TEST_COMPARE_DOM_XML2XML_RESULT_SUCCESS) DO_TEST_DIFFERENT("disk-virtio"); DO_TEST_DIFFERENT("disk-hyperv-physical"); @@ -162,18 +162,18 @@ mymain(void) DO_TEST_DIFFERENT("graphics-vnc-socket"); DO_TEST_DIFFERENT("graphics-vnc-socket-listen"); DO_TEST_DIFFERENT("graphics-listen-back-compat"); - DO_TEST_FULL("graphics-listen-back-compat-mismatch", 0, false, + DO_TEST_FULL("graphics-listen-back-compat-mismatch", 0, true, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); DO_TEST_DIFFERENT("graphics-vnc-listen-attr-only"); DO_TEST_DIFFERENT("graphics-vnc-listen-element-minimal"); DO_TEST_DIFFERENT("graphics-vnc-listen-element-with-address"); DO_TEST_DIFFERENT("graphics-vnc-socket-attr-listen-address"); DO_TEST_DIFFERENT("graphics-vnc-socket-attr-listen-socket"); - DO_TEST_FULL("graphics-vnc-socket-attr-listen-socket-mismatch", 0, false, + DO_TEST_FULL("graphics-vnc-socket-attr-listen-socket-mismatch", 0, true, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); DO_TEST("graphics-vnc-autoport-no"); - DO_TEST_FULL("name-slash-fail", 0, false, + DO_TEST_FULL("name-slash-fail", 0, true, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); DO_TEST("perf"); @@ -188,46 +188,46 @@ mymain(void) DO_TEST("network-interface-mac-check"); DO_TEST_DIFFERENT("chardev-tcp"); - DO_TEST_FULL("chardev-tcp-missing-host", 0, false, + DO_TEST_FULL("chardev-tcp-missing-host", 0, true, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); - DO_TEST_FULL("chardev-tcp-missing-service", 0, false, + DO_TEST_FULL("chardev-tcp-missing-service", 0, true, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); - DO_TEST_FULL("chardev-tcp-multiple-source", 0, false, + DO_TEST_FULL("chardev-tcp-multiple-source", 0, true, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); DO_TEST_DIFFERENT("chardev-udp"); - DO_TEST_FULL("chardev-udp-missing-connect-service", 0, false, + DO_TEST_FULL("chardev-udp-missing-connect-service", 0, true, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); - DO_TEST_FULL("chardev-udp-multiple-source", 0, false, + DO_TEST_FULL("chardev-udp-multiple-source", 0, true, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); DO_TEST_DIFFERENT("chardev-unix"); - DO_TEST_FULL("chardev-unix-smartcard-missing-path", 0, false, + DO_TEST_FULL("chardev-unix-smartcard-missing-path", 0, true, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); - DO_TEST_FULL("chardev-unix-redirdev-missing-path", 0, false, + DO_TEST_FULL("chardev-unix-redirdev-missing-path", 0, true, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); - DO_TEST_FULL("chardev-unix-rng-missing-path", 0, false, + DO_TEST_FULL("chardev-unix-rng-missing-path", 0, true, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); DO_TEST_DIFFERENT("chardev-reconnect"); - DO_TEST_FULL("chardev-reconnect-missing-timeout", 0, false, + DO_TEST_FULL("chardev-reconnect-missing-timeout", 0, true, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); - DO_TEST_FULL("chardev-reconnect-invalid-mode", 0, false, + DO_TEST_FULL("chardev-reconnect-invalid-mode", 0, true, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); DO_TEST("cachetune-small"); DO_TEST("cachetune-cdp"); DO_TEST_DIFFERENT("cachetune"); DO_TEST_DIFFERENT("cachetune-extra-tunes"); - DO_TEST_FULL("cachetune-colliding-allocs", false, true, + DO_TEST_FULL("cachetune-colliding-allocs", false, false, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); - DO_TEST_FULL("cachetune-colliding-tunes", false, true, + DO_TEST_FULL("cachetune-colliding-tunes", false, false, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); - DO_TEST_FULL("cachetune-colliding-types", false, true, + DO_TEST_FULL("cachetune-colliding-types", false, false, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); - DO_TEST_FULL("cachetune-colliding-monitor", false, true, + DO_TEST_FULL("cachetune-colliding-monitor", false, false, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); DO_TEST_DIFFERENT("memorytune"); - DO_TEST_FULL("memorytune-colliding-allocs", false, true, + DO_TEST_FULL("memorytune-colliding-allocs", false, false, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); - DO_TEST_FULL("memorytune-colliding-cachetune", false, true, + DO_TEST_FULL("memorytune-colliding-cachetune", false, false, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); DO_TEST("tseg"); -- 2.31.1

I have added 2 new macros to call tests which are expected to fail in order to make the code more consistent and readable. Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- tests/genericxml2xmltest.c | 63 +++++++++++++++----------------------- 1 file changed, 25 insertions(+), 38 deletions(-) diff --git a/tests/genericxml2xmltest.c b/tests/genericxml2xmltest.c index 3d5534ab54..7cf70203d7 100644 --- a/tests/genericxml2xmltest.c +++ b/tests/genericxml2xmltest.c @@ -153,6 +153,12 @@ mymain(void) #define DO_TEST_DIFFERENT(name) \ DO_TEST_FULL(name, 1, true, TEST_COMPARE_DOM_XML2XML_RESULT_SUCCESS) +#define DO_TEST_FAIL_ACTIVE(name) \ + DO_TEST_FULL(name, 0, true, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE) + +#define DO_TEST_FAIL_INACTIVE(name) \ + DO_TEST_FULL(name, 0, false, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE) + DO_TEST_DIFFERENT("disk-virtio"); DO_TEST_DIFFERENT("disk-hyperv-physical"); DO_TEST_DIFFERENT("disk-hyperv-virtual"); @@ -162,19 +168,16 @@ mymain(void) DO_TEST_DIFFERENT("graphics-vnc-socket"); DO_TEST_DIFFERENT("graphics-vnc-socket-listen"); DO_TEST_DIFFERENT("graphics-listen-back-compat"); - DO_TEST_FULL("graphics-listen-back-compat-mismatch", 0, true, - TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); + DO_TEST_FAIL_ACTIVE("graphics-listen-back-compat-mismatch"); DO_TEST_DIFFERENT("graphics-vnc-listen-attr-only"); DO_TEST_DIFFERENT("graphics-vnc-listen-element-minimal"); DO_TEST_DIFFERENT("graphics-vnc-listen-element-with-address"); DO_TEST_DIFFERENT("graphics-vnc-socket-attr-listen-address"); DO_TEST_DIFFERENT("graphics-vnc-socket-attr-listen-socket"); - DO_TEST_FULL("graphics-vnc-socket-attr-listen-socket-mismatch", 0, true, - TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); + DO_TEST_FAIL_ACTIVE("graphics-vnc-socket-attr-listen-socket-mismatch"); DO_TEST("graphics-vnc-autoport-no"); - DO_TEST_FULL("name-slash-fail", 0, true, - TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); + DO_TEST_FAIL_ACTIVE("name-slash-fail"); DO_TEST("perf"); @@ -188,47 +191,31 @@ mymain(void) DO_TEST("network-interface-mac-check"); DO_TEST_DIFFERENT("chardev-tcp"); - DO_TEST_FULL("chardev-tcp-missing-host", 0, true, - TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); - DO_TEST_FULL("chardev-tcp-missing-service", 0, true, - TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); - DO_TEST_FULL("chardev-tcp-multiple-source", 0, true, - TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); + DO_TEST_FAIL_ACTIVE("chardev-tcp-missing-host"); + DO_TEST_FAIL_ACTIVE("chardev-tcp-missing-service"); + DO_TEST_FAIL_ACTIVE("chardev-tcp-multiple-source"); DO_TEST_DIFFERENT("chardev-udp"); - DO_TEST_FULL("chardev-udp-missing-connect-service", 0, true, - TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); - DO_TEST_FULL("chardev-udp-multiple-source", 0, true, - TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); + DO_TEST_FAIL_ACTIVE("chardev-udp-missing-connect-service"); + DO_TEST_FAIL_ACTIVE("chardev-udp-multiple-source"); DO_TEST_DIFFERENT("chardev-unix"); - DO_TEST_FULL("chardev-unix-smartcard-missing-path", 0, true, - TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); - DO_TEST_FULL("chardev-unix-redirdev-missing-path", 0, true, - TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); - DO_TEST_FULL("chardev-unix-rng-missing-path", 0, true, - TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); + DO_TEST_FAIL_ACTIVE("chardev-unix-smartcard-missing-path"); + DO_TEST_FAIL_ACTIVE("chardev-unix-redirdev-missing-path"); + DO_TEST_FAIL_ACTIVE("chardev-unix-rng-missing-path"); DO_TEST_DIFFERENT("chardev-reconnect"); - DO_TEST_FULL("chardev-reconnect-missing-timeout", 0, true, - TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); - DO_TEST_FULL("chardev-reconnect-invalid-mode", 0, true, - TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); + DO_TEST_FAIL_ACTIVE("chardev-reconnect-missing-timeout"); + DO_TEST_FAIL_ACTIVE("chardev-reconnect-invalid-mode"); DO_TEST("cachetune-small"); DO_TEST("cachetune-cdp"); DO_TEST_DIFFERENT("cachetune"); DO_TEST_DIFFERENT("cachetune-extra-tunes"); - DO_TEST_FULL("cachetune-colliding-allocs", false, false, - TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); - DO_TEST_FULL("cachetune-colliding-tunes", false, false, - TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); - DO_TEST_FULL("cachetune-colliding-types", false, false, - TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); - DO_TEST_FULL("cachetune-colliding-monitor", false, false, - TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); + DO_TEST_FAIL_INACTIVE("cachetune-colliding-allocs"); + DO_TEST_FAIL_INACTIVE("cachetune-colliding-tunes"); + DO_TEST_FAIL_INACTIVE("cachetune-colliding-types"); + DO_TEST_FAIL_INACTIVE("cachetune-colliding-monitor"); DO_TEST_DIFFERENT("memorytune"); - DO_TEST_FULL("memorytune-colliding-allocs", false, false, - TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); - DO_TEST_FULL("memorytune-colliding-cachetune", false, false, - TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); + DO_TEST_FAIL_INACTIVE("memorytune-colliding-allocs"); + DO_TEST_FAIL_INACTIVE("memorytune-colliding-cachetune"); DO_TEST("tseg"); -- 2.31.1

Having negation in a name of a bool variable seems a bit confusing to me. I think the substitution makes the code much more readable. Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- diff to v1: * I forgot to change inactive_only variable in struct testInfo tests/genericxml2xmltest.c | 50 +++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/tests/genericxml2xmltest.c b/tests/genericxml2xmltest.c index 17448a3465..4eafb42a84 100644 --- a/tests/genericxml2xmltest.c +++ b/tests/genericxml2xmltest.c @@ -18,7 +18,7 @@ static virDomainXMLOption *xmlopt; struct testInfo { const char *name; int different; - bool inactive_only; + bool active_only; testCompareDomXML2XMLResult expectResult; }; @@ -37,7 +37,7 @@ testCompareXMLToXMLHelper(const void *data) ret = testCompareDomXML2XMLFiles(caps, xmlopt, xml_in, info->different ? xml_out : xml_in, - !info->inactive_only, 0, + info->active_only, 0, info->expectResult); VIR_FREE(xml_in); VIR_FREE(xml_out); @@ -138,9 +138,9 @@ mymain(void) if (!(xmlopt = virTestGenericDomainXMLConfInit())) return EXIT_FAILURE; -#define DO_TEST_FULL(name, is_different, inactive, expectResult) \ +#define DO_TEST_FULL(name, is_different, active, expectResult) \ do { \ - const struct testInfo info = {name, is_different, inactive, \ + const struct testInfo info = {name, is_different, active, \ expectResult}; \ if (virTestRun("GENERIC XML-2-XML " name, \ testCompareXMLToXMLHelper, &info) < 0) \ @@ -148,10 +148,10 @@ mymain(void) } while (0) #define DO_TEST(name) \ - DO_TEST_FULL(name, 0, false, TEST_COMPARE_DOM_XML2XML_RESULT_SUCCESS) + DO_TEST_FULL(name, 0, true, TEST_COMPARE_DOM_XML2XML_RESULT_SUCCESS) #define DO_TEST_DIFFERENT(name) \ - DO_TEST_FULL(name, 1, false, TEST_COMPARE_DOM_XML2XML_RESULT_SUCCESS) + DO_TEST_FULL(name, 1, true, TEST_COMPARE_DOM_XML2XML_RESULT_SUCCESS) DO_TEST_DIFFERENT("disk-virtio"); DO_TEST_DIFFERENT("disk-hyperv-physical"); @@ -162,18 +162,18 @@ mymain(void) DO_TEST_DIFFERENT("graphics-vnc-socket"); DO_TEST_DIFFERENT("graphics-vnc-socket-listen"); DO_TEST_DIFFERENT("graphics-listen-back-compat"); - DO_TEST_FULL("graphics-listen-back-compat-mismatch", 0, false, + DO_TEST_FULL("graphics-listen-back-compat-mismatch", 0, true, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); DO_TEST_DIFFERENT("graphics-vnc-listen-attr-only"); DO_TEST_DIFFERENT("graphics-vnc-listen-element-minimal"); DO_TEST_DIFFERENT("graphics-vnc-listen-element-with-address"); DO_TEST_DIFFERENT("graphics-vnc-socket-attr-listen-address"); DO_TEST_DIFFERENT("graphics-vnc-socket-attr-listen-socket"); - DO_TEST_FULL("graphics-vnc-socket-attr-listen-socket-mismatch", 0, false, + DO_TEST_FULL("graphics-vnc-socket-attr-listen-socket-mismatch", 0, true, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); DO_TEST("graphics-vnc-autoport-no"); - DO_TEST_FULL("name-slash-fail", 0, false, + DO_TEST_FULL("name-slash-fail", 0, true, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); DO_TEST("perf"); @@ -188,46 +188,46 @@ mymain(void) DO_TEST("network-interface-mac-check"); DO_TEST_DIFFERENT("chardev-tcp"); - DO_TEST_FULL("chardev-tcp-missing-host", 0, false, + DO_TEST_FULL("chardev-tcp-missing-host", 0, true, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); - DO_TEST_FULL("chardev-tcp-missing-service", 0, false, + DO_TEST_FULL("chardev-tcp-missing-service", 0, true, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); - DO_TEST_FULL("chardev-tcp-multiple-source", 0, false, + DO_TEST_FULL("chardev-tcp-multiple-source", 0, true, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); DO_TEST_DIFFERENT("chardev-udp"); - DO_TEST_FULL("chardev-udp-missing-connect-service", 0, false, + DO_TEST_FULL("chardev-udp-missing-connect-service", 0, true, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); - DO_TEST_FULL("chardev-udp-multiple-source", 0, false, + DO_TEST_FULL("chardev-udp-multiple-source", 0, true, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); DO_TEST_DIFFERENT("chardev-unix"); - DO_TEST_FULL("chardev-unix-smartcard-missing-path", 0, false, + DO_TEST_FULL("chardev-unix-smartcard-missing-path", 0, true, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); - DO_TEST_FULL("chardev-unix-redirdev-missing-path", 0, false, + DO_TEST_FULL("chardev-unix-redirdev-missing-path", 0, true, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); - DO_TEST_FULL("chardev-unix-rng-missing-path", 0, false, + DO_TEST_FULL("chardev-unix-rng-missing-path", 0, true, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); DO_TEST_DIFFERENT("chardev-reconnect"); - DO_TEST_FULL("chardev-reconnect-missing-timeout", 0, false, + DO_TEST_FULL("chardev-reconnect-missing-timeout", 0, true, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); - DO_TEST_FULL("chardev-reconnect-invalid-mode", 0, false, + DO_TEST_FULL("chardev-reconnect-invalid-mode", 0, true, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); DO_TEST("cachetune-small"); DO_TEST("cachetune-cdp"); DO_TEST("cachetune"); DO_TEST_DIFFERENT("cachetune-extra-tunes"); - DO_TEST_FULL("cachetune-colliding-allocs", false, true, + DO_TEST_FULL("cachetune-colliding-allocs", false, false, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); - DO_TEST_FULL("cachetune-colliding-tunes", false, true, + DO_TEST_FULL("cachetune-colliding-tunes", false, false, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); - DO_TEST_FULL("cachetune-colliding-types", false, true, + DO_TEST_FULL("cachetune-colliding-types", false, false, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); - DO_TEST_FULL("cachetune-colliding-monitor", false, true, + DO_TEST_FULL("cachetune-colliding-monitor", false, false, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); DO_TEST_DIFFERENT("memorytune"); - DO_TEST_FULL("memorytune-colliding-allocs", false, true, + DO_TEST_FULL("memorytune-colliding-allocs", false, false, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); - DO_TEST_FULL("memorytune-colliding-cachetune", false, true, + DO_TEST_FULL("memorytune-colliding-cachetune", false, false, TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE); DO_TEST("tseg"); -- 2.31.1

On 7/20/21 4:44 PM, Kristina Hanicova wrote:
Kristina Hanicova (2): genericxml2xmltest: Substitute 'inactive' variable with 'active' genericxml2xmltest: Refactor DO_TEST_FULL() calls
tests/genericxml2xmltest.c | 71 ++++++++++++++++---------------------- 1 file changed, 29 insertions(+), 42 deletions(-)
I had some difficulties applying these because they were not rebased on the top of your former patches. But that's okay, because they weren't merged when you send these. Reviewed-by: Michal Privoznik <mprivozn@redhat.com> and pushed. Michal
participants (2)
-
Kristina Hanicova
-
Michal Prívozník