From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35909) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WPaMK-0008RJ-OD for qemu-devel@nongnu.org; Mon, 17 Mar 2014 12:25:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WPaMF-000075-MW for qemu-devel@nongnu.org; Mon, 17 Mar 2014 12:25:52 -0400 Received: from mail-ob0-f169.google.com ([209.85.214.169]:65148) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WPaMF-00006x-Hc for qemu-devel@nongnu.org; Mon, 17 Mar 2014 12:25:47 -0400 Received: by mail-ob0-f169.google.com with SMTP id va2so5731903obc.14 for ; Mon, 17 Mar 2014 09:25:46 -0700 (PDT) Date: Mon, 17 Mar 2014 16:24:36 +0000 From: Leandro Dorileo Message-ID: <20140317162436.GA24576@dorilex> References: <1395004716-20229-1-git-send-email-l@dorileo.org> <53271FCC.6010504@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <53271FCC.6010504@redhat.com> Subject: Re: [Qemu-devel] [PATCH] QemuOpt: add unit tests List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: Wenchao Xia , Stefan Hajnoczi , Markus Armbruster , qemu-devel@nongnu.org, Anthony Liguori , Chunyan Liu , Andreas =?iso-8859-1?Q?F=E4rber?= On Mon, Mar 17, 2014 at 10:16:12AM -0600, Eric Blake wrote: > On 03/16/2014 03:18 PM, Leandro Dorileo wrote: > > Cover basic aspects and API usage for QemuOpt. The current implementation > > covers the API's planed to be changed by Chunyan Liu in his QEMUOptionParameter > > s/planed/planned/ > > > replacement/cleanup job. > > > > Other APIs should be covered in future improvements. > > > > Signed-off-by: Leandro Dorileo > > --- > > tests/Makefile | 3 + > > tests/test-qemu-opts.c | 309 +++++++++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 312 insertions(+) > > create mode 100644 tests/test-qemu-opts.c > > > > > + > > +static void test_find_unknown_opts(void) > > +{ > > + QemuOptsList *list; > > + > > + register_opts(); > > + > > + /** should not return anything, we don't known "unknown" option */ > > s/known/have an/ ok. > > Here, and throughout the file: /** */ comments are special to doc > markup, and should only be needed prior to declaring a function. Within > a function body, use the simpler /* */ comment. > > > +static void test_qemu_find_opts(void) > > +{ > > + QemuOptsList *list; > > + > > + register_opts(); > > + > > + /** we have an rtc option, should return it */ > > + list = qemu_find_opts("opts_list_01"); > > + g_assert(list != NULL); > > Should you do any further testing on the contents of the returned list? ok. I'll see the API. > > > +static void test_qemu_opt_get(void) > > +{ > > > + /** now we have set str2, should know about it */ > > + opt = qemu_opt_get(opts, "str2"); > > + g_assert(opt != NULL && !strcmp(opt, "value")); > > Isn't g_assert_cmpstr nicer for this, as in: > > g_assert_cmpstr(opt, ==, "value"); Yep, sure, I wasn't awere of g_assert_cmpstr, thanks. > > > +static void test_qemu_opt_get_size(void) > > +{ > > + QemuOptsList *list; > > > + > > + qemu_opts_absorb_qdict(opts, dict, &local_err); > > + g_assert(local_err == NULL); > > shorter as: > > qemu_opts_absorb_qdict(opts, dict, &error_abort); I see. > > Overall, I like where this is headed; looking forward to v2. > Thanks for reviewing... -- Leandro Dorileo