From 02c24cb2597e24dd8fe9267b27cc73f8d4d178cf Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Sun, 26 Feb 2017 21:16:40 -0500 Subject: [PATCH] Block Structure query count and API changes --- .../course_api/blocks/tests/test_api.py | 49 ++++++++++++++----- .../transformers/tests/test_milestones.py | 4 +- lms/djangoapps/grades/tests/test_grades.py | 11 +++-- lms/djangoapps/grades/tests/test_tasks.py | 4 +- .../tests/test_tasks_helper.py | 2 +- 5 files changed, 47 insertions(+), 23 deletions(-) diff --git a/lms/djangoapps/course_api/blocks/tests/test_api.py b/lms/djangoapps/course_api/blocks/tests/test_api.py index c36531e59b..f8b393b48c 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_api.py +++ b/lms/djangoapps/course_api/blocks/tests/test_api.py @@ -4,8 +4,12 @@ Tests for Blocks api.py import ddt from django.test.client import RequestFactory +from itertools import product from openedx.core.djangoapps.content.block_structure.api import clear_course_from_cache +from openedx.core.djangoapps.content.block_structure.config import STORAGE_BACKING_FOR_CACHE +from openedx.core.djangoapps.content.block_structure.tests.helpers import override_config_setting + from student.tests.factories import UserFactory from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase @@ -121,26 +125,45 @@ class TestGetBlocksQueryCounts(SharedModuleStoreTestCase): with self.store.default_store(store_type): return SampleCourseFactory.create() - def _get_blocks(self, course, expected_mongo_queries): + def _get_blocks(self, course, expected_mongo_queries, expected_sql_queries): """ Verifies the number of expected queries when calling get_blocks on the given course. """ with check_mongo_calls(expected_mongo_queries): - with self.assertNumQueries(2): + with self.assertNumQueries(expected_sql_queries): get_blocks(self.request, course.location, self.user) - @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) - def test_query_counts_cached(self, store_type): - course = self._create_course(store_type) - self._get_blocks(course, expected_mongo_queries=0) - @ddt.data( - (ModuleStoreEnum.Type.mongo, 5), - (ModuleStoreEnum.Type.split, 3), + *product( + (ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split), + (True, False), + ) ) @ddt.unpack - def test_query_counts_uncached(self, store_type, expected_mongo_queries): - course = self._create_course(store_type) - clear_course_from_cache(course.id) - self._get_blocks(course, expected_mongo_queries) + def test_query_counts_cached(self, store_type, with_storage_backing): + with override_config_setting(STORAGE_BACKING_FOR_CACHE, active=with_storage_backing): + course = self._create_course(store_type) + self._get_blocks( + course, + expected_mongo_queries=0, + expected_sql_queries=4 if with_storage_backing else 3, + ) + + @ddt.data( + *product( + ((ModuleStoreEnum.Type.mongo, 5), (ModuleStoreEnum.Type.split, 3)), + (True, False), + ) + ) + @ddt.unpack + def test_query_counts_uncached(self, store_type_tuple, with_storage_backing): + store_type, expected_mongo_queries = store_type_tuple + with override_config_setting(STORAGE_BACKING_FOR_CACHE, active=with_storage_backing): + course = self._create_course(store_type) + clear_course_from_cache(course.id) + self._get_blocks( + course, + expected_mongo_queries, + expected_sql_queries=9 if with_storage_backing else 3, + ) diff --git a/lms/djangoapps/course_api/blocks/transformers/tests/test_milestones.py b/lms/djangoapps/course_api/blocks/transformers/tests/test_milestones.py index 98b98119e9..b11b683e59 100644 --- a/lms/djangoapps/course_api/blocks/transformers/tests/test_milestones.py +++ b/lms/djangoapps/course_api/blocks/transformers/tests/test_milestones.py @@ -158,7 +158,7 @@ class MilestonesTransformerTestCase(CourseStructureTestCase, MilestonesTestCaseM self.course.enable_subsection_gating = True self.setup_gated_section(self.blocks[gated_block_ref], self.blocks[gating_block_ref]) - with self.assertNumQueries(3): + with self.assertNumQueries(6): self.get_blocks_and_check_against_expected(self.user, expected_blocks_before_completion) # clear the request cache to simulate a new request @@ -174,7 +174,7 @@ class MilestonesTransformerTestCase(CourseStructureTestCase, MilestonesTestCaseM self.blocks[gating_block_child], self.user.id, ) - with self.assertNumQueries(2): + with self.assertNumQueries(5): self.get_blocks_and_check_against_expected(self.user, self.ALL_BLOCKS_EXCEPT_SPECIAL) def test_staff_access(self): diff --git a/lms/djangoapps/grades/tests/test_grades.py b/lms/djangoapps/grades/tests/test_grades.py index 1310ce2475..6f3194b421 100644 --- a/lms/djangoapps/grades/tests/test_grades.py +++ b/lms/djangoapps/grades/tests/test_grades.py @@ -71,11 +71,11 @@ class TestGradeIteration(SharedModuleStoreTestCase): """ with patch.object( BlockStructureFactory, - 'create_from_cache', - wraps=BlockStructureFactory.create_from_cache - ) as mock_create_from_cache: + 'create_from_store', + wraps=BlockStructureFactory.create_from_store + ) as mock_create_from_store: all_course_grades, all_errors = self._course_grades_and_errors_for(self.course, self.students) - self.assertEquals(mock_create_from_cache.call_count, 1) + self.assertEquals(mock_create_from_store.call_count, 1) self.assertEqual(len(all_errors), 0) for course_grade in all_course_grades.values(): @@ -100,7 +100,8 @@ class TestGradeIteration(SharedModuleStoreTestCase): else mock_course_grade.return_value for student in self.students ] - all_course_grades, all_errors = self._course_grades_and_errors_for(self.course, self.students) + with self.assertNumQueries(4): + all_course_grades, all_errors = self._course_grades_and_errors_for(self.course, self.students) self.assertEqual( all_errors, { diff --git a/lms/djangoapps/grades/tests/test_tasks.py b/lms/djangoapps/grades/tests/test_tasks.py index 6d6dc57a6b..1ddf4efc8c 100644 --- a/lms/djangoapps/grades/tests/test_tasks.py +++ b/lms/djangoapps/grades/tests/test_tasks.py @@ -136,8 +136,8 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase): self.set_up_course() self.assertTrue(PersistentGradesEnabledFlag.feature_enabled(self.course.id)) with patch( - 'openedx.core.lib.block_structure.factory.BlockStructureFactory.create_from_cache', - side_effect=BlockStructureNotFound, + 'openedx.core.lib.block_structure.factory.BlockStructureFactory.create_from_store', + side_effect=BlockStructureNotFound(self.course.location), ) as mock_block_structure_create: self._apply_recalculate_subsection_grade() self.assertEquals(mock_block_structure_create.call_count, 1) diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index c72b675454..5e0d875842 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -1773,7 +1773,7 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): 'failed': 3, 'skipped': 2 } - with self.assertNumQueries(166): + with self.assertNumQueries(169): self.assertCertificatesGenerated(task_input, expected_results) expected_results = {