-
Notifications
You must be signed in to change notification settings - Fork 88
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Explicit interfaces for matchers and mismatches #211
Closed
Closed
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
# Copyright (c) 2009-2012 testtools developers. See LICENSE for details. | ||
# Copyright (c) 2009-2016 testtools developers. See LICENSE for details. | ||
|
||
"""Matchers, a way to express complex assertions outside the testcase. | ||
|
||
|
@@ -11,6 +11,7 @@ | |
""" | ||
|
||
__all__ = [ | ||
'IMatcher', | ||
'Matcher', | ||
'Mismatch', | ||
'MismatchDecorator', | ||
|
@@ -25,25 +26,33 @@ | |
) | ||
|
||
|
||
class Matcher(object): | ||
class IMatcher(object): | ||
"""A pattern matcher. | ||
|
||
A Matcher must implement match and __str__ to be used by | ||
testtools.TestCase.assertThat. Matcher.match(thing) returns None when | ||
thing is completely matched, and a Mismatch object otherwise. | ||
This describes the IMatcher protocol that any matcher must implement. | ||
|
||
A Matcher must implement ``match`` and ``__str__`` to be used by | ||
:py:method:`testtools.TestCase.assertThat`. | ||
|
||
Matchers can be useful outside of test cases, as they are simply a | ||
pattern matching language expressed as objects. | ||
pattern-matching language expressed as objects. | ||
|
||
testtools.matchers is inspired by hamcrest, but is pythonic rather than | ||
a Java transcription. | ||
:py:module:`testtools.matchers` is inspired by hamcrest, but is Pythonic | ||
rather than a Java transcription. | ||
""" | ||
|
||
def match(self, something): | ||
"""Return None if this matcher matches something, a Mismatch otherwise. | ||
"""Match against ``something``. | ||
|
||
:param something: The thing to be matched against. Type will vary based | ||
on matcher implementation. | ||
:return: ``None`` if ``something`` matched, an :py:class:`IMismatch` | ||
if it did not. | ||
:rtype: Optional[IMismatch] | ||
""" | ||
raise NotImplementedError(self.match) | ||
|
||
# XXX: We have a bug saying that this ought to be __repr__. jml agrees. | ||
def __str__(self): | ||
"""Get a sensible human representation of the matcher. | ||
|
||
|
@@ -53,7 +62,49 @@ def __str__(self): | |
raise NotImplementedError(self.__str__) | ||
|
||
|
||
class Mismatch(object): | ||
Matcher = IMatcher | ||
|
||
|
||
class IMismatch(object): | ||
"""A mismatch detected by an IMatcher. | ||
|
||
Describes the protocol that mismatches are required to implement. | ||
""" | ||
|
||
# XXX: jml would like to extend this interface to include the thing that | ||
# failed to match. | ||
|
||
def describe(self): | ||
"""Describe the mismatch. | ||
|
||
:return: Either a human-readable string or something that can be cast | ||
to a string. On Python 2, this should be either ``unicode`` or a | ||
plain ASCII ``str``, and care should be taken to escape control | ||
characters. | ||
""" | ||
raise NotImplementedError(self.describe) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure, but I think we want this to actually be a string, always. I realise its just a move. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you mean by "string"? |
||
|
||
def get_details(self): | ||
"""Get extra details about the mismatch. | ||
|
||
This allows the mismatch to provide extra information beyond the basic | ||
description, including large text or binary files, or debugging internals | ||
without having to force it to fit in the output of :py:method:`describe`. | ||
|
||
The testtools assertion :py:method:`~testtools.TestCase.assertThat` | ||
will query :py:method:`get_details` and attach all its values to the | ||
test, permitting them to be reported in whatever manner the test | ||
environment chooses. | ||
|
||
:return: a dict mapping names to Content objects. name is a string to | ||
name the detail, and the Content object is the detail to add | ||
to the result. For more information see the API to which items from | ||
this dict are passed testtools.TestCase.addDetail. | ||
""" | ||
raise NotImplementedError(self.get_details) | ||
|
||
|
||
class Mismatch(IMismatch): | ||
"""An object describing a mismatch detected by a Matcher.""" | ||
|
||
def __init__(self, description=None, details=None): | ||
|
@@ -71,33 +122,12 @@ def __init__(self, description=None, details=None): | |
self._details = details | ||
|
||
def describe(self): | ||
"""Describe the mismatch. | ||
|
||
This should be either a human-readable string or castable to a string. | ||
In particular, is should either be plain ascii or unicode on Python 2, | ||
and care should be taken to escape control characters. | ||
""" | ||
try: | ||
return self._description | ||
except AttributeError: | ||
raise NotImplementedError(self.describe) | ||
|
||
def get_details(self): | ||
"""Get extra details about the mismatch. | ||
|
||
This allows the mismatch to provide extra information beyond the basic | ||
description, including large text or binary files, or debugging internals | ||
without having to force it to fit in the output of 'describe'. | ||
|
||
The testtools assertion assertThat will query get_details and attach | ||
all its values to the test, permitting them to be reported in whatever | ||
manner the test environment chooses. | ||
|
||
:return: a dict mapping names to Content objects. name is a string to | ||
name the detail, and the Content object is the detail to add | ||
to the result. For more information see the API to which items from | ||
this dict are passed testtools.TestCase.addDetail. | ||
""" | ||
return getattr(self, '_details', {}) | ||
|
||
def __repr__(self): | ||
|
@@ -143,7 +173,7 @@ def __str__(self): | |
return self.__unicode__().encode("ascii", "backslashreplace") | ||
|
||
|
||
class MismatchDecorator(object): | ||
class MismatchDecorator(IMismatch): | ||
"""Decorate a ``Mismatch``. | ||
|
||
Forwards all messages to the original mismatch object. Probably the best | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, I'm +1 on this - I frequently find myself writing a test that fails, then adding the matchee as the third parameter to the assertion so it gets printed out, then fixing the test and removing that third parameter. Annoying. I'm not sure if that should go in the matchers themselves, or in (assert|expect)That.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, interesting. I mostly want it for composition, e.g. to help with
Not
. ISTR wanting it for https://github.com/ClusterHQ/flocker/blob/master/flocker/testtools/matchers.py but looking at it, I can't see what was essential.Whether we or not we extend the interface this way, I want second opinions on whether it should be done in this patch / this release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rbtcollins Any thoughts on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jml Just for extra clarity, does "include the thing that" mean "the matchee argument passed to match()"?
From a very quick look at the code it seems that implementations of IMatcher typically pass or could pass the matchee object to the constructor of an IMismatch, in order to have describe() take it into account.
Why do we need to pass it to describe()?
An example of the "composition" use case you allude to would be appreciated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes.
Did I propose this?
Sorry, the intervening 13 months have somehow scrubbed it from my memory. However, looking at the file linked to in my comment provides just such an example. You'll have to figure out what it does, I'm afraid.