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

Proposed by Graham Binns on 2012-06-08
Status: Merged
Approved by: Graham Binns on 2012-06-08
Approved revision: 15377
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
To merge this branch: bzr merge lp:~gmb/launchpad/bug-1009712
Reviewer Review Type Date Requested Status
Abel Deuring code 2012-06-08 Approve on 2012-06-08
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.
Abel Deuring (adeuring) wrote :

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

review: Approve (code)
lp:~gmb/launchpad/bug-1009712 updated on 2012-06-08
15378. By Graham Binns on 2012-06-08

Removed unwanted test.

Preview Diff

1=== modified file 'lib/lp/services/features/__init__.py'
2--- lib/lp/services/features/__init__.py 2011-10-27 15:04:26 +0000
3+++ lib/lp/services/features/__init__.py 2012-06-08 10:09:34 +0000
4@@ -205,9 +205,16 @@
5 per_thread.features = controller
6
7
8+def uninstall_feature_controller():
9+ """Remove the current feature controller from this thread.
10+
11+ This function is used to create a pristine environment in tests.
12+ """
13+ del per_thread.features
14+
15+
16 def get_relevant_feature_controller():
17 """Get a `FeatureController` for this thread."""
18-
19 # The noncommittal name "relevant" is because this function may change to
20 # look things up from the current request or some other mechanism in
21 # future.
22
23=== modified file 'lib/lp/testing/tests/test_testing.py'
24--- lib/lp/testing/tests/test_testing.py 2012-01-01 02:58:52 +0000
25+++ lib/lp/testing/tests/test_testing.py 2012-06-08 10:09:34 +0000
26@@ -9,7 +9,10 @@
27 import tempfile
28
29 from lp.services.config import config
30-from lp.services.features import getFeatureFlag
31+from lp.services.features import (
32+ getFeatureFlag,
33+ uninstall_feature_controller,
34+ )
35 from lp.testing import (
36 feature_flags,
37 NestedTempfile,
38@@ -25,7 +28,13 @@
39 layer = DatabaseFunctionalLayer
40
41 def test_set_feature_flags_raises_if_not_available(self):
42- """set_feature_flags prevents mistakes mistakes by raising."""
43+ """set_feature_flags raises an error if there is no feature
44+ controller available for the current thread.
45+ """
46+ # Remove any existing feature controller for the sake of this
47+ # test (other tests will re-add it). This prevents weird
48+ # interactions in a parallel test environment.
49+ uninstall_feature_controller()
50 self.assertRaises(AssertionError, set_feature_flag, u'name', u'value')
51
52 def test_flags_set_within_feature_flags_context(self):