Merge pull request #8504 from cpennington/bdero/ccx-query-tests

A version of #8260 that is consistent on jenkins and locally

cms/djangoapps/contentstore/tests/utils.py

@@ -11,15 +11,16 @@ from django.contrib.auth.models import User
 from django.test.client import Client
 from opaque_keys.edx.locations import SlashSeparatedCourseKey, AssetLocation
 
-from contentstore.utils import reverse_url
-from student.models import Registration
+from contentstore.utils import reverse_url  # pylint: disable=import-error
+from student.models import Registration  # pylint: disable=import-error
 from xmodule.modulestore.split_mongo.split import SplitMongoModuleStore
 from xmodule.contentstore.django import contentstore
 from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.inheritance import own_metadata
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
-from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
+from xmodule.modulestore.tests.factories import CourseFactory
 from xmodule.modulestore.xml_importer import import_course_from_xml
+from xmodule.modulestore.tests.utils import ProceduralCourseTestMixin
 
 TEST_DATA_DIR = settings.COMMON_TEST_DATA_ROOT
 
@@ -67,7 +68,7 @@ class AjaxEnabledTestClient(Client):
         return self.get(path, data or {}, follow, HTTP_ACCEPT="application/json", **extra)
 
 
-class CourseTestCase(ModuleStoreTestCase):
+class CourseTestCase(ProceduralCourseTestMixin, ModuleStoreTestCase):
     """
     Base class for Studio tests that require a logged in user and a course.
     Also provides helper methods for manipulating and verifying the course.
@@ -100,26 +101,6 @@ class CourseTestCase(ModuleStoreTestCase):
         nonstaff.is_authenticated = lambda: authenticate
         return client, nonstaff
 
-    def populate_course(self, branching=2):
-        """
-        Add k chapters, k^2 sections, k^3 verticals, k^4 problems to self.course (where k = branching)
-        """
-        user_id = self.user.id
-        self.populated_usage_keys = {}
-
-        def descend(parent, stack):
-            if not stack:
-                return
-
-            xblock_type = stack[0]
-            for _ in range(branching):
-                child = ItemFactory.create(category=xblock_type, parent_location=parent.location, user_id=user_id)
-                print child.location
-                self.populated_usage_keys.setdefault(xblock_type, []).append(child.location)
-                descend(child, stack[1:])
-
-        descend(self.course, ['chapter', 'sequential', 'vertical', 'problem'])
-
     def reload_course(self):
         """
         Reloads the course object from the database

common/djangoapps/request_cache/middleware.py

@@ -18,7 +18,11 @@ class RequestCache(object):
         """
         return _request_cache_threadlocal.request
 
-    def clear_request_cache(self):
+    @classmethod
+    def clear_request_cache(cls):
+        """
+        Empty the request cache.
+        """
         _request_cache_threadlocal.data = {}
         _request_cache_threadlocal.request = None
 

common/lib/xmodule/xmodule/modulestore/mongo/base.py

@@ -228,6 +228,14 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin):
         Return an XModule instance for the specified location
         """
         assert isinstance(location, UsageKey)
+
+        if location.run is None:
+            # self.module_data is keyed on locations that have full run information.
+            # If the supplied location is missing a run, then we will miss the cache and
+            # incur an additional query.
+            # TODO: make module_data a proper class that can handle this itself.
+            location = location.replace(course_key=self.modulestore.fill_in_run(location.course_key))
+
         json_data = self.module_data.get(location)
         if json_data is None:
             module = self.modulestore.get_item(location, using_descriptor_system=self)
@@ -258,7 +266,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin):
                         else ModuleStoreEnum.Branch.draft_preferred
                     )
                     if parent_url:
-                        parent = BlockUsageLocator.from_string(parent_url)
+                        parent = self._convert_reference_to_key(parent_url)
                 if not parent and category != 'course':
                     # try looking it up just-in-time (but not if we're working with a root node (course).
                     parent = self.modulestore.get_parent_location(
@@ -324,7 +332,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin):
         """
         Convert a single serialized UsageKey string in a ReferenceField into a UsageKey.
         """
-        key = Location.from_string(ref_string)
+        key = UsageKey.from_string(ref_string)
         return key.replace(run=self.modulestore.fill_in_run(key.course_key).run)
 
     def __setattr__(self, name, value):

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

+"""
+Factories for use in tests of XBlocks.
+"""
+
+import inspect
 import pprint
 import threading
 from uuid import uuid4
