On (23/05/14 16:43), Yassir Elley wrote:
>
>
>----- Original Message -----
>> On Fri, May 23, 2014 at 03:50:18PM -0400, Yassir Elley wrote:
>> >
>> >
>> > ----- Original Message -----
>> > > On Fri, May 23, 2014 at 12:29:37PM -0400, Yassir Elley wrote:
>> > > >
>> > > >
>> > > > ----- Original Message -----
>> > > > > On Tue, May 20, 2014 at 10:30:15PM -0400, Yassir Elley wrote:
>> > > > > > Hi,
>> > > > > >
>> > > > > > The attached patch adds unit tests for some of the ad_gpo
>> > > > > > functionality. It
>> > > > > > also fixes some failure mode processing code in ad_gpo.c that was
>> > > > > > uncovered by the unit tests. This patch depends on a previously
>> > > > > > submitted
>> > > > > > patch ("Remove GPO dependency on libsamba-security"), which has not
>> > > > > > yet
>> > > > > > been pushed to master.
>> > > > > >
>> > > > > > Regards,
>> > > > > > Yassir.
>> > > > >
>> > > > > Can you send a rebased version on top of the latest version of your
>> > > > > 'AD-GPO: Remove dependency on libsamba-security' patch?
>> > > > >
>> > > > > bye,
>> > > > > Sumit
>> > > > > _______________________________________________
>> > > > > sssd-devel mailing list
>> > > > > [email protected]
>> > > > > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>> > > > >
>> > > >
>> > > > Rebased version of patch attached.
>> > >
>> > > I'm sorry, but the test does not compile on top of you other patches. I
>> > > think it is due to adding the idmap context:
>> > >
>> > > CC src/tests/cmocka/ad_gpo_tests-test_ad_gpo.o
>> > > ../src/tests/cmocka/test_ad_gpo.c: In function
>> > > 'test_ad_gpo_ace_includes_client_sid':
>> > > ../src/tests/cmocka/test_ad_gpo.c:269:42: warning: passing argument 5 of
>> > > 'ad_gpo_ace_includes_client_sid' from incompatible pointer type [enabled
>> > > by
>> > > default]
>> > > ace_dom_sid,
>> > > &includes_client_sid);
>> > > ^
>> > > In file included from ../src/tests/cmocka/test_ad_gpo.c:33:0:
>> > > ../src/providers/ad/ad_gpo.c:253:1: note: expected 'struct sss_idmap_ctx
>> > > *'
>> > > but argument is of type '_Bool *'
>> > > ad_gpo_ace_includes_client_sid(const char *user_sid,
>> > > ^
>> > > ../src/tests/cmocka/test_ad_gpo.c:269:42: error: too few arguments to
>> > > function 'ad_gpo_ace_includes_client_sid'
>> > > ace_dom_sid,
>> > > &includes_client_sid);
>> > > ^
>> > > In file included from ../src/tests/cmocka/test_ad_gpo.c:33:0:
>> > > ../src/providers/ad/ad_gpo.c:253:1: note: declared here
>> > > ad_gpo_ace_includes_client_sid(const char *user_sid,
>> > > ^
>> > > make[3]: *** [src/tests/cmocka/ad_gpo_tests-test_ad_gpo.o] Error 1
>> > >
>> > >
>> > > bye,
>> > > Sumit
>> > >
>> >
>> > That was very sloppy of me. Sorry about that. I've attached a revised
>> > patch.
>>
>> The test failed for me because of an issue in test_populate_som_list().
>> You should either set num_soms = 0 in the declaration or skip
>>
>> assert_int_equal(num_soms, expected->num_soms);
>>
>> if ret != EOK. Because otherwise you would compare with an uninitialized
>> value.
>>
>> I think skipping the assert_int_equal() makes more sense because you do
>> not expect num_soms to be set in the error case.
>>
>> bye,
>> Sumit
>>
>> >
>
>Although it is curious that the tests are failing for you but not for me,
>I do agree with your comments. There is a similar issue
>in test_populate_gplink_list. I have fixed them both and attached
>a revised patch.
>
>Regards,
>Yassir.
>From f075dd0ec7e1538e2f674b6cfd1a3fd4b842e480 Mon Sep 17 00:00:00 2001
>From: Yassir Elley <[email protected]>
>Date: Thu, 3 Apr 2014 11:21:43 -0400
>Subject: [PATCH] AD-GPO: Add ad_gpo unit test; Fix some failure modes in
> ad_gpo.c
>
>---
> Makefile.am | 29 +++
> src/providers/ad/ad_gpo.c | 26 ++-
> src/tests/cmocka/test_ad_gpo.c | 388 +++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 434 insertions(+), 9 deletions(-)
> create mode 100644 src/tests/cmocka/test_ad_gpo.c
>
>diff --git a/Makefile.am b/Makefile.am
>index
>510cfc78ba59f08fc73ed0f971e77ce6f2288461..134163fee47a923808b866daed92453b147fd79d
> 100644
>--- a/Makefile.am
>+++ b/Makefile.am
>@@ -177,6 +177,7 @@ if HAVE_CMOCKA
> test_sss_idmap \
> test_ipa_idmap \
> test_utils \
>+ ad_gpo_tests \
> ad_common_tests \
> dp_opt_tests \
> responder-get-domains-tests \
>@@ -1723,6 +1724,34 @@ ad_access_filter_tests_LDADD = \
> libsss_ad_common.la \
> libsss_test_common.la
>
>+ad_gpo_tests_SOURCES = \
>+ $(sssd_be_SOURCES) \
>+ src/util/sss_ldap.c \
>+ src/util/sss_krb5.c \
>+ src/util/find_uid.c \
>+ src/util/user_info_msg.c \
>+ src/providers/ad/ad_common.c \
>+ src/tests/cmocka/test_ad_gpo.c
>+ad_gpo_tests_CFLAGS = \
>+ $(AM_CFLAGS) \
>+ $(SYSTEMD_LOGIN_CFLAGS) \
>+ $(NDR_NBT_CFLAGS) \
>+ -DUNIT_TESTING
>+ad_gpo_tests_LDADD = \
>+ $(PAM_LIBS) \
>+ $(CMOCKA_LIBS) \
>+ $(SSSD_LIBS) \
>+ $(CARES_LIBS) \
>+ $(KRB5_LIBS) \
>+ $(SSSD_INTERNAL_LTLIBS) \
>+ $(SYSTEMD_LOGIN_LIBS) \
>+ $(NDR_NBT_LIBS) \
>+ libsss_ldap_common.la \
>+ libsss_idmap.la \
>+ libsss_krb5_common.la \
>+ libsss_ad_common.la \
>+ libsss_test_common.la
There were recent changes in Makefile.am and some lines can be removed.
src/util/user_info_msg.c was moved to libsss_util.so
src/util/find_uid.c was moved to libsss_util.so
src/util/sss_ldap.c was moved to libsss_ldap_common.so
src/util/sss_krb5.c was moved to libsss_krb5_common.so
Here is diff:
ad_gpo_tests_SOURCES = \
$(sssd_be_SOURCES) \
- src/util/sss_ldap.c \
- src/util/sss_krb5.c \
- src/util/find_uid.c \
- src/util/user_info_msg.c \
src/providers/ad/ad_common.c \
src/tests/cmocka/test_ad_gpo.c
ad_gpo_tests_CFLAGS = \
$(AM_CFLAGS) \
- $(SYSTEMD_LOGIN_CFLAGS) \
$(NDR_NBT_CFLAGS) \
-DUNIT_TESTING
ad_gpo_tests_LDADD = \
@@ -1752,9 +1747,7 @@ ad_gpo_tests_LDADD = \
$(CMOCKA_LIBS) \
$(SSSD_LIBS) \
$(CARES_LIBS) \
- $(KRB5_LIBS) \
$(SSSD_INTERNAL_LTLIBS) \
- $(SYSTEMD_LOGIN_LIBS) \
$(NDR_NBT_LIBS) \
libsss_ldap_common.la \
libsss_idmap.la \
Test passed without any problem. I let code review for Sumit.
LS
_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
