1. Patchew
  2. QEMU
  3. [PATCH v3 0/3] qapi: Add detection for the 'savevm' fix for blockdev
  4. View patch

[PATCH v3 2/3] tests: qapi: Test 'features' of commands

Peter Krempa posted 3 patches 6 years, 4 months ago
There is a newer version of this series
[PATCH v3 2/3] tests: qapi: Test 'features' of commands
Posted by Peter Krempa 6 years, 4 months ago 
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 tests/qapi-schema/qapi-schema-test.json | 26 ++++++++++++++++++++++
 tests/qapi-schema/qapi-schema-test.out  | 29 +++++++++++++++++++++++++
 tests/qapi-schema/test-qapi.py          |  4 ++++
 tests/test-qmp-cmds.c                   | 28 ++++++++++++++++++++++++
 4 files changed, 87 insertions(+)

diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 75c42eb0e3..220859d4c9 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -290,3 +290,29 @@
             'cfs1': 'CondFeatureStruct1',
             'cfs2': 'CondFeatureStruct2',
             'cfs3': 'CondFeatureStruct3' } }
+
+# test 'features' for command
+
+{ 'command': 'test-command-features1',
+  'features': [] }
+
+{ 'command': 'test-command-features2',
+  'features': [ 'feature1' ] }
+
+{ 'command': 'test-command-features3',
+  'features': [ 'feature1', 'feature2' ] }
+
+{ 'command': 'test-command-features4',
+  'features': [ { 'name': 'feature1', 'if': 'defined(TEST_IF_FEATURE_1)'} ] }
+
+{ 'command': 'test-command-features5',
+  'features': [ { 'name': 'feature1', 'if': 'defined(TEST_IF_FEATURE_1)'},
+                { 'name': 'feature2', 'if': 'defined(TEST_IF_FEATURE_2)'} ] }
+
+{ 'command': 'test-command-features6',
+  'features': [ { 'name': 'feature1', 'if': 'defined(TEST_IF_FEATURE_1)'},
+                { 'name': 'feature2', 'if': 'defined(TEST_IF_FEATURE_2)'} ] }
+
+{ 'command': 'test-command-features7',
+  'features': [ { 'name': 'feature1', 'if': [ 'defined(TEST_IF_COND_1)',
+                                              'defined(TEST_IF_COND_2)'] } ] }
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 98031da96f..a38e348d54 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -412,3 +412,32 @@ object q_obj_test-features-arg
     member cfs3: CondFeatureStruct3 optional=False
 command test-features q_obj_test-features-arg -> None
    gen=True success_response=True boxed=False oob=False preconfig=False
+command test-command-features1 None -> None
+   gen=True success_response=True boxed=False oob=False preconfig=False
+command test-command-features2 None -> None
+   gen=True success_response=True boxed=False oob=False preconfig=False
+   feature feature1
+command test-command-features3 None -> None
+   gen=True success_response=True boxed=False oob=False preconfig=False
+   feature feature1
+   feature feature2
+command test-command-features4 None -> None
+   gen=True success_response=True boxed=False oob=False preconfig=False
+   feature feature1
+        if ['defined(TEST_IF_FEATURE_1)']
+command test-command-features5 None -> None
+   gen=True success_response=True boxed=False oob=False preconfig=False
+   feature feature1
+        if ['defined(TEST_IF_FEATURE_1)']
+   feature feature2
+        if ['defined(TEST_IF_FEATURE_2)']
+command test-command-features6 None -> None
+   gen=True success_response=True boxed=False oob=False preconfig=False
+   feature feature1
+        if ['defined(TEST_IF_FEATURE_1)']
+   feature feature2
+        if ['defined(TEST_IF_FEATURE_2)']
+command test-command-features7 None -> None
+   gen=True success_response=True boxed=False oob=False preconfig=False
+   feature feature1
+        if ['defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)']
diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index e13c2e8671..62e65b6a7d 100755
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -80,6 +80,10 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor):
         print('   gen=%s success_response=%s boxed=%s oob=%s preconfig=%s'
               % (gen, success_response, boxed, allow_oob, allow_preconfig))
         self._print_if(ifcond)
+        if features:
+            for f in features:
+                print('   feature %s' % f.name)
+                self._print_if(f.ifcond, 8)

     def visit_event(self, name, info, ifcond, arg_type, boxed):
         print('event %s %s' % (name, arg_type and arg_type.name))
diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c
index 36fdf5b115..19f6e06ba7 100644
--- a/tests/test-qmp-cmds.c
+++ b/tests/test-qmp-cmds.c
@@ -51,6 +51,34 @@ void qmp_test_features(FeatureStruct0 *fs0, FeatureStruct1 *fs1,
 {
 }

+void qmp_test_command_features1(Error **errp)
+{
+}
+
+void qmp_test_command_features2(Error **errp)
+{
+}
+
+void qmp_test_command_features3(Error **errp)
+{
+}
+
+void qmp_test_command_features4(Error **errp)
+{
+}
+
+void qmp_test_command_features5(Error **errp)
+{
+}
+
+void qmp_test_command_features6(Error **errp)
+{
+}
+
+void qmp_test_command_features7(Error **errp)
+{
+}
+
 UserDefTwo *qmp_user_def_cmd2(UserDefOne *ud1a,
                               bool has_udb1, UserDefOne *ud1b,
                               Error **errp)
-- 
2.21.0
Re: [PATCH v3 2/3] tests: qapi: Test 'features' of commands
Posted by Markus Armbruster 6 years, 4 months ago 
Peter Krempa <pkrempa@redhat.com> writes:

> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>  tests/qapi-schema/qapi-schema-test.json | 26 ++++++++++++++++++++++
>  tests/qapi-schema/qapi-schema-test.out  | 29 +++++++++++++++++++++++++
>  tests/qapi-schema/test-qapi.py          |  4 ++++
>  tests/test-qmp-cmds.c                   | 28 ++++++++++++++++++++++++
>  4 files changed, 87 insertions(+)

More thorough than I asked for.  I'm not complaining :)

>
> diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
> index 75c42eb0e3..220859d4c9 100644
> --- a/tests/qapi-schema/qapi-schema-test.json
> +++ b/tests/qapi-schema/qapi-schema-test.json
> @@ -290,3 +290,29 @@
>              'cfs1': 'CondFeatureStruct1',
>              'cfs2': 'CondFeatureStruct2',
>              'cfs3': 'CondFeatureStruct3' } }
> +
> +# test 'features' for command
> +
> +{ 'command': 'test-command-features1',
> +  'features': [] }
> +
> +{ 'command': 'test-command-features2',
> +  'features': [ 'feature1' ] }
> +
> +{ 'command': 'test-command-features3',
> +  'features': [ 'feature1', 'feature2' ] }
> +
> +{ 'command': 'test-command-features4',
> +  'features': [ { 'name': 'feature1', 'if': 'defined(TEST_IF_FEATURE_1)'} ] }
> +
> +{ 'command': 'test-command-features5',
> +  'features': [ { 'name': 'feature1', 'if': 'defined(TEST_IF_FEATURE_1)'},
> +                { 'name': 'feature2', 'if': 'defined(TEST_IF_FEATURE_2)'} ] }
> +
> +{ 'command': 'test-command-features6',
> +  'features': [ { 'name': 'feature1', 'if': 'defined(TEST_IF_FEATURE_1)'},
> +                { 'name': 'feature2', 'if': 'defined(TEST_IF_FEATURE_2)'} ] }

Do you need both test-command-features5 and 6?  They look the same to me...

