← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~mars/launchpad/add-profiling-feature-flag into lp:launchpad/devel

 

Māris Fogels has proposed merging lp:~mars/launchpad/add-profiling-feature-flag 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 changes our request profiling code to be controlled by feature flags
instead of config values.  The code does this using the new FeatureFixture added
in the prerequisite branch.  Now we can turn profiling on and off from the user
interface.


-- 
Māris Fogels -- https://launchpad.net/~mars
Launchpad.net -- cross-project collaboration and hosting
-- 
https://code.launchpad.net/~mars/launchpad/add-profiling-feature-flag/+merge/39177
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~mars/launchpad/add-profiling-feature-flag into lp:launchpad/devel.
=== modified file 'lib/canonical/launchpad/doc/profiling.txt'
--- lib/canonical/launchpad/doc/profiling.txt	2010-08-26 15:57:56 +0000
+++ lib/canonical/launchpad/doc/profiling.txt	2010-10-22 20:11:28 +0000
@@ -100,11 +100,10 @@
 
 The feature has two modes.
 
--   It can be configured to optionally profile requests.  To turn this on, in
-    ``launchpad-lazr.conf`` (e.g.,
-    ``configs/development/launchpad-lazr.conf``) , in the ``[profiling]``
-    section, set ``profiling_allowed: True``.  As of this writing, this
-    is the default value for development.
+-   It can be configured to optionally profile requests.  To turn this on you
+    must add a feature flag that sets the 'request_profiling' flag to 'on'.
+    This flag can be set directly in the database or using bin/harness.  See
+    https://dev.launchpad.net/LEP/FeatureFlags for the details.
 
     Once it is turned on, you can insert /++profile++/ in the URL to get
     basic instructions on how to use the feature.  You use the
@@ -123,6 +122,19 @@
     lp/services/profile is hooked up properly.  This is intended to be a
     smoke test.  The unit tests verify further functionality.
 
+    # XXX mars 2010-09-23
+    # We will temporarily turn on the profiling feature so that this test
+    # still passes. Profiling used to be active for all developers, but the
+    # feature flags work has shut it off. This fixture can be removed once
+    # profiling for developers is active by default once again.
+    >>> from lp.services.features.testing import FeatureFixture
+    >>> fixture = FeatureFixture({'request_profiling': 'on'})
+    >>> fixture.setUp()
+
+    >>> from lp.services.features import getFeatureFlag
+    >>> getFeatureFlag('request_profiling')
+    u'on'
+
     >>> response = http('GET /++profile++ HTTP/1.0')
     >>> '<h1>Profiling Information</h1>' in response.getBody()
     True
@@ -214,3 +226,7 @@
     >>> shutil.rmtree(pagetests_profile_dir)
     >>> shutil.rmtree(profile_dir)
     >>> old_config = config.pop('memory_profile')
+
+
+    # Turn profiling off
+    >>> fixture.cleanUp()

=== modified file 'lib/lp/services/features/__init__.py'
--- lib/lp/services/features/__init__.py	2010-09-29 08:36:22 +0000
+++ lib/lp/services/features/__init__.py	2010-10-22 20:11:28 +0000
@@ -131,6 +131,40 @@
 The flags active during a page request, and the scopes that were looked
 up are visible in the comment at the bottom of every standard Launchpad
 page.
+
+
+Setting flags in your tests
+===========================
+
+lp.services.features.testing contains a fixture that can help you temporarily
+set feature flags during a test run.  All existing flags will be removed and
+the new flag values set.  The old flags are restored when the fixture is
+cleaned up.
+
+The lp.testing.TestCase class has built-in support for test fixtures.  To
+set a flag for the duration of a test::
+
+    from lp.services.features.testing import FeatureFixture
+
+    def setUp(self):
+        self.useFixture(FeatureFixture({'myflag', 'on'}))
+
+
+You can also use the fixture as a context manager::
+
+    with FeatureFixture({'myflag': 'on'}):
+        ...
+
+
+You can call the fixture's setUp() and cleanUp() methods for doctests and
+other environments that have no explicit setup and teardown::
+
+    >>> from lp.services.features.testing import FeatureFixture
+    >>> fixture = FeatureFixture({'my-doctest-flag': 'on'})
+    >>> fixture.setUp()
+    ...
+    >>> fixture.cleanUp()
+
 """
 
 import threading

=== 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-22 20:11:28 +0000
@@ -27,8 +27,8 @@
     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 +67,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.
         """

