[Python-Dev] Questions on unittest behaviour

[Nick Coghlan]

Collin Winter wrote:
> [Sorry for accidentally cross-posting this to python-list]
> 
> While working on a test suite for unittest these past few weeks, I've
> run across some behaviours that, while not obviously wrong, don't
> strike me as quite right, either. Submitted for your consideration:
> 
> 1) TestCase.tearDown() is only run if TestCase.setUp() succeeded. It
> seems to me that tearDown() should always be run, regardless of any
> failures in setUp() or the test method itself.
> The case I'm considering is something like this, ie, a multi-part setUp():
> 
>> def setUp(self)
>>     lock_file(testfile) # open_socket(), connect_to_database(), etc
>>
>>     something_that_raises_an_exception()
>>
>> def tearDown(self):
>>     if file_is_locked(testfile):
>>         unlock_file(testfile)
> 
> In this pseudo-code example, the file won't be unlocked if some later
> operation in setUp() raises an exception. I propose that
> TestCase.run() be changed to always run tearDown(), even if setUp()
> raise an exception.
> 
> I'm undecided if this is a new feature (so it should go in for 2.6) or
> a bug fix; I'm leaning toward the latter.

On this point, I believe the current behaviour is correct and should be kept. 
If setUp() fails internally (such that the test step isn't going to be run) it 
needs to cleanup after itself, just like any other function. That way, the 
tearDown() method is allowed to assume that setUp() succeeded completely, 
instead of having to guard against the possibility that setUp() may have died 
partway through.

IOW, I consider the setUp() method in your example to be buggy. It should be 
written something like this:

  def setUp(self)
      lock_file(testfile) # open_socket(), connect_to_database(), etc
      try:
          something_that_may_raise_an_exception()
      except:
          unlock_file(testfile)
          raise

  def tearDown(self):
      unlock_file(testfile)

Alternatively, someone who prefers your style (with a tearDown() method that 
can handle a partially executed call to the setUp() method), can just write it as:

  def setUp(self)
      try:
          lock_file(testfile) # open_socket(), connect_to_database(), etc
          something_that_may_raise_an_exception()
      except:
          self.tearDown()
          raise

  def tearDown(self):
      if file_is_locked(testfile):
          unlock_file(testfile)

Consider the parallel to PEP 343's __enter__ and __exit__ methods - __exit__ 
is allowed to assume that it will only be called if __enter__ succeeded, 
because that is part of the semantics of the with statement.

Cheers,
Nick.

-- 
Nick Coghlan   |   ncoghlan at gmail.com   |   Brisbane, Australia
---------------------------------------------------------------
             http://www.boredomandlaziness.org