[SSSD] [PATCH] intg: Add more LDAP tests

Jakub Hrozek jhrozek at redhat.com
Wed Oct 7 13:13:50 UTC 2015


On Wed, Oct 07, 2015 at 04:02:32PM +0300, Nikolai Kondrashov wrote:
> On 10/06/2015 12:27 PM, Jakub Hrozek wrote:
> >On Wed, Sep 30, 2015 at 06:15:52PM +0300, Nikolai Kondrashov wrote:
> >>Hi everyone,
> >>
> >>Here is a patch set fixing some things in integration tests and adding more
> >>LDAP tests:
> >
> >(Not a full review, just adding my ideas and impressions)
> >
> >I read these patches when I tried to add tests for the failing POSIX
> >check. That didn't work out, but at least I have a better idea about the
> >integration tests now :-)
> 
> Was the problem with the integration test framework or with something else?
> If it's the former, can I offer my help?

No, it was that while the code that triggered the error was in the
generic LDAP provider, the bug itself manifested only in AD provider..so
we'd need to wrap Samba to really test the bug.

> 
> >Most importantly, the current tests are largely using enumeration. That
> >not wrong and we want this codepath to be tested, but it's not the
> >default of SSSD, so we want the non-enumeration also.
> 
> Alright, I'll try to use the py.test fixture parameters for this. It's a
> cumbersome facility, but perhaps we can use it. If that fails, I'll try
> something else.
> 
> I worry a bit about the doubled test runtime, though. Can we have some tests
> running only with either disabled or enabled enumeration?

Maybe..in general I think non-enumeration are more important so if the
rigorous/essential parameters also apply to which integration tests we
run, then I would prefer non-enumeration tests.

> 
> >I also wonder if instead of bool-like parameter that says if we're using
> >rfc2307 or rfc2307bis we should have a more generic 'schema' parameter.
> >
> >Both are in some way here:
> >     https://fedorapeople.org/cgit/jhrozek/public_git/sssd.git/commit/?h=intg_test&id=568a0090c664d61e0491c2c2e4f8a3b68189a816
> >
> >Most of the (not well formatted) patch is the schema change, you can
> >also see ldap_schema=ad being added. The test_broken_posix_in_ad()
> >function also shows a test without enumeration -- I think we want tests
> >for adding/removing a user/group/membership with rfc2307(bis) schema
> >along these lines as well.
> 
> Riiight, I completely forgot about the other schemas. Sure, we'll need
> something more than a boolean. However, how about we define a couple constant
> string variables and supply them instead of literal strings to reduce the
> likelihood of typos?

Yes, that's even better. Would you prefer if I sent such patch (atop
your patches) ?


More information about the sssd-devel mailing list