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

Michal Židek mzidek at redhat.com
Sun Nov 8 22:49:24 UTC 2015


I am attaching patch that can be applied and squashed into
Nick's patch. I am sending both patches to see what
changes have been made, *please squash them before pushing*.

This patch just removes the negchace (together with the
asserts that are testing it) and memcache,
and adds global constant to avoid using magic values.

It is an ACK from me.

CI:
http://sssd-ci.duckdns.org/logs/job/32/19/summary.html

I removed some tests from Nick's patch that need more
refactoring. I will send them as separate patch.

On 11/05/2015 03:27 PM, Nikolai Kondrashov wrote:
> On 11/04/2015 05:01 PM, Nikolai Kondrashov wrote:
>> On 11/04/2015 04:25 PM, Michal Židek wrote:
>>>
>>> That is good! If the patches do not work for you without the sleep()s
>>> you can send them with the sleep()s for now and I will take a look.

Just for the record, it did not work for me without the
sleeps either. It probably worked before, because I had some
unsaved changes in code. So Nick was right and the sleeps
are necessary because of the enum_cache_timeout.

>
> Please find attached patches with what I have working right now. The
> remaining
> work includes:
>
> * Put the 4s timeout into a global variable and use it instead of all the
>    literal timeouts throughout the code. Instead of 2s timeouts use that
>    variable divided by 2 - that's its meaning.
>
> * Attempt refactoring configuration generation inside loops within tests
> into
>    separate tests with separate fixtures. That concerns the following
>    functions:
>
>      test_filter_users
>      test_filter_groups_rfc2307
>      test_filter_groups_rfc2307_bis
>
>    Michal's comment was:
>
>      Instead of generating the conf files in nested for loops, it would be
>      better to split the test to multiple smaller tests where each has
>      different config file fixture (instad of void_conf and void_sssd).
>      This may seem like code duplication, but I believe it will result in
>      better readable code.
>
>    My response was:
>
>      I would really like to do that, but the problem is we will either
> need to
>      copy-paste a ton of tests and fixtures, or use py.test's convoluted
> means
>      of sharing parameters between fixtures and tests which will produce
> much
>      more complicated code that this. At least this is my impression after
>      researching that.
>
>    We can try doing that and seeing how it works.
>
> * Consider avoiding any assertions hitting negative cache, and disabling
>    negative cache to possibly increase reliability - Michal's suggestion.
>    Michal, please correct me if I cite it wrong or misunderstood it.
>
> * Check the patch with pep8 and resolve any found issues.
>
> * After the tests are merged, rebase the "intg: Add a memcache invalidation
>    failure test" patch on top of it and attach it to ticket #2726 as the
>    possible regression tracker.
>
> Nick

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-intg-Add-more-LDAP-tests.patch
Type: text/x-patch
Size: 22483 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20151108/47ebc9de/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Fixup-this-to-Nick-s-patch.patch
Type: text/x-patch
Size: 13181 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20151108/47ebc9de/attachment-0003.bin>


More information about the sssd-devel mailing list