=== 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-22 20:11:28 +0000
@@ -0,0 +1,78 @@
+# 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 StormFeatureRuleSource
+
+
+class FeatureFixture(Fixture):
+    """A fixture that sets a feature.
+
+    The fixture takes a dictionary as its constructor argument. The keys of
+    the dictionary are features to be set. All existing flags will be cleared.
+
+    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 dictionary'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())
+        rule_source.setAllRules(self.makeNewRules())
+
+        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)
+
+    def makeNewRules(self):
+        """Make a set of new feature flag rules."""
+        # XXX mars 2010-10-22
+        # This would be much nicer if the rules were built as a namedtuple
+        # instead of a list of position-dependant tuple elements.
+        # Still waiting for Python 2.6 though.
+
+        # 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.
+        #
+        # Flags that are not present in the set of new rules will be deleted
+        # by setAllRules().
+
+        new_rules = [
+            (flag_name,         # Flag name.
+             'default',         # Scope.
+             1,                 # Priority 1 is a safe default.
+             unicode(value))    # Flag value.
+            for flag_name, value in self.desired_features.iteritems()
+                if value is not None]
+
+        return new_rules

=== 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-22 20:11:28 +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-10-18 22:24:59 +0000
+++ lib/lp/services/memcache/doc/tales-cache.txt	2010-10-22 20:11:28 +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 import per_thread
-    >>> from lp.services.features.flags import FeatureController
-    >>> from lp.services.features.webapp import ScopesFromRequest
-    >>> 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()

=== modified file 'lib/lp/services/profile/profile.py'
--- lib/lp/services/profile/profile.py	2010-08-27 14:20:28 +0000
+++ lib/lp/services/profile/profile.py	2010-10-22 20:11:28 +0000
@@ -30,12 +30,15 @@
     memory,
     resident,
     )
+from lp.services import features
 
 
 class ProfilingOops(Exception):
     """Fake exception used to log OOPS information when profiling pages."""
 
 
+# This variable holds all of the data about an active profile run for the
+# duration of the request.
 _profilers = threading.local()
 
 
@@ -46,8 +49,16 @@
     If profiling is enabled, start a profiler for this thread. If memory
     profiling is requested, save the VSS and RSS.
     """
-    if not config.profiling.profiling_allowed:
+    if features.getFeatureFlag('request_profiling'):
+        # Set a flag so that the end_request hook knows it has some work to
+        # do. We can not use the feature flag itself to control our
+        # end_request event handlers because the handler that tears down the
+        # feature flags and the handler that ends the profiling run do not
+        # fire in a predictable order.
+        _profilers.active = True
+    else:
         return
+
     actions = get_desired_profile_actions(event.request)
     if config.profiling.profile_all_requests:
         actions.add('log')
@@ -71,7 +82,7 @@
 @adapter(IEndRequestEvent)
 def end_request(event):
     """If profiling is turned on, save profile data for the request."""
-    if not config.profiling.profiling_allowed:
+    if not getattr(_profilers, 'active', False):
         return
     try:
         actions = _profilers.actions

=== modified file 'lib/lp/services/profile/tests.py'
--- lib/lp/services/profile/tests.py	2010-08-26 22:10:56 +0000
+++ lib/lp/services/profile/tests.py	2010-10-22 20:11:28 +0000
@@ -14,7 +14,6 @@
 import time
 import unittest
 
-from lp.testing import TestCase
 from bzrlib.lsprof import BzrProfiler
 from zope.app.publication.interfaces import EndRequestEvent
 from zope.component import getSiteManager
@@ -26,7 +25,11 @@
     )
 from canonical.launchpad.webapp.servers import LaunchpadTestRequest
 from canonical.launchpad.webapp.interfaces import StartRequestEvent
+from canonical.testing import layers
 from lp.services.profile import profile
