This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: document (lack of) interaction between @expectedFailure on a test_method and setUp
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.11, Python 3.10, Python 3.9
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: michael.foord Nosy List: hpk, iritkatriel, michael.foord, miss-islington, ncoghlan, r.david.murray, rbcollins
Priority: normal Keywords: patch

Created on 2010-11-27 15:16 by michael.foord, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 23201 merged iritkatriel, 2020-11-08 20:23
PR 26044 merged miss-islington, 2021-05-11 21:48
PR 26045 merged miss-islington, 2021-05-11 21:48
Messages (25)
msg122530 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2010-11-27 15:16
Reported by Konrad Delong.

class MyTest(unittest.TestCase):
    def setUp(self):
        raise Exception
    @unittest.expectedFailure
    def testSomething(self):
        assert False, "test method failed"




This code will report error, not expected failure.
msg122735 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2010-11-28 21:37
The same is also true for tearDown and cleanUp functions.
msg122745 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2010-11-28 23:27
Note that if an error is raised in a tearDown or cleanUp then unexpected-success should not be reported either. Not very important but might as well be fixed at the same time.
msg123485 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-12-06 18:29
I would expect this code to report an error of some sort, not pass as an expected failure.  The expected failure should be in the test case *only*, not in the setup or teardown methods.  That is, I don't think this is a bug, I think it is a feature that allows one to debug one's test infrastructure.
msg123493 - (view) Author: holger krekel (hpk) Date: 2010-12-06 19:06
FWIW i tend to agree and would probably prefer setup/teardown to result in an error rather than be subsumed in an expected-to-fail marked test.  I guess if one regards setup/teardown as a place to implement pre/post-conditions than the changes suggested by Michael make more sense.  I'd like to see a more "real" use case / user than the abstract case provided here.
msg123508 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2010-12-06 23:53
(made slightly redundant by Holger's comment but I'll continue anyway)

I think the issue is that setUp / tearDown are used for two different purposes.

The first is setting up and tearing down test infrastructure - where you do want to see to errors.

The other is for asserting pre and post conditions. If these are expected to fail (for whatever reason) then it may be perfectly reasonable to mark them as expectedFailure.

The fact that it was reported as a bug, and also that Antoine has requested being able to skip in a tear down (separate issue) shows that people are doing this.

So on the one hand - a small proportion of tests are marked expectedFailure and a very small subset of those might have a test infrastructure setup error. On the other hand for people who want setUp to test pre-conditions and want expectedFailure to work here will be completely unable to do this. It seems like not having consistent behaviour for expectedFailure will be more of a problem for those who want it than having it would be for those who don't need it.

As expectedFailure is not intended to be widely used anyway I would rather have consistency. It also allows the implementation to be simplified by unifying skip / expected fail / exception handling for all of setUp / tearDown / testMethod / cleanUp.
msg123513 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-12-07 02:08
I have to say that it would never have occurred to me to assert a pre or post condition and an expected failure where I expected the pre or post condition to fail, but if you've got a real use case and it would make the code simpler, I suppose I have no serious objection.  I don't use expected failure myself.  Just make sure you document it well, since it is not a behavior I would expect when using expected failure, and I'm sure there will be others like me.
msg123538 - (view) Author: holger krekel (hpk) Date: 2010-12-07 09:15
Michael, if you have it i'd like to see the original post/concrete use case. thanks, holger
msg123598 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2010-12-08 10:26
Well, the original report is here:

http://code.google.com/p/unittest-ext/issues/detail?id=21

I copied all the details provided into this issue though. Obviously the original reporter feels that they have a genuine use case.

There is also issue 9857 where Antoine is asking for test skipping in a tearDown. His use case works just as well for wanting to mark an expected fail in a tearDown (or a clean up function). As soon as we allow skips in tearDown / cleanUp functions it seems wise to also allow them in setUps for consistency (otherwise it becomes difficult to remember which parts of the test can skip, which can be expected fail etc):

http://bugs.python.org/issue9857

Raising SkipTest when in a tearDown method is reported as an error, rather than a skipped test.
Now doing this sounds like a weird use case, but it would be actually useful when you have a worker thread, and the tearDown method collects the exception raised in that thread and raises it again. For the worker thread to be able to use skipTest(), a SkipTest exception raised in tearDown should be properly reported as a skip.
msg144258 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2011-09-18 23:34
As another data point, this question came up again in the context of issue #12958.

The new test_socket.ThreadableTest uses tearDown() to pick up and reraise any exception that occurred in the client thread. This meant that my initial attempts at flagging some expected failures (due to Mac OS X limitations) didn't work - the client half of the failure was thrown in tearDown() and reported as an error.

While I've determined how to avoid that problem in the test_socket case, the general question of whether or not we consider it legitimate to put common assertions in setUp() and tearDown(), or expect that test code explicitly cope with tearDown() failures that occur due to expected test failures still needs to be addressed.

