launchpad-reviewers team mailing list archive

[Aaron Bentley <aaron@xxxxxxxxxxxxx>]

Aaron Bentley has proposed merging lp:~abentley/launchpad/test-feature-flags into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)


= Summary =
Provide test helpers for feature flags.

== Proposed fix ==
Provide a feature_flags context manager that allows feature flags to work
Provide a set_feature_flag function that allows a feature flag to be set.

== Pre-implementation notes ==
None, but these are based on the memcache tests that Robert gave as an example
of good feature flags tests.

== Implementation details ==
None

== Tests ==
bin/test -vvt TestFeatureFlags testing

== Demo and Q/A ==
None

= Launchpad lint =

The import issues are because testing imports from other modules to provide a
central location to import from.  The rest are bogus.

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/testing/__init__.py
  lib/lp/testing/tests/test_testing.py

./lib/lp/testing/__init__.py
     132: 'anonymous_logged_in' imported but unused
     132: 'with_anonymous_login' imported but unused
     132: 'is_logged_in' imported but unused
     151: 'launchpadlib_for' imported but unused
     151: 'launchpadlib_credentials_for' imported but unused
     132: 'person_logged_in' imported but unused
     151: 'oauth_access_token_for' imported but unused
     132: 'login_celebrity' imported but unused
     132: 'with_celebrity_logged_in' imported but unused
     150: 'test_tales' imported but unused
     132: 'celebrity_logged_in' imported but unused
     132: 'run_with_login' imported but unused
     132: 'with_person_logged_in' imported but unused
     132: 'login_team' imported but unused
     132: 'login_person' imported but unused
     132: 'login_as' imported but unused
     692: E301 expected 1 blank line, found 0
     875: E301 expected 1 blank line, found 0
     901: E302 expected 2 blank lines, found 1
     977: E302 expected 2 blank lines, found 1
-- 
https://code.launchpad.net/~abentley/launchpad/test-feature-flags/+merge/37061
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/test-feature-flags into lp:launchpad.

=== modified file 'lib/lp/testing/__init__.py'
--- lib/lp/testing/__init__.py	2010-09-22 18:37:57 +0000
+++ lib/lp/testing/__init__.py	2010-09-29 20:36:52 +0000
@@ -112,7 +112,10 @@
     )
 from canonical.launchpad.webapp.errorlog import ErrorReportEvent
 from canonical.launchpad.webapp.interaction import ANONYMOUS
-from canonical.launchpad.webapp.servers import WebServiceTestRequest
+from canonical.launchpad.webapp.servers import (
+    LaunchpadTestRequest,
+    WebServiceTestRequest,
+    )
 from canonical.launchpad.windmill.testing import constants
 from lp.codehosting.vfs import (
     branch_id_to_path,
@@ -120,6 +123,10 @@
     )
 from lp.registry.interfaces.packaging import IPackagingUtil
 from lp.services.osutils import override_environ
+from lp.services import features
+from lp.services.features.flags import FeatureController
+from lp.services.features.model import getFeatureStore, FeatureFlag
+from lp.services.features.webapp import ScopesFromRequest
 # Import the login helper functions here as it is a much better
 # place to import them from in tests.
 from lp.testing._login import (
@@ -222,9 +229,10 @@
 
 class StormStatementRecorder:
     """A storm tracer to count queries.
-    
-    This exposes the count and queries as lp.testing._webservice.QueryCollector
-    does permitting its use with the HasQueryCount matcher.
+
+    This exposes the count and queries as
+    lp.testing._webservice.QueryCollector does permitting its use with the
+    HasQueryCount matcher.
 
     It also meets the context manager protocol, so you can gather queries
     easily:
@@ -875,6 +883,19 @@
         zope.event.subscribers[:] = old_subscribers
 
 
+@contextmanager
+def feature_flags():
+    """Provide a context in which feature flags work."""
+    empty_request = LaunchpadTestRequest()
+    old_features = getattr(features.per_thread, 'features', None)
+    features.per_thread.features = FeatureController(
+        ScopesFromRequest(empty_request).lookup)
+    try:
+        yield
+    finally:
+        features.per_thread.features = old_features
+
+
 # XXX: This doesn't seem like a generically-useful testing function. Perhaps
 # it should go in a sub-module or something? -- jml
 def get_lsb_information():
@@ -977,6 +998,21 @@
     return contents
 
 
+def set_feature_flag(name, value, scope=u'default', priority=1):
+    """Set a feature flag to the specified value.
+
+    In order to access the flag, use the feature_flags context manager or
+    populate features.per_thread.features some other way.
+    :param name: The name of the flag.
+    :param value: The value of the flag.
+    :param scope: The scope in which the specified value applies.
+    """
+    assert getattr(features.per_thread, 'features', None) is not None
+    flag = FeatureFlag(
+        scope=scope, flag=name, value=value, priority=priority)
+    getFeatureStore().add(flag)
+
+
 def validate_mock_class(mock_class):
     """Validate method signatures in mock classes derived from real classes.
 

=== added file 'lib/lp/testing/tests/test_testing.py'
--- lib/lp/testing/tests/test_testing.py	1970-01-01 00:00:00 +0000
+++ lib/lp/testing/tests/test_testing.py	2010-09-29 20:36:52 +0000
@@ -0,0 +1,43 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+from __future__ import with_statement
+
+"""Tests for the login helpers."""
+
+__metaclass__ = type
+
+import unittest
+
+from canonical.testing.layers import DatabaseFunctionalLayer
+from lp.services.features import getFeatureFlag
+from lp.testing import (
+    feature_flags,
+    set_feature_flag,
+    TestCase,
+    )
+
+
+class TestFeatureFlags(TestCase):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_set_feature_flags_raises_if_not_available(self):
+        """set_feature_flags prevents mistakes mistakes by raising."""
+        self.assertRaises(AssertionError, set_feature_flag, u'name', u'value')
+
+    def test_flags_set_within_feature_flags_context(self):
+        """In the feature_flags context, set/get works."""
+        self.useContext(feature_flags())
+        set_feature_flag(u'name', u'value')
+        self.assertEqual('value', getFeatureFlag('name'))
+
+    def test_flags_unset_outside_feature_flags_context(self):
+        """get fails when used outside the feature_flags context."""
+        with feature_flags():
+            set_feature_flag(u'name', u'value')
+        self.assertIs(None, getFeatureFlag('name'))
+
+
+def test_suite():
+    return unittest.TestLoader().loadTestsFromName(__name__)