classification
Title: Add more exception related assertions to unittest
Type: enhancement Stage: needs patch
Components: Versions: Python 3.4
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Julian, aliles, eric.araujo, ezio.melotti, michael.foord, ncoghlan, pitrou, rbcollins, rhettinger, serhiy.storchaka, vila
Priority: normal Keywords:

Created on 2013-05-25 06:55 by ncoghlan, last changed 2013-11-29 16:48 by pitrou.

Messages (13)
msg189945 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2013-05-25 06:55
When creating the error handling tests for the new ipaddress module, one of the things I added was an "assertCleanError" method on the test cases [1].

This is a wrapper around assertRaisesRegex which ensures that either no exception context is set, or if there is one set, ensures that the display is suppressed by default.

While I don't think assertCleanError itself is suitable for adoption (there are two many use case specific details), I think it *would* be useful to provide two related helper assertions along the lines of the following (the failure message in the first case could likely be improved).

Firstly, ensuring that the context has been *suppressed* when it should have been:

    def assertCleanTraceback(self, exc, msg=None):
        exc_context = exc.__context__
        if exc_context is not None and not exc.__suppress_context__:
            if msg is None:
                fmt = "{} has context set: {}"
                msg = fmt.format(type(exc), type(exc_context))
            self.fail(msg)
        return exc_context

and secondly ensuring that the cause has been *set* when it should have been:

    def assertRaisedFrom(self, exc, expected_cause, expected_regex, msg=None):
        exc_cause = exc.__cause__
        # Use the guts of assertRaises to compare the actual cause
        # and the expected cause and raise an appropriate failure
        # if they don't match
        return exc_cause

Both proposed helpers return the actual context/cause so they can be used to test longer deliberate exception chaings.

[1] http://hg.python.org/cpython/file/04ca3f1515cf/Lib/test/test_ipaddress.py#l37
msg190183 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2013-05-28 09:44
I'm slightly torn. 

TestCase has a wide (waaaay too wide) API and adding methods makes it wider. The use cases for these methods are relatively niche, so my instinct would be that these don't meet the bar for inclusion on TestCase.

On the other hand, despite their brevity they're hard to get right, so having them in the standard library is helpful from that point of view.

I might be more sympathetic to having some helper mixin classes, so that we can provide esoteric-but-difficult assert methods without having to dump them all on TestCase.
msg190188 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-05-28 10:04
I'm sure not all user of the TestCase class even know about all it's existing methods.
msg190190 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2013-05-28 10:17
I like the idea of a separate mixin class like TracebackMixin or TracebackHelper.
msg190570 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2013-06-03 22:47
What about adding a "recipes" section in the docs with sample implementation of specific assertMethods?
msg190581 - (view) Author: Julian Berman (Julian) * Date: 2013-06-04 03:26
Can I throw in, and hopefully not in a way that's too out of place, that I think that unittest might want to rethink it's strategy re: assertion methods? The fact that the object that has all the assertions and the object that has the logic for running a test, and the thing that holds all the tests on it, makes things difficult. For a concrete example, subclasses of unittest.TestCase like e.g. trial.unittest.TestCase do not have easy ways of allowing pluggable assertions, by which I mean you can't show up and say "Here I have a thing with some assertions, use this". So, for trial, if we want to support unittest2 assertions (which I'd personally like), we have to put code in trial to do so.

It would seem to me that it'd be nicer to (obviously keep what we have for backwards compatibility) but rather than adding a bunch of mixins, have an API that decouples things holding assertion methods from a TestCase, and mix those in by calling a method on the TestCase rather than mixing in a bunch of assertion mixins.

(also, in a slightly lighter note, I might like an assertEqualAndAlsoNotUnequal method :D)
msg190587 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2013-06-04 04:30
+1 to what Michael said.  The current API is way too big, but these proposed methods aren't trivially easy to get right.  It would be nice to have them done once and done well.
msg190592 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2013-06-04 08:49
I like the idea of using the strategy pattern to at least decouple the assertion API from the main testcase API. I also like it better than the mixin suggested earlier.

We would obviously have to keep the existing methods for backwards compatibility, but could avoid adding new ones.

I'm not sure what such an API would look like, though :(
msg190601 - (view) Author: Julian Berman (Julian) * Date: 2013-06-04 14:46
I don't either really off the top of my head, though I've toyed with some things.

If there's support for some exploration I wouldn't mind attempting a POC externally though.
msg190602 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2013-06-04 14:48
There's always (almost) support for exploration...
msg190843 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2013-06-09 05:11
Hey, feel free to +nosy me on unittest things :). Julian pinged me on IRC about this - Jml and I would be delight to prepare a patchset for assertThat for unittest and as much of the core of Matchers as makes sense. The bare core can come in without any of the things that Michael is concerned about vis-a-vis unneeded complexity in testtools [e.g. nothing about arbitrary attachments is needed].

We can state from experience - both ours and users that have adopted it - that the decoupling assertThat brings is very effective at ending the forced-hierarchy-or-mixin mess that self-homed assertions brings. A while back we rewrote the entire core of testtools own assertions to be Matcher based :).

https://testtools.readthedocs.org/en/latest/for-test-authors.html#matchers has the documentation on the Matcher protocol and what it looks like for users.

Structurally, we decouple the raising of AssertionError from the act of determining whether a particular condition is met - this frees one from needing to have a reference to a test case to honour failureException, and from needing to subclass to share condition logic - unrelated test classes can how share condition logic by sharing a matcher definition.

It also lends itself rather naturally to higher order definitions of matchers, as matchers are now standalone objects with no required connection to a test case : you can curry and compose matchers easily.

We make matchers have a nice __str__, so that their structure can be printed out by assertThat when an error has occurred even if they have been curried or compose or defined much earlier.

So - is there some interest in this? If so, I can put together a patchset with just the core, and let you see what it looks like.
msg190849 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2013-06-09 09:44
I think something like "assertThat" could address the problem nicely. Probably best to propose it as a separate issue, then we can make this one depend on that one if we decide to go that way.
msg204739 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-11-29 16:48
Also, see issue #19645.
History
Date User Action Args
2013-11-29 16:48:46pitrousetnosy: + pitrou
messages: + msg204739
2013-11-22 20:52:03eric.araujosetnosy: + eric.araujo
2013-06-12 10:10:25vilasetnosy: + vila
2013-06-09 09:44:18ncoghlansetmessages: + msg190849
2013-06-09 05:11:18rbcollinssetnosy: + rbcollins
messages: + msg190843
2013-06-04 14:48:11michael.foordsetmessages: + msg190602
2013-06-04 14:46:09Juliansetmessages: + msg190601
2013-06-04 08:49:52ncoghlansetmessages: + msg190592
2013-06-04 04:30:40rhettingersetnosy: + rhettinger
messages: + msg190587
2013-06-04 03:26:18Juliansetnosy: + Julian
messages: + msg190581
2013-06-03 22:47:03ezio.melottisetnosy: + ezio.melotti
messages: + msg190570
2013-05-28 10:38:17alilessetnosy: + aliles
2013-05-28 10:17:04ncoghlansetmessages: + msg190190
2013-05-28 10:04:58serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg190188
2013-05-28 09:44:35michael.foordsetmessages: + msg190183
2013-05-25 06:55:59ncoghlancreate