Merge lp:~cbrandily/python-fixtures/python-fixtures into lp:python-fixtures

Proposed by Cedric Brandily
Status: Needs review
Proposed branch: lp:~cbrandily/python-fixtures/python-fixtures
Merge into: lp:python-fixtures
Diff against target: 48 lines (+19/-1)
2 files modified
fixtures/fixture.py (+1/-1)
fixtures/tests/test_fixture.py (+18/-0)
To merge this branch: bzr merge lp:~cbrandily/python-fixtures/python-fixtures
Reviewer Review Type Date Requested Status
Robert Collins Disapprove
Review via email: mp+259559@code.launchpad.net

Description of the change

Ensure cleanups are called even if setUp fails with useFixture

This change ensures that a nested fixture cleanups will be called even if its setUp fails.

Example:

  with MyFixture() as fixture1:
    fixture1.useFixture(fixture2)

fixture2.cleanUp will be called in fixture1.__exit__ even if fixture2.setUp fails.

To post a comment you must log in.
Revision history for this message
Robert Collins (lifeless) wrote :

Thanks for this patch. Unfortunately this fixes the symptom not the problem- Fixtures are really just enhanced context managers, and so setUp == __enter__ - and to fix leaks we have to do it within setUp, not from the outside.

Additionally - and I'm sorry about this - you got mislead by the somewhat confusing thing of us having a mirror of the code here: the code is actually on github at https://testing-cabal/fixtures. (LP does have a link to that but its not obvious - I hope I've made it clearer by editing the homepage a bit).

review: Disapprove

Unmerged revisions

96. By Cedric Brandily

Ensure cleanups are called even if setUp fails with useFixture

This change ensures that a nested fixture cleanups will be called even if
its setUp fails.

Example:

  with MyFixture() as fixture1:
    fixture1.useFixture(fixture2)

fixture2.cleanUp will be called in fixture1.__exit__ even if
fixture2.setUp fails.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'fixtures/fixture.py'
2--- fixtures/fixture.py 2014-09-25 03:01:50 +0000
3+++ fixtures/fixture.py 2015-05-20 00:33:22 +0000
4@@ -188,6 +188,7 @@
5 :return: The fixture, after setting it up and scheduling a cleanup for
6 it.
7 """
8+ self.addCleanup(fixture.cleanUp)
9 try:
10 fixture.setUp()
11 except:
12@@ -197,7 +198,6 @@
13 gather_details(fixture.getDetails(), self._details)
14 raise
15 else:
16- self.addCleanup(fixture.cleanUp)
17 # Calls to getDetails while this fixture is setup will return
18 # details from the child fixture.
19 self._detail_sources.append(fixture)
20
21=== modified file 'fixtures/tests/test_fixture.py'
22--- fixtures/tests/test_fixture.py 2014-09-25 03:01:50 +0000
23+++ fixtures/tests/test_fixture.py 2015-05-20 00:33:22 +0000
24@@ -119,6 +119,24 @@
25 ['setUp-outer', 'setUp-inner', 'cleanUp-inner', 'cleanUp-outer'],
26 parent.calls)
27
28+ def test_useFixture_cleanUp_with_failed_setUp(self):
29+ class FailedSetUpFixture(fixtures.Fixture):
30+ def __init__(self):
31+ self.cleanup_called = False
32+ def setUp(self):
33+ super(FailedSetUpFixture, self).setUp()
34+ raise ValueError()
35+ def cleanUp(self):
36+ self.cleanup_called = True
37+
38+ parent = LoggingFixture('-outer')
39+ nested = FailedSetUpFixture()
40+ parent.setUp()
41+ self.assertRaises(ValueError, parent.useFixture, nested)
42+ self.assertFalse(nested.cleanup_called)
43+ parent.cleanUp()
44+ self.assertTrue(nested.cleanup_called)
45+
46 @require_gather_details
47 def test_useFixture_details_captured_from_setUp(self):
48 # Details added during fixture set-up are gathered even if setUp()

Subscribers

People subscribed via source and target branches

to all changes: