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(a)redhat.com>
AuthorDate: Mon Nov 9 10:59:55 2015 +0100
Commit: Jakub Hrozek <jhrozek(a)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(a)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(a)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(a)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(a)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(a)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(a)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(a)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(a)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..