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(a)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.