[SSSD] [PATCH] TEST: recent_valid filter testing

Petr Cech pcech at redhat.com
Fri Nov 13 11:45:00 UTC 2015


On 11/13/2015 11:32 AM, Jakub Hrozek wrote:
> On Fri, Nov 13, 2015 at 10:52:08AM +0100, Petr Cech wrote:
>> >On 11/13/2015 10:30 AM, Petr Cech wrote:
>>> > >On 11/13/2015 10:27 AM, Petr Cech wrote:
>>>> > >>
>>>> > >>Patches are rebased now. I hope it will be ok now.
>>>> > >>
>>>> > >>Petr
>>> > >Sorry, now my local CI tests failed... I will rebase it again.
>> >
>> >Well, now it is right. Local CI tests passed. There has been patch:
>> >
>> >   "TESTS: Fix warnings -Wshadow":
>> >   commit df9e9a1f9b7dc255eb62c390163c25917b08f5a2
>> >   Refs: sssd-1_13_1-137-gdf9e9a1
>> >   Author:     Lukas Slebodnik<lslebodn at redhat.com>
>> >   AuthorDate: Mon Nov 9 10:59:55 2015 +0100
>> >   Commit:     Jakub Hrozek<jhrozek at redhat.com>
>> >   CommitDate: Tue Nov 10 15:34:41 2015 +0100
>> >
>> >There is change
>> ># -                         time_t time)
>> ># +                         time_t transaction_time)
>> >in static void prepare_user().
>> >My patches were in conflict with it.
>> >
>> >Regards
>> >
>> >Petr
>> > From 3ce6073dda27fd7a4626f5cbac1c765274ca5fe0 Mon Sep 17 00:00:00 2001
>> >From: Petr Cech<pcech at redhat.com>
>> >Date: Fri, 2 Oct 2015 07:34:08 -0400
>> >Subject: [PATCH 1/8] TEST: Add test_user_by_recent_filter_valid
>> >
>> >Test users_by_filter_valid() was removed in past. We will add two new
>> >tests instead of it. Logic of those tests is connected to RECENT
>> >filter. It returns only records which have been wrote or updated after
>> >filter was created (or another given time).
>> >
>> >users_by_filter_valid() --> user_by_recent_filter_valid()
>> >                             users_by_recent_filter_valid()
>> >
>> >The first of new tests, user_by_recent_filter_valid(), counts with two
>> >users. One is stored before filter request creation and the second user
>> >is stored after filter request creation. So filter returns only one
>> >user.
>> >
>> >The second of new tests, users_by_recent_filter_valid(), counts with
>> >three users. One is stored before filter request creation and two users
>> >are stored after filter request creation. So filter returns two users.
>> >
>> >This patch adds user_by_recent_filter_valid().
>> >
>> >Resolves:
>> >https://fedorahosted.org/sssd/ticket/2730
>> >---
>> >  src/tests/cmocka/test_responder_cache_req.c | 50 +++++++++++++++++++++++++++++
>> >  1 file changed, 50 insertions(+)
>> >
>> >diff --git a/src/tests/cmocka/test_responder_cache_req.c b/src/tests/cmocka/test_responder_cache_req.c
>> >index 85d986bd7d159dc238bce4bc770272d18288f2dd..14a40ae6e56b2f6d0b18608bac09bc4680245153 100644
>> >--- a/src/tests/cmocka/test_responder_cache_req.c
>> >+++ b/src/tests/cmocka/test_responder_cache_req.c
>> >@@ -1239,6 +1239,53 @@ static void cache_req_user_by_filter_test_done(struct tevent_req *req)
>> >      ctx->tctx->done = true;
>> >  }
>> >
>> >+void test_user_by_recent_filter_valid(void **state)
>> >+{
>> >+    struct cache_req_test_ctx *test_ctx = NULL;
>> >+    TALLOC_CTX *req_mem_ctx = NULL;
>> >+    struct tevent_req *req = NULL;
>> >+    const char *ldbname = NULL;
>> >+    errno_t ret;
>> >+
>> >+    test_ctx = talloc_get_type_abort(*state, struct cache_req_test_ctx);
>> >+    test_ctx->create_user = true;
>> >+
>> >+    ret = sysdb_store_user(test_ctx->tctx->dom, TEST_USER_NAME2,
>> >+                           "pwd", 1001, 1001, NULL, NULL, NULL,
>> >+                           "cn="TEST_USER_NAME2",dc=test",
>> >+                           NULL, NULL, 1000, time(NULL)-1);
>> >+    assert_int_equal(ret, EOK);
>> >+
>> >+    req_mem_ctx = talloc_new(test_ctx->tctx);
>> >+    check_leaks_push(req_mem_ctx);
> I think the last question is whether we want to use this new context or
> just call check_leaks_push(test_ctx) recursively. I don't really mind
> too much, both would work for me.
>
> Unless someone opposes, I would push the patch as-is.
OK.

>
>> >+
>> >+    /* Filters always go to DP */
>> >+    will_return(__wrap_sss_dp_get_account_send, test_ctx);
>> >+    mock_account_recv_simple();
>> >+
>> >+    /* User TEST_USER is created with a DP callback. */
>> >+    req = cache_req_user_by_filter_send(req_mem_ctx, test_ctx->tctx->ev,
>> >+                                        test_ctx->rctx,
>> >+                                        test_ctx->tctx->dom->name,
>> >+                                        "test*");
>> >+    assert_non_null(req);
>> > From df9717ca932f95f55b528024829758dd9b2f2f56 Mon Sep 17 00:00:00 2001
>> >From: Petr Cech<pcech at redhat.com>
>> >Date: Wed, 4 Nov 2015 06:50:33 -0500
>> >Subject: [PATCH 2/8] TEST: Refactor of test_responder_cache_req.c
>> >
>> >This patch only defines constant TEST_USER_FILTER. So code will be more
>                                     ~~~~~~~~~~~~~~~~
>                                     TEST_USER_PREFIX is defined.
Fixed.
>
> The code is fine.
>
>
>> > From ae448cc95f9ab9fbca3aba5107bb964caf8250ec Mon Sep 17 00:00:00 2001
>> >From: Petr Cech<pcech at redhat.com>
>> >Date: Tue, 27 Oct 2015 03:53:18 -0400
>> >Subject: [PATCH 3/8] TEST: Refactor of test_responder_cache_req.c
>> >
>> >We need little more in background of responder_cache_req tests. There
>> >will be tests which will use three test users. This patch add support
>> >for it.
> ACK
Thanks.

>
>> > From 943d828ec283284269f954b3044292fb491cf5fa Mon Sep 17 00:00:00 2001
>> >From: Petr Cech<pcech at redhat.com>
>> >Date: Tue, 10 Nov 2015 06:04:45 -0500
>> >Subject: [PATCH 4/8] TEST: Add common function are_values_in_array()
>> >
>> >This patch adds function are_values_in_array() to common test code. And
>> >there is tc_are_values_in_array macro defined which is usefull for
>> >talloc allocated values and arrays.
> Thanks, ACK
Thanks.

>
>> > From 5e6274406deeb64e0c8a125d6409300b3bda6d61 Mon Sep 17 00:00:00 2001
>> >From: Petr Cech<pcech at redhat.com>
>> >Date: Tue, 27 Oct 2015 04:02:46 -0400
>> >Subject: [PATCH 5/8] TEST: Add test_users_by_recent_filter_valid
>> >
>> >Test users_by_filter_valid() was removed in past. We will add two new
>> >tests instead of it. Logic of those tests is connected to RECENT
>> >filter. It returns only records which have been wrote or updated
>> >after filter was created (or another given time).
>> >
>> >users_by_filter_valid() --> user_by_recent_filter_valid()
>> >                             users_by_recent_filter_valid()
>> >
>> >The first of new tests, user_by_recent_filter_valid(), counts with
>> >two users. One is stored before filter request creation and the second
>> >user is stored after filter request creation. So filter returns only one
>> >user.
>> >
>> >The second of new tests, users_by_recent_filter_valid(), counts with
>> >three users. One is stored before filter request creation and two
>> >users are stored after filter request creation. So filter returns two
>> >users.
>> >
>> >This patch adds users_by_recent_filter_valid().
>> >
>> >Resolves:
>> >https://fedorahosted.org/sssd/ticket/2730
> Two remarks about memory contexts
>
>> >---
>> >  src/tests/cmocka/test_responder_cache_req.c | 68 ++++++++++++++++++++++++++++-
>> >  1 file changed, 67 insertions(+), 1 deletion(-)
>> >
>> >diff --git a/src/tests/cmocka/test_responder_cache_req.c b/src/tests/cmocka/test_responder_cache_req.c
>> >index 439084c85c97c8405227c2599621d89c3dc85fe4..b680bd8f24ede5b149d0f814fbf0a2aa9cbfa040 100644
>> >--- a/src/tests/cmocka/test_responder_cache_req.c
>> >+++ b/src/tests/cmocka/test_responder_cache_req.c
>> >@@ -318,7 +318,6 @@ static void check_group(struct cache_req_test_ctx *test_ctx,
>> >      assert_string_equal(exp_dom->name, test_ctx->domain->name);
>> >  }
>> >
>> >-
>> >  struct tevent_req *
>> >  __wrap_sss_dp_get_account_send(TALLOC_CTX *mem_ctx,
>> >                                 struct resp_ctx *rctx,
>> >@@ -1317,6 +1316,72 @@ void test_user_by_recent_filter_valid(void **state)
>> >      assert_string_equal(ldbname, TEST_USER_NAME);
>> >  }
>> >
>> >+void test_users_by_recent_filter_valid(void **state)
>> >+{
>> >+    struct cache_req_test_ctx *test_ctx = NULL;
>> >+    TALLOC_CTX *req_mem_ctx = NULL;
>> >+    TALLOC_CTX *tmp_ctx = NULL;
> [...]
>
>> >+    tmp_ctx = talloc_new(req_mem_ctx);
> I don't see the need for tmp_ctx really, you can just use test_ctx..
Fixed.

>
>> >+
>> >+    user_names = talloc_array(tmp_ctx, const char *, 2);
>> >+    assert_non_null(user_names);
>> >+    user_names[0] = TEST_USER_NAME;
>> >+    user_names[1] = TEST_USER_NAME2;
>> >+
>> >+    ldb_results = talloc_array(tmp_ctx, const char *, 2);
>> >+    assert_non_null(ldb_results);
>> >+    for (int i = 0; i < 2; ++i) {
>> >+        ldbname = ldb_msg_find_attr_as_string(test_ctx->result->msgs[i],
>> >+                                              SYSDB_NAME, NULL);
>> >+        assert_non_null(ldbname);
>> >+        ldb_results[i] = ldbname;
>> >+    }
>> >+
>> >+    assert_string_not_equal(ldb_results[0], ldb_results[1]);
>> >+
>> >+    assert_true(tc_are_values_in_array(user_names, ldb_results));
>> >+
>> >+    talloc_zfree(tmp_ctx);
> Please either free both tmp_ctx and req_mem_ctx or none of them. I think
> freeing none of them and relying on talloc to free them when test_ctx is
> gone is fine.
Fixed.

>
>
>> > From 8a38a1c3d03f9c89725f9b9e686696882eed305e Mon Sep 17 00:00:00 2001
>> >From: Petr Cech<pcech at redhat.com>
>> >Date: Sun, 1 Nov 2015 07:09:28 -0500
>> >Subject: [PATCH 6/8] TEST: Add test_group_by_recent_filter_valid
> ACK
Thanks.

>
>> > From 71347406a22bc0a9a6931abe83dcb65b8f3eca8a Mon Sep 17 00:00:00 2001
>> >From: Petr Cech<pcech at redhat.com>
>> >Date: Sun, 1 Nov 2015 07:21:18 -0500
>> >Subject: [PATCH 7/8] TEST: Refactor of test_responder_cache_req.c
>> >
>> >We need little more in backroung of responder_cache_req tests. There
>> >will be tests which will use three test groups. This patch add support
>> >for it.
>> >
>> >Resolves:
>> >https://fedorahosted.org/sssd/ticket/2730
> ACK
Thanks.

>
>> > From 331a7b0f995aadb69dfa15a89aa46875e67d4377 Mon Sep 17 00:00:00 2001
>> >From: Petr Cech<pcech at redhat.com>
>> >Date: Sun, 1 Nov 2015 07:45:56 -0500
>> >Subject: [PATCH 8/8] TEST: Add test_groups_by_recent_filter_valid
>> >
>> >Test groups_by_filter_valid() was removed in past. We will add two new
>> >tests instead of it. Logic of those tests is connected to RECENT
>> >filter. It returns only records which have been wrote or updated after
>> >filter was created (or another given time).
>> >
>> >groups_by_filter_valid() --> group_by_recent_filter_valid()
>> >                              grous_by_recent_filter_valid()
>> >
>> >The first of new tests, group_by_recent_filter_valid(), counts with two
>> >groups. One is stored before filter request creation and the second
>> >group is stored after filter request creation. So filter returns only
>> >one group.
>> >
>> >The second of new tests, groups_by_recent_filter_valid(), counts with
>> >three users. One is stored before filter request creation and two
>> >groups are stored after filter request creation. So filter returns two
>> >groups.
>> >
>> >This patch adds groups_by_recent_filter_valid().
>> >
>> >Resolves:
>> >https://fedorahosted.org/sssd/ticket/2730
>> >---
>> >  src/tests/cmocka/test_responder_cache_req.c | 68 +++++++++++++++++++++++++++++
>> >  1 file changed, 68 insertions(+)
>> >
>> >diff --git a/src/tests/cmocka/test_responder_cache_req.c b/src/tests/cmocka/test_responder_cache_req.c
>> >index 5030d3e3fcee9223e855309c70403e4f7e25a749..7725c76651e2caf545b5b26488bc8b6417b9b2e5 100644
>> >--- a/src/tests/cmocka/test_responder_cache_req.c
>> >+++ b/src/tests/cmocka/test_responder_cache_req.c
>> >@@ -1557,6 +1557,73 @@ void test_group_by_recent_filter_valid(void **state)
>> >      assert_string_equal(ldbname, TEST_GROUP_NAME);
>> >  }
>> >
>> >+void test_groups_by_recent_filter_valid(void **state)
>> >+{
>> >+    struct cache_req_test_ctx *test_ctx = NULL;
>> >+    TALLOC_CTX *req_mem_ctx = NULL;
>> >+    TALLOC_CTX *tmp_ctx = NULL;
>> >+    struct tevent_req *req = NULL;
>> >+    const char **group_names = NULL;
>> >+    const char **ldb_results = NULL;
>> >+    const char *ldbname = NULL;
>> >+    errno_t ret;
>> >+
>> >+    test_ctx = talloc_get_type_abort(*state, struct cache_req_test_ctx);
>> >+    test_ctx->create_group1 = true;
>> >+    test_ctx->create_group2 = true;
>> >+
>> >+    ret = sysdb_store_group(test_ctx->tctx->dom, TEST_GROUP_NAME2,
>> >+                            1001, NULL, 1001, time(NULL));
> Can we use time(NULL)-1 and drop the sleep(1) ?
Yes, of course. I fixed it.

>
> Otherwise ACK.
Thanks.

>
> CI is still running..

There is new patch set attached.

Petr
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-TEST-Add-test_user_by_recent_filter_valid.patch
Type: text/x-patch
Size: 4164 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20151113/456d01a7/attachment-0008.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-TEST-Refactor-of-test_responder_cache_req.c.patch
Type: text/x-patch
Size: 2046 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20151113/456d01a7/attachment-0009.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-TEST-Refactor-of-test_responder_cache_req.c.patch
Type: text/x-patch
Size: 5938 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20151113/456d01a7/attachment-0010.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-TEST-Add-common-function-are_values_in_array.patch
Type: text/x-patch
Size: 2391 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20151113/456d01a7/attachment-0011.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-TEST-Add-test_users_by_recent_filter_valid.patch
Type: text/x-patch
Size: 4682 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20151113/456d01a7/attachment-0012.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0006-TEST-Add-test_group_by_recent_filter_valid.patch
Type: text/x-patch
Size: 3760 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20151113/456d01a7/attachment-0013.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0007-TEST-Refactor-of-test_responder_cache_req.c.patch
Type: text/x-patch
Size: 3120 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20151113/456d01a7/attachment-0014.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0008-TEST-Add-test_groups_by_recent_filter_valid.patch
Type: text/x-patch
Size: 4508 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20151113/456d01a7/attachment-0015.bin>


More information about the sssd-devel mailing list