diff mbox

[RFC,v2,24/47] tests/qapi-schema: Convert test harness to QAPISchemaVisitor

Message ID 1435782155-31412-25-git-send-email-armbru@redhat.com
State New
Headers show

Commit Message

Markus Armbruster July 1, 2015, 8:22 p.m. UTC
The old code prints the result of parsing (list of expression
dictionaries), and partial results of semantic analysis (list of enum
dictionaries, list of struct dictionaries).

The new code prints a trace of a schema visit, i.e. what the back-ends
are going to use.  Built-in and array types are omitted, because
they're boring.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 tests/qapi-schema/alternate-good.out            |  15 +-
 tests/qapi-schema/comments.out                  |   4 +-
 tests/qapi-schema/data-member-array.out         |  13 +-
 tests/qapi-schema/empty.out                     |   3 -
 tests/qapi-schema/enum-empty.out                |   4 +-
 tests/qapi-schema/event-case.out                |   4 +-
 tests/qapi-schema/flat-union-reverse-define.out |  21 +--
 tests/qapi-schema/ident-with-escape.out         |   7 +-
 tests/qapi-schema/include-relpath.out           |   4 +-
 tests/qapi-schema/include-repetition.out        |   4 +-
 tests/qapi-schema/include-simple.out            |   4 +-
 tests/qapi-schema/indented-expr.out             |   7 +-
 tests/qapi-schema/qapi-schema-test.out          | 186 +++++++++++++++++-------
 tests/qapi-schema/returns-int.out               |   5 +-
 tests/qapi-schema/test-qapi.py                  |  37 ++++-
 tests/qapi-schema/type-bypass.out               |   7 +-
 16 files changed, 210 insertions(+), 115 deletions(-)

Comments

Eric Blake July 21, 2015, 10:23 p.m. UTC | #1
On 07/01/2015 02:22 PM, Markus Armbruster wrote:
> The old code prints the result of parsing (list of expression
> dictionaries), and partial results of semantic analysis (list of enum
> dictionaries, list of struct dictionaries).
> 
> The new code prints a trace of a schema visit, i.e. what the back-ends
> are going to use.  Built-in and array types are omitted, because
> they're boring.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  tests/qapi-schema/alternate-good.out            |  15 +-
>  tests/qapi-schema/comments.out                  |   4 +-
>  tests/qapi-schema/data-member-array.out         |  13 +-
>  tests/qapi-schema/empty.out                     |   3 -
>  tests/qapi-schema/enum-empty.out                |   4 +-
>  tests/qapi-schema/event-case.out                |   4 +-
>  tests/qapi-schema/flat-union-reverse-define.out |  21 +--
>  tests/qapi-schema/ident-with-escape.out         |   7 +-
>  tests/qapi-schema/include-relpath.out           |   4 +-
>  tests/qapi-schema/include-repetition.out        |   4 +-
>  tests/qapi-schema/include-simple.out            |   4 +-
>  tests/qapi-schema/indented-expr.out             |   7 +-
>  tests/qapi-schema/qapi-schema-test.out          | 186 +++++++++++++++++-------
>  tests/qapi-schema/returns-int.out               |   5 +-
>  tests/qapi-schema/test-qapi.py                  |  37 ++++-
>  tests/qapi-schema/type-bypass.out               |   7 +-
>  16 files changed, 210 insertions(+), 115 deletions(-)

We have a lot more negative than positive tests of the parser (good
thing, because that meant fewer .out files to update to the new format).

No change to actual qemu code, and proves that the previous three
patches have set up enough of a framework to accurately cover our testsuite.

> 
> diff --git a/tests/qapi-schema/alternate-good.out b/tests/qapi-schema/alternate-good.out
> index 99848ee..0cbdfa1 100644
> --- a/tests/qapi-schema/alternate-good.out
> +++ b/tests/qapi-schema/alternate-good.out
> @@ -1,6 +1,9 @@
> -[OrderedDict([('struct', 'Data'), ('data', OrderedDict([('*number', 'int'), ('*name', 'str')]))]),
> - OrderedDict([('enum', 'Enum'), ('data', ['hello', 'world'])]),
> - OrderedDict([('alternate', 'Alt'), ('data', OrderedDict([('value', 'int'), ('string', 'Enum'), ('struct', 'Data')]))])]
> -[{'enum_name': 'Enum', 'enum_values': ['hello', 'world']},
> - {'enum_name': 'AltKind', 'enum_values': None}]
> -[OrderedDict([('struct', 'Data'), ('data', OrderedDict([('*number', 'int'), ('*name', 'str')]))])]
> +alternate Alt
> +    case value: int flat=False
> +    case string: Enum flat=False
> +    case struct: Data flat=False

I'm still not convinced whether we need .flat exposed through this much
detail, or if we should just normalize plain unions into flat unions
with implicit structs for each branch.  Changing your design will have
obvious ripple effects here.

> +++ b/tests/qapi-schema/data-member-array.out
> @@ -1,5 +1,8 @@
> -[OrderedDict([('enum', 'abc'), ('data', ['a', 'b', 'c'])]),
> - OrderedDict([('struct', 'def'), ('data', OrderedDict([('array', ['abc'])]))]),
> - OrderedDict([('command', 'okay'), ('data', OrderedDict([('member1', ['int']), ('member2', ['def'])]))])]
> -[{'enum_name': 'abc', 'enum_values': ['a', 'b', 'c']}]
> -[OrderedDict([('struct', 'def'), ('data', OrderedDict([('array', ['abc'])]))])]
> +object :obj-okay-args
> +    member member1: intList optional=False

