Message ID | 1487886317-27400-4-git-send-email-armbru@redhat.com |
---|---|
State | New |
Headers | show |
On 02/23/2017 03:44 PM, Markus Armbruster wrote: > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > tests/Makefile.include | 5 +- > tests/libqtest.c | 17 ++++-- > tests/libqtest.h | 8 +++ > tests/qmp-test.c | 139 +++++++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 163 insertions(+), 6 deletions(-) > create mode 100644 tests/qmp-test.c > > +++ b/tests/libqtest.h > @@ -32,6 +32,14 @@ extern QTestState *global_qtest; > QTestState *qtest_init(const char *extra_args); > > /** > + * qtest_init: Wrong name (too much copy-and-paste) > + * @extra_args: other arguments to pass to QEMU. > + * > + * Returns: #QTestState instance. > + */ > +QTestState *qtest_init_without_qmp_handshake(const char *extra_args); > + > +/** > * qtest_quit: > * @s: #QTestState instance to operate on. > * > diff --git a/tests/qmp-test.c b/tests/qmp-test.c > new file mode 100644 > index 0000000..405e49e > --- /dev/null > +++ b/tests/qmp-test.c > + > +static void test_malformed(void) > +{ > + QDict *resp; > + > + /* Not even a dictionary */ > + resp = qmp("null"); > + g_assert_cmpstr(get_error_class(resp), ==, "GenericError"); > + QDECREF(resp); > + Shall we test an array, integer, and/or boolean literal as well? > + /* No "execute" key */ > + resp = qmp("{}"); > + g_assert_cmpstr(get_error_class(resp), ==, "GenericError"); > + QDECREF(resp); > + > + /* "execute" isn't a string */ > + resp = qmp("{ 'execute': true }"); > + g_assert_cmpstr(get_error_class(resp), ==, "GenericError"); > + QDECREF(resp); > + > + /* "arguments" isn't a dictionary */ > + resp = qmp("{ 'execute': 'no-such-cmd', 'arguments': [] }"); > + g_assert_cmpstr(get_error_class(resp), ==, "GenericError"); > + QDECREF(resp); > + > + /* extra key */ > + resp = qmp("{ 'execute': 'no-such-cmd', 'extra': true }"); > + g_assert_cmpstr(get_error_class(resp), ==, "GenericError"); > + QDECREF(resp); Worth testing "{ 'arguments': {} }"? > +} > + > +static void test_qmp_protocol(void) > +{ > + QDict *resp, *q, *ret; > + QList *capabilities; > + > + global_qtest = qtest_init_without_qmp_handshake(common_args); > + > + /* Test greeting */ > + resp = qmp_receive(); > + q = qdict_get_qdict(resp, "QMP"); > + g_assert(q); > + test_version(qdict_get(q, "version")); > + capabilities = qdict_get_qlist(q, "capabilities"); > + g_assert(capabilities && qlist_empty(capabilities)); > + QDECREF(resp); > + > + /* Test valid command before handshake */ > + resp = qmp("{ 'execute': 'query-version' }"); > + g_assert_cmpstr(get_error_class(resp), ==, "CommandNotFound"); > + QDECREF(resp); > + > + /* Test malformed commands before handshake */ > + test_malformed(); > + > + /* Test handshake */ > + resp = qmp("{ 'execute': 'qmp_capabilities' }"); > + ret = qdict_get_qdict(resp, "return"); > + g_assert(ret && !qdict_size(ret)); > + QDECREF(resp); > + > + /* Test repeated handshake */ > + resp = qmp("{ 'execute': 'qmp_capabilities' }"); > + g_assert_cmpstr(get_error_class(resp), ==, "CommandNotFound"); > + QDECREF(resp); > + > + /* Test valid command */ > + resp = qmp("{ 'execute': 'query-version' }"); > + test_version(qdict_get(resp, "return")); > + QDECREF(resp); > + > + /* Test malformed commands */ > + test_malformed(); > + > + /* Test 'id' */ > + resp = qmp("{ 'execute': 'query-name', 'id': 'cookie#1' }"); > + ret = qdict_get_qdict(resp, "return"); > + g_assert(ret); > + g_assert_cmpstr(qdict_get_try_str(resp, "id"), ==, "cookie#1"); > + QDECREF(resp); > + > + /* Test command failure with 'id' */ > + resp = qmp("{ 'execute': 'human-monitor-command', 'id': 2 }"); > + g_assert_cmpstr(get_error_class(resp), ==, "GenericError"); > + g_assert_cmpint(qdict_get_int(resp, "id"), ==, 2); > + QDECREF(resp); id can be _any JSON_ (at one point, QMP crashed if id included a null literal); so maybe we should have more id tests such as "'id': null,", "'id': [ { }, true ]" But any tests at all is better than our previous state of none, so even if you don't add tests, but just fix the typo: Reviewed-by: Eric Blake <eblake@redhat.com>
Eric Blake <eblake@redhat.com> writes: > On 02/23/2017 03:44 PM, Markus Armbruster wrote: >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> --- >> tests/Makefile.include | 5 +- >> tests/libqtest.c | 17 ++++-- >> tests/libqtest.h | 8 +++ >> tests/qmp-test.c | 139 +++++++++++++++++++++++++++++++++++++++++++++++++ >> 4 files changed, 163 insertions(+), 6 deletions(-) >> create mode 100644 tests/qmp-test.c >> > >> +++ b/tests/libqtest.h >> @@ -32,6 +32,14 @@ extern QTestState *global_qtest; >> QTestState *qtest_init(const char *extra_args); >> >> /** >> + * qtest_init: > > Wrong name (too much copy-and-paste) One of the several reasons why I detest this comment style. I'll fix it, of course. >> + * @extra_args: other arguments to pass to QEMU. >> + * >> + * Returns: #QTestState instance. >> + */ >> +QTestState *qtest_init_without_qmp_handshake(const char *extra_args); >> + >> +/** >> * qtest_quit: >> * @s: #QTestState instance to operate on. >> * >> diff --git a/tests/qmp-test.c b/tests/qmp-test.c >> new file mode 100644 >> index 0000000..405e49e >> --- /dev/null >> +++ b/tests/qmp-test.c > >> + >> +static void test_malformed(void) >> +{ >> + QDict *resp; >> + >> + /* Not even a dictionary */ >> + resp = qmp("null"); >> + g_assert_cmpstr(get_error_class(resp), ==, "GenericError"); >> + QDECREF(resp); >> + > > Shall we test an array, integer, and/or boolean literal as well? Not necessary for code coverage. Wouldn't hurt, of course. >> + /* No "execute" key */ >> + resp = qmp("{}"); >> + g_assert_cmpstr(get_error_class(resp), ==, "GenericError"); >> + QDECREF(resp); >> + >> + /* "execute" isn't a string */ >> + resp = qmp("{ 'execute': true }"); >> + g_assert_cmpstr(get_error_class(resp), ==, "GenericError"); >> + QDECREF(resp); >> + >> + /* "arguments" isn't a dictionary */ >> + resp = qmp("{ 'execute': 'no-such-cmd', 'arguments': [] }"); >> + g_assert_cmpstr(get_error_class(resp), ==, "GenericError"); >> + QDECREF(resp); >> + >> + /* extra key */ >> + resp = qmp("{ 'execute': 'no-such-cmd', 'extra': true }"); >> + g_assert_cmpstr(get_error_class(resp), ==, "GenericError"); >> + QDECREF(resp); > > Worth testing "{ 'arguments': {} }"? Likewise. >> +} >> + >> +static void test_qmp_protocol(void) >> +{ >> + QDict *resp, *q, *ret; >> + QList *capabilities; >> + >> + global_qtest = qtest_init_without_qmp_handshake(common_args); >> + >> + /* Test greeting */ >> + resp = qmp_receive(); >> + q = qdict_get_qdict(resp, "QMP"); >> + g_assert(q); >> + test_version(qdict_get(q, "version")); >> + capabilities = qdict_get_qlist(q, "capabilities"); >> + g_assert(capabilities && qlist_empty(capabilities)); >> + QDECREF(resp); >> + >> + /* Test valid command before handshake */ >> + resp = qmp("{ 'execute': 'query-version' }"); >> + g_assert_cmpstr(get_error_class(resp), ==, "CommandNotFound"); >> + QDECREF(resp); >> + >> + /* Test malformed commands before handshake */ >> + test_malformed(); >> + >> + /* Test handshake */ >> + resp = qmp("{ 'execute': 'qmp_capabilities' }"); >> + ret = qdict_get_qdict(resp, "return"); >> + g_assert(ret && !qdict_size(ret)); >> + QDECREF(resp); >> + >> + /* Test repeated handshake */ >> + resp = qmp("{ 'execute': 'qmp_capabilities' }"); >> + g_assert_cmpstr(get_error_class(resp), ==, "CommandNotFound"); >> + QDECREF(resp); >> + >> + /* Test valid command */ >> + resp = qmp("{ 'execute': 'query-version' }"); >> + test_version(qdict_get(resp, "return")); >> + QDECREF(resp); >> + >> + /* Test malformed commands */ >> + test_malformed(); >> + >> + /* Test 'id' */ >> + resp = qmp("{ 'execute': 'query-name', 'id': 'cookie#1' }"); >> + ret = qdict_get_qdict(resp, "return"); >> + g_assert(ret); >> + g_assert_cmpstr(qdict_get_try_str(resp, "id"), ==, "cookie#1"); >> + QDECREF(resp); >> + >> + /* Test command failure with 'id' */ >> + resp = qmp("{ 'execute': 'human-monitor-command', 'id': 2 }"); >> + g_assert_cmpstr(get_error_class(resp), ==, "GenericError"); >> + g_assert_cmpint(qdict_get_int(resp, "id"), ==, 2); >> + QDECREF(resp); > > id can be _any JSON_ (at one point, QMP crashed if id included a null > literal); so maybe we should have more id tests such as "'id': null,", > "'id': [ { }, true ]" > > But any tests at all is better than our previous state of none, so even > if you don't add tests, but just fix the typo: > > Reviewed-by: Eric Blake <eblake@redhat.com> I'm in a bit of a time squeeze, so I'll take your offer to just fix the typo for now. Thanks!
diff --git a/tests/Makefile.include b/tests/Makefile.include index e60bb6c..b212150 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -133,7 +133,9 @@ check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh # All QTests for now are POSIX-only, but the dependencies are # really in libqtest, not in the testcases themselves. -check-qtest-generic-y = tests/device-introspect-test$(EXESUF) +check-qtest-generic-y = tests/qmp-test$(EXESUF) +gcov-files-generic-y = monitor.c qapi/qmp-dispatch.c +check-qtest-generic-y += tests/device-introspect-test$(EXESUF) gcov-files-generic-y = qdev-monitor.c qmp.c gcov-files-ipack-y += hw/ipack/ipack.c @@ -654,6 +656,7 @@ libqos-imx-obj-y = $(libqos-obj-y) tests/libqos/i2c-imx.o libqos-usb-obj-y = $(libqos-spapr-obj-y) $(libqos-pc-obj-y) tests/libqos/usb.o libqos-virtio-obj-y = $(libqos-spapr-obj-y) $(libqos-pc-obj-y) tests/libqos/virtio.o tests/libqos/virtio-pci.o tests/libqos/virtio-mmio.o tests/libqos/malloc-generic.o +tests/qmp-test$(EXESUF): tests/qmp-test.o tests/device-introspect-test$(EXESUF): tests/device-introspect-test.o tests/rtc-test$(EXESUF): tests/rtc-test.o tests/m48t59-test$(EXESUF): tests/m48t59-test.o diff --git a/tests/libqtest.c b/tests/libqtest.c index ad23ce9..cf27afc 100644 --- a/tests/libqtest.c +++ b/tests/libqtest.c @@ -149,7 +149,7 @@ void qtest_add_abrt_handler(GHookFunc fn, const void *data) g_hook_prepend(&abrt_hooks, hook); } -QTestState *qtest_init(const char *extra_args) +QTestState *qtest_init_without_qmp_handshake(const char *extra_args) { QTestState *s; int sock, qmpsock, i; @@ -204,10 +204,6 @@ QTestState *qtest_init(const char *extra_args) s->irq_level[i] = false; } - /* Read the QMP greeting and then do the handshake */ - qtest_qmp_discard_response(s, ""); - qtest_qmp_discard_response(s, "{ 'execute': 'qmp_capabilities' }"); - if (getenv("QTEST_STOP")) { kill(s->qemu_pid, SIGSTOP); } @@ -219,6 +215,17 @@ QTestState *qtest_init(const char *extra_args) return s; } +QTestState *qtest_init(const char *extra_args) +{ + QTestState *s = qtest_init_without_qmp_handshake(extra_args); + + /* Read the QMP greeting and then do the handshake */ + qtest_qmp_discard_response(s, ""); + qtest_qmp_discard_response(s, "{ 'execute': 'qmp_capabilities' }"); + + return s; +} + void qtest_quit(QTestState *s) { qtest_instances = g_list_remove(qtest_instances, s); diff --git a/tests/libqtest.h b/tests/libqtest.h index 90f182e..23bd76c 100644 --- a/tests/libqtest.h +++ b/tests/libqtest.h @@ -32,6 +32,14 @@ extern QTestState *global_qtest; QTestState *qtest_init(const char *extra_args); /** + * qtest_init: + * @extra_args: other arguments to pass to QEMU. + * + * Returns: #QTestState instance. + */ +QTestState *qtest_init_without_qmp_handshake(const char *extra_args); + +/** * qtest_quit: * @s: #QTestState instance to operate on. * diff --git a/tests/qmp-test.c b/tests/qmp-test.c new file mode 100644 index 0000000..405e49e --- /dev/null +++ b/tests/qmp-test.c @@ -0,0 +1,139 @@ +/* + * QMP protocol test cases + * + * Copyright (c) 2017 Red Hat Inc. + * + * Authors: + * Markus Armbruster <armbru@redhat.com>, + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +#include "qemu/osdep.h" +#include "libqtest.h" +#include "qapi-visit.h" +#include "qapi/error.h" +#include "qapi/qobject-input-visitor.h" +#include "qapi/visitor.h" + +const char common_args[] = "-nodefaults -machine none"; + +static const char *get_error_class(QDict *resp) +{ + QDict *error = qdict_get_qdict(resp, "error"); + const char *desc = qdict_get_try_str(error, "desc"); + + g_assert(desc); + return error ? qdict_get_try_str(error, "class") : NULL; +} + +static void test_version(QObject *version) +{ + Visitor *v; + VersionInfo *vinfo; + + g_assert(version); + v = qobject_input_visitor_new(version, true); + visit_type_VersionInfo(v, "version", &vinfo, &error_abort); + qapi_free_VersionInfo(vinfo); + visit_free(v); +} + +static void test_malformed(void) +{ + QDict *resp; + + /* Not even a dictionary */ + resp = qmp("null"); + g_assert_cmpstr(get_error_class(resp), ==, "GenericError"); + QDECREF(resp); + + /* No "execute" key */ + resp = qmp("{}"); + g_assert_cmpstr(get_error_class(resp), ==, "GenericError"); + QDECREF(resp); + + /* "execute" isn't a string */ + resp = qmp("{ 'execute': true }"); + g_assert_cmpstr(get_error_class(resp), ==, "GenericError"); + QDECREF(resp); + + /* "arguments" isn't a dictionary */ + resp = qmp("{ 'execute': 'no-such-cmd', 'arguments': [] }"); + g_assert_cmpstr(get_error_class(resp), ==, "GenericError"); + QDECREF(resp); + + /* extra key */ + resp = qmp("{ 'execute': 'no-such-cmd', 'extra': true }"); + g_assert_cmpstr(get_error_class(resp), ==, "GenericError"); + QDECREF(resp); +} + +static void test_qmp_protocol(void) +{ + QDict *resp, *q, *ret; + QList *capabilities; + + global_qtest = qtest_init_without_qmp_handshake(common_args); + + /* Test greeting */ + resp = qmp_receive(); + q = qdict_get_qdict(resp, "QMP"); + g_assert(q); + test_version(qdict_get(q, "version")); + capabilities = qdict_get_qlist(q, "capabilities"); + g_assert(capabilities && qlist_empty(capabilities)); + QDECREF(resp); + + /* Test valid command before handshake */ + resp = qmp("{ 'execute': 'query-version' }"); + g_assert_cmpstr(get_error_class(resp), ==, "CommandNotFound"); + QDECREF(resp); + + /* Test malformed commands before handshake */ + test_malformed(); + + /* Test handshake */ + resp = qmp("{ 'execute': 'qmp_capabilities' }"); + ret = qdict_get_qdict(resp, "return"); + g_assert(ret && !qdict_size(ret)); + QDECREF(resp); + + /* Test repeated handshake */ + resp = qmp("{ 'execute': 'qmp_capabilities' }"); + g_assert_cmpstr(get_error_class(resp), ==, "CommandNotFound"); + QDECREF(resp); + + /* Test valid command */ + resp = qmp("{ 'execute': 'query-version' }"); + test_version(qdict_get(resp, "return")); + QDECREF(resp); + + /* Test malformed commands */ + test_malformed(); + + /* Test 'id' */ + resp = qmp("{ 'execute': 'query-name', 'id': 'cookie#1' }"); + ret = qdict_get_qdict(resp, "return"); + g_assert(ret); + g_assert_cmpstr(qdict_get_try_str(resp, "id"), ==, "cookie#1"); + QDECREF(resp); + + /* Test command failure with 'id' */ + resp = qmp("{ 'execute': 'human-monitor-command', 'id': 2 }"); + g_assert_cmpstr(get_error_class(resp), ==, "GenericError"); + g_assert_cmpint(qdict_get_int(resp, "id"), ==, 2); + QDECREF(resp); + + qtest_end(); +} + +int main(int argc, char *argv[]) +{ + g_test_init(&argc, &argv, NULL); + + qtest_add_func("qmp/protocol", test_qmp_protocol); + + return g_test_run(); +}
Signed-off-by: Markus Armbruster <armbru@redhat.com> --- tests/Makefile.include | 5 +- tests/libqtest.c | 17 ++++-- tests/libqtest.h | 8 +++ tests/qmp-test.c | 139 +++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 163 insertions(+), 6 deletions(-) create mode 100644 tests/qmp-test.c