diff --git a/cms/djangoapps/contentstore/views/tests/test_group_configurations.py b/cms/djangoapps/contentstore/views/tests/test_group_configurations.py index e87e601d38..5e04336d60 100644 --- a/cms/djangoapps/contentstore/views/tests/test_group_configurations.py +++ b/cms/djangoapps/contentstore/views/tests/test_group_configurations.py @@ -88,7 +88,7 @@ class HelperMethods(object): self.save_course() return (vertical, split_test) - def _create_problem_with_content_group(self, cid, group_id, name_suffix='', special_characters=''): + def _create_problem_with_content_group(self, cid, group_id, name_suffix='', special_characters='', orphan=False): """ Create a problem Assign content group to the problem. @@ -112,7 +112,8 @@ class HelperMethods(object): data={'metadata': group_access_content} ) - self.course.children.append(vertical.location) + if not orphan: + self.course.children.append(vertical.location) self.save_course() return vertical, problem @@ -682,24 +683,16 @@ class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods): @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_can_get_correct_usage_info_with_orphan(self, module_store_type): """ - Test if content group json updated successfully with usage information even if there is - an orphan in content group. + Test if content group json updated successfully with usage information + even if there is an orphan in content group. """ self.course = CourseFactory.create(default_store=module_store_type) self._add_user_partitions(count=1, scheme_id='cohort') - vertical, problem = self._create_problem_with_content_group(cid=0, group_id=1, name_suffix='0') + vertical, __ = self._create_problem_with_content_group(cid=0, group_id=1, name_suffix='0', orphan=True) - # Assert that there is no orphan in the course yet. - self.assertEqual(len(self.store.get_orphans(self.course.id)), 0) - - # Update problem(created earlier) to an orphan. - with self.store.branch_setting(ModuleStoreEnum.Branch.published_only): - vertical = self.store.get_item(vertical.location) - vertical.children.remove(problem.location) - self.store.update_item(vertical, self.user.id) - - # Assert that the problem is orphan now. - self.assertIn(problem.location, self.store.get_orphans(self.course.id)) + # Assert that there is an orphan in the course, and that it's the vertical + self.assertEqual(len(self.store.get_orphans(self.course.id)), 1) + self.assertIn(vertical.location, self.store.get_orphans(self.course.id)) # Get the expected content group information based on module store. if module_store_type == ModuleStoreEnum.Type.mongo: diff --git a/common/djangoapps/request_cache/__init__.py b/common/djangoapps/request_cache/__init__.py index aedc785cad..d38255bc59 100644 --- a/common/djangoapps/request_cache/__init__.py +++ b/common/djangoapps/request_cache/__init__.py @@ -8,6 +8,7 @@ is installed in order to clear the cache after each request. import logging from urlparse import urlparse +from celery.signals import task_postrun from django.conf import settings from django.test.client import RequestFactory @@ -17,6 +18,15 @@ from request_cache import middleware log = logging.getLogger(__name__) +@task_postrun.connect +def clear_request_cache(**kwargs): # pylint: disable=unused-argument + """ + Once a celery task completes, clear the request cache to + prevent memory leaks. + """ + middleware.RequestCache.clear_request_cache() + + def get_cache(name): """ Return the request cache named ``name``. @@ -50,8 +60,8 @@ def get_request_or_stub(): if request is None: log.warning( - "Could not retrieve the current request. " - "A stub request will be created instead using settings.SITE_NAME. " + "Could not retrieve the current request. " + "A stub request will be created instead using settings.SITE_NAME. " "This should be used *only* in test cases, never in production!" ) diff --git a/common/djangoapps/request_cache/tests.py b/common/djangoapps/request_cache/tests.py index 3883fe873b..1842a13250 100644 --- a/common/djangoapps/request_cache/tests.py +++ b/common/djangoapps/request_cache/tests.py @@ -1,10 +1,12 @@ """ Tests for the request cache. """ +from celery.task import task from django.conf import settings from django.test import TestCase from request_cache import get_request_or_stub +from xmodule.modulestore.django import modulestore class TestRequestCache(TestCase): @@ -13,8 +15,21 @@ class TestRequestCache(TestCase): """ def test_get_request_or_stub(self): - # Outside the context of the request, we should still get a request - # that allows us to build an absolute URI. + """ + Outside the context of the request, we should still get a request + that allows us to build an absolute URI. + """ stub = get_request_or_stub() expected_url = "http://{site_name}/foobar".format(site_name=settings.SITE_NAME) self.assertEqual(stub.build_absolute_uri("foobar"), expected_url) + + @task + def _dummy_task(self): + """ Create a task that adds stuff to the request cache. """ + cache = {"course_cache": "blah blah blah"} + modulestore().request_cache.data.update(cache) + + def test_clear_cache_celery(self): + """ Test that the request cache is cleared after a task is run. """ + self._dummy_task.apply(args=(self,)).get() + self.assertEqual(modulestore().request_cache.data, {}) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py index 4a91344a47..a1649dd8b2 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py @@ -630,8 +630,8 @@ class SplitModuleCourseTests(SplitModuleTest): @patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json) def test_get_courses_with_same_course_index(self, _from_json): """ - Test that if two courses pointing to same course index, - get_courses should return both. + Test that if two courses point to same course index, + `get_courses` should return both courses. """ courses = modulestore().get_courses(branch=BRANCH_NAME_DRAFT) # Should have gotten 3 draft courses. diff --git a/openedx/core/djangoapps/bookmarks/tests/test_models.py b/openedx/core/djangoapps/bookmarks/tests/test_models.py index e452a5a92e..2c3072ad5e 100644 --- a/openedx/core/djangoapps/bookmarks/tests/test_models.py +++ b/openedx/core/djangoapps/bookmarks/tests/test_models.py @@ -255,10 +255,10 @@ class BookmarkModelTests(BookmarksTestsBase): @ddt.data( (ModuleStoreEnum.Type.mongo, 'course', [], 3), - (ModuleStoreEnum.Type.mongo, 'chapter_1', [], 3), - (ModuleStoreEnum.Type.mongo, 'sequential_1', ['chapter_1'], 4), - (ModuleStoreEnum.Type.mongo, 'vertical_1', ['chapter_1', 'sequential_1'], 5), - (ModuleStoreEnum.Type.mongo, 'html_1', ['chapter_1', 'sequential_2', 'vertical_2'], 6), + (ModuleStoreEnum.Type.mongo, 'chapter_1', [], 4), + (ModuleStoreEnum.Type.mongo, 'sequential_1', ['chapter_1'], 6), + (ModuleStoreEnum.Type.mongo, 'vertical_1', ['chapter_1', 'sequential_1'], 8), + (ModuleStoreEnum.Type.mongo, 'html_1', ['chapter_1', 'sequential_2', 'vertical_2'], 10), (ModuleStoreEnum.Type.split, 'course', [], 3), (ModuleStoreEnum.Type.split, 'chapter_1', [], 2), (ModuleStoreEnum.Type.split, 'sequential_1', ['chapter_1'], 2), @@ -268,8 +268,9 @@ class BookmarkModelTests(BookmarksTestsBase): @ddt.unpack def test_path_and_queries_on_create(self, store_type, block_to_bookmark, ancestors_attrs, expected_mongo_calls): """ - In case of mongo, 1 query is used to fetch the block, and 2 by path_to_location(), and then - 1 query per parent in path is needed to fetch the parent blocks. + In case of mongo, 1 query is used to fetch the block, and 2 + by path_to_location(), and then 1 query per parent in path + is needed to fetch the parent blocks. """ self.setup_test_data(store_type)