Re: [Qemu-devel] [PATCH 03/21] qmp-test: New, covering basic QMP protocol

[Eric Blake]

On 02/23/2017 03:44 PM, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
>  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 <address@hidden>

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

signature.asc
Description: OpenPGP digital signature