Eric Blake <ebl...@redhat.com> writes: > On 09/29/2015 07:38 AM, Markus Armbruster wrote: >> Eric Blake <ebl...@redhat.com> writes: >> >>> 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 <ebl...@redhat.com> > >>> +++ b/tests/test-qmp-input-visitor.c >>> @@ -371,12 +371,133 @@ static void >>> test_visitor_in_alternate(TestInputVisitorData *data, >>> UserDefAlternate *tmp; >>> >>> v = visitor_input_test_init(data, "42"); >>> - >>> - visit_type_UserDefAlternate(v, &tmp, NULL, &err); >>> - g_assert(err == NULL); >>> + visit_type_UserDefAlternate(v, &tmp, NULL, &error_abort); >>> g_assert_cmpint(tmp->kind, ==, USER_DEF_ALTERNATE_KIND_I); >>> g_assert_cmpint(tmp->i, ==, 42); >>> qapi_free_UserDefAlternate(tmp); >>> + visitor_input_teardown(data, NULL); >> >> Ugly in this test: visitor_input_test_init() is to be paired with >> visitor_input_teardown(), but each test's last visitor_input_teardown() >> can be omitted, because we also pass visitor_input_teardown to >> g_test_add(). Not your patch's fault. Let's ignore it for now. > > v5 25/46 claims otherwise (valgrind was complaining about leaks that > additional calls to visitor_input_teardown() resolved; maybe the test > engine is not automatically doing the expected teardown?). And maybe it > would be smarter to rework the tests to allow reusing an existing > visitor on a new string, instead of completely allocating a new visitor > framework for every new string to parse. But indeed, any further > cleanups are better done in that later patch.
Let me rephrase. visitor_input_test_init() creates what visitor_input_teardown() destroys. The two need to be paired. The pairing is done in an ugly way: the tests call visitor_input_teardown() for every visitor_input_test_init() *except* the last one. That one's teardown is set up by input_visitor_test_add(), which passes visitor_input_teardown() to g_test_add(). When a function calls visitor_input_test_init() just once, this ugliness isn't very visible. When it calls it multiple times, it's quite visible and actually confusing.
