All suggestions implemented.

Tomas

----- Original Message -----
From: "Martin Kosek" <mko...@redhat.com>
To: "Tomas Babej" <tba...@redhat.com>
Cc: freeipa-devel@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" <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.
> 


From 1804dbf2153efe13f7c29b80273e8a6e361183e7 Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Fri, 3 Aug 2012 09:29:31 -0400
Subject: [PATCH] Adds check for ipa-join.

If the executable ipa-client/ipa-join is not found, the relevant
tests are skipped. Implemented in setUpClass() method, also moved
the mkstemp() call there.

https://fedorahosted.org/freeipa/ticket/2905
---
 tests/test_xmlrpc/test_host_plugin.py | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/tests/test_xmlrpc/test_host_plugin.py b/tests/test_xmlrpc/test_host_plugin.py
index 03aa089a2739486f6033a9d1870d7567bfdf1f5a..c9ec6a2bdda6eaebed72b1088e9e3cbb223d528b 100644
--- a/tests/test_xmlrpc/test_host_plugin.py
+++ b/tests/test_xmlrpc/test_host_plugin.py
@@ -28,6 +28,7 @@ from ipapython import ipautil
 from ipalib import api, errors, x509
 from ipalib.dn import *
 from nose.tools import raises, assert_raises
+from nose.plugins.skip import Skip, SkipTest
 from tests.test_xmlrpc.xmlrpc_test import (Declarative, XMLRPC_test,
     fuzzy_uuid, fuzzy_digits, fuzzy_hash, fuzzy_date, fuzzy_issuer,
     fuzzy_hex)
@@ -751,10 +752,17 @@ class test_host_false_pwd_change(XMLRPC_test):
     fqdn1 = u'testhost1.%s' % api.env.domain
     short1 = u'testhost1'
     new_pass = u'pass_123'
-
     command = "ipa-client/ipa-join"
-    [keytabfd, keytabname] = tempfile.mkstemp()
-    os.close(keytabfd)
+
+    @classmethod
+    def setUpClass(cls):
+        [cls.keytabfd,cls.keytabname] = tempfile.mkstemp()
+        os.close(cls.keytabfd)
+
+        does_command_exist = os.path.isfile(cls.command)
+
+        if not does_command_exist:
+            raise SkipTest("Command '%s' not found" % cls.command)
 
     # auxiliary function for checking whether the join operation has set
     # correct attributes
@@ -767,6 +775,7 @@ class test_host_false_pwd_change(XMLRPC_test):
         """
         Create a test host and join him into IPA.
         """
+
         # create a test host with bulk enrollment password
         random_pass = api.Command['host_add'](self.fqdn1, random=True, force=True)['result']['randompassword']
 
-- 
1.7.11.2

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to