On 03/15/2012 10:36 PM, Rob Crittenden wrote:
Petr Viktorin wrote:
Can you spot the bug in this test code?

try:
do_invalid_operation()
except ExpectedError:
pass


Our test suite had several of those.
Nose provides nice tools, `raises` (a decorator) and `assert_raises` (a
context manager) that make checking expected exceptions a lot easier and
less error-prone. This patch makes our tests use them.

If you didn't catch it, the error is that the test will pass when no
exception is raised. Some of our tests handled that by adding an `else:
assert False`, or an `assert False` at the end of the try block.
For consistency, the patch switches these correct ones to
raises/assert_raises as well.

I've also uncovered and fixed a few test bugs that were hidden by this.



test_1a_automountmap_add_indirect() was failing, checking for the wrong
exception.

Silly me. Thanks for catching this.

I also suggest using @raises for clarity in another spot. Here are my
suggested changes:


Attaching updated patch.



--
PetrĀ³
From 2945d42140861bebe8d66dcae29a5c9d577f3dce Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Fri, 9 Mar 2012 09:41:16 -0500
Subject: [PATCH] Use nose tools to check for exceptions

Some of our tests checked for exceptions using an error-prone
try block: they allowed the expected exception to pass, but sometimes
forgot an else block, so the test passed when an exception wasn't
thrown.

This changes the tests to use the appropriate nose tools (raises,
assert_raises).
For consistency, tests that had a correct else block are also changed.

Also fix some test problems that were hidden by the above:
- in some sudorule and HBAC tests, change the *_add_user argument name
  from `users` to `user`
- don't remove HBAC testing data while it was still used
---
 tests/test_install/test_updates.py         |   15 ++---
 tests/test_xmlrpc/test_automount_plugin.py |   86 +++++------------------
 tests/test_xmlrpc/test_cert.py             |    7 +-
 tests/test_xmlrpc/test_hbac_plugin.py      |  103 ++++++++++++----------------
 tests/test_xmlrpc/test_passwd_plugin.py    |    9 +--
 tests/test_xmlrpc/test_sudorule_plugin.py  |   74 ++++++++------------
 6 files changed, 105 insertions(+), 189 deletions(-)

diff --git a/tests/test_install/test_updates.py b/tests/test_install/test_updates.py
index eb376f19113e534563d02eae51a798f7b9d9c773..8b9ac28eb68535d3820feea6d47639d32df7be56 100644
--- a/tests/test_install/test_updates.py
+++ b/tests/test_install/test_updates.py
@@ -24,7 +24,8 @@ import os
 import sys
 import ldap
 import nose
-from tests.util import raises, PluginTester
+from nose.tools import raises
+from tests.util import PluginTester
 from tests.data import unicode_str
 from ipalib import api
 from ipalib import errors
@@ -182,20 +183,16 @@ class test_update(object):
 
         assert(modified == True)
 
+    @raises(BadSyntax)
     def test_8_badsyntax(self):
         """
         Test the updater with an unknown keyword
         """
-        try:
-            modified = self.updater.update([self.testdir + "8_badsyntax.update"])
-        except BadSyntax:
-            pass
+        modified = self.updater.update([self.testdir + "8_badsyntax.update"])
 
+    @raises(BadSyntax)
     def test_9_badsyntax(self):
         """
         Test the updater with an incomplete line
         """
-        try:
-            modified = self.updater.update([self.testdir + "9_badsyntax.update"])
-        except BadSyntax:
-            pass
+        modified = self.updater.update([self.testdir + "9_badsyntax.update"])
diff --git a/tests/test_xmlrpc/test_automount_plugin.py b/tests/test_xmlrpc/test_automount_plugin.py
index cfea61270fca423fe29e00747b507d170e05d255..dedd2346aaca52434074e028ff9ed1629d69f9d2 100644
--- a/tests/test_xmlrpc/test_automount_plugin.py
+++ b/tests/test_xmlrpc/test_automount_plugin.py
@@ -22,6 +22,8 @@ Test the `ipalib/plugins/automount.py' module.
 """
 
 import sys
+from nose.tools import raises, assert_raises  # pylint: disable=E0611
+
 from xmlrpc_test import XMLRPC_test, assert_attr_equal
 from ipalib import api
 from ipalib import errors
@@ -77,16 +79,12 @@ class test_automount(XMLRPC_test):
         assert res
         assert_attr_equal(res, 'automountkey', self.keyname)
 
+    @raises(errors.DuplicateEntry)
     def test_4_automountkey_add(self):
         """
         Test adding a duplicate key using `xmlrpc.automountkey_add` method.
         """
-        try:
-            api.Command['automountkey_add'](self.locname, self.mapname, **self.key_kw)
-        except errors.DuplicateEntry:
-            pass
-        else:
-            assert False
+        res = api.Command['automountkey_add'](self.locname, self.mapname, **self.key_kw)
 
     def test_5_automountmap_show(self):
         """
@@ -153,12 +151,8 @@ class test_automount(XMLRPC_test):
         assert_attr_equal(res, 'failed', '')
 
         # Verify that it is gone
-        try:
+        with assert_raises(errors.NotFound):
             api.Command['automountkey_show'](self.locname, self.mapname, **delkey_kw)
-        except errors.NotFound:
-            pass
-        else:
-            assert False
 
     def test_c_automountlocation_del(self):
         """
@@ -169,12 +163,8 @@ class test_automount(XMLRPC_test):
         assert_attr_equal(res, 'failed', '')
 
         # Verify that it is gone
-        try:
+        with assert_raises(errors.NotFound):
             api.Command['automountlocation_show'](self.locname)
-        except errors.NotFound:
-            pass
-        else:
-            assert False
 
     def test_d_automountmap_del(self):
         """
@@ -182,12 +172,8 @@ class test_automount(XMLRPC_test):
         """
         # Verify that the second key we added is gone
         key_kw = {'automountkey': self.keyname2, 'automountinformation': self.info, 'raw': True}
-        try:
+        with assert_raises(errors.NotFound):
             api.Command['automountkey_show'](self.locname, self.mapname, **key_kw)
-        except errors.NotFound:
-            pass
-        else:
-            assert False
 
 class test_automount_direct(XMLRPC_test):
     """
@@ -214,16 +200,12 @@ class test_automount_direct(XMLRPC_test):
         assert res
         assert_attr_equal(res, 'automountmapname', self.mapname)
 
+    @raises(errors.DuplicateEntry)
     def test_2_automountmap_add_duplicate(self):
         """
         Test adding a duplicate direct map.
         """
-        try:
-            res = api.Command['automountmap_add_indirect'](self.locname, self.mapname, **self.direct_kw)['result']
-        except errors.DuplicateEntry:
-            pass
-        else:
-            assert False
+        res = api.Command['automountmap_add_indirect'](self.locname, self.mapname, **self.direct_kw)['result']
 
     def test_3_automountlocation_del(self):
         """
@@ -234,12 +216,8 @@ class test_automount_direct(XMLRPC_test):
         assert_attr_equal(res, 'failed', '')
 
         # Verity that it is gone
-        try:
+        with assert_raises(errors.NotFound):
             api.Command['automountlocation_show'](self.locname)
-        except errors.NotFound:
-            pass
-        else:
-            assert False
 
 class test_automount_indirect(XMLRPC_test):
     """
@@ -269,16 +247,12 @@ class test_automount_indirect(XMLRPC_test):
         assert res
         assert_attr_equal(res, 'automountmapname', self.mapname)
 
+    @raises(errors.DuplicateEntry)
     def test_1a_automountmap_add_indirect(self):
         """
         Test adding a duplicate indirect map.
         """
-        try:
-            api.Command['automountmap_add_indirect'](self.locname, self.mapname, **self.map_kw)['result']
-        except errors.DuplicateEntry:
-            pass
-        else:
-            assert False
+        api.Command['automountmap_add_indirect'](self.locname, self.mapname, **self.map_kw)['result']
 
     def test_2_automountmap_show(self):
         """
@@ -297,12 +271,8 @@ class test_automount_indirect(XMLRPC_test):
         assert_attr_equal(res, 'failed', '')
 
         # Verify that it is gone
-        try:
+        with assert_raises(errors.NotFound):
             api.Command['automountkey_show'](self.locname, self.parentmap, **self.key_kw)
-        except errors.NotFound:
-            pass
-        else:
-            assert False
 
     def test_4_automountmap_del(self):
         """
@@ -313,12 +283,8 @@ class test_automount_indirect(XMLRPC_test):
         assert_attr_equal(res, 'failed', '')
 
         # Verify that it is gone
-        try:
+        with assert_raises(errors.NotFound):
             api.Command['automountmap_show'](self.locname, self.mapname)
-        except errors.NotFound:
-            pass
-        else:
-            assert False
 
     def test_5_automountlocation_del(self):
         """
@@ -329,12 +295,8 @@ class test_automount_indirect(XMLRPC_test):
         assert_attr_equal(res, 'failed', '')
 
         # Verity that it is gone
-        try:
+        with assert_raises(errors.NotFound):
             api.Command['automountlocation_show'](self.locname)
-        except errors.NotFound:
-            pass
-        else:
-            assert False
 
 
 class test_automount_indirect_no_parent(XMLRPC_test):
@@ -382,12 +344,8 @@ class test_automount_indirect_no_parent(XMLRPC_test):
         assert_attr_equal(res, 'failed', '')
 
         # Verify that it is gone
-        try:
+        with assert_raises(errors.NotFound):
             api.Command['automountkey_show'](self.locname, self.parentmap, **delkey_kw)
-        except errors.NotFound:
-            pass
-        else:
-            assert False
 
     def test_4_automountmap_del(self):
         """
@@ -398,12 +356,8 @@ class test_automount_indirect_no_parent(XMLRPC_test):
         assert_attr_equal(res, 'failed', '')
 
         # Verify that it is gone
-        try:
+        with assert_raises(errors.NotFound):
             api.Command['automountmap_show'](self.locname, self.mapname)
-        except errors.NotFound:
-            pass
-        else:
-            assert False
 
     def test_5_automountlocation_del(self):
         """
@@ -414,9 +368,5 @@ class test_automount_indirect_no_parent(XMLRPC_test):
         assert_attr_equal(res, 'failed', '')
 
         # Verity that it is gone
-        try:
+        with assert_raises(errors.NotFound):
             api.Command['automountlocation_show'](self.locname)
-        except errors.NotFound:
-            pass
-        else:
-            assert False
diff --git a/tests/test_xmlrpc/test_cert.py b/tests/test_xmlrpc/test_cert.py
index 20aba16521101feb1f8d3fc23c16aeb2799610a4..253373a2ae2f390c482416e6dfa5472b1715996d 100644
--- a/tests/test_xmlrpc/test_cert.py
+++ b/tests/test_xmlrpc/test_cert.py
@@ -23,6 +23,8 @@ Test the `ipalib/plugins/cert.py` module against the selfsign plugin.
 import sys
 import os
 import shutil
+from nose.tools import assert_raises  # pylint: disable=E0611
+
 from xmlrpc_test import XMLRPC_test, assert_attr_equal
 from ipalib import api
 from ipalib import errors
@@ -105,11 +107,8 @@ class test_cert(XMLRPC_test):
         res = api.Command['host_add'](self.host_fqdn, force= True)['result']
 
         csr = unicode(self.generateCSR(str(self.subject)))
-        try:
+        with assert_raises(errors.NotFound):
             res = api.Command['cert_request'](csr, principal=self.service_princ)
-            assert False
-        except errors.NotFound:
-            pass
 
     def test_2_cert_add(self):
         """
diff --git a/tests/test_xmlrpc/test_hbac_plugin.py b/tests/test_xmlrpc/test_hbac_plugin.py
index ef61d68fc07a314d7b7e9083bf888c9f71da8b86..268c8d9d2217de71a936f691e5dc1666e9afb4b1 100644
--- a/tests/test_xmlrpc/test_hbac_plugin.py
+++ b/tests/test_xmlrpc/test_hbac_plugin.py
@@ -20,6 +20,8 @@
 Test the `ipalib/plugins/hbacrule.py` module.
 """
 
+from nose.tools import raises, assert_raises  # pylint: disable=E0611
+
 from xmlrpc_test import XMLRPC_test, assert_attr_equal
 from ipalib import api
 from ipalib import errors
@@ -63,18 +65,14 @@ class test_hbac(XMLRPC_test):
         assert_attr_equal(entry, 'ipaenabledflag', 'TRUE')
         assert_attr_equal(entry, 'description', self.rule_desc)
 
+    @raises(errors.DuplicateEntry)
     def test_1_hbacrule_add(self):
         """
         Test adding an existing HBAC rule using `xmlrpc.hbacrule_add'.
         """
-        try:
-            api.Command['hbacrule_add'](
+        api.Command['hbacrule_add'](
                 self.rule_name, accessruletype=self.rule_type
             )
-        except errors.DuplicateEntry:
-            pass
-        else:
-            assert False
 
     def test_2_hbacrule_show(self):
         """
@@ -400,20 +398,6 @@ class test_hbac(XMLRPC_test):
         assert not failed['sourcehost']['hostgroup']
         entry = ret['result']
 
-    def test_c_hbacrule_zap_testing_data(self):
-        """
-        Clear data for HBAC plugin testing.
-        """
-        api.Command['hbacrule_remove_host'](self.rule_name, host=self.test_host)
-        api.Command['hbacrule_remove_host'](self.rule_name, hostgroup=self.test_hostgroup)
-        api.Command['user_del'](self.test_user)
-        api.Command['group_del'](self.test_group)
-        api.Command['host_del'](self.test_host)
-        api.Command['hostgroup_del'](self.test_hostgroup)
-        api.Command['host_del'](self.test_sourcehost)
-        api.Command['hostgroup_del'](self.test_sourcehostgroup)
-        api.Command['hbacsvc_del'](self.test_service)
-
     def test_d_hbacrule_disable(self):
         """
         Test disabling HBAC rule using `xmlrpc.hbacrule_disable`.
@@ -433,6 +417,7 @@ class test_hbac(XMLRPC_test):
          # FIXME: Should this be 'enabled' or 'TRUE'?
         assert_attr_equal(entry, 'ipaenabledflag', 'TRUE')
 
+    @raises(errors.MutuallyExclusiveError)
     def test_f_hbacrule_exclusiveuser(self):
         """
         Test adding a user to an HBAC rule when usercat='all'
@@ -440,22 +425,21 @@ class test_hbac(XMLRPC_test):
         api.Command['hbacrule_mod'](self.rule_name, usercategory=u'all')
         try:
             api.Command['hbacrule_add_user'](self.rule_name, users='admin')
-        except errors.MutuallyExclusiveError:
-            pass
-        api.Command['hbacrule_mod'](self.rule_name, usercategory=u'')
+        finally:
+            api.Command['hbacrule_mod'](self.rule_name, usercategory=u'')
 
+    @raises(errors.MutuallyExclusiveError)
     def test_g_hbacrule_exclusiveuser(self):
         """
         Test setting usercat='all' in an HBAC rule when there are users
         """
-        api.Command['hbacrule_add_user'](self.rule_name, users='admin')
+        api.Command['hbacrule_add_user'](self.rule_name, user='admin')
         try:
             api.Command['hbacrule_mod'](self.rule_name, usercategory=u'all')
-        except errors.MutuallyExclusiveError:
-            pass
         finally:
-            api.Command['hbacrule_remove_user'](self.rule_name, users='admin')
+            api.Command['hbacrule_remove_user'](self.rule_name, user='admin')
 
+    @raises(errors.MutuallyExclusiveError)
     def test_h_hbacrule_exclusivehost(self):
         """
         Test adding a host to an HBAC rule when hostcat='all'
@@ -463,10 +447,10 @@ class test_hbac(XMLRPC_test):
         api.Command['hbacrule_mod'](self.rule_name, hostcategory=u'all')
         try:
             api.Command['hbacrule_add_host'](self.rule_name, host=self.test_host)
-        except errors.MutuallyExclusiveError:
-            pass
-        api.Command['hbacrule_mod'](self.rule_name, hostcategory=u'')
+        finally:
+            api.Command['hbacrule_mod'](self.rule_name, hostcategory=u'')
 
+    @raises(errors.MutuallyExclusiveError)
     def test_i_hbacrule_exclusivehost(self):
         """
         Test setting hostcat='all' in an HBAC rule when there are hosts
@@ -474,22 +458,21 @@ class test_hbac(XMLRPC_test):
         api.Command['hbacrule_add_host'](self.rule_name, host=self.test_host)
         try:
             api.Command['hbacrule_mod'](self.rule_name, hostcategory=u'all')
-        except errors.MutuallyExclusiveError:
-            pass
         finally:
             api.Command['hbacrule_remove_host'](self.rule_name, host=self.test_host)
 
+    @raises(errors.MutuallyExclusiveError)
     def test_j_hbacrule_exclusiveservice(self):
         """
         Test adding a service to an HBAC rule when servicecat='all'
         """
         api.Command['hbacrule_mod'](self.rule_name, servicecategory=u'all')
         try:
-            api.Command['hbacrule_add_host'](self.rule_name, hbacsvc=self.test_service)
-        except errors.MutuallyExclusiveError:
-            pass
-        api.Command['hbacrule_mod'](self.rule_name, servicecategory=u'')
+            api.Command['hbacrule_add_service'](self.rule_name, hbacsvc=self.test_service)
+        finally:
+            api.Command['hbacrule_mod'](self.rule_name, servicecategory=u'')
 
+    @raises(errors.MutuallyExclusiveError)
     def test_k_hbacrule_exclusiveservice(self):
         """
         Test setting servicecat='all' in an HBAC rule when there are services
@@ -497,35 +480,43 @@ class test_hbac(XMLRPC_test):
         api.Command['hbacrule_add_service'](self.rule_name, hbacsvc=self.test_service)
         try:
             api.Command['hbacrule_mod'](self.rule_name, servicecategory=u'all')
-        except errors.MutuallyExclusiveError:
-            pass
         finally:
             api.Command['hbacrule_remove_service'](self.rule_name, hbacsvc=self.test_service)
 
+    @raises(errors.ValidationError)
     def test_l_hbacrule_add(self):
         """
         Test adding a new HBAC rule with a deny type.
         """
-        try:
-            api.Command['hbacrule_add'](
-                u'denyrule',
-                accessruletype=u'deny',
-                description=self.rule_desc,
-            )
-        except errors.ValidationError:
-            pass
+        api.Command['hbacrule_add'](
+            u'denyrule',
+            accessruletype=u'deny',
+            description=self.rule_desc,
+        )
 
+    @raises(errors.ValidationError)
     def test_m_hbacrule_add(self):
         """
         Test changing an HBAC rule to the deny type
         """
-        try:
-            api.Command['hbacrule_mod'](
-                self.rule_name,
-                accessruletype=u'deny',
-            )
-        except errors.ValidationError:
-            pass
+        api.Command['hbacrule_mod'](
+            self.rule_name,
+            accessruletype=u'deny',
+        )
+
+    def test_y_hbacrule_zap_testing_data(self):
+        """
+        Clear data for HBAC plugin testing.
+        """
+        api.Command['hbacrule_remove_host'](self.rule_name, host=self.test_host)
+        api.Command['hbacrule_remove_host'](self.rule_name, hostgroup=self.test_hostgroup)
+        api.Command['user_del'](self.test_user)
+        api.Command['group_del'](self.test_group)
+        api.Command['host_del'](self.test_host)
+        api.Command['hostgroup_del'](self.test_hostgroup)
+        api.Command['host_del'](self.test_sourcehost)
+        api.Command['hostgroup_del'](self.test_sourcehostgroup)
+        api.Command['hbacsvc_del'](self.test_service)
 
     def test_z_hbacrule_del(self):
         """
@@ -533,9 +524,5 @@ class test_hbac(XMLRPC_test):
         """
         api.Command['hbacrule_del'](self.rule_name)
         # verify that it's gone
-        try:
+        with assert_raises(errors.NotFound):
             api.Command['hbacrule_show'](self.rule_name)
-        except errors.NotFound:
-            pass
-        else:
-            assert False
diff --git a/tests/test_xmlrpc/test_passwd_plugin.py b/tests/test_xmlrpc/test_passwd_plugin.py
index 32e75acbed6cd43db6f4a12e24884c5093799066..2a44da711bf5696c561e8b5bac8b97222ca2e045 100644
--- a/tests/test_xmlrpc/test_passwd_plugin.py
+++ b/tests/test_xmlrpc/test_passwd_plugin.py
@@ -21,6 +21,9 @@ Test the `ipalib/plugins/passwd.py` module.
 """
 
 import sys
+
+from nose.tools import assert_raises  # pylint: disable=E0611
+
 from xmlrpc_test import XMLRPC_test, assert_attr_equal
 from ipalib import api
 from ipalib import errors
@@ -62,9 +65,5 @@ class test_passwd(XMLRPC_test):
         api.Command['user_del'](self.uid)
 
         # Verify that it is gone
-        try:
+        with assert_raises(errors.NotFound):
             api.Command['user_show'](self.uid)