+from lp.services.features.testing import FeatureFixture
+from lp.testing import TestCase
+
 
 EXAMPLE_HTML_START = '''\
 <html><head><title>Random!</title></head>
@@ -43,6 +46,8 @@
 
 class BaseTest(TestCase):
 
+    layer = layers.DatabaseFunctionalLayer
+
     def _get_request(self, path='/'):
         """Return a test request for the given path."""
         return LaunchpadTestRequest(PATH_INFO=path)
@@ -58,13 +63,19 @@
                 getattr(profile._profilers, name, None), None,
                 'Profiler state (%s) is dirty; %s.' % (name, message))
 
+    def turnOnProfiling(self):
+        """Turn on the request_profiling feature flag."""
+        self.useFixture(FeatureFixture({'request_profiling': 'on'}))
+
+    def turnOffProfiling(self):
+        """Turn off the request_profiling feature flag."""
+        self.useFixture(FeatureFixture({'request_profiling': None}))
+
     def pushProfilingConfig(
-        self, profiling_allowed='False', profile_all_requests='False',
-        memory_profile_log=''):
+        self, profile_all_requests='False', memory_profile_log=''):
         """This is a convenience for setting profile configs."""
         self.pushConfig(
             'profiling',
-            profiling_allowed=profiling_allowed,
             profile_all_requests=profile_all_requests,
             memory_profile_log=memory_profile_log)
 
@@ -86,11 +97,12 @@
                 delattr(profile._profilers, name)
         TestCase.tearDown(self)
 
-    def test_config_stops_profiling(self):
-        """The ``profiling_allowed`` configuration should disable all
+    def test_feature_flag_stops_profiling(self):
+        """The ``request_profiling`` feature flag should disable all
         profiling, even if it is requested"""
+        self.turnOffProfiling()
         self.pushProfilingConfig(
-            profiling_allowed='False', profile_all_requests='True',
+            profile_all_requests='True',
             memory_profile_log='.')
         profile.start_request(self._get_start_event('/++profile++show,log'))
         self.assertCleanProfilerState('config was ignored')
@@ -98,7 +110,7 @@
     def test_optional_profiling_without_marked_request_has_no_profile(self):
         """Even if profiling is allowed, it does not happen with a normal
         request."""
-        self.pushProfilingConfig(profiling_allowed='True')
+        self.turnOnProfiling()
         profile.start_request(self._get_start_event('/'))
         self.assertEqual(profile._profilers.actions, set())
         self.assertIs(getattr(profile._profilers, 'profiler', None), None)
@@ -108,7 +120,7 @@
     def test_optional_profiling_with_show_request_starts_profiling(self):
         """If profiling is allowed and a request with the "show" marker
         URL segment is made, profiling starts."""
-        self.pushProfilingConfig(profiling_allowed='True')
+        self.turnOnProfiling()
         profile.start_request(self._get_start_event('/++profile++show/'))
         self.assertIsInstance(profile._profilers.profiler, BzrProfiler)
         self.assertIs(
@@ -119,7 +131,7 @@
     def test_optional_profiling_with_log_request_starts_profiling(self):
         """If profiling is allowed and a request with the "log" marker
         URL segment is made, profiling starts."""
-        self.pushProfilingConfig(profiling_allowed='True')
+        self.turnOnProfiling()
         profile.start_request(self._get_start_event('/++profile++log/'))
         self.assertIsInstance(profile._profilers.profiler, BzrProfiler)
         self.assertIs(
@@ -130,7 +142,7 @@
     def test_optional_profiling_with_combined_request_starts_profiling(self):
         """If profiling is allowed and a request with the "log" and
         "show" marker URL segment is made, profiling starts."""
-        self.pushProfilingConfig(profiling_allowed='True')
+        self.turnOnProfiling()
         profile.start_request(self._get_start_event('/++profile++log,show/'))
         self.assertIsInstance(profile._profilers.profiler, BzrProfiler)
         self.assertIs(
@@ -141,7 +153,7 @@
     def test_optional_profiling_with_reversed_request_starts_profiling(self):
         """If profiling is allowed and a request with the "show" and
         "log" marker URL segment is made, profiling starts."""
-        self.pushProfilingConfig(profiling_allowed='True')
+        self.turnOnProfiling()
         # The fact that this is reversed from the previous request is the only
         # difference from the previous test.  Also, it doesn't have a
         # trailing slash. :-P
@@ -154,8 +166,8 @@
 
     def test_forced_profiling_registers_action(self):
         """profile_all_requests should register as a log action"""
-        self.pushProfilingConfig(
-            profiling_allowed='True', profile_all_requests='True')
+        self.turnOnProfiling()
+        self.pushProfilingConfig(profile_all_requests='True')
         profile.start_request(self._get_start_event('/'))
         self.assertIsInstance(profile._profilers.profiler, BzrProfiler)
         self.assertIs(
@@ -167,7 +179,7 @@
         """If profiling is allowed and a request with the marker URL segment
         is made incorrectly, profiling does not start and help is an action.
         """
-        self.pushProfilingConfig(profiling_allowed='True')
+        self.turnOnProfiling()
         profile.start_request(self._get_start_event('/++profile++/'))
         self.assertIs(getattr(profile._profilers, 'profiler', None), None)
         self.assertIs(
@@ -179,8 +191,8 @@
         """If profiling is forced and a request with the marker URL segment
         is made incorrectly, profiling starts and help is an action.
         """
-        self.pushProfilingConfig(
-            profiling_allowed='True', profile_all_requests='True')
+        self.turnOnProfiling()
+        self.pushProfilingConfig(profile_all_requests='True')
         profile.start_request(self._get_start_event('/++profile++/'))
         self.assertIsInstance(profile._profilers.profiler, BzrProfiler)
         self.assertIs(
@@ -189,8 +201,8 @@
         self.assertEquals(profile._profilers.actions, set(('help', 'log')))
 
     def test_memory_profile_start(self):
-        self.pushProfilingConfig(
-            profiling_allowed='True', memory_profile_log='.')
+        self.turnOnProfiling()
+        self.pushProfilingConfig(memory_profile_log='.')
         profile.start_request(self._get_start_event('/'))
         self.assertIs(getattr(profile._profilers, 'profiler', None), None)
         self.assertIsInstance(profile._profilers.memory_profile_start, tuple)
@@ -198,8 +210,8 @@
         self.assertEqual(profile._profilers.actions, set())
 
     def test_combo_memory_and_profile_start(self):
-        self.pushProfilingConfig(
-            profiling_allowed='True', memory_profile_log='.')
+        self.turnOnProfiling()
+        self.pushProfilingConfig(memory_profile_log='.')
         profile.start_request(self._get_start_event('/++profile++show/'))
         self.assertIsInstance(profile._profilers.profiler, BzrProfiler)
         self.assertIsInstance(profile._profilers.memory_profile_start, tuple)
@@ -267,10 +279,11 @@
     #########################################################################
     # Tests
     def test_config_stops_profiling(self):
-        """The ``profiling_allowed`` configuration should disable all
+        """The ``request_profiling`` feature should disable all
         profiling, even if it is requested"""
+        self.turnOffProfiling()
         self.pushProfilingConfig(
-            profiling_allowed='False', profile_all_requests='True',
+            profile_all_requests='True',
             memory_profile_log=self.memory_profile_log)
         request = self.endRequest('/++profile++show,log')
         self.assertIs(getattr(request, 'oops', None), None)
@@ -282,7 +295,7 @@
     def test_optional_profiling_without_marked_request_has_no_profile(self):
         """Even if profiling is allowed, it does not happen with a normal
         request."""
-        self.pushProfilingConfig(profiling_allowed='True')
+        self.turnOnProfiling()
         request = self.endRequest('/')
         self.assertIs(getattr(request, 'oops', None), None)
         self.assertEqual(self.getAddedResponse(request), '')
@@ -293,7 +306,7 @@
     def test_optional_profiling_with_show_request_profiles(self):
         """If profiling is allowed and a request with the "show" marker
         URL segment is made, profiling starts."""
-        self.pushProfilingConfig(profiling_allowed='True')
+        self.turnOnProfiling()
         request = self.endRequest('/++profile++show/')
         self.assertIsInstance(request.oops, ErrorReport)
         self.assertIn('Top Inline Time', self.getAddedResponse(request))
@@ -304,7 +317,7 @@
     def test_optional_profiling_with_log_request_profiles(self):
         """If profiling is allowed and a request with the "log" marker
         URL segment is made, profiling starts."""
-        self.pushProfilingConfig(profiling_allowed='True')
+        self.turnOnProfiling()
         request = self.endRequest('/++profile++log/')
         self.assertIsInstance(request.oops, ErrorReport)
         response = self.getAddedResponse(request)
@@ -319,7 +332,7 @@
     def test_optional_profiling_with_combined_request_profiles(self):
         """If profiling is allowed and a request with the "log" and
         "show" marker URL segment is made, profiling starts."""
-        self.pushProfilingConfig(profiling_allowed='True')
+        self.turnOnProfiling()
         request = self.endRequest('/++profile++log,show')
         self.assertIsInstance(request.oops, ErrorReport)
         response = self.getAddedResponse(request)
@@ -333,8 +346,8 @@
 
     def test_forced_profiling_logs(self):
         """profile_all_requests should register as a log action"""
-        self.pushProfilingConfig(
-            profiling_allowed='True', profile_all_requests='True')
+        self.turnOnProfiling()
+        self.pushProfilingConfig(profile_all_requests='True')
         request = self.endRequest('/')
         self.assertIsInstance(request.oops, ErrorReport)
         response = self.getAddedResponse(request)
@@ -351,7 +364,7 @@
         """If profiling is allowed and a request with the marker URL segment
         is made incorrectly, profiling does not start and help is an action.
         """
-        self.pushProfilingConfig(profiling_allowed='True')
+        self.turnOnProfiling()
         request = self.endRequest('/++profile++')
         self.assertIs(getattr(request, 'oops', None), None)
         response = self.getAddedResponse(request)
@@ -365,8 +378,8 @@
         """If profiling is forced and a request with the marker URL segment
         is made incorrectly, profiling starts and help is an action.
         """
-        self.pushProfilingConfig(
-            profiling_allowed='True', profile_all_requests='True')
+        self.turnOnProfiling()
+        self.pushProfilingConfig(profile_all_requests='True')
         request = self.endRequest('/++profile++')
         self.assertIsInstance(request.oops, ErrorReport)
         response = self.getAddedResponse(request)
@@ -382,9 +395,8 @@
 
     def test_memory_profile(self):
         "Does the memory profile work?"
-        self.pushProfilingConfig(
-            profiling_allowed='True',
-            memory_profile_log=self.memory_profile_log)
+        self.turnOnProfiling()
+        self.pushProfilingConfig(memory_profile_log=self.memory_profile_log)
         request = self.endRequest('/')
         self.assertIs(getattr(request, 'oops', None), None)
         self.assertEqual(self.getAddedResponse(request), '')
@@ -400,9 +412,8 @@
 
     def test_memory_profile_with_non_defaults(self):
         "Does the memory profile work with an oops and pageid?"
-        self.pushProfilingConfig(
-            profiling_allowed='True',
-            memory_profile_log=self.memory_profile_log)
+        self.turnOnProfiling()
+        self.pushProfilingConfig(memory_profile_log=self.memory_profile_log)
         request = self.endRequest('/++profile++show/no-such-file',
                                   KeyError(), pageid='Foo')
         log = self.getMemoryLog()
@@ -413,9 +424,8 @@
         self.assertCleanProfilerState()
 
     def test_combo_memory_and_profile(self):
-        self.pushProfilingConfig(
-            profiling_allowed='True',
-            memory_profile_log=self.memory_profile_log)
+        self.turnOnProfiling()
+        self.pushProfilingConfig(memory_profile_log=self.memory_profile_log)
         request = self.endRequest('/++profile++show/')
         self.assertIsInstance(request.oops, ErrorReport)
         self.assertIn('Top Inline Time', self.getAddedResponse(request))
@@ -424,7 +434,7 @@
         self.assertCleanProfilerState()
 
     def test_profiling_oops_is_informational(self):
-        self.pushProfilingConfig(profiling_allowed='True')
+        self.turnOnProfiling()
         request = self.endRequest('/++profile++show/')
         response = self.getAddedResponse(request)
         self.assertIsInstance(request.oops, ErrorReport)
@@ -433,7 +443,7 @@
         self.assertCleanProfilerState()
 
     def test_real_oops_trumps_profiling_oops(self):
-        self.pushProfilingConfig(profiling_allowed='True')
+        self.turnOnProfiling()
         request = self.endRequest('/++profile++show/no-such-file',
                                   KeyError('foo'))
         self.assertIsInstance(request.oops, ErrorReport)
@@ -444,7 +454,7 @@
         self.assertCleanProfilerState()
 
     def test_oopsid_is_in_profile_filename(self):
-        self.pushProfilingConfig(profiling_allowed='True')
+        self.turnOnProfiling()
         request = self.endRequest('/++profile++log/')
         self.assertIn("-" + request.oopsid + "-", self.getProfilePaths()[0])
         self.assertCleanProfilerState()