[SSSD] [PATCH] TESTS: Do not rely on order of hash items
Jakub Hrozek
jhrozek at redhat.com
Mon Jul 7 10:00:55 UTC 2014
On Mon, Jul 07, 2014 at 09:27:44AM +0200, Lukas Slebodnik wrote:
> On (04/07/14 14:44), Jakub Hrozek wrote:
> >Hi,
> >
> >the attached patch should fix a bug Sumit was seeing in the unit tests.
>
> >From 17dd2ef636f537fa67eb140717b4b4b4376346c9 Mon Sep 17 00:00:00 2001
> >From: Jakub Hrozek <jhrozek at redhat.com>
> >Date: Fri, 4 Jul 2014 14:42:45 +0200
> >Subject: [PATCH] TESTS: Do not rely on order of hash items
> >
> >The nested group test was checking returned elements in a particular
> >order. That's not reliable because the returned values are fetched from
> >a hash iterator that doesn't guarantee the same order on different
> >systems.
> >---
> > src/tests/cmocka/test_nested_groups.c | 76 ++++++++++++++++++++++-------------
> > 1 file changed, 47 insertions(+), 29 deletions(-)
> >
> nestedgroups-tests passed with your patch on ix86.
>
> >diff --git a/src/tests/cmocka/test_nested_groups.c b/src/tests/cmocka/test_nested_groups.c
> >index 463d66efbbde28934d54cc71365b32b0c7fbbee1..261353643c76b6c7b3985069c0732d205a4f3c30 100644
> >--- a/src/tests/cmocka/test_nested_groups.c
> >+++ b/src/tests/cmocka/test_nested_groups.c
> >@@ -46,6 +46,9 @@
> > #define GROUP_BASE_DN "cn=groups," OBJECT_BASE_DN
> > #define USER_BASE_DN "cn=users," OBJECT_BASE_DN
> >
> >+#define N_ELEMENTS(arr) \
> >+ (sizeof(arr) / sizeof(arr[0]))
> >+
> > struct nested_groups_test_ctx {
> > struct sss_test_ctx *tctx;
> >
> >@@ -59,6 +62,36 @@ struct nested_groups_test_ctx {
> > unsigned long num_groups;
> > };
> >
> >+/* Both arrays must have the same length! */
> >+static void compare_sysdb_string_array_noorder(struct sysdb_attrs **sysdb_array,
> >+ const char **string_array,
> >+ size_t len)
> >+{
> >+ int i, ii;
> >+ errno_t ret;
> >+ const char *name;
> >+
> >+ /* Check the returned groups. The order is irrelevant. */
> >+ for (i = 0; i < len; i++) {
> >+ ret = sysdb_attrs_get_string(sysdb_array[i], SYSDB_NAME, &name);
> >+ assert_int_equal(ret, ERR_OK);
> >+
> >+ for (ii = 0; ii < len; ii++) {
> >+ if (string_array[ii] == NULL) {
> >+ continue;
> >+ }
> >+ if (strcmp(name, string_array[ii]) == 0) {
> >+ string_array[ii] = NULL;
> >+ break;
> >+ }
> >+ }
> >+ }
> >+
> >+ for (i = 0; i < len; i++) {
> >+ assert_null(string_array[i]);
> >+ }
> >+}
> >+
> > static void nested_groups_test_done(struct tevent_req *req)
> > {
> > struct nested_groups_test_ctx *ctx = NULL;
> >@@ -122,12 +155,14 @@ static void nested_groups_test_one_group_unique_members(void **state)
> > 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 };
> >+ const char * expected[] = { "user1",
> >+ "user2" };
> >+
> >
> > test_ctx = talloc_get_type_abort(*state, struct nested_groups_test_ctx);
> >
> >@@ -169,13 +204,8 @@ static void nested_groups_test_one_group_unique_members(void **state)
> > assert_int_equal(test_ctx->num_users, 2);
> ^^^
> Could you replace number with N_ELEMENTS(expected)
Sure, I prefer this option.
> or use test_ctx->num_users as argument
> in compare_sysdb_string_array_noorder
Thanks for the review, new patch is attached.
-------------- next part --------------
>From 4f067bbe180a255ff7b873050a092ab3823b9cb5 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Fri, 4 Jul 2014 14:42:45 +0200
Subject: [PATCH] TESTS: Do not rely on order of hash items
The nested group test was checking returned elements in a particular
order. That's not reliable because the returned values are fetched from
a hash iterator that doesn't guarantee the same order on different
systems.
---
src/tests/cmocka/test_nested_groups.c | 82 +++++++++++++++++++++--------------
1 file changed, 50 insertions(+), 32 deletions(-)
diff --git a/src/tests/cmocka/test_nested_groups.c b/src/tests/cmocka/test_nested_groups.c
index 463d66efbbde28934d54cc71365b32b0c7fbbee1..955d05dd426af5219b7187046920de86437da4b3 100644
--- a/src/tests/cmocka/test_nested_groups.c
+++ b/src/tests/cmocka/test_nested_groups.c
@@ -46,6 +46,9 @@
#define GROUP_BASE_DN "cn=groups," OBJECT_BASE_DN
#define USER_BASE_DN "cn=users," OBJECT_BASE_DN
+#define N_ELEMENTS(arr) \
+ (sizeof(arr) / sizeof(arr[0]))
+
struct nested_groups_test_ctx {
struct sss_test_ctx *tctx;
@@ -59,6 +62,36 @@ struct nested_groups_test_ctx {
unsigned long num_groups;
};
+/* Both arrays must have the same length! */
+static void compare_sysdb_string_array_noorder(struct sysdb_attrs **sysdb_array,
+ const char **string_array,
+ size_t len)
+{
+ int i, ii;
+ errno_t ret;
+ const char *name;
+
+ /* Check the returned groups. The order is irrelevant. */
+ for (i = 0; i < len; i++) {
+ ret = sysdb_attrs_get_string(sysdb_array[i], SYSDB_NAME, &name);
+ assert_int_equal(ret, ERR_OK);
+
+ for (ii = 0; ii < len; ii++) {
+ if (string_array[ii] == NULL) {
+ continue;
+ }
+ if (strcmp(name, string_array[ii]) == 0) {
+ string_array[ii] = NULL;
+ break;
+ }
+ }
+ }
+
+ for (i = 0; i < len; i++) {
+ assert_null(string_array[i]);
+ }
+}
+
static void nested_groups_test_done(struct tevent_req *req)
{
struct nested_groups_test_ctx *ctx = NULL;
@@ -122,12 +155,14 @@ static void nested_groups_test_one_group_unique_members(void **state)
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 };
+ const char * expected[] = { "user1",
+ "user2" };
+
test_ctx = talloc_get_type_abort(*state, struct nested_groups_test_ctx);
@@ -166,16 +201,11 @@ static void nested_groups_test_one_group_unique_members(void **state)
assert_int_equal(ret, ERR_OK);
/* Check the users */
- assert_int_equal(test_ctx->num_users, 2);
+ assert_int_equal(test_ctx->num_users, N_ELEMENTS(expected));
assert_int_equal(test_ctx->num_groups, 1);
- ret = sysdb_attrs_get_string(test_ctx->users[0], SYSDB_NAME, &name);
- assert_int_equal(ret, ERR_OK);
- assert_string_equal(name, "user1");
-
- ret = sysdb_attrs_get_string(test_ctx->users[1], SYSDB_NAME, &name);
- assert_int_equal(ret, ERR_OK);
- assert_string_equal(name, "user2");
+ compare_sysdb_string_array_noorder(test_ctx->users,
+ expected, N_ELEMENTS(expected));
}
static void nested_groups_test_one_group_dup_users(void **state)
@@ -244,12 +274,14 @@ static void nested_groups_test_one_group_unique_group_members(void **state)
struct tevent_req *req = NULL;
TALLOC_CTX *req_mem_ctx = NULL;
errno_t ret;
- const char *name;
const char *groups[] = { "cn=emptygroup1,"GROUP_BASE_DN,
"cn=emptygroup2,"GROUP_BASE_DN,
NULL };
const struct sysdb_attrs *group1_reply[2] = { NULL };
const struct sysdb_attrs *group2_reply[2] = { NULL };
+ const char * expected[] = { "rootgroup",
+ "emptygroup1",
+ "emptygroup2" };
test_ctx = talloc_get_type_abort(*state, struct nested_groups_test_ctx);
@@ -291,19 +323,10 @@ static void nested_groups_test_one_group_unique_group_members(void **state)
/* Check the users */
assert_int_equal(test_ctx->num_users, 0);
- assert_int_equal(test_ctx->num_groups, 3);
+ assert_int_equal(test_ctx->num_groups, N_ELEMENTS(expected));
- ret = sysdb_attrs_get_string(test_ctx->groups[0], SYSDB_NAME, &name);
- assert_int_equal(ret, ERR_OK);
- assert_string_equal(name, "rootgroup");
-
- ret = sysdb_attrs_get_string(test_ctx->groups[1], SYSDB_NAME, &name);
- assert_int_equal(ret, ERR_OK);
- assert_string_equal(name, "emptygroup2");
-
- ret = sysdb_attrs_get_string(test_ctx->groups[2], SYSDB_NAME, &name);
- assert_int_equal(ret, ERR_OK);
- assert_string_equal(name, "emptygroup1");
+ compare_sysdb_string_array_noorder(test_ctx->groups,
+ expected, N_ELEMENTS(expected));
}
static void nested_groups_test_one_group_dup_group_members(void **state)
@@ -313,12 +336,13 @@ static void nested_groups_test_one_group_dup_group_members(void **state)
struct tevent_req *req = NULL;
TALLOC_CTX *req_mem_ctx = NULL;
errno_t ret;
- const char *name;
const char *groups[] = { "cn=emptygroup1,"GROUP_BASE_DN,
"cn=emptygroup1,"GROUP_BASE_DN,
NULL };
const struct sysdb_attrs *group1_reply[2] = { NULL };
const struct sysdb_attrs *group2_reply[2] = { NULL };
+ const char * expected[] = { "rootgroup",
+ "emptygroup1" };
test_ctx = talloc_get_type_abort(*state, struct nested_groups_test_ctx);
@@ -358,17 +382,11 @@ static void nested_groups_test_one_group_dup_group_members(void **state)
/* check return code */
assert_int_equal(ret, ERR_OK);
- /* Check the users */
assert_int_equal(test_ctx->num_users, 0);
- assert_int_equal(test_ctx->num_groups, 2);
+ assert_int_equal(test_ctx->num_groups, N_ELEMENTS(expected));
- ret = sysdb_attrs_get_string(test_ctx->groups[0], SYSDB_NAME, &name);
- assert_int_equal(ret, ERR_OK);
- assert_string_equal(name, "rootgroup");
-
- ret = sysdb_attrs_get_string(test_ctx->groups[1], SYSDB_NAME, &name);
- assert_int_equal(ret, ERR_OK);
- assert_string_equal(name, "emptygroup1");
+ compare_sysdb_string_array_noorder(test_ctx->groups,
+ expected, N_ELEMENTS(expected));
}
void nested_groups_test_setup(void **state)
--
1.9.3
More information about the sssd-devel
mailing list