<html> <head> <meta content="text/html; charset=windows-1252" http-equiv="Content-Type"> </head> <body bgcolor="#FFFFFF" text="#000000"> <div class="moz-cite-prefix">On 23.06.2016 06:55, Lenka Doudova wrote: </div> <blockquote cite="mid:0a5ec8ab-6ec0-ee67-9fb0-4536705cf254@redhat.com" type="cite"> <meta content="text/html; charset=windows-1252" http-equiv="Content-Type"> <div class="moz-cite-prefix">On 06/22/2016 05:11 PM, Lenka Doudova wrote: </div> <blockquote cite="mid:0dc6726b-f597-ad9f-3cee-a78f96223cca@redhat.com" type="cite"> <meta content="text/html; charset=windows-1252" http-equiv="Content-Type"> <div class="moz-cite-prefix">On 06/22/2016 04:37 PM, Lenka Doudova wrote: </div> <blockquote cite="mid:8900bfe6-6495-aaa9-39ce-c201cb5b25a0@redhat.com" type="cite"> <meta content="text/html; charset=windows-1252" http-equiv="Content-Type"> <div class="moz-cite-prefix">On 06/22/2016 08:33 AM, Martin Basti wrote: </div> <blockquote cite="mid:33c965cf-ac80-5e8e-feae-81a48ba81fba@redhat.com" type="cite"> <meta content="text/html; charset=windows-1252" http-equiv="Content-Type"> <div class="moz-cite-prefix">On 22.06.2016 07:37, Lenka Doudova wrote: </div> <blockquote cite="mid:4ab90b76-0a95-40cd-8721-783d8908d37c@redhat.com" type="cite"> <meta content="text/html; charset=windows-1252" http-equiv="Content-Type"> <div class="moz-cite-prefix">On 06/21/2016 06:57 PM, Martin Basti wrote: </div> <blockquote cite="mid:5d0b933a-d96d-8dfa-a0f3-e59240618f67@redhat.com" type="cite"> <meta content="text/html; charset=windows-1252" http-equiv="Content-Type"> <div class="moz-cite-prefix">On 21.06.2016 15:39, Lenka Doudova wrote: </div> <blockquote cite="mid:6ff234ed-6cf3-7742-b388-d192f27e40a7@redhat.com" type="cite">Hi, attaching patch for failing location tests (ipatests/test_xmlrpc/test_location_plugin.py). Lenka <fieldset class="mimeAttachmentHeader"></fieldset> </blockquote> 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. </blockquote> 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 <a moz-do-not-send="true" class="moz-txt-link-freetext" href="https://paste.fedoraproject.org/382936/65732411/"><a class="moz-txt-link-freetext" href="https://paste.fedoraproject.org/382936/65732411/">https://paste.fedoraproject.org/382936/65732411/</a></a> for result of test without applied patch). Lenka </blockquote> 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 </blockquote> Ah, right you are, my good sir. I fixed both issues, hopefully will be fine now. Fixed patch attached. Lenka <fieldset class="mimeAttachmentHeader"></fieldset> </blockquote> Self NACK, will provide some more changes tomorrow. Lenka <fieldset class="mimeAttachmentHeader"></fieldset> </blockquote> And here it goes... Lenka <fieldset class="mimeAttachmentHeader"></fieldset> </blockquote> 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 </body> </html>