On 10/05/2016 04:18 PM, Michal Židek wrote:
> On 10/05/2016 03:47 PM, Philip Prindeville wrote:
>>
>>> On Oct 5, 2016, at 7:18 AM, Michal Židek <mzidek(a)redhat.com> wrote:
>>>
>>> I forgot to attach the patches.
>>>
>>> Again the first one is acked by me, the second
>>> needs a review.
>>>
>>> Michal
>>
>>
>> Thanks for writing those tests.
>>
>> Minor comment, dhash_ut_check.c and the existing checks don’t have any
>> negative tests, such as attempting to delete a non-existent key, or
>> deleting an already deleted key… or entering a key which is already
>> present.
>>
>> -Philip
>>
>
> Good point. I added some delete operations to these tests.
> However these are just some sanity tests to cover the code that
> was changed. My intention was not to test everything here.
>
> New tests are attached.
>
> Michal
Hello Michal,
I comment two things online.
I agree with the comments. New test attached.
>
> 0002-DHASH-Add-check-based-unit-test.patch
>
>
> From 83b18c3ca4d70086bbdc645e0d09e7e027e5e9b3 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzidek(a)redhat.com>
> Date: Wed, 5 Oct 2016 13:10:35 +0200
> Subject: [PATCH 2/2] DHASH: Add check based unit test
>
> ---
> Makefile.am | 14 ++++
> dhash/dhash_ut_check.c | 210
> +++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 224 insertions(+)
> create mode 100644 dhash/dhash_ut_check.c
>
> diff --git a/Makefile.am b/Makefile.am
> index 65528a8..ca9710e 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -114,12 +114,26 @@ libdhash_la_LDFLAGS = \
> check_PROGRAMS += dhash_test dhash_example
> TESTS += dhash_test dhash_example
>
> +if HAVE_CHECK
> + check_PROGRAMS += dhash_ut_check
> + TESTS += dhash_ut_check
> +endif
> +
> +
> dhash_test_SOURCES = dhash/examples/dhash_test.c
> dhash_test_LDADD = libdhash.la
>
> dhash_example_SOURCES = dhash/examples/dhash_example.c
> dhash_example_LDADD = libdhash.la
>
> +dhash_ut_check_SOURCES = dhash/dhash_ut_check.c
> +dhash_ut_chech_CFLAGS = $(AM_CFLAGS) \
> + $(CHECK_CFLAGS) \
> + $(NULL)
> +dhash_ut_check_LDADD = libdhash.la \
> + $(CHECK_LIBS) \
> + $(NULL)
> +
> dist_examples_DATA += \
> dhash/examples/dhash_test.c \
> dhash/examples/dhash_example.c
> diff --git a/dhash/dhash_ut_check.c b/dhash/dhash_ut_check.c
> new file mode 100644
> index 0000000..b4b36fa
> --- /dev/null
> +++ b/dhash/dhash_ut_check.c
> @@ -0,0 +1,210 @@
> +/*
> + Authors:
> + Michal Zidek <mzidek(a)redhat.com>
> +
> + Copyright (C) 2016 Red Hat
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU Lesser General Public License as
> published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> License
> + along with this program. If not, see
> <
http://www.gnu.org/licenses/>.
> +*/
> +
> +#include "config.h"
> +
> +#include <errno.h>
> +#include <stdio.h>
> +#include <string.h>
IMO, this is unnecessary.
> +#include <stdlib.h>
> +#include <check.h>
> +
> +/* #define TRACE_LEVEL 7 */
> +#define TRACE_HOME
> +#include "dhash.h"
> +#include "path_utils.h"
IMO, this is unnecessary.
> +
> +#define HTABLE_SIZE 128
> +
> +int verbose = 0;
> +
> +/* There must be no warnings generated during this test
> + * without having to cast the key value. */
> +START_TEST(test_key_const_string)
> +{
> + hash_table_t *htable;
> + int ret;
> + hash_value_t ret_val;
> + hash_value_t enter_val1 = {.type = HASH_VALUE_INT, .i = 1};
> + hash_value_t enter_val2 = {.type = HASH_VALUE_INT, .i = 2};
> + hash_key_t key = {.type = HASH_KEY_CONST_STRING, .c_str =
> "constant"};
> +
> + ret = hash_create(HTABLE_SIZE, &htable, NULL, NULL);
> + fail_unless(ret == 0);
> +
> + /* The table is empty, lookup should return error */
> + ret = hash_lookup(htable, &key, &ret_val);
> + fail_unless(ret == HASH_ERROR_KEY_NOT_FOUND);
> +
> + /* Deleting with non-existing key should return error */
> + ret = hash_delete(htable, &key);
> + fail_unless(ret == HASH_ERROR_KEY_NOT_FOUND);
> +
> + ret = hash_enter(htable, &key, &enter_val1);
> + fail_unless(ret == 0);
> +
> + hash_lookup(htable, &key, &ret_val);
> + fail_unless(ret == 0);
> + fail_unless(ret_val.i == 1);
> +
> + /* Overwrite the entry */
> + ret = hash_enter(htable, &key, &enter_val2);
> + fail_unless(ret == 0);
> +
> + hash_lookup(htable, &key, &ret_val);
> + fail_unless(ret == 0);
> + fail_unless(ret_val.i == 2);
> +
> + ret = hash_delete(htable, &key);
> + fail_unless(ret == 0);
> +
> + /* Delete again with the same key */
> + ret = hash_delete(htable, &key);
> + fail_unless(ret == HASH_ERROR_KEY_NOT_FOUND);
> +
> + ret = hash_destroy(htable);
> + fail_unless(ret == 0);
> +}
> +END_TEST
> +
> +START_TEST(test_key_string)
> +{
> + hash_table_t *htable;
> + int ret;
> + hash_value_t ret_val;
> + hash_value_t enter_val1 = {.type = HASH_VALUE_INT, .i = 1};
> + hash_value_t enter_val2 = {.type = HASH_VALUE_INT, .i = 2};
> + hash_key_t key;
> + char str[] = "non_constant";
> +
> + key.type = HASH_KEY_STRING;
> + key.str = str;
> +
> + ret = hash_create(HTABLE_SIZE, &htable, NULL, NULL);
> + fail_unless(ret == 0);
> +
> + /* The table is empty, lookup should return error */
> + ret = hash_lookup(htable, &key, &ret_val);
> + fail_unless(ret == HASH_ERROR_KEY_NOT_FOUND);
> +
> + /* Deleting with non-existing key should return error */
> + ret = hash_delete(htable, &key);
> + fail_unless(ret == HASH_ERROR_KEY_NOT_FOUND);
> +
> + ret = hash_enter(htable, &key, &enter_val1);
> + fail_unless(ret == 0);
> +
> + hash_lookup(htable, &key, &ret_val);
> + fail_unless(ret == 0);
> + fail_unless(ret_val.i == 1);
> +
> + /* Overwrite the entry */
> + ret = hash_enter(htable, &key, &enter_val2);
> + fail_unless(ret == 0);
> +
> + hash_lookup(htable, &key, &ret_val);
> + fail_unless(ret == 0);
> + fail_unless(ret_val.i == 2);
> +
> + ret = hash_delete(htable, &key);
> + fail_unless(ret == 0);
> +
> + /* Delete again with the same key */
> + ret = hash_delete(htable, &key);
> + fail_unless(ret == HASH_ERROR_KEY_NOT_FOUND);
> +
> +
> + ret = hash_destroy(htable);
> + fail_unless(ret == 0);
> +}
> +END_TEST
> +
> +START_TEST(test_key_ulong)
> +{
> + hash_table_t *htable;
> + int ret;
> + hash_value_t ret_val;
> + hash_value_t enter_val1 = {.type = HASH_VALUE_INT, .i = 1};
> + hash_value_t enter_val2 = {.type = HASH_VALUE_INT, .i = 2};
> + hash_key_t key = {.type = HASH_KEY_ULONG, .ul = 68ul};
> +
> + ret = hash_create(HTABLE_SIZE, &htable, NULL, NULL);
> + fail_unless(ret == 0);
> +
> + /* The table is empty, lookup should return error */
> + ret = hash_lookup(htable, &key, &ret_val);
> + fail_unless(ret == HASH_ERROR_KEY_NOT_FOUND);
> +
> + /* Deleting with non-existing key should return error */
> + ret = hash_delete(htable, &key);
> + fail_unless(ret == HASH_ERROR_KEY_NOT_FOUND);
> +
> + ret = hash_enter(htable, &key, &enter_val1);
> + fail_unless(ret == 0);
> +
> + hash_lookup(htable, &key, &ret_val);
> + fail_unless(ret == 0);
> + fail_unless(ret_val.i == 1);
> +
> + /* Overwrite the entry */
> + ret = hash_enter(htable, &key, &enter_val2);
> + fail_unless(ret == 0);
> +
> + hash_lookup(htable, &key, &ret_val);
> + fail_unless(ret == 0);
> + fail_unless(ret_val.i == 2);
> +
> + ret = hash_delete(htable, &key);
> + fail_unless(ret == 0);
> +
> + /* Delete again with the same key */
> + ret = hash_delete(htable, &key);
> + fail_unless(ret == HASH_ERROR_KEY_NOT_FOUND);
> +
> + ret = hash_destroy(htable);
> + fail_unless(ret == 0);
> +}
> +END_TEST
> +
> +static Suite *dhash_suite(void)
> +{
> + Suite *s = suite_create("");
> +
> + TCase *tc_basic = tcase_create("dhash API tests");
> + tcase_add_test(tc_basic, test_key_const_string);
> + tcase_add_test(tc_basic, test_key_string);
> + tcase_add_test(tc_basic, test_key_ulong);
> + suite_add_tcase(s, tc_basic);
> +
> + return s;
> +}
> +
> +int main(void)
> +{
> + int number_failed;
> +
> + Suite *s = dhash_suite();
> + SRunner *sr = srunner_create(s);
> + /* If CK_VERBOSITY is set, use that, otherwise it defaults to
> CK_NORMAL */
> + srunner_run_all(sr, CK_ENV);
> + number_failed = srunner_ntests_failed(sr);
> + srunner_free(sr);
> + return (number_failed == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
> +}
> -- 1.8.3.1