To my mind, bugs in my test infrastructure are going to get flushed out by tests that I'm neither skipping nor marking as expected failures. If I have a test that is known to fail in a way that invalidates the standard tearDown procedure for the test infrastructure, having to special case that situation in the tearDown code seems to go against the spirit of offering the "expectedFailure" decorator in the first place.

I don't think the same reasoning holds for setUp though - there's no way for a failing test to reach back and force setUp to fail, so any errors raised there are always going to be infrastructure errors.
msg144259 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2011-09-18 23:37
I think Twisted uses the tearDown to fail tests as well. As we have two use cases perhaps we should allow expectedFailure to work with failues in tearDown? (And if we do that it should cover setUp as well for symmetry or it becomes a morass of special cases.)
msg229860 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2014-10-23 07:42
Just to note that unittest2 tip (unreleased)had michaels proposed fix, which is different to that here. I'm going to back that out before doing a release, because they should be harmonisious.
msg229861 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2014-10-23 07:54
My take on this, FWIW, is that any methods in the under-test API - setUp, tearDown, test_* and anything registered via addCleanup should all support the same protocol as much as possible, whatever it is.

That is, raising a skip in setUp should skip the test. raising a skip in tearDown should skip the test, and raising a skip from a cleanup should skip the test.

This is complicated by the case where some code is called after exceptions- teardown and cleanups. Thats fairly straight forward: errors are higher precedence than failure than skips than anything which resolved as a pass.
msg229862 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2014-10-23 08:20
While I agree with Robert's comments in general, the main question to be resolved here is the scope of the "expectedFailure" decorator.

Yes, it's applied specifically to the test execution phase when writing the code, but the question is how the following scenarios should be handled when the test is marked that way:

- setUp throws an exception
- test passes, tearDown or cleanUp throw an exception
- test fails, tearDown or cleanUp throw an exception
- test throws an exception, tearDown or cleanUp throw an exception

From my perspective, those cases represent:

- error
- unexpected success OR error
- expected failure
- expected failure

If the test itself fails as expected, then I'm OK with cleanup code also failing as a consequence.

However, I'd also be OK with the simpler model that treats the decorator as covering the whole setUp/test/cleanUp/tearDown cycle, in which case an exception from *any* of those methods would be categorised as satisfying the "expected failure" decorator.
msg229863 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2014-10-23 08:47
I'd argue for 
 - An error - its not covered by the decorator.
 - An error - the last two are not covered by the decorator.
 - An error - the last two are not covered by the decorator.
 - An error - the exception isn't a 'failure' [today - we should revisit this but its a separate issue to the code coverage aspect and the last two aren't covered by the decorator.

Perhaps I'm infected by knowing too much about the plumbing, but we can make things *super* complex (both implementation and reasoning-about-for-users) if we are too clever - and making the decorator affect multiple different methods is very much across that line IMO.

"If the test itself fails as expected, then I'm OK with cleanup code also failing as a consequence."

I'm not unless we've got a really specific reason to be OK with it - in my experience that will mask nasty things like leaked temp files which can have bad consequences - and because its masked its then hard for developers to diagnose. So its bad all around.

"However, I'd also be OK with the simpler model that treats the decorator as covering the whole setUp/test/cleanUp/tearDown cycle, in which case an exception from *any* of those methods would be categorised as satisfying the "expected failure" decorator."

So - I think a simpler still model which is that the decorator covers the decorated code is better still, as it doesn't need any explanation about how its different to regular python decorators.
msg229875 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-10-23 14:51
I agree with Robert.  I'd rather get notified of a failure in the cleanup...if the test is an *expected* failure, then its cleanup should be designed to succeed when the test fails; anything else would be a bug in the design of the test, IMO.
msg230005 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2014-10-25 14:43
If you want the simple model, there's nothing to change (except perhaps documentation), as that's the way things work today.

subtest support also makes it much easier to factor out common assertions without relying on the setUp/tearDown protocol, so I'd be OK with explicitly declaring having common assertions in those methods to be an abuse of the protocol.
msg230008 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-10-25 15:21
Oh, now I understand what the issue is.  It never even *occurred* to me that someone might put assertions in setUp or tearDown.  So, yeah, count me in on considering that an abuse of the protocol :)

(If I want to factor out common assertions, and I do it a lot, I put them in a helper method and call it explicitly.)
msg230009 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2014-10-25 15:28
Heh, yeah, I've written *many* check_* methods in my life - I just hadn't made the link between that practice, and this suggestion.

I think that switches me firmly to the "don't change the behaviour" camp.
msg230064 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2014-10-27 09:17
Assertions are not uncommon in setUp. setUp is for setting up common state shared between tests and I regularly want to assert that state creation / preconditions are correct. 

