few issues found:

1) There is a change that should not be there:

-                    ipakrbauthzdata=[u'MS-PAC'],

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.


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" <pvikt...@redhat.com>
> To: freeipa-devel@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.

Freeipa-devel mailing list

Reply via email to