From 1ffe818a395cb883746f3baf8d9a0b6988375e8b Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Tue, 29 Sep 2015 16:20:59 -0600 Subject: qapi: Sort qapi-schema tests Recent changes to qapi have provided quite a bit of churn in the makefile, because we are inconsistent on what order test names appear in, and on whether to re-wrap the list of tests or just add arbitrary line lengths. Writing the list in a sorted fashion, one test per line, will make future patches easier to see what tests are being added or removed by a patch. Although it is tempting to use $(wildcard qapi-schema/*.json) for a more compact listing, such an approach would risk picking up leftover garbage .json files in the directory; so keeping the list explicit is safer for ensuring reproducible tarballs and test results. Signed-off-by: Eric Blake Message-Id: <1443565276-4535-2-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster --- tests/Makefile | 160 ++++++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 114 insertions(+), 46 deletions(-) (limited to 'tests') diff --git a/tests/Makefile b/tests/Makefile index dbd32a6..164dea3 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -224,52 +224,120 @@ check-qtest-xtensaeb-y = $(check-qtest-xtensa-y) check-qtest-generic-y += tests/qom-test$(EXESUF) -check-qapi-schema-y := $(addprefix tests/qapi-schema/, \ - comments.json empty.json enum-empty.json enum-missing-data.json \ - enum-wrong-data.json enum-int-member.json enum-dict-member.json \ - enum-clash-member.json enum-max-member.json enum-union-clash.json \ - enum-bad-name.json enum-bad-prefix.json \ - funny-char.json indented-expr.json \ - missing-type.json bad-ident.json ident-with-escape.json \ - escape-outside-string.json unknown-escape.json \ - escape-too-short.json escape-too-big.json unicode-str.json \ - double-type.json bad-base.json bad-type-bool.json bad-type-int.json \ - bad-type-dict.json double-data.json unknown-expr-key.json \ - redefined-type.json redefined-command.json redefined-builtin.json \ - redefined-event.json command-int.json bad-data.json event-max.json \ - type-bypass-bad-gen.json \ - args-invalid.json \ - args-array-empty.json args-array-unknown.json args-int.json \ - args-unknown.json args-member-unknown.json args-member-array.json \ - args-member-array-bad.json args-alternate.json args-union.json \ - args-any.json \ - returns-array-bad.json returns-int.json returns-dict.json \ - returns-unknown.json returns-alternate.json returns-whitelist.json \ - missing-colon.json missing-comma-list.json missing-comma-object.json \ - struct-data-invalid.json struct-member-invalid.json \ - nested-struct-data.json non-objects.json \ - qapi-schema-test.json quoted-structural-chars.json \ - leading-comma-list.json leading-comma-object.json \ - trailing-comma-list.json trailing-comma-object.json \ - unclosed-list.json unclosed-object.json unclosed-string.json \ - duplicate-key.json union-invalid-base.json union-bad-branch.json \ - union-optional-branch.json union-unknown.json union-max.json \ - flat-union-optional-discriminator.json flat-union-no-base.json \ - flat-union-invalid-discriminator.json flat-union-inline.json \ - flat-union-invalid-branch-key.json flat-union-reverse-define.json \ - flat-union-string-discriminator.json union-base-no-discriminator.json \ - flat-union-bad-discriminator.json flat-union-bad-base.json \ - flat-union-base-any.json \ - flat-union-array-branch.json flat-union-int-branch.json \ - flat-union-base-union.json flat-union-branch-clash.json \ - alternate-nested.json alternate-unknown.json alternate-clash.json \ - alternate-good.json alternate-base.json alternate-array.json \ - alternate-conflict-string.json alternate-conflict-dict.json \ - include-simple.json include-relpath.json include-format-err.json \ - include-non-file.json include-no-file.json include-before-err.json \ - include-nested-err.json include-self-cycle.json include-cycle.json \ - include-repetition.json event-nest-struct.json event-case.json \ - struct-base-clash.json struct-base-clash-deep.json ) +qapi-schema += alternate-array.json +qapi-schema += alternate-base.json +qapi-schema += alternate-clash.json +qapi-schema += alternate-conflict-dict.json +qapi-schema += alternate-conflict-string.json +qapi-schema += alternate-good.json +qapi-schema += alternate-nested.json +qapi-schema += alternate-unknown.json +qapi-schema += args-alternate.json +qapi-schema += args-any.json +qapi-schema += args-array-empty.json +qapi-schema += args-array-unknown.json +qapi-schema += args-int.json +qapi-schema += args-invalid.json +qapi-schema += args-member-array-bad.json +qapi-schema += args-member-array.json +qapi-schema += args-member-unknown.json +qapi-schema += args-union.json +qapi-schema += args-unknown.json +qapi-schema += bad-base.json +qapi-schema += bad-data.json +qapi-schema += bad-ident.json +qapi-schema += bad-type-bool.json +qapi-schema += bad-type-dict.json +qapi-schema += bad-type-int.json +qapi-schema += command-int.json +qapi-schema += comments.json +qapi-schema += double-data.json +qapi-schema += double-type.json +qapi-schema += duplicate-key.json +qapi-schema += empty.json +qapi-schema += enum-bad-name.json +qapi-schema += enum-bad-prefix.json +qapi-schema += enum-clash-member.json +qapi-schema += enum-dict-member.json +qapi-schema += enum-empty.json +qapi-schema += enum-int-member.json +qapi-schema += enum-max-member.json +qapi-schema += enum-missing-data.json +qapi-schema += enum-union-clash.json +qapi-schema += enum-wrong-data.json +qapi-schema += escape-outside-string.json +qapi-schema += escape-too-big.json +qapi-schema += escape-too-short.json +qapi-schema += event-case.json +qapi-schema += event-max.json +qapi-schema += event-nest-struct.json +qapi-schema += flat-union-array-branch.json +qapi-schema += flat-union-bad-base.json +qapi-schema += flat-union-bad-discriminator.json +qapi-schema += flat-union-base-any.json +qapi-schema += flat-union-base-union.json +qapi-schema += flat-union-branch-clash.json +qapi-schema += flat-union-inline.json +qapi-schema += flat-union-int-branch.json +qapi-schema += flat-union-invalid-branch-key.json +qapi-schema += flat-union-invalid-discriminator.json +qapi-schema += flat-union-no-base.json +qapi-schema += flat-union-optional-discriminator.json +qapi-schema += flat-union-reverse-define.json +qapi-schema += flat-union-string-discriminator.json +qapi-schema += funny-char.json +qapi-schema += ident-with-escape.json +qapi-schema += include-before-err.json +qapi-schema += include-cycle.json +qapi-schema += include-format-err.json +qapi-schema += include-nested-err.json +qapi-schema += include-no-file.json +qapi-schema += include-non-file.json +qapi-schema += include-relpath.json +qapi-schema += include-repetition.json +qapi-schema += include-self-cycle.json +qapi-schema += include-simple.json +qapi-schema += indented-expr.json +qapi-schema += leading-comma-list.json +qapi-schema += leading-comma-object.json +qapi-schema += missing-colon.json +qapi-schema += missing-comma-list.json +qapi-schema += missing-comma-object.json +qapi-schema += missing-type.json +qapi-schema += nested-struct-data.json +qapi-schema += non-objects.json +qapi-schema += qapi-schema-test.json +qapi-schema += quoted-structural-chars.json +qapi-schema += redefined-builtin.json +qapi-schema += redefined-command.json +qapi-schema += redefined-event.json +qapi-schema += redefined-type.json +qapi-schema += returns-alternate.json +qapi-schema += returns-array-bad.json +qapi-schema += returns-dict.json +qapi-schema += returns-int.json +qapi-schema += returns-unknown.json +qapi-schema += returns-whitelist.json +qapi-schema += struct-base-clash-deep.json +qapi-schema += struct-base-clash.json +qapi-schema += struct-data-invalid.json +qapi-schema += struct-member-invalid.json +qapi-schema += trailing-comma-list.json +qapi-schema += trailing-comma-object.json +qapi-schema += type-bypass-bad-gen.json +qapi-schema += unclosed-list.json +qapi-schema += unclosed-object.json +qapi-schema += unclosed-string.json +qapi-schema += unicode-str.json +qapi-schema += union-bad-branch.json +qapi-schema += union-base-no-discriminator.json +qapi-schema += union-invalid-base.json +qapi-schema += union-max.json +qapi-schema += union-optional-branch.json +qapi-schema += union-unknown.json +qapi-schema += unknown-escape.json +qapi-schema += unknown-expr-key.json +check-qapi-schema-y := $(addprefix tests/qapi-schema/, $(qapi-schema)) GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h \ tests/test-qmp-commands.h tests/test-qapi-event.h \ -- cgit v1.1 From 7408fb67c0f9403f6e40aecf97cf798fc14e2cd8 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Tue, 29 Sep 2015 16:21:00 -0600 Subject: qapi: Improve 'include' error message Use of '"...%s" % include' to print non-strings can lead to ugly messages, such as this (if the .json change is applied without the qapi.py change): Expected a file name (string), got: OrderedDict() Better is to just omit the actual non-string value in the message. Signed-off-by: Eric Blake Message-Id: <1443565276-4535-3-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster --- tests/qapi-schema/include-non-file.err | 2 +- tests/qapi-schema/include-non-file.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'tests') diff --git a/tests/qapi-schema/include-non-file.err b/tests/qapi-schema/include-non-file.err index 9658c78..faae1ea 100644 --- a/tests/qapi-schema/include-non-file.err +++ b/tests/qapi-schema/include-non-file.err @@ -1 +1 @@ -tests/qapi-schema/include-non-file.json:1: Expected a file name (string), got: ['foo', 'bar'] +tests/qapi-schema/include-non-file.json:1: Value of 'include' must be a string diff --git a/tests/qapi-schema/include-non-file.json b/tests/qapi-schema/include-non-file.json index cd43c3f..4711aa4 100644 --- a/tests/qapi-schema/include-non-file.json +++ b/tests/qapi-schema/include-non-file.json @@ -1 +1 @@ -{ 'include': [ 'foo', 'bar' ] } +{ 'include': {} } -- cgit v1.1 From d220fbcd1db2097de5ff3037e85317fcb5433e4e Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Tue, 29 Sep 2015 16:21:03 -0600 Subject: qapi: Test for various name collisions Expose some weaknesses in the generator: we don't always forbid the generation of structs that contain multiple members that map to the same C or QMP name. This has already been marked FIXME in qapi.py in commit d90675f, but having more tests will make sure future patches produce desired behavior; and updating existing patches to better document things doesn't hurt, either. Some of these collisions are already caught in the old-style parser checks, but ultimately we want all collisions to be caught in the new-style QAPISchema*.check() methods. This patch focuses on C struct members, and does not consider collisions between commands and events (affecting C function names), or even collisions between generated C type names with user type names (for things like automatic FOOList struct representing array types or FOOKind for an implicit enum). There are two types of struct collisions we want to catch: 1) Collision between two keys in a JSON object. qapi.py prevents that within a single struct (see test duplicate-key), but it is possible to have collisions between a type's members and its base type's members (existing tests struct-base-clash, struct-base-clash-deep), and its flat union variant members (renamed test flat-union-clash-member). 2) Collision between two members of the C struct that is generated for a given QAPI type: a) Multiple QAPI names map to the same C name (new test args-name-clash) b) A QAPI name maps to a C name that is used for another purpose (new tests flat-union-clash-branch, struct-base-clash-base, union-clash-data). We already fixed some such cases in commit 0f61af3e and 1e6c1616, but more remain. c) Two C names generated for other purposes clash (updated test alternate-clash, new test union-clash-branches, union-clash-type, flat-union-clash-type) Ultimately, if we need to have a flat union where a tag value clashes with a base member name, we could change the generator to name the union (using 'foo.u.value' rather than 'foo.value') or otherwise munge the C name corresponding to tag values. But unless such a need arises, it will probably be easier to just forbid these collisions. Some of these negative tests will be deleted later, and positive tests added to qapi-schema-test.json in their place, when the generator code is reworked to avoid particular code generation collisions in class 2). [Note that viewing this patch with git rename detection enabled may see some confusion due to renaming some tests while adding others, but where the content is similar enough that git picks the wrong pre- and post-patch files to associate] Signed-off-by: Eric Blake Message-Id: <1443565276-4535-6-git-send-email-eblake@redhat.com> [Improve commit message and comments a bit, drop an unrelated test] Signed-off-by: Markus Armbruster --- tests/Makefile | 9 ++++++++- tests/qapi-schema/alternate-clash.err | 2 +- tests/qapi-schema/alternate-clash.json | 9 +++++++-- tests/qapi-schema/args-name-clash.err | 0 tests/qapi-schema/args-name-clash.exit | 1 + tests/qapi-schema/args-name-clash.json | 5 +++++ tests/qapi-schema/args-name-clash.out | 6 ++++++ tests/qapi-schema/duplicate-key.err | 2 +- tests/qapi-schema/duplicate-key.json | 1 + tests/qapi-schema/flat-union-base-union.err | 2 +- tests/qapi-schema/flat-union-base-union.json | 5 ++++- tests/qapi-schema/flat-union-branch-clash.err | 1 - tests/qapi-schema/flat-union-branch-clash.exit | 1 - tests/qapi-schema/flat-union-branch-clash.json | 14 -------------- tests/qapi-schema/flat-union-branch-clash.out | 0 tests/qapi-schema/flat-union-clash-branch.err | 0 tests/qapi-schema/flat-union-clash-branch.exit | 1 + tests/qapi-schema/flat-union-clash-branch.json | 18 ++++++++++++++++++ tests/qapi-schema/flat-union-clash-branch.out | 14 ++++++++++++++ tests/qapi-schema/flat-union-clash-member.err | 1 + tests/qapi-schema/flat-union-clash-member.exit | 1 + tests/qapi-schema/flat-union-clash-member.json | 15 +++++++++++++++ tests/qapi-schema/flat-union-clash-member.out | 0 tests/qapi-schema/flat-union-clash-type.err | 16 ++++++++++++++++ tests/qapi-schema/flat-union-clash-type.exit | 1 + tests/qapi-schema/flat-union-clash-type.json | 16 ++++++++++++++++ tests/qapi-schema/flat-union-clash-type.out | 0 tests/qapi-schema/qapi-schema-test.json | 7 +++++-- tests/qapi-schema/qapi-schema-test.out | 2 ++ tests/qapi-schema/struct-base-clash-base.err | 0 tests/qapi-schema/struct-base-clash-base.exit | 1 + tests/qapi-schema/struct-base-clash-base.json | 9 +++++++++ tests/qapi-schema/struct-base-clash-base.out | 5 +++++ tests/qapi-schema/struct-base-clash-deep.err | 2 +- tests/qapi-schema/struct-base-clash-deep.json | 5 ++++- tests/qapi-schema/struct-base-clash.err | 2 +- tests/qapi-schema/struct-base-clash.json | 3 ++- tests/qapi-schema/union-clash-branches.err | 1 + tests/qapi-schema/union-clash-branches.exit | 1 + tests/qapi-schema/union-clash-branches.json | 5 +++++ tests/qapi-schema/union-clash-branches.out | 0 tests/qapi-schema/union-clash-data.err | 0 tests/qapi-schema/union-clash-data.exit | 1 + tests/qapi-schema/union-clash-data.json | 7 +++++++ tests/qapi-schema/union-clash-data.out | 6 ++++++ tests/qapi-schema/union-clash-type.err | 16 ++++++++++++++++ tests/qapi-schema/union-clash-type.exit | 1 + tests/qapi-schema/union-clash-type.json | 10 ++++++++++ tests/qapi-schema/union-clash-type.out | 0 49 files changed, 196 insertions(+), 29 deletions(-) create mode 100644 tests/qapi-schema/args-name-clash.err create mode 100644 tests/qapi-schema/args-name-clash.exit create mode 100644 tests/qapi-schema/args-name-clash.json create mode 100644 tests/qapi-schema/args-name-clash.out delete mode 100644 tests/qapi-schema/flat-union-branch-clash.err delete mode 100644 tests/qapi-schema/flat-union-branch-clash.exit delete mode 100644 tests/qapi-schema/flat-union-branch-clash.json delete mode 100644 tests/qapi-schema/flat-union-branch-clash.out create mode 100644 tests/qapi-schema/flat-union-clash-branch.err create mode 100644 tests/qapi-schema/flat-union-clash-branch.exit create mode 100644 tests/qapi-schema/flat-union-clash-branch.json create mode 100644 tests/qapi-schema/flat-union-clash-branch.out create mode 100644 tests/qapi-schema/flat-union-clash-member.err create mode 100644 tests/qapi-schema/flat-union-clash-member.exit create mode 100644 tests/qapi-schema/flat-union-clash-member.json create mode 100644 tests/qapi-schema/flat-union-clash-member.out create mode 100644 tests/qapi-schema/flat-union-clash-type.err create mode 100644 tests/qapi-schema/flat-union-clash-type.exit create mode 100644 tests/qapi-schema/flat-union-clash-type.json create mode 100644 tests/qapi-schema/flat-union-clash-type.out create mode 100644 tests/qapi-schema/struct-base-clash-base.err create mode 100644 tests/qapi-schema/struct-base-clash-base.exit create mode 100644 tests/qapi-schema/struct-base-clash-base.json create mode 100644 tests/qapi-schema/struct-base-clash-base.out create mode 100644 tests/qapi-schema/union-clash-branches.err create mode 100644 tests/qapi-schema/union-clash-branches.exit create mode 100644 tests/qapi-schema/union-clash-branches.json create mode 100644 tests/qapi-schema/union-clash-branches.out create mode 100644 tests/qapi-schema/union-clash-data.err create mode 100644 tests/qapi-schema/union-clash-data.exit create mode 100644 tests/qapi-schema/union-clash-data.json create mode 100644 tests/qapi-schema/union-clash-data.out create mode 100644 tests/qapi-schema/union-clash-type.err create mode 100644 tests/qapi-schema/union-clash-type.exit create mode 100644 tests/qapi-schema/union-clash-type.json create mode 100644 tests/qapi-schema/union-clash-type.out (limited to 'tests') diff --git a/tests/Makefile b/tests/Makefile index 164dea3..49fdbe2 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -241,6 +241,7 @@ qapi-schema += args-invalid.json qapi-schema += args-member-array-bad.json qapi-schema += args-member-array.json qapi-schema += args-member-unknown.json +qapi-schema += args-name-clash.json qapi-schema += args-union.json qapi-schema += args-unknown.json qapi-schema += bad-base.json @@ -276,7 +277,9 @@ qapi-schema += flat-union-bad-base.json qapi-schema += flat-union-bad-discriminator.json qapi-schema += flat-union-base-any.json qapi-schema += flat-union-base-union.json -qapi-schema += flat-union-branch-clash.json +qapi-schema += flat-union-clash-branch.json +qapi-schema += flat-union-clash-member.json +qapi-schema += flat-union-clash-type.json qapi-schema += flat-union-inline.json qapi-schema += flat-union-int-branch.json qapi-schema += flat-union-invalid-branch-key.json @@ -318,6 +321,7 @@ qapi-schema += returns-dict.json qapi-schema += returns-int.json qapi-schema += returns-unknown.json qapi-schema += returns-whitelist.json +qapi-schema += struct-base-clash-base.json qapi-schema += struct-base-clash-deep.json qapi-schema += struct-base-clash.json qapi-schema += struct-data-invalid.json @@ -331,6 +335,9 @@ qapi-schema += unclosed-string.json qapi-schema += unicode-str.json qapi-schema += union-bad-branch.json qapi-schema += union-base-no-discriminator.json +qapi-schema += union-clash-branches.json +qapi-schema += union-clash-data.json +qapi-schema += union-clash-type.json qapi-schema += union-invalid-base.json qapi-schema += union-max.json qapi-schema += union-optional-branch.json diff --git a/tests/qapi-schema/alternate-clash.err b/tests/qapi-schema/alternate-clash.err index 51bea3e..a475ab6 100644 --- a/tests/qapi-schema/alternate-clash.err +++ b/tests/qapi-schema/alternate-clash.err @@ -1 +1 @@ -tests/qapi-schema/alternate-clash.json:2: Alternate 'Alt1' member 'ONE' clashes with 'one' +tests/qapi-schema/alternate-clash.json:7: Alternate 'Alt1' member 'a_b' clashes with 'a-b' diff --git a/tests/qapi-schema/alternate-clash.json b/tests/qapi-schema/alternate-clash.json index 3947935..6d73bc5 100644 --- a/tests/qapi-schema/alternate-clash.json +++ b/tests/qapi-schema/alternate-clash.json @@ -1,3 +1,8 @@ -# we detect C enum collisions in an alternate +# Alternate branch name collision +# Reject an alternate that would result in a collision in generated C +# names (this would try to generate two enum values 'ALT1_KIND_A_B'). +# TODO: In the future, if alternates are simplified to not generate +# the implicit Alt1Kind enum, we would still have a collision with the +# resulting C union trying to have two members named 'a_b'. { 'alternate': 'Alt1', - 'data': { 'one': 'str', 'ONE': 'int' } } + 'data': { 'a-b': 'str', 'a_b': 'int' } } diff --git a/tests/qapi-schema/args-name-clash.err b/tests/qapi-schema/args-name-clash.err new file mode 100644 index 0000000..e69de29 diff --git a/tests/qapi-schema/args-name-clash.exit b/tests/qapi-schema/args-name-clash.exit new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/qapi-schema/args-name-clash.exit @@ -0,0 +1 @@ +0 diff --git a/tests/qapi-schema/args-name-clash.json b/tests/qapi-schema/args-name-clash.json new file mode 100644 index 0000000..9e8f889 --- /dev/null +++ b/tests/qapi-schema/args-name-clash.json @@ -0,0 +1,5 @@ +# C member name collision +# FIXME - This parses, but fails to compile, because the C struct is given +# two 'a_b' members. Either reject this at parse time, or munge the C names +# to avoid the collision. +{ 'command': 'oops', 'data': { 'a-b': 'str', 'a_b': 'str' } } diff --git a/tests/qapi-schema/args-name-clash.out b/tests/qapi-schema/args-name-clash.out new file mode 100644 index 0000000..9b2f6e4 --- /dev/null +++ b/tests/qapi-schema/args-name-clash.out @@ -0,0 +1,6 @@ +object :empty +object :obj-oops-arg + member a-b: str optional=False + member a_b: str optional=False +command oops :obj-oops-arg -> None + gen=True success_response=True diff --git a/tests/qapi-schema/duplicate-key.err b/tests/qapi-schema/duplicate-key.err index 768b276..6d02f83 100644 --- a/tests/qapi-schema/duplicate-key.err +++ b/tests/qapi-schema/duplicate-key.err @@ -1 +1 @@ -tests/qapi-schema/duplicate-key.json:2:10: Duplicate key "key" +tests/qapi-schema/duplicate-key.json:3:10: Duplicate key "key" diff --git a/tests/qapi-schema/duplicate-key.json b/tests/qapi-schema/duplicate-key.json index 1b55d88..14ac0e8 100644 --- a/tests/qapi-schema/duplicate-key.json +++ b/tests/qapi-schema/duplicate-key.json @@ -1,2 +1,3 @@ +# QAPI cannot include the same key more than once in any {} { 'key': 'value', 'key': 'value' } diff --git a/tests/qapi-schema/flat-union-base-union.err b/tests/qapi-schema/flat-union-base-union.err index ede9859..28725ed 100644 --- a/tests/qapi-schema/flat-union-base-union.err +++ b/tests/qapi-schema/flat-union-base-union.err @@ -1 +1 @@ -tests/qapi-schema/flat-union-base-union.json:11: Base 'UnionBase' is not a valid struct +tests/qapi-schema/flat-union-base-union.json:14: Base 'UnionBase' is not a valid struct diff --git a/tests/qapi-schema/flat-union-base-union.json b/tests/qapi-schema/flat-union-base-union.json index 6a8ea68..98b4eba 100644 --- a/tests/qapi-schema/flat-union-base-union.json +++ b/tests/qapi-schema/flat-union-base-union.json @@ -1,4 +1,7 @@ -# we require the base to be a struct +# For now, we require the base to be a struct without variants +# TODO: It would be possible to allow a union as a base, as long as all +# permutations of QMP names exposed by base do not clash with any QMP +# member names added by local variants. { 'enum': 'TestEnum', 'data': [ 'value1', 'value2' ] } { 'struct': 'TestTypeA', diff --git a/tests/qapi-schema/flat-union-branch-clash.err b/tests/qapi-schema/flat-union-branch-clash.err deleted file mode 100644 index f112766..0000000 --- a/tests/qapi-schema/flat-union-branch-clash.err +++ /dev/null @@ -1 +0,0 @@ -tests/qapi-schema/flat-union-branch-clash.json:10: Member name 'name' of branch 'value1' clashes with base 'Base' diff --git a/tests/qapi-schema/flat-union-branch-clash.exit b/tests/qapi-schema/flat-union-branch-clash.exit deleted file mode 100644 index d00491f..0000000 --- a/tests/qapi-schema/flat-union-branch-clash.exit +++ /dev/null @@ -1 +0,0 @@ -1 diff --git a/tests/qapi-schema/flat-union-branch-clash.json b/tests/qapi-schema/flat-union-branch-clash.json deleted file mode 100644 index 8fb054f..0000000 --- a/tests/qapi-schema/flat-union-branch-clash.json +++ /dev/null @@ -1,14 +0,0 @@ -# we check for no duplicate keys between branches and base -{ 'enum': 'TestEnum', - 'data': [ 'value1', 'value2' ] } -{ 'struct': 'Base', - 'data': { 'enum1': 'TestEnum', '*name': 'str' } } -{ 'struct': 'Branch1', - 'data': { 'name': 'str' } } -{ 'struct': 'Branch2', - 'data': { 'value': 'int' } } -{ 'union': 'TestUnion', - 'base': 'Base', - 'discriminator': 'enum1', - 'data': { 'value1': 'Branch1', - 'value2': 'Branch2' } } diff --git a/tests/qapi-schema/flat-union-branch-clash.out b/tests/qapi-schema/flat-union-branch-clash.out deleted file mode 100644 index e69de29..0000000 diff --git a/tests/qapi-schema/flat-union-clash-branch.err b/tests/qapi-schema/flat-union-clash-branch.err new file mode 100644 index 0000000..e69de29 diff --git a/tests/qapi-schema/flat-union-clash-branch.exit b/tests/qapi-schema/flat-union-clash-branch.exit new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/qapi-schema/flat-union-clash-branch.exit @@ -0,0 +1 @@ +0 diff --git a/tests/qapi-schema/flat-union-clash-branch.json b/tests/qapi-schema/flat-union-clash-branch.json new file mode 100644 index 0000000..e593336 --- /dev/null +++ b/tests/qapi-schema/flat-union-clash-branch.json @@ -0,0 +1,18 @@ +# Flat union branch name collision +# FIXME: this parses, but then fails to compile due to a duplicate 'c_d' +# (one from the base member, the other from the branch name). We should +# either reject the collision at parse time, or munge the generated branch +# name to allow this to compile. +{ 'enum': 'TestEnum', + 'data': [ 'base', 'c-d' ] } +{ 'struct': 'Base', + 'data': { 'enum1': 'TestEnum', '*c_d': 'str' } } +{ 'struct': 'Branch1', + 'data': { 'string': 'str' } } +{ 'struct': 'Branch2', + 'data': { 'value': 'int' } } +{ 'union': 'TestUnion', + 'base': 'Base', + 'discriminator': 'enum1', + 'data': { 'base': 'Branch1', + 'c-d': 'Branch2' } } diff --git a/tests/qapi-schema/flat-union-clash-branch.out b/tests/qapi-schema/flat-union-clash-branch.out new file mode 100644 index 0000000..8e0da73 --- /dev/null +++ b/tests/qapi-schema/flat-union-clash-branch.out @@ -0,0 +1,14 @@ +object :empty +object Base + member enum1: TestEnum optional=False + member c_d: str optional=True +object Branch1 + member string: str optional=False +object Branch2 + member value: int optional=False +enum TestEnum ['base', 'c-d'] +object TestUnion + base Base + tag enum1 + case base: Branch1 + case c-d: Branch2 diff --git a/tests/qapi-schema/flat-union-clash-member.err b/tests/qapi-schema/flat-union-clash-member.err new file mode 100644 index 0000000..2f0397a --- /dev/null +++ b/tests/qapi-schema/flat-union-clash-member.err @@ -0,0 +1 @@ +tests/qapi-schema/flat-union-clash-member.json:11: Member name 'name' of branch 'value1' clashes with base 'Base' diff --git a/tests/qapi-schema/flat-union-clash-member.exit b/tests/qapi-schema/flat-union-clash-member.exit new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/tests/qapi-schema/flat-union-clash-member.exit @@ -0,0 +1 @@ +1 diff --git a/tests/qapi-schema/flat-union-clash-member.json b/tests/qapi-schema/flat-union-clash-member.json new file mode 100644 index 0000000..9efc771 --- /dev/null +++ b/tests/qapi-schema/flat-union-clash-member.json @@ -0,0 +1,15 @@ +# We check for no duplicate keys between branch members and base +# base's member 'name' clashes with Branch1's +{ 'enum': 'TestEnum', + 'data': [ 'value1', 'value2' ] } +{ 'struct': 'Base', + 'data': { 'enum1': 'TestEnum', '*name': 'str' } } +{ 'struct': 'Branch1', + 'data': { 'name': 'str' } } +{ 'struct': 'Branch2', + 'data': { 'value': 'int' } } +{ 'union': 'TestUnion', + 'base': 'Base', + 'discriminator': 'enum1', + 'data': { 'value1': 'Branch1', + 'value2': 'Branch2' } } diff --git a/tests/qapi-schema/flat-union-clash-member.out b/tests/qapi-schema/flat-union-clash-member.out new file mode 100644 index 0000000..e69de29 diff --git a/tests/qapi-schema/flat-union-clash-type.err b/tests/qapi-schema/flat-union-clash-type.err new file mode 100644 index 0000000..6e64d1d --- /dev/null +++ b/tests/qapi-schema/flat-union-clash-type.err @@ -0,0 +1,16 @@ +Traceback (most recent call last): + File "tests/qapi-schema/test-qapi.py", line 55, in + schema = QAPISchema(sys.argv[1]) + File "scripts/qapi.py", line 1116, in __init__ + self.check() + File "scripts/qapi.py", line 1299, in check + ent.check(self) + File "scripts/qapi.py", line 962, in check + self.variants.check(schema, members, seen) + File "scripts/qapi.py", line 1024, in check + v.check(schema, self.tag_member.type, vseen) + File "scripts/qapi.py", line 1032, in check + QAPISchemaObjectTypeMember.check(self, schema, [], seen) + File "scripts/qapi.py", line 994, in check + assert self.name not in seen +AssertionError diff --git a/tests/qapi-schema/flat-union-clash-type.exit b/tests/qapi-schema/flat-union-clash-type.exit new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/tests/qapi-schema/flat-union-clash-type.exit @@ -0,0 +1 @@ +1 diff --git a/tests/qapi-schema/flat-union-clash-type.json b/tests/qapi-schema/flat-union-clash-type.json new file mode 100644 index 0000000..3db6ea0 --- /dev/null +++ b/tests/qapi-schema/flat-union-clash-type.json @@ -0,0 +1,16 @@ +# Flat union branch 'type' +# FIXME: this triggers an assertion failure. But even with that fixed, +# we would have a clash in generated C, between the member 'type' +# inherited from 'Base' and the branch name 'type' within the +# union. We should either reject this, or munge the generated C to let +# it compile. +{ 'enum': 'TestEnum', + 'data': [ 'type' ] } +{ 'struct': 'Base', + 'data': { 'type': 'TestEnum' } } +{ 'struct': 'Branch1', + 'data': { 'string': 'str' } } +{ 'union': 'TestUnion', + 'base': 'Base', + 'discriminator': 'type', + 'data': { 'type': 'Branch1' } } diff --git a/tests/qapi-schema/flat-union-clash-type.out b/tests/qapi-schema/flat-union-clash-type.out new file mode 100644 index 0000000..e69de29 diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json index 6897a6e..c511338 100644 --- a/tests/qapi-schema/qapi-schema-test.json +++ b/tests/qapi-schema/qapi-schema-test.json @@ -32,11 +32,14 @@ 'dict1': 'UserDefTwoDict' } } # for testing unions +# Among other things, test that a name collision between branches does +# not cause any problems (since only one branch can be in use at a time), +# by intentionally using two branches that both have a C member 'a_b' { 'struct': 'UserDefA', - 'data': { 'boolean': 'bool' } } + 'data': { 'boolean': 'bool', '*a_b': 'int' } } { 'struct': 'UserDefB', - 'data': { 'intb': 'int' } } + 'data': { 'intb': 'int', '*a-b': 'bool' } } { 'union': 'UserDefFlatUnion', 'base': 'UserDefUnionBase', # intentional forward reference diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out index 1f6e858..28a0b3c 100644 --- a/tests/qapi-schema/qapi-schema-test.out +++ b/tests/qapi-schema/qapi-schema-test.out @@ -71,6 +71,7 @@ enum QEnumTwo ['value1', 'value2'] prefix QENUM_TWO object UserDefA member boolean: bool optional=False + member a_b: int optional=True alternate UserDefAlternate case uda: UserDefA case s: str @@ -78,6 +79,7 @@ alternate UserDefAlternate enum UserDefAlternateKind ['uda', 's', 'i'] object UserDefB member intb: int optional=False + member a-b: bool optional=True object UserDefC member string1: str optional=False member string2: str optional=False diff --git a/tests/qapi-schema/struct-base-clash-base.err b/tests/qapi-schema/struct-base-clash-base.err new file mode 100644 index 0000000..e69de29 diff --git a/tests/qapi-schema/struct-base-clash-base.exit b/tests/qapi-schema/struct-base-clash-base.exit new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/qapi-schema/struct-base-clash-base.exit @@ -0,0 +1 @@ +0 diff --git a/tests/qapi-schema/struct-base-clash-base.json b/tests/qapi-schema/struct-base-clash-base.json new file mode 100644 index 0000000..0c84025 --- /dev/null +++ b/tests/qapi-schema/struct-base-clash-base.json @@ -0,0 +1,9 @@ +# Struct member 'base' +# FIXME: this parses, but then fails to compile due to a duplicate 'base' +# (one explicit in QMP, the other used to box the base class members). +# We should either reject the collision at parse time, or change the +# generated struct to allow this to compile. +{ 'struct': 'Base', 'data': {} } +{ 'struct': 'Sub', + 'base': 'Base', + 'data': { 'base': 'str' } } diff --git a/tests/qapi-schema/struct-base-clash-base.out b/tests/qapi-schema/struct-base-clash-base.out new file mode 100644 index 0000000..e69a416 --- /dev/null +++ b/tests/qapi-schema/struct-base-clash-base.out @@ -0,0 +1,5 @@ +object :empty +object Base +object Sub + base Base + member base: str optional=False diff --git a/tests/qapi-schema/struct-base-clash-deep.err b/tests/qapi-schema/struct-base-clash-deep.err index e3e9f8d..f7a25a3 100644 --- a/tests/qapi-schema/struct-base-clash-deep.err +++ b/tests/qapi-schema/struct-base-clash-deep.err @@ -1 +1 @@ -tests/qapi-schema/struct-base-clash-deep.json:7: Member name 'name' clashes with base 'Base' +tests/qapi-schema/struct-base-clash-deep.json:10: Member name 'name' clashes with base 'Base' diff --git a/tests/qapi-schema/struct-base-clash-deep.json b/tests/qapi-schema/struct-base-clash-deep.json index 552fe94..fa873ab 100644 --- a/tests/qapi-schema/struct-base-clash-deep.json +++ b/tests/qapi-schema/struct-base-clash-deep.json @@ -1,4 +1,7 @@ -# we check for no duplicate keys with indirect base +# Reject attempts to duplicate QMP members +# Here, 'name' would have to appear twice on the wire, locally and +# indirectly for the grandparent base; the collision doesn't care that +# one instance is optional. { 'struct': 'Base', 'data': { 'name': 'str' } } { 'struct': 'Mid', diff --git a/tests/qapi-schema/struct-base-clash.err b/tests/qapi-schema/struct-base-clash.err index 3ac37fb..3a9f66b 100644 --- a/tests/qapi-schema/struct-base-clash.err +++ b/tests/qapi-schema/struct-base-clash.err @@ -1 +1 @@ -tests/qapi-schema/struct-base-clash.json:4: Member name 'name' clashes with base 'Base' +tests/qapi-schema/struct-base-clash.json:5: Member name 'name' clashes with base 'Base' diff --git a/tests/qapi-schema/struct-base-clash.json b/tests/qapi-schema/struct-base-clash.json index f2afc9b..11aec80 100644 --- a/tests/qapi-schema/struct-base-clash.json +++ b/tests/qapi-schema/struct-base-clash.json @@ -1,4 +1,5 @@ -# we check for no duplicate keys with base +# Reject attempts to duplicate QMP members +# Here, 'name' would have to appear twice on the wire, locally and for base. { 'struct': 'Base', 'data': { 'name': 'str' } } { 'struct': 'Sub', diff --git a/tests/qapi-schema/union-clash-branches.err b/tests/qapi-schema/union-clash-branches.err new file mode 100644 index 0000000..005c48d --- /dev/null +++ b/tests/qapi-schema/union-clash-branches.err @@ -0,0 +1 @@ +tests/qapi-schema/union-clash-branches.json:4: Union 'TestUnion' member 'a_b' clashes with 'a-b' diff --git a/tests/qapi-schema/union-clash-branches.exit b/tests/qapi-schema/union-clash-branches.exit new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/tests/qapi-schema/union-clash-branches.exit @@ -0,0 +1 @@ +1 diff --git a/tests/qapi-schema/union-clash-branches.json b/tests/qapi-schema/union-clash-branches.json new file mode 100644 index 0000000..31d135f --- /dev/null +++ b/tests/qapi-schema/union-clash-branches.json @@ -0,0 +1,5 @@ +# Union branch name collision +# Reject a union that would result in a collision in generated C names (this +# would try to generate two enum values 'TEST_UNION_KIND_A_B'). +{ 'union': 'TestUnion', + 'data': { 'a-b': 'int', 'a_b': 'str' } } diff --git a/tests/qapi-schema/union-clash-branches.out b/tests/qapi-schema/union-clash-branches.out new file mode 100644 index 0000000..e69de29 diff --git a/tests/qapi-schema/union-clash-data.err b/tests/qapi-schema/union-clash-data.err new file mode 100644 index 0000000..e69de29 diff --git a/tests/qapi-schema/union-clash-data.exit b/tests/qapi-schema/union-clash-data.exit new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/qapi-schema/union-clash-data.exit @@ -0,0 +1 @@ +0 diff --git a/tests/qapi-schema/union-clash-data.json b/tests/qapi-schema/union-clash-data.json new file mode 100644 index 0000000..7308e69 --- /dev/null +++ b/tests/qapi-schema/union-clash-data.json @@ -0,0 +1,7 @@ +# Union branch 'data' +# FIXME: this parses, but then fails to compile due to a duplicate 'data' +# (one from the branch name, another as a filler to avoid an empty union). +# we should either detect the collision at parse time, or change the +# generated struct to allow this to compile. +{ 'union': 'TestUnion', + 'data': { 'data': 'int' } } diff --git a/tests/qapi-schema/union-clash-data.out b/tests/qapi-schema/union-clash-data.out new file mode 100644 index 0000000..6277239 --- /dev/null +++ b/tests/qapi-schema/union-clash-data.out @@ -0,0 +1,6 @@ +object :empty +object :obj-int-wrapper + member data: int optional=False +object TestUnion + case data: :obj-int-wrapper +enum TestUnionKind ['data'] diff --git a/tests/qapi-schema/union-clash-type.err b/tests/qapi-schema/union-clash-type.err new file mode 100644 index 0000000..6e64d1d --- /dev/null +++ b/tests/qapi-schema/union-clash-type.err @@ -0,0 +1,16 @@ +Traceback (most recent call last): + File "tests/qapi-schema/test-qapi.py", line 55, in + schema = QAPISchema(sys.argv[1]) + File "scripts/qapi.py", line 1116, in __init__ + self.check() + File "scripts/qapi.py", line 1299, in check + ent.check(self) + File "scripts/qapi.py", line 962, in check + self.variants.check(schema, members, seen) + File "scripts/qapi.py", line 1024, in check + v.check(schema, self.tag_member.type, vseen) + File "scripts/qapi.py", line 1032, in check + QAPISchemaObjectTypeMember.check(self, schema, [], seen) + File "scripts/qapi.py", line 994, in check + assert self.name not in seen +AssertionError diff --git a/tests/qapi-schema/union-clash-type.exit b/tests/qapi-schema/union-clash-type.exit new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/tests/qapi-schema/union-clash-type.exit @@ -0,0 +1 @@ +1 diff --git a/tests/qapi-schema/union-clash-type.json b/tests/qapi-schema/union-clash-type.json new file mode 100644 index 0000000..52c21f7 --- /dev/null +++ b/tests/qapi-schema/union-clash-type.json @@ -0,0 +1,10 @@ +# Union branch 'type' +# FIXME: this triggers an assertion failure. But even with that fixed, +# we would have a clash in generated C, between the simple union's +# implicit tag member 'kind' and the branch name 'kind' within the +# union. We should either reject this, or munge the generated C to let +# it compile. +# TODO: Even when the generated C is switched to use 'type' rather than +# 'kind', to match the QMP spelling, the collision should still be detected. +{ 'union': 'TestUnion', + 'data': { 'kind': 'int', 'type': 'str' } } diff --git a/tests/qapi-schema/union-clash-type.out b/tests/qapi-schema/union-clash-type.out new file mode 100644 index 0000000..e69de29 -- cgit v1.1 From 7b2a5c2f9a52c4a08630fa741052f03fe5d3cc8a Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Tue, 29 Sep 2015 16:21:04 -0600 Subject: qapi: Avoid assertion failure on union 'type' collision The previous commit added two tests that triggered an assertion failure. It's fairly straightforward to avoid the failure by just outright forbidding the collision between a union's tag values and its discriminator name (including the implicit name 'kind' supplied for simple unions [*]). Ultimately, we'd like to move the collision detection into QAPISchema*.check(), but for now it is easier just to enhance the existing checks. [*] Of course, down the road, we have plans to rename the simple union tag name to 'type' to match the QMP wire name, but the idea of the collision will still be present even then. Technically, we could avoid the collision by naming the C union members representing each enum value as '_case_value' rather than 'value'; but until we have an actual qapi client (and not just our testsuite) that has a legitimate reason to match a case label to the name of a QMP key and needs the name munging to satisfy the compiler, it's easier to just reject the qapi as invalid. Signed-off-by: Eric Blake Message-Id: <1443565276-4535-7-git-send-email-eblake@redhat.com> [Polished a few comments] Signed-off-by: Markus Armbruster --- tests/qapi-schema/flat-union-clash-type.err | 17 +---------------- tests/qapi-schema/flat-union-clash-type.json | 8 +++----- tests/qapi-schema/union-clash-type.err | 17 +---------------- tests/qapi-schema/union-clash-type.json | 9 ++++----- 4 files changed, 9 insertions(+), 42 deletions(-) (limited to 'tests') diff --git a/tests/qapi-schema/flat-union-clash-type.err b/tests/qapi-schema/flat-union-clash-type.err index 6e64d1d..b44dd40 100644 --- a/tests/qapi-schema/flat-union-clash-type.err +++ b/tests/qapi-schema/flat-union-clash-type.err @@ -1,16 +1 @@ -Traceback (most recent call last): - File "tests/qapi-schema/test-qapi.py", line 55, in - schema = QAPISchema(sys.argv[1]) - File "scripts/qapi.py", line 1116, in __init__ - self.check() - File "scripts/qapi.py", line 1299, in check - ent.check(self) - File "scripts/qapi.py", line 962, in check - self.variants.check(schema, members, seen) - File "scripts/qapi.py", line 1024, in check - v.check(schema, self.tag_member.type, vseen) - File "scripts/qapi.py", line 1032, in check - QAPISchemaObjectTypeMember.check(self, schema, [], seen) - File "scripts/qapi.py", line 994, in check - assert self.name not in seen -AssertionError +tests/qapi-schema/flat-union-clash-type.json:11: Discriminator name 'type' collides with enum value in 'TestEnum' diff --git a/tests/qapi-schema/flat-union-clash-type.json b/tests/qapi-schema/flat-union-clash-type.json index 3db6ea0..8f710f0 100644 --- a/tests/qapi-schema/flat-union-clash-type.json +++ b/tests/qapi-schema/flat-union-clash-type.json @@ -1,9 +1,7 @@ # Flat union branch 'type' -# FIXME: this triggers an assertion failure. But even with that fixed, -# we would have a clash in generated C, between the member 'type' -# inherited from 'Base' and the branch name 'type' within the -# union. We should either reject this, or munge the generated C to let -# it compile. +# Reject this, because we would have a clash in generated C, between the +# outer tag 'type' and the branch name 'type' within the union. +# TODO: We could munge the generated C branch name to let it compile. { 'enum': 'TestEnum', 'data': [ 'type' ] } { 'struct': 'Base', diff --git a/tests/qapi-schema/union-clash-type.err b/tests/qapi-schema/union-clash-type.err index 6e64d1d..a5dead1 100644 --- a/tests/qapi-schema/union-clash-type.err +++ b/tests/qapi-schema/union-clash-type.err @@ -1,16 +1 @@ -Traceback (most recent call last): - File "tests/qapi-schema/test-qapi.py", line 55, in - schema = QAPISchema(sys.argv[1]) - File "scripts/qapi.py", line 1116, in __init__ - self.check() - File "scripts/qapi.py", line 1299, in check - ent.check(self) - File "scripts/qapi.py", line 962, in check - self.variants.check(schema, members, seen) - File "scripts/qapi.py", line 1024, in check - v.check(schema, self.tag_member.type, vseen) - File "scripts/qapi.py", line 1032, in check - QAPISchemaObjectTypeMember.check(self, schema, [], seen) - File "scripts/qapi.py", line 994, in check - assert self.name not in seen -AssertionError +tests/qapi-schema/union-clash-type.json:8: Union 'TestUnion' member 'kind' clashes with '(automatic)' diff --git a/tests/qapi-schema/union-clash-type.json b/tests/qapi-schema/union-clash-type.json index 52c21f7..cfc256b 100644 --- a/tests/qapi-schema/union-clash-type.json +++ b/tests/qapi-schema/union-clash-type.json @@ -1,10 +1,9 @@ # Union branch 'type' -# FIXME: this triggers an assertion failure. But even with that fixed, -# we would have a clash in generated C, between the simple union's -# implicit tag member 'kind' and the branch name 'kind' within the -# union. We should either reject this, or munge the generated C to let -# it compile. +# Reject this, because we would have a clash in generated C, between the +# simple union's implicit tag member 'kind' and the branch name 'kind' +# within the union. # TODO: Even when the generated C is switched to use 'type' rather than # 'kind', to match the QMP spelling, the collision should still be detected. +# Or, we could munge the branch name to allow compilation. { 'union': 'TestUnion', 'data': { 'kind': 'int', 'type': 'str' } } -- cgit v1.1 From 8d25dd101f759425456b8005b3180062689d71e7 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Tue, 29 Sep 2015 16:21:05 -0600 Subject: qapi: Add tests for empty unions The documentation claims that alternates are useful for allowing two or more types, although nothing enforces this. Meanwhile, it is silent on whether empty unions are allowed. In practice, the generated code will compile, in part because we have a 'void *data' branch; but attempting to visit such a type will cause an abort(). While there's no technical reason that a degenerate union could not be made to work, it's harder to justify the time spent in chasing known (the current abort() during visit) and unknown corner cases, than it would be to just outlaw them. A future patch will probably take the approach of forbidding them; in the meantime, we can at least add testsuite coverage to make it obvious where things stand. In addition to adding tests to expose the problems, we also need to adjust existing tests that are meant to test something else, but which could fail for the wrong reason if we reject degenerate alternates/unions. Note that empty structs are explicitly supported (for example, right now they are the only way to specify that one branch of a flat union adds no additional members), and empty enums are covered by the testsuite as working (even if they do not seem to have much use). Signed-off-by: Eric Blake Message-Id: <1443565276-4535-8-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster --- tests/Makefile | 3 +++ tests/qapi-schema/alternate-empty.err | 0 tests/qapi-schema/alternate-empty.exit | 1 + tests/qapi-schema/alternate-empty.json | 2 ++ tests/qapi-schema/alternate-empty.out | 4 ++++ tests/qapi-schema/alternate-nested.json | 2 +- tests/qapi-schema/alternate-unknown.json | 2 +- tests/qapi-schema/flat-union-empty.err | 0 tests/qapi-schema/flat-union-empty.exit | 1 + tests/qapi-schema/flat-union-empty.json | 4 ++++ tests/qapi-schema/flat-union-empty.out | 7 +++++++ tests/qapi-schema/union-empty.err | 0 tests/qapi-schema/union-empty.exit | 1 + tests/qapi-schema/union-empty.json | 2 ++ tests/qapi-schema/union-empty.out | 3 +++ 15 files changed, 30 insertions(+), 2 deletions(-) create mode 100644 tests/qapi-schema/alternate-empty.err create mode 100644 tests/qapi-schema/alternate-empty.exit create mode 100644 tests/qapi-schema/alternate-empty.json create mode 100644 tests/qapi-schema/alternate-empty.out create mode 100644 tests/qapi-schema/flat-union-empty.err create mode 100644 tests/qapi-schema/flat-union-empty.exit create mode 100644 tests/qapi-schema/flat-union-empty.json create mode 100644 tests/qapi-schema/flat-union-empty.out create mode 100644 tests/qapi-schema/union-empty.err create mode 100644 tests/qapi-schema/union-empty.exit create mode 100644 tests/qapi-schema/union-empty.json create mode 100644 tests/qapi-schema/union-empty.out (limited to 'tests') diff --git a/tests/Makefile b/tests/Makefile index 49fdbe2..209eca9 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -229,6 +229,7 @@ qapi-schema += alternate-base.json qapi-schema += alternate-clash.json qapi-schema += alternate-conflict-dict.json qapi-schema += alternate-conflict-string.json +qapi-schema += alternate-empty.json qapi-schema += alternate-good.json qapi-schema += alternate-nested.json qapi-schema += alternate-unknown.json @@ -280,6 +281,7 @@ qapi-schema += flat-union-base-union.json qapi-schema += flat-union-clash-branch.json qapi-schema += flat-union-clash-member.json qapi-schema += flat-union-clash-type.json +qapi-schema += flat-union-empty.json qapi-schema += flat-union-inline.json qapi-schema += flat-union-int-branch.json qapi-schema += flat-union-invalid-branch-key.json @@ -338,6 +340,7 @@ qapi-schema += union-base-no-discriminator.json qapi-schema += union-clash-branches.json qapi-schema += union-clash-data.json qapi-schema += union-clash-type.json +qapi-schema += union-empty.json qapi-schema += union-invalid-base.json qapi-schema += union-max.json qapi-schema += union-optional-branch.json diff --git a/tests/qapi-schema/alternate-empty.err b/tests/qapi-schema/alternate-empty.err new file mode 100644 index 0000000..e69de29 diff --git a/tests/qapi-schema/alternate-empty.exit b/tests/qapi-schema/alternate-empty.exit new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/qapi-schema/alternate-empty.exit @@ -0,0 +1 @@ +0 diff --git a/tests/qapi-schema/alternate-empty.json b/tests/qapi-schema/alternate-empty.json new file mode 100644 index 0000000..db3820f --- /dev/null +++ b/tests/qapi-schema/alternate-empty.json @@ -0,0 +1,2 @@ +# FIXME - alternates should list at least two types to be useful +{ 'alternate': 'Alt', 'data': { 'i': 'int' } } diff --git a/tests/qapi-schema/alternate-empty.out b/tests/qapi-schema/alternate-empty.out new file mode 100644 index 0000000..0f153b6 --- /dev/null +++ b/tests/qapi-schema/alternate-empty.out @@ -0,0 +1,4 @@ +object :empty +alternate Alt + case i: int +enum AltKind ['i'] diff --git a/tests/qapi-schema/alternate-nested.json b/tests/qapi-schema/alternate-nested.json index c4233b9..8e22186 100644 --- a/tests/qapi-schema/alternate-nested.json +++ b/tests/qapi-schema/alternate-nested.json @@ -2,4 +2,4 @@ { 'alternate': 'Alt1', 'data': { 'name': 'str', 'value': 'int' } } { 'alternate': 'Alt2', - 'data': { 'nested': 'Alt1' } } + 'data': { 'nested': 'Alt1', 'b': 'bool' } } diff --git a/tests/qapi-schema/alternate-unknown.json b/tests/qapi-schema/alternate-unknown.json index ad5c103..08c80dc 100644 --- a/tests/qapi-schema/alternate-unknown.json +++ b/tests/qapi-schema/alternate-unknown.json @@ -1,3 +1,3 @@ # we reject an alternate with unknown type in branch { 'alternate': 'Alt', - 'data': { 'unknown': 'MissingType' } } + 'data': { 'unknown': 'MissingType', 'i': 'int' } } diff --git a/tests/qapi-schema/flat-union-empty.err b/tests/qapi-schema/flat-union-empty.err new file mode 100644 index 0000000..e69de29 diff --git a/tests/qapi-schema/flat-union-empty.exit b/tests/qapi-schema/flat-union-empty.exit new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/qapi-schema/flat-union-empty.exit @@ -0,0 +1 @@ +0 diff --git a/tests/qapi-schema/flat-union-empty.json b/tests/qapi-schema/flat-union-empty.json new file mode 100644 index 0000000..67dd297 --- /dev/null +++ b/tests/qapi-schema/flat-union-empty.json @@ -0,0 +1,4 @@ +# FIXME - flat unions should not be empty +{ 'enum': 'Empty', 'data': [ ] } +{ 'struct': 'Base', 'data': { 'type': 'Empty' } } +{ 'union': 'Union', 'base': 'Base', 'discriminator': 'type', 'data': { } } diff --git a/tests/qapi-schema/flat-union-empty.out b/tests/qapi-schema/flat-union-empty.out new file mode 100644 index 0000000..0e0665a --- /dev/null +++ b/tests/qapi-schema/flat-union-empty.out @@ -0,0 +1,7 @@ +object :empty +object Base + member type: Empty optional=False +enum Empty [] +object Union + base Base + tag type diff --git a/tests/qapi-schema/union-empty.err b/tests/qapi-schema/union-empty.err new file mode 100644 index 0000000..e69de29 diff --git a/tests/qapi-schema/union-empty.exit b/tests/qapi-schema/union-empty.exit new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/qapi-schema/union-empty.exit @@ -0,0 +1 @@ +0 diff --git a/tests/qapi-schema/union-empty.json b/tests/qapi-schema/union-empty.json new file mode 100644 index 0000000..1785007 --- /dev/null +++ b/tests/qapi-schema/union-empty.json @@ -0,0 +1,2 @@ +# FIXME - unions should not be empty +{ 'union': 'Union', 'data': { } } diff --git a/tests/qapi-schema/union-empty.out b/tests/qapi-schema/union-empty.out new file mode 100644 index 0000000..8b5a7bf --- /dev/null +++ b/tests/qapi-schema/union-empty.out @@ -0,0 +1,3 @@ +object :empty +object Union +enum UnionKind [] -- cgit v1.1 From 9c51b4412959c5331a8a931d848c4b755b5bb36a Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Tue, 29 Sep 2015 16:21:06 -0600 Subject: qapi: Test use of 'number' within alternates 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 Message-Id: <1443565276-4535-9-git-send-email-eblake@redhat.com> [Eric's follow-up fixes squashed in] Signed-off-by: Markus Armbruster --- tests/qapi-schema/qapi-schema-test.json | 8 ++ tests/qapi-schema/qapi-schema-test.out | 24 ++++++ tests/test-qmp-input-visitor.c | 131 +++++++++++++++++++++++++++++++- 3 files changed, 160 insertions(+), 3 deletions(-) (limited to 'tests') diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json index c511338..abe59fd 100644 --- a/tests/qapi-schema/qapi-schema-test.json +++ b/tests/qapi-schema/qapi-schema-test.json @@ -67,6 +67,14 @@ { 'struct': 'UserDefC', 'data': { 'string1': 'str', 'string2': 'str' } } +# for testing use of 'number' within alternates +{ 'alternate': 'AltStrBool', 'data': { 's': 'str', 'b': 'bool' } } +{ 'alternate': 'AltStrNum', 'data': { 's': 'str', 'n': 'number' } } +{ 'alternate': 'AltNumStr', 'data': { 'n': 'number', 's': 'str' } } +{ 'alternate': 'AltStrInt', 'data': { 's': 'str', 'i': 'int' } } +{ 'alternate': 'AltIntNum', 'data': { 'i': 'int', 'n': 'number' } } +{ 'alternate': 'AltNumInt', 'data': { 'n': 'number', 'i': 'int' } } + # for testing native lists { 'union': 'UserDefNativeListUnion', 'data': { 'integer': ['int'], diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out index 28a0b3c..8f81784 100644 --- a/tests/qapi-schema/qapi-schema-test.out +++ b/tests/qapi-schema/qapi-schema-test.out @@ -53,6 +53,30 @@ object :obj-user_def_cmd2-arg object :obj-user_def_cmd3-arg member a: int optional=False member b: int optional=True +alternate AltIntNum + case i: int + case n: number +enum AltIntNumKind ['i', 'n'] +alternate AltNumInt + case n: number + case i: int +enum AltNumIntKind ['n', 'i'] +alternate AltNumStr + case n: number + case s: str +enum AltNumStrKind ['n', 's'] +alternate AltStrBool + case s: str + case b: bool +enum AltStrBoolKind ['s', 'b'] +alternate AltStrInt + case s: str + case i: int +enum AltStrIntKind ['s', 'i'] +alternate AltStrNum + case s: str + case n: number +enum AltStrNumKind ['s', 'n'] event EVENT_A None event EVENT_B None event EVENT_C :obj-EVENT_C-arg diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c index 61715b3..8941963 100644 --- a/tests/test-qmp-input-visitor.c +++ b/tests/test-qmp-input-visitor.c @@ -371,12 +371,135 @@ 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); + + v = visitor_input_test_init(data, "'string'"); + visit_type_UserDefAlternate(v, &tmp, NULL, &error_abort); + g_assert_cmpint(tmp->kind, ==, USER_DEF_ALTERNATE_KIND_S); + g_assert_cmpstr(tmp->s, ==, "string"); + qapi_free_UserDefAlternate(tmp); + visitor_input_teardown(data, NULL); + + v = visitor_input_test_init(data, "false"); + visit_type_UserDefAlternate(v, &tmp, NULL, &err); + g_assert(err); + error_free(err); + err = NULL; + qapi_free_UserDefAlternate(tmp); + visitor_input_teardown(data, NULL); +} + +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); + error_free(err); + err = NULL; + qapi_free_AltStrBool(asb); + visitor_input_teardown(data, 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; + qapi_free_AltStrNum(asn); + visitor_input_teardown(data, NULL); + + v = visitor_input_test_init(data, "42"); + visit_type_AltNumStr(v, &ans, NULL, &error_abort); + g_assert_cmpint(ans->kind, ==, ALT_NUM_STR_KIND_N); + g_assert_cmpfloat(ans->n, ==, 42); + qapi_free_AltNumStr(ans); + visitor_input_teardown(data, NULL); + + v = visitor_input_test_init(data, "42"); + visit_type_AltStrInt(v, &asi, NULL, &error_abort); + g_assert_cmpint(asi->kind, ==, ALT_STR_INT_KIND_I); + g_assert_cmpint(asi->i, ==, 42); + qapi_free_AltStrInt(asi); + visitor_input_teardown(data, NULL); + + v = visitor_input_test_init(data, "42"); + visit_type_AltIntNum(v, &ain, NULL, &error_abort); + g_assert_cmpint(ain->kind, ==, ALT_INT_NUM_KIND_I); + g_assert_cmpint(ain->i, ==, 42); + qapi_free_AltIntNum(ain); + visitor_input_teardown(data, NULL); + + v = visitor_input_test_init(data, "42"); + visit_type_AltNumInt(v, &ani, NULL, &error_abort); + g_assert_cmpint(ani->kind, ==, ALT_NUM_INT_KIND_I); + g_assert_cmpint(ani->i, ==, 42); + qapi_free_AltNumInt(ani); + visitor_input_teardown(data, NULL); + + /* Parsing a double */ + + v = visitor_input_test_init(data, "42.5"); + visit_type_AltStrBool(v, &asb, NULL, &err); + g_assert(err); + error_free(err); + err = NULL; + qapi_free_AltStrBool(asb); + visitor_input_teardown(data, NULL); + + v = visitor_input_test_init(data, "42.5"); + visit_type_AltStrNum(v, &asn, NULL, &error_abort); + g_assert_cmpint(asn->kind, ==, ALT_STR_NUM_KIND_N); + g_assert_cmpfloat(asn->n, ==, 42.5); + qapi_free_AltStrNum(asn); + visitor_input_teardown(data, NULL); + + v = visitor_input_test_init(data, "42.5"); + visit_type_AltNumStr(v, &ans, NULL, &error_abort); + g_assert_cmpint(ans->kind, ==, ALT_NUM_STR_KIND_N); + g_assert_cmpfloat(ans->n, ==, 42.5); + qapi_free_AltNumStr(ans); + visitor_input_teardown(data, NULL); + + v = visitor_input_test_init(data, "42.5"); + visit_type_AltStrInt(v, &asi, NULL, &err); + g_assert(err); + error_free(err); + err = NULL; + qapi_free_AltStrInt(asi); + visitor_input_teardown(data, NULL); + + v = visitor_input_test_init(data, "42.5"); + visit_type_AltIntNum(v, &ain, NULL, &error_abort); + g_assert_cmpint(ain->kind, ==, ALT_INT_NUM_KIND_N); + g_assert_cmpfloat(ain->n, ==, 42.5); + qapi_free_AltIntNum(ain); + visitor_input_teardown(data, NULL); + + v = visitor_input_test_init(data, "42.5"); + visit_type_AltNumInt(v, &ani, NULL, &error_abort); + g_assert_cmpint(ani->kind, ==, ALT_NUM_INT_KIND_N); + g_assert_cmpfloat(ani->n, ==, 42.5); + qapi_free_AltNumInt(ani); + visitor_input_teardown(data, NULL); } static void test_native_list_integer_helper(TestInputVisitorData *data, @@ -720,6 +843,8 @@ int main(int argc, char **argv) &in_visitor_data, test_visitor_in_alternate); input_visitor_test_add("/visitor/input/errors", &in_visitor_data, test_visitor_in_errors); + input_visitor_test_add("/visitor/input/alternate-number", + &in_visitor_data, test_visitor_in_alternate_number); input_visitor_test_add("/visitor/input/native_list/int", &in_visitor_data, test_visitor_in_native_list_int); -- cgit v1.1 From 376863ef4895ae709aadb6f26365a5973310ef09 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Tue, 29 Sep 2015 16:21:07 -0600 Subject: qapi: Reuse code for flat union base validation Rather than open-code the check for a valid base type, we should reuse the common functionality. This allows for consistent error messages, and also makes it easier for a later patch to turn on support for inline anonymous base structures. Test flat-union-inline is updated to test only one feature (anonymous branch dictionaries), which can be implemented independently (test flat-union-bad-base already covers the idea of an anonymous base dictionary). Signed-off-by: Eric Blake Message-Id: <1443565276-4535-10-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster --- tests/qapi-schema/flat-union-bad-base.err | 2 +- tests/qapi-schema/flat-union-base-any.err | 2 +- tests/qapi-schema/flat-union-base-union.err | 2 +- tests/qapi-schema/flat-union-inline.err | 2 +- tests/qapi-schema/flat-union-inline.json | 4 ++-- tests/qapi-schema/flat-union-no-base.err | 2 +- tests/qapi-schema/union-invalid-base.err | 2 +- 7 files changed, 8 insertions(+), 8 deletions(-) (limited to 'tests') diff --git a/tests/qapi-schema/flat-union-bad-base.err b/tests/qapi-schema/flat-union-bad-base.err index f9c31b2..79b8a71 100644 --- a/tests/qapi-schema/flat-union-bad-base.err +++ b/tests/qapi-schema/flat-union-bad-base.err @@ -1 +1 @@ -tests/qapi-schema/flat-union-bad-base.json:9: Flat union 'TestUnion' must have a string base field +tests/qapi-schema/flat-union-bad-base.json:9: 'base' for union 'TestUnion' should be a type name diff --git a/tests/qapi-schema/flat-union-base-any.err b/tests/qapi-schema/flat-union-base-any.err index ad4d629..646f1c9 100644 --- a/tests/qapi-schema/flat-union-base-any.err +++ b/tests/qapi-schema/flat-union-base-any.err @@ -1 +1 @@ -tests/qapi-schema/flat-union-base-any.json:8: Base 'any' is not a valid struct +tests/qapi-schema/flat-union-base-any.json:8: 'base' for union 'TestUnion' cannot use built-in type 'any' diff --git a/tests/qapi-schema/flat-union-base-union.err b/tests/qapi-schema/flat-union-base-union.err index 28725ed..f138395 100644 --- a/tests/qapi-schema/flat-union-base-union.err +++ b/tests/qapi-schema/flat-union-base-union.err @@ -1 +1 @@ -tests/qapi-schema/flat-union-base-union.json:14: Base 'UnionBase' is not a valid struct +tests/qapi-schema/flat-union-base-union.json:14: 'base' for union 'TestUnion' cannot use union type 'UnionBase' diff --git a/tests/qapi-schema/flat-union-inline.err b/tests/qapi-schema/flat-union-inline.err index ec58627..2333358 100644 --- a/tests/qapi-schema/flat-union-inline.err +++ b/tests/qapi-schema/flat-union-inline.err @@ -1 +1 @@ -tests/qapi-schema/flat-union-inline.json:7: Flat union 'TestUnion' must have a string base field +tests/qapi-schema/flat-union-inline.json:7: Member 'value1' of union 'TestUnion' should be a type name diff --git a/tests/qapi-schema/flat-union-inline.json b/tests/qapi-schema/flat-union-inline.json index 6bfdd65..62c7cda 100644 --- a/tests/qapi-schema/flat-union-inline.json +++ b/tests/qapi-schema/flat-union-inline.json @@ -1,11 +1,11 @@ # we require branches to be a struct name -# TODO: should we allow anonymous inline types? +# TODO: should we allow anonymous inline branch types? { 'enum': 'TestEnum', 'data': [ 'value1', 'value2' ] } { 'struct': 'Base', 'data': { 'enum1': 'TestEnum', 'kind': 'str' } } { 'union': 'TestUnion', - 'base': { 'enum1': 'TestEnum', 'kind': 'str' }, + 'base': 'Base', 'discriminator': 'enum1', 'data': { 'value1': { 'string': 'str' }, 'value2': { 'integer': 'int' } } } diff --git a/tests/qapi-schema/flat-union-no-base.err b/tests/qapi-schema/flat-union-no-base.err index bb3f708..841c93b 100644 --- a/tests/qapi-schema/flat-union-no-base.err +++ b/tests/qapi-schema/flat-union-no-base.err @@ -1 +1 @@ -tests/qapi-schema/flat-union-no-base.json:9: Flat union 'TestUnion' must have a string base field +tests/qapi-schema/flat-union-no-base.json:9: Flat union 'TestUnion' must have a base diff --git a/tests/qapi-schema/union-invalid-base.err b/tests/qapi-schema/union-invalid-base.err index 9f63796..03d7b97 100644 --- a/tests/qapi-schema/union-invalid-base.err +++ b/tests/qapi-schema/union-invalid-base.err @@ -1 +1 @@ -tests/qapi-schema/union-invalid-base.json:8: Base 'int' is not a valid struct +tests/qapi-schema/union-invalid-base.json:8: 'base' for union 'TestUnion' cannot use built-in type 'int' -- cgit v1.1