Merge lp:~jml/testtools/basic-mismatch into lp:~testtools-committers/testtools/trunk

[Jonathan Lange]

This branch changes the base "Mismatch" so that it takes optional description and details parameters.

I wanted to do this because for a lot of Matchers I don't really want to make my own Mismatch subclass and I shouldn't need to.

As for implementation, I've preserved the old API, but am not particularly happy with the gymnastics required to do so. I could have my arm twisted to move the changes out of the base class and into a new class like SimpleMismatch. The reason I didn't is that I think Mismatch("description") is a less surprising API for users than a second class.

Māris points out that with this change, many of the Mismatch classes in testtools.matchers could disappear, along with their nice names.

[Māris Fogels (mars)]

Not a developer, but why did you use getatter() in Mismatch.get_details()? It looks like the constructor guarantees that the object always has a valid ._details attribute.

I think the changes look good.

Maris

review: Approve

[Jonathan Lange (jml)]

On Tue, Aug 17, 2010 at 3:41 PM, Māris Fogels <email address hidden> wrote:
> Review: Approve
> Not a developer, but why did you use getatter() in Mismatch.get_details()?  It looks like the constructor guarantees that the object always has a valid ._details attribute.

Not all subclasses upcall. I could change the subclasses in
testtools.matchers, but I can't change the subclasses that other
people have made elsewhere.

[Robert Collins (lifeless)]

So the whole point of mismatch is to be lazy, to allow calculating the
difference once and showing it later; i agree that much of the time
this is simple and won't be deferred; keeping the ability to do so is
extremely important, and we shouldn't make it hard for folk that do
need custom mismatch classes.

That said, sure, I've no problem with the base class being more helpful.

Deleting stuff will be tricky, we've some users that have used some of
our mismatchers directly.

-Rob

[Jonathan Lange (jml)]

The base patch keeps the ability to defer, which I agree is important, and pretty much guaranteed by the interface.

Not planning on deleting mismatches that we have.

You OK for me to merge this?

[Robert Collins (lifeless)]

sure, you have a +1 from maris ;)

=== modified file 'NEWS'
--- NEWS	2010-08-05 10:19:28 +0000
+++ NEWS	2010-08-17 13:15:56 +0000
@@ -7,6 +7,9 @@
 Improvements
 ------------
 
+ * 'Mismatch' now takes optional description and details parameters, so
+   custom Matchers aren't compelled to make their own subclass.
+
  * jml added a built-in UTF8_TEXT ContentType to make it slightly easier to
    add details to test results.
 
@@ -16,7 +19,7 @@
  * New 'Is' matcher, which lets you assert that a thing is identical to
    another thing.
 
- * New 'LessThan' matcher which lets you assert that a thing is less than 
+ * New 'LessThan' matcher which lets you assert that a thing is less than
    another thing.
 
  * TestCase now has a 'patch()' method to make it easier to monkey-patching
 
=== modified file 'testtools/matchers.py'
--- testtools/matchers.py	2010-08-05 06:20:48 +0000
+++ testtools/matchers.py	2010-08-17 13:15:56 +0000
@@ -58,12 +58,29 @@
 class Mismatch(object):
     """An object describing a mismatch detected by a Matcher."""
 
+    def __init__(self, description=None, details=None):
+        """Construct a `Mismatch`.
+
+        :param description: A description to use.  If not provided,
+            `Mismatch.describe` must be implemented.
+        :param details: Extra details about the mismatch.  Defaults
+            to the empty dict.
+        """
+        if description:
+            self._description = description
+        if details is None:
+            details = {}
+        self._details = details
+
     def describe(self):
         """Describe the mismatch.
 
         This should be either a human-readable string or castable to a string.
         """
-        raise NotImplementedError(self.describe_difference)
+        try:
+            return self._description
+        except AttributeError:
+            raise NotImplementedError(self.describe)
 
     def get_details(self):
         """Get extra details about the mismatch.
@@ -81,7 +98,7 @@
             to the result. For more information see the API to which items from
             this dict are passed testtools.TestCase.addDetail.
         """
-        return {}
+        return getattr(self, '_details', {})
 
 
 class DocTestMatches(object):
 
=== modified file 'testtools/tests/test_matchers.py'
--- testtools/tests/test_matchers.py	2010-08-05 06:20:48 +0000
+++ testtools/tests/test_matchers.py	2010-08-17 13:15:56 +0000
@@ -16,6 +16,7 @@
     LessThan,
     MatchesAny,
     MatchesAll,
+    Mismatch,
     Not,
     NotEquals,
     )
@@ -24,6 +25,19 @@
 Matcher
 
 
+class TestMismatch(TestCase):
+
+    def test_constructor_arguments(self):
+        mismatch = Mismatch("some description", {'detail': "things"})
+        self.assertEqual("some description", mismatch.describe())
+        self.assertEqual({'detail': "things"}, mismatch.get_details())
+
+    def test_constructor_no_arguments(self):
+        mismatch = Mismatch()
+        self.assertRaises(NotImplementedError, mismatch.describe)
+        self.assertEqual({}, mismatch.get_details())
+
+
 class TestMatchersInterface(object):
 
     def test_matches_match(self):