> +
> +{ 'command': 'test-command-features7',
> +  'features': [ { 'name': 'feature1', 'if': [ 'defined(TEST_IF_COND_1)',
> +                                              'defined(TEST_IF_COND_2)'] } ] }
> diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
> index 98031da96f..a38e348d54 100644
> --- a/tests/qapi-schema/qapi-schema-test.out
> +++ b/tests/qapi-schema/qapi-schema-test.out
> @@ -412,3 +412,32 @@ object q_obj_test-features-arg
>      member cfs3: CondFeatureStruct3 optional=False
>  command test-features q_obj_test-features-arg -> None
>     gen=True success_response=True boxed=False oob=False preconfig=False
> +command test-command-features1 None -> None
> +   gen=True success_response=True boxed=False oob=False preconfig=False
> +command test-command-features2 None -> None
> +   gen=True success_response=True boxed=False oob=False preconfig=False
> +   feature feature1
> +command test-command-features3 None -> None
> +   gen=True success_response=True boxed=False oob=False preconfig=False
> +   feature feature1
> +   feature feature2
> +command test-command-features4 None -> None
> +   gen=True success_response=True boxed=False oob=False preconfig=False
> +   feature feature1
> +        if ['defined(TEST_IF_FEATURE_1)']
> +command test-command-features5 None -> None
> +   gen=True success_response=True boxed=False oob=False preconfig=False
> +   feature feature1
> +        if ['defined(TEST_IF_FEATURE_1)']
> +   feature feature2
> +        if ['defined(TEST_IF_FEATURE_2)']
> +command test-command-features6 None -> None
> +   gen=True success_response=True boxed=False oob=False preconfig=False
> +   feature feature1
> +        if ['defined(TEST_IF_FEATURE_1)']
> +   feature feature2
> +        if ['defined(TEST_IF_FEATURE_2)']
> +command test-command-features7 None -> None
> +   gen=True success_response=True boxed=False oob=False preconfig=False
> +   feature feature1
> +        if ['defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)']
> diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
> index e13c2e8671..62e65b6a7d 100755
> --- a/tests/qapi-schema/test-qapi.py
> +++ b/tests/qapi-schema/test-qapi.py
> @@ -80,6 +80,10 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor):
>          print('   gen=%s success_response=%s boxed=%s oob=%s preconfig=%s'
>                % (gen, success_response, boxed, allow_oob, allow_preconfig))
>          self._print_if(ifcond)
> +        if features:
> +            for f in features:
> +                print('   feature %s' % f.name)
> +                self._print_if(f.ifcond, 8)

Copied from visit_object_type().  Let's factor it into a @staticmethod
_print_features().

>
>      def visit_event(self, name, info, ifcond, arg_type, boxed):
>          print('event %s %s' % (name, arg_type and arg_type.name))
> diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c
> index 36fdf5b115..19f6e06ba7 100644
> --- a/tests/test-qmp-cmds.c
> +++ b/tests/test-qmp-cmds.c
> @@ -51,6 +51,34 @@ void qmp_test_features(FeatureStruct0 *fs0, FeatureStruct1 *fs1,
>  {
>  }
>
> +void qmp_test_command_features1(Error **errp)
> +{
> +}
> +
> +void qmp_test_command_features2(Error **errp)
> +{
> +}
> +
> +void qmp_test_command_features3(Error **errp)
> +{
> +}
> +
> +void qmp_test_command_features4(Error **errp)
> +{
> +}
> +
> +void qmp_test_command_features5(Error **errp)
> +{
> +}
> +
> +void qmp_test_command_features6(Error **errp)
> +{
> +}
> +
> +void qmp_test_command_features7(Error **errp)
> +{
> +}
> +
>  UserDefTwo *qmp_user_def_cmd2(UserDefOne *ud1a,
>                                bool has_udb1, UserDefOne *ud1b,
>                                Error **errp)

Any particular reason why we shouldn't squash this into PATCH 1?
Re: [PATCH v3 2/3] tests: qapi: Test 'features' of commands
Posted by Peter Krempa 6 years, 4 months ago 
On Thu, Oct 10, 2019 at 15:53:30 +0200, Markus Armbruster wrote:
> Peter Krempa <pkrempa@redhat.com> writes:
> 
> > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> > ---
> >  tests/qapi-schema/qapi-schema-test.json | 26 ++++++++++++++++++++++
> >  tests/qapi-schema/qapi-schema-test.out  | 29 +++++++++++++++++++++++++
> >  tests/qapi-schema/test-qapi.py          |  4 ++++
> >  tests/test-qmp-cmds.c                   | 28 ++++++++++++++++++++++++
> >  4 files changed, 87 insertions(+)
> 
> More thorough than I asked for.  I'm not complaining :)
> 
> >
> > diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
> > index 75c42eb0e3..220859d4c9 100644
> > --- a/tests/qapi-schema/qapi-schema-test.json
> > +++ b/tests/qapi-schema/qapi-schema-test.json
> > @@ -290,3 +290,29 @@
> >              'cfs1': 'CondFeatureStruct1',
> >              'cfs2': 'CondFeatureStruct2',
> >              'cfs3': 'CondFeatureStruct3' } }
> > +
> > +# test 'features' for command
> > +
> > +{ 'command': 'test-command-features1',
> > +  'features': [] }
> > +
> > +{ 'command': 'test-command-features2',
> > +  'features': [ 'feature1' ] }
> > +
> > +{ 'command': 'test-command-features3',
> > +  'features': [ 'feature1', 'feature2' ] }
> > +
> > +{ 'command': 'test-command-features4',
> > +  'features': [ { 'name': 'feature1', 'if': 'defined(TEST_IF_FEATURE_1)'} ] }
> > +
> > +{ 'command': 'test-command-features5',
> > +  'features': [ { 'name': 'feature1', 'if': 'defined(TEST_IF_FEATURE_1)'},
> > +                { 'name': 'feature2', 'if': 'defined(TEST_IF_FEATURE_2)'} ] }
> > +
> > +{ 'command': 'test-command-features6',
> > +  'features': [ { 'name': 'feature1', 'if': 'defined(TEST_IF_FEATURE_1)'},
> > +                { 'name': 'feature2', 'if': 'defined(TEST_IF_FEATURE_2)'} ] }
> 
> Do you need both test-command-features5 and 6?  They look the same to me...

No. It can be dropped. Looks like I mistakenly appropriated
'CondFeatureStruct2' test twice :/

> > +{ 'command': 'test-command-features7',
> > +  'features': [ { 'name': 'feature1', 'if': [ 'defined(TEST_IF_COND_1)',
> > +                                              'defined(TEST_IF_COND_2)'] } ] }
> > diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
> > index 98031da96f..a38e348d54 100644
> > --- a/tests/qapi-schema/qapi-schema-test.out
> > +++ b/tests/qapi-schema/qapi-schema-test.out
> > @@ -412,3 +412,32 @@ object q_obj_test-features-arg
> >      member cfs3: CondFeatureStruct3 optional=False
> >  command test-features q_obj_test-features-arg -> None
> >     gen=True success_response=True boxed=False oob=False preconfig=False
> > +command test-command-features1 None -> None
> > +   gen=True success_response=True boxed=False oob=False preconfig=False
> > +command test-command-features2 None -> None
> > +   gen=True success_response=True boxed=False oob=False preconfig=False
> > +   feature feature1
> > +command test-command-features3 None -> None
> > +   gen=True success_response=True boxed=False oob=False preconfig=False
> > +   feature feature1
> > +   feature feature2
> > +command test-command-features4 None -> None
> > +   gen=True success_response=True boxed=False oob=False preconfig=False
> > +   feature feature1
> > +        if ['defined(TEST_IF_FEATURE_1)']
> > +command test-command-features5 None -> None
> > +   gen=True success_response=True boxed=False oob=False preconfig=False
> > +   feature feature1
> > +        if ['defined(TEST_IF_FEATURE_1)']
> > +   feature feature2
> > +        if ['defined(TEST_IF_FEATURE_2)']
> > +command test-command-features6 None -> None
> > +   gen=True success_response=True boxed=False oob=False preconfig=False
> > +   feature feature1
> > +        if ['defined(TEST_IF_FEATURE_1)']
> > +   feature feature2
> > +        if ['defined(TEST_IF_FEATURE_2)']
> > +command test-command-features7 None -> None
> > +   gen=True success_response=True boxed=False oob=False preconfig=False
> > +   feature feature1
> > +        if ['defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)']
> > diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
> > index e13c2e8671..62e65b6a7d 100755
> > --- a/tests/qapi-schema/test-qapi.py
> > +++ b/tests/qapi-schema/test-qapi.py
> > @@ -80,6 +80,10 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor):
> >          print('   gen=%s success_response=%s boxed=%s oob=%s preconfig=%s'
> >                % (gen, success_response, boxed, allow_oob, allow_preconfig))
> >          self._print_if(ifcond)
> > +        if features:
> > +            for f in features:
> > +                print('   feature %s' % f.name)
> > +                self._print_if(f.ifcond, 8)
> 
> Copied from visit_object_type().  Let's factor it into a @staticmethod
> _print_features().

I'm not sure if that's intentional but the 'struct' and 'command'
feature printers differ in indentation level by one space. I went for
aligning it with the 'gen' line above. I thought it's for visual
separation with other possible lines.

