1. Patchew
  2. QEMU
  3. [Qemu-devel] [PATCH RFC 0/6] qapi: rewrite string-input-visitor
  4. View patch

[Qemu-devel] [PATCH RFC 5/6] test-string-input-visitor: split off uint64 list tests

David Hildenbrand posted 6 patches 7 years, 3 months ago
There is a newer version of this series
[Qemu-devel] [PATCH RFC 5/6] test-string-input-visitor: split off uint64 list tests
Posted by David Hildenbrand 7 years, 3 months ago 
Basically copy all int64 list tests but adapt them to work on uint64
instead.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 tests/test-string-input-visitor.c | 71 +++++++++++++++++++++++++++++--
 1 file changed, 67 insertions(+), 4 deletions(-)

diff --git a/tests/test-string-input-visitor.c b/tests/test-string-input-visitor.c
index 2f6360e9ca..731094f789 100644
--- a/tests/test-string-input-visitor.c
+++ b/tests/test-string-input-visitor.c
@@ -111,7 +111,6 @@ static void test_visitor_in_intList(TestInputVisitorData *data,
                           6, 7, 8 };
     int64_t expect2[] = { 32767, -32768, -32767 };
     int64_t expect3[] = { INT64_MIN, INT64_MAX };
-    uint64_t expect4[] = { UINT64_MAX };
     Error *err = NULL;
     int64List *res = NULL;
     Visitor *v;
@@ -129,9 +128,6 @@ static void test_visitor_in_intList(TestInputVisitorData *data,
                                 "-9223372036854775808,9223372036854775807");
     check_ilist(v, expect3, ARRAY_SIZE(expect3));
 
-    v = visitor_input_test_init(data, "18446744073709551615");
-    check_ulist(v, expect4, ARRAY_SIZE(expect4));
-
     /* Empty list */
 
     v = visitor_input_test_init(data, "");
@@ -174,6 +170,71 @@ static void test_visitor_in_intList(TestInputVisitorData *data,
     visit_end_list(v, NULL);
 }
 
+static void test_visitor_in_uintList(TestInputVisitorData *data,
+                                     const void *unused)
+{
+    uint64_t expect1[] = { 1, 2, 0, 2, 3, 4, 20, 5, 6, 7, 8, 9, 1, 2, 3, 4, 5,
+                           6, 7, 8 };
+    uint64_t expect2[] = { 32767, -32768, -32767 };
+    uint64_t expect3[] = { UINT64_MAX };
+    Error *err = NULL;
+    uint64List *res = NULL;
+    Visitor *v;
+    uint64_t val;
+
+    /* Valid lists */
+
+    v = visitor_input_test_init(data, "1,2,0,2-4,20,5-9,1-8");
+    check_ulist(v, expect1, ARRAY_SIZE(expect1));
+
+    v = visitor_input_test_init(data, "32767,-32768--32767");
+    check_ulist(v, expect2, ARRAY_SIZE(expect2));
+
+    v = visitor_input_test_init(data, "18446744073709551615");
+    check_ulist(v, expect3, ARRAY_SIZE(expect3));
+
+    /* Empty list */
+
+    v = visitor_input_test_init(data, "");
+    visit_type_uint64List(v, NULL, &res, &error_abort);
+    g_assert(!res);
+
+    /* Not a list */
+
+    v = visitor_input_test_init(data, "not an uint list");
+
+    visit_type_uint64List(v, NULL, &res, &err);
+    error_free_or_abort(&err);
+    g_assert(!res);
+
+    /* Unvisited list tail */
+
+    v = visitor_input_test_init(data, "0,2-3");
+
+    visit_start_list(v, NULL, NULL, 0, &error_abort);
+    visit_type_uint64(v, NULL, &val, &error_abort);
+    g_assert_cmpuint(val, ==, 0);
+    visit_type_uint64(v, NULL, &val, &error_abort);
+    g_assert_cmpuint(val, ==, 2);
+
+    visit_check_list(v, &err);
+    error_free_or_abort(&err);
+    visit_end_list(v, NULL);
+
+    /* Visit beyond end of list */
+
+    v = visitor_input_test_init(data, "0");
+
+    visit_start_list(v, NULL, NULL, 0, &error_abort);
+    visit_type_uint64(v, NULL, &val, &err);
+    g_assert_cmpuint(val, ==, 0);
+    visit_type_uint64(v, NULL, &val, &err);
+    error_free_or_abort(&err);
+
+    visit_check_list(v, &error_abort);
+    visit_end_list(v, NULL);
+}
+
 static void test_visitor_in_bool(TestInputVisitorData *data,
                                  const void *unused)
 {
@@ -334,6 +395,8 @@ int main(int argc, char **argv)
                            &in_visitor_data, test_visitor_in_int);
     input_visitor_test_add("/string-visitor/input/intList",
                            &in_visitor_data, test_visitor_in_intList);
+    input_visitor_test_add("/string-visitor/input/uintList",
+                           &in_visitor_data, test_visitor_in_uintList);
     input_visitor_test_add("/string-visitor/input/bool",
                            &in_visitor_data, test_visitor_in_bool);
     input_visitor_test_add("/string-visitor/input/number",
-- 
2.17.2
Re: [Qemu-devel] [PATCH RFC 5/6] test-string-input-visitor: split off uint64 list tests
Posted by Markus Armbruster 7 years, 2 months ago 
David Hildenbrand <david@redhat.com> writes:

> Basically copy all int64 list tests but adapt them to work on uint64
> instead.
>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  tests/test-string-input-visitor.c | 71 +++++++++++++++++++++++++++++--
>  1 file changed, 67 insertions(+), 4 deletions(-)
>
> diff --git a/tests/test-string-input-visitor.c b/tests/test-string-input-visitor.c
> index 2f6360e9ca..731094f789 100644
> --- a/tests/test-string-input-visitor.c
> +++ b/tests/test-string-input-visitor.c
> @@ -111,7 +111,6 @@ static void test_visitor_in_intList(TestInputVisitorData *data,
>                            6, 7, 8 };
>      int64_t expect2[] = { 32767, -32768, -32767 };
>      int64_t expect3[] = { INT64_MIN, INT64_MAX };
> -    uint64_t expect4[] = { UINT64_MAX };
>      Error *err = NULL;
>      int64List *res = NULL;
>      Visitor *v;
> @@ -129,9 +128,6 @@ static void test_visitor_in_intList(TestInputVisitorData *data,
>                                  "-9223372036854775808,9223372036854775807");
>      check_ilist(v, expect3, ARRAY_SIZE(expect3));
>  
> -    v = visitor_input_test_init(data, "18446744073709551615");
> -    check_ulist(v, expect4, ARRAY_SIZE(expect4));
> -

Hmm.  Testing behavior for this input is still worthwhile, isn't it?

The use of check_ulist() here is admittedly unclean.

>      /* Empty list */
>  
>      v = visitor_input_test_init(data, "");
> @@ -174,6 +170,71 @@ static void test_visitor_in_intList(TestInputVisitorData *data,
>      visit_end_list(v, NULL);
>  }
>  
> +static void test_visitor_in_uintList(TestInputVisitorData *data,
> +                                     const void *unused)
> +{
> +    uint64_t expect1[] = { 1, 2, 0, 2, 3, 4, 20, 5, 6, 7, 8, 9, 1, 2, 3, 4, 5,

Please wrap this line a bit earlier.

> +                           6, 7, 8 };
> +    uint64_t expect2[] = { 32767, -32768, -32767 };
> +    uint64_t expect3[] = { UINT64_MAX };
> +    Error *err = NULL;
> +    uint64List *res = NULL;
> +    Visitor *v;
> +    uint64_t val;
> +
> +    /* Valid lists */
> +
> +    v = visitor_input_test_init(data, "1,2,0,2-4,20,5-9,1-8");
> +    check_ulist(v, expect1, ARRAY_SIZE(expect1));
> +
> +    v = visitor_input_test_init(data, "32767,-32768--32767");
> +    check_ulist(v, expect2, ARRAY_SIZE(expect2));
> +
> +    v = visitor_input_test_init(data, "18446744073709551615");
> +    check_ulist(v, expect3, ARRAY_SIZE(expect3));

Test behavior for large negative numbers?

> +
> +    /* Empty list */
> +
> +    v = visitor_input_test_init(data, "");
> +    visit_type_uint64List(v, NULL, &res, &error_abort);
> +    g_assert(!res);
> +
> +    /* Not a list */
> +
> +    v = visitor_input_test_init(data, "not an uint list");
> +
> +    visit_type_uint64List(v, NULL, &res, &err);
> +    error_free_or_abort(&err);
> +    g_assert(!res);
> +
> +    /* Unvisited list tail */
> +
> +    v = visitor_input_test_init(data, "0,2-3");
> +
> +    visit_start_list(v, NULL, NULL, 0, &error_abort);
> +    visit_type_uint64(v, NULL, &val, &error_abort);
> +    g_assert_cmpuint(val, ==, 0);
> +    visit_type_uint64(v, NULL, &val, &error_abort);
> +    g_assert_cmpuint(val, ==, 2);
> +
> +    visit_check_list(v, &err);
> +    error_free_or_abort(&err);
> +    visit_end_list(v, NULL);
> +
> +    /* Visit beyond end of list */
> +
> +    v = visitor_input_test_init(data, "0");
> +
> +    visit_start_list(v, NULL, NULL, 0, &error_abort);
> +    visit_type_uint64(v, NULL, &val, &err);
> +    g_assert_cmpuint(val, ==, 0);
> +    visit_type_uint64(v, NULL, &val, &err);
> +    error_free_or_abort(&err);
> +
> +    visit_check_list(v, &error_abort);
> +    visit_end_list(v, NULL);
> +}
> +
>  static void test_visitor_in_bool(TestInputVisitorData *data,
>                                   const void *unused)
>  {
> @@ -334,6 +395,8 @@ int main(int argc, char **argv)
>                             &in_visitor_data, test_visitor_in_int);
>      input_visitor_test_add("/string-visitor/input/intList",
>                             &in_visitor_data, test_visitor_in_intList);
> +    input_visitor_test_add("/string-visitor/input/uintList",
> +                           &in_visitor_data, test_visitor_in_uintList);
>      input_visitor_test_add("/string-visitor/input/bool",
>                             &in_visitor_data, test_visitor_in_bool);
>      input_visitor_test_add("/string-visitor/input/number",
Re: [Qemu-devel] [PATCH RFC 5/6] test-string-input-visitor: split off uint64 list tests
Posted by David Hildenbrand 7 years, 2 months ago 
On 14.11.18 17:21, Markus Armbruster wrote:
> David Hildenbrand <david@redhat.com> writes:
> 
>> Basically copy all int64 list tests but adapt them to work on uint64
>> instead.
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>>  tests/test-string-input-visitor.c | 71 +++++++++++++++++++++++++++++--
>>  1 file changed, 67 insertions(+), 4 deletions(-)
>>
>> diff --git a/tests/test-string-input-visitor.c b/tests/test-string-input-visitor.c
>> index 2f6360e9ca..731094f789 100644
>> --- a/tests/test-string-input-visitor.c
>> +++ b/tests/test-string-input-visitor.c
>> @@ -111,7 +111,6 @@ static void test_visitor_in_intList(TestInputVisitorData *data,
>>                            6, 7, 8 };
>>      int64_t expect2[] = { 32767, -32768, -32767 };
>>      int64_t expect3[] = { INT64_MIN, INT64_MAX };
>> -    uint64_t expect4[] = { UINT64_MAX };
>>      Error *err = NULL;
>>      int64List *res = NULL;
>>      Visitor *v;
>> @@ -129,9 +128,6 @@ static void test_visitor_in_intList(TestInputVisitorData *data,
>>                                  "-9223372036854775808,9223372036854775807");
>>      check_ilist(v, expect3, ARRAY_SIZE(expect3));
>>  
>> -    v = visitor_input_test_init(data, "18446744073709551615");
>> -    check_ulist(v, expect4, ARRAY_SIZE(expect4));
>> -
> 
> Hmm.  Testing behavior for this input is still worthwhile, isn't it?
> 
> The use of check_ulist() here is admittedly unclean.

This check has been moved to the other function where we test ulists.

Or do you want this check here to test again ilist and see that an error
gets reported as the value is too big? Will add such range checks.

> 
>>      /* Empty list */
>>  
>>      v = visitor_input_test_init(data, "");
>> @@ -174,6 +170,71 @@ static void test_visitor_in_intList(TestInputVisitorData *data,
>>      visit_end_list(v, NULL);
>>  }
>>  
>> +static void test_visitor_in_uintList(TestInputVisitorData *data,
>> +                                     const void *unused)
>> +{
>> +    uint64_t expect1[] = { 1, 2, 0, 2, 3, 4, 20, 5, 6, 7, 8, 9, 1, 2, 3, 4, 5,
> 
> Please wrap this line a bit earlier.

Yes.

> 
>> +                           6, 7, 8 };
>> +    uint64_t expect2[] = { 32767, -32768, -32767 };
>> +    uint64_t expect3[] = { UINT64_MAX };
>> +    Error *err = NULL;
>> +    uint64List *res = NULL;
>> +    Visitor *v;
>> +    uint64_t val;
>> +
>> +    /* Valid lists */
>> +
>> +    v = visitor_input_test_init(data, "1,2,0,2-4,20,5-9,1-8");
>> +    check_ulist(v, expect1, ARRAY_SIZE(expect1));
>> +
>> +    v = visitor_input_test_init(data, "32767,-32768--32767");
>> +    check_ulist(v, expect2, ARRAY_SIZE(expect2));
>> +
>> +    v = visitor_input_test_init(data, "18446744073709551615");
>> +    check_ulist(v, expect3, ARRAY_SIZE(expect3));
> 
> Test behavior for large negative numbers?

Yes, will add.



-- 

Thanks,

David / dhildenb
Re: [Qemu-devel] [PATCH RFC 5/6] test-string-input-visitor: split off uint64 list tests
Posted by Markus Armbruster 7 years, 2 months ago 
David Hildenbrand <david@redhat.com> writes:

> On 14.11.18 17:21, Markus Armbruster wrote:
>> David Hildenbrand <david@redhat.com> writes:
>> 
>>> Basically copy all int64 list tests but adapt them to work on uint64
>>> instead.
>>>
>>> Signed-off-by: David Hildenbrand <david@redhat.com>
>>> ---
>>>  tests/test-string-input-visitor.c | 71 +++++++++++++++++++++++++++++--
>>>  1 file changed, 67 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/tests/test-string-input-visitor.c b/tests/test-string-input-visitor.c
>>> index 2f6360e9ca..731094f789 100644
>>> --- a/tests/test-string-input-visitor.c
>>> +++ b/tests/test-string-input-visitor.c
>>> @@ -111,7 +111,6 @@ static void test_visitor_in_intList(TestInputVisitorData *data,
>>>                            6, 7, 8 };
>>>      int64_t expect2[] = { 32767, -32768, -32767 };
>>>      int64_t expect3[] = { INT64_MIN, INT64_MAX };
>>> -    uint64_t expect4[] = { UINT64_MAX };
>>>      Error *err = NULL;
>>>      int64List *res = NULL;
>>>      Visitor *v;
>>> @@ -129,9 +128,6 @@ static void test_visitor_in_intList(TestInputVisitorData *data,
>>>                                  "-9223372036854775808,9223372036854775807");
>>>      check_ilist(v, expect3, ARRAY_SIZE(expect3));
>>>  
>>> -    v = visitor_input_test_init(data, "18446744073709551615");
>>> -    check_ulist(v, expect4, ARRAY_SIZE(expect4));
>>> -
>> 
>> Hmm.  Testing behavior for this input is still worthwhile, isn't it?
>> 
>> The use of check_ulist() here is admittedly unclean.
>
> This check has been moved to the other function where we test ulists.

You're right, you didn't reduce test coverage.  I got confused.

But there's an opportunity to extend coverage: test
visit_type_int64List() for this input.  Separate patch.  If you put it
before PATCH 3, it'll ensure PATCH 3 doesn't change behavior for this
case.  Suggestion, not demand.  Use your judgement.

> Or do you want this check here to test again ilist and see that an error
> gets reported as the value is too big? Will add such range checks.

Be careful with new range checks, they can easily break backward
compatibility.  Deciding whether the break is acceptable then requires
analysis.

>> 
>>>      /* Empty list */
>>>  
>>>      v = visitor_input_test_init(data, "");
>>> @@ -174,6 +170,71 @@ static void test_visitor_in_intList(TestInputVisitorData *data,
>>>      visit_end_list(v, NULL);
>>>  }
>>>  
>>> +static void test_visitor_in_uintList(TestInputVisitorData *data,
>>> +                                     const void *unused)
>>> +{
>>> +    uint64_t expect1[] = { 1, 2, 0, 2, 3, 4, 20, 5, 6, 7, 8, 9, 1, 2, 3, 4, 5,
>> 
>> Please wrap this line a bit earlier.
>
> Yes.
>
>> 
>>> +                           6, 7, 8 };
>>> +    uint64_t expect2[] = { 32767, -32768, -32767 };
>>> +    uint64_t expect3[] = { UINT64_MAX };
>>> +    Error *err = NULL;
>>> +    uint64List *res = NULL;
>>> +    Visitor *v;
>>> +    uint64_t val;
>>> +
>>> +    /* Valid lists */
>>> +
>>> +    v = visitor_input_test_init(data, "1,2,0,2-4,20,5-9,1-8");
>>> +    check_ulist(v, expect1, ARRAY_SIZE(expect1));
>>> +
>>> +    v = visitor_input_test_init(data, "32767,-32768--32767");
>>> +    check_ulist(v, expect2, ARRAY_SIZE(expect2));
>>> +
>>> +    v = visitor_input_test_init(data, "18446744073709551615");
>>> +    check_ulist(v, expect3, ARRAY_SIZE(expect3));
>> 
>> Test behavior for large negative numbers?
>
> Yes, will add.

Again, a separate patch before PATCH 3 might be best.

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.