* [Qemu-devel] [PATCH 0/3] tests: trivial enhancements for OOB
@ 2018-03-21 6:55 Peter Xu
2018-03-21 6:55 ` [Qemu-devel] [PATCH 1/3] tests: let qapi-schema tests detect oob Peter Xu
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Peter Xu @ 2018-03-21 6:55 UTC (permalink / raw)
To: qemu-devel; +Cc: Eric Blake, mdroth, peterx, Markus Armbruster
First two patches add OOB detection for current qapi-schema tests
(which I missed in the OOB series but pointed out by Eric Blake). The
3rd patch addressed one suggestion from Eric too here:
http://lists.gnu.org/archive/html/qemu-devel/2018-03/msg03177.html
I tried to batch the commands in a single string buffer but it's not
that easy - because currently qtest_async_qmp() (and finally,
qmp_fd_sendv()) does not really support multiple qobjects in a single
command buffer. Let's put that aside. After all even calling
qtest_async_qmp() many times would be really fast, since we are
basically filling things to the write buffer very quickly (I believe
that's much faster than the IO really flushed to the receiver side).
But, adding the "id" field and check that would be far easier, that's
what I did in that last patch.
It's fine even for 2.12, but I'll let people decide.
Please review, thanks.
Peter Xu (3):
tests: let qapi-schema tests detect oob
tests: add oob-test for qapi-schema
tests: more strict command batching test
tests/Makefile.include | 1 +
tests/qapi-schema/doc-good.out | 4 ++--
tests/qapi-schema/ident-with-escape.out | 2 +-
tests/qapi-schema/indented-expr.out | 4 ++--
tests/qapi-schema/oob-test.err | 0
tests/qapi-schema/oob-test.exit | 1 +
tests/qapi-schema/oob-test.json | 2 ++
tests/qapi-schema/oob-test.out | 6 ++++++
tests/qapi-schema/qapi-schema-test.out | 18 +++++++++---------
tests/qapi-schema/test-qapi.py | 4 ++--
tests/qmp-test.c | 8 +++++++-
11 files changed, 33 insertions(+), 17 deletions(-)
create mode 100644 tests/qapi-schema/oob-test.err
create mode 100644 tests/qapi-schema/oob-test.exit
create mode 100644 tests/qapi-schema/oob-test.json
create mode 100644 tests/qapi-schema/oob-test.out
--
2.14.3
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 1/3] tests: let qapi-schema tests detect oob
2018-03-21 6:55 [Qemu-devel] [PATCH 0/3] tests: trivial enhancements for OOB Peter Xu
@ 2018-03-21 6:55 ` Peter Xu
2018-03-21 12:47 ` Eric Blake
2018-03-21 6:55 ` [Qemu-devel] [PATCH 2/3] tests: add oob-test for qapi-schema Peter Xu
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Peter Xu @ 2018-03-21 6:55 UTC (permalink / raw)
To: qemu-devel; +Cc: Eric Blake, mdroth, peterx, Markus Armbruster
The allow_oob parameter was passed in but not used in tests. Now
reflect that in the tests, so we need to touch up other command testers
with that new change.
Signed-off-by: Peter Xu <peterx@redhat.com>
---
tests/qapi-schema/doc-good.out | 4 ++--
tests/qapi-schema/ident-with-escape.out | 2 +-
tests/qapi-schema/indented-expr.out | 4 ++--
tests/qapi-schema/qapi-schema-test.out | 18 +++++++++---------
tests/qapi-schema/test-qapi.py | 4 ++--
5 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
index 430b5a87db..63058b1590 100644
--- a/tests/qapi-schema/doc-good.out
+++ b/tests/qapi-schema/doc-good.out
@@ -28,9 +28,9 @@ object q_obj_cmd-arg
member arg2: str optional=True
member arg3: bool optional=False
command cmd q_obj_cmd-arg -> Object
- gen=True success_response=True boxed=False
+ gen=True success_response=True boxed=False oob=False
command cmd-boxed Object -> None
- gen=True success_response=True boxed=True
+ gen=True success_response=True boxed=True oob=False
doc freeform
body=
= Section
diff --git a/tests/qapi-schema/ident-with-escape.out b/tests/qapi-schema/ident-with-escape.out
index ee3b34e623..82213aa51d 100644
--- a/tests/qapi-schema/ident-with-escape.out
+++ b/tests/qapi-schema/ident-with-escape.out
@@ -5,4 +5,4 @@ module ident-with-escape.json
object q_obj_fooA-arg
member bar1: str optional=False
command fooA q_obj_fooA-arg -> None
- gen=True success_response=True boxed=False
+ gen=True success_response=True boxed=False oob=False
diff --git a/tests/qapi-schema/indented-expr.out b/tests/qapi-schema/indented-expr.out
index a79935e8c3..862678f8f4 100644
--- a/tests/qapi-schema/indented-expr.out
+++ b/tests/qapi-schema/indented-expr.out
@@ -3,6 +3,6 @@ enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool']
prefix QTYPE
module indented-expr.json
command eins None -> None
- gen=True success_response=True boxed=False
+ gen=True success_response=True boxed=False oob=False
command zwei None -> None
- gen=True success_response=True boxed=False
+ gen=True success_response=True boxed=False oob=False
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 012e7fc06a..4f43370017 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -16,7 +16,7 @@ object Empty1
object Empty2
base Empty1
command user_def_cmd0 Empty2 -> Empty2
- gen=True success_response=True boxed=False
+ gen=True success_response=True boxed=False oob=False
enum QEnumTwo ['value1', 'value2']
prefix QENUM_TWO
object UserDefOne
@@ -143,29 +143,29 @@ object UserDefNativeListUnion
case sizes: q_obj_sizeList-wrapper
case any: q_obj_anyList-wrapper
command user_def_cmd None -> None
- gen=True success_response=True boxed=False
+ gen=True success_response=True boxed=False oob=False
object q_obj_user_def_cmd1-arg
member ud1a: UserDefOne optional=False
command user_def_cmd1 q_obj_user_def_cmd1-arg -> None
- gen=True success_response=True boxed=False
+ gen=True success_response=True boxed=False oob=False
object q_obj_user_def_cmd2-arg
member ud1a: UserDefOne optional=False
member ud1b: UserDefOne optional=True
command user_def_cmd2 q_obj_user_def_cmd2-arg -> UserDefTwo
- gen=True success_response=True boxed=False
+ gen=True success_response=True boxed=False oob=False
object q_obj_guest-get-time-arg
member a: int optional=False
member b: int optional=True
command guest-get-time q_obj_guest-get-time-arg -> int
- gen=True success_response=True boxed=False
+ gen=True success_response=True boxed=False oob=False
object q_obj_guest-sync-arg
member arg: any optional=False
command guest-sync q_obj_guest-sync-arg -> any
- gen=True success_response=True boxed=False
+ gen=True success_response=True boxed=False oob=False
command boxed-struct UserDefZero -> None
- gen=True success_response=True boxed=True
+ gen=True success_response=True boxed=True oob=False
command boxed-union UserDefNativeListUnion -> None
- gen=True success_response=True boxed=True
+ gen=True success_response=True boxed=True oob=False
object UserDefOptions
member i64: intList optional=True
member u64: uint64List optional=True
@@ -229,4 +229,4 @@ object q_obj___org.qemu_x-command-arg
member c: __org.qemu_x-Union2 optional=False
member d: __org.qemu_x-Alt optional=False
command __org.qemu_x-command q_obj___org.qemu_x-command-arg -> __org.qemu_x-Union1
- gen=True success_response=True boxed=False
+ gen=True success_response=True boxed=False oob=False
diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index 10e68b01d9..c1a144ba29 100644
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -45,8 +45,8 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor):
gen, success_response, boxed, allow_oob):
print('command %s %s -> %s' % \
(name, arg_type and arg_type.name, ret_type and ret_type.name))
- print(' gen=%s success_response=%s boxed=%s' % \
- (gen, success_response, boxed))
+ print(' gen=%s success_response=%s boxed=%s oob=%s' % \
+ (gen, success_response, boxed, allow_oob))
def visit_event(self, name, info, arg_type, boxed):
print('event %s %s' % (name, arg_type and arg_type.name))
--
2.14.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 2/3] tests: add oob-test for qapi-schema
2018-03-21 6:55 [Qemu-devel] [PATCH 0/3] tests: trivial enhancements for OOB Peter Xu
2018-03-21 6:55 ` [Qemu-devel] [PATCH 1/3] tests: let qapi-schema tests detect oob Peter Xu
@ 2018-03-21 6:55 ` Peter Xu
2018-03-21 12:52 ` Eric Blake
2018-03-21 6:55 ` [Qemu-devel] [PATCH 3/3] tests: more strict command batching test Peter Xu
2018-03-21 12:45 ` [Qemu-devel] [PATCH 0/3] tests: trivial enhancements for OOB Eric Blake
3 siblings, 1 reply; 10+ messages in thread
From: Peter Xu @ 2018-03-21 6:55 UTC (permalink / raw)
To: qemu-devel; +Cc: Eric Blake, mdroth, peterx, Markus Armbruster
It simply tests the new OOB capability, and make sure the QAPISchema can
parse it correctly.
Signed-off-by: Peter Xu <peterx@redhat.com>
---
tests/Makefile.include | 1 +
tests/qapi-schema/oob-test.err | 0
tests/qapi-schema/oob-test.exit | 1 +
tests/qapi-schema/oob-test.json | 2 ++
tests/qapi-schema/oob-test.out | 6 ++++++
5 files changed, 10 insertions(+)
create mode 100644 tests/qapi-schema/oob-test.err
create mode 100644 tests/qapi-schema/oob-test.exit
create mode 100644 tests/qapi-schema/oob-test.json
create mode 100644 tests/qapi-schema/oob-test.out
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 0b277036df..059523e2d1 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -523,6 +523,7 @@ qapi-schema += missing-comma-object.json
qapi-schema += missing-type.json
qapi-schema += nested-struct-data.json
qapi-schema += non-objects.json
+qapi-schema += oob-test.json
qapi-schema += pragma-doc-required-crap.json
qapi-schema += pragma-extra-junk.json
qapi-schema += pragma-name-case-whitelist-crap.json
diff --git a/tests/qapi-schema/oob-test.err b/tests/qapi-schema/oob-test.err
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/oob-test.exit b/tests/qapi-schema/oob-test.exit
new file mode 100644
index 0000000000..573541ac97
--- /dev/null
+++ b/tests/qapi-schema/oob-test.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/oob-test.json b/tests/qapi-schema/oob-test.json
new file mode 100644
index 0000000000..096c34b2fb
--- /dev/null
+++ b/tests/qapi-schema/oob-test.json
@@ -0,0 +1,2 @@
+# Some Out-Of-Band specific tests
+{ 'command': 'an-oob-command', 'allow-oob': true }
diff --git a/tests/qapi-schema/oob-test.out b/tests/qapi-schema/oob-test.out
new file mode 100644
index 0000000000..ce5a130dcf
--- /dev/null
+++ b/tests/qapi-schema/oob-test.out
@@ -0,0 +1,6 @@
+object q_empty
+enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool']
+ prefix QTYPE
+module oob-test.json
+command an-oob-command None -> None
+ gen=True success_response=True boxed=False oob=True
--
2.14.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 3/3] tests: more strict command batching test
2018-03-21 6:55 [Qemu-devel] [PATCH 0/3] tests: trivial enhancements for OOB Peter Xu
2018-03-21 6:55 ` [Qemu-devel] [PATCH 1/3] tests: let qapi-schema tests detect oob Peter Xu
2018-03-21 6:55 ` [Qemu-devel] [PATCH 2/3] tests: add oob-test for qapi-schema Peter Xu
@ 2018-03-21 6:55 ` Peter Xu
2018-03-21 12:55 ` Eric Blake
2018-03-21 12:45 ` [Qemu-devel] [PATCH 0/3] tests: trivial enhancements for OOB Eric Blake
3 siblings, 1 reply; 10+ messages in thread
From: Peter Xu @ 2018-03-21 6:55 UTC (permalink / raw)
To: qemu-devel; +Cc: Eric Blake, mdroth, peterx, Markus Armbruster
Add "id" fields to the commands, and check that the command returns are
in order.
Signed-off-by: Peter Xu <peterx@redhat.com>
---
tests/qmp-test.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/tests/qmp-test.c b/tests/qmp-test.c
index 07c0b87e27..c861c3b550 100644
--- a/tests/qmp-test.c
+++ b/tests/qmp-test.c
@@ -83,6 +83,7 @@ static void test_qmp_protocol(void)
const QListEntry *entry;
QString *qstr;
int i;
+ char buf[128];
qts = qtest_init_without_qmp_handshake(common_args);
@@ -150,7 +151,10 @@ static void test_qmp_protocol(void)
* best-effort test.
*/
for (i = 0; i < 16; i++) {
- qtest_async_qmp(qts, "{ 'execute': 'query-version' }");
+ snprintf(buf, sizeof(buf) - 1, "{ 'execute': 'query-version', "
+ "'id': %d }", i);
+ buf[sizeof(buf) - 1] = '\0';
+ qtest_async_qmp(qts, buf);
}
/* Verify the replies to make sure no command is dropped. */
for (i = 0; i < 16; i++) {
@@ -158,6 +162,8 @@ static void test_qmp_protocol(void)
/* It should never be dropped. Each of them should be a reply. */
g_assert(qdict_haskey(resp, "return"));
g_assert(!qdict_haskey(resp, "event"));
+ g_assert(qdict_haskey(resp, "id"));
+ g_assert(qdict_get_int(resp, "id") == i);
QDECREF(resp);
}
--
2.14.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 0/3] tests: trivial enhancements for OOB
2018-03-21 6:55 [Qemu-devel] [PATCH 0/3] tests: trivial enhancements for OOB Peter Xu
` (2 preceding siblings ...)
2018-03-21 6:55 ` [Qemu-devel] [PATCH 3/3] tests: more strict command batching test Peter Xu
@ 2018-03-21 12:45 ` Eric Blake
3 siblings, 0 replies; 10+ messages in thread
From: Eric Blake @ 2018-03-21 12:45 UTC (permalink / raw)
To: Peter Xu, qemu-devel; +Cc: mdroth, Markus Armbruster
On 03/21/2018 01:55 AM, Peter Xu wrote:
> First two patches add OOB detection for current qapi-schema tests
> (which I missed in the OOB series but pointed out by Eric Blake). The
> 3rd patch addressed one suggestion from Eric too here:
>
> http://lists.gnu.org/archive/html/qemu-devel/2018-03/msg03177.html
>
> I tried to batch the commands in a single string buffer but it's not
> that easy - because currently qtest_async_qmp() (and finally,
> qmp_fd_sendv()) does not really support multiple qobjects in a single
> command buffer. Let's put that aside. After all even calling
> qtest_async_qmp() many times would be really fast, since we are
> basically filling things to the write buffer very quickly (I believe
> that's much faster than the IO really flushed to the receiver side).
>
> But, adding the "id" field and check that would be far easier, that's
> what I did in that last patch.
>
> It's fine even for 2.12, but I'll let people decide.
I consider added testsuite coverage of a new feature to be a bug fix
(the feature was incomplete if the testsuite doesn't prevent regressions
in the feature) and safe for freeze (the testsuite changes don't impact
the main binary, so they can't break anything), so I'm happy to queue
this through my qapi tree for 2.12 once it is reviewed.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] tests: let qapi-schema tests detect oob
2018-03-21 6:55 ` [Qemu-devel] [PATCH 1/3] tests: let qapi-schema tests detect oob Peter Xu
@ 2018-03-21 12:47 ` Eric Blake
0 siblings, 0 replies; 10+ messages in thread
From: Eric Blake @ 2018-03-21 12:47 UTC (permalink / raw)
To: Peter Xu, qemu-devel; +Cc: mdroth, Markus Armbruster
On 03/21/2018 01:55 AM, Peter Xu wrote:
> The allow_oob parameter was passed in but not used in tests. Now
> reflect that in the tests, so we need to touch up other command testers
> with that new change.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
> tests/qapi-schema/doc-good.out | 4 ++--
> tests/qapi-schema/ident-with-escape.out | 2 +-
> tests/qapi-schema/indented-expr.out | 4 ++--
> tests/qapi-schema/qapi-schema-test.out | 18 +++++++++---------
> tests/qapi-schema/test-qapi.py | 4 ++--
> 5 files changed, 16 insertions(+), 16 deletions(-)
>
> +++ b/tests/qapi-schema/test-qapi.py
> @@ -45,8 +45,8 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor):
> gen, success_response, boxed, allow_oob):
> print('command %s %s -> %s' % \
> (name, arg_type and arg_type.name, ret_type and ret_type.name))
> - print(' gen=%s success_response=%s boxed=%s' % \
> - (gen, success_response, boxed))
> + print(' gen=%s success_response=%s boxed=%s oob=%s' % \
> + (gen, success_response, boxed, allow_oob))
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] tests: add oob-test for qapi-schema
2018-03-21 6:55 ` [Qemu-devel] [PATCH 2/3] tests: add oob-test for qapi-schema Peter Xu
@ 2018-03-21 12:52 ` Eric Blake
2018-03-22 3:43 ` Peter Xu
0 siblings, 1 reply; 10+ messages in thread
From: Eric Blake @ 2018-03-21 12:52 UTC (permalink / raw)
To: Peter Xu, qemu-devel; +Cc: mdroth, Markus Armbruster
On 03/21/2018 01:55 AM, Peter Xu wrote:
> It simply tests the new OOB capability, and make sure the QAPISchema can
> parse it correctly.
We also want negative tests that cover any new error messages in the
qapi generator (such as 'allow-oob':'bad' diagnosing a non-bool, or
'allow-oob':false giving an error message that false is already the
default such that only 'allow-oob':true makes sense).
Also, it's often easier to merge the positive test into the giant
existing qapi-schema-test.json rather than creating a new positive test.
> +++ b/tests/qapi-schema/oob-test.out
> @@ -0,0 +1,6 @@
> +object q_empty
> +enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool']
> + prefix QTYPE
> +module oob-test.json
> +command an-oob-command None -> None
> + gen=True success_response=True boxed=False oob=True
At any rate, the positive test addition is good. I may beat you to
submitting a v2 patch that covers the error messages that I'm thinking of.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] tests: more strict command batching test
2018-03-21 6:55 ` [Qemu-devel] [PATCH 3/3] tests: more strict command batching test Peter Xu
@ 2018-03-21 12:55 ` Eric Blake
2018-03-22 3:48 ` Peter Xu
0 siblings, 1 reply; 10+ messages in thread
From: Eric Blake @ 2018-03-21 12:55 UTC (permalink / raw)
To: Peter Xu, qemu-devel; +Cc: mdroth, Markus Armbruster
On 03/21/2018 01:55 AM, Peter Xu wrote:
> Add "id" fields to the commands, and check that the command returns are
> in order.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
> tests/qmp-test.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> +++ b/tests/qmp-test.c
> @@ -83,6 +83,7 @@ static void test_qmp_protocol(void)
> const QListEntry *entry;
> QString *qstr;
> int i;
> + char buf[128];
Eww, we shouldn't need this.
>
> qts = qtest_init_without_qmp_handshake(common_args);
>
> @@ -150,7 +151,10 @@ static void test_qmp_protocol(void)
> * best-effort test.
> */
> for (i = 0; i < 16; i++) {
> - qtest_async_qmp(qts, "{ 'execute': 'query-version' }");
> + snprintf(buf, sizeof(buf) - 1, "{ 'execute': 'query-version', "
> + "'id': %d }", i);
> + buf[sizeof(buf) - 1] = '\0';
> + qtest_async_qmp(qts, buf);
snprintf() of JSON strings is prone to failures especially when the JSON
string is reinterpreted as a printf argument in qtest_async_qmp. Better
is to let qtest_async_qmp() directly do the formatting, as in:
qtest_async_qmp(qts, "{ 'execute':'query-version', 'id':%d}", i);
And then I was right - you don't need buf.
Otherwise the addition is good.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] tests: add oob-test for qapi-schema
2018-03-21 12:52 ` Eric Blake
@ 2018-03-22 3:43 ` Peter Xu
0 siblings, 0 replies; 10+ messages in thread
From: Peter Xu @ 2018-03-22 3:43 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, mdroth, Markus Armbruster
On Wed, Mar 21, 2018 at 07:52:06AM -0500, Eric Blake wrote:
> On 03/21/2018 01:55 AM, Peter Xu wrote:
> > It simply tests the new OOB capability, and make sure the QAPISchema can
> > parse it correctly.
>
> We also want negative tests that cover any new error messages in the qapi
> generator (such as 'allow-oob':'bad' diagnosing a non-bool, or
> 'allow-oob':false giving an error message that false is already the default
> such that only 'allow-oob':true makes sense).
>
> Also, it's often easier to merge the positive test into the giant existing
> qapi-schema-test.json rather than creating a new positive test.
It seems that for one QAPI schema negative test only the first error
will be reported, then the script halts (so the 2nd negative test in
the same .json won't be reported). To make it simple - I'll put the
positive test into qapi-schema-test.json, and add one negative test in
oob-test.json to check again strings (though in the code I'll only
allow 'false').
(Actually I'll need one liner change to check that value when parsing
since it was not checked before...)
>
> > +++ b/tests/qapi-schema/oob-test.out
> > @@ -0,0 +1,6 @@
> > +object q_empty
> > +enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool']
> > + prefix QTYPE
> > +module oob-test.json
> > +command an-oob-command None -> None
> > + gen=True success_response=True boxed=False oob=True
>
> At any rate, the positive test addition is good. I may beat you to
> submitting a v2 patch that covers the error messages that I'm thinking of.
Will post another version (and I'll see whether I should queue more to
fix existing reported OOB problems). Thanks,
--
Peter Xu
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] tests: more strict command batching test
2018-03-21 12:55 ` Eric Blake
@ 2018-03-22 3:48 ` Peter Xu
0 siblings, 0 replies; 10+ messages in thread
From: Peter Xu @ 2018-03-22 3:48 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, mdroth, Markus Armbruster
On Wed, Mar 21, 2018 at 07:55:19AM -0500, Eric Blake wrote:
> On 03/21/2018 01:55 AM, Peter Xu wrote:
> > Add "id" fields to the commands, and check that the command returns are
> > in order.
> >
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> > tests/qmp-test.c | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
>
> > +++ b/tests/qmp-test.c
> > @@ -83,6 +83,7 @@ static void test_qmp_protocol(void)
> > const QListEntry *entry;
> > QString *qstr;
> > int i;
> > + char buf[128];
>
> Eww, we shouldn't need this.
>
> > qts = qtest_init_without_qmp_handshake(common_args);
> > @@ -150,7 +151,10 @@ static void test_qmp_protocol(void)
> > * best-effort test.
> > */
> > for (i = 0; i < 16; i++) {
> > - qtest_async_qmp(qts, "{ 'execute': 'query-version' }");
> > + snprintf(buf, sizeof(buf) - 1, "{ 'execute': 'query-version', "
> > + "'id': %d }", i);
> > + buf[sizeof(buf) - 1] = '\0';
> > + qtest_async_qmp(qts, buf);
>
> snprintf() of JSON strings is prone to failures especially when the JSON
> string is reinterpreted as a printf argument in qtest_async_qmp. Better is
> to let qtest_async_qmp() directly do the formatting, as in:
>
> qtest_async_qmp(qts, "{ 'execute':'query-version', 'id':%d}", i);
>
> And then I was right - you don't need buf.
Ouch. :)
Will fix that. Thanks,
>
> Otherwise the addition is good.
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc. +1-919-301-3266
> Virtualization: qemu.org | libvirt.org
--
Peter Xu
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-03-22 3:48 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-21 6:55 [Qemu-devel] [PATCH 0/3] tests: trivial enhancements for OOB Peter Xu
2018-03-21 6:55 ` [Qemu-devel] [PATCH 1/3] tests: let qapi-schema tests detect oob Peter Xu
2018-03-21 12:47 ` Eric Blake
2018-03-21 6:55 ` [Qemu-devel] [PATCH 2/3] tests: add oob-test for qapi-schema Peter Xu
2018-03-21 12:52 ` Eric Blake
2018-03-22 3:43 ` Peter Xu
2018-03-21 6:55 ` [Qemu-devel] [PATCH 3/3] tests: more strict command batching test Peter Xu
2018-03-21 12:55 ` Eric Blake
2018-03-22 3:48 ` Peter Xu
2018-03-21 12:45 ` [Qemu-devel] [PATCH 0/3] tests: trivial enhancements for OOB Eric Blake
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.