Skip to content
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

decouple unittest assertions from the TestCase class #63844

Open
GregorySalvan mannequin opened this issue Nov 18, 2013 · 18 comments
Open

decouple unittest assertions from the TestCase class #63844

GregorySalvan mannequin opened this issue Nov 18, 2013 · 18 comments
Labels
3.9 only security fixes tests Tests in the Lib/test dir type-feature A feature request or enhancement

Comments

@GregorySalvan
Copy link
Mannequin

GregorySalvan mannequin commented Nov 18, 2013

BPO 19645
Nosy @ncoghlan, @rbtcollins, @merwok, @pakal, @bitdancer, @voidspace, @Julian, @vadmium, @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:

assignee = None
closed_at = None
created_at = <Date 2013-11-18.14:38:14.516>
labels = ['type-feature', 'tests', '3.9']
title = 'decouple unittest assertions from the TestCase class'
updated_at = <Date 2019-07-10.01:01:52.598>
user = 'https://bugs.python.org/GregorySalvan'

bugs.python.org fields:

activity = <Date 2019-07-10.01:01:52.598>
actor = 'rbcollins'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Tests']
creation = <Date 2013-11-18.14:38:14.516>
creator = 'Gregory.Salvan'
dependencies = []
files = []
hgrepos = []
issue_num = 19645
keywords = []
message_count = 18.0
messages = ['203295', '203303', '203351', '227588', '230813', '254427', '345477', '345483', '345484', '345487', '345496', '345497', '345498', '345499', '345500', '347584', '347586', '347587']
nosy_count = 10.0
nosy_names = ['ncoghlan', 'rbcollins', 'eric.araujo', 'pakal', 'r.david.murray', 'michael.foord', 'Julian', 'martin.panter', 'serhiy.storchaka', 'Gregory.Salvan']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue19645'
versions = ['Python 3.9']

@GregorySalvan
Copy link
Mannequin Author

GregorySalvan mannequin commented Nov 18, 2013

Actually unittest assertions depends on testcase class forcing us to extend it to add assertions and to use it to make assertions outside tests.

Seeing interests in rethinking the way assertions are done in unittest, this issue first intent to collect feedback in order to suggest an implementation that fit the most.

Some notes from private discussions:

  • it was briefly discussed here bpo-18054.
  • taking care of popular solutions like py.test's rich assert
    statements and the testtools matcher objects.
  • avoid unnecessary complexity, staying focused on value

My opinion:

  • must not change unittest api
  • may be put in a seperate package (splitting "unittest" in "unittest" and "assertions")
  • Open to hide assertions exceptions in optimized mode or providing a simple way to change default behaviour (log, skip... instead of throwing unhandled exception).

@GregorySalvan GregorySalvan mannequin added tests Tests in the Lib/test dir type-feature A feature request or enhancement labels Nov 18, 2013
@bitdancer
Copy link
Member

You should probably start by summarizing the assertThat proposal from bpo-18054, which suggested making a separate issue for assertThat.

@bitdancer bitdancer changed the title Improving unittest assertions decouple unittest assertions from the TestCase class Nov 18, 2013
@GregorySalvan
Copy link
Mannequin Author

GregorySalvan mannequin commented Nov 19, 2013

bpo-18054 :

  • adding assertCleanError in the ipaddress module,
  • suggesting assertCleanTraceback, assertRaisedFrom in unittest

-> usefull but TestCase has already a wide api.

