From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:53105) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RkKht-0008Eo-6c for qemu-devel@nongnu.org; Mon, 09 Jan 2012 14:16:35 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RkKhr-0000ok-7y for qemu-devel@nongnu.org; Mon, 09 Jan 2012 14:16:33 -0500 Received: from e4.ny.us.ibm.com ([32.97.182.144]:33251) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RkKhr-0000oU-3m for qemu-devel@nongnu.org; Mon, 09 Jan 2012 14:16:31 -0500 Received: from /spool/local by e4.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 9 Jan 2012 14:16:26 -0500 Received: from d01av01.pok.ibm.com (d01av01.pok.ibm.com [9.56.224.215]) by d01relay04.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q09JFTeB343630 for ; Mon, 9 Jan 2012 14:15:29 -0500 Received: from d01av01.pok.ibm.com (loopback [127.0.0.1]) by d01av01.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q09JFS2I019615 for ; Mon, 9 Jan 2012 14:15:28 -0500 Message-ID: <4F0B3CCF.7030808@us.ibm.com> Date: Mon, 09 Jan 2012 13:15:27 -0600 From: Anthony Liguori MIME-Version: 1.0 References: <1326124572-8312-1-git-send-email-aliguori@us.ibm.com> <1326124572-8312-3-git-send-email-aliguori@us.ibm.com> <20120109162732.58e364ad@doriath> In-Reply-To: <20120109162732.58e364ad@doriath> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 03/11] check-qdict: convert to gtest List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: qemu-devel@nongnu.org On 01/09/2012 12:27 PM, Luiz Capitulino wrote: >> -START_TEST(qdict_put_obj_test) >> +static void qdict_put_obj_test(void) >> { >> QInt *qi; >> QDict *qdict; >> @@ -49,10 +48,10 @@ START_TEST(qdict_put_obj_test) >> // key "" will have tdb hash 12345 >> qdict_put_obj(qdict, "", QOBJECT(qint_from_int(num))); >> >> - fail_unless(qdict_size(qdict) == 1); >> + g_assert(qdict_size(qdict) == 1); >> ent = QLIST_FIRST(&qdict->table[12345 % QDICT_BUCKET_MAX]); >> qi = qobject_to_qint(ent->value); >> - fail_unless(qint_get_int(qi) == num); >> + g_assert(qint_get_int(qi) == num); > > Would be nice to use g_assert_cmpint()& friends, but I can do it later... Yup. >> -static QDict *tests_dict = NULL; >> - >> -static void qdict_setup(void) >> -{ >> - tests_dict = qdict_new(); >> - fail_unless(tests_dict != NULL); >> -} >> >> -static void qdict_teardown(void) >> -{ >> - QDECREF(tests_dict); >> - tests_dict = NULL; >> -} >> - > > Why not use setup/teardown in the gtest version too? This is not hugely > important, but I don't see why duplicate the code in all tests either. Mainly because an emacs macro actually did the conversion so it was easier to make everything look the same and then run the macro. It didn't seem significant enough to worry that much about. But of course, we can change it after the fact too. > Also, wouldn't be better to rename this to test-qdict (and the others too)? Yes, but we should also find a sane way to express the build rules. Right now, it's a major hassle to figure out what unit tests need to link against. I figure that in the not too distant future, I'll take a pass at the build rules and along the way, rename everything so we can have an implicit rule like: test-%: test-%.o $(test-objs-y) But I don't know yet what that rule is going to look like so I think we should wait to do the rename. Regards, Anthony Liguori > Otherwise the whole series look good, you can add: > > Reviewed-by: Luiz Capitulino > > If you think it's worth it. > >> -START_TEST(qdict_get_test) >> +static void qdict_get_test(void) >> { >> QInt *qi; >> QObject *obj; >> const int value = -42; >> const char *key = "test"; >> + QDict *tests_dict = qdict_new(); >> >> qdict_put(tests_dict, key, qint_from_int(value)); >> >> obj = qdict_get(tests_dict, key); >> - fail_unless(obj != NULL); >> + g_assert(obj != NULL); >> >> qi = qobject_to_qint(obj); >> - fail_unless(qint_get_int(qi) == value); >> + g_assert(qint_get_int(qi) == value); >> + >> + QDECREF(tests_dict); >> } >> -END_TEST >> >> -START_TEST(qdict_get_int_test) >> +static void qdict_get_int_test(void) >> { >> int ret; >> const int value = 100; >> const char *key = "int"; >> + QDict *tests_dict = qdict_new(); >> >> qdict_put(tests_dict, key, qint_from_int(value)); >> >> ret = qdict_get_int(tests_dict, key); >> - fail_unless(ret == value); >> + g_assert(ret == value); >> + >> + QDECREF(tests_dict); >> } >> -END_TEST >> >> -START_TEST(qdict_get_try_int_test) >> +static void qdict_get_try_int_test(void) >> { >> int ret; >> const int value = 100; >> const char *key = "int"; >> + QDict *tests_dict = qdict_new(); >> >> qdict_put(tests_dict, key, qint_from_int(value)); >> >> ret = qdict_get_try_int(tests_dict, key, 0); >> - fail_unless(ret == value); >> + g_assert(ret == value); >> + >> + QDECREF(tests_dict); >> } >> -END_TEST >> >> -START_TEST(qdict_get_str_test) >> +static void qdict_get_str_test(void) >> { >> const char *p; >> const char *key = "key"; >> const char *str = "string"; >> + QDict *tests_dict = qdict_new(); >> >> qdict_put(tests_dict, key, qstring_from_str(str)); >> >> p = qdict_get_str(tests_dict, key); >> - fail_unless(p != NULL); >> - fail_unless(strcmp(p, str) == 0); >> + g_assert(p != NULL); >> + g_assert(strcmp(p, str) == 0); >> + >> + QDECREF(tests_dict); >> } >> -END_TEST >> >> -START_TEST(qdict_get_try_str_test) >> +static void qdict_get_try_str_test(void) >> { >> const char *p; >> const char *key = "key"; >> const char *str = "string"; >> + QDict *tests_dict = qdict_new(); >> >> qdict_put(tests_dict, key, qstring_from_str(str)); >> >> p = qdict_get_try_str(tests_dict, key); >> - fail_unless(p != NULL); >> - fail_unless(strcmp(p, str) == 0); >> + g_assert(p != NULL); >> + g_assert(strcmp(p, str) == 0); >> + >> + QDECREF(tests_dict); >> } >> -END_TEST >> >> -START_TEST(qdict_haskey_not_test) >> +static void qdict_haskey_not_test(void) >> { >> - fail_unless(qdict_haskey(tests_dict, "test") == 0); >> + QDict *tests_dict = qdict_new(); >> + g_assert(qdict_haskey(tests_dict, "test") == 0); >> + >> + QDECREF(tests_dict); >> } >> -END_TEST >> >> -START_TEST(qdict_haskey_test) >> +static void qdict_haskey_test(void) >> { >> const char *key = "test"; >> + QDict *tests_dict = qdict_new(); >> >> qdict_put(tests_dict, key, qint_from_int(0)); >> - fail_unless(qdict_haskey(tests_dict, key) == 1); >> + g_assert(qdict_haskey(tests_dict, key) == 1); >> + >> + QDECREF(tests_dict); >> } >> -END_TEST >> >> -START_TEST(qdict_del_test) >> +static void qdict_del_test(void) >> { >> const char *key = "key test"; >> + QDict *tests_dict = qdict_new(); >> >> qdict_put(tests_dict, key, qstring_from_str("foo")); >> - fail_unless(qdict_size(tests_dict) == 1); >> + g_assert(qdict_size(tests_dict) == 1); >> >> qdict_del(tests_dict, key); >> >> - fail_unless(qdict_size(tests_dict) == 0); >> - fail_unless(qdict_haskey(tests_dict, key) == 0); >> + g_assert(qdict_size(tests_dict) == 0); >> + g_assert(qdict_haskey(tests_dict, key) == 0); >> + >> + QDECREF(tests_dict); >> } >> -END_TEST >> >> -START_TEST(qobject_to_qdict_test) >> +static void qobject_to_qdict_test(void) >> { >> - fail_unless(qobject_to_qdict(QOBJECT(tests_dict)) == tests_dict); >> + QDict *tests_dict = qdict_new(); >> + g_assert(qobject_to_qdict(QOBJECT(tests_dict)) == tests_dict); >> + >> + QDECREF(tests_dict); >> } >> -END_TEST >> >> -START_TEST(qdict_iterapi_test) >> +static void qdict_iterapi_test(void) >> { >> int count; >> const QDictEntry *ent; >> + QDict *tests_dict = qdict_new(); >> >> - fail_unless(qdict_first(tests_dict) == NULL); >> + g_assert(qdict_first(tests_dict) == NULL); >> >> qdict_put(tests_dict, "key1", qint_from_int(1)); >> qdict_put(tests_dict, "key2", qint_from_int(2)); >> @@ -207,47 +209,52 @@ START_TEST(qdict_iterapi_test) >> >> count = 0; >> for (ent = qdict_first(tests_dict); ent; ent = qdict_next(tests_dict, ent)){ >> - fail_unless(qdict_haskey(tests_dict, qdict_entry_key(ent)) == 1); >> + g_assert(qdict_haskey(tests_dict, qdict_entry_key(ent)) == 1); >> count++; >> } >> >> - fail_unless(count == qdict_size(tests_dict)); >> + g_assert(count == qdict_size(tests_dict)); >> >> /* Do it again to test restarting */ >> count = 0; >> for (ent = qdict_first(tests_dict); ent; ent = qdict_next(tests_dict, ent)){ >> - fail_unless(qdict_haskey(tests_dict, qdict_entry_key(ent)) == 1); >> + g_assert(qdict_haskey(tests_dict, qdict_entry_key(ent)) == 1); >> count++; >> } >> >> - fail_unless(count == qdict_size(tests_dict)); >> + g_assert(count == qdict_size(tests_dict)); >> + >> + QDECREF(tests_dict); >> } >> -END_TEST >> >> /* >> * Errors test-cases >> */ >> >> -START_TEST(qdict_put_exists_test) >> +static void qdict_put_exists_test(void) >> { >> int value; >> const char *key = "exists"; >> + QDict *tests_dict = qdict_new(); >> >> qdict_put(tests_dict, key, qint_from_int(1)); >> qdict_put(tests_dict, key, qint_from_int(2)); >> >> value = qdict_get_int(tests_dict, key); >> - fail_unless(value == 2); >> + g_assert(value == 2); >> >> - fail_unless(qdict_size(tests_dict) == 1); >> + g_assert(qdict_size(tests_dict) == 1); >> + >> + QDECREF(tests_dict); >> } >> -END_TEST >> >> -START_TEST(qdict_get_not_exists_test) >> +static void qdict_get_not_exists_test(void) >> { >> - fail_unless(qdict_get(tests_dict, "foo") == NULL); >> + QDict *tests_dict = qdict_new(); >> + g_assert(qdict_get(tests_dict, "foo") == NULL); >> + >> + QDECREF(tests_dict); >> } >> -END_TEST >> >> /* >> * Stress test-case >> @@ -276,7 +283,7 @@ static QString *read_line(FILE *file, char *key) >> >> #define reset_file(file) fseek(file, 0L, SEEK_SET) >> >> -START_TEST(qdict_stress_test) >> +static void qdict_stress_test(void) >> { >> size_t lines; >> char key[128]; >> @@ -286,11 +293,11 @@ START_TEST(qdict_stress_test) >> const char *test_file_path = "qdict-test-data.txt"; >> >> test_file = fopen(test_file_path, "r"); >> - fail_unless(test_file != NULL); >> + g_assert(test_file != NULL); >> >> // Create the dict >> qdict = qdict_new(); >> - fail_unless(qdict != NULL); >> + g_assert(qdict != NULL); >> >> // Add everything from the test file >> for (lines = 0;; lines++) { >> @@ -300,7 +307,7 @@ START_TEST(qdict_stress_test) >> >> qdict_put(qdict, key, value); >> } >> - fail_unless(qdict_size(qdict) == lines); >> + g_assert(qdict_size(qdict) == lines); >> >> // Check if everything is really in there >> reset_file(test_file); >> @@ -314,9 +321,9 @@ START_TEST(qdict_stress_test) >> str1 = qstring_get_str(value); >> >> str2 = qdict_get_str(qdict, key); >> - fail_unless(str2 != NULL); >> + g_assert(str2 != NULL); >> >> - fail_unless(strcmp(str1, str2) == 0); >> + g_assert(strcmp(str1, str2) == 0); >> >> QDECREF(value); >> } >> @@ -331,72 +338,41 @@ START_TEST(qdict_stress_test) >> qdict_del(qdict, key); >> QDECREF(value); >> >> - fail_unless(qdict_haskey(qdict, key) == 0); >> + g_assert(qdict_haskey(qdict, key) == 0); >> } >> fclose(test_file); >> >> - fail_unless(qdict_size(qdict) == 0); >> + g_assert(qdict_size(qdict) == 0); >> QDECREF(qdict); >> } >> -END_TEST >> >> -static Suite *qdict_suite(void) >> +int main(int argc, char **argv) >> { >> - Suite *s; >> - TCase *qdict_public_tcase; >> - TCase *qdict_public2_tcase; >> - TCase *qdict_stress_tcase; >> - TCase *qdict_errors_tcase; >> - >> - s = suite_create("QDict test-suite"); >> + g_test_init(&argc,&argv, NULL); >> >> - qdict_public_tcase = tcase_create("Public Interface"); >> - suite_add_tcase(s, qdict_public_tcase); >> - tcase_add_test(qdict_public_tcase, qdict_new_test); >> - tcase_add_test(qdict_public_tcase, qdict_put_obj_test); >> - tcase_add_test(qdict_public_tcase, qdict_destroy_simple_test); >> + g_test_add_func("/public/new", qdict_new_test); >> + g_test_add_func("/public/put_obj", qdict_put_obj_test); >> + g_test_add_func("/public/destroy_simple", qdict_destroy_simple_test); >> >> /* Continue, but now with fixtures */ >> - qdict_public2_tcase = tcase_create("Public Interface (2)"); >> - suite_add_tcase(s, qdict_public2_tcase); >> - tcase_add_checked_fixture(qdict_public2_tcase, qdict_setup, qdict_teardown); >> - tcase_add_test(qdict_public2_tcase, qdict_get_test); >> - tcase_add_test(qdict_public2_tcase, qdict_get_int_test); >> - tcase_add_test(qdict_public2_tcase, qdict_get_try_int_test); >> - tcase_add_test(qdict_public2_tcase, qdict_get_str_test); >> - tcase_add_test(qdict_public2_tcase, qdict_get_try_str_test); >> - tcase_add_test(qdict_public2_tcase, qdict_haskey_not_test); >> - tcase_add_test(qdict_public2_tcase, qdict_haskey_test); >> - tcase_add_test(qdict_public2_tcase, qdict_del_test); >> - tcase_add_test(qdict_public2_tcase, qobject_to_qdict_test); >> - tcase_add_test(qdict_public2_tcase, qdict_iterapi_test); >> - >> - qdict_errors_tcase = tcase_create("Errors"); >> - suite_add_tcase(s, qdict_errors_tcase); >> - tcase_add_checked_fixture(qdict_errors_tcase, qdict_setup, qdict_teardown); >> - tcase_add_test(qdict_errors_tcase, qdict_put_exists_test); >> - tcase_add_test(qdict_errors_tcase, qdict_get_not_exists_test); >> + g_test_add_func("/public/get", qdict_get_test); >> + g_test_add_func("/public/get_int", qdict_get_int_test); >> + g_test_add_func("/public/get_try_int", qdict_get_try_int_test); >> + g_test_add_func("/public/get_str", qdict_get_str_test); >> + g_test_add_func("/public/get_try_str", qdict_get_try_str_test); >> + g_test_add_func("/public/haskey_not", qdict_haskey_not_test); >> + g_test_add_func("/public/haskey", qdict_haskey_test); >> + g_test_add_func("/public/del", qdict_del_test); >> + g_test_add_func("/public/to_qdict", qobject_to_qdict_test); >> + g_test_add_func("/public/iterapi", qdict_iterapi_test); >> + >> + g_test_add_func("/errors/put_exists", qdict_put_exists_test); >> + g_test_add_func("/errors/get_not_exists", qdict_get_not_exists_test); >> >> /* The Big one */ >> - qdict_stress_tcase = tcase_create("Stress Test"); >> - suite_add_tcase(s, qdict_stress_tcase); >> - tcase_add_test(qdict_stress_tcase, qdict_stress_test); >> - >> - return s; >> -} >> - >> -int main(void) >> -{ >> - int nf; >> - Suite *s; >> - SRunner *sr; >> - >> - s = qdict_suite(); >> - sr = srunner_create(s); >> - >> - srunner_run_all(sr, CK_NORMAL); >> - nf = srunner_ntests_failed(sr); >> - srunner_free(sr); >> + if (g_test_slow()) { >> + g_test_add_func("/stress/test", qdict_stress_test); >> + } >> >> - return (nf == 0) ? EXIT_SUCCESS : EXIT_FAILURE; >> + return g_test_run(); >> } >> diff --git a/configure b/configure >> index 79790b3..7e3f28f 100755 >> --- a/configure >> +++ b/configure >> @@ -2800,10 +2800,10 @@ if test "$softmmu" = yes ; then >> tools="qemu-ga\$(EXESUF) $tools" >> fi >> if [ "$check_utests" = "yes" ]; then >> - checks="check-qint check-qstring check-qdict check-qlist" >> + checks="check-qint check-qstring check-qlist" >> checks="check-qfloat check-qjson $checks" >> fi >> - test_progs="$checks test-coroutine test-qmp-output-visitor test-qmp-input-visitor" >> + test_progs="$checks check-qdict test-coroutine test-qmp-output-visitor test-qmp-input-visitor" >> fi >> fi >> >> diff --git a/tests/Makefile b/tests/Makefile >> index c11d980..cff9ff3 100644 >> --- a/tests/Makefile >> +++ b/tests/Makefile >> @@ -29,3 +29,9 @@ test-qmp-input-visitor: test-qmp-input-visitor.o $(qobject-obj-y) $(qapi-obj-y) >> test-qmp-commands.o: $(addprefix $(qapi-dir)/, test-qapi-types.c test-qapi-types.h test-qapi-visit.c test-qapi-visit.h test-qmp-marshal.c test-qmp-commands.h) $(qapi-obj-y) >> test-qmp-commands: test-qmp-commands.o $(qobject-obj-y) $(qapi-obj-y) $(tools-obj-y) $(qapi-dir)/test-qapi-visit.o $(qapi-dir)/test-qapi-types.o $(qapi-dir)/test-qmp-marshal.o module.o >> >> +.PHONY: check >> +check: $(patsubst %,run-check-%,$(CHECKS)) >> + >> +run-check-%: % >> + ./$< >> + >