diff mbox series

[v2,2/6] tests/qapi-schema: Test for good feature lists in structs

Message ID 20190517144232.18965-3-kwolf@redhat.com (mailing list archive)
State New, archived
Headers show
Series file-posix: Add dynamic-auto-read-only QAPI feature | expand

Commit Message

Kevin Wolf May 17, 2019, 2:42 p.m. UTC
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qapi-schema/qapi-schema-test.json | 30 +++++++++++++++++++++++
 tests/qapi-schema/qapi-schema-test.out  | 32 +++++++++++++++++++++++++
 tests/qapi-schema/test-qapi.py          |  4 ++++
 3 files changed, 66 insertions(+)

Comments

Markus Armbruster May 24, 2019, 1:29 p.m. UTC | #1
Kevin Wolf <kwolf@redhat.com> writes:

> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  tests/qapi-schema/qapi-schema-test.json | 30 +++++++++++++++++++++++
>  tests/qapi-schema/qapi-schema-test.out  | 32 +++++++++++++++++++++++++
>  tests/qapi-schema/test-qapi.py          |  4 ++++
>  3 files changed, 66 insertions(+)
>
> diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
> index 0952c68734..35a50fbe54 100644
> --- a/tests/qapi-schema/qapi-schema-test.json
> +++ b/tests/qapi-schema/qapi-schema-test.json
> @@ -242,3 +242,33 @@
>    { 'foo': 'TestIfStruct',
>      'bar': { 'type': ['TestIfEnum'], 'if': 'defined(TEST_IF_EVT_BAR)' } },
>    'if': 'defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)' }
> +
> +# test 'features' for structs
> +
> +{ 'struct': 'FeatureStruct0',
> +  'data': { 'foo': 'int' },
> +  'features': [] }
> +{ 'struct': 'FeatureStruct1',
> +  'data': { 'foo': 'int' },
> +  'features': [ 'feature1' ] }
> +{ 'struct': 'FeatureStruct2',
> +  'data': { 'foo': 'int' },
> +  'features': [ { 'name': 'feature1' } ] }
> +{ 'struct': 'FeatureStruct3',
> +  'data': { 'foo': 'int' },
> +  'features': [ 'feature1', 'feature2' ] }
> +{ 'struct': 'FeatureStruct4',
> +  'data': { 'namespace-test': 'int' },
> +  'features': [ 'namespace-test', 'int', 'name', 'if' ] }
> +
> +{ 'struct': 'CondFeatureStruct1',
> +  'data': { 'foo': 'int' },
> +  'features': [ { 'name': 'feature1', 'if': 'defined(TEST_IF_FEATURE_1)'} ] }
> +{ 'struct': 'CondFeatureStruct2',
> +  'data': { 'foo': 'int' },
> +  'features': [ { 'name': 'feature1', 'if': 'defined(TEST_IF_FEATURE_1)'},
> +                { 'name': 'feature2', 'if': 'defined(TEST_IF_FEATURE_2)'} ] }
> +{ 'struct': 'CondFeatureStruct3',
> +  'data': { 'foo': 'int' },
> +  'features': [ { 'name': 'feature1', 'if': [ 'defined(TEST_IF_COND_1)',
> +                                              'defined(TEST_IF_COND_2)'] } ] }

Let's add

   { 'command': 'test-features',
     'data': { 'fs0': 'FeatureStruct0',
               'fs1': 'FeatureStruct1',
               'fs2': 'FeatureStruct2',
               'fs3': 'FeatureStruct3',
               'cfs1': 'CondFeatureStruct1',
               'cfs2': 'CondFeatureStruct2',
               'cfs3': 'CondFeatureStruct3' } }

because without it, the feature test cases won't generate introspection
code.

[...]
Kevin Wolf May 29, 2019, 10:09 p.m. UTC | #2
Am 24.05.2019 um 15:29 hat Markus Armbruster geschrieben:
> Let's add
> 
>    { 'command': 'test-features',
>      'data': { 'fs0': 'FeatureStruct0',
>                'fs1': 'FeatureStruct1',
>                'fs2': 'FeatureStruct2',
>                'fs3': 'FeatureStruct3',
>                'cfs1': 'CondFeatureStruct1',
>                'cfs2': 'CondFeatureStruct2',
>                'cfs3': 'CondFeatureStruct3' } }
> 
> because without it, the feature test cases won't generate introspection
> code.

Of course, like everything else you requested, I'll just do this to get
the series off my table, but I'm still curious: Where would
introspection code ever be generated for the test cases? I saw neither
test code that generates the source files nor reference output that it
would be compared against.

Kevin
Markus Armbruster June 3, 2019, 6:35 a.m. UTC | #3
Kevin Wolf <kwolf@redhat.com> writes:

> Am 24.05.2019 um 15:29 hat Markus Armbruster geschrieben:
>> Let's add
>> 
>>    { 'command': 'test-features',
>>      'data': { 'fs0': 'FeatureStruct0',
>>                'fs1': 'FeatureStruct1',
>>                'fs2': 'FeatureStruct2',
>>                'fs3': 'FeatureStruct3',
>>                'cfs1': 'CondFeatureStruct1',
>>                'cfs2': 'CondFeatureStruct2',
>>                'cfs3': 'CondFeatureStruct3' } }
>> 
>> because without it, the feature test cases won't generate introspection
>> code.
>
> Of course, like everything else you requested, I'll just do this to get
> the series off my table, but I'm still curious: Where would
> introspection code ever be generated for the test cases? I saw neither
> test code that generates the source files nor reference output that it
> would be compared against.

Asking me to explain why I want something done when you can't see it
yourself is much, much better than blindly implementing it.

Makefile.include feeds the two positive tests qapi-schema-test.json and
doc-good.json to qapi-gen.py.

The .o for the former's .c get linked into a bunch of tests via Make
variable $(test-qapi-obj-y).  One of them is test-qobject-input-visitor.
Its test case "/visitor/input/qapi-introspect" checks the generated
QObject conforms to the schema.

qapi-schema.json gets tested end-to-end instead: qmp-cmd-tests tests
query-qmp-schema.

Both tests only check schema conformance, they don't compare to expected
output.  Perhaps they should.  But I can still diff the generated
qmp-introspect.c manually, which I routinely do when messing with the
generator.

Makes sense?
Kevin Wolf June 3, 2019, 7:36 a.m. UTC | #4
Am 03.06.2019 um 08:35 hat Markus Armbruster geschrieben:
> Kevin Wolf <kwolf@redhat.com> writes:
> 
> > Am 24.05.2019 um 15:29 hat Markus Armbruster geschrieben:
> >> Let's add
> >> 
> >>    { 'command': 'test-features',
> >>      'data': { 'fs0': 'FeatureStruct0',
> >>                'fs1': 'FeatureStruct1',
> >>                'fs2': 'FeatureStruct2',
> >>                'fs3': 'FeatureStruct3',
> >>                'cfs1': 'CondFeatureStruct1',
> >>                'cfs2': 'CondFeatureStruct2',
> >>                'cfs3': 'CondFeatureStruct3' } }
> >> 
> >> because without it, the feature test cases won't generate introspection
> >> code.
> >
> > Of course, like everything else you requested, I'll just do this to get
> > the series off my table, but I'm still curious: Where would
> > introspection code ever be generated for the test cases? I saw neither
> > test code that generates the source files nor reference output that it
> > would be compared against.
> 
> Asking me to explain why I want something done when you can't see it
> yourself is much, much better than blindly implementing it.

Well, adding a command can't hurt anyway, so doing both in parallel
seemed like a good option. :-)

