Re: [PATCH V3 3/3] tests/qtest/qom-test: unit test for qom-list-getv

[Markus Armbruster <armbru@redhat.com>]

Steven Sistare <steven.sistare@oracle.com> writes:

> On 7/8/2025 6:02 PM, Philippe Mathieu-Daudé wrote:
>> Hi Steve,
>> On 8/7/25 19:24, Steve Sistare wrote:
>>> Add a unit test for qom-list-getv.
>>>
>>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>>> ---
>>>   tests/qtest/qom-test.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>   1 file changed, 64 insertions(+)
>>>
>>> diff --git a/tests/qtest/qom-test.c b/tests/qtest/qom-test.c
>>> index 27d70bc..4defff1 100644
>>> --- a/tests/qtest/qom-test.c
>>> +++ b/tests/qtest/qom-test.c
>>> @@ -11,11 +11,72 @@
>>>   #include "qobject/qdict.h"
>>>   #include "qobject/qlist.h"
>>> +#include "qobject/qstring.h"
>>>   #include "qemu/cutils.h"
>>>   #include "libqtest.h"
>>>   static int verbosity_level;
>>> +static void test_getv(QTestState *qts, QList *paths)
>>> +{
>>> +    QListEntry *entry, *prop_entry, *path_entry;
>>> +    g_autoptr(QDict) response = NULL;
>>> +    QDict *args = qdict_new();
>>> +    QDict *prop;
>>> +    QList *return_list;
>>> +
>>> +    if (verbosity_level >= 2) {
>>
>> Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>
>> But I note this doesn't assert anything except if you use V=3 and
>> look at the output.
>
> I don't follow.  It unconditionally traverses the whole tree and asserts
> that properties are present.  Plus, for V >= 2, it prints paths.

It starts with path "/machine".

For each property returned for that path, it checks there is a name and
type.

It collects the paths where the type starts with "child<" into a list,
and recurses.  So it walks the entire tree under "/machine".

It does not examine property values at all.  This is a gap in testing.

It does not check properties against expectations.  The test would
silently pass if qom-list-getv always returned [], or if it always
returned crap types like "".  Another gap.

Checking all this for the entire tree is entirely impractical.  But we
could check it for a suitable path of our choice.  I recommend to avoid
going into machine-dependent weeds there.  Maybe create some suitable
thing with -object, and check that.

If you can't afford implementing this, document the gaps in a TODO
comment.

> It is patterned after test_properties() in the same file.

Same gaps :)

[...]