[Freeipa-devel] [Patch] 0001 Adds check for ipa-join.
Tomas Babej
tbabej at redhat.com
Fri Aug 3 13:41:26 UTC 2012
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.
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0001-3-Adds-check-for-ipa-join.patch
Type: text/x-patch
Size: 2162 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20120803/3f4075c7/attachment.bin>
More information about the Freeipa-devel
mailing list