[SSSD] [PATCH] tests: be_ptask

Lukas Slebodnik lslebodn at redhat.com
Tue Nov 18 17:32:29 UTC 2014


On (18/11/14 18:26), Lukas Slebodnik wrote:
>On (10/11/14 14:29), Pavel Březina wrote:
>>https://fedorahosted.org/sssd/ticket/1939
>
>>From 8eb7a424bccfcbb6140fc2d6eed91759d765c1a4 Mon Sep 17 00:00:00 2001
>>From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina at redhat.com>
>>Date: Mon, 10 Nov 2014 13:30:28 +0100
>>Subject: [PATCH 1/4] be_ptask: create a private header file
>>
>>This is done so we gain access to the be_ptask structure in unit tests.
>>---
>ACK
>
>>From 09dc2fe755b0ac1af21fe888c2dbdd9b6693a3ed Mon Sep 17 00:00:00 2001
>>From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina at redhat.com>
>>Date: Mon, 10 Nov 2014 13:59:27 +0100
>>Subject: [PATCH 2/4] be_ptask: handle OFFLINE_DISABLE mode before task
>> execution
>>
>>---
>ACK
>
>>From 30cef74931f242ec5ee7bb8790cc52140671ae86 Mon Sep 17 00:00:00 2001
>>From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina at redhat.com>
>>Date: Mon, 10 Nov 2014 14:26:32 +0100
>>Subject: [PATCH 3/4] be_ptask: add next_execution time to struct be_ptask
>>
>>For debugging and testing purposes.
>>---
>ACK
>
>>From 4bf4b559a7f1990aa420cd73425af5107f078ba5 Mon Sep 17 00:00:00 2001
>>From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina at redhat.com>
>>Date: Fri, 7 Nov 2014 14:47:12 +0100
>>Subject: [PATCH 4/4] tests: be_ptask
>>
>>Resolves:
>>https://fedorahosted.org/sssd/ticket/1939
>>---
>> Makefile.am                      |  16 +
>> src/tests/cmocka/test_be_ptask.c | 661 +++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 677 insertions(+)
>> create mode 100644 src/tests/cmocka/test_be_ptask.c
>>
>>diff --git a/Makefile.am b/Makefile.am
>>index 5c30aac82c9aa818d490b909f73c3be2894a3a57..e3057d09bf7662a1d1bf26b9d5ec9294157d24e1 100644
>>--- a/Makefile.am
>>+++ b/Makefile.am
>>@@ -219,6 +219,7 @@ if HAVE_CMOCKA
>>         test_search_bases \
>>         sdap-tests \
>>         test_sysdb_views \
>>+        test_be_ptask \
>>         $(NULL)
>> 
>> if BUILD_IFP
>>@@ -2079,6 +2080,21 @@ test_sysdb_views_LDADD = \
>>     $(SSSD_INTERNAL_LTLIBS) \
>>     libsss_test_common.la \
>>     $(NULL)
>>+    
>  ^^^^
>  trailing character '\t'
>>+test_be_ptask_SOURCES = \
>>+    src/tests/cmocka/test_be_ptask.c \
>>+    src/providers/dp_ptask.c \
>>+    $(NULL)
>>+test_be_ptask_CFLAGS = \
>>+    $(AM_CFLAGS) \
>>+    $(NULL)
>>+test_be_ptask_LDADD = \
>>+    $(CMOCKA_LIBS) \
>>+    $(POPT_LIBS) \
>>+    $(TALLOC_LIBS) \
>>+    $(SSSD_INTERNAL_LTLIBS) \
>>+    libsss_test_common.la \
>>+    $(NULL)
>> 
>> endif # HAVE_CMOCKA
>> 
>>diff --git a/src/tests/cmocka/test_be_ptask.c b/src/tests/cmocka/test_be_ptask.c
>>new file mode 100644
>>index 0000000000000000000000000000000000000000..00acce2758b9dc7711e9415671f11cdf0bb47ebd
>>--- /dev/null
>>+++ b/src/tests/cmocka/test_be_ptask.c
>>@@ -0,0 +1,661 @@
>>+/*
>>+    Authors:
>>+        Pavel Březina <pbrezina at redhat.com>
>>+
>>+    Copyright (C) 2014 Red Hat
>>+
>>+    This program is free software; you can redistribute it and/or modify
>>+    it under the terms of the GNU 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 General Public License for more details.
>>+
>>+    You should have received a copy of the GNU General Public License
>>+    along with this program.  If not, see <http://www.gnu.org/licenses/>.
>>+*/
>>+
>>+#include <talloc.h>
>>+#include <tevent.h>
>>+#include <errno.h>
>>+#include <popt.h>
>>+#include <time.h>
>>+
>>+#include "providers/dp_backend.h"
>>+#include "providers/dp_ptask_private.h"
>>+#include "providers/dp_ptask.h"
>>+#include "tests/cmocka/common_mock.h"
>>+#include "tests/common.h"
>>+
>>+#define new_test(test) \
>>+    unit_test_setup_teardown(test_ ## test, test_setup, test_teardown)
>>+
>>+struct test_ctx {
>>+    struct be_ctx *be_ctx;
>>+
>>+    time_t when;
>>+    bool done;
>>+
>>+    bool add_online_cb_called;
>>+    bool add_offline_cb_called;
>>+};
>>+
>>+#define mark_online(test_ctx) do { \
>>+    test_ctx->be_ctx->offstat.went_offline = 0; \
>>+    test_ctx->be_ctx->offstat.offline = false; \
>>+} while (0)
>   The macro mark_online is unused.
>   Did you plan to use this macro somehere?
>
>//snip
>
>>+void test_setup(void **state)
>>+{
>>+    struct test_ctx *test_ctx = NULL;
>>+
>>+    //assert_true(leak_check_setup());
>      ^^^
>    Could you remove comment?
>>+
>>+    test_ctx = talloc_zero(global_talloc_context, struct test_ctx);
>>+    assert_non_null(test_ctx);
>>+
>>+    /* create be_ctx, only ev and offline field should be used */
>>+    test_ctx->be_ctx = talloc_zero(test_ctx, struct be_ctx);
>>+    assert_non_null(test_ctx->be_ctx);
>>+
>>+    test_ctx->be_ctx->ev = tevent_context_init(test_ctx->be_ctx);
>>+    assert_non_null(test_ctx->be_ctx->ev);
>>+
>>+    *state = test_ctx;
>>+}
>>+
>>+void test_teardown(void **state)
>>+{
>>+    talloc_zfree(*state);
>>+    //assert_true(leak_check_teardown());
>      ^^^
>   Could you remove comment here as well?
>   I didn't have any problem with allocation leak check assertions.
>>+}
>>+
>>+void test_be_ptask_create_first_delay(void **state)
>>+{
>>+    struct test_ctx *test_ctx = (struct test_ctx *)(*state);
>>+    struct be_ptask *ptask = NULL;
>>+    time_t delay = 2;
>      IMHO, it would be better to use constant here (with uppercase)
>      It would improve readability.
>      The same applies to period in other tests.
>
>>+    time_t now;
>>+    errno_t ret;
>>+
>>+    now = time(NULL);
>>+    ret = be_ptask_create(test_ctx, test_ctx->be_ctx, 10, delay, 0, 0, 0,
>>+                          BE_PTASK_OFFLINE_SKIP, 0, test_be_ptask_send,
>>+                          test_be_ptask_recv, test_ctx, "Test ptask", &ptask);
>>+    assert_int_equal(ret, ERR_OK);
>>+    assert_non_null(ptask);
>>+    assert_non_null(ptask->timer);
>>+
>>+}
>>+
>>+    out_period = be_ptask_get_period(ptask);
>>+    assert_true(period == out_period);
>>+
>>+    be_ptask_destroy(&ptask);
>>+    assert_null(ptask);
>>+}
>>+
>>+void test_be_ptask_create_sync(void **state)
>>+{
>>+    struct test_ctx *test_ctx = (struct test_ctx *)(*state);
>>+    struct be_ptask *ptask = NULL;
>>+    time_t now;
>>+    errno_t ret;
>>+
>>+    now = time(NULL);
>>+    ret = be_ptask_create_sync(test_ctx, test_ctx->be_ctx, 10, 0, 0, 0, 0,
>>+                          BE_PTASK_OFFLINE_SKIP, 0, test_be_ptask_sync,
>>+                          test_ctx, "Test ptask", &ptask);
>                            ^^^
>                        Indentation issue.
>                        (simple diff is attached.)
>>+    assert_int_equal(ret, ERR_OK);
>>+    assert_non_null(ptask);
>>+    assert_non_null(ptask->timer);
>>+
>>+    while (!test_ctx->done) {
>>+        tevent_loop_once(test_ctx->be_ctx->ev);
>>+    }
>>+
>>+    assert_true(now <= ptask->last_execution);
>>+    assert_true(now <= test_ctx->when);
>>+    assert_true(ptask->last_execution <= test_ctx->when);
>>+
>>+    be_ptask_destroy(&ptask);
>>+    assert_null(ptask);
>>+}
>>+
>>+int main(int argc, const char *argv[])
>>+{
>>+    poptContext pc;
>>+    int opt;
>>+    struct poptOption long_options[] = {
>>+        POPT_AUTOHELP
>>+        SSSD_DEBUG_OPTS
>>+        POPT_TABLEEND
>>+    };
>>+
>>+    const UnitTest tests[] = {
>>+        new_test(be_ptask_create_einval_be),
>>+        new_test(be_ptask_create_einval_period),
>>+        new_test(be_ptask_create_einval_send),
>>+        new_test(be_ptask_create_einval_recv),
>>+        new_test(be_ptask_create_einval_name),
>>+        new_test(be_ptask_create_no_delay),
>>+        new_test(be_ptask_create_first_delay),
>>+        new_test(be_ptask_disable),
>>+        new_test(be_ptask_enable),
>>+        new_test(be_ptask_enable_delay),
>>+        new_test(be_ptask_offline_skip),
>>+        new_test(be_ptask_offline_disable),
>>+        new_test(be_ptask_offline_execute),
>>+        new_test(be_ptask_reschedule_ok),
>>+        new_test(be_ptask_reschedule_error),
>>+        new_test(be_ptask_get_period),
>>+        new_test(be_ptask_create_sync)
>
>Very nice unit test.
>
>It would be good to add few more tests with corner cases.
>There aren't test for failed functions (_send, _recv)
>(Attached is diff with test for error occured in be_ptask_sync_t.)
>
>There are 6 "time_t" arguments be_ptask_create.
>perriod and first_delay are tested.
>enabled_delay, random_offset, timeout, max_backoff are tested just with zero.
>It would be good to add tests at least for timeout, max_backoff.
>The most lines without code coverage are due to this two variables.
>

BTW: gcc 4.9 produces a warning in unit test.

  CC       src/providers/ad_gpo_tests-dp_ptask.o
src/tests/cmocka/test_be_ptask.c: In function ‘test_be_ptask_reschedule_error’:
src/tests/cmocka/test_be_ptask.c:558:53: error: ‘now’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
     assert_true(now + period <= ptask->next_execution);
                                                     ^
cc1: all warnings being treated as errors
Makefile:16655: recipe for target 'src/tests/cmocka/test_be_ptask-test_be_ptask.o' failed

LS



More information about the sssd-devel mailing list