testtools

Merge lp:~lifeless/testtools/0.9.1 into lp:~testtools-dev/testtools/0.9

  1. 0.9.1
  2. Merge into 0.9
Proposed by Robert Collins
Status: Merged
Approved by: Jonathan Lange
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~lifeless/testtools/0.9.1
Merge into: lp:~testtools-dev/testtools/0.9
Diff against target: 193 lines (+65/-30)
5 files modified
NEWS (+10/-0)
testtools/matchers.py (+33/-15)
testtools/testcase.py (+3/-2)
testtools/tests/test_matchers.py (+9/-6)
testtools/tests/test_testtools.py (+10/-7)
To merge this branch: bzr merge lp:~lifeless/testtools/0.9.1
Reviewer Review Type Date Requested Status
Jonathan Lange Approve
Martin Pool (community) Needs Fixing
Review via email: mp+15256@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Robert Collins (lifeless) wrote :

Better API for Matcher.

Revision history for this message
Martin Pool (mbp) wrote :

You're inconsistent about MisMatch vs Mismatch. Since it's a single English word I would prefer the latter.

107 - if matcher.matches(matchee):
108 + match = matcher.match(matchee)
109 + if not match:
110 return
111 self.fail('Match failed. Matchee: "%s"\nMatcher: %s\nDifference: %s\n'
112 - % (matchee, matcher, matcher.describe_difference(matchee)))
113 + % (matchee, matcher, match.describe()))
114

I would call the variable 'mismatch' otherwise the sense looks inverted.

review: Needs Fixing
Revision history for this message
Robert Collins (lifeless) wrote :

On Thu, 2009-11-26 at 05:29 +0000, Martin Pool wrote:
> Review: Needs Fixing
> You're inconsistent about MisMatch vs Mismatch. Since it's a single English word I would prefer the latter.

I am?.... oh, DocTestMisMatch - sure, fixed.

> 107 - if matcher.matches(matchee):
> 108 + match = matcher.match(matchee)
> 109 + if not match:
> 110 return
> 111 self.fail('Match failed. Matchee: "%s"\nMatcher: %s\nDifference: %s\n'
> 112 - % (matchee, matcher, matcher.describe_difference(matchee)))
> 113 + % (matchee, matcher, match.describe()))
> 114
>
> I would call the variable 'mismatch' otherwise the sense looks inverted.

Done - thanks for the review.

-Rob

lp:~lifeless/testtools/0.9.1 updated
34. By Robert Collins

Clarity tweaks from Martin.

Revision history for this message
Jonathan Lange (jml) wrote :

Ahh, I think this API is much more sensible, and wish I'd thought of this earlier.

 * In the NEWS file you say "sideffects" to mean "side effects"
 * In Mismatch.describe's docstring, "human readable" should say "human-readable"
 * I think saying "assertIsNot" is nicer than "assertNotEqual" when contrasting with None, but whatever.

