testtools-dev team mailing list archive

[Jonathan Lange <jml@xxxxxxxxx>]

Jonathan Lange has proposed merging lp:~jml/testtools/unprintable-assertThat-804127 into lp:testtools.

Requested reviews:
  testtools committers (testtools-committers)
Related bugs:
  Bug #804127 in testtools: "assertThat(..., verbose=True) sometimes generates unprintable AssertionErrors"
  https://bugs.launchpad.net/testtools/+bug/804127

For more details, see:
https://code.launchpad.net/~jml/testtools/unprintable-assertThat-804127/+merge/70530


This branch addresses bug 804127, and I believe fixes it.  The approach is to raise our own kind of exception, one that actually works with extended unicode information.

In addition, I've added a to_text() method to compat, that is a way of saying 'unicode' in Python 2 and 3.

Points of concern:
 * Using to_text() in test_verbose_unicode but str() in all the other tests feels wrong, but I'm not sure what's right.
 * I haven't changed or better defined the contract for describe(). Is this really a problem?
 * I was surprised that _exception_to_text could have been so broken for Python 3, but it turns out that it wasn't used at all for Python 3. This makes me wonder how we should be actually testing this bug.
-- 
https://code.launchpad.net/~jml/testtools/unprintable-assertThat-804127/+merge/70530
Your team testtools developers is subscribed to branch lp:testtools.

