[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 09/12] tests: add a few qemu-qmp tests
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 09/12] tests: add a few qemu-qmp tests |
Date: |
Tue, 17 Jul 2018 10:01:55 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) |
Marc-André Lureau <address@hidden> writes:
> These 2 tests exhibited two qmp bugs that were fixed in 2.7
> (series from commit e64c75a9752c5d0fd64eb2e684c656a5ea7d03c6 to
> commit 1382d4abdf9619985e4078e37e49e487cea9935e)
>
> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
> tests/qmp-test.c | 38 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
>
> diff --git a/tests/qmp-test.c b/tests/qmp-test.c
> index ceaf4a6789..084c5edff0 100644
> --- a/tests/qmp-test.c
> +++ b/tests/qmp-test.c
> @@ -249,7 +249,39 @@ static void test_qmp_oob(void)
> recv_cmd_id(qts, "blocks-2");
> recv_cmd_id(qts, "err-2");
> cleanup_blocking_cmd();
> +}
> +
> +static void test_object_add_without_props(void)
> +{
> + QTestState *qts;
> + QDict *ret;
> +
> + qts = qtest_init(common_args);
> +
> + ret = qtest_qmp(qts, "{'execute': 'object-add',"
> + " 'arguments': { 'qom-type': 'memory-backend-ram', 'id': 'ram1' }
> }");
Please break lines between arguments instead of within. More of the
same below.
> + g_assert_nonnull(ret);
> +
> + g_assert_cmpstr(get_error_class(ret), ==, "GenericError");
> +
> + qobject_unref(ret);
> + qtest_quit(qts);
> +}
> +
> +static void test_qom_set_without_value(void)
> +{
> + QTestState *qts;
> + QDict *ret;
> +
> + qts = qtest_init(common_args);
>
> + ret = qtest_qmp(qts, "{'execute': 'qom-set',"
> + " 'arguments': { 'path': '/machine', 'property': 'rtc-time' }
> }");
> + g_assert_nonnull(ret);
> +
> + g_assert_cmpstr(get_error_class(ret), ==, "GenericError");
> +
> + qobject_unref(ret);
> qtest_quit(qts);
> }
>
> @@ -479,8 +511,13 @@ int main(int argc, char *argv[])
>
> g_test_init(&argc, &argv, NULL);
>
> + qtest_add_func("qmp/object-add-without-props",
> + test_object_add_without_props);
> + qtest_add_func("qmp/qom-set-without-value",
> + test_qom_set_without_value);
> qtest_add_func("qmp/protocol", test_qmp_protocol);
> qtest_add_func("qmp/oob", test_qmp_oob);
> +
> qmp_schema_init(&schema);
> add_query_tests(&schema);
> qtest_add_func("qmp/preconfig", test_qmp_preconfig);
> @@ -488,5 +525,6 @@ int main(int argc, char *argv[])
> ret = g_test_run();
>
> qmp_schema_cleanup(&schema);
> +
> return ret;
> }
Is this hunk intentional?
Taking a step back: the test cases look good, but is this file an
appropriate home? The file comment states it's about "QMP protocol test
cases". These test cases test commands, not the protocol.
I figure test_qom_set_without_value() belongs to qom-test.c.
test_object_add_without_props() could go into a memory backend test
collection, or an object-add test collection. Sadly, neither exists.
We could have a qmp command test collection as a home of last resort.
Temptation to just throw a few random test cases there instead of
covering (a set of related) commands with a proper test case collection.
As is, your patch turns qmp-test.c into such a home of last resort. If
that's what we want, we should update the file comment. But I think I'd
rather have a separate qmp-cmd-test.c.
Thoughts?
[Qemu-devel] [PATCH 10/12] tests: add a qmp success-response test, Marc-André Lureau, 2018/07/06
[Qemu-devel] [PATCH 11/12] qga: process_event() simplification, Marc-André Lureau, 2018/07/06
[Qemu-devel] [PATCH 12/12] RFC: qmp: rework 'id' handling, Marc-André Lureau, 2018/07/06