A solution like Testtools assertThat with matcher protocol (https://testtools.readthedocs.org/en/latest/for-test-authors.html#matchers) would not expand too much TestCase api and permit to easily extend assertions.

@serhiy-storchaka
Copy link
Member

Here are most popular idioms which deserve special assertion methods:

assertHasAttr(obj, name) == assertTrue(hasattr(obj, name))
assertIsSubclass(type, expected) == assertTrue(issubclass(type, expected))
assertTypeIs(obj, expected) == assertIs(type(obj), expected)
assertTypedEqual(actual, expected) == assertIs(type(actual), type(expected)) and assertEqual(actual, expected) # or assertIsInstance(actual, type(expected))?
assertStartsWith(actual, prefix) == assertTrue(s.startswith(prefix))
assertEndsWith(actual, suffix) == assertTrue(s.endswith(suffix))
assertUnorderedSequenceEqual(first, second) == assertTrue(all(x in second for x in first)) and assertTrue(all(x in first for x in second)) and assertEqual(len(first), len(second))

@rbtcollins
Copy link
Member

Hi, I'm glad you're interested in this. I very much want to see a matcher/hamcrest approach rather than a library of assertions per se - because match-or-except makes layering things harder.

@vadmium
Copy link
Member

vadmium commented Nov 10, 2015

Looking at Gregory and Robert’s “matchers” link, which also covers “assertThat”, it seems to provide a library of matcher assertion objects. E.g. instead of this TestCase-coupled code:

self.assertEquals(something, "expected value")

you would write

from testtools.matchers import Equals
self.assertThat(something, Equals("expected value"))

Implementing a custom matcher (say HasAttr for Serhiy’s first use case) seems to involve creating the HasAttr class with a match() method, and maybe another HasAttrMismatch class (though maybe you could get away with just a shared generic mismatch class?).

It seems to me that if you ignore the vaguely-documented TestCase.failureException attribute, you can fairly easily decouple the existing methods from a TestCase instance:

def assert_equal(first, second, msg=None):
    # Implementation could be copied independently of any test state
    TestCase().assertEqual(first, second, msg)

The matcher scheme would be more flexible though, because you can compose matcher objects.

@rbtcollins
Copy link
Member

Sorry for the slow reply here;

There are API breaks involved in any decoupling that involves the exception raising because of the failureException attribute. Something with signalling that can be adapted by other test suites etc might have merit, but I think we are lacking a clear use case for doing this to the existing exceptions. Setting up a way for new things to be more easily used by users of other test frameworks is quite attractive; perhaps just writing them as separate functions with an adapter to failureException would be sufficient.

@rbtcollins rbtcollins added the 3.9 only security fixes label Jun 13, 2019
@pakal
Copy link
Mannequin

pakal mannequin commented Jun 13, 2019

(Redirected here from https://bugs.python.org/issue37262)

I haven't dug the assertThat() idea, but why not make, as a first step, turn assertion methods in TestCase to staticmethods/classmethods, instead of instance methods?

Since they (to my knowledge) don't need to access an instance dict, they could be turned into such instance-less methods, and thus be usable from other testing frameworks (like pytest, for those who want to use pytest fixtures and yet benefit from advanced assertions like Django's TestCase's assertions).

"failureException" and others are meant to be (sub)class attributes, so no backwards incompatible change should occur (unless someone did really weird things with manually instantiated TestCases).

@voidspace
Copy link
Contributor

Has anyone seen any real world use cases for failureException? It's a real hindrance to a whole bunch of changes sounds decoupling.

On the other hand something like assertThat could catch a custom exception from the matchers (subclass of AssertionError) and raise failureException

@pakal
Copy link
Mannequin

pakal mannequin commented Jun 13, 2019

I don't get it, why would failureException block anything ? The unittest.TestCase API must remain the same anyway, but it could become just a wrapper towards external assertions.

For example :

class TestCase:

   assertEqual = wrap(assertions.assert_equal)

Where "wrap" for example is some kind of functools.partial() injecting into external assertions a parameter "failure_exception_class". Having all these external assertions take such a parameter (defaulting to AssertionError) would be a great plus for adaptability anyway.

@pakal pakal mannequin removed the 3.9 only security fixes label Jun 13, 2019
@voidspace
Copy link
Contributor

Suppose failureException is set to TypeError on that TestCase class, how would your assertEquals signal failure to the test runner?

@voidspace
Copy link
Contributor

Hmm, it could be done by __init_subclass__ potentially.

@voidspace
Copy link
Contributor

Or even making the assert methods into custom descriptors.

@rbtcollins
Copy link
Member

Right now that attribute could be set by each test separately, or even varied within a test.

TBH I'm not sure that the attribute really should be supported; perhaps thinking about breaking the API is worth doing.

But - what are we solving for here. The OP here seems interested in using the assertion like things entirely outside of a test context.

What would a nice clean API for that be? (Yes I like matchers, but put that aside - if the APIs aren't close enough, lets make sure we do a good job for each audience rather than a compromise..)

@pakal
Copy link
Mannequin

pakal mannequin commented Jun 13, 2019

"Suppose failureException is set to TypeError on that TestCase class, how would your assertEquals signal failure to the test runner?"

failureException is an artefact from unittest.TestCase. It's only supposed to be used in a TestCase context, with an unittest-compatible runner. If people corrupt it, I guess it's their problem?

The point of decoupling is imho that other test runner might use the separate set of assertions. These assertions should raise a sensible default (i.e AssertionError) when encountering troubles, and accepting an alternate class as parameter will allow each test framework to customize the way these assertions behave for it.

@bitdancer
Copy link
Member

"But - what are we solving for here?" I'll tell you what my fairly common use case is. Suppose I have some test infrastructure code, and I want to make some assertions in it. What I invariably end up doing is passing 'self' into the infrastructure method/class just so I can call the assert methods from it. I'd much rather be just calling the assertions, without carrying the whole test object around. It *works* to do that, but it bothers me every time I do it or read it in code, and it makes the infrastructure code needlessly more complicated and slightly harder to understand/read.

@rbtcollins
Copy link
Member

Ok so design wise - there is state on the TestCase that influences assertions; in potentially two ways.

The first way is formatting - the amount of detail shown in long list comparisons etc.

The second way I don't think we have at the moment, but potentially it could influence the fidelity of comparisons for NearlyEquals and the like - generally though we tend to pass those in as parameters.

So just ripping everything off into standalone functions loses the home for that state. It either becomes global (ugh), or a new object that isn't a test case but is basically the same magic object that has to be known is carried around, or we find some other way of delegating the formatting choice and controls.

hamcrest has some prior art in this space, and testtools experimented with that too. wordpress has managed to naff up the formatting on my old blog post about this https://rbtcollins.wordpress.com/2010/05/10/maintainable-pyunit-test-suites/ and https://testtools.readthedocs.io/en/latest/for-test-authors.html#matchers

Its been on my TODO for a very long time to put together a PEP for adding matchers to the stdlib; I find the full system we did in testtools very useful because it can represent everything from a trivial in-memory string error through to a disk image for a broken postgresql database, without running out of memory or generating mojibake.... but if we wanted to do something smaller that didn't prejuidice extensions like testtools still doing more that would be fine too.

The core idea of matchers is that rather than a standalone function f() -> nil/raise, you build a callable object f() -> Option(Mismatch), and a Mismatch can be shown to users, combined with other mismatches to form composites or sequences and so forth. So this would give room for the state around object formatting and the like too.

@rbtcollins rbtcollins added the 3.9 only security fixes label Jul 10, 2019
@rbtcollins
Copy link
Member

Oh, I didn't mean to imply that these are the only options I'd support - just that these are the things I've thought through and that I think will all work well... I'm sure there are more options available ;)

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.9 only security fixes tests Tests in the Lib/test dir type-feature A feature request or enhancement
Projects
Status: No status
Development

No branches or pull requests

5 participants