Repository navigation
document (lack of) interaction between @expectedFailure on a test_method and setUp #54757
Description
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 = 'https://github.com/voidspace'
closed_at = <Date 2021-05-11.22:24:12.596>
created_at = <Date 2010-11-27.15:16:36.990>
labels = ['type-bug', 'library', '3.9', '3.10', '3.11']
title = 'document (lack of) interaction between @expectedFailure on a test_method and setUp'
updated_at = <Date 2021-05-11.22:24:12.596>
user = 'https://github.com/voidspace'bugs.python.org fields:
activity = <Date 2021-05-11.22:24:12.596>
actor = 'iritkatriel'
assignee = 'michael.foord'
closed = True
closed_date = <Date 2021-05-11.22:24:12.596>
closer = 'iritkatriel'
components = ['Library (Lib)']
creation = <Date 2010-11-27.15:16:36.990>
creator = 'michael.foord'
dependencies = []
files = []
hgrepos = []
issue_num = 10548
keywords = ['patch']
message_count = 25.0
messages = ['122530', '122735', '122745', '123485', '123493', '123508', '123513', '123538', '123598', '144258', '144259', '229860', '229861', '229862', '229863', '229875', '230005', '230008', '230009', '230064', '230087', '230089', '230105', '393476', '393477']
nosy_count = 7.0
nosy_names = ['ncoghlan', 'rbcollins', 'r.david.murray', 'michael.foord', 'hpk', 'miss-islington', 'iritkatriel']
pr_nums = ['23201', '26044', '26045']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue10548'
versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']
Activity
voidspace commentedon Nov 27, 2010
Reported by Konrad Delong.
This code will report error, not expected failure.
voidspace commentedon Nov 28, 2010
The same is also true for tearDown and cleanUp functions.
voidspace commentedon Nov 28, 2010
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.
bitdancer commentedon Dec 6, 2010
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.
hpk commentedon Dec 6, 2010
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.
voidspace commentedon Dec 6, 2010
(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.
bitdancer commentedon Dec 7, 2010
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.
hpk commentedon Dec 7, 2010
Michael, if you have it i'd like to see the original post/concrete use case. thanks, holger
voidspace commentedon Dec 8, 2010
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 bpo-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.
ncoghlan commentedon Sep 18, 2011
As another data point, this question came up again in the context of issue bpo-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.
voidspace commentedon Sep 18, 2011
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.)
rbtcollins commentedon Oct 23, 2014
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.
7 remaining items
voidspace commentedon Oct 27, 2014
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.
bitdancer commentedon Oct 27, 2014
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.
voidspace commentedon Oct 27, 2014
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.
rbtcollins commentedon Oct 27, 2014
assertions in setUp are fine IMO. But here's the thing. WHat should this code do?
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.
[-]Error in setUp not reported as expectedFailure (unittest)[/-][+]document (lack of) interaction between @expectedException on a test_method and setUp[/+][-]document (lack of) interaction between @expectedException on a test_method and setUp[/-][+]document (lack of) interaction between @expectedFailure on a test_method and setUp[/+]iritkatriel commentedon May 11, 2021
New changeset 1e4ca09 by Miss Islington (bot) in branch '3.10':
bpo-10548: expectedFailure does not apply to fixtures (GH-23201) (bpo-26044)
1e4ca09
iritkatriel commentedon May 11, 2021
New changeset c9049cf by Miss Islington (bot) in branch '3.9':
bpo-10548: expectedFailure does not apply to fixtures (GH-23201) (bpo-26045)
c9049cf