From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39720) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YbD7Z-0001O0-Pv for qemu-devel@nongnu.org; Thu, 26 Mar 2015 15:07:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YbD7W-0003Ln-HD for qemu-devel@nongnu.org; Thu, 26 Mar 2015 15:07:13 -0400 Received: from mx1.redhat.com ([209.132.183.28]:53958) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YbD7W-0003Ld-9x for qemu-devel@nongnu.org; Thu, 26 Mar 2015 15:07:10 -0400 Message-ID: <551458DC.9050701@redhat.com> Date: Thu, 26 Mar 2015 13:07:08 -0600 From: Eric Blake MIME-Version: 1.0 References: <1427227433-5030-1-git-send-email-eblake@redhat.com> <1427227433-5030-20-git-send-email-eblake@redhat.com> <87lhijwsgw.fsf@blackfin.pond.sub.org> In-Reply-To: <87lhijwsgw.fsf@blackfin.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="nBBxU4ndlmiAUFHG1CBntVj3evXNIf8g2" Subject: Re: [Qemu-devel] [PATCH v5 19/28] qapi: Add some type check tests List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: kwolf@redhat.com, lcapitulino@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, wenchaoqemu@gmail.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --nBBxU4ndlmiAUFHG1CBntVj3evXNIf8g2 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 03/26/2015 11:58 AM, Markus Armbruster wrote: > Eric Blake writes: >=20 >> 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 >> --- > [...] >> diff --git a/tests/qapi-schema/data-array-empty.err b/tests/qapi-schem= a/data-array-empty.err >> new file mode 100644 >> index 0000000..e69de29 >> diff --git a/tests/qapi-schema/data-array-empty.exit b/tests/qapi-sche= ma/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-sche= ma/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': [ ] } } >=20 > v4 tested >=20 > { 'command': 'oops', 'data': [ ] } >=20 > 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' ] } >=20 > 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 fo= r > my taste. >=20 > You could use the commit message to prepare the reader. >=20 > 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. >=20 >> diff --git a/tests/qapi-schema/returns-whitelist.out b/tests/qapi-sche= ma/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', OrderedD= ict([('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'), ('retu= rns', ['int'])])] >> +[{'enum_name': 'TpmModel', 'enum_values': ['tpm-tis']}] >> +[] >=20 > Since I found nothing that's actually wrong: >=20 > Reviewed-by: Markus Armbruster >=20 >=20 --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --nBBxU4ndlmiAUFHG1CBntVj3evXNIf8g2 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJVFFjcAAoJEKeha0olJ0NqCl4IAKkce8nfAAdEJwpyBaLrRWb4 hXZNSu09cALOpCJg76WZf8NU5W3Fq+Wf2rckeKdeyVE50bo8cO0l8twj2UnSC2fQ 2NXoBCW5qNo67gRqu6gVMy0QgeP3EDeZSs2aFsFTBtIoqNk1b4CI2+T3saMIdNeI qWW1YavDN49bwfMaIo76G+sYtCXJEqzpw8W+rzpkfhOCAnlki/zHFaF+bMcwq2H5 1ndywPP4R6WzE8wHoeHoJcmHHHr9pBY5goK202jl9nCdeA2GeYs89LjF2qu3bweu yYDXdSYERzGKWrSGLC4u/ZywJ9XqejXWgtffJdKq5XeW4v55K09vHHvcPlwb7nc= =+M9X -----END PGP SIGNATURE----- --nBBxU4ndlmiAUFHG1CBntVj3evXNIf8g2--