Re: [Qemu-devel] [PATCH 1/9] tests: QAPI schema parser tests

[Eric Blake]

On 07/26/2013 08:16 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
> 
>> On 07/26/2013 06:39 AM, Markus Armbruster wrote:
>>> The parser handles erroneous input badly.  To be improved shortly.
>>>
>>> Signed-off-by: Markus Armbruster <address@hidden>
>>> ---
>>
>> Lots of proof on how bad it is!  I'd also like to see a couple tests on
>> trailing commas:
>>
>> { 'enum': 'Foo', [ 'bar' ], }
>> { 'enum': 'Gur', [ 'ble', ] }
> 
> I figure you mean
> 
> { 'enum': 'Foo', 'data': [ 'bar' ], }
> { 'enum': 'Gur', 'data': [ 'ble', ] }

Yep, you got my intent, even if I didn't type it right.

> 
> My parser rejects both:
> 
> <stdin>:1:37: Expected string
> <stdin>:2:35: Expected "{", "[" or string

Good!

> 
> I commented out the first to get the second error.  Making the parser
> continue after errors didn't seem to be worthwhile.

Agree about not continuing after errors; once the file is clean, anyone
adding a new command will have errors in at most the one command they
are trying to add, and even if it is an iterative approach to get them
to find all the problems, it's a lot easier to have that one developer
fix their work than trying to bake error recovery into the parser.

If you do add these two test cases (which I recommend), it should indeed
be two separate tests.

Another thought - is it worth testing other valid JSON but invalid
schema constructs, such as:

{ 'enum': 1, 'data': false }

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature