From 57cdbfa0134936e8b034e8def7e97390cac7c4c3 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Thu, 25 Jul 2024 15:42:40 -0400 Subject: [PATCH] test: Remove test variants that test without a cache. Since the cache is now always on, remove test cases that test the case when it's disabled. --- .../course_api/blocks/tests/test_api.py | 44 +++++++----------- .../content/block_structure/models.py | 4 +- .../block_structure/tests/test_manager.py | 22 ++++----- .../block_structure/tests/test_store.py | 45 +++++++------------ 4 files changed, 41 insertions(+), 74 deletions(-) diff --git a/lms/djangoapps/course_api/blocks/tests/test_api.py b/lms/djangoapps/course_api/blocks/tests/test_api.py index e6fd74463a..93e400d4a8 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_api.py +++ b/lms/djangoapps/course_api/blocks/tests/test_api.py @@ -3,16 +3,13 @@ Tests for Blocks api.py """ -from itertools import product from unittest.mock import patch import ddt from django.test.client import RequestFactory -from edx_toggles.toggles.testutils import override_waffle_switch from common.djangoapps.student.tests.factories import UserFactory 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 xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.tests.factories import SampleCourseFactory, check_mongo_calls # lint-amnesty, pylint: disable=wrong-import-order @@ -209,34 +206,25 @@ class TestGetBlocksQueryCounts(TestGetBlocksQueryCountsBase): Tests query counts for the get_blocks function. """ - @ddt.data( - *product( - (ModuleStoreEnum.Type.split, ), - (True, False), + @ddt.data(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, + expected_sql_queries=14, ) - ) - @ddt.unpack - def test_query_counts_cached(self, store_type, with_storage_backing): - with override_waffle_switch(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=14 if with_storage_backing else 13, - ) @ddt.data( - (ModuleStoreEnum.Type.split, 2, True, 24), - (ModuleStoreEnum.Type.split, 2, False, 14), + (ModuleStoreEnum.Type.split, 2, 24), ) @ddt.unpack - def test_query_counts_uncached(self, store_type, expected_mongo_queries, with_storage_backing, num_sql_queries): - with override_waffle_switch(STORAGE_BACKING_FOR_CACHE, active=with_storage_backing): - course = self._create_course(store_type) - clear_course_from_cache(course.id) + def test_query_counts_uncached(self, store_type, expected_mongo_queries, num_sql_queries): + course = self._create_course(store_type) + clear_course_from_cache(course.id) - self._get_blocks( - course, - expected_mongo_queries, - expected_sql_queries=num_sql_queries, - ) + self._get_blocks( + course, + expected_mongo_queries, + expected_sql_queries=num_sql_queries, + ) diff --git a/openedx/core/djangoapps/content/block_structure/models.py b/openedx/core/djangoapps/content/block_structure/models.py index 4872e09346..c2837e1a77 100644 --- a/openedx/core/djangoapps/content/block_structure/models.py +++ b/openedx/core/djangoapps/content/block_structure/models.py @@ -74,7 +74,6 @@ def _bs_model_storage(): # .. setting_default: None # .. setting_description: Specifies the storage used for storage-backed block structure cache. # For more information, check https://github.com/openedx/edx-platform/pull/14571. - # .. setting_warnings: Depends on `block_structure.storage_backing_for_cache`. storage_class = settings.BLOCK_STRUCTURES_SETTINGS.get('STORAGE_CLASS') # .. setting_name: BLOCK_STRUCTURES_SETTINGS['STORAGE_KWARGS'] @@ -82,8 +81,7 @@ def _bs_model_storage(): # .. setting_description: Specifies the keyword arguments needed to setup the storage, which # would be used for storage-backed block structure cache. # For more information, check https://github.com/openedx/edx-platform/pull/14571. - # .. setting_warnings: Depends on `BLOCK_STRUCTURES_SETTINGS['STORAGE_CLASS']` and on - # `block_structure.storage_backing_for_cache`. + # .. setting_warnings: Depends on `BLOCK_STRUCTURES_SETTINGS['STORAGE_CLASS']` storage_kwargs = settings.BLOCK_STRUCTURES_SETTINGS.get('STORAGE_KWARGS', {}) return get_storage(storage_class, **storage_kwargs) diff --git a/openedx/core/djangoapps/content/block_structure/tests/test_manager.py b/openedx/core/djangoapps/content/block_structure/tests/test_manager.py index 8e5b585ca8..acde986eee 100644 --- a/openedx/core/djangoapps/content/block_structure/tests/test_manager.py +++ b/openedx/core/djangoapps/content/block_structure/tests/test_manager.py @@ -5,10 +5,8 @@ Tests for manager.py import pytest import ddt from django.test import TestCase -from edx_toggles.toggles.testutils import override_waffle_switch from ..block_structure import BlockStructureBlockData -from ..config import STORAGE_BACKING_FOR_CACHE from ..exceptions import UsageKeyNotInBlockStructure from ..manager import BlockStructureManager from ..transformers import BlockStructureTransformers @@ -177,20 +175,18 @@ class TestBlockStructureManager(UsageKeyFactoryMixin, ChildrenMapTestMixin, Test self.collect_and_verify(expect_modulestore_called=False, expect_cache_updated=False) assert TestTransformer1.collect_call_count == 1 - @ddt.data(True, False) - def test_update_collected_if_needed(self, with_storage_backing): - with override_waffle_switch(STORAGE_BACKING_FOR_CACHE, active=with_storage_backing): - with mock_registered_transformers(self.registered_transformers): - assert TestTransformer1.collect_call_count == 0 + def test_update_collected_if_needed(self): + with mock_registered_transformers(self.registered_transformers): + assert TestTransformer1.collect_call_count == 0 - self.bs_manager.update_collected_if_needed() - assert TestTransformer1.collect_call_count == 1 + self.bs_manager.update_collected_if_needed() + assert TestTransformer1.collect_call_count == 1 - self.bs_manager.update_collected_if_needed() - expected_count = 1 if with_storage_backing else 2 - assert TestTransformer1.collect_call_count == expected_count + self.bs_manager.update_collected_if_needed() + expected_count = 1 + assert TestTransformer1.collect_call_count == expected_count - self.collect_and_verify(expect_modulestore_called=False, expect_cache_updated=False) + self.collect_and_verify(expect_modulestore_called=False, expect_cache_updated=False) def test_get_collected_transformer_version(self): self.collect_and_verify(expect_modulestore_called=True, expect_cache_updated=True) diff --git a/openedx/core/djangoapps/content/block_structure/tests/test_store.py b/openedx/core/djangoapps/content/block_structure/tests/test_store.py index 8d6fc8017d..63a02efa7a 100644 --- a/openedx/core/djangoapps/content/block_structure/tests/test_store.py +++ b/openedx/core/djangoapps/content/block_structure/tests/test_store.py @@ -4,11 +4,9 @@ Tests for block_structure/cache.py import pytest import ddt -from edx_toggles.toggles.testutils import override_waffle_switch from openedx.core.djangolib.testing.utils import CacheIsolationTestCase -from ..config import STORAGE_BACKING_FOR_CACHE from ..config.models import BlockStructureConfiguration from ..exceptions import BlockStructureNotFound from ..store import BlockStructureStore @@ -46,40 +44,27 @@ class TestBlockStructureStore(UsageKeyFactoryMixin, ChildrenMapTestMixin, CacheI value=f'{transformer.name()} val', ) - @ddt.data(True, False) - def test_get_none(self, with_storage_backing): - with override_waffle_switch(STORAGE_BACKING_FOR_CACHE, active=with_storage_backing): - with pytest.raises(BlockStructureNotFound): - self.store.get(self.block_structure.root_block_usage_key) + def test_get_none(self): + with pytest.raises(BlockStructureNotFound): + self.store.get(self.block_structure.root_block_usage_key) - @ddt.data(True, False) - def test_add_and_get(self, with_storage_backing): - with override_waffle_switch(STORAGE_BACKING_FOR_CACHE, active=with_storage_backing): - self.store.add(self.block_structure) - stored_value = self.store.get(self.block_structure.root_block_usage_key) - assert stored_value is not None - self.assert_block_structure(stored_value, self.children_map) - - @ddt.data(True, False) - def test_delete(self, with_storage_backing): - with override_waffle_switch(STORAGE_BACKING_FOR_CACHE, active=with_storage_backing): - self.store.add(self.block_structure) - self.store.delete(self.block_structure.root_block_usage_key) - with pytest.raises(BlockStructureNotFound): - self.store.get(self.block_structure.root_block_usage_key) - - def test_uncached_without_storage(self): + def test_add_and_get(self): self.store.add(self.block_structure) - self.mock_cache.map.clear() + stored_value = self.store.get(self.block_structure.root_block_usage_key) + assert stored_value is not None + self.assert_block_structure(stored_value, self.children_map) + + def test_delete(self): + self.store.add(self.block_structure) + self.store.delete(self.block_structure.root_block_usage_key) with pytest.raises(BlockStructureNotFound): self.store.get(self.block_structure.root_block_usage_key) def test_uncached_with_storage(self): - with override_waffle_switch(STORAGE_BACKING_FOR_CACHE, active=True): - self.store.add(self.block_structure) - self.mock_cache.map.clear() - stored_value = self.store.get(self.block_structure.root_block_usage_key) - self.assert_block_structure(stored_value, self.children_map) + self.store.add(self.block_structure) + self.mock_cache.map.clear() + stored_value = self.store.get(self.block_structure.root_block_usage_key) + self.assert_block_structure(stored_value, self.children_map) @ddt.data(1, 5, None) def test_cache_timeout(self, timeout):