<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 07/14/2016 09:20 AM, Lenka Doudova wrote: </div> <blockquote cite="mid:fdd5a89f-6774-08ba-e810-a621cfa78669@redhat.com" type="cite"> <meta content="text/html; charset=windows-1252" http-equiv="Content-Type"> <div class="moz-cite-prefix">On 07/13/2016 04:48 PM, Milan Kubík wrote: </div> <blockquote cite="mid:a17fed62-31ec-2998-5b54-ebbd442dd325@redhat.com" type="cite"> <meta content="text/html; charset=windows-1252" http-equiv="Content-Type"> <div class="moz-cite-prefix">On 07/11/2016 01:34 PM, Lenka Doudova wrote: </div> <blockquote cite="mid:fa2afe96-1a10-7727-2282-028396d42ed7@redhat.com" type="cite"> <meta content="text/html; charset=windows-1252" http-equiv="Content-Type"> <div class="moz-cite-prefix">On 07/08/2016 02:24 PM, Milan Kubík wrote: </div> <blockquote cite="mid:8eb18fba-f6dd-d4e9-d2c7-dfd85c035903@redhat.com" type="cite"> <meta content="text/html; charset=windows-1252" http-equiv="Content-Type"> <div class="moz-cite-prefix">On 07/01/2016 05:13 PM, Lenka Doudova wrote: </div> <blockquote cite="mid:0aa06017-fcd2-5867-73a9-4dfde6223215@redhat.com" type="cite"> <meta content="text/html; charset=windows-1252" http-equiv="Content-Type"> <div class="moz-cite-prefix">On 07/01/2016 02:42 PM, Milan Kubík wrote: </div> <blockquote cite="mid:556ae9fd-a110-c5a3-b80a-f45d80b01c16@redhat.com" type="cite"> <meta content="text/html; charset=windows-1252" http-equiv="Content-Type"> <div class="moz-cite-prefix">On 06/16/2016 03:23 PM, Lenka Doudova wrote: </div> <blockquote cite="mid:766fd34a-b83b-1105-57d0-bd51420720a7@redhat.com" type="cite">Hi, attached are tests for authentication indicators. Please note: 1. newly created service tracker is not exactly complete, list of unimplemented methods is in doc. These methods can be filled in when existing declarative tests are refactored. 2. patch 0015 depends on 0014, so it should not be pushed without it. Lenka <fieldset class="mimeAttachmentHeader"></fieldset> </blockquote> patch 0014: In the update method, what happens when the updated attributes contain addattr? It is not clear to me. Is it necessary? </blockquote> Example: ipa service-mod SRV --addattr="authind=radius" Result: The way the tracker works, this adds u'addattr="authind=radius"' to the list of expected results (result of self.attrs.update(updates). Of course nothing like that appears anywhere, so in case there's the --addattr option, it's necessary to ensure it won't get to the self.attrs atribute. <blockquote cite="mid:556ae9fd-a110-c5a3-b80a-f45d80b01c16@redhat.com" type="cite"> patch 0015: host1 and service2 do not tell anything about the purpose of the fixture. Please assign more descriptive names to them. Why do the fixtures have 'function' scope? Does the service entry exist during the second and third test case? </blockquote> Renamed. <blockquote cite="mid:556ae9fd-a110-c5a3-b80a-f45d80b01c16@redhat.com" type="cite"> patch 0016: Per offline discussion, admin user has no special privileges here, LGTM. <pre class="moz-signature" cols="72">-- Milan Kubik</pre> </blockquote> Thanks for review, fixed patches (14.2 and 15.2) attached. Lenka </blockquote> NACK, the update method of ServiceTracker creates the entry if it doesn't exist. Why? I know the base class has this problem also [1], though. Given this will be addressed, the fixtures in the xmlrpc test will fail since the fixture scope is wrong - function instead of class. [1]: <a moz-do-not-send="true" class="moz-txt-link-freetext" href="https://fedorahosted.org/freeipa/ticket/6045">https://fedorahosted.org/freeipa/ticket/6045</a> <pre class="moz-signature" cols="72">-- Milan Kubik</pre> </blockquote> Hi, fixed patch attached. I chose to leave the scope as it was (in case one test breaks and entry is not even created, the other tests can still be successful), but I removed the creation command from ServiceTracker update method and called it directly in the tests, so there shouldn't be hidden actions. Lenka </blockquote> Thanks for the updated patches. There are still some issues I haven't noticed before: service tracker: track_create method doesn't set self.exists flag which is needed In the xmlrpc test method `test_adding_second_indicator` uses 'addattr' to set the indicator, why? <pre class="moz-signature" cols="72">-- Milan Kubik</pre> </blockquote> Hi, fixes for comments in attached patches. After offline discussion, I removed the usage of "addattr" and replaced it with test to add two indicators in one command. Lenka <fieldset class="mimeAttachmentHeader"></fieldset> </blockquote> One more problem was pointed out to me offline, attaching changed patch 0014. Lenka </body> </html>