[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v7 08/18] qapi: Test use of 'number' within alte
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v7 08/18] qapi: Test use of 'number' within alternates |
Date: |
Tue, 06 Oct 2015 09:24:23 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> writes:
> On 09/29/2015 04:21 PM, Eric Blake wrote:
>> Add some testsuite exposure for use of a 'number' as part of
>> an alternate. The current state of the tree has a few bugs
>> exposed by this: our input parser depends on the ordering of
>> how the qapi schema declared the alternate, and the parser
>> does not accept integers for a 'number' in an alternate even
>> though it does for numbers outside of an alternate.
>>
>> Mixing 'int' and 'number' in the same alternate is unusual,
>> since both are supplied by json-numbers, but there does not
>> seem to be a technical reason to forbid it given that our
>> json lexer distinguishes between json-numbers that can be
>> represented as an int vs. those that cannot.
>>
>> Improve the existing test_visitor_in_alternate() to match the
>> style of the new test_visitor_in_alternate_number(), and to
>> ensure full coverage of all possible qtype parsing.
>>
>> Signed-off-by: Eric Blake <address@hidden>
>>
>> ---
>
>> +static void test_visitor_in_alternate_number(TestInputVisitorData *data,
>> + const void *unused)
>> +{
>> + Visitor *v;
>> + Error *err = NULL;
>> + AltStrBool *asb;
>> + AltStrNum *asn;
>> + AltNumStr *ans;
>> + AltStrInt *asi;
>> + AltIntNum *ain;
>> + AltNumInt *ani;
>> +
>> + /* Parsing an int */
>> +
>> + v = visitor_input_test_init(data, "42");
>> + visit_type_AltStrBool(v, &asb, NULL, &err);
>> + g_assert(err);
>> + qapi_free_AltStrBool(asb);
>> + visitor_input_teardown(data, NULL);
>
> This fails to reset err = NULL...
>
>> +
>> + /* FIXME: Order of alternate should not affect semantics; asn should
>> + * parse the same as ans */
>> + v = visitor_input_test_init(data, "42");
>> + visit_type_AltStrNum(v, &asn, NULL, &err);
>> + /* FIXME g_assert_cmpint(asn->kind, == ALT_STR_NUM_KIND_N); */
>> + /* FIXME g_assert_cmpfloat(asn->n, ==, 42); */
>> + g_assert(err);
>> + error_free(err);
>> + err = NULL;
>
> ...which means that this test is not reliable. Do you need a v8, or can
> you squash this in?
>
>
> diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
> index 1b5a369..6104ac6 100644
> --- a/tests/test-qmp-input-visitor.c
> +++ b/tests/test-qmp-input-visitor.c
> @@ -395,6 +395,8 @@ static void
> test_visitor_in_alternate_number(TestInputVisitorData *data,
> v = visitor_input_test_init(data, "42");
> visit_type_AltStrBool(v, &asb, NULL, &err);
> g_assert(err);
> + error_free(err);
> + err = NULL;
> qapi_free_AltStrBool(asb);
>
> v = visitor_input_test_init(data, "42");
Squashed & pushed to qapi-next.
I also updated commit messages to document the tweaks made on commit.
- [Qemu-devel] [PATCH v7 13/18] qapi: Consistent generated code: prefer common indentation, (continued)
- [Qemu-devel] [PATCH v7 13/18] qapi: Consistent generated code: prefer common indentation, Eric Blake, 2015/10/08
- [Qemu-devel] [PATCH v7 11/18] qapi: Consistent generated code: prefer visitor 'v', Eric Blake, 2015/10/08
- [Qemu-devel] [RFC PATCH v7 18/18] qapi: Use gen_err_check() in more places, Eric Blake, 2015/10/08
- [Qemu-devel] [RFC PATCH v7 17/18] qapi: Simplify gen_visit_fields() error handling, Eric Blake, 2015/10/08
- [Qemu-devel] [PATCH v7 03/18] qapi: Invoke exception superclass initializer, Eric Blake, 2015/10/08
- [Qemu-devel] [PATCH v7 09/18] qapi: Reuse code for flat union base validation, Eric Blake, 2015/10/08
- [Qemu-devel] [PATCH v7 10/18] qapi: Consistent generated code: prefer error 'err', Eric Blake, 2015/10/08
- [Qemu-devel] [PATCH v7 08/18] qapi: Test use of 'number' within alternates, Eric Blake, 2015/10/08