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

[David Hildenbrand <david@redhat.com>]

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