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 <l...@dorileo.org>
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 <ebl...@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
