testtools-dev team mailing list archive

["Martin \[gz\]" <gzlist@xxxxxxxxxxxxxx>]

Martin [gz] has proposed merging lp:~gz/testtools/raises_regressions_675327 into lp:testtools.

Requested reviews:
  testtools developers (testtools-dev)
Related bugs:
  #675327 Raises/MatchesException regressions
  https://bugs.launchpad.net/bugs/675327


Fixes the fallout on Python 2.4 and Python 3 from merging <https://code.launchpad.net/~lifeless/testtools/matchers/+merge/40606>.

The only behaviour change is str(MatchesException(Exception)) now returns "MatchesException(Exception)" rather than "MatchesException(<type 'exceptions.Exception'>)" which I think is nicer and makes life easier as across 2.4->2.5->3 you get three different reprs. Oh, and that exception classes custom repr methods now won't get used, which may be a good or bad thing for clarity, but should be safer at least.

Did need to add some extra hacks in testtools.compat however for Python 2.4 support. Adding isbaseexception was on my todo list anyway to tidy up all the places which currently catch KeyboardInterrupt separately (I'll put up anther mp for using it there, as it's another behaviour change). The _error_repr function just does what the new in Python 2.5 BaseException_repr does, which makes testing easier and mismatches on 2.4 equally pretty.

I initially thought the first revision which changed the invalid-on-Python-3 `raise a, b, c` to bare `raise` was incorrect, but actually it's fine. A different exception could be raised before that statement, but it will always either propagate, or be in a lower scope.
-- 
https://code.launchpad.net/~gz/testtools/raises_regressions_675327/+merge/41342
Your team testtools developers is requested to review the proposed merge of lp:~gz/testtools/raises_regressions_675327 into lp:testtools.

=== modified file 'testtools/compat.py'
--- testtools/compat.py	2010-10-19 22:04:09 +0000
+++ testtools/compat.py	2010-11-19 18:27:12 +0000
@@ -65,6 +65,27 @@
 _u.__doc__ = __u_doc
 
 
+if sys.version_info > (2, 5):
+    _error_repr = BaseException.__repr__
+    def isbaseexception(exception):
+        """Return whether exception inherits from BaseException only"""
+        return (isinstance(exception, BaseException)
+            and not isinstance(exception, Exception))
+else:
+    def _error_repr(exception):
+        """Format an exception instance as Python 2.5 and later do"""
+        return exception.__class__.__name__ + repr(exception.args)
+    def isbaseexception(exception):
+        """Return whether exception would inherit from BaseException only
+
+        This approximates the hierarchy in Python 2.5 and later, compare the
+        difference between the diagrams at the bottom of the pages:
+        <http://docs.python.org/release/2.4.4/lib/module-exceptions.html>
+        <http://docs.python.org/release/2.5.4/lib/module-exceptions.html>
+        """
+        return isinstance(exception, (KeyboardInterrupt, SystemExit))
+
+
 def unicode_output_stream(stream):
     """Get wrapper for given stream that writes any unicode without exception
 

=== modified file 'testtools/matchers.py'
--- testtools/matchers.py	2010-11-11 17:53:42 +0000
+++ testtools/matchers.py	2010-11-19 18:27:12 +0000
@@ -33,6 +33,8 @@
 from pprint import pformat
 import sys
 
+from testtools.compat import classtypes, _error_repr, isbaseexception
+
 
 class Matcher(object):
     """A pattern matcher.
@@ -344,25 +346,24 @@
         """
         Matcher.__init__(self)
         self.expected = exception
-
-    def _expected_type(self):
-        if type(self.expected) is type:
-            return self.expected
-        return type(self.expected)
+        self._is_instance = type(self.expected) not in classtypes()
 
     def match(self, other):
         if type(other) != tuple:
             return Mismatch('%r is not an exc_info tuple' % other)
-        if not issubclass(other[0], self._expected_type()):
-            return Mismatch('%r is not a %r' % (
-                other[0], self._expected_type()))
-        if (type(self.expected) is not type and
-            other[1].args != self.expected.args):
-            return Mismatch('%r has different arguments to %r.' % (
-                other[1], self.expected))
+        expected_class = self.expected
+        if self._is_instance:
+            expected_class = expected_class.__class__
+        if not issubclass(other[0], expected_class):
+            return Mismatch('%r is not a %r' % (other[0], expected_class))
+        if self._is_instance and other[1].args != self.expected.args:
+            return Mismatch('%s has different arguments to %s.' % (
+                _error_repr(other[1]), _error_repr(self.expected)))
 
     def __str__(self):
-        return "MatchesException(%r)" % self.expected
+        if self._is_instance:
+            return "MatchesException(%s)" % _error_repr(self.expected)
+        return "MatchesException(%s)" % self.expected.__name__
 
 
 class StartsWith(Matcher):
@@ -467,7 +468,6 @@
         # Catch all exceptions: Raises() should be able to match a
         # KeyboardInterrupt or SystemExit.
         except:
-            exc_info = sys.exc_info()
             if self.exception_matcher:
                 mismatch = self.exception_matcher.match(sys.exc_info())
                 if not mismatch:
@@ -476,9 +476,9 @@
                 mismatch = None
             # The exception did not match, or no explicit matching logic was
             # performed. If the exception is a non-user exception (that is, not
-            # a subclass of Exception) then propogate it.
-            if not issubclass(exc_info[0], Exception):
-                raise exc_info[0], exc_info[1], exc_info[2]
+            # a subclass of Exception on Python 2.5+) then propogate it.
+            if isbaseexception(sys.exc_info()[1]):
+                raise
             return mismatch
 
     def __str__(self):

=== modified file 'testtools/tests/test_matchers.py'
--- testtools/tests/test_matchers.py	2010-11-11 17:53:42 +0000
+++ testtools/tests/test_matchers.py	2010-11-19 18:27:12 +0000
@@ -181,8 +181,7 @@
          MatchesException(Exception('foo')))
         ]
     describe_examples = [
-        ("<type 'exceptions.Exception'> is not a "
-         "<type 'exceptions.ValueError'>",
+        ("%r is not a %r" % (Exception, ValueError),
          error_base_foo,
          MatchesException(ValueError("foo"))),
         ("ValueError('bar',) has different arguments to ValueError('foo',).",
@@ -201,12 +200,11 @@
     matches_mismatches = [error_base_foo]
 
     str_examples = [
-        ("MatchesException(<type 'exceptions.Exception'>)",
+        ("MatchesException(Exception)",
          MatchesException(Exception))
         ]
     describe_examples = [
-        ("<type 'exceptions.Exception'> is not a "
-         "<type 'exceptions.ValueError'>",
+        ("%r is not a %r" % (Exception, ValueError),
          error_base_foo,
          MatchesException(ValueError)),
         ]
@@ -362,7 +360,12 @@
         # Exception, it is propogated.
         match_keyb = Raises(MatchesException(KeyboardInterrupt))
         def raise_keyb_from_match():
-            matcher = Raises(MatchesException(Exception))
+            if sys.version_info > (2, 5):
+                matcher = Raises(MatchesException(Exception))
+            else:
+                # On Python 2.4 KeyboardInterrupt is a StandardError subclass
+                # but should propogate from less generic exception matchers
+                matcher = Raises(MatchesException(EnvironmentError))
             matcher.match(self.raiser)
         self.assertThat(raise_keyb_from_match, match_keyb)