From 1280add0473e6a930f3152f741de5b63e438fdf8 Mon Sep 17 00:00:00 2001
From: holger krekel <holger@merlinux.eu>
Date: Wed, 17 Jul 2013 10:29:11 +0200
Subject: [PATCH] SO-17664702: call fixture finalizers even if the fixture
 function partially failed (finalizers would not always be called before)

---
 CHANGELOG                 |  3 +++
 _pytest/__init__.py       |  2 +-
 _pytest/python.py         | 22 ++++++++++++++--------
 setup.py                  |  2 +-
 testing/python/fixture.py | 33 +++++++++++++++++++++++++++++++++
 5 files changed, 52 insertions(+), 10 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index cf73bbcf3..dda33f9d5 100644
--- a/CHANGELOG
+++ b/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.
 
diff --git a/_pytest/__init__.py b/_pytest/__init__.py
index b9dd21817..5005bd8d1 100644
--- a/_pytest/__init__.py
+++ b/_pytest/__init__.py
@@ -1,2 +1,2 @@
 #
-__version__ = '2.4.0.dev6'
+__version__ = '2.4.0.dev7'
diff --git a/_pytest/python.py b/_pytest/python.py
index 6eda8b7d3..4915f5a5b 100644
--- a/_pytest/python.py
+++ b/_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
-        val = fixturedef.execute(request=self)
+        try:
+            # 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
-        # (XXX analyse exact finalizing mechanics / cleanup)
-        self.session._setupstate.addfinalizer(fixturedef.finish, self.node)
-        self._fixturemanager.addargfinalizer(fixturedef.finish, argname)
-        for subargname in fixturedef.argnames: # XXX all deps?
-            self._fixturemanager.addargfinalizer(fixturedef.finish, subargname)
+            # prepare finalization according to scope
+            # (XXX analyse exact finalizing mechanics / cleanup)
+            self.session._setupstate.addfinalizer(fixturedef.finish,
+                                                  self.node)
+            self._fixturemanager.addargfinalizer(fixturedef.finish, argname)
+            for subargname in fixturedef.argnames: # XXX all deps?
+                self._fixturemanager.addargfinalizer(fixturedef.finish,
+                                                     subargname)
         mp.undo()
         return val
 
diff --git a/setup.py b/setup.py
index f99a8f256..0f95de07d 100644
--- a/setup.py
+++ b/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'],
diff --git a/testing/python/fixture.py b/testing/python/fixture.py
index 09856edce..2cf3062dc 100644
--- a/testing/python/fixture.py
+++ b/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 = []