Test Speedup: Isolate Modulestore Signals

There are a number of Django Signals that are on the modulestore's
SignalHandler class, such as SignalHandler.course_published. These
signals can trigger very expensive processes to occur, such as course
overview or block structures generation. Most of the time, the test
author doesn't care about these side-effects.

This commit does a few things:

* Converts the signals on SignalHandler to be instances of a new
  SwitchedSignal class, that allows signal sending to be disabled.

* Creates a SignalIsolationMixin helper similar in spirit to the
  CacheIsolationMixin, and adds it to the ModuleStoreIsolationMixin
  (and thus to ModuleStoreTestCase and SharedModuleStoreTestCase).

* Converts our various tests to use this new mechanism. In some cases,
  this means adjusting query counts downwards because they no longer
  have to account for publishing listener actions.

Modulestore generated signals are now muted by default during test runs.
Calls to send() them will result in no-ops. You can choose to enable
specific signals for a given subclass of ModuleStoreTestCase or
SharedModuleStoreTestCase by specifying an ENABLED_SIGNALS class
attribute, like the following example:

    from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase

    class MyPublishTestCase(ModuleStoreTestCase):
        ENABLED_SIGNALS = ['course_published', 'pre_publish']

You should take great care when disabling signals outside of a
ModuleStoreTestCase or SharedModuleStoreTestCase, since they can leak
out into other tests. Be sure to always clean up, and never disable
signals outside of testing. Because signals are essentially process
globals, it can have a lot of unpleasant side-effects if we start
mucking around with them during live requests.

Overall, this change has cut the total test execution time for
edx-platform by a bit over a third, though we still spend a lot in
pre-test setup during our test builds.

[PERF-413]

common/djangoapps/enrollment/tests/test_views.py

@@ -142,6 +142,7 @@ class EnrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase, APITestCase):
     OTHER_EMAIL = "jane@example.com"
 
     ENABLED_CACHES = ['default', 'mongo_metadata_inheritance', 'loc_cache']
+    ENABLED_SIGNALS = ['course_published']
 
     def setUp(self):
         """ Create a course and user, then log in. """

common/djangoapps/student/tests/test_course_listing.py

@@ -29,6 +29,8 @@ class TestCourseListing(ModuleStoreTestCase, MilestonesTestCaseMixin):
     """
     Unit tests for getting the list of courses for a logged in user
     """
+    ENABLED_SIGNALS = ['course_deleted']
+
     def setUp(self):
         """
         Add a student & teacher

common/djangoapps/student/tests/tests.py

@@ -229,6 +229,7 @@ class DashboardTest(ModuleStoreTestCase):
     """
     Tests for dashboard utility functions
     """
+    ENABLED_SIGNALS = ['course_published']
 
     def setUp(self):
         super(DashboardTest, self).setUp()
@@ -434,10 +435,11 @@ class DashboardTest(ModuleStoreTestCase):
         # If user has a certificate with valid linked-in config then Add Certificate to LinkedIn button
         # should be visible. and it has URL value with valid parameters.
         self.client.login(username="jack", password="test")
-        LinkedInAddToProfileConfiguration(
+
+        LinkedInAddToProfileConfiguration.objects.create(
             company_identifier='0_mC_o2MizqdtZEmkVXjH4eYwMj4DnkCWrZP_D9',
             enabled=True
-        ).save()
+        )
 
         CourseModeFactory.create(
             course_id=self.course.id,
@@ -474,6 +476,7 @@ class DashboardTest(ModuleStoreTestCase):
             u'pfCertificationUrl=www.edx.org&'
             u'source=o'
         ).format(platform=quote(settings.PLATFORM_NAME.encode('utf-8')))
+
         self.assertContains(response, escape(expected_url))
 
     @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')

common/lib/xmodule/xmodule/modulestore/django.py

@@ -57,6 +57,86 @@ log = logging.getLogger(__name__)
 ASSET_IGNORE_REGEX = getattr(settings, "ASSET_IGNORE_REGEX", r"(^\._.*$)|(^\.DS_Store$)|(^.*~$)")
 
 
+class SwitchedSignal(django.dispatch.Signal):
+    """
+    SwitchedSignal is like a normal Django signal, except that you can turn it
+    on and off. This is especially useful for tests where we want to be able to
+    isolate signals and disable expensive operations that are irrelevant to
+    what's being tested (like everything that triggers off of a course publish).
+
+    SwitchedSignals default to being on. You should be very careful if you ever
+    turn one off -- the only instances of this class are shared class attributes
+    of `SignalHandler`. You have to make sure that you re-enable the signal when
+    you're done, or else you may permanently turn that signal off for that
+    process. I can't think of any reason you'd want to disable signals outside
+    of running tests.
+    """
+    def __init__(self, name, *args, **kwargs):
+        """
+        The `name` parameter exists only to make debugging more convenient.
+
+        All other args are passed to the constructor for django.dispatch.Signal.
+        """
+        super(SwitchedSignal, self).__init__(*args, **kwargs)
+        self.name = name
+        self._allow_signals = True
+
+    def disable(self):
+        """
+        Turn off signal sending.
+
+        All calls to send/send_robust will no-op.
+        """
+        self._allow_signals = False
+
+    def enable(self):
+        """
+        Turn on signal sending.
+
+        Calls to send/send_robust will behave like normal Django Signals.
+        """
+        self._allow_signals = True
+
+    def send(self, *args, **kwargs):
+        """
+        See `django.dispatch.Signal.send()`
+
+        This method will no-op and return an empty list if the signal has been
+        disabled.
+        """
+        log.debug(
+            "SwitchedSignal %s's send() called with args %s, kwargs %s - %s",
+            self.name,
+            args,
+            kwargs,
+            "ALLOW" if self._allow_signals else "BLOCK"
+        )
+        if self._allow_signals:
+            return super(SwitchedSignal, self).send(*args, **kwargs)
+        return []
+
+    def send_robust(self, *args, **kwargs):
+        """
+        See `django.dispatch.Signal.send_robust()`
+
+        This method will no-op and return an empty list if the signal has been
+        disabled.
+        """
+        log.debug(
+            "SwitchedSignal %s's send_robust() called with args %s, kwargs %s - %s",
+            self.name,
+            args,
+            kwargs,
+            "ALLOW" if self._allow_signals else "BLOCK"
+        )
+        if self._allow_signals:
+            return super(SwitchedSignal, self).send_robust(*args, **kwargs)
+        return []
+
+    def __repr__(self):
+        return u"SwitchedSignal('{}')".format(self.name)
+
+
 class SignalHandler(object):
     """
     This class is to allow the modulestores to emit signals that can be caught
@@ -88,23 +168,34 @@ class SignalHandler(object):
        almost no work. Its main job is to kick off the celery task that will
        do the actual work.
     """
-    pre_publish = django.dispatch.Signal(providing_args=["course_key"])
-    course_published = django.dispatch.Signal(providing_args=["course_key"])
-    course_deleted = django.dispatch.Signal(providing_args=["course_key"])
-    library_updated = django.dispatch.Signal(providing_args=["library_key"])
-    item_deleted = django.dispatch.Signal(providing_args=["usage_key", "user_id"])
+
+    # If you add a new signal, please don't forget to add it to the _mapping
+    # as well.
+    pre_publish = SwitchedSignal("pre_publish", providing_args=["course_key"])
+    course_published = SwitchedSignal("course_published", providing_args=["course_key"])
+    course_deleted = SwitchedSignal("course_deleted", providing_args=["course_key"])
+    library_updated = SwitchedSignal("library_updated", providing_args=["library_key"])
+    item_deleted = SwitchedSignal("item_deleted", providing_args=["usage_key", "user_id"])
 
     _mapping = {
-        "pre_publish": pre_publish,
-        "course_published": course_published,
-        "course_deleted": course_deleted,
-        "library_updated": library_updated,
-        "item_deleted": item_deleted,
+        signal.name: signal
+        for signal
+        in [pre_publish, course_published, course_deleted, library_updated, item_deleted]
     }
 
     def __init__(self, modulestore_class):
         self.modulestore_class = modulestore_class
 
+    @classmethod
+    def all_signals(cls):
+        """Return a list with all our signals in it."""
+        return cls._mapping.values()
+
+    @classmethod
+    def signal_by_name(cls, signal_name):
+        """Given a signal name, return the appropriate signal."""
+        return cls._mapping[signal_name]
+
     def send(self, signal_name, **kwargs):
         """
         Send the signal to the receivers.

common/lib/xmodule/xmodule/modulestore/tests/django_utils.py

@@ -194,7 +194,46 @@ TEST_DATA_SPLIT_MODULESTORE = functools.partial(
 )
 
 
