From cd33165a522ca02960c1d4a68e18d3709648c569 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Wed, 23 Jul 2014 23:11:33 -0400 Subject: [PATCH] Unit test for import performance STUD-1994 --- .../contentstore/tests/test_course_listing.py | 4 +- .../contentstore/tests/test_import.py | 18 +++++ .../xmodule/modulestore/tests/factories.py | 73 +++++++++++++------ .../xmodule/modulestore/tests/test_publish.py | 17 +++-- 4 files changed, 79 insertions(+), 33 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_course_listing.py b/cms/djangoapps/contentstore/tests/test_course_listing.py index 94642b6591..ce6fd24791 100644 --- a/cms/djangoapps/contentstore/tests/test_course_listing.py +++ b/cms/djangoapps/contentstore/tests/test_course_listing.py @@ -200,10 +200,10 @@ class TestCourseListing(ModuleStoreTestCase): # Now count the db queries store = modulestore()._get_modulestore_by_type(ModuleStoreEnum.Type.mongo) - with check_mongo_calls(store.collection, USER_COURSES_COUNT): + with check_mongo_calls(store, USER_COURSES_COUNT): courses_list = _accessible_courses_list_from_groups(self.request) - with check_mongo_calls(store.collection, 1): + with check_mongo_calls(store, 1): courses_list = _accessible_courses_list(self.request) def test_get_course_list_with_same_course_id(self): diff --git a/cms/djangoapps/contentstore/tests/test_import.py b/cms/djangoapps/contentstore/tests/test_import.py index 478227d386..5d063ce3dd 100644 --- a/cms/djangoapps/contentstore/tests/test_import.py +++ b/cms/djangoapps/contentstore/tests/test_import.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- # pylint: disable=E1101 +# pylint: disable=protected-access """ Tests for import_from_xml using the mongo modulestore. """ @@ -10,8 +11,10 @@ from django.conf import settings import copy from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore from xmodule.contentstore.django import contentstore +from xmodule.modulestore.tests.factories import check_exact_number_of_calls, check_number_of_calls from opaque_keys.edx.locations import SlashSeparatedCourseKey, AssetLocation from xmodule.modulestore.xml_importer import import_from_xml from xmodule.exceptions import NotFoundError @@ -149,6 +152,21 @@ class ContentStoreImportTest(ModuleStoreTestCase): print "course tabs = {0}".format(course.tabs) self.assertEqual(course.tabs[2]['name'], 'Syllabus') + def test_import_performance_mongo(self): + store = modulestore()._get_modulestore_by_type(ModuleStoreEnum.Type.mongo) + + # we try to refresh the inheritance tree for each update_item in the import + with check_exact_number_of_calls(store, store.refresh_cached_metadata_inheritance_tree, 46): + + # the post-publish step loads each item in the subtree, which calls _get_cached_metadata_inheritance_tree + with check_exact_number_of_calls(store, store._get_cached_metadata_inheritance_tree, 22): + + # with bulk-edit in progress, the inheritance tree should be recomputed only at the end of the import + # NOTE: On Jenkins, with memcache enabled, the number of calls here is only 1. + # Locally, without memcache, the number of calls is actually 2 (once more during the publish step) + with check_number_of_calls(store, store._compute_metadata_inheritance_tree, 2): + self.load_test_import_course() + def test_rewrite_reference_list(self): module_store = modulestore() target_course_id = SlashSeparatedCourseKey('testX', 'conditional_copy', 'copy_run') diff --git a/common/lib/xmodule/xmodule/modulestore/tests/factories.py b/common/lib/xmodule/xmodule/modulestore/tests/factories.py index 8f5453d91e..eece8f252e 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/factories.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/factories.py @@ -9,7 +9,7 @@ from xblock.core import XBlock from xmodule.tabs import StaticTab from decorator import contextmanager from mock import Mock, patch -from nose.tools import assert_less_equal +from nose.tools import assert_less_equal, assert_greater_equal class Dummy(object): @@ -220,30 +220,57 @@ class ItemFactory(XModuleFactory): @contextmanager -def check_mongo_calls(mongo_store, max_finds=0, max_sends=None): +def check_exact_number_of_calls(object_with_method, method, num_calls, method_name=None): """ - Instruments the given store to count the number of calls to find (incl find_one) and the number - of calls to send_message which is for insert, update, and remove (if you provide max_sends). At the - end of the with statement, it compares the counts to the max_finds and max_sends using a simple - assertLessEqual. - - :param mongo_store: the MongoModulestore or subclass to watch - :param max_finds: the maximum number of find calls to allow - :param max_sends: If none, don't instrument the send calls. If non-none, count and compare to - the given int value. + Instruments the given method on the given object to verify the number of calls to the + method is exactly equal to 'num_calls'. """ - try: - find_wrap = Mock(wraps=mongo_store.collection.find) - wrap_patch = patch.object(mongo_store.collection, 'find', find_wrap) - wrap_patch.start() - if max_sends: - sends_wrap = Mock(wraps=mongo_store.database.connection._send_message) - sends_patch = patch.object(mongo_store.database.connection, '_send_message', sends_wrap) - sends_patch.start() + with check_number_of_calls(object_with_method, method, num_calls, num_calls, method_name): yield + + +@contextmanager +def check_number_of_calls(object_with_method, method, maximum_calls, minimum_calls=1, method_name=None): + """ + Instruments the given method on the given object to verify the number of calls to the method is + less than or equal to the expected maximum_calls and greater than or equal to the expected minimum_calls. + """ + method_wrap = Mock(wraps=method) + wrap_patch = patch.object(object_with_method, method_name or method.__name__, method_wrap) + + try: + wrap_patch.start() + yield + finally: wrap_patch.stop() - if max_sends: - sends_patch.stop() - assert_less_equal(sends_wrap.call_count, max_sends) - assert_less_equal(find_wrap.call_count, max_finds) + + # verify the counter actually worked by ensuring we have counted greater than (or equal to) the minimum calls + assert_greater_equal(method_wrap.call_count, minimum_calls) + + # now verify the number of actual calls is less than (or equal to) the expected maximum + assert_less_equal(method_wrap.call_count, maximum_calls) + + +@contextmanager +def check_mongo_calls(mongo_store, num_finds=0, num_sends=None): + """ + Instruments the given store to count the number of calls to find (incl find_one) and the number + of calls to send_message which is for insert, update, and remove (if you provide num_sends). At the + end of the with statement, it compares the counts to the num_finds and num_sends. + + :param mongo_store: the MongoModulestore or subclass to watch + :param num_finds: the exact number of find calls expected + :param num_sends: If none, don't instrument the send calls. If non-none, count and compare to + the given int value. + """ + with check_exact_number_of_calls(mongo_store.collection, mongo_store.collection.find, num_finds): + if num_sends: + with check_exact_number_of_calls( + mongo_store.database.connection, + mongo_store.database.connection._send_message, # pylint: disable=protected-access + num_sends, + ): + yield + else: + yield diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py b/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py index a69d718404..c62a072eff 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py @@ -19,23 +19,23 @@ class TestPublish(SplitWMongoCourseBoostrapper): # There are 12 created items and 7 parent updates # create course: finds: 1 to verify uniqueness, 1 to find parents # sends: 1 to create course, 1 to create overview - with check_mongo_calls(self.draft_mongo, 6, 2): + with check_mongo_calls(self.draft_mongo, 5, 2): super(TestPublish, self)._create_course(split=False) # 2 inserts (course and overview) # with bulk will delay all inheritance computations which won't be added into the mongo_calls with self.draft_mongo.bulk_write_operations(self.old_course_key): - # finds: 1 for parent to add child, 1 for parent to update edit info - # sends: 1 for insert, 2 for parent (add child, change edit info) - with check_mongo_calls(self.draft_mongo, 5, 3): + # finds: 1 for parent to add child + # sends: 1 for insert, 1 for parent (add child) + with check_mongo_calls(self.draft_mongo, 1, 2): self._create_item('chapter', 'Chapter1', {}, {'display_name': 'Chapter 1'}, 'course', 'runid', split=False) - with check_mongo_calls(self.draft_mongo, 5, 3): + with check_mongo_calls(self.draft_mongo, 2, 2): self._create_item('chapter', 'Chapter2', {}, {'display_name': 'Chapter 2'}, 'course', 'runid', split=False) # update info propagation is 2 levels. create looks for draft and then published and then creates - with check_mongo_calls(self.draft_mongo, 16, 8): + with check_mongo_calls(self.draft_mongo, 8, 6): self._create_item('vertical', 'Vert1', {}, {'display_name': 'Vertical 1'}, 'chapter', 'Chapter1', split=False) self._create_item('vertical', 'Vert2', {}, {'display_name': 'Vertical 2'}, 'chapter', 'Chapter1', split=False) - with check_mongo_calls(self.draft_mongo, 36, 36): + with check_mongo_calls(self.draft_mongo, 20, 12): self._create_item('html', 'Html1', "

Goodbye

", {'display_name': 'Parented Html'}, 'vertical', 'Vert1', split=False) self._create_item( 'discussion', 'Discussion1', @@ -62,7 +62,8 @@ class TestPublish(SplitWMongoCourseBoostrapper): 'vertical', 'Vert2', split=False ) - with check_mongo_calls(self.draft_mongo, 2, 2): + + with check_mongo_calls(self.draft_mongo, 0, 2): # 2 finds b/c looking for non-existent parents self._create_item('static_tab', 'staticuno', "

tab

", {'display_name': 'Tab uno'}, None, None, split=False) self._create_item('course_info', 'updates', "

  1. Sep 22

    test

", {}, None, None, split=False)