[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] QemuOpt: add unit tests
From: |
Leandro Dorileo |
Subject: |
Re: [Qemu-devel] [PATCH] QemuOpt: add unit tests |
Date: |
Mon, 17 Mar 2014 16:24:36 +0000 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
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 <address@hidden>
> > ---
> > 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