Re: [PATCH 12/22] test-qobject-input-visitor: Wean off UserDefListUnion

2021-09-13 Thread Markus Armbruster
Eric Blake  writes:

> On Mon, Sep 13, 2021 at 02:39:22PM +0200, Markus Armbruster wrote:
>> The test_visitor_in_list_union_FOO() use simple union UserDefListUnion
>> to cover lists of builtin types.  Rewrite as
>> test_visitor_in_list_struct(), using struct ArrayStruct and a lot less
>> code.
>> 
>> test_visitor_in_fail_union_list() uses UserDefListUnion to cover
>> "variant members don't match the discriminator value".  Cover that in
>> test_visitor_in_fail_union_flat() instead, and drop
>> test_visitor_in_fail_union_list().  Appropriating the former for this
>> purpose is okay, because it actually failed due to missing
>> discriminator, which is still covered by
>> test_visitor_in_fail_union_flat_no_discrim().
>> 
>> Signed-off-by: Markus Armbruster 
>> ---
>>  tests/unit/test-qobject-input-visitor.c | 460 
>>  1 file changed, 148 insertions(+), 312 deletions(-)
>> 
>> @@ -1206,7 +1066,7 @@ static void 
>> test_visitor_in_fail_union_flat(TestInputVisitorData *data,
>>  Error *err = NULL;
>>  Visitor *v;
>>  
>> -v = visitor_input_test_init(data, "{ 'string': 'c', 'integer': 41, 
>> 'boolean': true }");
>> +v = visitor_input_test_init(data, "{ 'enum1': 'value2', 'string': 'c', 
>> 'integer': 41, 'boolean': true }");
>
> Long line; do we care?

We have loads of similar lines in unit tests, and I've given up on
them.

> Reviewed-by: Eric Blake 

Thanks!




Re: [PATCH 12/22] test-qobject-input-visitor: Wean off UserDefListUnion

2021-09-13 Thread Eric Blake
On Mon, Sep 13, 2021 at 02:39:22PM +0200, Markus Armbruster wrote:
> The test_visitor_in_list_union_FOO() use simple union UserDefListUnion
> to cover lists of builtin types.  Rewrite as
> test_visitor_in_list_struct(), using struct ArrayStruct and a lot less
> code.
> 
> test_visitor_in_fail_union_list() uses UserDefListUnion to cover
> "variant members don't match the discriminator value".  Cover that in
> test_visitor_in_fail_union_flat() instead, and drop
> test_visitor_in_fail_union_list().  Appropriating the former for this
> purpose is okay, because it actually failed due to missing
> discriminator, which is still covered by
> test_visitor_in_fail_union_flat_no_discrim().
> 
> Signed-off-by: Markus Armbruster 
> ---
>  tests/unit/test-qobject-input-visitor.c | 460 
>  1 file changed, 148 insertions(+), 312 deletions(-)
> 
> @@ -1206,7 +1066,7 @@ static void 
> test_visitor_in_fail_union_flat(TestInputVisitorData *data,
>  Error *err = NULL;
>  Visitor *v;
>  
> -v = visitor_input_test_init(data, "{ 'string': 'c', 'integer': 41, 
> 'boolean': true }");
> +v = visitor_input_test_init(data, "{ 'enum1': 'value2', 'string': 'c', 
> 'integer': 41, 'boolean': true }");

Long line; do we care?

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org




[PATCH 12/22] test-qobject-input-visitor: Wean off UserDefListUnion

2021-09-13 Thread Markus Armbruster
The test_visitor_in_list_union_FOO() use simple union UserDefListUnion
to cover lists of builtin types.  Rewrite as
test_visitor_in_list_struct(), using struct ArrayStruct and a lot less
code.

test_visitor_in_fail_union_list() uses UserDefListUnion to cover
"variant members don't match the discriminator value".  Cover that in
test_visitor_in_fail_union_flat() instead, and drop
test_visitor_in_fail_union_list().  Appropriating the former for this
purpose is okay, because it actually failed due to missing
discriminator, which is still covered by
test_visitor_in_fail_union_flat_no_discrim().

Signed-off-by: Markus Armbruster 
---
 tests/unit/test-qobject-input-visitor.c | 460 
 1 file changed, 148 insertions(+), 312 deletions(-)

diff --git a/tests/unit/test-qobject-input-visitor.c 
b/tests/unit/test-qobject-input-visitor.c
index e41b91a2a6..6f59a7f432 100644
--- a/tests/unit/test-qobject-input-visitor.c
+++ b/tests/unit/test-qobject-input-visitor.c
@@ -464,6 +464,151 @@ static void test_visitor_in_list(TestInputVisitorData 
*data,
 g_assert(!head);
 }
 
+static void test_visitor_in_list_struct(TestInputVisitorData *data,
+const void *unused)
+{
+const char *int_member[] = {
+"integer", "s8", "s16", "s32", "s64", "u8", "u16", "u32", "u64" };
+g_autoptr(GString) json = g_string_new("");
+int i, j;
+const char *sep;
+g_autoptr(ArrayStruct) arrs = NULL;
+Visitor *v;
+intList *int_list;
+int8List *s8_list;
+int16List *s16_list;
+int32List *s32_list;
+int64List *s64_list;
+uint8List *u8_list;
+uint16List *u16_list;
+uint32List *u32_list;
+uint64List *u64_list;
+numberList *num_list;
+boolList *bool_list;
+strList *str_list;
+
+g_string_append_printf(json, "{");
+
+for (i = 0; i < G_N_ELEMENTS(int_member); i++) {
+g_string_append_printf(json, "'%s': [", int_member[i]);
+sep = "";
+for (j = 0; j < 32; j++) {
+g_string_append_printf(json, "%s%d", sep, j);
+sep = ", ";
+}
+g_string_append_printf(json, "], ");
+}
+
+g_string_append_printf(json, "'number': [");
+sep = "";
+for (i = 0; i < 32; i++) {
+g_string_append_printf(json, "%s%f", sep, (double)i / 3);
+sep = ", ";
+}
+g_string_append_printf(json, "], ");
+
+g_string_append_printf(json, "'boolean': [");
+sep = "";
+for (i = 0; i < 32; i++) {
+g_string_append_printf(json, "%s%s",
+   sep, i % 3 == 0 ? "true" : "false");
+sep = ", ";
+}
+g_string_append_printf(json, "], ");
+
+g_string_append_printf(json, "'string': [");
+sep = "";
+for (i = 0; i < 32; i++) {
+g_string_append_printf(json, "%s'%d'", sep, i);
+sep = ", ";
+}
+g_string_append_printf(json, "]");
+
+g_string_append_printf(json, "}");
+
+v = visitor_input_test_init_raw(data, json->str);
+visit_type_ArrayStruct(v, NULL, , _abort);
+
+i = 0;
+for (int_list = arrs->integer; int_list; int_list = int_list->next) {
+g_assert_cmpint(int_list->value, ==, i);
+i++;
+}
+
+i = 0;
+for (s8_list = arrs->s8; s8_list; s8_list = s8_list->next) {
+g_assert_cmpint(s8_list->value, ==, i);
+i++;
+}
+
+i = 0;
+for (s16_list = arrs->s16; s16_list; s16_list = s16_list->next) {
+g_assert_cmpint(s16_list->value, ==, i);
+i++;
+}
+
+i = 0;
+for (s32_list = arrs->s32; s32_list; s32_list = s32_list->next) {
+g_assert_cmpint(s32_list->value, ==, i);
+i++;
+}
+
+i = 0;
+for (s64_list = arrs->s64; s64_list; s64_list = s64_list->next) {
+g_assert_cmpint(s64_list->value, ==, i);
+i++;
+}
+
+i = 0;
+for (u8_list = arrs->u8; u8_list; u8_list = u8_list->next) {
+g_assert_cmpint(u8_list->value, ==, i);
+i++;
+}
+
+i = 0;
+for (u16_list = arrs->u16; u16_list; u16_list = u16_list->next) {
+g_assert_cmpint(u16_list->value, ==, i);
+i++;
+}
+
+i = 0;
+for (u32_list = arrs->u32; u32_list; u32_list = u32_list->next) {
+g_assert_cmpint(u32_list->value, ==, i);
+i++;
+}
+
+i = 0;
+for (u64_list = arrs->u64; u64_list; u64_list = u64_list->next) {
+g_assert_cmpint(u64_list->value, ==, i);
+i++;
+}
+
+i = 0;
+for (num_list = arrs->number; num_list; num_list = num_list->next) {
+char expected[32], actual[32];
+
+sprintf(expected, "%.6f", (double)i / 3);
+sprintf(actual, "%.6f", num_list->value);
+g_assert_cmpstr(expected, ==, actual);
+i++;
+}
+
+i = 0;
+for (bool_list = arrs->boolean; bool_list; bool_list = bool_list->next) {
+g_assert_cmpint(bool_list->value, ==, i % 3 == 0);
+i++;
+}
+
+i = 0;
+for (str_list = arrs->string; str_list; str_list =