From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57475) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V2jzP-0000e1-JM for qemu-devel@nongnu.org; Fri, 26 Jul 2013 11:31:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V2jzK-0003gd-Pr for qemu-devel@nongnu.org; Fri, 26 Jul 2013 11:31:31 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54660) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V2jzJ-0003fA-Pm for qemu-devel@nongnu.org; Fri, 26 Jul 2013 11:31:26 -0400 From: Markus Armbruster References: <1374842387-17146-1-git-send-email-armbru@redhat.com> <1374842387-17146-2-git-send-email-armbru@redhat.com> <51F27021.60905@redhat.com> <87bo5pl83s.fsf@blackfin.pond.sub.org> <51F28E44.2060701@redhat.com> Date: Fri, 26 Jul 2013 17:31:11 +0200 In-Reply-To: <51F28E44.2060701@redhat.com> (Eric Blake's message of "Fri, 26 Jul 2013 08:57:08 -0600") Message-ID: <87k3kd72yo.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH 1/9] tests: QAPI schema parser tests List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: aliguori@us.ibm.com, akong@redhat.com, qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com Eric Blake writes: > On 07/26/2013 08:16 AM, Markus Armbruster wrote: >> Eric Blake 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 >>>> --- >>> >>> 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: >> >> :1:37: Expected string >> :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. Could be done on top. If I need to respin anyway, I'll add them in the first patch, of course. > Another thought - is it worth testing other valid JSON but invalid > schema constructs, such as: > > { 'enum': 1, 'data': false } Maybe, but I limited myself just to the parser in this series.