> >      def visit_event(self, name, info, ifcond, arg_type, boxed):
> >          print('event %s %s' % (name, arg_type and arg_type.name))
> > diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c
> > index 36fdf5b115..19f6e06ba7 100644
> > --- a/tests/test-qmp-cmds.c
> > +++ b/tests/test-qmp-cmds.c
> > @@ -51,6 +51,34 @@ void qmp_test_features(FeatureStruct0 *fs0, FeatureStruct1 *fs1,
> >  {
> >  }
> >
> > +void qmp_test_command_features1(Error **errp)
> > +{
> > +}
> > +
> > +void qmp_test_command_features2(Error **errp)
> > +{
> > +}
> > +
> > +void qmp_test_command_features3(Error **errp)
> > +{
> > +}
> > +
> > +void qmp_test_command_features4(Error **errp)
> > +{
> > +}
> > +
> > +void qmp_test_command_features5(Error **errp)
> > +{
> > +}
> > +
> > +void qmp_test_command_features6(Error **errp)
> > +{
> > +}
> > +
> > +void qmp_test_command_features7(Error **errp)
> > +{
> > +}
> > +
> >  UserDefTwo *qmp_user_def_cmd2(UserDefOne *ud1a,
> >                                bool has_udb1, UserDefOne *ud1b,
> >                                Error **errp)
> 
> Any particular reason why we shouldn't squash this into PATCH 1?

Not really. I tend to prefer tests added separately and it was also done
so in case of features for 'structs' so I went with that approach. Said
that I'm perfectly fine with squashing them.
Re: [PATCH v3 2/3] tests: qapi: Test 'features' of commands
Posted by Markus Armbruster 6 years, 4 months ago 
Peter Krempa <pkrempa@redhat.com> writes:

> On Thu, Oct 10, 2019 at 15:53:30 +0200, Markus Armbruster wrote:
>> Peter Krempa <pkrempa@redhat.com> writes:
>> 
>> > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>> > ---
>> >  tests/qapi-schema/qapi-schema-test.json | 26 ++++++++++++++++++++++
>> >  tests/qapi-schema/qapi-schema-test.out  | 29 +++++++++++++++++++++++++
>> >  tests/qapi-schema/test-qapi.py          |  4 ++++
>> >  tests/test-qmp-cmds.c                   | 28 ++++++++++++++++++++++++
>> >  4 files changed, 87 insertions(+)
>> 
>> More thorough than I asked for.  I'm not complaining :)
>> 
>> >
>> > diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
>> > index 75c42eb0e3..220859d4c9 100644
>> > --- a/tests/qapi-schema/qapi-schema-test.json
>> > +++ b/tests/qapi-schema/qapi-schema-test.json
>> > @@ -290,3 +290,29 @@
>> >              'cfs1': 'CondFeatureStruct1',
>> >              'cfs2': 'CondFeatureStruct2',
>> >              'cfs3': 'CondFeatureStruct3' } }
>> > +
>> > +# test 'features' for command
>> > +
>> > +{ 'command': 'test-command-features1',
>> > +  'features': [] }
>> > +
>> > +{ 'command': 'test-command-features2',
>> > +  'features': [ 'feature1' ] }
>> > +
>> > +{ 'command': 'test-command-features3',
>> > +  'features': [ 'feature1', 'feature2' ] }
>> > +
>> > +{ 'command': 'test-command-features4',
>> > +  'features': [ { 'name': 'feature1', 'if': 'defined(TEST_IF_FEATURE_1)'} ] }
>> > +
>> > +{ 'command': 'test-command-features5',
>> > +  'features': [ { 'name': 'feature1', 'if': 'defined(TEST_IF_FEATURE_1)'},
>> > +                { 'name': 'feature2', 'if': 'defined(TEST_IF_FEATURE_2)'} ] }
>> > +
>> > +{ 'command': 'test-command-features6',
>> > +  'features': [ { 'name': 'feature1', 'if': 'defined(TEST_IF_FEATURE_1)'},
>> > +                { 'name': 'feature2', 'if': 'defined(TEST_IF_FEATURE_2)'} ] }
>> 
>> Do you need both test-command-features5 and 6?  They look the same to me...
>
> No. It can be dropped. Looks like I mistakenly appropriated
> 'CondFeatureStruct2' test twice :/

Easy enough to fix :)

