[SSSD] [PATCH] Improvements to nested LDAP unit tests
Lukas Slebodnik
lslebodn at redhat.com
Tue Jun 24 08:49:34 UTC 2014
On (23/06/14 19:36), Jakub Hrozek wrote:
>Hi,
>
>When I was reviewing PavelB's duplicate members patch, I realized our
>nested group unit tests could use some improvements. Please see the
>attached patches and their commit messages.
>From bc8fd57de30abad8c2b5e74def22927bb2870830 Mon Sep 17 00:00:00 2001
>From: Jakub Hrozek <jhrozek at redhat.com>
>Date: Mon, 23 Jun 2014 17:50:45 +0200
>Subject: [PATCH 1/6] TESTS: Add confdb domain base DN to sss_test_ctx
>
>Creation of the path to the domain's confdb entry was duplicated in the
>tests. Rather than adding yet another duplication, I added the path as
>another field of the sss_test_ctx structure.
>---
>From patch, it is clean that it was not used on many places.
It will be usefull in future.
ACK
>From 532cee578213b3d0d9d13229c72adefeb126f8cf Mon Sep 17 00:00:00 2001
>From: Jakub Hrozek <jhrozek at redhat.com>
>Date: Mon, 23 Jun 2014 17:52:03 +0200
>Subject: [PATCH 2/6] TESTS: Use the right confdb path
>
>The nested group test only worked by accident. Its confdb settings were
>not applied because a wrong confdb path was used.
>---
ACK
>From 09ca16557b3eb556f38a959d1749160c42a33578 Mon Sep 17 00:00:00 2001
>From: Jakub Hrozek <jhrozek at redhat.com>
>Date: Mon, 23 Jun 2014 18:07:51 +0200
>Subject: [PATCH 3/6] TESTS: Fix group search base
>
>After fixing the confdb initialization I realized the group DN couldn't
>be parsed. This patch fixes that.
>---
> src/tests/cmocka/test_nested_groups.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/src/tests/cmocka/test_nested_groups.c b/src/tests/cmocka/test_nested_groups.c
>index 9f38e3dd388dcafcdaa61cb39136c30fa58b399a..bf678c2b2025e54e6ed50b8b33fe41a78ffec007 100644
>--- a/src/tests/cmocka/test_nested_groups.c
>+++ b/src/tests/cmocka/test_nested_groups.c
>@@ -43,7 +43,7 @@
> /* put users and groups under the same container so we can easily run the
> * same tests cases for several search base scenarios */
> #define OBJECT_BASE_DN "cn=objects,dc=test,dc=com"
>-#define GROUP_BASE_DN "cn=groups" OBJECT_BASE_DN
>+#define GROUP_BASE_DN "cn=groups," OBJECT_BASE_DN
Could you used constant from SYSDB_GROUPS_CONTAINER from sysdb.h?
>From 0cd01ebe4f7f61e830cb06c907b04ff07200dbd1 Mon Sep 17 00:00:00 2001
>From: Jakub Hrozek <jhrozek at redhat.com>
>Date: Mon, 23 Jun 2014 19:06:52 +0200
>Subject: [PATCH 4/6] TESTS: Do not require replies from mocked
> sdap_get_generic_recv to be talloc contexts
>
>While it's beneficial for the real implementation of
>sdap_get_generic_recv() to move memory around, the mocked implementation
>can just pass around pointer.
>---
make sense.
ACK
>From 67a524e5ce1b7c48b99104a8726b18a090550ae4 Mon Sep 17 00:00:00 2001
>From: Jakub Hrozek <jhrozek at redhat.com>
>Date: Mon, 23 Jun 2014 19:07:41 +0200
>Subject: [PATCH 5/6] TESTS: Change how mock_sysdb_user() is implemented
>
>For the purpose of unit tests, it's better to create a user object with
>a UID and a name.
>---
ACK
>From 3907236536cd4157ab068311b76f0b0babde2681 Mon Sep 17 00:00:00 2001
>From: Jakub Hrozek <jhrozek at redhat.com>
>Date: Mon, 23 Jun 2014 19:07:59 +0200
>Subject: [PATCH 6/6] TESTS: Add more tests for nested groups processing
>
>Adds unit test for basic group retrieval functionality as well as for
>testing duplicate members in the LDAP group entry.
Could you add reference (git hash) to patch with PB's fix?
>---
> src/tests/cmocka/test_nested_groups.c | 130 +++++++++++++++++++++++++++++++++-
> 1 file changed, 129 insertions(+), 1 deletion(-)
>
>diff --git a/src/tests/cmocka/test_nested_groups.c b/src/tests/cmocka/test_nested_groups.c
>index bf678c2b2025e54e6ed50b8b33fe41a78ffec007..693d0fb33837643566d7a9ab0dc02bd5cff9c849 100644
>--- a/src/tests/cmocka/test_nested_groups.c
>+++ b/src/tests/cmocka/test_nested_groups.c
>@@ -44,6 +44,7 @@
> * same tests cases for several search base scenarios */
> #define OBJECT_BASE_DN "cn=objects,dc=test,dc=com"
> #define GROUP_BASE_DN "cn=groups," OBJECT_BASE_DN
>+#define USER_BASE_DN "cn=users," OBJECT_BASE_DN
Could you used constant from SYSDB_USERS_CONTAINER from sysdb.h?
>
> struct nested_groups_test_ctx {
> struct sss_test_ctx *tctx;
>@@ -114,11 +115,136 @@ static void nested_groups_test_one_group_no_members(void **state)
> assert_true(rootgroup == test_ctx->groups[0]);
> }
>
>+static void nested_groups_test_one_group_unique_members(void **state)
>+{
>+ struct nested_groups_test_ctx *test_ctx = NULL;
>+ struct sysdb_attrs *rootgroup = NULL;
>+ struct tevent_req *req = NULL;
>+ TALLOC_CTX *req_mem_ctx = NULL;
>+ errno_t ret;
>+ const char *name;
>+ const char *users[] = { "cn=user1,"USER_BASE_DN,
>+ "cn=user2,"USER_BASE_DN,
>+ NULL };
>+ const struct sysdb_attrs *user1_reply[2] = { NULL };
>+ const struct sysdb_attrs *user2_reply[2] = { NULL };
>+
>+ test_ctx = talloc_get_type_abort(*state, struct nested_groups_test_ctx);
>+
>+ /* mock return values */
>+ rootgroup = mock_sysdb_group_rfc2307bis(test_ctx, GROUP_BASE_DN, 1000,
>+ "rootgroup", users);
>+
>+ user1_reply[0] = mock_sysdb_user(test_ctx, USER_BASE_DN, 2001, "user1");
>+ assert_non_null(user1_reply[0]);
>+ will_return(sdap_get_generic_recv, 1);
>+ will_return(sdap_get_generic_recv, user1_reply);
>+
>+ user2_reply[0] = mock_sysdb_user(test_ctx, USER_BASE_DN, 2002, "user2");
>+ assert_non_null(user2_reply[0]);
>+ will_return(sdap_get_generic_recv, 1);
>+ will_return(sdap_get_generic_recv, user2_reply);
>+
>+ sss_will_return_always(sdap_has_deref_support, false);
>+
>+ /* run test, check for memory leaks */
>+ req_mem_ctx = talloc_new(global_talloc_context);
>+ assert_non_null(req_mem_ctx);
>+ check_leaks_push(req_mem_ctx);
>+
>+ req = sdap_nested_group_send(req_mem_ctx, test_ctx->tctx->ev,
>+ test_ctx->sdap_domain, test_ctx->sdap_opts,
>+ test_ctx->sdap_handle, rootgroup);
>+ assert_non_null(req);
>+ tevent_req_set_callback(req, nested_groups_test_done, test_ctx);
>+
>+ ret = test_ev_loop(test_ctx->tctx);
>+ assert_true(check_leaks_pop(req_mem_ctx) == true);
>+ talloc_zfree(req_mem_ctx);
>+
>+ /* check return code */
>+ assert_int_equal(ret, ERR_OK);
>+
>+ /* Check the users */
>+ assert_int_equal(test_ctx->num_users, 2);
>+ assert_int_equal(test_ctx->num_groups, 1);
>+
>+ ret = sysdb_attrs_get_string(test_ctx->users[0], SYSDB_NAME, &name);
>+ assert_int_equal(ret, EOK);
>+ assert_string_equal(name, "user1");
>+
>+ ret = sysdb_attrs_get_string(test_ctx->users[1], SYSDB_NAME, &name);
>+ assert_int_equal(ret, EOK);
>+ assert_string_equal(name, "user2");
>+}
>+
>+static void nested_groups_test_one_group_dup_users(void **state)
>+{
>+ struct nested_groups_test_ctx *test_ctx = NULL;
>+ struct sysdb_attrs *rootgroup = NULL;
>+ struct tevent_req *req = NULL;
>+ TALLOC_CTX *req_mem_ctx = NULL;
>+ errno_t ret;
>+ const char *name;
>+ const char *users[] = { "cn=user1,"USER_BASE_DN,
>+ "cn=user1,"USER_BASE_DN,
>+ NULL };
>+ const struct sysdb_attrs *user1_reply[2] = { NULL };
>+ const struct sysdb_attrs *user2_reply[2] = { NULL };
>+
>+ test_ctx = talloc_get_type_abort(*state, struct nested_groups_test_ctx);
>+
>+ /* mock return values */
>+ rootgroup = mock_sysdb_group_rfc2307bis(test_ctx, GROUP_BASE_DN, 1000,
>+ "rootgroup", users);
>+
>+ user1_reply[0] = mock_sysdb_user(test_ctx, USER_BASE_DN, 2001, "user1");
>+ assert_non_null(user1_reply[0]);
>+ will_return(sdap_get_generic_recv, 1);
>+ will_return(sdap_get_generic_recv, user1_reply);
>+
>+ user2_reply[0] = mock_sysdb_user(test_ctx, USER_BASE_DN, 2001, "user1");
>+ assert_non_null(user2_reply[0]);
>+ will_return(sdap_get_generic_recv, 1);
>+ will_return(sdap_get_generic_recv, user2_reply);
>+
>+ sss_will_return_always(sdap_has_deref_support, false);
>+
>+ /* run test, check for memory leaks */
>+ req_mem_ctx = talloc_new(global_talloc_context);
>+ assert_non_null(req_mem_ctx);
>+ check_leaks_push(req_mem_ctx);
>+
>+ req = sdap_nested_group_send(req_mem_ctx, test_ctx->tctx->ev,
>+ test_ctx->sdap_domain, test_ctx->sdap_opts,
>+ test_ctx->sdap_handle, rootgroup);
>+ assert_non_null(req);
>+ tevent_req_set_callback(req, nested_groups_test_done, test_ctx);
>+
>+ ret = test_ev_loop(test_ctx->tctx);
>+ assert_true(check_leaks_pop(req_mem_ctx) == true);
>+ talloc_zfree(req_mem_ctx);
>+
>+ /* check return code */
>+ assert_int_equal(ret, ERR_OK);
^^^^^^
>+
>+ /* Check the users */
>+ assert_int_equal(test_ctx->num_users, 1);
>+ assert_int_equal(test_ctx->num_groups, 1);
>+
>+ ret = sysdb_attrs_get_string(test_ctx->users[0], SYSDB_NAME, &name);
>+ assert_int_equal(ret, EOK);
^^^
You mixed EOK and ERR_OK. According to git history, ERR_OK is newer.
We should decide and use just one in next patches.
>+ assert_string_equal(name, "user1");
>+}
>+
> void nested_groups_test_setup(void **state)
> {
> struct nested_groups_test_ctx *test_ctx = NULL;
> static struct sss_test_conf_param params[] = {
> { "ldap_schema", "rfc2307bis" }, /* enable nested groups */
>+ { "ldap_search_base", OBJECT_BASE_DN },
>+ { "ldap_user_search_base", USER_BASE_DN },
>+ { "ldap_group_search_base", GROUP_BASE_DN },
> { NULL, NULL }
> };
>
>@@ -163,7 +289,9 @@ int main(int argc, const char *argv[])
> };
>
> const UnitTest tests[] = {
>- new_test(one_group_no_members)
>+ new_test(one_group_no_members),
>+ new_test(one_group_unique_members),
>+ new_test(one_group_dup_users)
> };
>
> /* Set debug level to invalid value so we can deside if -d 0 was used. */
I tested with reverted aptch "nested groups: do not fail if we get one entry
twice" and unit test fail. It's good.
PB modified function sdap_nested_group_single_step_process on two places
1) in case SDAP_NESTED_GROUP_DN_USER
2) in case SDAP_NESTED_GROUP_DN_GROUP
Unit test covers only the first part. Do we want ot also test the 2nd one?
LS
More information about the sssd-devel
mailing list