[Qemu-devel] [PATCH v2 0/2] qdev-monitor-test: Simplify & make more robust

[Qemu-devel] [PATCH v2 0/2] qdev-monitor-test: Simplify & make more robust

[Markus Armbruster]

Stefan's patch has been rotting on the list since December.  Reposting
it together with a patch for a robustness issue I spotted when
reviewing it.

Markus Armbruster (1):
  qdev-monitor-test: Don't test human-readable error message

Stefan Hajnoczi (1):
  qdev-monitor-test: simplify using g_assert_cmpstr()

 tests/qdev-monitor-test.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

-- 
1.8.1.4

[Qemu-devel] [PATCH v2 1/2] qdev-monitor-test: simplify using g_assert_cmpstr()

[Markus Armbruster]

From: Stefan Hajnoczi <stefanha@redhat.com>

Use g_assert_cmpstr() instead of combining g_assert() and strcmp(3).
This simplifies the code since we no longer have to play games to
distinguish NULL from "" using "(null)".

gcc extension haters will also be happy that ?: was dropped.

Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 tests/qdev-monitor-test.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/qdev-monitor-test.c b/tests/qdev-monitor-test.c
index ba7f9cc..eefaab8 100644
--- a/tests/qdev-monitor-test.c
+++ b/tests/qdev-monitor-test.c
@@ -32,8 +32,9 @@ static void test_device_add(void)
                    "}}");
     g_assert(response);
     error = qdict_get_qdict(response, "error");
-    g_assert(!strcmp(qdict_get_try_str(error, "desc") ?: "",
-                     "Device needs media, but drive is empty"));
+    g_assert_cmpstr(qdict_get_try_str(error, "desc"),
+                    ==,
+                    "Device needs media, but drive is empty");
     QDECREF(response);
 
     /* Delete the drive */
@@ -42,7 +43,7 @@ static void test_device_add(void)
                    "   \"command-line\": \"drive_del drive0\""
                    "}}");
     g_assert(response);
-    g_assert(!strcmp(qdict_get_try_str(response, "return") ?: "(null)", ""));
+    g_assert_cmpstr(qdict_get_try_str(response, "return"), ==, "");
     QDECREF(response);
 
     /* Try to re-add the drive.  This fails with duplicate IDs if a leaked
@@ -53,8 +54,7 @@ static void test_device_add(void)
                    "   \"command-line\": \"drive_add pci-addr=auto if=none,id=drive0\""
                    "}}");
     g_assert(response);
-    g_assert(!strcmp(qdict_get_try_str(response, "return") ?: "",
-                     "OK\r\n"));
+    g_assert_cmpstr(qdict_get_try_str(response, "return"), ==, "OK\r\n");
     QDECREF(response);
 
     qtest_end();
-- 
1.8.1.4

[Qemu-devel] [PATCH v2 2/2] qdev-monitor-test: Don't test human-readable error message

[Markus Armbruster]

Test the error class instead.  Expecting a specific message is
fragile.  In fact, it broke once already, in commit 75884af.  Restore
the test of error member "class" dropped there, and drop the test of
error member "desc".

There are no other tests of "desc" as far as I can tell.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 tests/qdev-monitor-test.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tests/qdev-monitor-test.c b/tests/qdev-monitor-test.c
index eefaab8..e20ffd6 100644
--- a/tests/qdev-monitor-test.c
+++ b/tests/qdev-monitor-test.c
@@ -32,9 +32,7 @@ static void test_device_add(void)
                    "}}");
     g_assert(response);
     error = qdict_get_qdict(response, "error");
-    g_assert_cmpstr(qdict_get_try_str(error, "desc"),
-                    ==,
-                    "Device needs media, but drive is empty");
+    g_assert_cmpstr(qdict_get_try_str(error, "class"), ==, "GenericError");
     QDECREF(response);
 
     /* Delete the drive */
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH v2 0/2] qdev-monitor-test: Simplify & make more robust

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 657 bytes --]

On 03/06/2014 02:12 AM, Markus Armbruster wrote:
> Stefan's patch has been rotting on the list since December.  Reposting
> it together with a patch for a robustness issue I spotted when
> reviewing it.

Series: Reviewed-by: Eric Blake <eblake@redhat.com>

> 
> Markus Armbruster (1):
>   qdev-monitor-test: Don't test human-readable error message
> 
> Stefan Hajnoczi (1):
>   qdev-monitor-test: simplify using g_assert_cmpstr()
> 
>  tests/qdev-monitor-test.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH v2 0/2] qdev-monitor-test: Simplify & make more robust

[Stefan Hajnoczi]

On Thu, Mar 06, 2014 at 10:12:51AM +0100, Markus Armbruster wrote:
> Stefan's patch has been rotting on the list since December.  Reposting
> it together with a patch for a robustness issue I spotted when
> reviewing it.
> 
> Markus Armbruster (1):
>   qdev-monitor-test: Don't test human-readable error message
> 
> Stefan Hajnoczi (1):
>   qdev-monitor-test: simplify using g_assert_cmpstr()
> 
>  tests/qdev-monitor-test.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Re: [Qemu-devel] [PATCH v2 0/2] qdev-monitor-test: Simplify & make more robust

[Andreas Färber]

Am 06.03.2014 10:12, schrieb Markus Armbruster:
> Stefan's patch has been rotting on the list since December.  Reposting
> it together with a patch for a robustness issue I spotted when
> reviewing it.

Stefan had applied qdev-monitor-test through his block tree, and I was
on vacation for most of December and would've needed a ping or two in
the new year. qdev-monitor itself seems to be gray territory - I've been
taking care of QOM conversion issues but otherwise don't feel
responsible for the HMP/QMP interface and would expect the maintainer
that does feel responsible to handle the matching test cases.

> Markus Armbruster (1):
>   qdev-monitor-test: Don't test human-readable error message
> 
> Stefan Hajnoczi (1):
>   qdev-monitor-test: simplify using g_assert_cmpstr()
> 
>  tests/qdev-monitor-test.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)

Thanks, applied to qom-next:
https://github.com/afaerber/qemu-cpu/commits/qom-next

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH v2 0/2] qdev-monitor-test: Simplify & make more robust

[Markus Armbruster]

Andreas Färber <afaerber@suse.de> writes:

> Am 06.03.2014 10:12, schrieb Markus Armbruster:
>> Stefan's patch has been rotting on the list since December.  Reposting
>> it together with a patch for a robustness issue I spotted when
>> reviewing it.
>
> Stefan had applied qdev-monitor-test through his block tree, and I was
> on vacation for most of December and would've needed a ping or two in
> the new year. qdev-monitor itself seems to be gray territory - I've been
> taking care of QOM conversion issues but otherwise don't feel
> responsible for the HMP/QMP interface and would expect the maintainer
> that does feel responsible to handle the matching test cases.

The test has a finger each in the block, QMP and qdev pie.  It could've
gone through any of these trees.  It got lost around Christmas.
Happens, no big deal :)

>> Markus Armbruster (1):
>>   qdev-monitor-test: Don't test human-readable error message
>> 
>> Stefan Hajnoczi (1):
>>   qdev-monitor-test: simplify using g_assert_cmpstr()
>> 
>>  tests/qdev-monitor-test.c | 8 +++-----
>>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> Thanks, applied to qom-next:
> https://github.com/afaerber/qemu-cpu/commits/qom-next

Thanks!