[SSSD] [PATCH] TEST: recent_valid filter testing

Jakub Hrozek jhrozek at redhat.com
Fri Nov 13 10:32:50 UTC 2015


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.

> +
> +    /* 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.

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

> 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

> 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..

> +
> +    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.


> 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

> 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

> 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) ?

Otherwise ACK. 

CI is still running..


More information about the sssd-devel mailing list