[PATCH 0/6] tests: Detect for unused files
One of the functionality implemented into qemuxmlconftest is to detect unused test files. This generalizes the idea and implements it into two other tests (networkxmlconftest and cputest). But if we find this useful, it can be implemented into the rest of the tests. Now, I was contemplating an idea of turning `existingTestCases` variable into a global one (just like we have testDebug, testVerbose, etc.). It would make the changes a lot cleaner but also it's a global variable after all. What do you think? Michal Prívozník (6): testutils: Introduce unused file detection qemuxmlconftest: Switch to virTestEnumerateTestCases() networkxmlconfdata: Remove passthrough-pf.conf networkxmlconftest: Detect unused files cputestdata: Drop unused files cputest: Detect unused files tests/cputest.c | 103 ++++++++++++++---- tests/cputestdata/ppc64-guest-host-model.xml | 3 - .../ppc64-host+guest-host-model.xml | 3 - tests/cputestdata/x86_64-bogus-vendor.xml | 4 - tests/networkxmlconfdata/passthrough-pf.conf | 11 -- tests/networkxmlconftest.c | 35 +++++- tests/qemuxmlconftest.c | 82 ++++---------- tests/testutils.c | 58 ++++++++++ tests/testutils.h | 14 +++ 9 files changed, 207 insertions(+), 106 deletions(-) delete mode 100644 tests/cputestdata/ppc64-guest-host-model.xml delete mode 100644 tests/cputestdata/ppc64-host+guest-host-model.xml delete mode 100644 tests/cputestdata/x86_64-bogus-vendor.xml delete mode 100644 tests/networkxmlconfdata/passthrough-pf.conf -- 2.53.0
From: Michal Privoznik <mprivozn@redhat.com> This is basically a generalized version of what we have in qemuxmlconftest (functions testConfXMLEnumerate(), testQemuConfMarkUsed() and testConfXMLCheck()). The idea is to reuse the code in other tests. There's one slight difference to the original - while qemuxmlconftest always allocated the hash table, in this generalized version NULL table is okay. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/testutils.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++ tests/testutils.h | 14 ++++++++++++ 2 files changed, 72 insertions(+) diff --git a/tests/testutils.c b/tests/testutils.c index 35571fb2ad..484852ecc2 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -1245,3 +1245,61 @@ virCreateAnonymousFile(const uint8_t *data, size_t len) return -1; } #endif + +int +virTestEnumerateTestCases(const char *path, + virTestEnumerateTestCasesCB cb, + GHashTable **existingTestCases) +{ + g_autoptr(GHashTable) etc = virHashNew(NULL); + struct dirent *ent; + g_autoptr(DIR) dir = NULL; + int rc; + + /* If VIR_TEST_RANGE is in use don't bother filling in the data. */ + if (virTestHasRangeBitmap()) + return 0; + + if (virDirOpen(&dir, path) < 0) + return -1; + + while ((rc = virDirRead(dir, &ent, path)) > 0) { + if (cb(ent)) { + g_hash_table_insert(etc, + g_strdup_printf("%s/%s", path, ent->d_name), + NULL); + } + } + + if (rc == 0) + *existingTestCases = g_steal_pointer(&etc); + + return rc; +} + +void +virTestCaseMarkUsed(GHashTable *existingTestCases, + const char *file) +{ + if (existingTestCases && file) + g_hash_table_remove(existingTestCases, file); +} + +int +virTestCheckUnusedTestCases(GHashTable *existingTestCases) +{ + g_autofree virHashKeyValuePair *items = virHashGetItems(existingTestCases, NULL, true); + size_t i; + int ret = 0; + + for (i = 0; items && items[i].key; i++) { + if (ret == 0) { + fprintf(stderr, "\n"); + ret = -1; + } + + fprintf(stderr, "unused file: %s\n", (const char *) items[i].key); + } + + return ret; +} diff --git a/tests/testutils.h b/tests/testutils.h index e60876bd9e..ce348f1287 100644 --- a/tests/testutils.h +++ b/tests/testutils.h @@ -192,3 +192,17 @@ virTestStablePath(const char *path); #ifdef __linux__ int virCreateAnonymousFile(const uint8_t *data, size_t len); #endif + +typedef bool (*virTestEnumerateTestCasesCB)(struct dirent *); + +int +virTestEnumerateTestCases(const char *path, + virTestEnumerateTestCasesCB cb, + GHashTable **existingTestCases); + +void +virTestCaseMarkUsed(GHashTable *existingTestCases, + const char *file); + +int +virTestCheckUnusedTestCases(GHashTable *existingTestCases); -- 2.53.0
From: Michal Privoznik <mprivozn@redhat.com> The qemuxmlconftest detects unused files in qemuxmlconfdata/ directory. But it uses its own implementation for that. But now that there's a generic implementation available, switch to that. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemuxmlconftest.c | 82 ++++++++++------------------------------- 1 file changed, 19 insertions(+), 63 deletions(-) diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c index 6cfbca21a6..b64bbd6ccb 100644 --- a/tests/qemuxmlconftest.c +++ b/tests/qemuxmlconftest.c @@ -372,15 +372,6 @@ testInfoCheckDuplicate(testQemuInfo *info) } -static void -testQemuConfMarkUsed(testQemuInfo *info, - const char *file) -{ - if (file) - ignore_value(g_hash_table_remove(info->conf->existingTestCases, file)); -} - - /** * testQemuConfXMLCommon: Prepare common test data (e.g. parse input XML) * for a test case. @@ -413,10 +404,10 @@ testQemuConfXMLCommon(testQemuInfo *info, if (info->prepared) goto cleanup; - testQemuConfMarkUsed(info, info->infile); - testQemuConfMarkUsed(info, info->outfile); - testQemuConfMarkUsed(info, info->errfile); - testQemuConfMarkUsed(info, info->out_xml_inactive); + virTestCaseMarkUsed(info->conf->existingTestCases, info->infile); + virTestCaseMarkUsed(info->conf->existingTestCases, info->outfile); + virTestCaseMarkUsed(info->conf->existingTestCases, info->errfile); + virTestCaseMarkUsed(info->conf->existingTestCases, info->out_xml_inactive); if (testQemuInfoInitArgs((testQemuInfo *) info) < 0) goto cleanup; @@ -591,7 +582,7 @@ testExtDeviceArgv(testQemuInfo *info, outfile = g_strdup_printf("%s/qemuxmlconfdata/%s%s%s.%s%zu.args", abs_srcdir, info->name, info->suffix, info->args.capsvariant, helper, idx); - testQemuConfMarkUsed(info, outfile); + virTestCaseMarkUsed(info->conf->existingTestCases, outfile); if (!cmd) { err = virGetLastError(); @@ -787,51 +778,12 @@ testCompareXMLToArgv(const void *data) } -static int -testConfXMLCheck(GHashTable *existingTestCases) +static bool +testConfXMLEnumerate(struct dirent *ent) { - g_autofree virHashKeyValuePair *items = virHashGetItems(existingTestCases, NULL, true); - size_t i; - int ret = 0; - - for (i = 0; items[i].key; i++) { - if (ret == 0) - fprintf(stderr, "\n"); - - fprintf(stderr, "unused file: %s\n", (const char *) items[i].key); - ret = -1; - } - - return ret; -} - - -static int -testConfXMLEnumerate(GHashTable *existingTestCases) -{ - struct dirent *ent; - g_autoptr(DIR) dir = NULL; - int rc; - - /* If VIR_TEST_RANGE is in use don't bother filling in the data, which - * also makes testConfXMLCheck succeed. */ - if (virTestHasRangeBitmap()) - return 0; - - if (virDirOpen(&dir, abs_srcdir "/qemuxmlconfdata") < 0) - return -1; - - while ((rc = virDirRead(dir, &ent, abs_srcdir "/qemuxmlconfdata")) > 0) { - if (virStringHasSuffix(ent->d_name, ".xml") || - virStringHasSuffix(ent->d_name, ".args") || - virStringHasSuffix(ent->d_name, ".err")) { - g_hash_table_insert(existingTestCases, - g_strdup_printf(abs_srcdir "/qemuxmlconfdata/%s", ent->d_name), - NULL); - } - } - - return rc; + return virStringHasSuffix(ent->d_name, ".xml") || + virStringHasSuffix(ent->d_name, ".args") || + virStringHasSuffix(ent->d_name, ".err"); } @@ -902,23 +854,27 @@ mymain(void) int ret = 0; g_autoptr(virConnect) conn = NULL; g_autoptr(GHashTable) duplicateTests = virHashNew(NULL); - g_autoptr(GHashTable) existingTestCases = virHashNew(NULL); + g_autoptr(GHashTable) existingTestCases = NULL; g_autoptr(GHashTable) capslatest = testQemuGetLatestCaps(); g_autoptr(GHashTable) qapiSchemaCache = virHashNew((GDestroyNotify) g_hash_table_unref); g_autoptr(GHashTable) capscache = virHashNew(virObjectUnref); struct testQemuConf testConf = { .capslatest = capslatest, .capscache = capscache, .qapiSchemaCache = qapiSchemaCache, - .duplicateTests = duplicateTests, - .existingTestCases = existingTestCases }; + .duplicateTests = duplicateTests }; if (!capslatest) return EXIT_FAILURE; /* enumerate and store all available test cases to verify at the end that * all of them were invoked */ - if (testConfXMLEnumerate(existingTestCases) < 0) + if (virTestEnumerateTestCases(abs_srcdir "/qemuxmlconfdata", + testConfXMLEnumerate, + &existingTestCases) < 0) { return EXIT_FAILURE; + } + + testConf.existingTestCases = existingTestCases; /* Set the timezone because we are mocking the time() function. * If we don't do that, then localtime() may return unpredictable @@ -3078,7 +3034,7 @@ mymain(void) DO_TEST_CAPS_ARCH_LATEST("aarch64-virt-virtualization", "aarch64"); /* check that all input files were actually used here */ - if (testConfXMLCheck(existingTestCases) < 0) + if (virTestCheckUnusedTestCases(existingTestCases) < 0) ret = -1; qemuTestDriverFree(&driver); -- 2.53.0
From: Michal Privoznik <mprivozn@redhat.com> The passthrough-pf.conf file is not used really, because the test case is defined as: DO_TEST_VALIDATE_ERROR("passthrough-pf"); meaning the test is expected to fail in XML validation phase. Hence, no .conf file is ever generated for it. Remove the file. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/networkxmlconfdata/passthrough-pf.conf | 11 ----------- 1 file changed, 11 deletions(-) delete mode 100644 tests/networkxmlconfdata/passthrough-pf.conf diff --git a/tests/networkxmlconfdata/passthrough-pf.conf b/tests/networkxmlconfdata/passthrough-pf.conf deleted file mode 100644 index 1957dc8011..0000000000 --- a/tests/networkxmlconfdata/passthrough-pf.conf +++ /dev/null @@ -1,11 +0,0 @@ -##WARNING: THIS IS AN AUTO-GENERATED FILE. CHANGES TO IT ARE LIKELY TO BE -##OVERWRITTEN AND LOST. Changes to this configuration should be made using: -## virsh net-edit local -## or other application using the libvirt API. -## -## dnsmasq conf file created by libvirt -strict-order -except-interface=lo -bind-dynamic -interface=(null) -addn-hosts=/var/lib/libvirt/dnsmasq/local.addnhosts -- 2.53.0
From: Michal Privoznik <mprivozn@redhat.com> Use newly introduced testutils APIs to detect unused files. This is pretty much straightforward except for one test case: hostdev. This test case queries sysfs under the hood and thus is expected to succeed on Linux and fail everywhere else. Though, hostdev.expect.xml is thus used on Linux only. Therefore, do not collect it on non-Linux platforms. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/networkxmlconftest.c | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/tests/networkxmlconftest.c b/tests/networkxmlconftest.c index 7f98004c75..40eb3ffdb4 100644 --- a/tests/networkxmlconftest.c +++ b/tests/networkxmlconftest.c @@ -35,6 +35,7 @@ struct _testInfo { char *outxml; char *outconf; char *outhostsfile; + GHashTable *existingTestCases; }; typedef struct _testInfo testInfo; @@ -63,6 +64,7 @@ testCompareXMLToXMLFiles(const void *data) testCompareNetXML2XMLResult result = TEST_COMPARE_NET_XML2XML_RESULT_SUCCESS; g_autoptr(virNetworkDef) def = NULL; + virTestCaseMarkUsed(info->existingTestCases, info->inxml); if (!(def = virNetworkDefParse(NULL, info->inxml, info->xmlopt, false))) { result = TEST_COMPARE_NET_XML2XML_RESULT_FAIL_PARSE; goto cleanup; @@ -84,6 +86,7 @@ testCompareXMLToXMLFiles(const void *data) if (info->expectResult == TEST_COMPARE_NET_XML2XML_RESULT_FAIL_FORMAT) goto cleanup; + virTestCaseMarkUsed(info->existingTestCases, info->outxml); if (virTestCompareToFile(actual, info->outxml) < 0) { result = TEST_COMPARE_NET_XML2XML_RESULT_FAIL_COMPARE; goto cleanup; @@ -129,6 +132,7 @@ testCompareXMLToConfFiles(const void *data) if (!(def = g_steal_pointer(&info->def))) { /* Previous test wasn't executed. */ + virTestCaseMarkUsed(info->existingTestCases, info->inxml); if (!(def = virNetworkDefParse(NULL, info->inxml, info->xmlopt, false))) goto cleanup; @@ -168,10 +172,12 @@ testCompareXMLToConfFiles(const void *data) } #endif + virTestCaseMarkUsed(info->existingTestCases, info->outconf); if (virTestCompareToFile(confactual, info->outconf) < 0) compareFailed = true; if (hostsfileactual) { + virTestCaseMarkUsed(info->existingTestCases, info->outhostsfile); if (virTestCompareToFile(hostsfileactual, info->outhostsfile) < 0) { compareFailed = true; } @@ -232,6 +238,7 @@ testRun(const char *name, virNetworkXMLOption *xmlopt, dnsmasqCaps *caps, testCompareNetXML2XMLResult expectResult, + GHashTable *existingTestCases, unsigned int flags) { g_autofree char *name_xml2xml = g_strdup_printf("Network XML-2-XML %s", name); @@ -247,6 +254,7 @@ testRun(const char *name, info->outxml = g_strdup_printf("%s/networkxmlconfdata/%s.expect.xml", abs_srcdir, name); info->outconf = g_strdup_printf("%s/networkxmlconfdata/%s.conf", abs_srcdir, name); info->outhostsfile = g_strdup_printf("%s/networkxmlconfdata/%s.hostsfile", abs_srcdir, name); + info->existingTestCases = existingTestCases; virTestRunLog(ret, name_xml2xml, testCompareXMLToXMLFiles, info); @@ -254,13 +262,35 @@ testRun(const char *name, virTestRunLog(ret, name_xml2conf, testCompareXMLToConfFiles, info); } + +static bool +testCaseEnumerate(struct dirent *ent) +{ +#ifndef __linux__ + /* This test case is ran only on Linux. See comment in mymain(). */ + if (STREQ(ent->d_name, "hostdev.expect.xml")) + return false; +#endif + return virStringHasSuffix(ent->d_name, ".xml") || + virStringHasSuffix(ent->d_name, ".conf") || + virStringHasSuffix(ent->d_name, ".hostsfile"); +} + + static int mymain(void) { g_autoptr(virNetworkXMLOption) xmlopt = NULL; g_autoptr(dnsmasqCaps) caps = NULL; + g_autoptr(GHashTable) existingTestCases = NULL; int ret = 0; + if (virTestEnumerateTestCases(abs_srcdir "/networkxmlconfdata", + testCaseEnumerate, + &existingTestCases) < 0) { + return -1; + } + if (!(xmlopt = networkDnsmasqCreateXMLConf())) return -1; @@ -268,7 +298,7 @@ mymain(void) return -1; #define DO_TEST_FULL(name, flags, expectResult) \ - testRun(name, &ret, xmlopt, caps, expectResult, flags) + testRun(name, &ret, xmlopt, caps, expectResult, existingTestCases, flags) #define DO_TEST(name) \ DO_TEST_FULL(name, 0, TEST_COMPARE_NET_XML2XML_RESULT_SUCCESS) #define DO_TEST_FLAGS(name, flags) \ @@ -340,6 +370,9 @@ mymain(void) DO_TEST("leasetime-infinite"); DO_TEST("isolated-ports"); + if (virTestCheckUnusedTestCases(existingTestCases) < 0) + ret = -1; + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.53.0
From: Michal Privoznik <mprivozn@redhat.com> There are couple of files under tests/cputestdata/ that are not used by any test case: 1) ppc64-guest-host-model.xml - unused since its introduction in v1.2.19-rc1~31 2) ppc64-host+guest-host-model.xml - Same 3) x86_64-bogus-vendor.xml - Introduced in v0.8.7~195 under slightly different name, then renamed to the current name (in v3.1.0-rc1~3) but never used actually. Just drop these three files. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/cputestdata/ppc64-guest-host-model.xml | 3 --- tests/cputestdata/ppc64-host+guest-host-model.xml | 3 --- tests/cputestdata/x86_64-bogus-vendor.xml | 4 ---- 3 files changed, 10 deletions(-) delete mode 100644 tests/cputestdata/ppc64-guest-host-model.xml delete mode 100644 tests/cputestdata/ppc64-host+guest-host-model.xml delete mode 100644 tests/cputestdata/x86_64-bogus-vendor.xml diff --git a/tests/cputestdata/ppc64-guest-host-model.xml b/tests/cputestdata/ppc64-guest-host-model.xml deleted file mode 100644 index 188ebebb72..0000000000 --- a/tests/cputestdata/ppc64-guest-host-model.xml +++ /dev/null @@ -1,3 +0,0 @@ -<cpu mode='host-model'> - <model fallback='allow'/> -</cpu> diff --git a/tests/cputestdata/ppc64-host+guest-host-model.xml b/tests/cputestdata/ppc64-host+guest-host-model.xml deleted file mode 100644 index 188ebebb72..0000000000 --- a/tests/cputestdata/ppc64-host+guest-host-model.xml +++ /dev/null @@ -1,3 +0,0 @@ -<cpu mode='host-model'> - <model fallback='allow'/> -</cpu> diff --git a/tests/cputestdata/x86_64-bogus-vendor.xml b/tests/cputestdata/x86_64-bogus-vendor.xml deleted file mode 100644 index 2ffdefe413..0000000000 --- a/tests/cputestdata/x86_64-bogus-vendor.xml +++ /dev/null @@ -1,4 +0,0 @@ -<cpu match='minimum'> - <model>Penryn</model> - <vendor>Bogus</vendor> -</cpu> -- 2.53.0
From: Michal Privoznik <mprivozn@redhat.com> Use newly introduced testutils APIs to detect unused files. This is pretty much straightforward, except for one small thing: some test cases depend on QEMU (and are NOP if built without it). Hence the condition in testCaseEnumerate(). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/cputest.c | 103 ++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 82 insertions(+), 21 deletions(-) diff --git a/tests/cputest.c b/tests/cputest.c index a7d6ea8736..e29c81cb8e 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -60,6 +60,7 @@ struct data { int ncpus; unsigned int flags; int result; + GHashTable *existingTestCases; }; #if WITH_QEMU @@ -68,7 +69,9 @@ static virQEMUDriver driver; static virCPUDef * -cpuTestLoadXML(virArch arch, const char *name) +cpuTestLoadXML(virArch arch, + const char *name, + GHashTable *existingTestCases) { g_autofree char *xml = NULL; g_autoptr(xmlDoc) doc = NULL; @@ -78,6 +81,8 @@ cpuTestLoadXML(virArch arch, const char *name) xml = g_strdup_printf("%s/cputestdata/%s-%s.xml", abs_srcdir, virArchToString(arch), name); + virTestCaseMarkUsed(existingTestCases, xml); + if (!(doc = virXMLParseFileCtxt(xml, &ctxt))) return NULL; @@ -90,6 +95,7 @@ cpuTestLoadXML(virArch arch, const char *name) static virCPUDef ** cpuTestLoadMultiXML(virArch arch, const char *name, + GHashTable *existingTestCases, unsigned int *count) { g_autofree char *xml = NULL; @@ -103,6 +109,8 @@ cpuTestLoadMultiXML(virArch arch, xml = g_strdup_printf("%s/cputestdata/%s-%s.xml", abs_srcdir, virArchToString(arch), name); + virTestCaseMarkUsed(existingTestCases, xml); + if (!(doc = virXMLParseFileCtxt(xml, &ctxt))) return NULL; @@ -136,7 +144,8 @@ cpuTestLoadMultiXML(virArch arch, static int cpuTestCompareXML(virArch arch, virCPUDef *cpu, - const char *name) + const char *name, + GHashTable *existingTestCases) { g_autofree char *xml = NULL; g_autofree char *actual = NULL; @@ -144,6 +153,8 @@ cpuTestCompareXML(virArch arch, xml = g_strdup_printf("%s/cputestdata/%s-%s.xml", abs_srcdir, virArchToString(arch), name); + virTestCaseMarkUsed(existingTestCases, xml); + if (!(actual = virCPUDefFormat(cpu, NULL))) return -1; @@ -190,9 +201,12 @@ cpuTestCompare(const void *arg) g_autoptr(virCPUDef) cpu = NULL; virCPUCompareResult result; - if (!(host = cpuTestLoadXML(data->arch, data->host)) || - !(cpu = cpuTestLoadXML(data->arch, data->name))) + if (!(host = cpuTestLoadXML(data->arch, data->host, + data->existingTestCases)) || + !(cpu = cpuTestLoadXML(data->arch, data->name, + data->existingTestCases))) { return -1; + } result = virCPUCompare(host->arch, host, cpu, false); if (data->result == VIR_CPU_COMPARE_ERROR) @@ -222,9 +236,12 @@ cpuTestGuestCPU(const void *arg) g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; g_autofree char *result = NULL; - if (!(host = cpuTestLoadXML(data->arch, data->host)) || - !(cpu = cpuTestLoadXML(data->arch, data->name))) + if (!(host = cpuTestLoadXML(data->arch, data->host, + data->existingTestCases)) || + !(cpu = cpuTestLoadXML(data->arch, data->name, + data->existingTestCases))) { goto cleanup; + } if (virCPUConvertLegacy(host->arch, cpu) < 0) goto cleanup; @@ -249,7 +266,7 @@ cpuTestGuestCPU(const void *arg) result = virBufferContentAndReset(&buf); - if (cpuTestCompareXML(data->arch, cpu, result) < 0) + if (cpuTestCompareXML(data->arch, cpu, result, data->existingTestCases) < 0) goto cleanup; ret = 0; @@ -284,8 +301,10 @@ cpuTestBaseline(const void *arg) const char *suffix; size_t i; - if (!(cpus = cpuTestLoadMultiXML(data->arch, data->name, &ncpus))) + if (!(cpus = cpuTestLoadMultiXML(data->arch, data->name, + data->existingTestCases, &ncpus))) { goto cleanup; + } baseline = virCPUBaseline(data->arch, cpus, ncpus, NULL, NULL, !!(data->flags & VIR_CONNECT_BASELINE_CPU_MIGRATABLE)); @@ -317,8 +336,10 @@ cpuTestBaseline(const void *arg) suffix = "result"; result = g_strdup_printf("%s-%s", data->name, suffix); - if (cpuTestCompareXML(data->arch, baseline, result) < 0) + if (cpuTestCompareXML(data->arch, baseline, result, + data->existingTestCases) < 0) { goto cleanup; + } for (i = 0; i < ncpus; i++) { virCPUCompareResult cmp; @@ -356,9 +377,12 @@ cpuTestUpdate(const void *arg) g_autoptr(virCPUDef) cpu = NULL; g_autofree char *result = NULL; - if (!(host = cpuTestLoadXML(data->arch, data->host)) || - !(cpu = cpuTestLoadXML(data->arch, data->name))) + if (!(host = cpuTestLoadXML(data->arch, data->host, + data->existingTestCases)) || + !(cpu = cpuTestLoadXML(data->arch, data->name, + data->existingTestCases))) { return -1; + } if (!(migHost = virCPUCopyMigratable(data->arch, host))) return -1; @@ -368,7 +392,7 @@ cpuTestUpdate(const void *arg) result = g_strdup_printf("%s+%s", data->host, data->name); - return cpuTestCompareXML(data->arch, cpu, result); + return cpuTestCompareXML(data->arch, cpu, result, data->existingTestCases); } @@ -380,7 +404,7 @@ cpuTestHasFeature(const void *arg) g_autoptr(virCPUData) hostData = NULL; int result; - if (!(host = cpuTestLoadXML(data->arch, data->host))) + if (!(host = cpuTestLoadXML(data->arch, data->host, data->existingTestCases))) return -1; if (cpuEncode(host->arch, host, NULL, &hostData, @@ -415,7 +439,7 @@ cpuTestValidateFeatures(const void *arg) g_autoptr(virCPUDef) cpu = NULL; int result; - if (!(cpu = cpuTestLoadXML(data->arch, data->name))) + if (!(cpu = cpuTestLoadXML(data->arch, data->name, data->existingTestCases))) return -1; result = virCPUValidateFeatures(data->arch, cpu); @@ -469,6 +493,8 @@ cpuTestMakeQEMUCaps(const struct data *data) json = g_strdup_printf("%s/cputestdata/%s-cpuid-%s.json", abs_srcdir, virArchToString(data->arch), data->host); + virTestCaseMarkUsed(data->existingTestCases, json); + if (!(testMon = qemuMonitorTestNewFromFile(json, driver.xmlopt, true))) return NULL; @@ -556,6 +582,8 @@ cpuTestCPUID(bool guest, const void *arg) hostFile = g_strdup_printf("%s/cputestdata/%s-cpuid-%s.xml", abs_srcdir, virArchToString(data->arch), data->host); + virTestCaseMarkUsed(data->existingTestCases, hostFile); + if (virTestLoadFile(hostFile, &host) < 0 || !(hostData = virCPUDataParse(host))) return -1; @@ -583,7 +611,7 @@ cpuTestCPUID(bool guest, const void *arg) result = g_strdup_printf("cpuid-%s-%s", data->host, guest ? "guest" : "host"); - return cpuTestCompareXML(data->arch, cpu, result); + return cpuTestCompareXML(data->arch, cpu, result, data->existingTestCases); } @@ -602,8 +630,10 @@ cpuTestCPUIDBaseline(const void *arg) g_autofree char *name = NULL; name = g_strdup_printf("cpuid-%s-json", data->cpus[i]); - if (!(cpus[i] = cpuTestLoadXML(data->arch, name))) + if (!(cpus[i] = cpuTestLoadXML(data->arch, name, + data->existingTestCases))) { goto cleanup; + } } baseline = virCPUBaseline(data->arch, cpus, data->ncpus, NULL, NULL, false); @@ -612,8 +642,10 @@ cpuTestCPUIDBaseline(const void *arg) result = g_strdup_printf("cpuid-baseline-%s", data->name); - if (cpuTestCompareXML(data->arch, baseline, result) < 0) + if (cpuTestCompareXML(data->arch, baseline, result, + data->existingTestCases) < 0) { goto cleanup; + } for (i = 0; i < data->ncpus; i++) { virCPUCompareResult cmp; @@ -675,6 +707,7 @@ cpuTestCompareSignature(const struct data *data, "model: %3$3u (0x%3$02x)\n" "stepping: %4$3u (0x%4$02x)\n", signature, family, model, stepping); + virTestCaseMarkUsed(data->existingTestCases, result); return virTestCompareToFile(sigStr, result); } @@ -690,6 +723,8 @@ cpuTestCPUIDSignature(const void *arg) hostFile = g_strdup_printf("%s/cputestdata/%s-cpuid-%s.xml", abs_srcdir, virArchToString(data->arch), data->host); + virTestCaseMarkUsed(data->existingTestCases, hostFile); + if (virTestLoadFile(hostFile, &host) < 0 || !(hostData = virCPUDataParse(host))) return -1; @@ -805,24 +840,28 @@ cpuTestUpdateLive(const void *arg) g_autoptr(virDomainCapsCPUModels) models = NULL; cpuFile = g_strdup_printf("cpuid-%s-guest", data->host); - if (!(cpu = cpuTestLoadXML(data->arch, cpuFile))) + if (!(cpu = cpuTestLoadXML(data->arch, cpuFile, data->existingTestCases))) return -1; enabledFile = g_strdup_printf("%s/cputestdata/%s-cpuid-%s-enabled.xml", abs_srcdir, virArchToString(data->arch), data->host); + virTestCaseMarkUsed(data->existingTestCases, enabledFile); if (virTestLoadFile(enabledFile, &enabled) < 0 || !(enabledData = virCPUDataParse(enabled))) return -1; disabledFile = g_strdup_printf("%s/cputestdata/%s-cpuid-%s-disabled.xml", abs_srcdir, virArchToString(data->arch), data->host); + virTestCaseMarkUsed(data->existingTestCases, disabledFile); if (virTestLoadFile(disabledFile, &disabled) < 0 || !(disabledData = virCPUDataParse(disabled))) return -1; expectedFile = g_strdup_printf("cpuid-%s-json", data->host); - if (!(expected = cpuTestLoadXML(data->arch, expectedFile))) + if (!(expected = cpuTestLoadXML(data->arch, expectedFile, + data->existingTestCases))) { return -1; + } /* In case the host CPU signature does not exactly match any CPU model in * src/cpu_map, the CPU model we detect from CPUID may differ from the one @@ -889,7 +928,7 @@ cpuTestJSONCPUID(const void *arg) if (virQEMUCapsInitCPUModel(qemuCaps, VIR_DOMAIN_VIRT_KVM, cpu, false) != 0) return -1; - return cpuTestCompareXML(data->arch, cpu, result); + return cpuTestCompareXML(data->arch, cpu, result, data->existingTestCases); } @@ -938,9 +977,21 @@ cpuTestInitModels(const char **list) } +static bool +testCaseEnumerate(struct dirent *ent) +{ + return virStringHasSuffix(ent->d_name, ".xml") || +#if WITH_QEMU + virStringHasSuffix(ent->d_name, ".json") || +#endif + 0; +} + + static int mymain(void) { + g_autoptr(GHashTable) existingTestCases = NULL; virDomainCapsCPUModels *model486 = NULL; virDomainCapsCPUModels *nomodel = NULL; virDomainCapsCPUModels *models = NULL; @@ -948,6 +999,12 @@ mymain(void) virDomainCapsCPUModels *ppc_models = NULL; int ret = 0; + if (virTestEnumerateTestCases(abs_srcdir "/cputestdata", + testCaseEnumerate, + &existingTestCases) < 0) { + return EXIT_FAILURE; + } + #if WITH_QEMU if (qemuTestDriverInit(&driver) < 0) return EXIT_FAILURE; @@ -970,7 +1027,8 @@ mymain(void) struct data data = { \ arch, host, cpu, models, \ models == NULL ? NULL : #models, \ - cpus, ncpus, flags, result \ + cpus, ncpus, flags, result, \ + existingTestCases \ }; \ g_autofree char *testLabel = NULL; \ \ @@ -1296,6 +1354,9 @@ mymain(void) virObjectUnref(haswell); virObjectUnref(ppc_models); + if (virTestCheckUnusedTestCases(existingTestCases) < 0) + ret = -1; + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.53.0
On a Thursday in 2026, Michal Privoznik via Devel wrote:
One of the functionality implemented into qemuxmlconftest is to detect unused test files. This generalizes the idea and implements it into two other tests (networkxmlconftest and cputest). But if we find this useful, it can be implemented into the rest of the tests.
Now, I was contemplating an idea of turning `existingTestCases` variable into a global one (just like we have testDebug, testVerbose, etc.). It would make the changes a lot cleaner but also it's a global variable after all. What do you think?
Michal Prívozník (6): testutils: Introduce unused file detection qemuxmlconftest: Switch to virTestEnumerateTestCases() networkxmlconfdata: Remove passthrough-pf.conf networkxmlconftest: Detect unused files cputestdata: Drop unused files cputest: Detect unused files
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Michal Privoznik