All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: Michael Roth <mdroth@linux.vnet.ibm.com>,
	marcandre.lureau@redhat.com, armbru@redhat.com,
	ehabkost@redhat.com
Subject: [Qemu-devel] [PATCH v7 08/18] qapi: Test use of 'number' within alternates
Date: Tue, 29 Sep 2015 16:21:06 -0600	[thread overview]
Message-ID: <1443565276-4535-9-git-send-email-eblake@redhat.com> (raw)
In-Reply-To: <1443565276-4535-1-git-send-email-eblake@redhat.com>

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 <eblake@redhat.com>

---
v6: Rename from AltOne to AltStrBool (and so forth), drop
dead 'one = NULL' assignments, avoid leaking memory of
test data, tweak failing tests to give better hints of what
desired behavior is
---
 tests/qapi-schema/qapi-schema-test.json |   8 ++
 tests/qapi-schema/qapi-schema-test.out  |  24 ++++++
 tests/test-qmp-input-visitor.c          | 129 +++++++++++++++++++++++++++++++-
 3 files changed, 158 insertions(+), 3 deletions(-)

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..641a3ab 100644
--- a/tests/test-qmp-input-visitor.c
+++ 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);
+
+    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);
+    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_cmpint(ani->n, ==, 42.5);
+    qapi_free_AltNumInt(ani);
+    visitor_input_teardown(data, NULL);
 }

 static void test_native_list_integer_helper(TestInputVisitorData *data,
@@ -720,6 +841,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);
-- 
2.4.3

  parent reply	other threads:[~2015-09-29 22:21 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-29 22:20 [Qemu-devel] [PATCH v7 00/18] post-introspection cleanups, subset A Eric Blake
2015-09-29 22:20 ` [Qemu-devel] [PATCH v7 01/18] qapi: Sort qapi-schema tests Eric Blake
2015-09-29 22:21 ` [Qemu-devel] [PATCH v7 02/18] qapi: Improve 'include' error message Eric Blake
2015-09-29 22:21 ` [Qemu-devel] [PATCH v7 03/18] qapi: Invoke exception superclass initializer Eric Blake
2015-09-29 22:21 ` [Qemu-devel] [PATCH v7 04/18] qapi: Clean up qapi.py per pep8 Eric Blake
2015-09-29 22:21 ` [Qemu-devel] [PATCH v7 05/18] qapi: Test for various name collisions Eric Blake
2015-10-01  3:30   ` Eric Blake
2015-10-01 12:57     ` Markus Armbruster
2015-10-01 11:51   ` Markus Armbruster
2015-10-01 13:10     ` Eric Blake
2015-10-01 15:34       ` Markus Armbruster
2015-10-01 15:41         ` Eric Blake
2015-10-01 17:39           ` Markus Armbruster
2015-10-01 18:51             ` Eric Blake
2015-10-01 20:27               ` Markus Armbruster
2015-09-29 22:21 ` [Qemu-devel] [PATCH v7 06/18] qapi: Avoid assertion failure on union 'type' collision Eric Blake
2015-10-01 12:10   ` Markus Armbruster
2015-09-29 22:21 ` [Qemu-devel] [PATCH v7 07/18] qapi: Add tests for empty unions Eric Blake
2015-09-29 22:21 ` Eric Blake [this message]
2015-10-05 22:45   ` [Qemu-devel] [PATCH v7 08/18] qapi: Test use of 'number' within alternates Eric Blake
2015-10-06  7:24     ` Markus Armbruster
2015-09-29 22:21 ` [Qemu-devel] [PATCH v7 09/18] qapi: Reuse code for flat union base validation Eric Blake
2015-09-29 22:21 ` [Qemu-devel] [PATCH v7 10/18] qapi: Consistent generated code: prefer error 'err' Eric Blake
2015-09-29 22:21 ` [Qemu-devel] [PATCH v7 11/18] qapi: Consistent generated code: prefer visitor 'v' Eric Blake
2015-09-29 22:21 ` [Qemu-devel] [PATCH v7 12/18] qapi: Consistent generated code: prefer common labels Eric Blake
2015-09-29 22:21 ` [Qemu-devel] [PATCH v7 13/18] qapi: Consistent generated code: prefer common indentation Eric Blake
2015-09-29 22:21 ` [Qemu-devel] [PATCH v7 14/18] qapi: Consistent generated code: minimize push_indent() usage Eric Blake
2015-09-29 22:21 ` [Qemu-devel] [PATCH v7 15/18] qapi: Share gen_err_check() Eric Blake
2015-10-01 12:40   ` Markus Armbruster
2015-10-01 13:01     ` Eric Blake
2015-09-29 22:21 ` [Qemu-devel] [PATCH v7 16/18] qapi: Share gen_visit_fields() Eric Blake
2015-09-29 22:21 ` [Qemu-devel] [RFC PATCH v7 17/18] qapi: Simplify gen_visit_fields() error handling Eric Blake
2015-10-01 12:49   ` Markus Armbruster
2015-09-29 22:21 ` [Qemu-devel] [RFC PATCH v7 18/18] qapi: Use gen_err_check() in more places Eric Blake

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1443565276-4535-9-git-send-email-eblake@redhat.com \
    --to=eblake@redhat.com \
    --cc=armbru@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.