Took me a moment to realize the object is an implicit one (named
':obj-okay-args') and not a typo for 'object: obj-okay-args' consistent
with members being listed 'name: type'.  But not worth changing things,
as it is sufficiently unambiguous to serve as a valid test.

> +object UserDefFlatUnion
> +    base UserDefUnionBase
> +    tag enum1
> +    case value1: UserDefA flat=True
> +    case value2: UserDefB flat=True
> +    case value3: UserDefB flat=True
> +object UserDefFlatUnion2
> +    base UserDefUnionBase
> +    tag enum1
> +    case value1: UserDefC flat=True
> +    case value2: UserDefB flat=True
> +    case value3: UserDefA flat=True
> +object UserDefNativeListUnion
> +    case integer: intList flat=False
> +    case s8: int8List flat=False
> +    case s16: int16List flat=False
> +    case s32: int32List flat=False
> +    case s64: int64List flat=False
> +    case u8: uint8List flat=False
> +    case u16: uint16List flat=False
> +    case u32: uint32List flat=False
> +    case u64: uint64List flat=False
> +    case number: numberList flat=False
> +    case boolean: boolList flat=False
> +    case string: strList flat=False
> +    case sizes: sizeList flat=False
> +enum UserDefNativeListUnionKind ['integer', 's8', 's16', 's32', 's64', 'u8', 'u16', 'u32', 'u64', 'number', 'boolean', 'string', 'sizes']

Hmm. You are dumping the tag name and type of flat unions, but not of
simple unions.  I would have expected:

object UserDefNativeListUnion
    member kind: UserDefNativeListUnionKind
    tag kind
    case integer: intList flat=False
...

The above was fallout, while below is the meat of the new visitor.

> +++ b/tests/qapi-schema/test-qapi.py
> @@ -15,11 +15,34 @@ from pprint import pprint
>  import os
>  import sys
>  
> -try:
> -    exprs = QAPISchema(sys.argv[1]).get_exprs()
> -except SystemExit:
> -    raise
> +class QAPISchemaTestVisitor(QAPISchemaVisitor):
> +    def visit_enum_type(self, name, info, values):
> +        print 'enum %s %s' % (name, values)
> +    def visit_object_type(self, name, info, base, members, variants):
> +        print 'object %s' % name
> +        if base:
> +            print '    base %s' % base.name
> +        for m in members:
> +            print '    member %s: %s optional=%s' % (m.name, m.type.name,
> +                                                     m.optional)
> +        self._print_variants(variants)

So here is where information was missing when visiting a simple union.
Why didn't 'kind' get injected into members?  And...

> +    def visit_alternate_type(self, name, info, variants):
> +        print 'alternate %s' % name
> +        self._print_variants(variants)
> +    def visit_command(self, name, info, args, rets, gen, success_response):
> +        print 'command %s %s -> %s' % (name, (args and args.name),
> +                                       (rets and rets.name))
> +        print '   gen=%s success_response=%s' % (gen, success_response)
> +    def visit_event(self, name, info, data):
> +        print 'event %s %s' % (name, data and data.name)
>  
> -pprint(exprs)
> -pprint(enum_types)
> -pprint(struct_types)
> +    @staticmethod
> +    def _print_variants(variants):
> +        if variants:
> +            if variants.tag_name:
> +                print '    tag %s' % variants.tag_name

...shouldn't a simple union report 'kind' as its tag_name?

> +            for v in variants.variants:
> +                print '    case %s: %s flat=%s' % (v.name, v.type.name, v.flat)
> +
> +schema = QAPISchema(sys.argv[1])
> +schema.visit(QAPISchemaTestVisitor())
> diff --git a/tests/qapi-schema/type-bypass.out b/tests/qapi-schema/type-bypass.out

Overall, fairly slick, but I have enough questions about the
representation of a simple union that I'll wait for the non-RFC respin
to see if you change any design.
Markus Armbruster July 27, 2015, 2:03 p.m. UTC | #2
Eric Blake <eblake@redhat.com> writes:

