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

Steve Sistare posted 5 patches 8 months, 1 week ago
Maintainers: Markus Armbruster <armbru@redhat.com>, Michael Roth <michael.roth@amd.com>, Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>, "Dr. David Alan Gilbert" <dave@treblig.org>, Jason Wang <jasowang@redhat.com>
There is a newer version of this series
[PATCH V4 4/5] util: strList unit tests
Posted by Steve Sistare 8 months, 1 week ago
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 } },
+};
+
+static void test_strv(void)
+{
+    int i, j;
+    const char **expect;
+    strList *list;
+    GStrv args;
+
+    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++) {
+            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();
+}
-- 
1.8.3.1


Re: [PATCH V4 4/5] util: strList unit tests
Posted by Markus Armbruster 7 months ago
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();
> +}
Re: [PATCH V4 4/5] util: strList unit tests
Posted by Steven Sistare 7 months ago
On 2/21/2024 8:19 AM, Markus Armbruster wrote:
> 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.

ok.

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

ok.

>> +
>> +    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 &&.

|| is not safe.  After one array is exhausted, [j] will be out of bounds for
the other.  The g_assert_null calls guarantee the arrays are the same length
and all elements have been compared.

- Steve

>> +            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();
>> +}
>