Mock ES to stop series security failures (bug 734217)

This also fixed a few cases where Mock was backwards incompatible.

Author: Rob Hudson

apps/amo/tests/__init__.py

@@ -171,21 +171,29 @@ def __getattr__(self, name):
             raise AttributeError
 
 
+ES_patcher = mock.patch('elasticutils.get_es', spec=True)
+
+
 class TestCase(RedisTest, test_utils.TestCase):
     """Base class for all amo tests."""
     client_class = TestClient
+    mock_es = True
+
+    @classmethod
+    def setUpClass(cls):
+        super(TestCase, cls).setUpClass()
+        if cls.mock_es:
+            ES_patcher.start()
+
+    @classmethod
+    def tearDownClass(cls):
+        super(TestCase, cls).tearDownClass()
+        if cls.mock_es:
+            ES_patcher.stop()
 
     def _pre_setup(self):
         super(TestCase, self)._pre_setup()
         self.reset_featured_addons()
-        # Mock out ES indexing for non-ES tests.
-        if not getattr(self, 'es', False):
-            for p in ['addons.tasks.index_addons',
-                      'addons.tasks.unindex_addons',
-                      'amo.models.SearchMixin']:
-                patcher = mock.patch(p)
-                patcher.start()
-                self.addCleanup(patcher.stop)
 
     def reset_featured_addons(self):
         from addons.cron import reset_featured_addons
@@ -383,10 +391,12 @@ class ESTestCase(TestCase):
     # outside Django transactions so be careful to clean up afterwards.
     es = True
     use_es = None
+    mock_es = False
     exempt_from_fixture_bundling = True  # ES doesn't support bundling (yet?)
 
     @classmethod
     def setUpClass(cls):
+        super(ESTestCase, cls).setUpClass()
         cls.es = elasticutils.get_es()
 
         if ESTestCase.use_es is None:
@@ -410,7 +420,6 @@ def setUpClass(cls):
             except:
                 raise
 
-        super(ESTestCase, cls).setUpClass()
         addons.search.setup_mapping()
         stats.search.setup_indexes()
 
@@ -426,6 +435,7 @@ def tearDownClass(cls):
                   Translation, Addon, Collection, AppVersion, Application)
         for model in models:
             model.objects.all().delete()
+
         super(ESTestCase, cls).tearDownClass()
 
     @classmethod

apps/amo/tests/test_search.py

@@ -1,54 +1,48 @@
 from django.core import paginator
 
+import elasticutils
 import mock
 from nose.tools import eq_
 
+import amo
 import amo.tests
-import amo.utils
 from addons.models import Addon
-import addons.tasks
 
 
 class TestESIndexing(amo.tests.ESTestCase):
+    mock_es = False
     es = True
 
-    @classmethod
-    def setUpClass(cls):
-        super(TestESIndexing, cls).setUpClass()
-        cls.setUpIndex()
-
     # This needs to be in its own class for data isolation.
     def test_indexed_count(self):
         # Did all the right addons get indexed?
         eq_(Addon.search().filter(type=1, is_disabled=False).count(),
             Addon.objects.filter(disabled_by_user=False,
                                  status__in=amo.VALID_STATUSES).count())
 
-    def test_real_indexing(self):
-        def check(s):
-            assert callable(s) and not isinstance(s, mock.Mock)
-        check(addons.tasks.index_addons)
-        check(addons.tasks.unindex_addons)
-        check(amo.models.SearchMixin)
+    def test_get_es_not_mocked(self):
+        es = elasticutils.get_es()
+        assert not issubclass(es.__class__, mock.Mock)
 
 
 class TestNoESIndexing(amo.tests.TestCase):
 
+    mock_es = True
+
     def test_no_es(self):
         assert not getattr(self, 'es', False), (
             'TestCase should not have "es" attribute')
 
-    def test_mocked_indexing(self):
-        def check(s):
-            assert callable(s) and isinstance(s, mock.Mock)
-        check(addons.tasks.index_addons)
-        check(addons.tasks.unindex_addons)
-        check(amo.models.SearchMixin)
-
     def test_not_indexed(self):
         addon = Addon.objects.create(type=amo.ADDON_EXTENSION,
                                      status=amo.STATUS_PUBLIC)
-        eq_(Addon.search().filter(id__in=addon.id).count(), 0)
+        assert issubclass(
+            Addon.search().filter(id__in=addon.id).count().__class__,
+            mock.Mock)
+
+    def test_get_es_mocked(self):
+        es = elasticutils.get_es()
+        assert issubclass(es.__class__, mock.Mock)
 
 
 class TestES(amo.tests.ESTestCase):

apps/editors/tests/test_views.py

@@ -9,9 +9,10 @@
 from django.utils.datastructures import SortedDict
 
 import jingo
-from mock import patch
+from mock import Mock, patch
 from nose.tools import eq_
 from pyquery import PyQuery as pq
+import waffle
 
 import amo
 import amo.tests
@@ -245,10 +246,9 @@ def test_search_addon_doesnt_exist(self):
     def test_breadcrumbs(self):
         self._test_breadcrumbs([('Add-on Review Log', None)])
 
-    @patch('devhub.models.ActivityLog.arguments')
-    def test_addon_missing(self, arguments):
+    @patch('devhub.models.ActivityLog.arguments', new=Mock)
+    def test_addon_missing(self):
         self.make_approvals()
-        arguments.return_value = []
         r = self.client.get(self.url)
         eq_(pq(r.content)('#log-listing tr td').eq(1).text(),
             'Add-on has been deleted.')
@@ -1889,9 +1889,8 @@ def test_compare_link(self):
         ]
         check_links(expected, links, verify=False)
 
-    # TODO: Remove 'marketplace' switch.
-    @patch.dict(jingo.env.globals['waffle'], {'switch': lambda x: True})
     def test_show_premium(self):
+        waffle.models.Switch.objects.create(name='marketplace', active=True)
         for type_ in amo.ADDON_PREMIUMS:
             self.addon.update(premium_type=type_)
             res = self.client.get(self.url)

apps/stats/tests/test_views.py

@@ -86,6 +86,7 @@ def index(self):
 
 class TestSeriesSecurity(ESStatsTest):
     """Tests to make sure all restricted data remains restricted."""
+    mock_es = True  # We're checking only headers, not content.
 
     def _check_it(self, views, status):
         for view, kwargs in views:

mkt/site/tests/test_views.py

@@ -7,12 +7,12 @@
 
 class TestRobots(amo.tests.TestCase):
 
-    @mock.patch_object(settings, 'ENGAGE_ROBOTS', True)
+    @mock.patch.object(settings, 'ENGAGE_ROBOTS', True)
     def test_engage_robots(self):
         rs = self.client.get('/robots.txt')
         self.assertContains(rs, 'Allow: /')
 
-    @mock.patch_object(settings, 'ENGAGE_ROBOTS', False)
+    @mock.patch.object(settings, 'ENGAGE_ROBOTS', False)
     def test_do_not_engage_robots(self):
         rs = self.client.get('/robots.txt')
         self.assertContains(rs, 'Disallow: /')