I've never been bitten by this issue (I rarely use expectedFailure), but it's worth noting the use case.
msg230087 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-10-27 18:55
Can you ever imagine the assertions in the setUp being what you would want reported as an expected failure?  I would think that setUp assertion failure would be something you would want to be always reported as a failure, even if you expect the test itself to fail.
msg230089 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2014-10-27 18:57
Maybe if the expectedFailure is applied to the whole class and it's the setUp that is unable to work. I've never seen it used that way of course (mostly because it doesn't work that way) - but I *can* imagine it.
msg230105 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2014-10-27 21:23
assertions in setUp are fine IMO. But here's the thing. WHat should this code do?

class Demo(unittest.TestCase):

    def setUp(self):
        raise Exception('hi')

    def test_normal(self):
        # this should NOT be covered by expectedFailure
        pass

    @unittest.expectedFailure
    def test_expected_fail(self):
        pass

This will fail today because the decorator doesn't affect setUp.

If we apply a patch to change this, it will fail because test_normal doesn't apply the decorator.

I can imagine with dependency injection that one could set this up and have it genuinely configured correctly: but if one is doing that I'd expect the dimension of variance to be per scenario, not per test method. So it still wouldn't make sense to me.

@nick - yes, thats exactly right, this is at most docs IMO.
msg393476 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2021-05-11 22:23
New changeset 1e4ca09d825cc8059bbf80c8137164816b84cfe7 by Miss Islington (bot) in branch '3.10':
bpo-10548: expectedFailure does not apply to fixtures (GH-23201) (#26044)
https://github.com/python/cpython/commit/1e4ca09d825cc8059bbf80c8137164816b84cfe7
msg393477 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2021-05-11 22:23
New changeset c9049cf0aa9917abfd51b27e4258c395c5f66ff4 by Miss Islington (bot) in branch '3.9':
bpo-10548: expectedFailure does not apply to fixtures (GH-23201) (#26045)
https://github.com/python/cpython/commit/c9049cf0aa9917abfd51b27e4258c395c5f66ff4
History
Date User Action Args
2022-04-11 14:57:09adminsetgithub: 54757
2021-05-11 22:24:12iritkatrielsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2021-05-11 22:23:58iritkatrielsetmessages: + msg393477
2021-05-11 22:23:48iritkatrielsetmessages: + msg393476
2021-05-11 21:52:24iritkatrielsetversions: + Python 3.11, - Python 3.8
2021-05-11 21:48:33miss-islingtonsetpull_requests: + pull_request24689
2021-05-11 21:48:29miss-islingtonsetnosy: + miss-islington
pull_requests: + pull_request24688
2020-11-08 20:24:31iritkatrielsettitle: document (lack of) interaction between @expectedException on a test_method and setUp -> document (lack of) interaction between @expectedFailure on a test_method and setUp
versions: + Python 3.8, Python 3.9, Python 3.10, - Python 2.7, Python 3.2
2020-11-08 20:23:44iritkatrielsetkeywords: + patch
nosy: + iritkatriel

pull_requests: + pull_request22101
stage: patch review
2014-11-07 10:44:15rbcollinssettitle: Error in setUp not reported as expectedFailure (unittest) -> document (lack of) interaction between @expectedException on a test_method and setUp
2014-10-27 21:23:32rbcollinssetmessages: + msg230105
2014-10-27 18:57:10michael.foordsetmessages: + msg230089
2014-10-27 18:55:35r.david.murraysetmessages: + msg230087
2014-10-27 09:17:10michael.foordsetmessages: + msg230064
2014-10-25 15:28:17ncoghlansetmessages: + msg230009
2014-10-25 15:21:22r.david.murraysetmessages: + msg230008
2014-10-25 14:43:00ncoghlansetmessages: + msg230005
2014-10-23 14:51:48r.david.murraysetmessages: + msg229875
2014-10-23 08:47:19rbcollinssetmessages: + msg229863
2014-10-23 08:20:59ncoghlansetmessages: + msg229862
2014-10-23 07:54:17rbcollinssetmessages: + msg229861
2014-10-23 07:42:09rbcollinssetnosy: + rbcollins
messages: + msg229860
2011-09-18 23:37:22michael.foordsetmessages: + msg144259
2011-09-18 23:34:59ncoghlansetnosy: + ncoghlan
messages: + msg144258
2010-12-08 10:26:16michael.foordsetmessages: + msg123598
2010-12-07 09:15:23hpksetmessages: + msg123538
2010-12-07 02:08:29r.david.murraysetmessages: + msg123513
2010-12-06 23:53:48michael.foordsetmessages: + msg123508
2010-12-06 19:06:13hpksetmessages: + msg123493
2010-12-06 18:29:27r.david.murraysetnosy: + r.david.murray
messages: + msg123485
2010-12-06 17:28:46hpksetnosy: + hpk
2010-11-28 23:27:06michael.foordsetmessages: + msg122745
2010-11-28 21:37:17michael.foordsetmessages: + msg122735
2010-11-27 15:16:36michael.foordcreate