Re: [PATCH v1 0/7] Validate and test qapi examples

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 7/9/23 20:17, Victor Toso wrote:
> Hi,

>>    File "/home/berrange/src/virt/qemu/scripts/qapi/dumpexamples.py", line 118, in parse_examples_of
>>      assert((obj.doc is not None))
>>              ^^^^^^^^^^^^^^^^^^^
>> AssertionError
>> ninja: build stopped: subcommand failed.
>>
>> not sure if that's related to the examples that still need fixing or not ?
> 
> This is related to the script being fed with data without
> documentation. In general, asserting should be the right approach
> because we don't want API without docs but this failure comes
> from the tests, that is, adding the following diff:
> 
> diff --git a/scripts/qapi/dumpexamples.py b/scripts/qapi/dumpexamples.py
> index c14ed11774..a961c0575d 100644
> --- a/scripts/qapi/dumpexamples.py
> +++ b/scripts/qapi/dumpexamples.py
> @@ -115,6 +115,10 @@ def parse_examples_of(self:
> QAPISchemaGenExamplesVisitor,
> 
>       assert(name in self.schema._entity_dict)
>       obj = self.schema._entity_dict[name]
> +    if obj.doc is None:
> +        print(f"{name} does not have documentation")
> +        return
> +
>       assert((obj.doc is not None))
>       module_name = obj._module.name
> 
> gives:
> 
>      user-def-cmd0 does not have documentation
>      user-def-cmd does not have documentation
[...]

> So, not sure if we should:
>   1. Avoid asserting when running with tests

This seems the most sensible option, adding an argument to
the 'command' invoked by meson's test_qapi_files() target in
tests/meson.build.

>   2. Avoid running this generator with tests
>   3. Add some minimal docs to the tests
> 
> Both (1) and (2) are quite simple. Not sure if there is real
> benefit in (3). If we should tweak qemu tests with this, should
> be related to using the JSON output itself, to keep examples
> correct.

IMO (3) is a waste of time.

Regards,

Phil.

> Cheers,
> Victor