[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v7 03/14] qapi: Drop redundant alternate-good te
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v7 03/14] qapi: Drop redundant alternate-good test |
Date: |
Tue, 13 Oct 2015 10:12:46 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> writes:
> On 10/07/2015 10:15 AM, Markus Armbruster wrote:
>> Eric Blake <address@hidden> writes:
>>
>>> The alternate-good.json test was already covered by
>>> qapi-schema-test.json. As future commits will be tweaking
>>> how alternates are laid out, removing the duplicate test now
>>> reduces churn.
>>
>> Do we have more positive tests? They should probably all live in
>> qapi-schema-test.json, because there their generated code actually gets
>> compiled.
>
> Hmm, any test that has an empty .err and non-empty .out, but which does
> not also have an TODO/FIXME stating that it is a bug, is worth checking.
> So first, here's the list of non-empty .out files:
>
> empty.out [1]
> enum-empty.out
> event-case.out [4]
> union-empty.out [2]
> include-simple.out
> include-repetition.out
> include-relpath.out
> comments.out
> alternate-empty.out [2]
> returns-int.out
> struct-base-clash-base.out [3]
> flat-union-empty.out [2]
> indented-expr.out
> union-clash-data.out [3]
> ident-with-escape.out
> args-member-array.out
> flat-union-reverse-define.out
>
> [1] must stay separate, because we can't really make qapi-schema-test
> empty :)
> [2] marked FIXME, and turns into proper error message later in the v5 series
> [3] marked FIXME, and already removed later in the v5 series once a
> positive test is inlined into qapi-schema-test
> [4] marked TODO
>
> For the ones I didn't tag with a footnote, we could probably cover
> things directly in qapi-schema-test instead; the question then becomes
> which ones are already covered, and which ones need content moved.
Actually, the point isn't to move the positive test to
qapi-schema-test.json, the point is to compile-test its generated code.
Moving it to qapi-schema-test.json accomplishes that. However, we may
not want a single, monolithic positive test. Should we split up
qapi-schema-test.json instead? I don't know. Anyway, let's flush our
queue first.
Third case: the generated code isn't worth compile-testing; comparing
actual to expected .out suffices.
Let's sort your untagged tests into buckets:
Not worth compile-testing:
* comments.json
* include-simple.json
* include-repetition.json
* include-relpath.json
* indented-expr.json
* ident-with-escape.json
Not (completely) covered in qapi-schema-test.json:
* enum-empty.json
Not covered, but should be.
* flat-union-reverse-define.json
UserDefOne covers forward reference to struct base, UserDefFlatUnion
covers forward reference to union base, and UserDefFlatUnion2 covers
forward reference to member. We may want to cover forward reference
to the tag member's type.
Covered:
* returns-int.json
'user_def_cmd3' does the job.
* args-member-array.json
'__org.qemu_x-command' seems good enough.
- Re: [Qemu-devel] [RFC PATCH] qapi: Rename simple union's generated tag member to type, (continued)
[Qemu-devel] [PATCH v7 05/14] qapi: Lazy creation of array types, Eric Blake, 2015/10/08
[Qemu-devel] [PATCH v7 13/14] qapi: Add test for alternate branch 'kind' clash, Eric Blake, 2015/10/08
[Qemu-devel] [PATCH v7 03/14] qapi: Drop redundant alternate-good test, Eric Blake, 2015/10/08
[Qemu-devel] [PATCH v7 01/14] qapi: Use predicate callback to determine visit filtering, Eric Blake, 2015/10/08
[Qemu-devel] [PATCH v7 08/14] qapi: Track location that created an implicit type, Eric Blake, 2015/10/08
[Qemu-devel] [PATCH v7 09/14] qapi: Track owner of each object member, Eric Blake, 2015/10/08
[Qemu-devel] [PATCH v7 11/14] qapi: Move duplicate member checks to schema check(), Eric Blake, 2015/10/08