| Message ID | 20220830161545.84198-17-victortoso@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qapi examples fixes and rfc for another generator | expand |
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. 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?
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). > 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. Cheers, Victor
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?
Hi, On Wed, Aug 31, 2022 at 04:57:20PM +0200, Markus Armbruster wrote: > 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 :) Great :) > >> 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? Yeah. I'll resend this series fixing the style you have proposed and I'll be removing the patches that might need some extra discussion, like this rfc and examples that are cut short with a comment. I'll improve this generator and send it later, probably after the next iteration of qapi-go. This also gives some room to feedback from others, if any. Cheers, Victor
diff --git a/scripts/qapi/dumpexamples.py b/scripts/qapi/dumpexamples.py new file mode 100644 index 0000000000..c14ed11774 --- /dev/null +++ b/scripts/qapi/dumpexamples.py @@ -0,0 +1,194 @@ +""" +Dump examples for Developers +""" +# Copyright (c) 2022 Red Hat Inc. +# +# Authors: +# Victor Toso <victortoso@redhat.com> +# +# This work is licensed under the terms of the GNU GPL, version 2. +# See the COPYING file in the top-level directory. + +# Just for type hint on self +from __future__ import annotations + +import os +import json +import random +import string + +from typing import Dict, List, Optional + +from .schema import ( + QAPISchema, + QAPISchemaType, + QAPISchemaVisitor, + QAPISchemaEnumMember, + QAPISchemaFeature, + QAPISchemaIfCond, + QAPISchemaObjectType, + QAPISchemaObjectTypeMember, + QAPISchemaVariants, +) +from .source import QAPISourceInfo + + +def gen_examples(schema: QAPISchema, + output_dir: str, + prefix: str) -> None: + vis = QAPISchemaGenExamplesVisitor(prefix) + schema.visit(vis) + vis.write(output_dir) + + +def get_id(random, size: int) -> str: + letters = string.ascii_lowercase + return ''.join(random.choice(letters) for i in range(size)) + + +def next_object(text, start, end, context) -> Dict: + # Start of json object + start = text.find("{", start) + end = text.rfind("}", start, end+1) + + # try catch, pretty print issues + try: + ret = json.loads(text[start:end+1]) + except Exception as e: + print("Error: {}\nLocation: {}\nData: {}\n".format( + str(e), context, text[start:end+1])) + return {} + else: + return ret + + +def parse_text_to_dicts(text: str, context: str) -> List[Dict]: + examples, clients, servers = [], [], [] + + count = 1 + c, s = text.find("->"), text.find("<-") + while c != -1 or s != -1: + if c == -1 or (s != -1 and s < c): + start, target = s, servers + else: + start, target = c, clients + + # Find the client and server, if any + if c != -1: + c = text.find("->", start + 1) + if s != -1: + s = text.find("<-", start + 1) + + # Find the limit of current's object. + # We first look for the next message, either client or server. If none + # is avaible, we set the end of the text as limit. + if c == -1 and s != -1: + end = s + elif c != -1 and s == -1: + end = c + elif c != -1 and s != -1: + end = (c < s) and c or s + else: + end = len(text) - 1 + + message = next_object(text, start, end, context) + if len(message) > 0: + message_type = "return" + if "execute" in message: + message_type = "command" + elif "event" in message: + message_type = "event" + + target.append({ + "sequence-order": count, + "message-type": message_type, + "message": message + }) + count += 1 + + examples.append({"client": clients, "server": servers}) + return examples + + +def parse_examples_of(self: QAPISchemaGenExamplesVisitor, + name: str): + + assert(name in self.schema._entity_dict) + obj = self.schema._entity_dict[name] + assert((obj.doc is not None)) + module_name = obj._module.name + + # We initialize random with the name so that we get consistent example + # ids over different generations. The ids of a given example might + # change when adding/removing examples, but that's acceptable as the + # goal is just to grep $id to find what example failed at a given test + # with minimum chorn over regenerating. + random.seed(name, version=2) + + for s in obj.doc.sections: + if s.name != "Example": + continue + + if module_name not in self.target: + self.target[module_name] = [] + + context = f'''{name} at {obj.info.fname}:{obj.info.line}''' + examples = parse_text_to_dicts(s.text, context) + for example in examples: + self.target[module_name].append({ + "id": get_id(random, 10), + "client": example["client"], + "server": example["server"] + }) + + +class QAPISchemaGenExamplesVisitor(QAPISchemaVisitor): + + def __init__(self, prefix: str): + super().__init__() + self.target = {} + self.schema = None + + def visit_begin(self, schema): + self.schema = schema + + def visit_end(self): + self.schema = None + + def write(self: QAPISchemaGenExamplesVisitor, + output_dir: str) -> None: + for filename, content in self.target.items(): + pathname = os.path.join(output_dir, "examples", filename) + odir = os.path.dirname(pathname) + os.makedirs(odir, exist_ok=True) + result = {"examples": content} + + with open(pathname, "w") as outfile: + outfile.write(json.dumps(result, indent=2, sort_keys=True)) + + def visit_command(self: QAPISchemaGenExamplesVisitor, + name: str, + info: Optional[QAPISourceInfo], + ifcond: QAPISchemaIfCond, + features: List[QAPISchemaFeature], + arg_type: Optional[QAPISchemaObjectType], + ret_type: Optional[QAPISchemaType], + gen: bool, + success_response: bool, + boxed: bool, + allow_oob: bool, + allow_preconfig: bool, + coroutine: bool) -> None: + + if gen: + parse_examples_of(self, name) + + def visit_event(self: QAPISchemaGenExamplesVisitor, + name: str, + info: Optional[QAPISourceInfo], + ifcond: QAPISchemaIfCond, + features: List[QAPISchemaFeature], + arg_type: Optional[QAPISchemaObjectType], + boxed: bool): + + parse_examples_of(self, name) diff --git a/scripts/qapi/main.py b/scripts/qapi/main.py index fc216a53d3..9e771f4dd3 100644 --- a/scripts/qapi/main.py +++ b/scripts/qapi/main.py @@ -13,6 +13,7 @@ from .commands import gen_commands from .common import must_match +from .dumpexamples import gen_examples from .error import QAPIError from .events import gen_events from .introspect import gen_introspect @@ -54,6 +55,7 @@ def generate(schema_file: str, gen_events(schema, output_dir, prefix) gen_introspect(schema, output_dir, prefix, unmask) + gen_examples(schema, output_dir, prefix) def main() -> int: """
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> --- scripts/qapi/dumpexamples.py | 194 +++++++++++++++++++++++++++++++++++ scripts/qapi/main.py | 2 + 2 files changed, 196 insertions(+) create mode 100644 scripts/qapi/dumpexamples.py