> Makefile.include feeds the two positive tests qapi-schema-test.json and
> doc-good.json to qapi-gen.py.
> 
> The .o for the former's .c get linked into a bunch of tests via Make
> variable $(test-qapi-obj-y).  One of them is test-qobject-input-visitor.
> Its test case "/visitor/input/qapi-introspect" checks the generated
> QObject conforms to the schema.
> 
> qapi-schema.json gets tested end-to-end instead: qmp-cmd-tests tests
> query-qmp-schema.
> 
> Both tests only check schema conformance, they don't compare to expected
> output.  Perhaps they should.  But I can still diff the generated
> qmp-introspect.c manually, which I routinely do when messing with the
> generator.
> 
> Makes sense?

Ah, so basically we don't fully check the correctness of the
introspection output, but just that two different generated files are
consistent with each other. Yes, makes sense.

I just didn't realise that the generated code is actually compiled and
linked to some other test cases. My expectation was the schema testing
was only in tests/qapi-schema/.

Kevin
diff mbox series

Patch

diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 0952c68734..35a50fbe54 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -242,3 +242,33 @@ 
   { 'foo': 'TestIfStruct',
     'bar': { 'type': ['TestIfEnum'], 'if': 'defined(TEST_IF_EVT_BAR)' } },
   'if': 'defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)' }
+
+# test 'features' for structs
+
+{ 'struct': 'FeatureStruct0',
+  'data': { 'foo': 'int' },
+  'features': [] }
+{ 'struct': 'FeatureStruct1',
+  'data': { 'foo': 'int' },
+  'features': [ 'feature1' ] }
+{ 'struct': 'FeatureStruct2',
+  'data': { 'foo': 'int' },
+  'features': [ { 'name': 'feature1' } ] }
+{ 'struct': 'FeatureStruct3',
+  'data': { 'foo': 'int' },
+  'features': [ 'feature1', 'feature2' ] }
+{ 'struct': 'FeatureStruct4',
+  'data': { 'namespace-test': 'int' },
+  'features': [ 'namespace-test', 'int', 'name', 'if' ] }
+
+{ 'struct': 'CondFeatureStruct1',
+  'data': { 'foo': 'int' },
+  'features': [ { 'name': 'feature1', 'if': 'defined(TEST_IF_FEATURE_1)'} ] }
+{ 'struct': 'CondFeatureStruct2',
+  'data': { 'foo': 'int' },
+  'features': [ { 'name': 'feature1', 'if': 'defined(TEST_IF_FEATURE_1)'},
+                { 'name': 'feature2', 'if': 'defined(TEST_IF_FEATURE_2)'} ] }
+{ 'struct': 'CondFeatureStruct3',
+  'data': { 'foo': 'int' },
+  '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 77fb1e1aa9..54255e5708 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -354,3 +354,35 @@  object q_obj_TestIfEvent-arg
 event TestIfEvent q_obj_TestIfEvent-arg
    boxed=False
     if ['defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)']
+object FeatureStruct0
+    member foo: int optional=False
+object FeatureStruct1
+    member foo: int optional=False
+    feature feature1
+object FeatureStruct2
+    member foo: int optional=False
+    feature feature1
+object FeatureStruct3
+    member foo: int optional=False
+    feature feature1
+    feature feature2
+object FeatureStruct4
+    member namespace-test: int optional=False
+    feature namespace-test
+    feature int
+    feature name
+    feature if
+object CondFeatureStruct1
+    member foo: int optional=False
+    feature feature1
+        if ['defined(TEST_IF_FEATURE_1)']
+object CondFeatureStruct2
+    member foo: int optional=False
+    feature feature1
+        if ['defined(TEST_IF_FEATURE_1)']
+    feature feature2
+        if ['defined(TEST_IF_FEATURE_2)']
+object CondFeatureStruct3
+    member foo: int optional=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 f2d6815c86..b0f770b9bd 100644
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -49,6 +49,10 @@  class QAPISchemaTestVisitor(QAPISchemaVisitor):
             self._print_if(m.ifcond, 8)
         self._print_variants(variants)
         self._print_if(ifcond)
+        if features:
+            for f in features:
+                print('    feature %s' % f.name)
+                self._print_if(f.ifcond, 8)
 
     def visit_alternate_type(self, name, info, ifcond, variants):
         print('alternate %s' % name)