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