Re: [Qemu-devel] [PATCH 03/11] check-qdict: convert to gtest

[Anthony Liguori <aliguori@us.ibm.com>]

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<lcapitulino@redhat.com>
>
> 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-%: %
>> +	./$<
>> +
>