[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] QemuOpt: add unit tests
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v2] QemuOpt: add unit tests |
Date: |
Fri, 21 Mar 2014 08:37:40 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.3.0 |
On 03/17/2014 05:10 PM, Leandro Dorileo wrote:
> Cover basic aspects and API usage for QemuOpt. The current implementation
> covers the API's planned to be changed by Chunyan Liu in his
> QEMUOptionParameter
> replacement/cleanup job.
>
> Other APIs should be covered in future improvements.
>
> Signed-off-by: Leandro Dorileo <address@hidden>
Right here is where you should stick a --- marker.
>
> Changes:
> v2:
> + fixed comments;
> + make use of g_assert_cmpstr();
> + use error_abort instead of a local_err for qemu_opts_absorb_qdict();
> + asserts on QemuOptsList (empty and list name);
> + added test_qemu_opt_unset();
> + asserts on qemu_opt_*_set() return;
> + added test_qemu_opts_reset();
> + added test_qemu_opts_set();
> ---
It's okay to have a duplicate one; but the main point is that the v2
changelog is useful to reviewers but not to the git log; and anything
after the --- marker gets omitted by 'git am' when a maintainer accepts
your patch into their pull request.
> tests/Makefile | 3 +
> tests/test-qemu-opts.c | 455
> +++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 458 insertions(+)
> create mode 100644 tests/test-qemu-opts.c
>
> +static void test_qemu_find_opts(void)
> +{
> + QemuOptsList *list;
> +
> + register_opts();
> +
> + /* we have an "opts_list_01" option, should return it */
> + list = qemu_find_opts("opts_list_01");
> + g_assert(list != NULL);
> + g_assert_cmpstr(list->name, ==, "opts_list_01");
> +}
> +
> +static void test_qemu_opts_create(void)
> +{
> + QemuOptsList *list;
> + QemuOpts *opts;
> +
> + register_opts();
> +
> + list = qemu_find_opts("opts_list_01");
> + g_assert(list != NULL);
> + g_assert(QTAILQ_EMPTY(&list->head));
> + g_assert_cmpstr(list->name, ==, "opts_list_01");
This test starts as a duplicate of test_qemu_find_opts; I guess that's
okay (having both tests means a bit more insight into what broke if one
fails but the other passes).
> +
> +static void test_qemu_opt_unset(void)
> +{
> + QemuOpts *opts;
> + const char *value;
> + int ret;
> +
> + /* dinamically initialized (parsed) opts */
s/dinamically/dynamically/
That's trivially fixable, so feel free to add:
Reviewed-by: Eric Blake <address@hidden>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature