SO-17664702: call fixture finalizers even if the fixture function

partially failed (finalizers would not always be called before)

CHANGELOG

@@ -1,6 +1,9 @@
 Changes between 2.3.5 and 2.4.DEV
 -----------------------------------
 
+- SO-17664702: call fixture finalizers even if the fixture function
+  partially failed (finalizers would not always be called before)
+
 - color the last line red or green depending if failures/errors occured
   or everything passed.  thanks Christian Theunert.
 

_pytest/__init__.py

@@ -1,2 +1,2 @@
 #
-__version__ = '2.4.0.dev6'
+__version__ = '2.4.0.dev7'

_pytest/python.py

@@ -1290,15 +1290,21 @@ class FixtureRequest(FuncargnamesCompatAttr):
         # route request.addfinalizer to fixturedef
         mp.setattr(self, "addfinalizer", fixturedef.addfinalizer)
 
-        # perform the fixture call
+        try:
-        val = fixturedef.execute(request=self)
+            # perform the fixture call
+            val = fixturedef.execute(request=self)
+        finally:
+            # if the fixture function failed it might still have
+            # registered finalizers so we can register
 
-        # prepare finalization according to scope
+            # prepare finalization according to scope
-        # (XXX analyse exact finalizing mechanics / cleanup)
+            # (XXX analyse exact finalizing mechanics / cleanup)
-        self.session._setupstate.addfinalizer(fixturedef.finish, self.node)
+            self.session._setupstate.addfinalizer(fixturedef.finish,
-        self._fixturemanager.addargfinalizer(fixturedef.finish, argname)
+                                                  self.node)
-        for subargname in fixturedef.argnames: # XXX all deps?
+            self._fixturemanager.addargfinalizer(fixturedef.finish, argname)
-            self._fixturemanager.addargfinalizer(fixturedef.finish, subargname)
+            for subargname in fixturedef.argnames: # XXX all deps?
+                self._fixturemanager.addargfinalizer(fixturedef.finish,
+                                                     subargname)
         mp.undo()
         return val
 

setup.py

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

testing/python/fixture.py

@@ -309,6 +309,39 @@ class TestRequestBasic:
         print(ss.stack)
         assert teardownlist == [1]
 
+    def test_request_addfinalizer_failing_setup(self, testdir):
+        testdir.makepyfile("""
+            import pytest
+            l = [1]
+            @pytest.fixture
+            def myfix(request):
+                request.addfinalizer(l.pop)
+                assert 0
+            def test_fix(myfix):
+                pass
+            def test_finalizer_ran():
+                assert not l
+        """)
+        reprec = testdir.inline_run("-s")
+        reprec.assertoutcome(failed=1, passed=1)
+
+    def test_request_addfinalizer_failing_setup_module(self, testdir):
+        testdir.makepyfile("""
+            import pytest
+            l = [1, 2]
+            @pytest.fixture(scope="module")
+            def myfix(request):
+                request.addfinalizer(l.pop)
+                request.addfinalizer(l.pop)
+                assert 0
+            def test_fix(myfix):
+                pass
+        """)
+        reprec = testdir.inline_run("-s")
+        mod = reprec.getcalls("pytest_runtest_setup")[0].item.module
+        assert not mod.l
+
+
     def test_request_addfinalizer_partial_setup_failure(self, testdir):
         p = testdir.makepyfile("""
             l = []