Re: [PATCH V4 4/5] util: strList unit tests

[Markus Armbruster <armbru@redhat.com>]

Steve Sistare <steven.sistare@oracle.com> writes:

> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  tests/unit/meson.build    |  1 +
>  tests/unit/test-strList.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 81 insertions(+)
>  create mode 100644 tests/unit/test-strList.c
>
> diff --git a/tests/unit/meson.build b/tests/unit/meson.build
> index 69f9c05..113d12e 100644
> --- a/tests/unit/meson.build
> +++ b/tests/unit/meson.build
> @@ -35,6 +35,7 @@ tests = {
>    'test-rcu-simpleq': [],
>    'test-rcu-tailq': [],
>    'test-rcu-slist': [],
> +  'test-strList': [],
>    'test-qdist': [],
>    'test-qht': [],
>    'test-qtree': [],
> diff --git a/tests/unit/test-strList.c b/tests/unit/test-strList.c
> new file mode 100644
> index 0000000..49a1cfd
> --- /dev/null
> +++ b/tests/unit/test-strList.c
> @@ -0,0 +1,80 @@
> +/*
> + * Copyright (c) 2022 - 2024 Oracle and/or its affiliates.
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/strList.h"
> +
> +static strList *make_list(int length)
> +{
> +    strList *head = 0, *list, **prev = &head;
> +
> +    while (length--) {
> +        list = *prev = g_new0(strList, 1);
> +        list->value = g_strdup("aaa");
> +        prev = &list->next;
> +    }
> +    return head;
> +}
> +
> +static void test_length(void)
> +{
> +    strList *list;
> +    int i;
> +
> +    for (i = 0; i < 5; i++) {
> +        list = make_list(i);
> +        g_assert_cmpint(i, ==, QAPI_LIST_LENGTH(list));
> +        qapi_free_strList(list);
> +    }
> +}
> +
> +struct {
> +    const char *string;
> +    const char *delim;
> +    const char *args[5];
> +} list_data[] = {
> +    { 0, ",", { 0 } },
> +    { "", ",", { 0 } },
> +    { "a", ",", { "a", 0 } },
> +    { "a,b", ",", { "a", "b", 0 } },
> +    { "a,b,c", ",", { "a", "b", "c", 0 } },
> +    { "first last", " ", { "first", "last", 0 } },
> +    { "a:", ":", { "a", "", 0 } },
> +    { "a::b", ":", { "a", "", "b", 0 } },
> +    { ":", ":", { "", "", 0 } },
> +    { ":a", ":", { "", "a", 0 } },
> +    { "::a", ":", { "", "", "a", 0 } },
> +};

NULL instead of 0, please.

> +
> +static void test_strv(void)
> +{
> +    int i, j;
> +    const char **expect;
> +    strList *list;
> +    GStrv args;

I'd prefer char **argv;

> +
> +    for (i = 0; i < ARRAY_SIZE(list_data); i++) {
> +        expect = list_data[i].args;
> +        list = strList_from_string(list_data[i].string, list_data[i].delim);
> +        args = strv_from_strList(list);
> +        qapi_free_strList(list);
> +        for (j = 0; expect[j] && args[j]; j++) {

Loop stops when either array element is null.  That's wrong, we need to
exhaust both arrays: || instead of &&.

> +            g_assert_cmpstr(expect[j], ==, args[j]);
> +        }
> +        g_assert_null(expect[j]);
> +        g_assert_null(args[j]);
> +        g_strfreev(args);
> +    }
> +}
> +
> +int main(int argc, char **argv)
> +{
> +    g_test_init(&argc, &argv, NULL);
> +    g_test_add_func("/test-string/length", test_length);
> +    g_test_add_func("/test-string/strv", test_strv);
> +    return g_test_run();
> +}