Re: [Qemu-devel] [PATCH 8/8] OptsVisitor: introduce unit tests, with test cases for range flattening

[Laszlo Ersek <lersek@redhat.com>]

On 07/23/13 00:04, Eric Blake wrote:
> On 07/22/2013 03:07 PM, Laszlo Ersek wrote:
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>  tests/Makefile            |    6 +-
>>  qapi-schema-test.json     |   15 +++
>>  tests/test-opts-visitor.c |  275 +++++++++++++++++++++++++++++++++++++++++++++
>>  .gitignore                |    1 +
>>  4 files changed, 296 insertions(+), 1 deletions(-)
>>  create mode 100644 tests/test-opts-visitor.c
> 
>> +    add_test("/visitor/opts/i64/val1/errno",    &expect_fail,
>> +             "i64=0x8000000000000000");
>> +    add_test("/visitor/opts/i64/val1/empty",    &expect_fail, "i64=");
>> +    add_test("/visitor/opts/i64/val1/trailing", &expect_fail, "i64=5z");
>> +    add_test("/visitor/opts/i64/nonlist",       &expect_fail, "i64x=5-6");
>> +    add_test("/visitor/opts/i64/val2/errno",    &expect_fail,
>> +             "i64=0x7fffffffffffffff-0x8000000000000000");
>> +    add_test("/visitor/opts/i64/val2/empty",    &expect_fail, "i64=5-");
>> +    add_test("/visitor/opts/i64/val2/trailing", &expect_fail, "i64=5-6z");
>> +    add_test("/visitor/opts/i64/range/empty",   &expect_fail, "i64=6-5");
>> +    add_test("/visitor/opts/i64/range/minval",  &expect_i64_min,
>> +             "i64=-0x8000000000000000--0x8000000000000000");
>> +    add_test("/visitor/opts/i64/range/maxval",  &expect_i64_max,
>> +             "i64=0x7fffffffffffffff-0x7fffffffffffffff");
> 
> Pretty thorough, although I thought of a couple other ideas to test:
> i64=5z-6 should fail; i64=5-6-7 should fail

I can add them if you insist, but I wrote (and single-stepped all of)
the test cases so that all branches added by patches 3, 5 and 6 would be
covered. (Some of the final tests in this function are actually
redundant, but I liked how they looked :))

For example, "i64=5z-6" is no different from "i64=5z", in patch 3 both
the first added (*endptr == '\0') condition and the (*endptr == '-')
fail the same way for both input strings: we never look past the "z".

Likewise, "i64=5-6-7" is the same case as "i64=5-6z": both characters
after the "6" (ie. "-" and "z") violate the second added (*endptr ==
'\0') condition in patch 3 the same way.

Do you accept this argument? :)

Thanks!
Laszlo