Merge lp:~lifeless/testtools/matchers into lp:~testtools-committers/testtools/trunk

[Robert Collins]

Migrate all the implementations of assert* to be matcher based. Add additional matchers (and tweak existing ones [compatibly] as needed).

Also changed assertThat to take a message; we haven't done a release with verbose= yet, so I put the parameter in in the 'natural' place.

[Jonathan Lange (jml)]

Thanks for doing this.

doc/for-test-authors.rst:
 * The heading for the IsInstance matcher is "Is", should be "IsInstance".

testtools/testcase.py
 * Please delete the XXX in assertThat about taking an optional message parameter.
 * The line "raise matchee[0], matchee[1], matchee[2]" is invalid syntax in Python 3. Use testtools.compat.reraise(*matchee) instead.
 * Is there a reason that expectFailure wasn't also changed?

Generally, the new failure messages for the negative assert methods (e.g. assertNotIn) are worse than the current ones. To some extent, this would be addressed by a fix for bug 704219.

Separately from this branch, I think we want to provide an In or IsIn matcher to complement Contains.

review: Needs Fixing

[Robert Collins (lifeless)]

On Mon, Aug 15, 2011 at 11:44 PM, Jonathan Lange <email address hidden> wrote:
> Review: Needs Fixing
> Thanks for doing this.
>
> doc/for-test-authors.rst:
>  * The heading for the IsInstance matcher is "Is", should be "IsInstance".
>
> testtools/testcase.py
>  * Please delete the XXX in assertThat about taking an optional message parameter.
>  * The line "raise matchee[0], matchee[1], matchee[2]" is invalid syntax in Python 3. Use testtools.compat.reraise(*matchee) instead.

The rest seem shallow; I will get to them at some point(ETOOMUCHON),
perhaps you would like to just fix-as-landing ?

>  * Is there a reason that expectFailure wasn't also changed?

flippantly, it didn't have a TODO; more seriously, it was complex, and
the branch was big enough, and ugly enough, that I wanted to wrap it
up.

> Generally, the new failure messages for the negative assert methods (e.g. assertNotIn) are worse than the current ones. To some extent, this would be addressed by a fix for bug 704219.

What should we do about this?

> Separately from this branch, I think we want to provide an In or IsIn matcher to complement Contains.

Yes.

-Rob

[Jonathan Lange (jml)]

On Mon, Aug 15, 2011 at 1:01 PM, Robert Collins
<email address hidden> wrote:
> On Mon, Aug 15, 2011 at 11:44 PM, Jonathan Lange <email address hidden> wrote:
>> Review: Needs Fixing
>> Thanks for doing this.
>>
>> doc/for-test-authors.rst:
>>  * The heading for the IsInstance matcher is "Is", should be "IsInstance".
>>
>> testtools/testcase.py
>>  * Please delete the XXX in assertThat about taking an optional message parameter.
>>  * The line "raise matchee[0], matchee[1], matchee[2]" is invalid syntax in Python 3. Use testtools.compat.reraise(*matchee) instead.
>
> The rest seem shallow; I will get to them at some point(ETOOMUCHON),
> perhaps you would like to just fix-as-landing ?
>

Am happy to do so.

>>  * Is there a reason that expectFailure wasn't also changed?
>
> flippantly, it didn't have a TODO; more seriously, it was complex, and
> the branch was big enough, and ugly enough, that I wanted to wrap it
> up.
>

Fair enough.

>> Generally, the new failure messages for the negative assert methods (e.g. assertNotIn) are worse than the current ones. To some extent, this would be addressed by a fix for bug 704219.
>
> What should we do about this?
>

Fix the bug, later.

jml

=== modified file 'NEWS'
--- NEWS	2011-08-09 13:12:39 +0000
+++ NEWS	2011-08-14 12:18:23 +0000
@@ -34,6 +34,9 @@
   tells us to display. Old-style verbose output can be had by passing
   ``verbose=True`` to assertThat. (Jonathan Lange, #675323, #593190)
 
+* assertThat accepts a message which will be used to annotate the matcher. This
+  can be given as a third parameter or as a keyword parameter. (Robert Collins)
+
 * Automated the Launchpad part of the release process.
   (Jonathan Lange, #623486)
 
@@ -62,6 +65,9 @@
 * ``MatchesException`` now allows you to match exceptions against any matcher,
   rather than just regular expressions.  (Jonathan Lange, #791889)
 
+* ``MatchesException`` now permits a tuple of types rather than a single type
+  (when using the type matching mode).  (Robert Collins)
+
 * ``MatchesStructure.byEquality`` added to make the common case of matching
   many attributes by equality much easier.  ``MatchesStructure.byMatcher``
   added in case folk want to match by things other than equality.
@@ -75,6 +81,8 @@
   * ``AllMatch`` matches many values against a single matcher.
     (Jonathan Lange, #615108)
 
+  * ``Contains``. (Robert Collins)
+
   * ``GreaterThan``. (Christian Kampka)
 
 * New helper, ``safe_hasattr`` added. (Jonathan Lange)
 
=== modified file 'doc/for-test-authors.rst'
--- doc/for-test-authors.rst	2011-07-27 20:21:53 +0000
+++ doc/for-test-authors.rst	2011-08-14 12:18:23 +0000
@@ -312,6 +312,17 @@
       self.assertThat(foo, Is(foo))
 
 
+Is
+~~~
+
+Adapts isinstance() to use as a matcher.  For example::
+
+  def test_isinstance_example(self):
+      class MyClass:pass
+      self.assertThat(MyClass(), IsInstance(MyClass))
+      self.assertThat(MyClass(), IsInstance(MyClass, str))
+
+
 The raises helper
 ~~~~~~~~~~~~~~~~~
 
@@ -374,6 +385,16 @@
       self.assertThat('underground', EndsWith('und'))
 
 
+Contains
+~~~~~~~~
+
+This matcher checks to see if the given thing contains the thing in the
+matcher.  For example::
+
+  def test_contains_example(self):
+      self.assertThat('abc', Contains('b'))
+
+
 MatchesException
 ~~~~~~~~~~~~~~~~
 
@@ -474,6 +495,11 @@
   def test_annotate_example_2(self):
       self.assertThat("orange", PoliticallyEquals("yellow"))
 
+You can have assertThat perform the annotation for you as a convenience::
+
+  def test_annotate_example_3(self):
+      self.assertThat("orange", Equals("yellow"), "Death to the aristos!")
+
 
 AfterPreprocessing
 ~~~~~~~~~~~~~~~~~~
 
=== modified file 'testtools/matchers.py'
--- testtools/matchers.py	2011-08-08 11:14:01 +0000
+++ testtools/matchers.py	2011-08-14 12:18:23 +0000
@@ -15,11 +15,13 @@
     'AfterPreprocessing',
     'AllMatch',
     'Annotate',
+    'Contains',
     'DocTestMatches',
     'EndsWith',
     'Equals',
     'GreaterThan',
     'Is',
+    'IsInstance',
     'KeysEqual',
     'LessThan',
     'MatchesAll',
@@ -242,6 +244,21 @@
         return self.matcher._describe_difference(self.with_nl)
 
 
+class DoesNotContain(Mismatch):
+
+    def __init__(self, matchee, needle):
+        """Create a DoesNotContain Mismatch.
+
+        :param matchee: the object that did not contain needle.
+        :param needle: the needle that 'matchee' was expected to contain.
+        """
+        self.matchee = matchee
+        self.needle = needle
+
+    def describe(self):
+        return "%r not present in %r" % (self.needle, self.matchee)
+
+
 class DoesNotStartWith(Mismatch):
 
     def __init__(self, matchee, expected):
@@ -343,6 +360,42 @@
     mismatch_string = 'is not'
 
 
+class IsInstance(object):
+    """Matcher that wraps isinstance."""
+
+    def __init__(self, *types):
+        self.types = tuple(types)
+
+    def __str__(self):
+        return "%s(%s)" % (self.__class__.__name__,
+                ', '.join(type.__name__ for type in self.types))
+
+    def match(self, other):
+        if isinstance(other, self.types):
+            return None
+        return NotAnInstance(other, self.types)
+
+
+class NotAnInstance(Mismatch):
+
+    def __init__(self, matchee, types):
+        """Create a NotAnInstance Mismatch.
+
+        :param matchee: the thing which is not an instance of any of types.
+        :param types: A tuple of the types which were expected.
+        """
+        self.matchee = matchee
+        self.types = types
+
+    def describe(self):
+        if len(self.types) == 1:
+            typestr = self.types[0].__name__
+        else:
+            typestr = 'any of (%s)' % ', '.join(type.__name__ for type in
+                    self.types)
+        return "'%s' is not an instance of %s" % (self.matchee, typestr)
+
+
 class LessThan(_BinaryComparison):
     """Matches if the item is less than the matchers reference object."""
 
@@ -449,7 +502,8 @@
         :param exception: Either an exception instance or type.
             If an instance is given, the type and arguments of the exception
             are checked. If a type is given only the type of the exception is
-            checked.
+            checked. If a tuple is given, then as with isinstance, any of the
+            types in the tuple matching is sufficient to match.
         :param value_re: If 'exception' is a type, and the matchee exception
             is of the right type, then match against this.  If value_re is a
             string, then assume value_re is a regular expression and match
@@ -461,7 +515,7 @@
         if istext(value_re):
             value_re = AfterPreproccessing(str, MatchesRegex(value_re), False)
         self.value_re = value_re
-        self._is_instance = type(self.expected) not in classtypes()
+        self._is_instance = type(self.expected) not in classtypes() + (tuple,)
 
     def match(self, other):
         if type(other) != tuple:
@@ -484,6 +538,29 @@
         return "MatchesException(%s)" % repr(self.expected)
 
 
+class Contains(Matcher):
+    """Checks whether something is container in another thing."""
+
+    def __init__(self, needle):
+        """Create a Contains Matcher.
+
+        :param needle: the thing that needs to be contained by matchees.
+        """
+        self.needle = needle
+
+    def __str__(self):
+        return "Contains(%r)" % (self.needle,)
+
+    def match(self, matchee):
+        try:
+            if self.needle not in matchee:
+                return DoesNotContain(matchee, self.needle)
+        except TypeError:
+            # e.g. 1 in 2 will raise TypeError
+            return DoesNotContain(matchee, self.needle)
+        return None
+
+
 class StartsWith(Matcher):
     """Checks whether one string starts with another."""
 
@@ -613,16 +690,19 @@
         # 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())
+                mismatch = self.exception_matcher.match(exc_info)
                 if not mismatch:
+                    del exc_info
                     return
             else:
                 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 on Python 2.5+) then propogate it.
-            if isbaseexception(sys.exc_info()[1]):
+            if isbaseexception(exc_info[1]):
+                del exc_info
                 raise
             return mismatch
 
 
=== modified file 'testtools/testcase.py'
--- testtools/testcase.py	2011-07-27 19:47:22 +0000
+++ testtools/testcase.py	2011-08-14 12:18:23 +0000
@@ -27,10 +27,14 @@
 from testtools.compat import advance_iterator
 from testtools.matchers import (
     Annotate,
+    Contains,
     Equals,
+    MatchesAll,
     MatchesException,
     Is,
+    IsInstance,
     Not,
+    Raises,
     )
 from testtools.monkey import patch
 from testtools.runtest import RunTest
@@ -304,16 +308,14 @@
         :param observed: The observed value.
         :param message: An optional message to include in the error.
         """
-        matcher = Annotate.if_message(message, Equals(expected))
-        self.assertThat(observed, matcher)
+        matcher = Equals(expected)
+        self.assertThat(observed, matcher, message)
 
     failUnlessEqual = assertEquals = assertEqual
 
     def assertIn(self, needle, haystack):
         """Assert that needle is in haystack."""
-        # XXX: Re-implement with matchers.
-        if needle not in haystack:
-            self.fail('%r not in %r' % (needle, haystack))
+        self.assertThat(haystack, Contains(needle))
 
     def assertIsNone(self, observed, message=''):
         """Assert that 'observed' is equal to None.
@@ -321,8 +323,8 @@
         :param observed: The observed value.
         :param message: An optional message describing the error.
         """
-        matcher = Annotate.if_message(message, Is(None))
-        self.assertThat(observed, matcher)
+        matcher = Is(None)
+        self.assertThat(observed, matcher, message)
 
     def assertIsNotNone(self, observed, message=''):
         """Assert that 'observed' is not equal to None.
@@ -330,8 +332,8 @@
         :param observed: The observed value.
         :param message: An optional message describing the error.
         """
-        matcher = Annotate.if_message(message, Not(Is(None)))
-        self.assertThat(observed, matcher)
+        matcher = Not(Is(None))
+        self.assertThat(observed, matcher, message)
 
     def assertIs(self, expected, observed, message=''):
         """Assert that 'expected' is 'observed'.
@@ -340,33 +342,25 @@
         :param observed: The observed value.
         :param message: An optional message describing the error.
         """
-        # XXX: Re-implement with matchers.
-        if message:
-            message = ': ' + message
-        if expected is not observed:
-            self.fail('%r is not %r%s' % (expected, observed, message))
+        matcher = Is(expected)
+        self.assertThat(observed, matcher, message)
 
     def assertIsNot(self, expected, observed, message=''):
         """Assert that 'expected' is not 'observed'."""
-        # XXX: Re-implement with matchers.
-        if message:
-            message = ': ' + message
-        if expected is observed:
-            self.fail('%r is %r%s' % (expected, observed, message))
+        matcher = Not(Is(expected))
+        self.assertThat(observed, matcher, message)
 
     def assertNotIn(self, needle, haystack):
         """Assert that needle is not in haystack."""
-        # XXX: Re-implement with matchers.
-        if needle in haystack:
-            self.fail('%r in %r' % (needle, haystack))
+        matcher = Not(Contains(needle))
+        self.assertThat(haystack, matcher)
 
     def assertIsInstance(self, obj, klass, msg=None):
-        # XXX: Re-implement with matchers.
-        if msg is None:
-            msg = '%r is not an instance of %s' % (
-                obj, self._formatTypes(klass))
-        if not isinstance(obj, klass):
-            self.fail(msg)
+        if isinstance(klass, tuple):
+            matcher = IsInstance(*klass)
+        else:
+            matcher = IsInstance(klass)
+        self.assertThat(obj, matcher, msg)
 
     def assertRaises(self, excClass, callableObj, *args, **kwargs):
         """Fail unless an exception of class excClass is thrown
@@ -376,17 +370,22 @@
            deemed to have suffered an error, exactly as for an
            unexpected exception.
         """
-        # XXX: Re-implement with matchers.
-        try:
-            ret = callableObj(*args, **kwargs)
-        except excClass:
-            return sys.exc_info()[1]
-        else:
-            excName = self._formatTypes(excClass)
-            self.fail("%s not raised, %r returned instead." % (excName, ret))
+        class ReRaiseOtherTypes(object):
+            def match(self, matchee):
+                if not issubclass(matchee[0], excClass):
+                    raise matchee[0], matchee[1], matchee[2]
+        class CaptureMatchee(object):
+            def match(self, matchee):
+                self.matchee = matchee[1]
+        capture = CaptureMatchee()
+        matcher = Raises(MatchesAll(ReRaiseOtherTypes(),
+                MatchesException(excClass), capture))
+
+        self.assertThat(lambda:callableObj(*args, **kwargs), matcher)
+        return capture.matchee
     failUnlessRaises = assertRaises
 
-    def assertThat(self, matchee, matcher, verbose=False):
+    def assertThat(self, matchee, matcher, message='', verbose=False):
         """Assert that matchee is matched by matcher.
 
         :param matchee: An object to match with matcher.
@@ -395,6 +394,7 @@
         """
         # XXX: Should this take an optional 'message' parameter? Would kind of
         # make sense. The hamcrest one does.
+        matcher = Annotate.if_message(message, matcher)
         mismatch = matcher.match(matchee)
         if not mismatch:
             return
@@ -433,6 +433,7 @@
         be removed. This separation preserves the original intent of the test
         while it is in the expectFailure mode.
         """
+        # TODO: implement with matchers.
         self._add_reason(reason)
         try:
             predicate(*args, **kwargs)
 
=== modified file 'testtools/tests/test_matchers.py'
--- testtools/tests/test_matchers.py	2011-08-08 11:14:01 +0000
+++ testtools/tests/test_matchers.py	2011-08-14 12:18:23 +0000
@@ -19,6 +19,7 @@
     AllMatch,
     Annotate,
     AnnotatedMismatch,
+    Contains,
     Equals,
     DocTestMatches,
     DoesNotEndWith,
@@ -26,6 +27,7 @@
     EndsWith,
     KeysEqual,
     Is,
+    IsInstance,
     LessThan,
     GreaterThan,
     MatchesAny,
@@ -179,6 +181,26 @@
     describe_examples = [("1 is not 2", 2, Is(1))]
 
 
+class TestIsInstanceInterface(TestCase, TestMatchersInterface):
+
+    class Foo:pass
+
+    matches_matcher = IsInstance(Foo)
+    matches_matches = [Foo()]
+    matches_mismatches = [object(), 1, Foo]
+
+    str_examples = [
+            ("IsInstance(str)", IsInstance(str)),
+            ("IsInstance(str, int)", IsInstance(str, int)),
+            ]
+
+    describe_examples = [
+            ("'foo' is not an instance of int", 'foo', IsInstance(int)),
+            ("'foo' is not an instance of any of (int, type)", 'foo',
+             IsInstance(int, type)),
+            ]
+
+
 class TestLessThanInterface(TestCase, TestMatchersInterface):
 
     matches_matcher = LessThan(4)
@@ -211,6 +233,20 @@
         ]
 
 
+class TestContainsInterface(TestCase, TestMatchersInterface):
+
+    matches_matcher = Contains('foo')
+    matches_matches = ['foo', 'afoo', 'fooa']
+    matches_mismatches = ['f', 'fo', 'oo', 'faoo', 'foao']
+
+    str_examples = [
+        ("Contains(1)", Contains(1)),
+        ("Contains('foo')", Contains('foo')),
+        ]
+
+    describe_examples = [("1 not present in 2", 2, Contains(1))]
+
+
 def make_error(type, *args, **kwargs):
     try:
         raise type(*args, **kwargs)
 
=== 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-14 12:18:23 +0000
@@ -2,6 +2,7 @@
 
 """Tests for extensions to the base test library."""
 
+from doctest import ELLIPSIS
 from pprint import pformat
 import sys
 import unittest
@@ -20,6 +21,8 @@
     )
 from testtools.compat import _b
 from testtools.matchers import (
+    Annotate,
+    DocTestMatches,
     Equals,
     MatchesException,
     Raises,
@@ -244,16 +247,8 @@
         # assertRaises raises self.failureException when it's passed a
         # callable that raises no error.
         ret = ('orange', 42)
-        try:
-            self.assertRaises(RuntimeError, lambda: ret)
-        except self.failureException:
-            # We expected assertRaises to raise this exception.
-            e = sys.exc_info()[1]
-            self.assertEqual(
-                '%s not raised, %r returned instead.'
-                % (self._formatTypes(RuntimeError), ret), str(e))
-        else:
-            self.fail('Expected assertRaises to fail, but it did not.')
+        self.assertFails("<function <lambda> at ...> returned ('orange', 42)",
+            self.assertRaises, RuntimeError, lambda: ret)
 
     def test_assertRaises_fails_when_different_error_raised(self):
         # assertRaises re-raises an exception that it didn't expect.
@@ -298,15 +293,14 @@
         failure = self.assertRaises(
             self.failureException,
             self.assertRaises, expectedExceptions, lambda: None)
-        self.assertEqual(
-            '%s not raised, None returned instead.'
-            % self._formatTypes(expectedExceptions), str(failure))
+        self.assertFails('<function <lambda> at ...> returned None',
+            self.assertRaises, expectedExceptions, lambda: None)
 
     def assertFails(self, message, function, *args, **kwargs):
         """Assert that function raises a failure with the given message."""
         failure = self.assertRaises(
             self.failureException, function, *args, **kwargs)
-        self.assertEqual(message, str(failure))
+        self.assertThat(failure, DocTestMatches(message, ELLIPSIS))
 
     def test_assertIn_success(self):
         # assertIn(needle, haystack) asserts that 'needle' is in 'haystack'.
@@ -317,9 +311,9 @@
     def test_assertIn_failure(self):
         # assertIn(needle, haystack) fails the test when 'needle' is not in
         # 'haystack'.
-        self.assertFails('3 not in [0, 1, 2]', self.assertIn, 3, [0, 1, 2])
+        self.assertFails('3 not present in [0, 1, 2]', self.assertIn, 3, [0, 1, 2])
         self.assertFails(
-            '%r not in %r' % ('qux', 'foo bar baz'),
+            '%r not present in %r' % ('qux', 'foo bar baz'),
             self.assertIn, 'qux', 'foo bar baz')
 
     def test_assertNotIn_success(self):
@@ -331,9 +325,10 @@
     def test_assertNotIn_failure(self):
         # assertNotIn(needle, haystack) fails the test when 'needle' is in
         # 'haystack'.
-        self.assertFails('3 in [1, 2, 3]', self.assertNotIn, 3, [1, 2, 3])
+        self.assertFails('[1, 2, 3] matches Contains(3)', self.assertNotIn,
+            3, [1, 2, 3])
         self.assertFails(
-            '%r in %r' % ('foo', 'foo bar baz'),
+            "'foo bar baz' matches Contains('foo')",
             self.assertNotIn, 'foo', 'foo bar baz')
 
     def test_assertIsInstance(self):
@@ -367,7 +362,7 @@
             """Simple class for testing assertIsInstance."""
 
         self.assertFails(
-            '42 is not an instance of %s' % self._formatTypes(Foo),
+            "'42' is not an instance of %s" % self._formatTypes(Foo),
             self.assertIsInstance, 42, Foo)
 
     def test_assertIsInstance_failure_multiple_classes(self):
@@ -381,12 +376,13 @@
             """Another simple class for testing assertIsInstance."""
 
         self.assertFails(
-            '42 is not an instance of %s' % self._formatTypes([Foo, Bar]),
+            "'42' is not an instance of any of (%s)" % self._formatTypes([Foo, Bar]),
             self.assertIsInstance, 42, (Foo, Bar))
 
     def test_assertIsInstance_overridden_message(self):
         # assertIsInstance(obj, klass, msg) permits a custom message.
-        self.assertFails("foo", self.assertIsInstance, 42, str, "foo")
+        self.assertFails("'42' is not an instance of str: foo",
+            self.assertIsInstance, 42, str, "foo")
 
     def test_assertIs(self):
         # assertIs asserts that an object is identical to another object.
@@ -418,16 +414,17 @@
     def test_assertIsNot_fails(self):
         # assertIsNot raises assertion errors if one object is identical to
         # another.
-        self.assertFails('None is None', self.assertIsNot, None, None)
+        self.assertFails('None matches Is(None)', self.assertIsNot, None, None)
         some_list = [42]
         self.assertFails(
-            '[42] is [42]', self.assertIsNot, some_list, some_list)
+            '[42] matches Is([42])', self.assertIsNot, some_list, some_list)
 
     def test_assertIsNot_fails_with_message(self):
         # assertIsNot raises assertion errors if one object is identical to
         # another, and includes a user-supplied message if it's provided.
         self.assertFails(
-            'None is None: foo bar', self.assertIsNot, None, None, "foo bar")
+            'None matches Is(None): foo bar', self.assertIsNot, None, None,
+            "foo bar")
 
     def test_assertThat_matches_clean(self):
         class Matcher(object):
@@ -468,6 +465,12 @@
         expected = matcher.match(matchee).describe()
         self.assertFails(expected, self.assertThat, matchee, matcher)
 
+    def test_assertThat_message_is_annotated(self):
+        matchee = 'foo'
+        matcher = Equals('bar')
+        expected = Annotate('woo', matcher).match(matchee).describe()
+        self.assertFails(expected, self.assertThat, matchee, matcher, 'woo')
+
     def test_assertThat_verbose_output(self):
         matchee = 'foo'
         matcher = Equals('bar')