@@ -321,27 +326,40 @@ def check_sum_of_calls(object_, methods, maximum_calls, minimum_calls=1):
     Instruments the given methods on the given object to verify that the total sum of calls made to the
     methods falls between minumum_calls and maximum_calls.
     """
+
     mocks = {
         method: Mock(wraps=getattr(object_, method))
         for method in methods
     }
 
-    with patch.multiple(object_, **mocks):
+    if inspect.isclass(object_):
+        # If the object that we're intercepting methods on is a class, rather than a module,
+        # then we need to set the method to a real function, so that self gets passed to it,
+        # and then explicitly pass that self into the call to the mock
+        # pylint: disable=unnecessary-lambda,cell-var-from-loop
+        mock_kwargs = {
+            method: lambda self, *args, **kwargs: mocks[method](self, *args, **kwargs)
+            for method in methods
+        }
+    else:
+        mock_kwargs = mocks
+
+    with patch.multiple(object_, **mock_kwargs):
         yield
 
     call_count = sum(mock.call_count for mock in mocks.values())
-    calls = pprint.pformat({
-        method_name: mock.call_args_list
-        for method_name, mock in mocks.items()
-    })
 
     # Assertion errors don't handle multi-line values, so pretty-print to std-out instead
     if not minimum_calls <= call_count <= maximum_calls:
+        calls = {
+            method_name: mock.call_args_list
+            for method_name, mock in mocks.items()
+        }
         print "Expected between {} and {} calls, {} were made. Calls: {}".format(
             minimum_calls,
             maximum_calls,
             call_count,
-            calls,
+            pprint.pformat(calls),
         )
 
     # verify the counter actually worked by ensuring we have counted greater than (or equal to) the minimum calls

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

@@ -143,3 +143,33 @@ class MixedSplitTestCase(TestCase):
             modulestore=self.store,
             **extra
         )
+
+
+class ProceduralCourseTestMixin(object):
+    """
+    Contains methods for testing courses generated procedurally
+    """
+    def populate_course(self, branching=2):
+        """
+        Add k chapters, k^2 sections, k^3 verticals, k^4 problems to self.course (where k = branching)
+        """
+        user_id = self.user.id
+        self.populated_usage_keys = {}  # pylint: disable=attribute-defined-outside-init
+
+        def descend(parent, stack):  # pylint: disable=missing-docstring
+            if not stack:
+                return
+
+            xblock_type = stack[0]
+            for _ in range(branching):
+                child = ItemFactory.create(
+                    category=xblock_type,
+                    parent_location=parent.location,
+                    user_id=user_id
+                )
+                self.populated_usage_keys.setdefault(xblock_type, []).append(
+                    child.location
+                )
+                descend(child, stack[1:])
+
+        descend(self.course, ['chapter', 'sequential', 'vertical', 'problem'])

lms/djangoapps/ccx/tests/test_field_override_performance.py

+# coding=UTF-8
+"""
+Performance tests for field overrides.
+"""
+import ddt
+import itertools
+import mock
+
+from courseware.views import progress  # pylint: disable=import-error
+from datetime import datetime
+from django.conf import settings
+from django.core.cache import get_cache
+from django.test.client import RequestFactory
+from django.test.utils import override_settings
+from edxmako.middleware import MakoMiddleware  # pylint: disable=import-error
+from nose.plugins.attrib import attr
+from pytz import UTC
+from request_cache.middleware import RequestCache
+from student.models import CourseEnrollment
+from student.tests.factories import UserFactory  # pylint: disable=import-error
+from xblock.core import XBlock
+from xmodule.modulestore.django import modulestore
+from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, \
+    TEST_DATA_SPLIT_MODULESTORE, TEST_DATA_MONGO_MODULESTORE
+from xmodule.modulestore.tests.factories import check_mongo_calls, CourseFactory, check_sum_of_calls
+from xmodule.modulestore.tests.utils import ProceduralCourseTestMixin
+
+
+@attr('shard_1')
+@mock.patch.dict(
+    'django.conf.settings.FEATURES', {'ENABLE_XBLOCK_VIEW_ENDPOINT': True}
+)
+@ddt.ddt
+class FieldOverridePerformanceTestCase(ProceduralCourseTestMixin,
+                                       ModuleStoreTestCase):
+    """
+    Base class for instrumenting SQL queries and Mongo reads for field override
+    providers.
+    """
+    __test__ = False
+
+    # TEST_DATA must be overridden by subclasses
+    TEST_DATA = None
+
+    def setUp(self):
+        """
+        Create a test client, course, and user.
+        """
+        super(FieldOverridePerformanceTestCase, self).setUp()
+
+        self.request_factory = RequestFactory()
+        self.student = UserFactory.create()
+        self.request = self.request_factory.get("foo")
+        self.request.user = self.student
+        self.course = None
+
+        MakoMiddleware().process_request(self.request)
+
+    def setup_course(self, size):
+        """
+        Build a gradable course where each node has `size` children.
+        """
+        grading_policy = {
+            "GRADER": [
+                {
+                    "drop_count": 2,
+                    "min_count": 12,
+                    "short_label": "HW",
+                    "type": "Homework",
+                    "weight": 0.15
+                },
+                {
+                    "drop_count": 2,
+                    "min_count": 12,
+                    "type": "Lab",
+                    "weight": 0.15
+                },
+                {
+                    "drop_count": 0,
+                    "min_count": 1,
+                    "short_label": "Midterm",
+                    "type": "Midterm Exam",
+                    "weight": 0.3
+                },
+                {
+                    "drop_count": 0,
+                    "min_count": 1,
+                    "short_label": "Final",
+                    "type": "Final Exam",
+                    "weight": 0.4
+                }
+            ],
+            "GRADE_CUTOFFS": {
+                "Pass": 0.5
+            }
+        }
+
+        self.course = CourseFactory.create(
+            graded=True,
+            start=datetime.now(UTC),
+            grading_policy=grading_policy
+        )
+        self.populate_course(size)
+
+        CourseEnrollment.enroll(
+            self.student,
+            self.course.id
+        )
+
+    def grade_course(self, course):
+        """
+        Renders the progress page for the given course.
+        """
+        return progress(
+            self.request,
+            course_id=course.id.to_deprecated_string(),
+            student_id=self.student.id
+        )
+
+    def instrument_course_progress_render(self, dataset_index, queries, reads, xblocks):
+        """
+        Renders the progress page, instrumenting Mongo reads and SQL queries.
+        """
+        self.setup_course(dataset_index + 1)
+
+        # Switch to published-only mode to simulate the LMS
+        with self.settings(MODULESTORE_BRANCH='published-only'):
+            # Clear all caches before measuring
+            for cache in settings.CACHES:
+                get_cache(cache).clear()
+
+            # Refill the metadata inheritance cache
+            modulestore().get_course(self.course.id, depth=None)
+
+            # We clear the request cache to simulate a new request in the LMS.
+            RequestCache.clear_request_cache()
+
+            with self.assertNumQueries(queries):
+                with check_mongo_calls(reads):
+                    with check_sum_of_calls(XBlock, ['__init__'], xblocks):
+                        self.grade_course(self.course)
+
+    @ddt.data(*itertools.product(('no_overrides', 'ccx'), range(3)))
+    @ddt.unpack
+    @override_settings(
+        FIELD_OVERRIDE_PROVIDERS=(),
+    )
+    def test_field_overrides(self, overrides, dataset_index):
+        """
+        Test without any field overrides.
+        """
+        providers = {
+            'no_overrides': (),
+            'ccx': ('ccx.overrides.CustomCoursesForEdxOverrideProvider',)
+        }
+        with self.settings(FIELD_OVERRIDE_PROVIDERS=providers[overrides]):
+            queries, reads, xblocks = self.TEST_DATA[overrides][dataset_index]
+            self.instrument_course_progress_render(dataset_index, queries, reads, xblocks)
+
+
+class TestFieldOverrideMongoPerformance(FieldOverridePerformanceTestCase):
+    """
+    Test cases for instrumenting field overrides against the Mongo modulestore.
+    """
+    MODULESTORE = TEST_DATA_MONGO_MODULESTORE
+    __test__ = True
+
+    TEST_DATA = {
+        'no_overrides': [
+            (26, 7, 19), (134, 7, 131), (594, 7, 537)
+        ],
+        'ccx': [
+            (26, 7, 47), (134, 7, 455), (594, 7, 2037)
+        ],
+    }
+
+
+class TestFieldOverrideSplitPerformance(FieldOverridePerformanceTestCase):
+    """
+    Test cases for instrumenting field overrides against the Split modulestore.
+    """
+    MODULESTORE = TEST_DATA_SPLIT_MODULESTORE
+    __test__ = True
+
+    TEST_DATA = {
+        'no_overrides': [
+            (26, 4, 9), (134, 19, 54), (594, 84, 215)
+        ],
+        'ccx': [
+            (26, 4, 9), (134, 19, 54), (594, 84, 215)
+        ]
+    }

lms/djangoapps/ccx/tests/test_overrides.py

+# coding=UTF-8
 """
 tests for overrides
 """