Re: [Qemu-devel] [PATCH v7 05/18] qapi: Test for various name collisions

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 10/01/2015 05:51 AM, Markus Armbruster wrote:
[...]
>>> +++ b/tests/qapi-schema/flat-union-cycle.json
>>> @@ -0,0 +1,8 @@
>>> +# Ensure that we have a sane error message for attempts at self-inheritance
>>> +# This test currently fails because we don't permit a union base, but
>>> +# even if we relax things to allow that in the future (see
>>> +# flat-union-base-union), self-inheritance should still fail.
>> 
>> Do we have a test for the simpler case of a struct inheriting from
>> itself?
>
> Not here, but in v5 16/46. That's because it asserts, but while it was
> easy to fix up in the QAPISchema.check(), I did not find it worth the
> churn to fix it up in the ad hoc parse code just to rip it back out
> later, and the QAPISchema.check() code requires several scaffolding
> patches (so it wasn't as easy as fixing the union 'type' clash asserts).
>  Tracking an assertion failure for any more than one patch at a time is
> horrible (as any other change to qapi.py changes line numbers that
> affect the assertion failure).

Well, I'm happy to take a test for inheritance loops, or leave it
uncovered for now, but I don't want to take a non-working test of an
unimplemented obscure case "union" without a test for the implemented
case "struct".

I can:

A. Drop this test case.

B. Replace it with the test case from 16/46.

C. Add the test case from 16/46 and keep this one.

I very much prefer B.  You?

>> I believe us merging struct and union types into a single object type is
>> more likely than us implementing union bases.  If I'm correct, we won't
>> need this test.
>
> Maybe, but even then, we still have to decide if a base object can have
> variants.

Yes, but even if we want them, we'll have to rewrite this test case from
scratch.

I'm no friend of present complications to maybe save future work.  Even
less when it's so unlikely to save any.

[...]