Re: [Qemu-devel] [PATCH v3 18/49] tests: modify visit_enum_type() in test-qapi to print members

[Markus Armbruster]

The subject is misleading: visit_enum_type() prints members even before
this patch already.

Marc-André Lureau <address@hidden> writes:

> Use a common self._print_members() to print enum members details.
>
> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
>  tests/qapi-schema/comments.out           | 14 ++++++-
>  tests/qapi-schema/doc-bad-section.out    | 13 ++++++-
>  tests/qapi-schema/doc-good.out           | 17 ++++++--
>  tests/qapi-schema/empty.out              |  9 ++++-
>  tests/qapi-schema/event-case.out         |  9 ++++-
>  tests/qapi-schema/ident-with-escape.out  |  9 ++++-
>  tests/qapi-schema/include-relpath.out    | 14 ++++++-
>  tests/qapi-schema/include-repetition.out | 14 ++++++-
>  tests/qapi-schema/include-simple.out     | 14 ++++++-
>  tests/qapi-schema/indented-expr.out      |  9 ++++-
>  tests/qapi-schema/qapi-schema-test.out   | 49 +++++++++++++++++++-----
>  tests/qapi-schema/test-qapi.py           | 18 ++++++---
>  12 files changed, 158 insertions(+), 31 deletions(-)
>
> diff --git a/tests/qapi-schema/comments.out b/tests/qapi-schema/comments.out
> index 8d2f1ce8a2..d1abc4b5a1 100644
> --- a/tests/qapi-schema/comments.out
> +++ b/tests/qapi-schema/comments.out
> @@ -1,5 +1,15 @@
>  object q_empty
> -enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool']
> +enum QType
>      prefix QTYPE
> +    member none
> +    member qnull
> +    member qnum
> +    member qstring
> +    member qdict
> +    member qlist
> +    member qbool
>  module comments.json
> -enum Status ['good', 'bad', 'ugly']
> +enum Status
> +    member good
> +    member bad
> +    member ugly
[More of the same...]
> diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
> index 3623deae62..7e7b8f9f0f 100644
> --- a/tests/qapi-schema/test-qapi.py
> +++ b/tests/qapi-schema/test-qapi.py
> @@ -12,7 +12,8 @@
>  
>  from __future__ import print_function
>  import sys
> -from qapi.common import QAPIError, QAPISchema, QAPISchemaVisitor
> +from qapi.common import QAPIError, QAPISchema, QAPISchemaVisitor, \
> +    QAPISchemaObjectTypeMember
>  
>  
>  class QAPISchemaTestVisitor(QAPISchemaVisitor):
> @@ -24,18 +25,17 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor):
>          print('include %s' % name)
>  
>      def visit_enum_type(self, name, info, ifcond, members, prefix):
> -        print('enum %s %s' % (name, [m.name for m in members]))
> +        print('enum %s' % name)
>          if prefix:
>              print('    prefix %s' % prefix)
> +        self._print_members(members)
>          self._print_if(ifcond)
>  
>      def visit_object_type(self, name, info, ifcond, 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_members(members)
>          self._print_variants(variants)
>          self._print_if(ifcond)
>  
> @@ -57,6 +57,14 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor):
>          print('   boxed=%s' % boxed)
>          self._print_if(ifcond)
>  
> +    @staticmethod
> +    def _print_members(members):
> +        for m in members:
> +            print('    member %s%s' % (
> +                m.name,
> +                ': %s optional=%s' % (m.type.name, m.optional)
> +                if isinstance(m, QAPISchemaObjectTypeMember) else ''))
> +
>      @staticmethod
>      def _print_variants(variants):
>          if variants:

I don't think the de-duplication is worth the isinstance() ugliness,
even with PATCH 21's additional line.

Compare the stupidest possible solution:

   diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
   index 3623deae62..37547dc233 100644
   --- a/tests/qapi-schema/test-qapi.py
   +++ b/tests/qapi-schema/test-qapi.py
   @@ -24,9 +24,11 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor):
            print('include %s' % name)

        def visit_enum_type(self, name, info, ifcond, members, prefix):
   -        print('enum %s %s' % (name, [m.name for m in members]))
   +        print('enum %s' % name)
            if prefix:
                print('    prefix %s' % prefix)
   +        for m in members:
   +            print('    member %s' % m.name)
            self._print_if(ifcond)

        def visit_object_type(self, name, info, ifcond, base, members, 
variants):

PATCH 21 will then add two lines instead of one.  Still less code, and
simpler, too.

Revised commit message could be:

    tests: Print enum type members more like object type members

    Commit 93bda4dd461 changed the internal representation of enum type
    members from str to QAPISchemaMember, but we still print only a
    string.  Has been good enough, as the name is the member's only
    attribute of interest, but that's about to change.  To prepare,
    print them more like object type members.