-class ModuleStoreIsolationMixin(CacheIsolationMixin):
+class SignalIsolationMixin(object):
+    """
+    Simple utility mixin class to toggle ModuleStore signals on and off. This
+    class operates on `SwitchedSignal` objects on the modulestore's
+    `SignalHandler`.
+    """
+    @classmethod
+    def disable_all_signals(cls):
+        """Disable all signals in the modulestore's SignalHandler."""
+        for signal in SignalHandler.all_signals():
+            signal.disable()
+
+    @classmethod
+    def enable_all_signals(cls):
+        """Enable all signals in the modulestore's SignalHandler."""
+        for signal in SignalHandler.all_signals():
+            signal.enable()
+
+    @classmethod
+    def enable_signals_by_name(cls, *signal_names):
+        """
+        Enable specific signals in the modulestore's SignalHandler.
+
+        Arguments:
+            signal_names (list of `str`): Names of signals to enable.
+        """
+        for signal_name in signal_names:
+            try:
+                signal = SignalHandler.signal_by_name(signal_name)
+            except KeyError:
+                all_signal_names = sorted(s.name for s in SignalHandler.all_signals())
+                err_msg = (
+                    "You tried to enable signal '{}', but I don't recognize that "
+                    "signal name. Did you mean one of these?: {}"
+                )
+                raise ValueError(err_msg.format(signal_name, all_signal_names))
+            signal.enable()
+
+
+class ModuleStoreIsolationMixin(CacheIsolationMixin, SignalIsolationMixin):
     """
     A mixin to be used by TestCases that want to isolate their use of the
     Modulestore.
@@ -205,6 +244,8 @@ class ModuleStoreIsolationMixin(CacheIsolationMixin):
 
             MODULESTORE = <settings for the modulestore to test>
 
+            ENABLED_SIGNALS = ['course_published']
+
             def my_test(self):
                 self.start_modulestore_isolation()
                 self.addCleanup(self.end_modulestore_isolation)
@@ -213,10 +254,21 @@ class ModuleStoreIsolationMixin(CacheIsolationMixin):
                 ...
 
     """
-
     MODULESTORE = functools.partial(mixed_store_config, mkdtemp_clean(), {})
     CONTENTSTORE = functools.partial(contentstore_config)
     ENABLED_CACHES = ['default', 'mongo_metadata_inheritance', 'loc_cache']
+
+    # List of modulestore signals enabled for this test. Defaults to an empty
+    # list. The list of signals available is found on the SignalHandler class,
+    # in /common/lib/xmodule/xmodule/modulestore/django.py
+    #
+    # You must use the signal itself, and not its name. So for example:
+    #
+    # class MyPublishTestCase(ModuleStoreTestCase):
+    #     ENABLED_SIGNALS = ['course_published', 'pre_publish']
+    #
+    ENABLED_SIGNALS = []
+
     __settings_overrides = []
     __old_modulestores = []
     __old_contentstores = []
@@ -228,6 +280,8 @@ class ModuleStoreIsolationMixin(CacheIsolationMixin):
         :py:meth:`end_modulestore_isolation` is called, this modulestore will
         be flushed (all content will be deleted).
         """
+        cls.disable_all_signals()
+        cls.enable_signals_by_name(*cls.ENABLED_SIGNALS)
         cls.start_cache_isolation()
         override = override_settings(
             MODULESTORE=cls.MODULESTORE(),
@@ -256,6 +310,7 @@ class ModuleStoreIsolationMixin(CacheIsolationMixin):
         assert settings.MODULESTORE == cls.__old_modulestores.pop()
         assert settings.CONTENTSTORE == cls.__old_contentstores.pop()
         cls.end_cache_isolation()
+        cls.enable_all_signals()
 
 
 class SharedModuleStoreTestCase(ModuleStoreIsolationMixin, CacheIsolationTestCase):
@@ -384,6 +439,14 @@ class ModuleStoreTestCase(ModuleStoreIsolationMixin, TestCase):
     # Tell Django to clean out all databases, not just default
     multi_db = True
 
+    @classmethod
+    def setUpClass(cls):
+        super(ModuleStoreTestCase, cls).setUpClass()
+
+    @classmethod
+    def tearDownClass(cls):
+        super(ModuleStoreTestCase, cls).tearDownClass()
+
     def setUp(self):
         """
         Creates a test User if `self.CREATE_USER` is True.
@@ -400,8 +463,6 @@ class ModuleStoreTestCase(ModuleStoreIsolationMixin, TestCase):
 
         super(ModuleStoreTestCase, self).setUp()
 
-        SignalHandler.course_published.disconnect(trigger_update_xblocks_cache_task)
-
         self.store = modulestore()
 
         uname = 'testuser'