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

[Luiz Capitulino <lcapitulino@redhat.com>]

On Mon,  9 Jan 2012 09:56:04 -0600
Anthony Liguori <aliguori@us.ibm.com> wrote:

> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
>  Makefile       |    6 --
>  check-qdict.c  |  246 +++++++++++++++++++++++++-------------------------------
>  configure      |    4 +-
>  tests/Makefile |    6 ++
>  4 files changed, 119 insertions(+), 143 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 80fb512..4823144 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -294,12 +294,6 @@ endif
>  test speed: all
>  	$(MAKE) -C tests/tcg $@
>  
> -.PHONY: check
> -check: $(patsubst %,run-check-%,$(CHECKS))
> -
> -run-check-%: %
> -	./$<
> -
>  .PHONY: TAGS
>  TAGS:
>  	find "$(SRC_PATH)" -name '*.[hc]' -print0 | xargs -0 etags
> diff --git a/check-qdict.c b/check-qdict.c
> index 5515773..fc0d276 100644
> --- a/check-qdict.c
> +++ b/check-qdict.c
> @@ -9,7 +9,7 @@
>   * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
>   * See the COPYING.LIB file in the top-level directory.
>   */
> -#include <check.h>
> +#include <glib.h>
>  
>  #include "qint.h"
>  #include "qdict.h"
> @@ -22,22 +22,21 @@
>   * (with some violations to access 'private' data)
>   */
>  
> -START_TEST(qdict_new_test)
> +static void qdict_new_test(void)
>  {
>      QDict *qdict;
>  
>      qdict = qdict_new();
> -    fail_unless(qdict != NULL);
> -    fail_unless(qdict_size(qdict) == 0);
> -    fail_unless(qdict->base.refcnt == 1);
> -    fail_unless(qobject_type(QOBJECT(qdict)) == QTYPE_QDICT);
> +    g_assert(qdict != NULL);
> +    g_assert(qdict_size(qdict) == 0);
> +    g_assert(qdict->base.refcnt == 1);
> +    g_assert(qobject_type(QOBJECT(qdict)) == QTYPE_QDICT);
>  
>      // destroy doesn't exit yet
> -    free(qdict);
> +    g_free(qdict);
>  }
> -END_TEST
>  
> -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...

>  
>      // destroy doesn't exit yet
>      QDECREF(qi);
> @@ -60,9 +59,8 @@ START_TEST(qdict_put_obj_test)
>      g_free(ent);
>      g_free(qdict);
>  }
> -END_TEST
>  
> -START_TEST(qdict_destroy_simple_test)
> +static void qdict_destroy_simple_test(void)
>  {
>      QDict *qdict;
>  
> @@ -72,134 +70,138 @@ START_TEST(qdict_destroy_simple_test)
>  
>      QDECREF(qdict);
>  }
> -END_TEST
> -
> -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.

Also, wouldn't be better to rename this to test-qdict (and the others too)?

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-%: %
> +	./$<
> +