-        except errors.NotFound:
-            pass
-        else:
-            assert False
diff --git a/tests/test_xmlrpc/test_sudorule_plugin.py b/tests/test_xmlrpc/test_sudorule_plugin.py
index bddf00c4f28ebcb1fe64839a84b1ab366f39185b..2a597c37bea3d621941b66c2b7bec0cf8600be28 100644
--- a/tests/test_xmlrpc/test_sudorule_plugin.py
+++ b/tests/test_xmlrpc/test_sudorule_plugin.py
@@ -21,6 +21,8 @@
 Test the `ipalib/plugins/sudorule.py` module.
 """
 
+from nose.tools import raises, assert_raises  # pylint: disable=E0611
+
 from xmlrpc_test import XMLRPC_test, assert_attr_equal
 from ipalib import api
 from ipalib import errors
@@ -63,18 +65,14 @@ class test_sudorule(XMLRPC_test):
         assert_attr_equal(entry, 'cn', self.rule_name)
         assert_attr_equal(entry, 'description', self.rule_desc)
 
+    @raises(errors.DuplicateEntry)
     def test_1_sudorule_add(self):
         """
         Test adding an duplicate Sudo rule using `xmlrpc.sudorule_add'.
         """
-        try:
-            api.Command['sudorule_add'](
-                self.rule_name
-            )
-        except errors.DuplicateEntry:
-            pass
-        else:
-            assert False
+        api.Command['sudorule_add'](
+            self.rule_name
+        )
 
     def test_2_sudorule_show(self):
         """
@@ -521,29 +519,29 @@ class test_sudorule(XMLRPC_test):
         assert 'memberdenycmd_sudocmd' not in entry
         assert 'memberdenycmd_sudocmdgroup' not in entry
 
+    @raises(errors.MutuallyExclusiveError)
     def test_c_sudorule_exclusiveuser(self):
         """
         Test adding a user to an Sudo rule when usercat='all'
         """
         api.Command['sudorule_mod'](self.rule_name, usercategory=u'all')
         try:
-            api.Command['sudorule_add_user'](self.rule_name, users='admin')
-        except errors.MutuallyExclusiveError:
-            pass
-        api.Command['sudorule_mod'](self.rule_name, usercategory=u'')
+            api.Command['sudorule_add_user'](self.rule_name, user=u'admin')
+        finally:
+            api.Command['sudorule_mod'](self.rule_name, usercategory=u'')
 
+    @raises(errors.MutuallyExclusiveError)
     def test_d_sudorule_exclusiveuser(self):
         """
         Test setting usercat='all' in an Sudo rule when there are users
         """
-        api.Command['sudorule_add_user'](self.rule_name, users='admin')
+        api.Command['sudorule_add_user'](self.rule_name, user=u'admin')
         try:
             api.Command['sudorule_mod'](self.rule_name, usercategory=u'all')
-        except errors.MutuallyExclusiveError:
-            pass
         finally:
-            api.Command['sudorule_remove_user'](self.rule_name, users='admin')
+            api.Command['sudorule_remove_user'](self.rule_name, user=u'admin')
 
+    @raises(errors.MutuallyExclusiveError)
     def test_e_sudorule_exclusivehost(self):
         """
         Test adding a host to an Sudo rule when hostcat='all'
@@ -551,10 +549,10 @@ class test_sudorule(XMLRPC_test):
         api.Command['sudorule_mod'](self.rule_name, hostcategory=u'all')
         try:
             api.Command['sudorule_add_host'](self.rule_name, host=self.test_host)
-        except errors.MutuallyExclusiveError:
-            pass
-        api.Command['sudorule_mod'](self.rule_name, hostcategory=u'')
+        finally:
+            api.Command['sudorule_mod'](self.rule_name, hostcategory=u'')
 
+    @raises(errors.MutuallyExclusiveError)
     def test_f_sudorule_exclusivehost(self):
         """
         Test setting hostcat='all' in an Sudo rule when there are hosts
@@ -562,11 +560,10 @@ class test_sudorule(XMLRPC_test):
         api.Command['sudorule_add_host'](self.rule_name, host=self.test_host)
         try:
             api.Command['sudorule_mod'](self.rule_name, hostcategory=u'all')
-        except errors.MutuallyExclusiveError:
-            pass
         finally:
             api.Command['sudorule_remove_host'](self.rule_name, host=self.test_host)
 
+    @raises(errors.MutuallyExclusiveError)
     def test_g_sudorule_exclusivecommand(self):
         """
         Test adding a command to an Sudo rule when cmdcategory='all'
@@ -574,10 +571,10 @@ class test_sudorule(XMLRPC_test):
         api.Command['sudorule_mod'](self.rule_name, cmdcategory=u'all')
         try:
             api.Command['sudorule_add_allow_command'](self.rule_name, sudocmd=self.test_command)
-        except errors.MutuallyExclusiveError:
-            pass
-        api.Command['sudorule_mod'](self.rule_name, cmdcategory=u'')
+        finally:
+            api.Command['sudorule_mod'](self.rule_name, cmdcategory=u'')
 
+    @raises(errors.MutuallyExclusiveError)
     def test_h_sudorule_exclusivecommand(self):
         """
         Test setting cmdcategory='all' in an Sudo rule when there are commands
@@ -585,11 +582,10 @@ class test_sudorule(XMLRPC_test):
         api.Command['sudorule_add_allow_command'](self.rule_name, sudocmd=self.test_command)
         try:
             api.Command['sudorule_mod'](self.rule_name, cmdcategory=u'all')
-        except errors.MutuallyExclusiveError:
-            pass
         finally:
             api.Command['sudorule_remove_allow_command'](self.rule_name, sudocmd=self.test_command)
 
+    @raises(errors.MutuallyExclusiveError)
     def test_i_sudorule_exclusiverunas(self):
         """
         Test adding a runasuser to an Sudo rule when ipasudorunasusercategory='all'
@@ -597,10 +593,10 @@ class test_sudorule(XMLRPC_test):
         api.Command['sudorule_mod'](self.rule_name, ipasudorunasusercategory=u'all')
         try:
             api.Command['sudorule_add_runasuser'](self.rule_name, sudocmd=self.test_user)
-        except errors.MutuallyExclusiveError:
-            pass
-        api.Command['sudorule_mod'](self.rule_name, ipasudorunasusercategory=u'')
+        finally:
+            api.Command['sudorule_mod'](self.rule_name, ipasudorunasusercategory=u'')
 
+    @raises(errors.MutuallyExclusiveError)
     def test_j_sudorule_exclusiverunas(self):
         """
         Test setting ipasudorunasusercategory='all' in an Sudo rule when there are runas users
@@ -608,8 +604,6 @@ class test_sudorule(XMLRPC_test):
         api.Command['sudorule_add_runasuser'](self.rule_name, user=self.test_user)
         try:
             api.Command['sudorule_mod'](self.rule_name, ipasudorunasusercategory=u'all')
-        except errors.MutuallyExclusiveError:
-            pass
         finally:
             api.Command['sudorule_remove_runasuser'](self.rule_name, user=self.test_command)
 
@@ -644,24 +638,18 @@ class test_sudorule(XMLRPC_test):
         api.Command['sudorule_del'](self.rule_name2)
 
         # add a new rule with a duplicate order
-        try:
+        with assert_raises(errors.ValidationError):
             api.Command['sudorule_add'](self.rule_name2, sudoorder=1)
-        except errors.ValidationError:
-            pass
 
         # add a new rule with a unique order
         api.Command['sudorule_add'](self.rule_name2, sudoorder=2)
-        try:
+        with assert_raises(errors.ValidationError):
             api.Command['sudorule_mod'](self.rule_name2, sudoorder=1)
-        except errors.ValidationError:
-            pass
 
         # Try setting both to 0
         api.Command['sudorule_mod'](self.rule_name2, sudoorder=0)
-        try:
+        with assert_raises(errors.ValidationError):
             api.Command['sudorule_mod'](self.rule_name, sudoorder=0)
-        except errors.ValidationError:
-            pass
 
 
     def test_m_sudorule_del(self):
@@ -670,10 +658,6 @@ class test_sudorule(XMLRPC_test):
         """
         api.Command['sudorule_del'](self.rule_name)
         # verify that it's gone
-        try:
+        with assert_raises(errors.NotFound):
             api.Command['sudorule_show'](self.rule_name)
-        except errors.NotFound:
-            pass
-        else:
-            assert False
         api.Command['sudorule_del'](self.rule_name2)
-- 
1.7.7.6

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

Reply via email to