Add more exception related assertions to unittest

[ncoghlan]

BPO	18054
Nosy	@rhettinger, @ncoghlan, @pitrou, @rbtcollins, @ezio-melotti, @merwok, @voidspace, @Julian, @serhiy-storchaka

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields: ```python assignee = None closed_at = None created_at = labels = ['type-feature'] title = 'Add more exception related assertions to unittest' updated_at = user = 'https://github.com/ncoghlan' ``` bugs.python.org fields: ```python activity = actor = 'pitrou' assignee = 'none' closed = False closed_date = None closer = None components = [] creation = creator = 'ncoghlan' dependencies = [] files = [] hgrepos = [] issue_num = 18054 keywords = [] message_count = 13.0 messages = ['189945', '190183', '190188', '190190', '190570', '190581', '190587', '190592', '190601', '190602', '190843', '190849', '204739'] nosy_count = 11.0 nosy_names = ['rhettinger', 'ncoghlan', 'pitrou', 'rbcollins', 'vila', 'ezio.melotti', 'eric.araujo', 'michael.foord', 'Julian', 'aliles', 'serhiy.storchaka'] pr_nums = [] priority = 'normal' resolution = None stage = 'needs patch' status = 'open' superseder = None type = 'enhancement' url = 'https://bugs.python.org/issue18054' versions = ['Python 3.4'] ```

[ncoghlan]

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

[voidspace]

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.

[serhiy-storchaka]

I'm sure not all user of the TestCase class even know about all it's existing methods.

[ncoghlan]

I like the idea of a separate mixin class like TracebackMixin or TracebackHelper.

[ezio-melotti]

What about adding a "recipes" section in the docs with sample implementation of specific assertMethods?

[8a0c69e7-f5ea-4ea3-8880-4903ed9740d7]

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)

[rhettinger]

+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.

[ncoghlan]

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 :(

[8a0c69e7-f5ea-4ea3-8880-4903ed9740d7]

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.

[voidspace]

There's always (almost) support for exploration...

[rbtcollins]

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.

[ncoghlan]

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.

[pitrou]

Also, see issue bpo-19645.