launchpad-reviewers team mailing list archive

[Māris Fogels <mfogels@xxxxxxxxx>]

Māris Fogels has proposed merging lp:~mars/launchpad/feature-flag-fixture into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #645768 need feature flag test fixture
  https://bugs.launchpad.net/bugs/645768


Hi,

This branch adds a new test fixture for feature flags.  I converted the memcache
doctest test in order to demonstrate running the fixture inside a doctest.

To run the test suite: bin/test -cv lp.services.features

Māris

-- 
Māris Fogels -- https://launchpad.net/~mars
Launchpad.net -- cross-project collaboration and hosting
-- 
https://code.launchpad.net/~mars/launchpad/feature-flag-fixture/+merge/38969
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~mars/launchpad/feature-flag-fixture into lp:launchpad/devel.

=== modified file 'lib/lp/services/features/rulesource.py'
--- lib/lp/services/features/rulesource.py	2010-09-29 07:13:11 +0000
+++ lib/lp/services/features/rulesource.py	2010-10-20 19:06:13 +0000
@@ -12,6 +12,7 @@
 __metaclass__ = type
 
 import re
+from collections import namedtuple
 
 from storm.locals import Desc
 
@@ -21,14 +22,18 @@
     )
 
 
+# A convenient mapping for a feature flag rule in the database.
+Rule = namedtuple("Rule", "flag scope priority value")
+
+
 class FeatureRuleSource(object):
     """Access feature rule sources from the database or elsewhere."""
 
     def getAllRulesAsDict(self):
         """Return all rule definitions.
 
-        :returns: dict from flag name to a list of 
-            (scope, priority, value) 
+        :returns: dict from flag name to a list of
+            (scope, priority, value)
             in descending order by priority.
         """
         d = {}
@@ -67,7 +72,7 @@
         """Return a list of tuples for the parsed form of the text input.
 
         For each non-blank line gives back a tuple of (flag, scope, priority, value).
-        
+
         Returns a list rather than a generator so that you see any syntax
         errors immediately.
         """
@@ -90,7 +95,7 @@
                 .find(FeatureFlag)
                 .order_by(FeatureFlag.flag, Desc(FeatureFlag.priority)))
         for r in rs:
-            yield str(r.flag), str(r.scope), r.priority, r.value
+            yield Rule(str(r.flag), str(r.scope), r.priority, r.value)
 
     def setAllRules(self, new_rules):
         """Replace all existing rules with a new set.

=== added file 'lib/lp/services/features/testing.py'
--- lib/lp/services/features/testing.py	1970-01-01 00:00:00 +0000
+++ lib/lp/services/features/testing.py	2010-10-20 19:06:13 +0000
@@ -0,0 +1,69 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Helpers for writing tests that use feature flags."""
+
+__metaclass__ = type
+__all__ = ['active_features']
+
+
+from fixtures import Fixture
+from lp.services.features import per_thread
+from lp.services.features.flags import FeatureController
+from lp.services.features.rulesource import Rule, StormFeatureRuleSource
+
+
+class FeatureFixture(Fixture):
+    """A fixture that sets a feature.
+
+    The fixture takes a dictonary as its constructor argument. The keys of the
+    dictionary are features to be set.
+
+    Call the fixture's `setUp()' method to install the features with the
+    desired values.  Calling `cleanUp()' will restore the original values.
+    You can also install this fixture by inheriting from
+    `fixtures.TestWithFixtures' and then calling the TestCase's
+    `self.useFixture()' method.
+
+    The fixture can also be used as a context manager. The value of the
+    feature within the context block is set to the dictonary's key's value.
+    The values are restored when the block exits.
+    """
+
+    def __init__(self, features_dict):
+        """Constructor.
+
+        :param features_dict: A dictionary-like object with keys and values
+            that are flag names and those flags' settings.
+        """
+        self.desired_features = features_dict
+
+    def setUp(self):
+        """Set the feature flags that this fixture is responsible for."""
+        super(FeatureFixture, self).setUp()
+
+        rule_source = StormFeatureRuleSource()
+        self.addCleanup(
+            rule_source.setAllRules, rule_source.getAllRulesAsTuples())
+
+        # Create a list of the new rules. Note that rules with a None
+        # value are quietly dropped, since you can't assign None as a
+        # feature flag value (it would come out as u'None') and setting
+        # a flag to None essentially means turning it off anyway.
+        new_rules = [
+            Rule(
+                flag=flag_name,
+                scope='default',
+                priority=999,
+                value=unicode(value))
+            for flag_name, value in self.desired_features.iteritems()
+                if value is not None]
+
+        rule_source.setAllRules(new_rules)
+
+
+        original_controller = getattr(per_thread, 'features', None)
+        controller = FeatureController(lambda _: True, rule_source)
+        per_thread.features = controller
+        self.addCleanup(setattr, per_thread, 'features', original_controller)
+

