Re: [Qemu-devel] [PATCH v4 07/10] tests: add qmp/qom-set-without-value test

[Markus Armbruster]

Marc-André Lureau <address@hidden> writes:

> test_qom_set_without_value() is about a bug in infrastructure used by
> the QMP core, fixed in commit c489780203.  We covered the bug in
> infrastructure unit tests (commit bce3035a44).  I wrote that test
> earlier, to cover QMP level as well, the test could go into qmp-test.
>
> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
>  tests/qmp-test.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>
> diff --git a/tests/qmp-test.c b/tests/qmp-test.c
> index 4ae2245484..fdfe73b6d2 100644
> --- a/tests/qmp-test.c
> +++ b/tests/qmp-test.c
> @@ -348,6 +348,23 @@ static void test_qmp_preconfig(void)
>      qtest_quit(qs);
>  }
>  
> +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);
> +}
> +
>  int main(int argc, char *argv[])
>  {
>      g_test_init(&argc, &argv, NULL);
> @@ -355,6 +372,7 @@ int main(int argc, char *argv[])
>      qtest_add_func("qmp/protocol", test_qmp_protocol);
>      qtest_add_func("qmp/oob", test_qmp_oob);
>      qtest_add_func("qmp/preconfig", test_qmp_preconfig);
> +    qtest_add_func("qmp/qom-set-without-value", test_qom_set_without_value);
>  
>      return g_test_run();
>  }

What we're testing here is a missing 'any' parameter.  The test should
be named accordingly.  Perhaps:

       qtest_add_func("qmp/missing-any", test_missing_any);

I can't bring myself to care for this test.  There are countless ways to
run a command with invalid parameters, and this is just one.  The only
thing that sets it apart is it used to tickle a visitor bug we've long
fixed, complete with a unit test to prevent regressions.  But the cost
of carrying the test is pretty low, so if you really want it, then
taking it without further argument is probably cheaper even if it's as
redundant as I suspect it is.