> On 07/01/2015 02:22 PM, Markus Armbruster wrote:
>> The old code prints the result of parsing (list of expression
>> dictionaries), and partial results of semantic analysis (list of enum
>> dictionaries, list of struct dictionaries).
>> 
>> The new code prints a trace of a schema visit, i.e. what the back-ends
>> are going to use.  Built-in and array types are omitted, because
>> they're boring.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  tests/qapi-schema/alternate-good.out            |  15 +-
>>  tests/qapi-schema/comments.out                  |   4 +-
>>  tests/qapi-schema/data-member-array.out         |  13 +-
>>  tests/qapi-schema/empty.out                     |   3 -
>>  tests/qapi-schema/enum-empty.out                |   4 +-
>>  tests/qapi-schema/event-case.out                |   4 +-
>>  tests/qapi-schema/flat-union-reverse-define.out |  21 +--
>>  tests/qapi-schema/ident-with-escape.out         |   7 +-
>>  tests/qapi-schema/include-relpath.out           |   4 +-
>>  tests/qapi-schema/include-repetition.out        |   4 +-
>>  tests/qapi-schema/include-simple.out            |   4 +-
>>  tests/qapi-schema/indented-expr.out             |   7 +-
>>  tests/qapi-schema/qapi-schema-test.out | 186
>> +++++++++++++++++-------
>>  tests/qapi-schema/returns-int.out               |   5 +-
>>  tests/qapi-schema/test-qapi.py                  |  37 ++++-
>>  tests/qapi-schema/type-bypass.out               |   7 +-
>>  16 files changed, 210 insertions(+), 115 deletions(-)
>
> We have a lot more negative than positive tests of the parser (good
> thing, because that meant fewer .out files to update to the new format).
>
> No change to actual qemu code, and proves that the previous three
> patches have set up enough of a framework to accurately cover our testsuite.
>
>> 
>> diff --git a/tests/qapi-schema/alternate-good.out
>> b/tests/qapi-schema/alternate-good.out
>> index 99848ee..0cbdfa1 100644
>> --- a/tests/qapi-schema/alternate-good.out
>> +++ b/tests/qapi-schema/alternate-good.out
>> @@ -1,6 +1,9 @@
>> -[OrderedDict([('struct', 'Data'), ('data', OrderedDict([('*number',
>> 'int'), ('*name', 'str')]))]),
>> - OrderedDict([('enum', 'Enum'), ('data', ['hello', 'world'])]),
>> - OrderedDict([('alternate', 'Alt'), ('data', OrderedDict([('value',
>> 'int'), ('string', 'Enum'), ('struct', 'Data')]))])]
>> -[{'enum_name': 'Enum', 'enum_values': ['hello', 'world']},
>> - {'enum_name': 'AltKind', 'enum_values': None}]
>> -[OrderedDict([('struct', 'Data'), ('data', OrderedDict([('*number',
>> 'int'), ('*name', 'str')]))])]
>> +alternate Alt
>> +    case value: int flat=False
>> +    case string: Enum flat=False
>> +    case struct: Data flat=False
>
> I'm still not convinced whether we need .flat exposed through this much
> detail, or if we should just normalize plain unions into flat unions
> with implicit structs for each branch.  Changing your design will have
> obvious ripple effects here.

Yes.  I think it's okay as long as we keep it out of external
interfaces.

>> +++ b/tests/qapi-schema/data-member-array.out
>> @@ -1,5 +1,8 @@
>> -[OrderedDict([('enum', 'abc'), ('data', ['a', 'b', 'c'])]),
>> - OrderedDict([('struct', 'def'), ('data', OrderedDict([('array',
>> ['abc'])]))]),
>> - OrderedDict([('command', 'okay'), ('data',
>> OrderedDict([('member1', ['int']), ('member2', ['def'])]))])]
>> -[{'enum_name': 'abc', 'enum_values': ['a', 'b', 'c']}]
>> -[OrderedDict([('struct', 'def'), ('data', OrderedDict([('array',
>> ['abc'])]))])]
>> +object :obj-okay-args
>> +    member member1: intList optional=False
>
> Took me a moment to realize the object is an implicit one (named
> ':obj-okay-args') and not a typo for 'object: obj-okay-args' consistent
> with members being listed 'name: type'.  But not worth changing things,
> as it is sufficiently unambiguous to serve as a valid test.

Perhaps omitting the ':' after member names would be less confusing.

>> +object UserDefFlatUnion
>> +    base UserDefUnionBase
>> +    tag enum1
>> +    case value1: UserDefA flat=True
>> +    case value2: UserDefB flat=True
>> +    case value3: UserDefB flat=True
>> +object UserDefFlatUnion2
>> +    base UserDefUnionBase
>> +    tag enum1
>> +    case value1: UserDefC flat=True
>> +    case value2: UserDefB flat=True
>> +    case value3: UserDefA flat=True
>> +object UserDefNativeListUnion
>> +    case integer: intList flat=False
>> +    case s8: int8List flat=False
>> +    case s16: int16List flat=False
>> +    case s32: int32List flat=False
>> +    case s64: int64List flat=False
>> +    case u8: uint8List flat=False
>> +    case u16: uint16List flat=False
>> +    case u32: uint32List flat=False
>> +    case u64: uint64List flat=False
>> +    case number: numberList flat=False
>> +    case boolean: boolList flat=False
>> +    case string: strList flat=False
>> +    case sizes: sizeList flat=False
>> +enum UserDefNativeListUnionKind ['integer', 's8', 's16', 's32',
>> 's64', 'u8', 'u16', 'u32', 'u64', 'number', 'boolean', 'string',
>> 'sizes']
>
> Hmm. You are dumping the tag name and type of flat unions, but not of
> simple unions.  I would have expected:
>
> object UserDefNativeListUnion
>     member kind: UserDefNativeListUnionKind
>     tag kind
>     case integer: intList flat=False
> ...

See below.

> The above was fallout, while below is the meat of the new visitor.
>
>> +++ b/tests/qapi-schema/test-qapi.py
>> @@ -15,11 +15,34 @@ from pprint import pprint
>>  import os
>>  import sys
>>  
>> -try:
>> -    exprs = QAPISchema(sys.argv[1]).get_exprs()
>> -except SystemExit:
>> -    raise
>> +class QAPISchemaTestVisitor(QAPISchemaVisitor):
>> +    def visit_enum_type(self, name, info, values):
>> +        print 'enum %s %s' % (name, values)
>> +    def visit_object_type(self, name, info, base, members, variants):
>> +        print 'object %s' % name
>> +        if base:
>> +            print '    base %s' % base.name
>> +        for m in members:
>> +            print '    member %s: %s optional=%s' % (m.name, m.type.name,
>> +                                                     m.optional)
>> +        self._print_variants(variants)
>
> So here is where information was missing when visiting a simple union.
> Why didn't 'kind' get injected into members?  And...

Because members are the explicit members.  A simple union's implicit tag
member is in QAPISchemaObjectTypeVariants.  See further below.