>> > +{ 'command': 'test-command-features7',
>> > +  'features': [ { 'name': 'feature1', 'if': [ 'defined(TEST_IF_COND_1)',
>> > +                                              'defined(TEST_IF_COND_2)'] } ] }
>> > diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
>> > index 98031da96f..a38e348d54 100644
>> > --- a/tests/qapi-schema/qapi-schema-test.out
>> > +++ b/tests/qapi-schema/qapi-schema-test.out
>> > @@ -412,3 +412,32 @@ object q_obj_test-features-arg
>> >      member cfs3: CondFeatureStruct3 optional=False
>> >  command test-features q_obj_test-features-arg -> None
>> >     gen=True success_response=True boxed=False oob=False preconfig=False
>> > +command test-command-features1 None -> None
>> > +   gen=True success_response=True boxed=False oob=False preconfig=False
>> > +command test-command-features2 None -> None
>> > +   gen=True success_response=True boxed=False oob=False preconfig=False
>> > +   feature feature1
>> > +command test-command-features3 None -> None
>> > +   gen=True success_response=True boxed=False oob=False preconfig=False
>> > +   feature feature1
>> > +   feature feature2
>> > +command test-command-features4 None -> None
>> > +   gen=True success_response=True boxed=False oob=False preconfig=False
>> > +   feature feature1
>> > +        if ['defined(TEST_IF_FEATURE_1)']
>> > +command test-command-features5 None -> None
>> > +   gen=True success_response=True boxed=False oob=False preconfig=False
>> > +   feature feature1
>> > +        if ['defined(TEST_IF_FEATURE_1)']
>> > +   feature feature2
>> > +        if ['defined(TEST_IF_FEATURE_2)']
>> > +command test-command-features6 None -> None
>> > +   gen=True success_response=True boxed=False oob=False preconfig=False
>> > +   feature feature1
>> > +        if ['defined(TEST_IF_FEATURE_1)']
>> > +   feature feature2
>> > +        if ['defined(TEST_IF_FEATURE_2)']
>> > +command test-command-features7 None -> None
>> > +   gen=True success_response=True boxed=False oob=False preconfig=False
>> > +   feature feature1
>> > +        if ['defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)']
>> > diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
>> > index e13c2e8671..62e65b6a7d 100755
>> > --- a/tests/qapi-schema/test-qapi.py
>> > +++ b/tests/qapi-schema/test-qapi.py
>> > @@ -80,6 +80,10 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor):
>> >          print('   gen=%s success_response=%s boxed=%s oob=%s preconfig=%s'
>> >                % (gen, success_response, boxed, allow_oob, allow_preconfig))
>> >          self._print_if(ifcond)
>> > +        if features:
>> > +            for f in features:
>> > +                print('   feature %s' % f.name)
>> > +                self._print_if(f.ifcond, 8)
>> 
>> Copied from visit_object_type().  Let's factor it into a @staticmethod
>> _print_features().
>
> I'm not sure if that's intentional but the 'struct' and 'command'
> feature printers differ in indentation level by one space. I went for
> aligning it with the 'gen' line above. I thought it's for visual
> separation with other possible lines.

You're right.  I think I simply messed up the spacing in commit
156402e5042.  I'd like to clean it up.

I think it's easiest if I do the touch-ups here and in PATCH 1 (they're
all straightforward).  I'll post them as v4 so you can give them a quick
eye-over.

>> >      def visit_event(self, name, info, ifcond, arg_type, boxed):
>> >          print('event %s %s' % (name, arg_type and arg_type.name))
>> > diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c
>> > index 36fdf5b115..19f6e06ba7 100644
>> > --- a/tests/test-qmp-cmds.c
>> > +++ b/tests/test-qmp-cmds.c
>> > @@ -51,6 +51,34 @@ void qmp_test_features(FeatureStruct0 *fs0, FeatureStruct1 *fs1,
>> >  {
>> >  }
>> >
>> > +void qmp_test_command_features1(Error **errp)
>> > +{
>> > +}
>> > +
>> > +void qmp_test_command_features2(Error **errp)
>> > +{
>> > +}
>> > +
>> > +void qmp_test_command_features3(Error **errp)
>> > +{
>> > +}
>> > +
>> > +void qmp_test_command_features4(Error **errp)
>> > +{
>> > +}
>> > +
>> > +void qmp_test_command_features5(Error **errp)
>> > +{
>> > +}
>> > +
>> > +void qmp_test_command_features6(Error **errp)
>> > +{
>> > +}
>> > +
>> > +void qmp_test_command_features7(Error **errp)
>> > +{
>> > +}
>> > +
>> >  UserDefTwo *qmp_user_def_cmd2(UserDefOne *ud1a,
>> >                                bool has_udb1, UserDefOne *ud1b,
>> >                                Error **errp)
>> 
>> Any particular reason why we shouldn't squash this into PATCH 1?
>
> Not really. I tend to prefer tests added separately and it was also done
> so in case of features for 'structs' so I went with that approach. Said
> that I'm perfectly fine with squashing them.

Okay.

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.