On 03/26/2015 11:58 AM, Markus Armbruster wrote:
> Eric Blake <ebl...@redhat.com> writes:
>
>> Demonstrate that the qapi generator silently parses confusing
>> types, which may cause other errors later on. Later patches
>> will update the expected results as the generator is made stricter.
>>
>> Signed-off-by: Eric Blake <ebl...@redhat.com>
>> ---
> [...]
>> diff --git a/tests/qapi-schema/data-array-empty.err
>> b/tests/qapi-schema/data-array-empty.err
>> new file mode 100644
>> index 0000000..e69de29
>> diff --git a/tests/qapi-schema/data-array-empty.exit
>> b/tests/qapi-schema/data-array-empty.exit
>> new file mode 100644
>> index 0000000..573541a
>> --- /dev/null
>> +++ b/tests/qapi-schema/data-array-empty.exit
>> @@ -0,0 +1 @@
>> +0
>> diff --git a/tests/qapi-schema/data-array-empty.json
>> b/tests/qapi-schema/data-array-empty.json
>> new file mode 100644
>> index 0000000..edb543a
>> --- /dev/null
>> +++ b/tests/qapi-schema/data-array-empty.json
>> @@ -0,0 +1,2 @@
>> +# FIXME: we should reject an array for data if it does not contain a known
>> type
>> +{ 'command': 'oops', 'data': { 'empty': [ ] } }
>
> v4 tested
>
> { 'command': 'oops', 'data': [ ] }
>
> Is that still covered somewhere else?
Probably not. Sounds like a good test case to add, presumably on top if
I don't have to respin.
>> +
>> +{ 'command': 'no-way-this-will-get-whitelisted',
>> + 'returns': [ 'int' ] }
>
> I like the pattern "add tests to demonstrate issues, then code to
> address the issues". But this test appears too much out of the blue for
> my taste.
>
> You could use the commit message to prepare the reader.
>
> Or you could deviate from the pattern and add this test together with
> the actual whitelist. That's what I'd try.
Of course, if I do need to respin, I can shuffle and split patches as
needed to make the resubmission prettier.
>
>> diff --git a/tests/qapi-schema/returns-whitelist.out
>> b/tests/qapi-schema/returns-whitelist.out
>> new file mode 100644
>> index 0000000..2adcd8b
>> --- /dev/null
>> +++ b/tests/qapi-schema/returns-whitelist.out
>> @@ -0,0 +1,7 @@
>> +[OrderedDict([('command', 'human-monitor-command'), ('data',
>> OrderedDict([('command-line', 'str'), ('*cpu-index', 'int')])), ('returns',
>> 'str')]),
>> + OrderedDict([('enum', 'TpmModel'), ('data', ['tpm-tis'])]),
>> + OrderedDict([('command', 'query-tpm-models'), ('returns', ['TpmModel'])]),
>> + OrderedDict([('command', 'guest-get-time'), ('returns', 'int')]),
>> + OrderedDict([('command', 'no-way-this-will-get-whitelisted'), ('returns',
>> ['int'])])]
>> +[{'enum_name': 'TpmModel', 'enum_values': ['tpm-tis']}]
>> +[]
>
> Since I found nothing that's actually wrong:
>
> Reviewed-by: Markus Armbruster <arm...@redhat.com>
>
>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