>> +    def visit_alternate_type(self, name, info, variants):
>> +        print 'alternate %s' % name
>> +        self._print_variants(variants)
>> +    def visit_command(self, name, info, args, rets, gen, success_response):
>> +        print 'command %s %s -> %s' % (name, (args and args.name),
>> +                                       (rets and rets.name))
>> +        print '   gen=%s success_response=%s' % (gen, success_response)
>> +    def visit_event(self, name, info, data):
>> +        print 'event %s %s' % (name, data and data.name)
>>  
>> -pprint(exprs)
>> -pprint(enum_types)
>> -pprint(struct_types)
>> +    @staticmethod
>> +    def _print_variants(variants):
>> +        if variants:
>> +            if variants.tag_name:
>> +                print '    tag %s' % variants.tag_name
>
> ...shouldn't a simple union report 'kind' as its tag_name?

It could.  Here's why it currently isn't.

QAPISchemaObjectType has members .base and .local_members, which come
straight from the type definition.

These get flattened into .members.

We do create a simple union's implicit member 'kind', and store it in
.variants.tag_member.  But we don't add it to .local_members, only to
.members, similar to how .base's members are only in .members.

We don't print .members here.  Instead, we print .base, .local_members
and .variants.  The part printing .variants doesn't print
.variants.tag_member.

We need to draw the line on what to print *somewhere*.  Where exactly is
of course debatable.  My working idea on what to print here is "print
everything that isn't derived from other stuff".

>> +            for v in variants.variants:
>> + print ' case %s: %s flat=%s' % (v.name, v.type.name, v.flat)
>> +
>> +schema = QAPISchema(sys.argv[1])
>> +schema.visit(QAPISchemaTestVisitor())
>> diff --git a/tests/qapi-schema/type-bypass.out
>> b/tests/qapi-schema/type-bypass.out
>
> Overall, fairly slick, but I have enough questions about the
> representation of a simple union that I'll wait for the non-RFC respin
> to see if you change any design.

Thanks!
diff mbox

Patch

