[Freeipa-devel] [Patch] 0001 Adds check for ipa-join.

Martin Kosek mkosek at redhat.com
Fri Aug 3 14:27:58 UTC 2012


On 08/03/2012 03:41 PM, Tomas Babej wrote:
> All suggestions implemented.
> 
> Tomas
> 
> ----- Original Message -----
> From: "Martin Kosek" <mkosek at redhat.com>
> To: "Tomas Babej" <tbabej at redhat.com>
> Cc: freeipa-devel at redhat.com
> Sent: Friday, August 3, 2012 11:24:03 AM
> Subject: Re: [Freeipa-devel] [Patch] 0001 Add check for existence of ipa-join in the tree in test_host_plugin.py
> 
> Hello,
> 
> few issues found:
> 
> 1) There is a change that should not be there:
> 
>                      managedby_host=[fqdn1],
> -                    ipakrbauthzdata=[u'MS-PAC'],
>                      ipauniqueid=[fuzzy_uuid],
> 
> 2) When raising SkipTest it would be better to pass a reason why we are
> skipping the text so that it is more clear in test output. I.e. instead of:
> 
> +            raise SkipTest
> 
> I would do something like that:
> +            raise SkipTest("Command '%s' not found" % cls.command)
> 
> Otherwise the patch looks ok.
> 
> Martin
> 
> On 08/02/2012 05:26 PM, Tomas Babej wrote:
>> I implemented your suggestions. New version of the patch attached.
>>
>> ----- Original Message -----
>> From: "Petr Viktorin" <pviktori at redhat.com>
>> To: freeipa-devel at redhat.com
>> Sent: Friday, July 27, 2012 1:23:54 PM
>> Subject: Re: [Freeipa-devel] [Patch] 0001 Add check for existence of ipa-join in the tree in test_host_plugin.py
>>
>> On 07/27/2012 01:13 PM, Tomas Babej wrote:
>>> Hi,
>>>
>>> this patch simply checks if ipa-join exists in ipa-client folder, if not, skips tests relying on it.
>>> Uses nose.plugin.skip.
>>>
>>> https://fedorahosted.org/freeipa/ticket/2905
>>>
>>> Tomas Babej
>>>
>>
>>
>> Hello,
>> This would be better done in class setup, so you don't have to repeat it 
>> in every method. Look at XMLRPC_test.setUpClass() in xmlrpc_test.py for 
>> isnpiration.
>>
>> While you're at it, it would be good to move the mkstemp() call into 
>> setUpClass as well, so that importing the module doesn't have 
>> unnecessary side effects.

ACK. Pushed to master.

Martin




More information about the Freeipa-devel mailing list