Clarify dangerous use of exceptions in unit tests
Also, update unit tests to use more specific exception where possible to make auditing easier in the future. Change-Id: I4906e2f8e3ebacf0587f1471ca8dd46c73edc17a
This commit is contained in:
parent
7cdd6c3fb4
commit
c82dba22c2
|
@ -54,13 +54,33 @@ Writing Integration Tests
|
|||
|
||||
TBD
|
||||
|
||||
Tests and assertRaises
|
||||
----------------------
|
||||
When asserting that a test should raise an exception, test against the
|
||||
most specific exception possible. An overly broad exception type (like
|
||||
Exception) can mask errors in the unit test itself.
|
||||
Tests and Exceptions
|
||||
--------------------
|
||||
A properly written test asserts that particular behavior occurs. This can
|
||||
be a success condition or a failure condition, including an exception.
|
||||
When asserting that a particular exception is raised, the most specific
|
||||
exception possible should be used.
|
||||
|
||||
In particular, testing for Exception being raised is almost always a
|
||||
mistake since it will match (almost) every exception, even those
|
||||
unrelated to the exception intended to be tested.
|
||||
|
||||
This applies to catching exceptions manually with a try/except block,
|
||||
or using assertRaises().
|
||||
|
||||
Example::
|
||||
|
||||
self.assertRaises(exception.InstanceNotFound, db.instance_get_by_uuid,
|
||||
elevated, instance_uuid)
|
||||
|
||||
If a stubbed function/method needs a generic exception for testing
|
||||
purposes, test.TestingException is available.
|
||||
|
||||
Example::
|
||||
|
||||
def stubbed_method(self):
|
||||
raise test.TestingException()
|
||||
self.stubs.Set(cls, 'inner_method', stubbed_method)
|
||||
|
||||
obj = cls()
|
||||
self.assertRaises(test.TestingException, obj.outer_method)
|
||||
|
|
|
@ -290,10 +290,8 @@ class MiscFunctionsTest(test.TestCase):
|
|||
try:
|
||||
common.raise_http_conflict_for_instance_invalid_state(exc,
|
||||
'meow')
|
||||
except Exception, e:
|
||||
self.assertTrue(isinstance(e, webob.exc.HTTPConflict))
|
||||
msg = str(e)
|
||||
self.assertEqual(msg,
|
||||
except webob.exc.HTTPConflict as e:
|
||||
self.assertEqual(unicode(e),
|
||||
"Cannot 'meow' while instance is in fake_attr fake_state")
|
||||
else:
|
||||
self.fail("webob.exc.HTTPConflict was not raised")
|
||||
|
@ -303,10 +301,8 @@ class MiscFunctionsTest(test.TestCase):
|
|||
try:
|
||||
common.raise_http_conflict_for_instance_invalid_state(exc,
|
||||
'meow')
|
||||
except Exception, e:
|
||||
self.assertTrue(isinstance(e, webob.exc.HTTPConflict))
|
||||
msg = str(e)
|
||||
self.assertEqual(msg,
|
||||
except webob.exc.HTTPConflict as e:
|
||||
self.assertEqual(unicode(e),
|
||||
"Instance is in an invalid state for 'meow'")
|
||||
else:
|
||||
self.fail("webob.exc.HTTPConflict was not raised")
|
||||
|
|
|
@ -2264,7 +2264,7 @@ class ComputeTestCase(BaseTestCase):
|
|||
|
||||
try:
|
||||
raise NotImplementedError('test')
|
||||
except Exception:
|
||||
except NotImplementedError:
|
||||
exc_info = sys.exc_info()
|
||||
|
||||
self.stubs.Set(nova.db, 'instance_fault_create', fake_db_fault_create)
|
||||
|
@ -2292,7 +2292,7 @@ class ComputeTestCase(BaseTestCase):
|
|||
|
||||
try:
|
||||
raise user_exc
|
||||
except Exception:
|
||||
except exception.Invalid:
|
||||
exc_info = sys.exc_info()
|
||||
|
||||
self.stubs.Set(nova.db, 'instance_fault_create', fake_db_fault_create)
|
||||
|
|
|
@ -46,11 +46,10 @@ class DbApiTestCase(test.TestCase):
|
|||
return db.instance_create(self.context, args)
|
||||
|
||||
def test_ec2_ids_not_found_are_printable(self):
|
||||
|
||||
def check_exc_format(method):
|
||||
try:
|
||||
method(self.context, 'fake')
|
||||
except Exception as exc:
|
||||
except exception.NotFound as exc:
|
||||
self.assertTrue('fake' in unicode(exc))
|
||||
|
||||
check_exc_format(db.get_ec2_volume_id_by_uuid)
|
||||
|
|
Loading…
Reference in New Issue