=== modified file 'NEWS'
--- NEWS	2011-07-29 16:18:19 +0000
+++ NEWS	2011-08-05 08:57:51 +0000
@@ -14,6 +14,12 @@
   now deprecated.  Please stop using it.
   (Jonathan Lange, #813460)
 
+* ``assertThat`` raises ``MismatchError`` instead of
+  ``TestCase.failureException``.  ``MismatchError`` is a subclass of
+  ``AssertionError``, so in most cases this change will not matter. However,
+  if ``self.failureException`` has been set to a non-default value, then
+  mismatches will become test errors rather than test failures.
+
 * ``gather_details`` takes two dicts, rather than two detailed objects.
   (Jonathan Lange, #801027)
 
@@ -30,7 +36,10 @@
 * All public matchers are now in ``testtools.matchers.__all__``.
   (Jonathan Lange, #784859)
 
-* assertThat output is much less verbose, displaying only what the mismatch
+* ``assertThat`` can actually display mismatches and matchers that contain
+  extended unicode characters. (Jonathan Lange, Martin [gz], #804127)
+
+* ``assertThat`` output is much less verbose, displaying only what the mismatch
   tells us to display. Old-style verbose output can be had by passing
   ``verbose=True`` to assertThat. (Jonathan Lange, #675323, #593190)
 

=== modified file 'testtools/compat.py'
--- testtools/compat.py	2011-07-26 23:08:51 +0000
+++ testtools/compat.py	2011-08-05 08:57:51 +0000
@@ -143,8 +143,13 @@
                 stream.newlines, stream.line_buffering)
         except AttributeError:
             pass
-    return writer(stream, "replace")    
-
+    return writer(stream, "replace")
+
+
+try:
+    to_text = unicode
+except NameError:
+    to_text = str
 
 # The default source encoding is actually "iso-8859-1" until Python 2.5 but
 # using non-ascii causes a deprecation warning in 2.4 and it's cleaner to
@@ -214,7 +219,7 @@
 def _exception_to_text(evalue):
     """Try hard to get a sensible text value out of an exception instance"""
     try:
-        return unicode(evalue)
+        return to_text(evalue)
     except KeyboardInterrupt:
         raise
     except:

=== modified file 'testtools/matchers.py'
--- testtools/matchers.py	2011-07-29 16:16:43 +0000
+++ testtools/matchers.py	2011-08-05 08:57:51 +0000
@@ -129,6 +129,33 @@
             id(self), self.__dict__)
 
 
+class MismatchError(AssertionError):
+    """Raised when a mismatch occurs."""
+
+    # This class exists to work around
+    # <https://bugs.launchpad.net/testtools/+bug/804127>.  It provides a
+    # guaranteed way of getting a readable exception, no matter what crazy
+    # characters are in the matchee, matcher or mismatch.
+
+    def __init__(self, matchee, matcher, mismatch, verbose=False):
+        # Have to use old-style upcalling for Python 2.4 and 2.5
+        # compatibility.
+        AssertionError.__init__(self)
+        self.matchee = matchee
+        self.matcher = matcher
+        self.mismatch = mismatch
+        self.verbose = verbose
+
+    def __str__(self):
+        difference = self.mismatch.describe()
+        if self.verbose:
+            return (
+                'Match failed. Matchee: "%s"\nMatcher: %s\nDifference: %s\n'
+                % (self.matchee, self.matcher, difference))
+        else:
+            return difference
+
+
 class MismatchDecorator(object):
     """Decorate a ``Mismatch``.
 

=== modified file 'testtools/testcase.py'
--- testtools/testcase.py	2011-07-27 19:47:22 +0000
+++ testtools/testcase.py	2011-08-05 08:57:51 +0000
@@ -29,6 +29,7 @@
     Annotate,
     Equals,
     MatchesException,
+    MismatchError,
     Is,
     Not,
     )
@@ -391,7 +392,7 @@
 
         :param matchee: An object to match with matcher.
         :param matcher: An object meeting the testtools.Matcher protocol.
-        :raises self.failureException: When matcher does not match thing.
+        :raises MismatchError: When matcher does not match thing.
         """
         # XXX: Should this take an optional 'message' parameter? Would kind of
         # make sense. The hamcrest one does.
@@ -406,13 +407,7 @@
                 full_name = "%s-%d" % (name, suffix)
                 suffix += 1
             self.addDetail(full_name, content)
-        if verbose:
-            message = (
-                'Match failed. Matchee: "%s"\nMatcher: %s\nDifference: %s\n'
-                % (matchee, matcher, mismatch.describe()))
-        else:
-            message = mismatch.describe()
-        self.fail(message)
+        raise MismatchError(matchee, matcher, mismatch, verbose)
 
     def defaultTestResult(self):
         return TestResult()

=== modified file 'testtools/tests/test_matchers.py'
--- testtools/tests/test_matchers.py	2011-07-29 16:16:43 +0000
+++ testtools/tests/test_matchers.py	2011-08-05 08:57:51 +0000
@@ -12,6 +12,8 @@
     )
 from testtools.compat import (
     StringIO,
+    to_text,
+    _u,
     )
 from testtools.matchers import (
     AfterPreprocessing,
@@ -36,6 +38,7 @@
     MatchesStructure,
     Mismatch,
     MismatchDecorator,
+    MismatchError,
     Not,
     NotEquals,
     Raises,
@@ -61,6 +64,62 @@
         self.assertEqual({}, mismatch.get_details())
 
 
+class TestMismatchError(TestCase):
+
+    def test_is_assertion_error(self):
+        # MismatchError is an AssertionError, so that most of the time, it
+        # looks like a test failure, rather than an error.
+        def raise_mismatch_error():
+            raise MismatchError(2, Equals(3), Equals(3).match(2))
+        self.assertRaises(AssertionError, raise_mismatch_error)
+
+    def test_default_description_is_mismatch(self):
+        mismatch = Equals(3).match(2)
+        e = MismatchError(2, Equals(3), mismatch)
+        self.assertEqual(mismatch.describe(), str(e))
+
+    def test_default_description_unicode(self):
+        matchee = _u('\xa7')
+        matcher = Equals(_u('a'))
+        mismatch = matcher.match(matchee)
+        e = MismatchError(matchee, matcher, mismatch)
+        self.assertEqual(mismatch.describe(), str(e))
+
+    def test_verbose_description(self):
+        matchee = 2
+        matcher = Equals(3)
+        mismatch = matcher.match(2)
+        e = MismatchError(matchee, matcher, mismatch, True)
+        expected = (
+            'Match failed. Matchee: "%s"\n'
+            'Matcher: %s\n'
+            'Difference: %s\n' % (
+                matchee,
+                matcher,
+                matcher.match(matchee).describe(),
+                ))
+        self.assertEqual(expected, str(e))
+
+    def test_verbose_unicode(self):
+        # When assertThat is given matchees or matchers that contain non-ASCII
+        # unicode strings, we can still provide a meaningful error.
+        matchee = _u('\xa7')
+        matcher = Equals(_u('a'))
+        mismatch = matcher.match(matchee)
+        expected = (
+            'Match failed. Matchee: "%s"\n'
+            'Matcher: %s\n'
+            'Difference: %s\n' % (
+                matchee,
+                matcher,
+                mismatch.describe(),
+                ))
+        e = MismatchError(matchee, matcher, mismatch, True)
+        # XXX: Using to_text rather than str because, on Python 2, str will
+        # raise UnicodeEncodeError.
+        self.assertEqual(expected, to_text(e))
+
+
 class TestMatchersInterface(object):
 
     def test_matches_match(self):

=== modified file 'testtools/tests/test_testcase.py'
--- testtools/tests/test_testcase.py	2011-07-26 23:48:48 +0000
+++ testtools/tests/test_testcase.py	2011-08-05 08:57:51 +0000
@@ -18,7 +18,11 @@
     skipUnless,
     testcase,
     )
-from testtools.compat import _b
+from testtools.compat import (
+    _b,
+    _exception_to_text,
+    _u,
+    )
 from testtools.matchers import (
     Equals,
     MatchesException,
@@ -482,6 +486,24 @@
         self.assertFails(
             expected, self.assertThat, matchee, matcher, verbose=True)
 
+    def test_assertThat_verbose_unicode(self):
+        # When assertThat is given matchees or matchers that contain non-ASCII
+        # unicode strings, we can still provide a meaningful error.
+        matchee = _u('\xa7')
+        matcher = Equals(_u('a'))
+        expected = (
+            'Match failed. Matchee: "%s"\n'
+            'Matcher: %s\n'
+            'Difference: %s\n' % (
+                matchee,
+                matcher,
+                matcher.match(matchee).describe(),
+                ))
+        e = self.assertRaises(
+            self.failureException, self.assertThat, matchee, matcher,
+            verbose=True)
+        self.assertEqual(expected, _exception_to_text(e))
+
     def test_assertEqual_nice_formatting(self):
         message = "These things ought not be equal."
         a = ['apple', 'banana', 'cherry']