diff --git a/tests/qapi-schema/alternate-good.out b/tests/qapi-schema/alternate-good.out
index 99848ee..0cbdfa1 100644
--- a/tests/qapi-schema/alternate-good.out
+++ b/tests/qapi-schema/alternate-good.out
@@ -1,6 +1,9 @@ 
-[OrderedDict([('struct', 'Data'), ('data', OrderedDict([('*number', 'int'), ('*name', 'str')]))]),
- OrderedDict([('enum', 'Enum'), ('data', ['hello', 'world'])]),
- OrderedDict([('alternate', 'Alt'), ('data', OrderedDict([('value', 'int'), ('string', 'Enum'), ('struct', 'Data')]))])]
-[{'enum_name': 'Enum', 'enum_values': ['hello', 'world']},
- {'enum_name': 'AltKind', 'enum_values': None}]
-[OrderedDict([('struct', 'Data'), ('data', OrderedDict([('*number', 'int'), ('*name', 'str')]))])]
+alternate Alt
+    case value: int flat=False
+    case string: Enum flat=False
+    case struct: Data flat=False
+enum AltKind ['value', 'string', 'struct']
+object Data
+    member number: int optional=True
+    member name: str optional=True
+enum Enum ['hello', 'world']
diff --git a/tests/qapi-schema/comments.out b/tests/qapi-schema/comments.out
index 4ce3dcf..6161b90 100644
--- a/tests/qapi-schema/comments.out
+++ b/tests/qapi-schema/comments.out
@@ -1,3 +1 @@ 
-[OrderedDict([('enum', 'Status'), ('data', ['good', 'bad', 'ugly'])])]
-[{'enum_name': 'Status', 'enum_values': ['good', 'bad', 'ugly']}]
-[]
+enum Status ['good', 'bad', 'ugly']
diff --git a/tests/qapi-schema/data-member-array.out b/tests/qapi-schema/data-member-array.out
index c39fa25..8911179 100644
--- a/tests/qapi-schema/data-member-array.out
+++ b/tests/qapi-schema/data-member-array.out
@@ -1,5 +1,8 @@ 
-[OrderedDict([('enum', 'abc'), ('data', ['a', 'b', 'c'])]),
- OrderedDict([('struct', 'def'), ('data', OrderedDict([('array', ['abc'])]))]),
- OrderedDict([('command', 'okay'), ('data', OrderedDict([('member1', ['int']), ('member2', ['def'])]))])]
-[{'enum_name': 'abc', 'enum_values': ['a', 'b', 'c']}]
-[OrderedDict([('struct', 'def'), ('data', OrderedDict([('array', ['abc'])]))])]
+object :obj-okay-args
+    member member1: intList optional=False
+    member member2: defList optional=False
+enum abc ['a', 'b', 'c']
+object def
+    member array: abcList optional=False
+command okay :obj-okay-args -> None
+   gen=True success_response=True
diff --git a/tests/qapi-schema/empty.out b/tests/qapi-schema/empty.out
index b7f89a4..e69de29 100644
--- a/tests/qapi-schema/empty.out
+++ b/tests/qapi-schema/empty.out
@@ -1,3 +0,0 @@ 
-[]
-[]
-[]
diff --git a/tests/qapi-schema/enum-empty.out b/tests/qapi-schema/enum-empty.out
index 3b75c16..e09b00f 100644
--- a/tests/qapi-schema/enum-empty.out
+++ b/tests/qapi-schema/enum-empty.out
@@ -1,3 +1 @@ 
-[OrderedDict([('enum', 'MyEnum'), ('data', [])])]
-[{'enum_name': 'MyEnum', 'enum_values': []}]
-[]
+enum MyEnum []
diff --git a/tests/qapi-schema/event-case.out b/tests/qapi-schema/event-case.out
index 3764bc7..b5ae4c2 100644
--- a/tests/qapi-schema/event-case.out
+++ b/tests/qapi-schema/event-case.out
@@ -1,3 +1 @@ 
-[OrderedDict([('event', 'oops')])]
-[]
-[]
+event oops None
diff --git a/tests/qapi-schema/flat-union-reverse-define.out b/tests/qapi-schema/flat-union-reverse-define.out
index 1ed7b8a..e156202 100644
--- a/tests/qapi-schema/flat-union-reverse-define.out
+++ b/tests/qapi-schema/flat-union-reverse-define.out
@@ -1,9 +1,12 @@ 
-[OrderedDict([('union', 'TestUnion'), ('base', 'TestBase'), ('discriminator', 'enum1'), ('data', OrderedDict([('value1', 'TestTypeA'), ('value2', 'TestTypeB')]))]),
- OrderedDict([('struct', 'TestBase'), ('data', OrderedDict([('enum1', 'TestEnum')]))]),
- OrderedDict([('enum', 'TestEnum'), ('data', ['value1', 'value2'])]),
- OrderedDict([('struct', 'TestTypeA'), ('data', OrderedDict([('string', 'str')]))]),
- OrderedDict([('struct', 'TestTypeB'), ('data', OrderedDict([('integer', 'int')]))])]
-[{'enum_name': 'TestEnum', 'enum_values': ['value1', 'value2']}]
-[OrderedDict([('struct', 'TestBase'), ('data', OrderedDict([('enum1', 'TestEnum')]))]),
- OrderedDict([('struct', 'TestTypeA'), ('data', OrderedDict([('string', 'str')]))]),
- OrderedDict([('struct', 'TestTypeB'), ('data', OrderedDict([('integer', 'int')]))])]
+object TestBase
+    member enum1: TestEnum optional=False
+enum TestEnum ['value1', 'value2']
+object TestTypeA
+    member string: str optional=False
+object TestTypeB
+    member integer: int optional=False
+object TestUnion
+    base TestBase
+    tag enum1
+    case value1: TestTypeA flat=True
+    case value2: TestTypeB flat=True
diff --git a/tests/qapi-schema/ident-with-escape.out b/tests/qapi-schema/ident-with-escape.out
index 4028430..24abf5c 100644
--- a/tests/qapi-schema/ident-with-escape.out
+++ b/tests/qapi-schema/ident-with-escape.out
@@ -1,3 +1,4 @@ 
-[OrderedDict([('command', 'fooA'), ('data', OrderedDict([('bar1', 'str')]))])]
-[]
-[]
+object :obj-fooA-args
+    member bar1: str optional=False
+command fooA :obj-fooA-args -> None
+   gen=True success_response=True
diff --git a/tests/qapi-schema/include-relpath.out b/tests/qapi-schema/include-relpath.out
index 4ce3dcf..6161b90 100644
--- a/tests/qapi-schema/include-relpath.out
+++ b/tests/qapi-schema/include-relpath.out
@@ -1,3 +1 @@ 
-[OrderedDict([('enum', 'Status'), ('data', ['good', 'bad', 'ugly'])])]
-[{'enum_name': 'Status', 'enum_values': ['good', 'bad', 'ugly']}]
-[]
+enum Status ['good', 'bad', 'ugly']
diff --git a/tests/qapi-schema/include-repetition.out b/tests/qapi-schema/include-repetition.out
index 4ce3dcf..6161b90 100644
--- a/tests/qapi-schema/include-repetition.out
+++ b/tests/qapi-schema/include-repetition.out
@@ -1,3 +1 @@ 
-[OrderedDict([('enum', 'Status'), ('data', ['good', 'bad', 'ugly'])])]
-[{'enum_name': 'Status', 'enum_values': ['good', 'bad', 'ugly']}]
-[]
+enum Status ['good', 'bad', 'ugly']
diff --git a/tests/qapi-schema/include-simple.out b/tests/qapi-schema/include-simple.out
index 4ce3dcf..6161b90 100644
--- a/tests/qapi-schema/include-simple.out
+++ b/tests/qapi-schema/include-simple.out
@@ -1,3 +1 @@ 
-[OrderedDict([('enum', 'Status'), ('data', ['good', 'bad', 'ugly'])])]
-[{'enum_name': 'Status', 'enum_values': ['good', 'bad', 'ugly']}]
-[]
+enum Status ['good', 'bad', 'ugly']
diff --git a/tests/qapi-schema/indented-expr.out b/tests/qapi-schema/indented-expr.out
index b5ce915..c5af55a 100644
--- a/tests/qapi-schema/indented-expr.out
+++ b/tests/qapi-schema/indented-expr.out
@@ -1,3 +1,4 @@ 
-[OrderedDict([('command', 'eins')]), OrderedDict([('command', 'zwei')])]
-[]
-[]
+command eins None -> None
+   gen=True success_response=True
+command zwei None -> None
+   gen=True success_response=True
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index b0b7187..921d7fb 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -1,55 +1,131 @@ 
-[OrderedDict([('enum', 'EnumOne'), ('data', ['value1', 'value2', 'value3'])]),
- OrderedDict([('struct', 'NestedEnumsOne'), ('data', OrderedDict([('enum1', 'EnumOne'), ('*enum2', 'EnumOne'), ('enum3', 'EnumOne'), ('*enum4', 'EnumOne')]))]),
- OrderedDict([('struct', 'UserDefOne'), ('base', 'UserDefZero'), ('data', OrderedDict([('string', 'str'), ('*enum1', 'EnumOne')]))]),
- OrderedDict([('struct', 'UserDefZero'), ('data', OrderedDict([('integer', 'int')]))]),
- OrderedDict([('struct', 'UserDefTwoDictDict'), ('data', OrderedDict([('userdef', 'UserDefOne'), ('string', 'str')]))]),
- OrderedDict([('struct', 'UserDefTwoDict'), ('data', OrderedDict([('string1', 'str'), ('dict2', 'UserDefTwoDictDict'), ('*dict3', 'UserDefTwoDictDict')]))]),
- OrderedDict([('struct', 'UserDefTwo'), ('data', OrderedDict([('string0', 'str'), ('dict1', 'UserDefTwoDict')]))]),
- OrderedDict([('struct', 'UserDefA'), ('data', OrderedDict([('boolean', 'bool')]))]),
- OrderedDict([('struct', 'UserDefB'), ('data', OrderedDict([('intb', 'int')]))]),
- OrderedDict([('union', 'UserDefFlatUnion'), ('base', 'UserDefUnionBase'), ('discriminator', 'enum1'), ('data', OrderedDict([('value1', 'UserDefA'), ('value2', 'UserDefB'), ('value3', 'UserDefB')]))]),
- OrderedDict([('struct', 'UserDefUnionBase'), ('base', 'UserDefZero'), ('data', OrderedDict([('string', 'str'), ('enum1', 'EnumOne')]))]),
- OrderedDict([('union', 'UserDefFlatUnion2'), ('base', 'UserDefUnionBase'), ('discriminator', 'enum1'), ('data', OrderedDict([('value1', 'UserDefC'), ('value2', 'UserDefB'), ('value3', 'UserDefA')]))]),
- OrderedDict([('alternate', 'UserDefAlternate'), ('data', OrderedDict([('uda', 'UserDefA'), ('s', 'str'), ('i', 'int')]))]),
- OrderedDict([('struct', 'UserDefC'), ('data', OrderedDict([('string1', 'str'), ('string2', 'str')]))]),
- OrderedDict([('union', 'UserDefNativeListUnion'), ('data', OrderedDict([('integer', ['int']), ('s8', ['int8']), ('s16', ['int16']), ('s32', ['int32']), ('s64', ['int64']), ('u8', ['uint8']), ('u16', ['uint16']), ('u32', ['uint32']), ('u64', ['uint64']), ('number', ['number']), ('boolean', ['bool']), ('string', ['str']), ('sizes', ['size'])]))]),
- OrderedDict([('command', 'user_def_cmd'), ('data', OrderedDict())]),
- OrderedDict([('command', 'user_def_cmd1'), ('data', OrderedDict([('ud1a', 'UserDefOne')]))]),
- OrderedDict([('command', 'user_def_cmd2'), ('data', OrderedDict([('ud1a', 'UserDefOne'), ('*ud1b', 'UserDefOne')])), ('returns', 'UserDefTwo')]),
- OrderedDict([('command', 'user_def_cmd3'), ('data', OrderedDict([('a', 'int'), ('*b', 'int')])), ('returns', 'int')]),
- OrderedDict([('struct', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64')]))]),
- OrderedDict([('struct', 'EventStructOne'), ('data', OrderedDict([('struct1', 'UserDefOne'), ('string', 'str'), ('*enum2', 'EnumOne')]))]),
- OrderedDict([('event', 'EVENT_A')]),
- OrderedDict([('event', 'EVENT_B'), ('data', OrderedDict())]),
- OrderedDict([('event', 'EVENT_C'), ('data', OrderedDict([('*a', 'int'), ('*b', 'UserDefOne'), ('c', 'str')]))]),
- OrderedDict([('event', 'EVENT_D'), ('data', OrderedDict([('a', 'EventStructOne'), ('b', 'str'), ('*c', 'str'), ('*enum3', 'EnumOne')]))]),
- OrderedDict([('enum', '__org.qemu_x-Enum'), ('data', ['__org.qemu_x-value'])]),
- OrderedDict([('struct', '__org.qemu_x-Base'), ('data', OrderedDict([('__org.qemu_x-member1', '__org.qemu_x-Enum')]))]),
- OrderedDict([('struct', '__org.qemu_x-Struct'), ('base', '__org.qemu_x-Base'), ('data', OrderedDict([('__org.qemu_x-member2', 'str')]))]),
- OrderedDict([('union', '__org.qemu_x-Union1'), ('data', OrderedDict([('__org.qemu_x-branch', 'str')]))]),
- OrderedDict([('struct', '__org.qemu_x-Struct2'), ('data', OrderedDict([('array', ['__org.qemu_x-Union1'])]))]),
- OrderedDict([('union', '__org.qemu_x-Union2'), ('base', '__org.qemu_x-Base'), ('discriminator', '__org.qemu_x-member1'), ('data', OrderedDict([('__org.qemu_x-value', '__org.qemu_x-Struct2')]))]),
- OrderedDict([('alternate', '__org.qemu_x-Alt'), ('data', OrderedDict([('__org.qemu_x-branch', 'str'), ('b', '__org.qemu_x-Base')]))]),
- OrderedDict([('event', '__ORG.QEMU_X-EVENT'), ('data', '__org.qemu_x-Struct')]),
- OrderedDict([('command', '__org.qemu_x-command'), ('data', OrderedDict([('a', ['__org.qemu_x-Enum']), ('b', ['__org.qemu_x-Struct']), ('c', '__org.qemu_x-Union2'), ('d', '__org.qemu_x-Alt')])), ('returns', '__org.qemu_x-Union1')])]
-[{'enum_name': 'EnumOne', 'enum_values': ['value1', 'value2', 'value3']},
- {'enum_name': '__org.qemu_x-Enum', 'enum_values': ['__org.qemu_x-value']},
- {'enum_name': 'UserDefAlternateKind', 'enum_values': None},
- {'enum_name': 'UserDefNativeListUnionKind', 'enum_values': None},
- {'enum_name': '__org.qemu_x-Union1Kind', 'enum_values': None},
- {'enum_name': '__org.qemu_x-AltKind', 'enum_values': None}]
-[OrderedDict([('struct', 'NestedEnumsOne'), ('data', OrderedDict([('enum1', 'EnumOne'), ('*enum2', 'EnumOne'), ('enum3', 'EnumOne'), ('*enum4', 'EnumOne')]))]),
- OrderedDict([('struct', 'UserDefOne'), ('base', 'UserDefZero'), ('data', OrderedDict([('string', 'str'), ('*enum1', 'EnumOne')]))]),
- OrderedDict([('struct', 'UserDefZero'), ('data', OrderedDict([('integer', 'int')]))]),
- OrderedDict([('struct', 'UserDefTwoDictDict'), ('data', OrderedDict([('userdef', 'UserDefOne'), ('string', 'str')]))]),
- OrderedDict([('struct', 'UserDefTwoDict'), ('data', OrderedDict([('string1', 'str'), ('dict2', 'UserDefTwoDictDict'), ('*dict3', 'UserDefTwoDictDict')]))]),
- OrderedDict([('struct', 'UserDefTwo'), ('data', OrderedDict([('string0', 'str'), ('dict1', 'UserDefTwoDict')]))]),
- OrderedDict([('struct', 'UserDefA'), ('data', OrderedDict([('boolean', 'bool')]))]),
- OrderedDict([('struct', 'UserDefB'), ('data', OrderedDict([('intb', 'int')]))]),
- OrderedDict([('struct', 'UserDefUnionBase'), ('base', 'UserDefZero'), ('data', OrderedDict([('string', 'str'), ('enum1', 'EnumOne')]))]),
- OrderedDict([('struct', 'UserDefC'), ('data', OrderedDict([('string1', 'str'), ('string2', 'str')]))]),
- OrderedDict([('struct', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64')]))]),
- OrderedDict([('struct', 'EventStructOne'), ('data', OrderedDict([('struct1', 'UserDefOne'), ('string', 'str'), ('*enum2', 'EnumOne')]))]),
- OrderedDict([('struct', '__org.qemu_x-Base'), ('data', OrderedDict([('__org.qemu_x-member1', '__org.qemu_x-Enum')]))]),
- OrderedDict([('struct', '__org.qemu_x-Struct'), ('base', '__org.qemu_x-Base'), ('data', OrderedDict([('__org.qemu_x-member2', 'str')]))]),
- OrderedDict([('struct', '__org.qemu_x-Struct2'), ('data', OrderedDict([('array', ['__org.qemu_x-Union1'])]))])]
+object :obj-EVENT_C-data
+    member a: int optional=True
+    member b: UserDefOne optional=True
+    member c: str optional=False
+object :obj-EVENT_D-data
+    member a: EventStructOne optional=False
+    member b: str optional=False
+    member c: str optional=True
+    member enum3: EnumOne optional=True
+object :obj-__org.qemu_x-command-args
+    member a: __org.qemu_x-EnumList optional=False
+    member b: __org.qemu_x-StructList optional=False
+    member c: __org.qemu_x-Union2 optional=False
+    member d: __org.qemu_x-Alt optional=False
+object :obj-user_def_cmd1-args
+    member ud1a: UserDefOne optional=False
+object :obj-user_def_cmd2-args
+    member ud1a: UserDefOne optional=False
+    member ud1b: UserDefOne optional=True
+object :obj-user_def_cmd3-args
+    member a: int optional=False
+    member b: int optional=True
+event EVENT_A None
+event EVENT_B None
+event EVENT_C :obj-EVENT_C-data
+event EVENT_D :obj-EVENT_D-data
+enum EnumOne ['value1', 'value2', 'value3']
+object EventStructOne
+    member struct1: UserDefOne optional=False
+    member string: str optional=False
+    member enum2: EnumOne optional=True
+object NestedEnumsOne
+    member enum1: EnumOne optional=False
+    member enum2: EnumOne optional=True
+    member enum3: EnumOne optional=False
+    member enum4: EnumOne optional=True
+object UserDefA
+    member boolean: bool optional=False
+alternate UserDefAlternate
+    case uda: UserDefA flat=False
+    case s: str flat=False
+    case i: int flat=False
+enum UserDefAlternateKind ['uda', 's', 'i']
+object UserDefB
+    member intb: int optional=False
+object UserDefC
+    member string1: str optional=False
+    member string2: str optional=False
+object UserDefFlatUnion
+    base UserDefUnionBase
+    tag enum1
+    case value1: UserDefA flat=True
+    case value2: UserDefB flat=True
+    case value3: UserDefB flat=True
+object UserDefFlatUnion2
+    base UserDefUnionBase
+    tag enum1
+    case value1: UserDefC flat=True
+    case value2: UserDefB flat=True
+    case value3: UserDefA flat=True
+object UserDefNativeListUnion
+    case integer: intList flat=False
+    case s8: int8List flat=False
+    case s16: int16List flat=False
+    case s32: int32List flat=False
+    case s64: int64List flat=False
+    case u8: uint8List flat=False
+    case u16: uint16List flat=False
+    case u32: uint32List flat=False
+    case u64: uint64List flat=False
+    case number: numberList flat=False
+    case boolean: boolList flat=False
+    case string: strList flat=False
+    case sizes: sizeList flat=False
+enum UserDefNativeListUnionKind ['integer', 's8', 's16', 's32', 's64', 'u8', 'u16', 'u32', 'u64', 'number', 'boolean', 'string', 'sizes']
+object UserDefOne
+    base UserDefZero
+    member string: str optional=False
+    member enum1: EnumOne optional=True
+object UserDefOptions
+    member i64: intList optional=True
+    member u64: uint64List optional=True
+    member u16: uint16List optional=True
+    member i64x: int optional=True
+    member u64x: uint64 optional=True
+object UserDefTwo
+    member string0: str optional=False
+    member dict1: UserDefTwoDict optional=False
+object UserDefTwoDict
+    member string1: str optional=False
+    member dict2: UserDefTwoDictDict optional=False
+    member dict3: UserDefTwoDictDict optional=True
+object UserDefTwoDictDict
+    member userdef: UserDefOne optional=False
+    member string: str optional=False
+object UserDefUnionBase
+    base UserDefZero
+    member string: str optional=False
+    member enum1: EnumOne optional=False
+object UserDefZero
+    member integer: int optional=False
+event __ORG.QEMU_X-EVENT __org.qemu_x-Struct
+alternate __org.qemu_x-Alt
+    case __org.qemu_x-branch: str flat=False
+    case b: __org.qemu_x-Base flat=False
+enum __org.qemu_x-AltKind ['__org.qemu_x-branch', 'b']
+object __org.qemu_x-Base
+    member __org.qemu_x-member1: __org.qemu_x-Enum optional=False
+enum __org.qemu_x-Enum ['__org.qemu_x-value']
+object __org.qemu_x-Struct
+    base __org.qemu_x-Base
+    member __org.qemu_x-member2: str optional=False
+object __org.qemu_x-Struct2
+    member array: __org.qemu_x-Union1List optional=False
+object __org.qemu_x-Union1
+    case __org.qemu_x-branch: str flat=False
+enum __org.qemu_x-Union1Kind ['__org.qemu_x-branch']
+object __org.qemu_x-Union2
+    base __org.qemu_x-Base
+    tag __org.qemu_x-member1
+    case __org.qemu_x-value: __org.qemu_x-Struct2 flat=True
+command __org.qemu_x-command :obj-__org.qemu_x-command-args -> __org.qemu_x-Union1
+   gen=True success_response=True
+command user_def_cmd None -> None
+   gen=True success_response=True
+command user_def_cmd1 :obj-user_def_cmd1-args -> None
+   gen=True success_response=True
+command user_def_cmd2 :obj-user_def_cmd2-args -> UserDefTwo
+   gen=True success_response=True
+command user_def_cmd3 :obj-user_def_cmd3-args -> int
+   gen=True success_response=True
diff --git a/tests/qapi-schema/returns-int.out b/tests/qapi-schema/returns-int.out
index 70b3ac5..1ac3e1e 100644
--- a/tests/qapi-schema/returns-int.out
+++ b/tests/qapi-schema/returns-int.out
@@ -1,3 +1,2 @@ 
-[OrderedDict([('command', 'guest-get-time'), ('returns', 'int')])]
-[]
-[]
+command guest-get-time None -> int
+   gen=True success_response=True
diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index 461c713..259f515 100644
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -15,11 +15,34 @@  from pprint import pprint
 import os
 import sys
 
-try:
-    exprs = QAPISchema(sys.argv[1]).get_exprs()
-except SystemExit:
-    raise
+class QAPISchemaTestVisitor(QAPISchemaVisitor):
+    def visit_enum_type(self, name, info, values):
+        print 'enum %s %s' % (name, values)
+    def visit_object_type(self, name, info, base, members, variants):
+        print 'object %s' % name
+        if base:
+            print '    base %s' % base.name
+        for m in members:
+            print '    member %s: %s optional=%s' % (m.name, m.type.name,
+                                                     m.optional)
+        self._print_variants(variants)
+    def visit_alternate_type(self, name, info, variants):
+        print 'alternate %s' % name
+        self._print_variants(variants)
+    def visit_command(self, name, info, args, rets, gen, success_response):
+        print 'command %s %s -> %s' % (name, (args and args.name),
+                                       (rets and rets.name))
+        print '   gen=%s success_response=%s' % (gen, success_response)
+    def visit_event(self, name, info, data):
+        print 'event %s %s' % (name, data and data.name)
 
-pprint(exprs)
-pprint(enum_types)
-pprint(struct_types)
+    @staticmethod
+    def _print_variants(variants):
+        if variants:
+            if variants.tag_name:
+                print '    tag %s' % variants.tag_name
+            for v in variants.variants:
+                print '    case %s: %s flat=%s' % (v.name, v.type.name, v.flat)
+
+schema = QAPISchema(sys.argv[1])
+schema.visit(QAPISchemaTestVisitor())
diff --git a/tests/qapi-schema/type-bypass.out b/tests/qapi-schema/type-bypass.out
index eaf20f8..b147864 100644
--- a/tests/qapi-schema/type-bypass.out
+++ b/tests/qapi-schema/type-bypass.out
@@ -1,3 +1,4 @@ 
-[OrderedDict([('command', 'unsafe'), ('data', OrderedDict([('arg', '**')])), ('returns', '**'), ('gen', False)])]
-[]
-[]
+object :obj-unsafe-args
+    member arg: ** optional=False
+command unsafe :obj-unsafe-args -> **
+   gen=False success_response=True