Perhaps unittest is wrong, and tearDown should always run

I’ve been looking at unittest fixtures, and seeing how they are treated in unittest, nose, and pytest.

I started out with the assumption that unittest was correct.

Now. I don’t think it is.

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

From the unittest documentation

For tearDown(): “This method will only be called if the setUp() succeeds, regardless of the outcome of the test method.”

For tearDownClass(): “If an exception is raised during a setUpClass then the tests in the class are not run and the tearDownClass is not run.

For tearDownModule(): “If an exception is raised in a setUpModule then none of the tests in the module will be run and the tearDownModule will not be run.

OK. Clear enough. But is that really the right behavior?

The reasoning behind it, in my opinion

If we are only doing one thing in setUp(), and that thing fails, then there is nothing to clean up.

Good so far.

Where it falls apart

But what if we are doing more than one thing in setUp?

What if, in setUp, we:

  1. create a temporary directory, and create some files there to work on
  2. open a connection to a remote database
  3. some other stuff

Then, in tearDown, we:

  1. clean up the “other stuff”
  2. close the connection to the database
  3. remove the temporary directory and all of its contents

What if our connection to the database fails, and throws an exception?

In the unittest and nose implementation, the temporary directory and the “other stuff” will not be cleaned up.

I just think that’s wrong.

If you want to protect for that, currently, then you need to use addCleanup() to attach clean up functions to the tests.
These will be called, even if setUp() raises an exception.

That only works for setUp()/test()/tearDown()/cleanup().
It DOESN’T work for setUpClass().
It DOESN’T work for setUpModule().

Proposed change: ALWAYS call tearDown() regardless of the success of setUp()

This goes for tearDown(),tearDownClass(), and tearDownModule() as well.

Is it too late?

I don’t think so.

It it is too late for unittest, and nose, it’s NOT too late for pytest.
Perhaps we can just fix it there.

Feedback, please

I really want to know what others think of this.
Or does anyone even care about this corner case of unittest fixture behavior?

Cheers.

Comments

  1. masklinn says

    I don’t think you’ve thought this through.

    Consider your own example: setup is supposed to create a directory, open a connection and set up some other stuff. setup blows up and throws, you try to run teardown anyway.

    And it blows up because it tries to del an attribute which was never created in the first place.

    If your setup blows up, there is no guarantee about its partial state, and the teardown simply can’t be run. It’s the same reason why __exit__ is *not* run if __enter__ throws.

    For what you want, you need to build a stack of setup/teardown operations (similar to exitStack), but setup and teardown are not up to the task *no matter how much you fiddle with their invocation*.

  2. says

    > If your setup blows up, there is no guarantee about its partial state, and the teardown simply can’t be run

    Yes, it can. It just need to be aware of this possibly partial state and deal with it.

    • says

      Well, I agree for setUp. But it doesn’t help for setUpClass or setUpModule, since those don’t support addCleanup. At least I think they don’t.

  3. says

    Indeed, for method-scope the addCleanup should be workable but not for higher scopes.
    With pytest’s own fixture mechanism this is not a problem because you can call “request.addfinalizer()” from any scope and there isn’t even a “teardown” you could use :)

  4. says

    If you always run teardown you enter “destructor hell” of not knowing what partial state you are in and having to code extra defensively. Just catch and handle exceptions within setup.

  5. says

    In Python 3.3+, you can use contextlib.ExitStack to handle cleanup for resource management inside setupClass (using stack.pop_all() if you reach the end without incident). In 2.6+, you can get it from contextlib2 from PyPI.

    This is a lot easier than always having to write your teardown methods on the assumption that setup might have partially failed, and is also consist with the context management protocol (where __exit__ won’t be called if __enter__ throws an exception).

    The contextlib docs show an example of using ExitStack to cover part of a context manager’s __enter__ method with it’s own __exit__ cleanup method: http://docs.python.org/3/library/contextlib.html#cleaning-up-in-an-enter-implementation

    You can easily do the same thing to cover part of a complex setupClass implementation with the corresponding tearDown implementation.

Leave a Reply