Marc-André Lureau <marcandre.lur...@redhat.com> writes:
> test_object_add_without_props() tests a bug in qmp_object_add() we
> fixed in commit e64c75a975. Sadly, we don't have systematic
> object-add tests. This lone test can go into qmp-cmd-test for want of
> a better home.
>
> Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> ---
> tests/qmp-cmd-test.c | 31 +++++++++++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
>
> diff --git a/tests/qmp-cmd-test.c b/tests/qmp-cmd-test.c
> index c5b70df974..3ba8f68476 100644
> --- a/tests/qmp-cmd-test.c
> +++ b/tests/qmp-cmd-test.c
> @@ -19,6 +19,15 @@
>
> 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;
> +}
> +
> /* Query smoke tests */
>
> static int query_error_class(const char *cmd)
Copied from qmp-test.c. It should be factored out instead. Where to
put it? libqtest.c isn't quite right, as the function could
theoretically be useful in unit tests as well, but I guess it would do
for now.
Asserting presence of "desc" makes little sense outside qmp-test.c
protocol tests, but it doesn't hurt, either.
Grep shows more possible users in tests/drive_del-test.c and
tests/test-qga.c.
> @@ -197,6 +206,24 @@ static void add_query_tests(QmpSchema *schema)
> }
> }
>
> +static void test_object_add_without_props(void)
> +{
> + QTestState *qts;
> + QDict *ret;
qmp-test.c and qmp-cmd-test.c commonly use @resp for the response.
> +
> + qts = qtest_init(common_args);
> +
> + ret = qtest_qmp(qts,
> + "{'execute': 'object-add', 'arguments':"
> + " {'qom-type': 'memory-backend-ram', 'id': 'ram1' } }");
> + g_assert_nonnull(ret);
What's wrong with g_assert(!ret)?
> +
> + g_assert_cmpstr(get_error_class(ret), ==, "GenericError");
> +
> + qobject_unref(ret);
Permit me to digress.
When you expect success, you check @resp like this:
ret = qdict_get_qdict(resp, "return");
... laborously check @ret against expectations ...
If you feel pedantically thorough, you can throw in
g_assert(!qdict_haskey(resp, "error");
When you expect failure, you check like this:
error = qdict_get_qdict(resp, "error");
class = qdict_get_try_str(error, "class");
g_assert_cmpstr(class, ==, "GenericError");
and perhaps
g_assert(!qdict_haskey(resp, "return");
get_error_class() saves a little typing in the failure case. It's still
an awfully verbose overall, and the checking is full of holes more often
than not. There's got to be a better way.
> + qtest_quit(qts);
> +}
> +
> int main(int argc, char *argv[])
> {
> QmpSchema schema;
> @@ -206,6 +233,10 @@ int main(int argc, char *argv[])
>
> qmp_schema_init(&schema);
> add_query_tests(&schema);
> +
> + qtest_add_func("qmp/object-add-without-props",
> + test_object_add_without_props);
> +
> ret = g_test_run();
>
> qmp_schema_cleanup(&schema);
May I have a TODO comment asking for coverage of generic object-add
failure modes?
