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?
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?
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_...
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?
Nick