=== added file 'lib/lp/services/features/tests/test_helpers.py'
--- lib/lp/services/features/tests/test_helpers.py	1970-01-01 00:00:00 +0000
+++ lib/lp/services/features/tests/test_helpers.py	2010-10-20 19:06:13 +0000
@@ -0,0 +1,71 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for the feature flags test helpers."""
+
+from __future__ import with_statement
+
+
+__metaclass__ = type
+__all__ = []
+
+from canonical.testing import layers
+from lp.testing import TestCase
+from lp.services.features import getFeatureFlag
+from lp.services.features.testing import FeatureFixture
+
+
+class TestFeaturesContextManager(TestCase):
+    """Tests for the feature flags context manager test helper."""
+
+    layer = layers.DatabaseFunctionalLayer
+
+    def test_setting_one_flag_with_manager(self):
+        flag = self.getUniqueString()
+        value_outside_manager = getFeatureFlag(flag)
+        value_in_manager = None
+
+        with FeatureFixture({flag: u'on'}):
+            value_in_manager = getFeatureFlag(flag)
+
+        self.assertEqual(value_in_manager, u'on')
+        self.assertEqual(value_outside_manager, getFeatureFlag(flag))
+        self.assertNotEqual(value_outside_manager, value_in_manager)
+
+
+class TestFeaturesFixture(TestCase):
+    """Tests for the feature flags test fixture."""
+
+    layer = layers.DatabaseFunctionalLayer
+
+    def test_fixture_sets_one_flag_and_cleans_up_again(self):
+        flag = self.getUniqueString()
+        value_before_fixture_setup = getFeatureFlag(flag)
+        value_after_fixture_setup = None
+
+        fixture = FeatureFixture({flag: 'on'})
+        fixture.setUp()
+        value_after_fixture_setup = getFeatureFlag(flag)
+        fixture.cleanUp()
+
+        self.assertEqual(value_after_fixture_setup, 'on')
+        self.assertEqual(value_before_fixture_setup, getFeatureFlag(flag))
+        self.assertNotEqual(
+            value_before_fixture_setup, value_after_fixture_setup)
+
+    def test_fixture_deletes_existing_values(self):
+        self.useFixture(FeatureFixture({'one': '1'}))
+        self.useFixture(FeatureFixture({'two': '2'}))
+
+        self.assertEqual(getFeatureFlag('one'), None)
+        self.assertEqual(getFeatureFlag('two'), u'2')
+
+    def test_fixture_overrides_previously_set_flags(self):
+        self.useFixture(FeatureFixture({'one': '1'}))
+        self.useFixture(FeatureFixture({'one': '5'}))
+
+        self.assertEqual(getFeatureFlag('one'), u'5')
+
+    def test_fixture_does_not_set_value_for_flags_that_are_None(self):
+        self.useFixture(FeatureFixture({'nothing': None}))
+        self.assertEqual(getFeatureFlag('nothing'), None)

=== modified file 'lib/lp/services/memcache/doc/tales-cache.txt'
--- lib/lp/services/memcache/doc/tales-cache.txt	2010-09-12 05:10:19 +0000
+++ lib/lp/services/memcache/doc/tales-cache.txt	2010-10-20 19:06:13 +0000
@@ -349,17 +349,9 @@
 Memcache in templates can be disabled entirely by setting the memcache flag to
 'disabled'.
 
-    >>> from canonical.launchpad.webapp.servers import LaunchpadTestRequest
-    >>> from lp.services.features.model import FeatureFlag, getFeatureStore
-    >>> from lp.services.features.webapp import ScopesFromRequest
-    >>> from lp.services.features.flags import FeatureController
-    >>> from lp.services.features import per_thread
-    >>> ignore = getFeatureStore().add(FeatureFlag(
-    ...     scope=u'default', flag=u'memcache', value=u'disabled',
-    ...     priority=1))
-    >>> empty_request = LaunchpadTestRequest()
-    >>> per_thread.features = FeatureController(
-    ...     ScopesFromRequest(empty_request).lookup)
+    >>> from lp.services.features.testing import FeatureFixture
+    >>> fixture = FeatureFixture({'memcache': 'disabled'})
+    >>> fixture.setUp()
 
 And now what cached before will not cache.
 
@@ -376,3 +368,6 @@
     <div>
         <span>second</span>
     </div>
+
+    # Clean up our custom flag settings.
+    >>> fixture.cleanUp()