ref #322 cleanup all teardown calling to only happen when setup succeeded.

don't use autouse fixtures for now because it would cause a proliferation
and overhead for the execution of every test.  Rather introduce a
node.addfinalizer(fin) to attach a finalizer to the respective node
and call it from node.setup() functions if the setup phase succeeded
(i.e. there is no setup function or it finished successfully)

CHANGELOG

@@ -2,9 +2,10 @@ Changes between 2.3.5 and 2.4.DEV
 -----------------------------------
 
 - fix issue322: tearDownClass is not run if setUpClass failed. Thanks
-  Mathieu Agopian for fixing.  The patch moves handling setUpClass
+  Mathieu Agopian for the initial fix.  Also make all of pytest/nose finalizer 
-  into a new autofixture. (XXX impl-decide if rather adding addfinalizer()
+  mimick the same generic behaviour: if a setupX exists and fails, 
-  API to node's would have a similar effect)
+  don't run teardownX.  This also introduces a new method "node.addfinalizer()"
+  helper which can only be called during the setup phase of a node.
 
 - fix issue336: autouse fixture in plugins should work again.
 

_pytest/__init__.py

@@ -1,2 +1,2 @@
 #
-__version__ = '2.4.0.dev8'
+__version__ = '2.4.0.dev10'

_pytest/main.py

@@ -41,7 +41,7 @@ def pytest_addoption(parser):
                help="run pytest in strict mode, warnings become errors.")
 
     group = parser.getgroup("collect", "collection")
-    group.addoption('--collectonly', '--collect-only', action="store_true", 
+    group.addoption('--collectonly', '--collect-only', action="store_true",
         help="only collect tests, don't execute them."),
     group.addoption('--pyargs', action="store_true",
         help="try to interpret all arguments as python packages.")
@@ -326,6 +326,14 @@ class Node(object):
     def getplugins(self):
         return self.config._getmatchingplugins(self.fspath)
 
+    def addfinalizer(self, fin):
+        """ register a function to be called when this node is finalized.
+
+        This method can only be called when this node is active
+        in a setup chain, for example during self.setup().
+        """
+        self.session._setupstate.addfinalizer(fin, self)
+
     def getparent(self, cls):
         current = self
         while current and not isinstance(current, cls):

_pytest/python.py

@@ -384,19 +384,19 @@ class Module(pytest.File, PyCollector):
                 setup_module(self.obj)
             else:
                 setup_module()
-
+        fin = getattr(self.obj, 'tearDownModule', None)
-    def teardown(self):
+        if fin is None:
-        teardown_module = xunitsetup(self.obj, 'tearDownModule')
+            fin = getattr(self.obj, 'teardown_module', None)
-        if teardown_module is None:
+        if fin is not None:
-            teardown_module = xunitsetup(self.obj, 'teardown_module')
-        if teardown_module is not None:
             #XXX: nose compat hack, move to nose plugin
             # if it takes a positional arg, its probably a py.test style one
             # so we pass the current module object
-            if inspect.getargspec(teardown_module)[0]:
+            if inspect.getargspec(fin)[0]:
-                teardown_module(self.obj)
+                finalizer = lambda: fin(self.obj)
             else:
-                teardown_module()
+                finalizer = fin
+            self.addfinalizer(finalizer)
+
 
 class Class(PyCollector):
     """ Collector for test methods. """
@@ -415,12 +415,11 @@ class Class(PyCollector):
             setup_class = getattr(setup_class, '__func__', setup_class)
             setup_class(self.obj)
 
-    def teardown(self):
+        fin_class = getattr(self.obj, 'teardown_class', None)
-        teardown_class = xunitsetup(self.obj, 'teardown_class')
+        if fin_class is not None:
-        if teardown_class is not None:
+            fin_class = getattr(fin_class, 'im_func', fin_class)
-            teardown_class = getattr(teardown_class, 'im_func', teardown_class)
+            fin_class = getattr(fin_class, '__func__', fin_class)
-            teardown_class = getattr(teardown_class, '__func__', teardown_class)
+            self.addfinalizer(lambda: fin_class(self.obj))
-            teardown_class(self.obj)
 
 class Instance(PyCollector):
     def _getobj(self):
@@ -449,23 +448,17 @@ class FunctionMixin(PyobjMixin):
         else:
             obj = self.parent.obj
         if inspect.ismethod(self.obj):
-            name = 'setup_method'
+            setup_name = 'setup_method'
+            teardown_name = 'teardown_method'
         else:
-            name = 'setup_function'
+            setup_name = 'setup_function'
-        setup_func_or_method = xunitsetup(obj, name)
+            teardown_name = 'teardown_function'
+        setup_func_or_method = xunitsetup(obj, setup_name)
         if setup_func_or_method is not None:
             setup_func_or_method(self.obj)
-
+        fin = getattr(obj, teardown_name, None)
-    def teardown(self):
+        if fin is not None:
-        """ perform teardown for this test function. """
+            self.addfinalizer(lambda: fin(self.obj))
-        if inspect.ismethod(self.obj):
-            name = 'teardown_method'
-        else:
-            name = 'teardown_function'
-        obj = self.parent.obj
-        teardown_func_or_meth = xunitsetup(obj, name)
-        if teardown_func_or_meth is not None:
-            teardown_func_or_meth(self.obj)
 
     def _prunetraceback(self, excinfo):
         if hasattr(self, '_obj') and not self.config.option.fulltrace:

_pytest/unittest.py

@@ -19,21 +19,6 @@ def is_unittest(obj):
         return False
 
 
-@pytest.fixture(scope='class', autouse=True)
-def _xunit_setUpClass(request):
-    """Add support for unittest.TestCase setUpClass and tearDownClass."""
-    if not is_unittest(request.cls):
-        return  # only support setUpClass / tearDownClass for unittest.TestCase
-    if getattr(request.cls, '__unittest_skip__', False):
-        return  # skipped
-    setup = getattr(request.cls, 'setUpClass', None)
-    teardown = getattr(request.cls, 'tearDownClass', None)
-    if setup is not None:
-        setup()
-    if teardown is not None:
-        request.addfinalizer(teardown)
-
-
 def pytest_pycollect_makeitem(collector, name, obj):
     if is_unittest(obj):
         return UnitTestCase(name, parent=collector)
@@ -42,6 +27,18 @@ def pytest_pycollect_makeitem(collector, name, obj):
 class UnitTestCase(pytest.Class):
     nofuncargs = True  # marker for fixturemanger.getfixtureinfo()
                        # to declare that our children do not support funcargs
+                       #
+    def setup(self):
+        cls = self.obj
+        if getattr(cls, '__unittest_skip__', False):
+            return  # skipped
+        setup = getattr(cls, 'setUpClass', None)
+        if setup is not None:
+            setup()
+        teardown = getattr(cls, 'tearDownClass', None)
+        if teardown is not None:
+            self.addfinalizer(teardown)
+        super(UnitTestCase, self).setup()
 
     def collect(self):
         self.session._fixturemanager.parsefactories(self, unittest=True)

doc/en/xunit_setup.txt

@@ -13,8 +13,15 @@ names).  While these setup/teardown methods are and will remain fully
 supported you may also use pytest's more powerful :ref:`fixture mechanism
 <fixture>` which leverages the concept of dependency injection, allowing
 for a more modular and more scalable approach for managing test state, 
-especially for larger projects and for functional testing.  It is safe 
+especially for larger projects and for functional testing.  You can
-to mix both fixture mechanisms.
+mix both fixture mechanisms in the same file but unittest-based
+test methods cannot receive fixture arguments.
+
+.. note::
+
+    As of pytest-2.4, teardownX functions are not called if 
+    setupX existed and failed/was skipped.  This harmonizes
+    behaviour across all major python testing tools.
 
 Module level setup/teardown
 --------------------------------------

setup.py

@@ -11,7 +11,7 @@ def main():
         name='pytest',
         description='py.test: simple powerful testing with Python',
         long_description = long_description,
-        version='2.4.0.dev8',
+        version='2.4.0.dev10',
         url='http://pytest.org',
         license='MIT license',
         platforms=['unix', 'linux', 'osx', 'cygwin', 'win32'],

testing/python/fixture.py

@@ -834,8 +834,6 @@ class TestFixtureUsages:
         l = reprec.getfailedcollections()
         assert len(l) == 1
 
-    @pytest.mark.xfail(reason="unclear if it should be supported at all, "
-                              "currently broken")
     def test_request_can_be_overridden(self, testdir):
         testdir.makepyfile("""
             import pytest

testing/test_runner_xunit.py

@@ -32,6 +32,39 @@ def test_module_and_function_setup(testdir):
     rep = reprec.matchreport("test_module")
     assert rep.passed
 
+def test_module_setup_failure_no_teardown(testdir):
+    reprec = testdir.inline_runsource("""
+        l = []
+        def setup_module(module):
+            l.append(1)
+            0/0
+
+        def test_nothing():
+            pass
+
+        def teardown_module(module):
+            l.append(2)
+    """)
+    reprec.assertoutcome(failed=1)
+    calls = reprec.getcalls("pytest_runtest_setup")
+    assert calls[0].item.module.l == [1]
+
+def test_setup_function_failure_no_teardown(testdir):
+    reprec = testdir.inline_runsource("""
+        modlevel = []
+        def setup_function(function):
+            modlevel.append(1)
+            0/0
+
+        def teardown_function(module):
+            modlevel.append(2)
+
+        def test_func():
+            pass
+    """)
+    calls = reprec.getcalls("pytest_runtest_setup")
+    assert calls[0].item.module.modlevel == [1]
+
 def test_class_setup(testdir):
     reprec = testdir.inline_runsource("""
         class TestSimpleClassSetup:
@@ -55,6 +88,23 @@ def test_class_setup(testdir):
     """)
     reprec.assertoutcome(passed=1+2+1)
 
+def test_class_setup_failure_no_teardown(testdir):
+    reprec = testdir.inline_runsource("""
+        class TestSimpleClassSetup:
+            clslevel = []
+            def setup_class(cls):
+                0/0
+
+            def teardown_class(cls):
+                cls.clslevel.append(1)
+
+            def test_classlevel(self):
+                pass
+
+        def test_cleanup():
+            assert not TestSimpleClassSetup.clslevel
+    """)
+    reprec.assertoutcome(failed=1, passed=1)
 
 def test_method_setup(testdir):
     reprec = testdir.inline_runsource("""
@@ -72,6 +122,25 @@ def test_method_setup(testdir):
     """)
     reprec.assertoutcome(passed=2)
 
+def test_method_setup_failure_no_teardown(testdir):
+    reprec = testdir.inline_runsource("""
+        class TestMethodSetup:
+            clslevel = []
+            def setup_method(self, method):
+                self.clslevel.append(1)
+                0/0
+
+            def teardown_method(self, method):
+                self.clslevel.append(2)
+
+            def test_method(self):
+                pass
+
+        def test_cleanup():
+            assert TestMethodSetup.clslevel == [1]
+    """)
+    reprec.assertoutcome(failed=1, passed=1)
+
 def test_method_generator_setup(testdir):
     reprec = testdir.inline_runsource("""
         class TestSetupTeardownOnInstance:
@@ -134,23 +203,7 @@ def test_method_setup_uses_fresh_instances(testdir):
     """)
     reprec.assertoutcome(passed=2, failed=0)
 
-def test_failing_setup_calls_teardown(testdir):
+def test_setup_that_skips_calledagain(testdir):
-    p = testdir.makepyfile("""
-        def setup_module(mod):
-            raise ValueError(42)
-        def test_function():
-            assert 0
-        def teardown_module(mod):
-            raise ValueError(43)
-    """)
-    result = testdir.runpytest(p)
-    result.stdout.fnmatch_lines([
-        "*42*",
-        "*43*",
-        "*2 error*"
-    ])
-
-def test_setup_that_skips_calledagain_and_teardown(testdir):
     p = testdir.makepyfile("""
         import pytest
         def setup_module(mod):
@@ -159,14 +212,9 @@ def test_setup_that_skips_calledagain_and_teardown(testdir):
             pass
         def test_function2():
             pass
-        def teardown_module(mod):
-            raise ValueError(43)
     """)
-    result = testdir.runpytest(p)
+    reprec = testdir.inline_run(p)
-    result.stdout.fnmatch_lines([
+    reprec.assertoutcome(skipped=2)
-        "*ValueError*43*",
-        "*2 skipped*1 error*",
-    ])
 
 def test_setup_fails_again_on_all_tests(testdir):
     p = testdir.makepyfile("""
@@ -177,14 +225,9 @@ def test_setup_fails_again_on_all_tests(testdir):
             pass
         def test_function2():
             pass
-        def teardown_module(mod):
-            raise ValueError(43)
     """)
-    result = testdir.runpytest(p)
+    reprec = testdir.inline_run(p)
-    result.stdout.fnmatch_lines([
+    reprec.assertoutcome(failed=2)
-        "*3 error*"
-    ])
-    assert "passed" not in result.stdout.str()
 
 def test_setup_funcarg_setup_when_outer_scope_fails(testdir):
     p = testdir.makepyfile("""
@@ -207,6 +250,3 @@ def test_setup_funcarg_setup_when_outer_scope_fails(testdir):
         "*2 error*"
     ])
     assert "xyz43" not in result.stdout.str()
-
-
-

testing/test_unittest.py

@@ -65,7 +65,7 @@ def test_setup(testdir):
     rep = reprec.matchreport("test_both", when="teardown")
     assert rep.failed and '42' in str(rep.longrepr)
 
-def test_unittest_style_setup_teardown(testdir):
+def test_setUpModule(testdir):
     testpath = testdir.makepyfile("""
         l = []
 
@@ -86,6 +86,23 @@ def test_unittest_style_setup_teardown(testdir):
         "*2 passed*",
     ])
 
+def test_setUpModule_failing_no_teardown(testdir):
+    testpath = testdir.makepyfile("""
+        l = []
+
+        def setUpModule():
+            0/0
+
+        def tearDownModule():
+            l.append(1)
+
+        def test_hello():
+            pass
+    """)
+    reprec = testdir.inline_run(testpath)
+    reprec.assertoutcome(passed=0, failed=1)
+    call = reprec.getcalls("pytest_runtest_setup")[0]
+    assert not call.item.module.l
 
 def test_new_instances(testdir):
     testpath = testdir.makepyfile("""
@@ -636,3 +653,4 @@ def test_no_teardown_if_setupclass_failed(testdir):
     """)
     reprec = testdir.inline_run(testpath)
     reprec.assertoutcome(passed=1, failed=1)
+