| Message ID | 1374527256-27631-9-git-send-email-lersek@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 07/22/2013 03:07 PM, Laszlo Ersek wrote: > Signed-off-by: Laszlo Ersek <lersek@redhat.com> > --- > tests/Makefile | 6 +- > qapi-schema-test.json | 15 +++ > tests/test-opts-visitor.c | 275 +++++++++++++++++++++++++++++++++++++++++++++ > .gitignore | 1 + > 4 files changed, 296 insertions(+), 1 deletions(-) > create mode 100644 tests/test-opts-visitor.c > + add_test("/visitor/opts/i64/val1/errno", &expect_fail, > + "i64=0x8000000000000000"); > + add_test("/visitor/opts/i64/val1/empty", &expect_fail, "i64="); > + add_test("/visitor/opts/i64/val1/trailing", &expect_fail, "i64=5z"); > + add_test("/visitor/opts/i64/nonlist", &expect_fail, "i64x=5-6"); > + add_test("/visitor/opts/i64/val2/errno", &expect_fail, > + "i64=0x7fffffffffffffff-0x8000000000000000"); > + add_test("/visitor/opts/i64/val2/empty", &expect_fail, "i64=5-"); > + add_test("/visitor/opts/i64/val2/trailing", &expect_fail, "i64=5-6z"); > + add_test("/visitor/opts/i64/range/empty", &expect_fail, "i64=6-5"); > + add_test("/visitor/opts/i64/range/minval", &expect_i64_min, > + "i64=-0x8000000000000000--0x8000000000000000"); > + add_test("/visitor/opts/i64/range/maxval", &expect_i64_max, > + "i64=0x7fffffffffffffff-0x7fffffffffffffff"); Pretty thorough, although I thought of a couple other ideas to test: i64=5z-6 should fail; i64=5-6-7 should fail
On 07/23/13 00:04, Eric Blake wrote: > On 07/22/2013 03:07 PM, Laszlo Ersek wrote: >> Signed-off-by: Laszlo Ersek <lersek@redhat.com> >> --- >> tests/Makefile | 6 +- >> qapi-schema-test.json | 15 +++ >> tests/test-opts-visitor.c | 275 +++++++++++++++++++++++++++++++++++++++++++++ >> .gitignore | 1 + >> 4 files changed, 296 insertions(+), 1 deletions(-) >> create mode 100644 tests/test-opts-visitor.c > >> + add_test("/visitor/opts/i64/val1/errno", &expect_fail, >> + "i64=0x8000000000000000"); >> + add_test("/visitor/opts/i64/val1/empty", &expect_fail, "i64="); >> + add_test("/visitor/opts/i64/val1/trailing", &expect_fail, "i64=5z"); >> + add_test("/visitor/opts/i64/nonlist", &expect_fail, "i64x=5-6"); >> + add_test("/visitor/opts/i64/val2/errno", &expect_fail, >> + "i64=0x7fffffffffffffff-0x8000000000000000"); >> + add_test("/visitor/opts/i64/val2/empty", &expect_fail, "i64=5-"); >> + add_test("/visitor/opts/i64/val2/trailing", &expect_fail, "i64=5-6z"); >> + add_test("/visitor/opts/i64/range/empty", &expect_fail, "i64=6-5"); >> + add_test("/visitor/opts/i64/range/minval", &expect_i64_min, >> + "i64=-0x8000000000000000--0x8000000000000000"); >> + add_test("/visitor/opts/i64/range/maxval", &expect_i64_max, >> + "i64=0x7fffffffffffffff-0x7fffffffffffffff"); > > Pretty thorough, although I thought of a couple other ideas to test: > i64=5z-6 should fail; i64=5-6-7 should fail I can add them if you insist, but I wrote (and single-stepped all of) the test cases so that all branches added by patches 3, 5 and 6 would be covered. (Some of the final tests in this function are actually redundant, but I liked how they looked :)) For example, "i64=5z-6" is no different from "i64=5z", in patch 3 both the first added (*endptr == '\0') condition and the (*endptr == '-') fail the same way for both input strings: we never look past the "z". Likewise, "i64=5-6-7" is the same case as "i64=5-6z": both characters after the "6" (ie. "-" and "z") violate the second added (*endptr == '\0') condition in patch 3 the same way. Do you accept this argument? :) Thanks! Laszlo
On 07/22/2013 04:24 PM, Laszlo Ersek wrote: >> Pretty thorough, although I thought of a couple other ideas to test: >> i64=5z-6 should fail; i64=5-6-7 should fail > > I can add them if you insist, but I wrote (and single-stepped all of) > the test cases so that all branches added by patches 3, 5 and 6 would be > covered. (Some of the final tests in this function are actually > redundant, but I liked how they looked :)) > > For example, "i64=5z-6" is no different from "i64=5z", in patch 3 both > the first added (*endptr == '\0') condition and the (*endptr == '-') > fail the same way for both input strings: we never look past the "z". > > Likewise, "i64=5-6-7" is the same case as "i64=5-6z": both characters > after the "6" (ie. "-" and "z") violate the second added (*endptr == > '\0') condition in patch 3 the same way. > > Do you accept this argument? :) Yes, I can agree you have 100% code coverage as currently coded. Adding what currently forms redundant cases may avoid future patch-writers from breaking 100% coverage while actually triggering different paths between the cases; but at the same time, we can assume such a future patch-writer would be adding some new feature to the parser, and could expand the testsuite accordingly as part of their efforts. So no, I won't insist on a respin :)
On 07/23/13 00:26, Eric Blake wrote: > On 07/22/2013 04:24 PM, Laszlo Ersek wrote: >>> Pretty thorough, although I thought of a couple other ideas to test: >>> i64=5z-6 should fail; i64=5-6-7 should fail >> >> I can add them if you insist, but I wrote (and single-stepped all of) >> the test cases so that all branches added by patches 3, 5 and 6 would be >> covered. (Some of the final tests in this function are actually >> redundant, but I liked how they looked :)) >> >> For example, "i64=5z-6" is no different from "i64=5z", in patch 3 both >> the first added (*endptr == '\0') condition and the (*endptr == '-') >> fail the same way for both input strings: we never look past the "z". >> >> Likewise, "i64=5-6-7" is the same case as "i64=5-6z": both characters >> after the "6" (ie. "-" and "z") violate the second added (*endptr == >> '\0') condition in patch 3 the same way. >> >> Do you accept this argument? :) > > Yes, I can agree you have 100% code coverage as currently coded. Adding > what currently forms redundant cases may avoid future patch-writers from > breaking 100% coverage while actually triggering different paths between > the cases; but at the same time, we can assume such a future > patch-writer would be adding some new feature to the parser, and could > expand the testsuite accordingly as part of their efforts. Agreed! > So no, I > won't insist on a respin :) Thank you very much :) /me bows and scrapes Laszlo
On Mon, 22 Jul 2013 23:07:36 +0200 Laszlo Ersek <lersek@redhat.com> wrote: > > Signed-off-by: Laszlo Ersek <lersek@redhat.com> This patch now conflicts, can you respin please? > --- > tests/Makefile | 6 +- > qapi-schema-test.json | 15 +++ > tests/test-opts-visitor.c | 275 +++++++++++++++++++++++++++++++++++++++++++++ > .gitignore | 1 + > 4 files changed, 296 insertions(+), 1 deletions(-) > create mode 100644 tests/test-opts-visitor.c > > diff --git a/tests/Makefile b/tests/Makefile > index 425a9a8..8bb290e 100644 > --- a/tests/Makefile > +++ b/tests/Makefile > @@ -23,6 +23,8 @@ check-unit-y += tests/test-string-input-visitor$(EXESUF) > gcov-files-test-string-input-visitor-y = qapi/string-input-visitor.c > check-unit-y += tests/test-string-output-visitor$(EXESUF) > gcov-files-test-string-output-visitor-y = qapi/string-output-visitor.c > +check-unit-y += tests/test-opts-visitor$(EXESUF) > +gcov-files-test-opts-visitor-y = qapi/opts-visitor.c > check-unit-y += tests/test-coroutine$(EXESUF) > gcov-files-test-coroutine-y = coroutine-$(CONFIG_COROUTINE_BACKEND).c > check-unit-y += tests/test-visitor-serialization$(EXESUF) > @@ -81,7 +83,8 @@ test-obj-y = tests/check-qint.o tests/check-qstring.o tests/check-qdict.o \ > tests/test-string-input-visitor.o tests/test-qmp-output-visitor.o \ > tests/test-qmp-input-visitor.o tests/test-qmp-input-strict.o \ > tests/test-qmp-commands.o tests/test-visitor-serialization.o \ > - tests/test-x86-cpuid.o tests/test-mul64.o tests/test-int128.o > + tests/test-x86-cpuid.o tests/test-mul64.o tests/test-int128.o \ > + tests/test-opts-visitor.o > > test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o > > @@ -123,6 +126,7 @@ tests/test-qmp-input-visitor$(EXESUF): tests/test-qmp-input-visitor.o $(test-qap > tests/test-qmp-input-strict$(EXESUF): tests/test-qmp-input-strict.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a > tests/test-qmp-commands$(EXESUF): tests/test-qmp-commands.o tests/test-qmp-marshal.o $(test-qapi-obj-y) qapi-types.o qapi-visit.o libqemuutil.a libqemustub.a > tests/test-visitor-serialization$(EXESUF): tests/test-visitor-serialization.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a > +tests/test-opts-visitor$(EXESUF): tests/test-opts-visitor.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a > > tests/test-mul64$(EXESUF): tests/test-mul64.o libqemuutil.a > > diff --git a/qapi-schema-test.json b/qapi-schema-test.json > index 4434fa3..fe5af75 100644 > --- a/qapi-schema-test.json > +++ b/qapi-schema-test.json > @@ -51,3 +51,18 @@ > { 'command': 'user_def_cmd', 'data': {} } > { 'command': 'user_def_cmd1', 'data': {'ud1a': 'UserDefOne'} } > { 'command': 'user_def_cmd2', 'data': {'ud1a': 'UserDefOne', 'ud1b': 'UserDefOne'}, 'returns': 'UserDefTwo' } > + > +# For testing integer range flattening in opts-visitor. The following schema > +# corresponds to the option format: > +# > +# -userdef i64=3-6,i64=-5--1,u64=2,u16=1,u16=7-12 > +# > +# For simplicity, this example doesn't use [type=]discriminator nor optargs > +# specific to discriminator values. > +{ 'type': 'UserDefOptions', > + 'data': { > + '*i64' : [ 'int' ], > + '*u64' : [ 'uint64' ], > + '*u16' : [ 'uint16' ], > + '*i64x': 'int' , > + '*u64x': 'uint64' } } > diff --git a/tests/test-opts-visitor.c b/tests/test-opts-visitor.c > new file mode 100644 > index 0000000..9f902b5 > --- /dev/null > +++ b/tests/test-opts-visitor.c > @@ -0,0 +1,275 @@ > +/* > + * Options Visitor unit-tests. > + * > + * Copyright (C) 2013 Red Hat, Inc. > + * > + * Authors: > + * Laszlo Ersek <lersek@redhat.com> (based on test-string-output-visitor) > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > + * See the COPYING file in the top-level directory. > + */ > + > +#include <glib.h> > + > +#include "qemu/config-file.h" /* qemu_add_opts() */ > +#include "qemu/option.h" /* qemu_opts_parse() */ > +#include "qapi/opts-visitor.h" /* opts_visitor_new() */ > +#include "test-qapi-visit.h" /* visit_type_UserDefOptions() */ > +#include "qapi/dealloc-visitor.h" /* qapi_dealloc_visitor_new() */ > + > +static QemuOptsList userdef_opts = { > + .name = "userdef", > + .head = QTAILQ_HEAD_INITIALIZER(userdef_opts.head), > + .desc = { { 0 } } /* validated with OptsVisitor */ > +}; > + > +/* fixture (= glib test case context) and test case manipulation */ > + > +typedef struct OptsVisitorFixture { > + UserDefOptions *userdef; > + Error *err; > +} OptsVisitorFixture; > + > + > +static void > +setup_fixture(OptsVisitorFixture *f, gconstpointer test_data) > +{ > + const char *opts_string = test_data; > + QemuOpts *opts; > + OptsVisitor *ov; > + > + opts = qemu_opts_parse(qemu_find_opts("userdef"), opts_string, 0); > + g_assert(opts != NULL); > + > + ov = opts_visitor_new(opts); > + visit_type_UserDefOptions(opts_get_visitor(ov), &f->userdef, NULL, > + &f->err); > + opts_visitor_cleanup(ov); > + qemu_opts_del(opts); > +} > + > + > +static void > +teardown_fixture(OptsVisitorFixture *f, gconstpointer test_data) > +{ > + if (f->userdef != NULL) { > + QapiDeallocVisitor *dv; > + > + dv = qapi_dealloc_visitor_new(); > + visit_type_UserDefOptions(qapi_dealloc_get_visitor(dv), &f->userdef, > + NULL, NULL); > + qapi_dealloc_visitor_cleanup(dv); > + } > + error_free(f->err); > +} > + > + > +static void > +add_test(const char *testpath, > + void (*test_func)(OptsVisitorFixture *f, gconstpointer test_data), > + gconstpointer test_data) > +{ > + g_test_add(testpath, OptsVisitorFixture, test_data, setup_fixture, > + test_func, teardown_fixture); > +} > + > +/* test output evaluation */ > + > +static void > +expect_ok(OptsVisitorFixture *f, gconstpointer test_data) > +{ > + g_assert(f->err == NULL); > + g_assert(f->userdef != NULL); > +} > + > + > +static void > +expect_fail(OptsVisitorFixture *f, gconstpointer test_data) > +{ > + g_assert(f->err != NULL); > + > + /* The error message is printed when this test utility is invoked directly > + * (ie. without gtester) and the --verbose flag is passed: > + * > + * tests/test-opts-visitor --verbose > + */ > + g_test_message("'%s': %s", (const char *)test_data, > + error_get_pretty(f->err)); > +} > + > + > +static void > +test_value(OptsVisitorFixture *f, gconstpointer test_data) > +{ > + uint64_t magic, bitval; > + intList *i64; > + uint64List *u64; > + uint16List *u16; > + > + expect_ok(f, test_data); > + > + magic = 0; > + for (i64 = f->userdef->i64; i64 != NULL; i64 = i64->next) { > + g_assert(-16 <= i64->value && i64->value < 64-16); > + bitval = 1ull << (i64->value + 16); > + g_assert((magic & bitval) == 0); > + magic |= bitval; > + } > + g_assert(magic == 0xDEADBEEF); > + > + magic = 0; > + for (u64 = f->userdef->u64; u64 != NULL; u64 = u64->next) { > + g_assert(u64->value < 64); > + bitval = 1ull << u64->value; > + g_assert((magic & bitval) == 0); > + magic |= bitval; > + } > + g_assert(magic == 0xBADC0FFEE0DDF00D); > + > + magic = 0; > + for (u16 = f->userdef->u16; u16 != NULL; u16 = u16->next) { > + g_assert(u16->value < 64); > + bitval = 1ull << u16->value; > + g_assert((magic & bitval) == 0); > + magic |= bitval; > + } > + g_assert(magic == 0xD15EA5E); > +} > + > + > +static void > +expect_i64_min(OptsVisitorFixture *f, gconstpointer test_data) > +{ > + expect_ok(f, test_data); > + g_assert(f->userdef->has_i64); > + g_assert(f->userdef->i64->next == NULL); > + g_assert(f->userdef->i64->value == INT64_MIN); > +} > + > + > +static void > +expect_i64_max(OptsVisitorFixture *f, gconstpointer test_data) > +{ > + expect_ok(f, test_data); > + g_assert(f->userdef->has_i64); > + g_assert(f->userdef->i64->next == NULL); > + g_assert(f->userdef->i64->value == INT64_MAX); > +} > + > + > +static void > +expect_zero(OptsVisitorFixture *f, gconstpointer test_data) > +{ > + expect_ok(f, test_data); > + g_assert(f->userdef->has_u64); > + g_assert(f->userdef->u64->next == NULL); > + g_assert(f->userdef->u64->value == 0); > +} > + > + > +static void > +expect_u64_max(OptsVisitorFixture *f, gconstpointer test_data) > +{ > + expect_ok(f, test_data); > + g_assert(f->userdef->has_u64); > + g_assert(f->userdef->u64->next == NULL); > + g_assert(f->userdef->u64->value == UINT64_MAX); > +} > + > +/* test cases */ > + > +int > +main(int argc, char **argv) > +{ > + g_test_init(&argc, &argv, NULL); > + > + qemu_add_opts(&userdef_opts); > + > + /* Three hexadecimal magic numbers, "dead beef", "bad coffee, odd food" and > + * "disease", from > + * <http://en.wikipedia.org/wiki/Magic_number_%28programming%29>, were > + * converted to binary and dissected into bit ranges. Each magic number is > + * going to be recomposed using the lists called "i64", "u64" and "u16", > + * respectively. > + * > + * (Note that these types pertain to the individual bit shift counts, not > + * the magic numbers themselves; the intent is to exercise opts_type_int() > + * and opts_type_uint64().) > + * > + * The "i64" shift counts have been decreased by 16 (decimal) in order to > + * test negative values as well. Finally, the full list of QemuOpt elements > + * has been permuted with "shuf". > + * > + * Both "i64" and "u64" have some (distinct) single-element ranges > + * represented as both "a" and "a-a". "u16" is a special case of "i64" (see > + * visit_type_uint16()), so it wouldn't add a separate test in this regard. > + */ > + > + add_test("/visitor/opts/flatten/value", &test_value, > + "i64=-1-0,u64=12-16,u64=2-3,i64=-11--9,u64=57,u16=9,i64=5-5," > + "u16=1-4,u16=20,u64=63-63,i64=-16--13,u64=50-52,i64=14-15,u16=11," > + "i64=7,u16=18,i64=2-3,u16=6,u64=54-55,u64=0,u64=18-20,u64=33-43," > + "i64=9-12,u16=26-27,u64=59-61,u16=13-16,u64=29-31,u64=22-23," > + "u16=24,i64=-7--3"); > + > + add_test("/visitor/opts/i64/val1/errno", &expect_fail, > + "i64=0x8000000000000000"); > + add_test("/visitor/opts/i64/val1/empty", &expect_fail, "i64="); > + add_test("/visitor/opts/i64/val1/trailing", &expect_fail, "i64=5z"); > + add_test("/visitor/opts/i64/nonlist", &expect_fail, "i64x=5-6"); > + add_test("/visitor/opts/i64/val2/errno", &expect_fail, > + "i64=0x7fffffffffffffff-0x8000000000000000"); > + add_test("/visitor/opts/i64/val2/empty", &expect_fail, "i64=5-"); > + add_test("/visitor/opts/i64/val2/trailing", &expect_fail, "i64=5-6z"); > + add_test("/visitor/opts/i64/range/empty", &expect_fail, "i64=6-5"); > + add_test("/visitor/opts/i64/range/minval", &expect_i64_min, > + "i64=-0x8000000000000000--0x8000000000000000"); > + add_test("/visitor/opts/i64/range/maxval", &expect_i64_max, > + "i64=0x7fffffffffffffff-0x7fffffffffffffff"); > + > + add_test("/visitor/opts/u64/val1/errno", &expect_fail, "u64=-1"); > + add_test("/visitor/opts/u64/val1/empty", &expect_fail, "u64="); > + add_test("/visitor/opts/u64/val1/trailing", &expect_fail, "u64=5z"); > + add_test("/visitor/opts/u64/nonlist", &expect_fail, "u64x=5-6"); > + add_test("/visitor/opts/u64/val2/errno", &expect_fail, > + "u64=0xffffffffffffffff-0x10000000000000000"); > + add_test("/visitor/opts/u64/val2/empty", &expect_fail, "u64=5-"); > + add_test("/visitor/opts/u64/val2/trailing", &expect_fail, "u64=5-6z"); > + add_test("/visitor/opts/u64/range/empty", &expect_fail, "u64=6-5"); > + add_test("/visitor/opts/u64/range/minval", &expect_zero, "u64=0-0"); > + add_test("/visitor/opts/u64/range/maxval", &expect_u64_max, > + "u64=0xffffffffffffffff-0xffffffffffffffff"); > + > + /* Test maximum range sizes. The macro value is open-coded here > + * *intentionally*; the test case must use concrete values by design. If > + * OPTS_VISITOR_RANGE_MAX is changed, the following values need to be > + * recalculated as well. The assert and this comment should help with it. > + */ > + g_assert(OPTS_VISITOR_RANGE_MAX == 65536); > + > + /* The unsigned case is simple, a u64-u64 difference can always be > + * represented as a u64. > + */ > + add_test("/visitor/opts/u64/range/max", &expect_ok, "u64=0-65535"); > + add_test("/visitor/opts/u64/range/2big", &expect_fail, "u64=0-65536"); > + > + /* The same cannot be said about an i64-i64 difference. */ > + add_test("/visitor/opts/i64/range/max/pos/a", &expect_ok, > + "i64=0x7fffffffffff0000-0x7fffffffffffffff"); > + add_test("/visitor/opts/i64/range/max/pos/b", &expect_ok, > + "i64=0x7ffffffffffeffff-0x7ffffffffffffffe"); > + add_test("/visitor/opts/i64/range/2big/pos", &expect_fail, > + "i64=0x7ffffffffffeffff-0x7fffffffffffffff"); > + add_test("/visitor/opts/i64/range/max/neg/a", &expect_ok, > + "i64=-0x8000000000000000--0x7fffffffffff0001"); > + add_test("/visitor/opts/i64/range/max/neg/b", &expect_ok, > + "i64=-0x7fffffffffffffff--0x7fffffffffff0000"); > + add_test("/visitor/opts/i64/range/2big/neg", &expect_fail, > + "i64=-0x8000000000000000--0x7fffffffffff0000"); > + add_test("/visitor/opts/i64/range/2big/full", &expect_fail, > + "i64=-0x8000000000000000-0x7fffffffffffffff"); > + > + g_test_run(); > + return 0; > +} > diff --git a/.gitignore b/.gitignore > index 388cb45..60a0363 100644 > --- a/.gitignore > +++ b/.gitignore > @@ -47,6 +47,7 @@ vscclient > QMP/qmp-commands.txt > test-coroutine > test-int128 > +test-opts-visitor > test-qmp-input-visitor > test-qmp-output-visitor > test-string-input-visitor
On 08/19/13 21:26, Luiz Capitulino wrote: > On Mon, 22 Jul 2013 23:07:36 +0200 > Laszlo Ersek <lersek@redhat.com> wrote: > >> >> Signed-off-by: Laszlo Ersek <lersek@redhat.com> > > This patch now conflicts, can you respin please? Can you retry with "git am -3"? "git rebase -i" didn't ask me to do anything manually, so I'm guessing if you let "git am" to do a 3-way merge, it should work. (Of course I don't have anything against posting a v2, this is just an attempt to keep the traffic low.) Thanks! Laszlo
On 08/19/13 21:55, Laszlo Ersek wrote: > On 08/19/13 21:26, Luiz Capitulino wrote: >> On Mon, 22 Jul 2013 23:07:36 +0200 >> Laszlo Ersek <lersek@redhat.com> wrote: >> >>> >>> Signed-off-by: Laszlo Ersek <lersek@redhat.com> >> >> This patch now conflicts, can you respin please? > > Can you retry with "git am -3"? > > "git rebase -i" didn't ask me to do anything manually, so I'm guessing > if you let "git am" to do a 3-way merge, it should work. > > (Of course I don't have anything against posting a v2, this is just an > attempt to keep the traffic low.) ... yeah it looks like: - a trivial context change in "tests/Makefile" (due to commit 3464700f), - "qapi-schema-test.json" has been renamed to "tests/qapi-schema/qapi-schema-test.json" (commit 4f193e34): diff --git a/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json similarity index 100% rename from qapi-schema-test.json rename to tests/qapi-schema/qapi-schema-test.json These should be no problem for "git am -3". Thanks, Laszlo
diff --git a/tests/Makefile b/tests/Makefile index 425a9a8..8bb290e 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -23,6 +23,8 @@ check-unit-y += tests/test-string-input-visitor$(EXESUF) gcov-files-test-string-input-visitor-y = qapi/string-input-visitor.c check-unit-y += tests/test-string-output-visitor$(EXESUF) gcov-files-test-string-output-visitor-y = qapi/string-output-visitor.c +check-unit-y += tests/test-opts-visitor$(EXESUF) +gcov-files-test-opts-visitor-y = qapi/opts-visitor.c check-unit-y += tests/test-coroutine$(EXESUF) gcov-files-test-coroutine-y = coroutine-$(CONFIG_COROUTINE_BACKEND).c check-unit-y += tests/test-visitor-serialization$(EXESUF) @@ -81,7 +83,8 @@ test-obj-y = tests/check-qint.o tests/check-qstring.o tests/check-qdict.o \ tests/test-string-input-visitor.o tests/test-qmp-output-visitor.o \ tests/test-qmp-input-visitor.o tests/test-qmp-input-strict.o \ tests/test-qmp-commands.o tests/test-visitor-serialization.o \ - tests/test-x86-cpuid.o tests/test-mul64.o tests/test-int128.o + tests/test-x86-cpuid.o tests/test-mul64.o tests/test-int128.o \ + tests/test-opts-visitor.o test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o @@ -123,6 +126,7 @@ tests/test-qmp-input-visitor$(EXESUF): tests/test-qmp-input-visitor.o $(test-qap tests/test-qmp-input-strict$(EXESUF): tests/test-qmp-input-strict.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a tests/test-qmp-commands$(EXESUF): tests/test-qmp-commands.o tests/test-qmp-marshal.o $(test-qapi-obj-y) qapi-types.o qapi-visit.o libqemuutil.a libqemustub.a tests/test-visitor-serialization$(EXESUF): tests/test-visitor-serialization.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a +tests/test-opts-visitor$(EXESUF): tests/test-opts-visitor.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a tests/test-mul64$(EXESUF): tests/test-mul64.o libqemuutil.a diff --git a/qapi-schema-test.json b/qapi-schema-test.json index 4434fa3..fe5af75 100644 --- a/qapi-schema-test.json +++ b/qapi-schema-test.json @@ -51,3 +51,18 @@ { 'command': 'user_def_cmd', 'data': {} } { 'command': 'user_def_cmd1', 'data': {'ud1a': 'UserDefOne'} } { 'command': 'user_def_cmd2', 'data': {'ud1a': 'UserDefOne', 'ud1b': 'UserDefOne'}, 'returns': 'UserDefTwo' } + +# For testing integer range flattening in opts-visitor. The following schema +# corresponds to the option format: +# +# -userdef i64=3-6,i64=-5--1,u64=2,u16=1,u16=7-12 +# +# For simplicity, this example doesn't use [type=]discriminator nor optargs +# specific to discriminator values. +{ 'type': 'UserDefOptions', + 'data': { + '*i64' : [ 'int' ], + '*u64' : [ 'uint64' ], + '*u16' : [ 'uint16' ], + '*i64x': 'int' , + '*u64x': 'uint64' } } diff --git a/tests/test-opts-visitor.c b/tests/test-opts-visitor.c new file mode 100644 index 0000000..9f902b5 --- /dev/null +++ b/tests/test-opts-visitor.c @@ -0,0 +1,275 @@ +/* + * Options Visitor unit-tests. + * + * Copyright (C) 2013 Red Hat, Inc. + * + * Authors: + * Laszlo Ersek <lersek@redhat.com> (based on test-string-output-visitor) + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +#include <glib.h> + +#include "qemu/config-file.h" /* qemu_add_opts() */ +#include "qemu/option.h" /* qemu_opts_parse() */ +#include "qapi/opts-visitor.h" /* opts_visitor_new() */ +#include "test-qapi-visit.h" /* visit_type_UserDefOptions() */ +#include "qapi/dealloc-visitor.h" /* qapi_dealloc_visitor_new() */ + +static QemuOptsList userdef_opts = { + .name = "userdef", + .head = QTAILQ_HEAD_INITIALIZER(userdef_opts.head), + .desc = { { 0 } } /* validated with OptsVisitor */ +}; + +/* fixture (= glib test case context) and test case manipulation */ + +typedef struct OptsVisitorFixture { + UserDefOptions *userdef; + Error *err; +} OptsVisitorFixture; + + +static void +setup_fixture(OptsVisitorFixture *f, gconstpointer test_data) +{ + const char *opts_string = test_data; + QemuOpts *opts; + OptsVisitor *ov; + + opts = qemu_opts_parse(qemu_find_opts("userdef"), opts_string, 0); + g_assert(opts != NULL); + + ov = opts_visitor_new(opts); + visit_type_UserDefOptions(opts_get_visitor(ov), &f->userdef, NULL, + &f->err); + opts_visitor_cleanup(ov); + qemu_opts_del(opts); +} + + +static void +teardown_fixture(OptsVisitorFixture *f, gconstpointer test_data) +{ + if (f->userdef != NULL) { + QapiDeallocVisitor *dv; + + dv = qapi_dealloc_visitor_new(); + visit_type_UserDefOptions(qapi_dealloc_get_visitor(dv), &f->userdef, + NULL, NULL); + qapi_dealloc_visitor_cleanup(dv); + } + error_free(f->err); +} + + +static void +add_test(const char *testpath, + void (*test_func)(OptsVisitorFixture *f, gconstpointer test_data), + gconstpointer test_data) +{ + g_test_add(testpath, OptsVisitorFixture, test_data, setup_fixture, + test_func, teardown_fixture); +} + +/* test output evaluation */ + +static void +expect_ok(OptsVisitorFixture *f, gconstpointer test_data) +{ + g_assert(f->err == NULL); + g_assert(f->userdef != NULL); +} + + +static void +expect_fail(OptsVisitorFixture *f, gconstpointer test_data) +{ + g_assert(f->err != NULL); + + /* The error message is printed when this test utility is invoked directly + * (ie. without gtester) and the --verbose flag is passed: + * + * tests/test-opts-visitor --verbose + */ + g_test_message("'%s': %s", (const char *)test_data, + error_get_pretty(f->err)); +} + + +static void +test_value(OptsVisitorFixture *f, gconstpointer test_data) +{ + uint64_t magic, bitval; + intList *i64; + uint64List *u64; + uint16List *u16; + + expect_ok(f, test_data); + + magic = 0; + for (i64 = f->userdef->i64; i64 != NULL; i64 = i64->next) { + g_assert(-16 <= i64->value && i64->value < 64-16); + bitval = 1ull << (i64->value + 16); + g_assert((magic & bitval) == 0); + magic |= bitval; + } + g_assert(magic == 0xDEADBEEF); + + magic = 0; + for (u64 = f->userdef->u64; u64 != NULL; u64 = u64->next) { + g_assert(u64->value < 64); + bitval = 1ull << u64->value; + g_assert((magic & bitval) == 0); + magic |= bitval; + } + g_assert(magic == 0xBADC0FFEE0DDF00D); + + magic = 0; + for (u16 = f->userdef->u16; u16 != NULL; u16 = u16->next) { + g_assert(u16->value < 64); + bitval = 1ull << u16->value; + g_assert((magic & bitval) == 0); + magic |= bitval; + } + g_assert(magic == 0xD15EA5E); +} + + +static void +expect_i64_min(OptsVisitorFixture *f, gconstpointer test_data) +{ + expect_ok(f, test_data); + g_assert(f->userdef->has_i64); + g_assert(f->userdef->i64->next == NULL); + g_assert(f->userdef->i64->value == INT64_MIN); +} + + +static void +expect_i64_max(OptsVisitorFixture *f, gconstpointer test_data) +{ + expect_ok(f, test_data); + g_assert(f->userdef->has_i64); + g_assert(f->userdef->i64->next == NULL); + g_assert(f->userdef->i64->value == INT64_MAX); +} + + +static void +expect_zero(OptsVisitorFixture *f, gconstpointer test_data) +{ + expect_ok(f, test_data); + g_assert(f->userdef->has_u64); + g_assert(f->userdef->u64->next == NULL); + g_assert(f->userdef->u64->value == 0); +} + + +static void +expect_u64_max(OptsVisitorFixture *f, gconstpointer test_data) +{ + expect_ok(f, test_data); + g_assert(f->userdef->has_u64); + g_assert(f->userdef->u64->next == NULL); + g_assert(f->userdef->u64->value == UINT64_MAX); +} + +/* test cases */ + +int +main(int argc, char **argv) +{ + g_test_init(&argc, &argv, NULL); + + qemu_add_opts(&userdef_opts); + + /* Three hexadecimal magic numbers, "dead beef", "bad coffee, odd food" and + * "disease", from + * <http://en.wikipedia.org/wiki/Magic_number_%28programming%29>, were + * converted to binary and dissected into bit ranges. Each magic number is + * going to be recomposed using the lists called "i64", "u64" and "u16", + * respectively. + * + * (Note that these types pertain to the individual bit shift counts, not + * the magic numbers themselves; the intent is to exercise opts_type_int() + * and opts_type_uint64().) + * + * The "i64" shift counts have been decreased by 16 (decimal) in order to + * test negative values as well. Finally, the full list of QemuOpt elements + * has been permuted with "shuf". + * + * Both "i64" and "u64" have some (distinct) single-element ranges + * represented as both "a" and "a-a". "u16" is a special case of "i64" (see + * visit_type_uint16()), so it wouldn't add a separate test in this regard. + */ + + add_test("/visitor/opts/flatten/value", &test_value, + "i64=-1-0,u64=12-16,u64=2-3,i64=-11--9,u64=57,u16=9,i64=5-5," + "u16=1-4,u16=20,u64=63-63,i64=-16--13,u64=50-52,i64=14-15,u16=11," + "i64=7,u16=18,i64=2-3,u16=6,u64=54-55,u64=0,u64=18-20,u64=33-43," + "i64=9-12,u16=26-27,u64=59-61,u16=13-16,u64=29-31,u64=22-23," + "u16=24,i64=-7--3"); + + add_test("/visitor/opts/i64/val1/errno", &expect_fail, + "i64=0x8000000000000000"); + add_test("/visitor/opts/i64/val1/empty", &expect_fail, "i64="); + add_test("/visitor/opts/i64/val1/trailing", &expect_fail, "i64=5z"); + add_test("/visitor/opts/i64/nonlist", &expect_fail, "i64x=5-6"); + add_test("/visitor/opts/i64/val2/errno", &expect_fail, + "i64=0x7fffffffffffffff-0x8000000000000000"); + add_test("/visitor/opts/i64/val2/empty", &expect_fail, "i64=5-"); + add_test("/visitor/opts/i64/val2/trailing", &expect_fail, "i64=5-6z"); + add_test("/visitor/opts/i64/range/empty", &expect_fail, "i64=6-5"); + add_test("/visitor/opts/i64/range/minval", &expect_i64_min, + "i64=-0x8000000000000000--0x8000000000000000"); + add_test("/visitor/opts/i64/range/maxval", &expect_i64_max, + "i64=0x7fffffffffffffff-0x7fffffffffffffff"); + + add_test("/visitor/opts/u64/val1/errno", &expect_fail, "u64=-1"); + add_test("/visitor/opts/u64/val1/empty", &expect_fail, "u64="); + add_test("/visitor/opts/u64/val1/trailing", &expect_fail, "u64=5z"); + add_test("/visitor/opts/u64/nonlist", &expect_fail, "u64x=5-6"); + add_test("/visitor/opts/u64/val2/errno", &expect_fail, + "u64=0xffffffffffffffff-0x10000000000000000"); + add_test("/visitor/opts/u64/val2/empty", &expect_fail, "u64=5-"); + add_test("/visitor/opts/u64/val2/trailing", &expect_fail, "u64=5-6z"); + add_test("/visitor/opts/u64/range/empty", &expect_fail, "u64=6-5"); + add_test("/visitor/opts/u64/range/minval", &expect_zero, "u64=0-0"); + add_test("/visitor/opts/u64/range/maxval", &expect_u64_max, + "u64=0xffffffffffffffff-0xffffffffffffffff"); + + /* Test maximum range sizes. The macro value is open-coded here + * *intentionally*; the test case must use concrete values by design. If + * OPTS_VISITOR_RANGE_MAX is changed, the following values need to be + * recalculated as well. The assert and this comment should help with it. + */ + g_assert(OPTS_VISITOR_RANGE_MAX == 65536); + + /* The unsigned case is simple, a u64-u64 difference can always be + * represented as a u64. + */ + add_test("/visitor/opts/u64/range/max", &expect_ok, "u64=0-65535"); + add_test("/visitor/opts/u64/range/2big", &expect_fail, "u64=0-65536"); + + /* The same cannot be said about an i64-i64 difference. */ + add_test("/visitor/opts/i64/range/max/pos/a", &expect_ok, + "i64=0x7fffffffffff0000-0x7fffffffffffffff"); + add_test("/visitor/opts/i64/range/max/pos/b", &expect_ok, + "i64=0x7ffffffffffeffff-0x7ffffffffffffffe"); + add_test("/visitor/opts/i64/range/2big/pos", &expect_fail, + "i64=0x7ffffffffffeffff-0x7fffffffffffffff"); + add_test("/visitor/opts/i64/range/max/neg/a", &expect_ok, + "i64=-0x8000000000000000--0x7fffffffffff0001"); + add_test("/visitor/opts/i64/range/max/neg/b", &expect_ok, + "i64=-0x7fffffffffffffff--0x7fffffffffff0000"); + add_test("/visitor/opts/i64/range/2big/neg", &expect_fail, + "i64=-0x8000000000000000--0x7fffffffffff0000"); + add_test("/visitor/opts/i64/range/2big/full", &expect_fail, + "i64=-0x8000000000000000-0x7fffffffffffffff"); + + g_test_run(); + return 0; +} diff --git a/.gitignore b/.gitignore index 388cb45..60a0363 100644 --- a/.gitignore +++ b/.gitignore @@ -47,6 +47,7 @@ vscclient QMP/qmp-commands.txt test-coroutine test-int128 +test-opts-visitor test-qmp-input-visitor test-qmp-output-visitor test-string-input-visitor
Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- tests/Makefile | 6 +- qapi-schema-test.json | 15 +++ tests/test-opts-visitor.c | 275 +++++++++++++++++++++++++++++++++++++++++++++ .gitignore | 1 + 4 files changed, 296 insertions(+), 1 deletions(-) create mode 100644 tests/test-opts-visitor.c