[SSSD] [PATCH] tests: be_ptask

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


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.

LS
-------------- next part --------------
diff --git a/src/tests/cmocka/test_be_ptask.c b/src/tests/cmocka/test_be_ptask.c
index 00acce2..057b26d 100644
--- a/src/tests/cmocka/test_be_ptask.c
+++ b/src/tests/cmocka/test_be_ptask.c
@@ -592,8 +592,8 @@ void test_be_ptask_create_sync(void **state)
 
     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);
+                               BE_PTASK_OFFLINE_SKIP, 0, test_be_ptask_sync,
+                               test_ctx, "Test ptask", &ptask);
     assert_int_equal(ret, ERR_OK);
     assert_non_null(ptask);
     assert_non_null(ptask->timer);
@@ -644,9 +644,9 @@ int main(int argc, const char *argv[])
     debug_level = SSSDBG_INVALID;
 
     pc = poptGetContext(argv[0], argc, argv, long_options, 0);
-    while((opt = poptGetNextOpt(pc)) != -1) {
-        switch(opt) {
-       default:
+    while ((opt = poptGetNextOpt(pc)) != -1) {
+        switch (opt) {
+        default:
             fprintf(stderr, "\nInvalid option %s: %s\n\n",
                     poptBadOption(pc, 0), poptStrerror(opt));
             poptPrintUsage(pc, stderr, 0);
-------------- next part --------------
diff --git a/src/tests/cmocka/test_be_ptask.c b/src/tests/cmocka/test_be_ptask.c
index 00acce2..63c14b9 100644
--- a/src/tests/cmocka/test_be_ptask.c
+++ b/src/tests/cmocka/test_be_ptask.c
@@ -171,6 +171,27 @@ errno_t test_be_ptask_sync(TALLOC_CTX *mem_ctx,
     return ERR_OK;
 }
 
+errno_t test_be_ptask_sync_ENOENT(TALLOC_CTX *mem_ctx,
+                                  struct tevent_context *ev,
+                                  struct be_ctx *be_ctx,
+                                  struct be_ptask *be_ptask,
+                                  void *pvt)
+{
+    struct test_ctx *test_ctx = NULL;
+
+    assert_non_null(ev);
+    assert_non_null(be_ctx);
+    assert_non_null(be_ptask);
+    assert_non_null(pvt);
+
+    test_ctx = talloc_get_type(pvt, struct test_ctx);
+    assert_non_null(test_ctx);
+
+    test_ctx->when = time(NULL);
+    test_ctx->done = true;
+
+    return ENOENT;
+}
 void test_setup(void **state)
 {
     struct test_ctx *test_ctx = NULL;
@@ -610,6 +631,39 @@ void test_be_ptask_create_sync(void **state)
     assert_null(ptask);
 }
 
+void test_be_ptask_create_sync_error(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_ENOENT,
+                          test_ctx, "Test ptask", &ptask);
+    assert_int_equal(ret, ERR_OK);
+    assert_non_null(ptask);
+    assert_non_null(ptask->timer);
+
+    enum tevent_req_state tev_state;
+    uint64_t error;
+
+    while (ptask->req == NULL
+           || ( tevent_req_is_error(ptask->req, &tev_state, &error)
+                && tev_state != TEVENT_REQ_USER_ERROR)) {
+        tevent_loop_once(test_ctx->be_ctx->ev);
+    }
+
+    assert_int_equal(error, ENOENT);
+    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;
@@ -637,7 +691,8 @@ int main(int argc, const char *argv[])
         new_test(be_ptask_reschedule_ok),
         new_test(be_ptask_reschedule_error),
         new_test(be_ptask_get_period),
-        new_test(be_ptask_create_sync)
+        new_test(be_ptask_create_sync),
+        new_test(be_ptask_create_sync_error),
     };
 
     /* Set debug level to invalid value so we can deside if -d 0 was used. */


More information about the sssd-devel mailing list