Please feel free to land the patch after fixing the first two points.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'NEWS'
--- NEWS 2009-11-21 19:24:13 +0000
+++ NEWS 2009-11-26 05:44:09 +0000
@@ -1,6 +1,16 @@
1testtools NEWS1testtools NEWS
2==============2==============
33
40.9.1
5~~~~~
6
7The new matcher API introduced in 0.9.0 had a small flaw where the matchee
8would be evaluated twice to get a description of the mismatch. This could
9lead to bugs if the act of matching caused side effects to occur in the
10matchee. While having such sideffects isn't desirable, as we could change the
11API to make it robust even in the presence of such side effects, we have done
12so now before the API has become widespread.
13
40.9.0140.9.0
5~~~~~15~~~~~
616
717
=== modified file 'testtools/matchers.py'
--- testtools/matchers.py 2009-11-20 00:46:00 +0000
+++ testtools/matchers.py 2009-11-26 05:44:09 +0000
@@ -21,8 +21,9 @@
21class Matcher:21class Matcher:
22 """A pattern matcher.22 """A pattern matcher.
2323
24 A Matcher must implement matches, __str__ and describe_difference to be24 A Matcher must implement match and __str__ to be used by
25 used by testtools.TestCase.assertThat.25 testtools.TestCase.assertThat. Matcher.match(thing) returns None when
26 thing is completely matched, and a Mismatch object otherwise.
2627
27 Matchers can be useful outside of test cases, as they are simply a 28 Matchers can be useful outside of test cases, as they are simply a
28 pattern matching language expressed as objects.29 pattern matching language expressed as objects.
@@ -31,9 +32,10 @@
31 a Java transcription.32 a Java transcription.
32 """33 """
3334
34 def matches(self, something):35 def match(self, something):
35 """Returns True if this matcher matches something, False otherwise."""36 """Return None if this matcher matches something, a Mismatch otherwise.
36 raise NotImplementedError(self.matches)37 """
38 raise NotImplementedError(self.match)
3739
38 def __str__(self):40 def __str__(self):
39 """Get a sensible human representation of the matcher.41 """Get a sensible human representation of the matcher.
@@ -43,10 +45,14 @@
43 """45 """
44 raise NotImplementedError(self.__str__)46 raise NotImplementedError(self.__str__)
4547
46 def describe_difference(self, something):48
47 """Describe why something did not match.49class Mismatch:
50 """An object describing a mismatch detected by a Matcher."""
51
52 def describe(self):
53 """Describe the mismatch.
48 54
49 This should be either human readable or castable to a string.55 This should be either a human readable string or castable to a string.
50 """56 """
51 raise NotImplementedError(self.describe_difference)57 raise NotImplementedError(self.describe_difference)
5258
@@ -80,10 +86,22 @@
80 result += '\n'86 result += '\n'
81 return result87 return result
8288
83 def matches(self, actual):89 def match(self, actual):
84 return self._checker.check_output(self.want, self._with_nl(actual),90 with_nl = self._with_nl(actual)
85 self.flags)91 if self._checker.check_output(self.want, with_nl, self.flags):
8692 return None
87 def describe_difference(self, actual):93 return DocTestMismatch(self, with_nl)
88 return self._checker.output_difference(self, self._with_nl(actual),94
89 self.flags)95 def _describe_difference(self, with_nl):
96 return self._checker.output_difference(self, with_nl, self.flags)
97
98
99class DocTestMismatch:
100 """Mismatch object for DocTestMatches."""
101
102 def __init__(self, matcher, with_nl):
103 self.matcher = matcher
104 self.with_nl = with_nl
105
106 def describe(self):
107 return self.matcher._describe_difference(self.with_nl)
90108
=== modified file 'testtools/testcase.py'
--- testtools/testcase.py 2009-11-20 00:46:00 +0000
+++ testtools/testcase.py 2009-11-26 05:44:09 +0000
@@ -191,10 +191,11 @@
191 :param matcher: An object meeting the testtools.Matcher protocol.191 :param matcher: An object meeting the testtools.Matcher protocol.
192 :raises self.failureException: When matcher does not match thing.192 :raises self.failureException: When matcher does not match thing.
193 """193 """
194 if matcher.matches(matchee):194 mismatch = matcher.match(matchee)
195 if not mismatch:
195 return196 return
196 self.fail('Match failed. Matchee: "%s"\nMatcher: %s\nDifference: %s\n'197 self.fail('Match failed. Matchee: "%s"\nMatcher: %s\nDifference: %s\n'
197 % (matchee, matcher, matcher.describe_difference(matchee)))198 % (matchee, matcher, mismatch.describe()))
198199
199 def expectFailure(self, reason, predicate, *args, **kwargs):200 def expectFailure(self, reason, predicate, *args, **kwargs):
200 """Check that a test fails in a particular way.201 """Check that a test fails in a particular way.
201202
=== modified file 'testtools/tests/test_matchers.py'
--- testtools/tests/test_matchers.py 2009-11-20 00:46:00 +0000
+++ testtools/tests/test_matchers.py 2009-11-26 05:44:09 +0000
@@ -15,14 +15,16 @@
1515
16class TestDocTestMatchesInterface(TestCase):16class TestDocTestMatchesInterface(TestCase):
1717
18 def test_matches_matches(self):18 def test_matches_match(self):
19 matcher = DocTestMatches("Ran 1 test in ...s", doctest.ELLIPSIS)19 matcher = DocTestMatches("Ran 1 test in ...s", doctest.ELLIPSIS)
20 matches = ["Ran 1 test in 0.000s", "Ran 1 test in 1.234s"]20 matches = ["Ran 1 test in 0.000s", "Ran 1 test in 1.234s"]
21 mismatches = ["Ran 1 tests in 0.000s", "Ran 2 test in 0.000s"]21 mismatches = ["Ran 1 tests in 0.000s", "Ran 2 test in 0.000s"]
22 for candidate in matches:22 for candidate in matches:
23 self.assertTrue(matcher.matches(candidate))23 self.assertEqual(None, matcher.match(candidate))
24 for candidate in mismatches:24 for candidate in mismatches:
25 self.assertFalse(matcher.matches(candidate))25 mismatch = matcher.match(candidate)
26 self.assertNotEqual(None, mismatch)
27 self.assertNotEqual(None, getattr(mismatch, 'describe', None))
2628
27 def test__str__(self):29 def test__str__(self):
28 # [(expected, object to __str__)].30 # [(expected, object to __str__)].
@@ -35,11 +37,12 @@
3537
36 def test_describe_difference(self):38 def test_describe_difference(self):
37 # [(expected, matchee, matcher), ...]39 # [(expected, matchee, matcher), ...]
38 examples = [('Expected:\n Ran 1 test in ...s\nGot:\n'40 examples = [('Expected:\n Ran 1 tests in ...s\nGot:\n'
39 ' Ran 1 test in 0.123s\n', "Ran 1 test in 0.123s",41 ' Ran 1 test in 0.123s\n', "Ran 1 test in 0.123s",
40 DocTestMatches("Ran 1 test in ...s", doctest.ELLIPSIS))]42 DocTestMatches("Ran 1 tests in ...s", doctest.ELLIPSIS))]
41 for difference, matchee, matcher in examples:43 for difference, matchee, matcher in examples:
42 self.assertEqual(difference, matcher.describe_difference(matchee))44 mismatch = matcher.match(matchee)
45 self.assertEqual(difference, mismatch.describe())
4346
4447
45class TestDocTestMatchesSpecific(TestCase):48class TestDocTestMatchesSpecific(TestCase):
4649
=== modified file 'testtools/tests/test_testtools.py'
--- testtools/tests/test_testtools.py 2009-11-19 10:56:58 +0000
+++ testtools/tests/test_testtools.py 2009-11-26 05:44:09 +0000
@@ -235,22 +235,25 @@
235235
236 def test_assertThat_matches_clean(self):236 def test_assertThat_matches_clean(self):
237 class Matcher:237 class Matcher:
238 def matches(self, foo):238 def match(self, foo):
239 return True239 return None
240 self.assertThat("foo", Matcher())240 self.assertThat("foo", Matcher())
241241
242 def test_assertThat_mismatch_raises_description(self):242 def test_assertThat_mismatch_raises_description(self):
243 calls = []243 calls = []
244 class Mismatch:
245 def __init__(self, thing):
246 self.thing = thing
247 def describe(self):
248 calls.append(('describe_diff', self.thing))
249 return "object is not a thing"
244 class Matcher:250 class Matcher:
245 def matches(self, thing):251 def match(self, thing):
246 calls.append(('match', thing))252 calls.append(('match', thing))
247 return False253 return Mismatch(thing)
248 def __str__(self):254 def __str__(self):
249 calls.append(('__str__',))255 calls.append(('__str__',))
250 return "a description"256 return "a description"
251 def describe_difference(self, thing):
252 calls.append(('describe_diff', thing))
253 return "object is not a thing"
254 class Test(TestCase):257 class Test(TestCase):
255 def test(self):258 def test(self):
256 self.assertThat("foo", Matcher())259 self.assertThat("foo", Matcher())

Subscribers

People subscribed via source and target branches

to all changes: