[Freeipa-devel] [PATCH 0014-0016][Tests] Authentication indicators
Lenka Doudova
ldoudova at redhat.com
Thu Jul 14 09:25:34 UTC 2016
On 07/14/2016 09:20 AM, Lenka Doudova wrote:
>
>
>
> On 07/13/2016 04:48 PM, Milan Kubík wrote:
>> On 07/11/2016 01:34 PM, Lenka Doudova wrote:
>>>
>>>
>>>
>>> On 07/08/2016 02:24 PM, Milan Kubík wrote:
>>>> On 07/01/2016 05:13 PM, Lenka Doudova wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 07/01/2016 02:42 PM, Milan Kubík wrote:
>>>>>> On 06/16/2016 03:23 PM, Lenka Doudova wrote:
>>>>>>> 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
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> patch 0014:
>>>>>>
>>>>>> In the update method, what happens when the updated attributes
>>>>>> contain addattr? It is not clear to me. Is it necessary?
>>>>>>
>>>>> 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.
>>>>>
>>>>>> 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?
>>>>>>
>>>>> Renamed.
>>>>>>
>>>>>> patch 0016:
>>>>>>
>>>>>> Per offline discussion, admin user has no special privileges
>>>>>> here, LGTM.
>>>>>>
>>>>>> --
>>>>>> Milan Kubik
>>>>>
>>>>> Thanks for review, fixed patches (14.2 and 15.2) attached.
>>>>> Lenka
>>>>
>>>> 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]: https://fedorahosted.org/freeipa/ticket/6045
>>>>
>>>> --
>>>> Milan Kubik
>>> 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
>>
>> 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?
>>
>> --
>> Milan Kubik
>
> 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
>
>
One more problem was pointed out to me offline, attaching changed patch
0014.
Lenka
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160714/225b1c7c/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-ldoudova-0014.5-Tests-Tracker-class-for-services.patch
Type: text/x-patch
Size: 6392 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160714/225b1c7c/attachment.bin>
More information about the Freeipa-devel
mailing list