From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54827) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZpIFw-00072b-OF for qemu-devel@nongnu.org; Thu, 22 Oct 2015 11:58:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZpIFs-0001MU-Hx for qemu-devel@nongnu.org; Thu, 22 Oct 2015 11:58:20 -0400 Received: from mx1.redhat.com ([209.132.183.28]:51636) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZpIFs-0001MN-Cy for qemu-devel@nongnu.org; Thu, 22 Oct 2015 11:58:16 -0400 From: Markus Armbruster References: <1445056549-7815-1-git-send-email-eblake@redhat.com> <1445056549-7815-2-git-send-email-eblake@redhat.com> Date: Thu, 22 Oct 2015 17:58:13 +0200 In-Reply-To: <1445056549-7815-2-git-send-email-eblake@redhat.com> (Eric Blake's message of "Fri, 16 Oct 2015 22:35:36 -0600") Message-ID: <87fv12uc8a.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v7 01/14] qapi: Use generated TestStruct machinery in tests List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: qemu-devel@nongnu.org, Michael Roth Eric Blake writes: > Commit d88f5fd and friends first introduced the various test-qmp-* > tests in 2011, with duplicated hand-rolled TestStruct machinery, > to make sure the qapi visitor interface was tested. Later, commit > 4f193e3 in 2013 added a .json file for further testing use by the > files, but without consolidating any of the existing hand-rolled > visitors. And with four copies, subtle differences have crept in. The only one between the hand-rolled copies I can see apart from whitespace is tests/test-visitor-serialization.c passing NULL rather than "TestStruct" to visit_start_struct(). Compared to the generated version, the hand-rolled copies neglect to skip visiting members when !*obj. If I remember correctly, we do that so we can use the visitor to deallocate partly allocated stuff (worth a comment, by the way), and that's not necessary in these tests. > Of course, just because the visitor interface is tested does not > mean it is a sane interface; and future patches will be changing > some of the visitor contracts. Rather than having to duplicate > the cleanup work in each copy of the TestStruct visitor, and keep > each hand-rolled copy in sync with what the generator supplies, we > might as well just test what the generator should give us in the > first place. > > Signed-off-by: Eric Blake Patch looks good.