[Freeipa-devel] [PATCH 0014-0016][Tests] Authentication indicators
Milan Kubík
mkubik at redhat.com
Wed Jul 13 14:48:45 UTC 2016
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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160713/88d4af4a/attachment.htm>
More information about the Freeipa-devel
mailing list