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

[Steven Sistare <steven.sistare@oracle.com>]

On 7/10/2025 4:54 AM, Markus Armbruster wrote:
> 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.

I will assert that the returned list is not empty.

> 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.

OK.  I will keep the existing traversal test, and add a test for
an object's property value.

- Steve

> 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 :)
> 
> [...]
>