[Freeipa-devel] [PATCH 0019][Tests] Fix for failing location tests

[Lenka Doudova]

On 06/23/2016 10:30 AM, Martin Basti wrote:
>
>
>
> On 23.06.2016 06:55, Lenka Doudova wrote:
>>
>>
>>
>> On 06/22/2016 05:11 PM, Lenka Doudova wrote:
>>>
>>>
>>>
>>> On 06/22/2016 04:37 PM, Lenka Doudova wrote:
>>>>
>>>>
>>>>
>>>> On 06/22/2016 08:33 AM, Martin Basti wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 22.06.2016 07:37, Lenka Doudova wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 06/21/2016 06:57 PM, Martin Basti wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 21.06.2016 15:39, Lenka Doudova wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> attaching patch for failing location tests 
>>>>>>>> (ipatests/test_xmlrpc/test_location_plugin.py).
>>>>>>>>
>>>>>>>> Lenka
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> Hello,
>>>>>>>
>>>>>>> 1)
>>>>>>> + expected_updates={u'ipalocation_location': 
>>>>>>> [location.idnsname_obj],
>>>>>>> + u'enabled_role_servrole': (
>>>>>>> +                                  u'CA server', u'DNS server', 
>>>>>>> u'NTP server')},
>>>>>>>
>>>>>>> This depends on services installed on server, so server without 
>>>>>>> DNS will cause test failures. We probably should skip test id 
>>>>>>> DNS isn't installed.
>>>>>>> Without DNS installed you get much more different warnings
>>>>>>>
>>>>>>>
>>>>>>> 2)
>>>>>>> +    def update(self, updates, expected_updates=None, 
>>>>>>> messages=None):
>>>>>>> ....
>>>>>>> +        self.messages = messages
>>>>>>>
>>>>>>> Why is this needed? I'm puzzled by this
>>>>>>>
>>>>>>> It is defined outside __init__ what is wrong and it is never used.
>>>>>>>
>>>>>> Hi, thanks for review.
>>>>>> ad 1: will fix
>>>>>> ad 2: the 'messages' key is indeed used, because this key is 
>>>>>> returned every time a server is attached to/removed from a 
>>>>>> location. If the 'messages' is not supplied to the result 
>>>>>> comparison, the test fails (see 
>>>>>> https://paste.fedoraproject.org/382936/65732411/ for result of 
>>>>>> test without applied patch).
>>>>>>
>>>>>> Lenka
>>>>>
>>>>> Please point me to the line where ServerTracker.messages is used, 
>>>>> I still don't see it. In your fpaste, removed self.messages in 
>>>>> that context is TestLocationsServer.messages not 
>>>>> ServerTracker.messages
>>>>>
>>>>> Martin^2
>>>>
>>>> Ah, right you are, my good sir. I fixed both issues, hopefully will 
>>>> be fine now. Fixed patch attached.
>>>> Lenka
>>>>
>>>>
>>> Self NACK, will provide some more changes tomorrow.
>>> Lenka
>>>
>>>
>> And here it goes...
>>
>> Lenka
>>
>>
> This is really weird
>
> fuzzy_servrole = Fuzzy(
>     type=list,
>      test=lambda other: False not in set(
>         item in (u'CA server', u'DNS server', u'NTP server')
>         for item in other)
>     and u'DNS server' in other)
>
> 1) Why is there special case 'DNS server' in other, IMO general fuzzy 
> object should not enforce DNS server role
>
> 2) Several servroles related to AD trust are missing there, so if you 
> are doing general fuzzy object for server roles it should contains all 
> roles
>
> 3)
> I suggest something like
> lambda other: all(item in {u'CA server', u'DNS server', u'NTP server', 
> ...} for item in other)
>
> 4) during yesterday's interactive review, I suggested to use just 
> lambda other: True, as serveroles are tested in different test suite, 
> but If you fix issues above, it can stay as it is
>
> Martin^2
Fixed according to suggestion from interactiove review. Attached.
Lenka
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160623/12d3ce55/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-ldoudova-0019.4-Tests-Fix-for-failing-location-tests.patch
Type: text/x-patch
Size: 9729 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160623/12d3ce55/attachment.bin>