Merge lp:~gmb/launchpad/bug-1009712 into lp:launchpad

Proposed by Graham Binns
Status: Merged
Approved by: Graham Binns
Approved revision: no longer in the source branch.
Merged at revision: 15384
Proposed branch: lp:~gmb/launchpad/bug-1009712
Merge into: lp:launchpad
Diff against target: 52 lines (+19/-3)
2 files modified
lib/lp/services/features/__init__.py (+8/-1)
lib/lp/testing/tests/test_testing.py (+11/-2)
To merge this branch: bzr merge lp:~gmb/launchpad/bug-1009712
Reviewer Review Type Date Requested Status
Abel Deuring (community) code Approve
Review via email: mp+109315@code.launchpad.net

Commit message

test_set_feature_flags_raises_if_not_available() will no longer fail in parallel testing environments after being disabused of the assumption that no feature controller has been installed.

Description of the change

This branch fixes bug 1009712, which was caused by test_set_feature_flags_raises_if_not_available assuming that there is no feature controller when it gets run (hurrah for implied test isolation).

The fix was simple: I added a function that allows us to uninstall an existing feature controller from the thread and then called that from within the test. I also updated the test's docstring, which was pretty uninformative.

To post a comment you must log in.
Revision history for this message
Abel Deuring (adeuring) wrote :

Aside from the "test_noise" noise, this looks good.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/services/features/__init__.py'
--- lib/lp/services/features/__init__.py 2011-10-27 15:04:26 +0000
+++ lib/lp/services/features/__init__.py 2012-06-08 10:09:34 +0000
@@ -205,9 +205,16 @@
205 per_thread.features = controller205 per_thread.features = controller
206206
207207
208def uninstall_feature_controller():
209 """Remove the current feature controller from this thread.
210
211 This function is used to create a pristine environment in tests.
212 """
213 del per_thread.features
214
215
208def get_relevant_feature_controller():216def get_relevant_feature_controller():
209 """Get a `FeatureController` for this thread."""217 """Get a `FeatureController` for this thread."""
210
211 # The noncommittal name "relevant" is because this function may change to218 # The noncommittal name "relevant" is because this function may change to
212 # look things up from the current request or some other mechanism in219 # look things up from the current request or some other mechanism in
213 # future.220 # future.
214221
=== modified file 'lib/lp/testing/tests/test_testing.py'
--- lib/lp/testing/tests/test_testing.py 2012-01-01 02:58:52 +0000
+++ lib/lp/testing/tests/test_testing.py 2012-06-08 10:09:34 +0000
@@ -9,7 +9,10 @@
9import tempfile9import tempfile
1010
11from lp.services.config import config11from lp.services.config import config
12from lp.services.features import getFeatureFlag12from lp.services.features import (
13 getFeatureFlag,
14 uninstall_feature_controller,
15 )
13from lp.testing import (16from lp.testing import (
14 feature_flags,17 feature_flags,
15 NestedTempfile,18 NestedTempfile,
@@ -25,7 +28,13 @@
25 layer = DatabaseFunctionalLayer28 layer = DatabaseFunctionalLayer
2629
27 def test_set_feature_flags_raises_if_not_available(self):30 def test_set_feature_flags_raises_if_not_available(self):
28 """set_feature_flags prevents mistakes mistakes by raising."""31 """set_feature_flags raises an error if there is no feature
32 controller available for the current thread.
33 """
34 # Remove any existing feature controller for the sake of this
35 # test (other tests will re-add it). This prevents weird
36 # interactions in a parallel test environment.
37 uninstall_feature_controller()
29 self.assertRaises(AssertionError, set_feature_flag, u'name', u'value')38 self.assertRaises(AssertionError, set_feature_flag, u'name', u'value')
3039
31 def test_flags_set_within_feature_flags_context(self):40 def test_flags_set_within_feature_flags_context(self):