Re: [PATCH v1 16/16] RFC: add a generator for qapi's examples

[Markus Armbruster]

Victor Toso <victortoso@redhat.com> writes:

> Hi,
>
> On Wed, Aug 31, 2022 at 02:01:54PM +0200, Markus Armbruster wrote:
>> Victor Toso <victortoso@redhat.com> writes:
>>
>> > The goal of this generator is to validate QAPI examples and transform
>> > them into a format that can be used for 3rd party applications to
>> > validate their QAPI/QMP introspection.
>> >
>> > For each Example section, we parse server and client messages into a
>> > python dictionary. This step alone has found several ill formatted
>> > JSON messages in the examples.
>> >
>> > The generator outputs another JSON file with all the examples in the
>> > QAPI module that they came from. This can be used to validate the
>> > introspection between QAPI/QMP to language bindings.
>> >
>> > When used with the POC qapi-go branch, we have found bad QMP messages
>> > with wrong member names, mandatory members that were missing and
>> > optional members that were being set with null (not needed).
>> >
>> > A simple example of the output format is:
>> >
>> >  { "examples": [
>> >    {
>> >      "id": "ksuxwzfayw",
>> >      "client": [
>> >      {
>> >        "sequence-order": 1
>> >        "message-type": "command",
>> >        "message":
>> >        { "arguments":
>> >          { "device": "scratch", "size": 1073741824 },
>> >          "execute": "block_resize"
>> >        },
>> >     } ],
>> >     "server": [
>> >     {
>> >       "sequence-order": 2
>> >       "message-type": "return",
>> >       "message": { "return": {} },
>> >     } ]
>> >     }
>> >   ] }
>> >
>> > If this idea seems reasonable, we can add python-qemu-qmp to validate
>> > each message at generation time already.
>> >
>> > Signed-off-by: Victor Toso <victortoso@redhat.com>
>>
>> If I understand you correctly, there are two benefits:
>>
>> 1. Mechanical syntax check for examples
>>
>>    Love it.
>
> Not just JSON syntax but can be extend to the introspection
> layer. Errors like wrong member names would fail while parsing
> the examples (issues such as fixed by patches 11 and 13/16 should
> not happen anymore).

It's also a mechanical check against the schema.  Still love it :)

>> 2. Can extract examples for use as test cases
>>
>>    Sounds good to me.  Possible redundancy with existing tests.
>>    Probably nothing to worry about.
>>
>>    Can you explain in a bit more detail how the extracted data
>>    is (to be) used?
>
> Sure.
>
> The Golang test that consumes this is 152 lines of code [0]. The
> idea is that we can use the examples to feed Golang unmarshalling
> code and then marshall it back to JSON and compare input JSON
> with output JSON and see that their content matches.
>
> [0] https://gitlab.com/victortoso/qapi-go/-/blob/wip-v3/test/examples_test.go
>
> I have generated the examples with this patch series and stored
> the output here [1]
>
> [1] https://gitlab.com/victortoso/qapi-go/-/tree/wip-v3/test/data/examples
>
> The examples are QMP messages that are either sent by Client "->"
> or sent by Server "<-". The order matters so I take the order set
> in the examples and store it as "sequence-order".
>
> In the Go test code, I follow the sequence-order. One example of
> this being useful is that we know which Return type to expect
> after a Command is issued.
>
> I've also included metadata about the type of message, which is
> one of three options: command, event or return. (Errors are
> return too).
>
> This is important because it makes the tests very easy to write.
> Different Unmarshal/Marshal code can be set in the code block of
> the specific message type.
>
> --
>
> The things that makes me quite excited with this idea are:
>
>  1. We have valid functional examples documented. If the examples
>     break, we would have the software in place to know it (plug
>     to ci or some other ninja check seems reasonable to me)
>
>  2. Developers should get more interested in documenting examples
>     as that alone is is a valid test case, even if only useful
>     for language binding's syntax.

Thanks!  Would you like to work some of this into your commit message?