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

[Philippe Mathieu-Daudé]

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