Re: [PATCH 1/3] tests: convert check-qom-proplist to keyval

[Eric Blake <eblake@redhat.com>]

On 3/11/21 11:24 AM, Paolo Bonzini wrote:
> The command-line creation test is using QemuOpts.  Switch it to keyval,
> since the emulator has some special needs and thus the last user of
> user_creatable_add_opts will go away with the next patch.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  tests/check-qom-proplist.c | 74 ++++++++++++++++++++++++++------------
>  1 file changed, 52 insertions(+), 22 deletions(-)
> 

>  static void test_dummy_createcmdl(void)
>  {
> -    QemuOpts *opts;
> +    QDict *qdict;
>      DummyObject *dobj;
>      Error *err = NULL;
> -    const char *params = TYPE_DUMMY \
> -                         ",id=dev0," \
> -                         "bv=yes,sv=Hiss hiss hiss,av=platypus";
> +    bool help;
> +    const char *params = "bv=yes,sv=Hiss hiss hiss,av=platypus";
>  
> +    /* Needed for user_creatable_del.  */
>      qemu_add_opts(&qemu_object_opts);
> -    opts = qemu_opts_parse(&qemu_object_opts, params, true, &err);
> +
> +    qdict = keyval_parse(params, "qom-type", &help, &err);
>      g_assert(err == NULL);
> -    g_assert(opts);
> +    g_assert(qdict);
> +    g_assert(!help);
>  
> -    dobj = DUMMY_OBJECT(user_creatable_add_opts(opts, &err));
> +    g_assert(test_create_obj(qdict, &err));

This performs a side-effect inside of g_assert().  Do we care?  On the
one hand, this is a testsuite (where disabling g_assert weakens the
test), on the other hand, even if we can guarantee g_assert is not
disabled in the testsuite, it lends itself to poor copy-and-paste
practice to other sites.  Better would be to assign to a bool outside
g_assert(), then assert the variable.

>      g_assert(err == NULL);
> +    qobject_unref(qdict);
> +
> +    dobj = DUMMY_OBJECT(object_resolve_path_component(object_get_objects_root(),
> +                                                      "dev0"));
>      g_assert(dobj);
>      g_assert_cmpstr(dobj->sv, ==, "Hiss hiss hiss");
>      g_assert(dobj->bv == true);
>      g_assert(dobj->av == DUMMY_PLATYPUS);
>  
> +    qdict = keyval_parse(params, "qom-type", &help, &err);
> +    g_assert(!test_create_obj(qdict, &err));

And again.

> +    g_assert(err);
> +    g_assert(object_resolve_path_component(object_get_objects_root(), "dev0")
> +             == OBJECT(dobj));
> +    qobject_unref(qdict);
> +    error_free(err);
> +    err = NULL;
> +
> +    qdict = keyval_parse(params, "qom-type", &help, &err);
>      user_creatable_del("dev0", &error_abort);
> +    g_assert(object_resolve_path_component(object_get_objects_root(), "dev0")
> +             == NULL);
>  
> -    object_unref(OBJECT(dobj));
> -
> -    /*
> -     * cmdline-parsing via qemu_opts_parse() results in a QemuOpts entry
> -     * corresponding to the Object's ID to be added to the QemuOptsList
> -     * for objects. To avoid having this entry conflict with future
> -     * Objects using the same ID (which can happen in cases where
> -     * qemu_opts_parse() is used to parse the object params, such as
> -     * with hmp_object_add() at the time of this comment), we need to
> -     * check for this in user_creatable_del() and remove the QemuOpts if
> -     * it is present.
> -     *
> -     * The below check ensures this works as expected.
> -     */
> -    g_assert_null(qemu_opts_find(&qemu_object_opts, "dev0"));
> +    g_assert(test_create_obj(qdict, &err));

and again

> +    g_assert(err == NULL);
> +    qobject_unref(qdict);
> +
> +    dobj = DUMMY_OBJECT(object_resolve_path_component(object_get_objects_root(),
> +                                                      "dev0"));
> +    g_assert(dobj);
> +    g_assert_cmpstr(dobj->sv, ==, "Hiss hiss hiss");
> +    g_assert(dobj->bv == true);
> +    g_assert(dobj->av == DUMMY_PLATYPUS);
> +    g_assert(object_resolve_path_component(object_get_objects_root(), "dev0")
> +             == OBJECT(dobj));
> +
> +    object_unparent(OBJECT(dobj));
>  }
>  
>  static void test_dummy_badenum(void)
> 

Otherwise, this looks like a sane thing to do.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org