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

Teardown should always occur #917

Closed
ghostsquad opened this issue Aug 5, 2015 · 8 comments
Closed

Teardown should always occur #917

ghostsquad opened this issue Aug 5, 2015 · 8 comments

Comments

@ghostsquad
Copy link

teardown should always occur even if setup fails.

http://pythontesting.net/framework/unittest/unittest-wrong-teardown/

consider the following

class TestStuff:
    def setup(self):
        print "setup class:TestStuff"
        raise Exception

    def teardown(self):
        print "teardown class:TestStuff"

    def test_foo(self):
        print 'test_foo'

teardown never occurs.

The real-world scenario would be similar to this:
http://effbot.org/zone/python-with-statement.htm

with open("x.txt") as f:
   with open("y.txt") as f2:
       data1 = f.read()
       data2 = f2.read()

This is the correct usage if multiple resources need to be used simultaneously and cleaned up after.

Here's a real world testing scenario:

class TestStuff:
    f = None
    f2 = None

    def setup(self):
        self.f = open("x.txt")
        self.f2 = open("y.txt")

    def teardown(self):
       if self.f is not None:
           self.f.Close()
       if self.f2 is not None:
           self.f2.Close()

    def test_foo(self):
        # do something with f & f2

if x.txt exists, but y.txt does not exist, we have no chance to close self.f.

@ghostsquad ghostsquad changed the title Teardown like __del__ or __exit__ Teardown should always occur Aug 5, 2015
@RonnyPfannschmidt
Copy link
Member

my personal impression is that more fine-grained finalizer's are needed

the thing is robust setup/tear-down is practically only possible if the setup registers all finalizes and there is no fully grown tear-down (because else someone will always do tear-down wrong if given the chance)

@hpk42
Copy link
Contributor

hpk42 commented Aug 5, 2015

IIRC we reverted at some point towards the unittest/nose behaviour of only running teardown when setup succeeded. I wouldn't like to change this now again.

Note that with pytest fixtures can call addfinalizer and unittest methods can call addCleanup and those finalizing functions will be called irrespective of the outcome of setup functions.

@hpk42 hpk42 closed this as completed Aug 5, 2015
@The-Compiler
Copy link
Member

Note even the blogpost you linked says:

Update: Hmmm… changed my stance on this. … there are better ways to ensure teardown happens. But it was an interesting discussion…

@ghostsquad
Copy link
Author

Yes, I did read the author's stance change, as well as the comments about addCleanup.

Is this well documented though? I found no mention of this here:
http://pytest.org/latest/xunit_setup.html

Coming from .NET and xUnit, where I don't really have to think about those things, I can simply create a constructor and a dispose method, and the test framework correctly disposes of the class the way I would expect it to.

@ghostsquad
Copy link
Author

talking about xUnit, if pytest used __del__ properly, wouldn't that also work?
ref: http://www.python-course.eu/object_oriented_programming.php

@The-Compiler
Copy link
Member

You can never be sure __del__ is run in Python, and it's best to never use it - that page seems inaccurate.

@The-Compiler
Copy link
Member

Similar to what I said in #918, I consider setup/teardown to be there mainly for nose compatibility, so they should stay compatible with what nose/unittest does either way - as @hpk42 said, if you use pytest fixtures (for example @pytest.yield_fixture(autouse=True) you'll get a teardown which always runs.

@nicoddemus
Copy link
Member

To illustrate what @The-Compiler is saying, I just posted an example of replacing setup/teardown style by an autouse fixture in #517. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants