diff mbox series

[05/12] tests: qemuxml2argv: add testInfoSetPaths

Message ID 360558146ac0c43c7779c522d0b4d90078ccd950.1554137098.git.crobinso@redhat.com
State Accepted
Commit 180bf85c72a78119f548de3d0a1b7dd82b5b349b
Headers show
Series tests: qemuxml2xml: add DO_TEST_CAPS* | expand

Commit Message

Cole Robinson April 1, 2019, 4:47 p.m. UTC 
This moves infile and outfile building outside the test case,
which better fits the pattern of qemuxml2xmltest. It also lets us
drop the qemuxml2argtest-specific 'suffix' from testInfo

Signed-off-by: Cole Robinson <crobinso@redhat.com>

---
 tests/qemuxml2argvtest.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

-- 
2.21.0

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

Comments

Andrea Bolognani April 10, 2019, 3:58 p.m. UTC | #1 
On Mon, 2019-04-01 at 12:47 -0400, Cole Robinson wrote:
[...]
> @@ -758,6 +747,17 @@ testInfoClear(struct testInfo *info)

>      virObjectUnref(info->qemuCaps);

>  }

>  

> +static int

> +testInfoSetPaths(struct testInfo *info, const char *suffix)


One argument per line.

> +{

> +    if (virAsprintf(&info->infile, "%s/qemuxml2argvdata/%s.xml",

> +                    abs_srcdir, info->name) < 0 ||

> +        virAsprintf(&info->outfile, "%s/qemuxml2argvdata/%s%s.args",

> +                    abs_srcdir, info->name, suffix ? suffix : "") < 0)

> +        return -1;


This is very ugly. Please at least put curly braces around the if()
body and leave an empty line after it.

[...]
> @@ -883,11 +883,12 @@ mymain(void)

>      do { \

>          static struct testInfo info = { \

>              .name = _name, \

> -            .suffix = _suffix, \

>          }; \

>          if (testInfoSetArgs(&info, capslatest, \

>                              __VA_ARGS__, ARG_END) < 0) \

>              return EXIT_FAILURE; \

> +        if (testInfoSetPaths(&info, _suffix)) \

> +            return EXIT_FAILURE; \


You should check whether testInfoSetPaths() returns a negative
value.

Also in xml2xml you print out a nice message with VIR_TEST_DEBUG()
when the function fails: you should do the same here as well.

With the above addressed,

  Reviewed-by: Andrea Bolognani <abologna@redhat.com>


-- 
Andrea Bolognani / Red Hat / Virtualization

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

Patch

diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index ff7bacf8db..a3fee41ea9 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -298,7 +298,6 @@  typedef enum {
 
 struct testInfo {
     const char *name;
-    const char *suffix;
     char *infile;
     char *outfile;
     virQEMUCapsPtr qemuCaps;
@@ -431,7 +430,6 @@  testCompareXMLToArgv(const void *data)
     struct testInfo *info = (void *) data;
     char *migrateURI = NULL;
     char *actualargv = NULL;
-    const char *suffix = info->suffix;
     unsigned int flags = info->flags;
     unsigned int parseFlags = info->parseFlags;
     int ret = -1;
@@ -448,9 +446,6 @@  testCompareXMLToArgv(const void *data)
     if (!(conn = virGetConnect()))
         goto cleanup;
 
-    if (!suffix)
-        suffix = "";
-
     conn->secretDriver = &fakeSecretDriver;
     conn->storageDriver = &fakeStorageDriver;
     conn->nwfilterDriver = &fakeNWFilterDriver;
@@ -471,12 +466,6 @@  testCompareXMLToArgv(const void *data)
     if (qemuTestCapsCacheInsert(driver.qemuCapsCache, info->qemuCaps) < 0)
         goto cleanup;
 
-    if (virAsprintf(&info->infile, "%s/qemuxml2argvdata/%s.xml",
-                    abs_srcdir, info->name) < 0 ||
-        virAsprintf(&info->outfile, "%s/qemuxml2argvdata/%s%s.args",
-                    abs_srcdir, info->name, suffix) < 0)
-        goto cleanup;
-
     if (info->migrateFrom &&
         !(migrateURI = qemuMigrationDstGetURI(info->migrateFrom,
                                               info->migrateFd)))
@@ -758,6 +747,17 @@  testInfoClear(struct testInfo *info)
     virObjectUnref(info->qemuCaps);
 }
 
+static int
+testInfoSetPaths(struct testInfo *info, const char *suffix)
+{
+    if (virAsprintf(&info->infile, "%s/qemuxml2argvdata/%s.xml",
+                    abs_srcdir, info->name) < 0 ||
+        virAsprintf(&info->outfile, "%s/qemuxml2argvdata/%s%s.args",
+                    abs_srcdir, info->name, suffix ? suffix : "") < 0)
+        return -1;
+    return 0;
+}
+
 # define FAKEROOTDIRTEMPLATE abs_builddir "/fakerootdir-XXXXXX"
 
 static int
@@ -883,11 +883,12 @@  mymain(void)
     do { \
         static struct testInfo info = { \
             .name = _name, \
-            .suffix = _suffix, \
         }; \
         if (testInfoSetArgs(&info, capslatest, \
                             __VA_ARGS__, ARG_END) < 0) \
             return EXIT_FAILURE; \
+        if (testInfoSetPaths(&info, _suffix)) \
+            return EXIT_FAILURE; \
         if (virTestRun("QEMU XML-2-ARGV " _name _suffix, \
                        testCompareXMLToArgv, &info) < 0) \
             ret = -1; \