From f9a85c4471f6fc64996bc44a85a860527bd46778 Mon Sep 17 00:00:00 2001 From: John Eskew Date: Mon, 21 Dec 2015 12:28:59 -0500 Subject: [PATCH 01/22] Remove XML modulestore code - discovery work --- .../tests/test_courseware_index.py | 9 ----- .../lib/xmodule/xmodule/modulestore/mixed.py | 11 +------ .../xmodule/modulestore/tests/django_utils.py | 25 -------------- .../tests/test_mixed_modulestore.py | 9 ----- .../courseware/tests/test_module_render.py | 33 ++----------------- lms/djangoapps/courseware/tests/tests.py | 19 ----------- lms/envs/common.py | 8 ----- 7 files changed, 3 insertions(+), 111 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_courseware_index.py b/cms/djangoapps/contentstore/tests/test_courseware_index.py index d3d2205cea..7f1e9adfc6 100644 --- a/cms/djangoapps/contentstore/tests/test_courseware_index.py +++ b/cms/djangoapps/contentstore/tests/test_courseware_index.py @@ -124,15 +124,6 @@ class MixedWithOptionsTestCase(MixedSplitTestCase): 'DOC_STORE_CONFIG': DOC_STORE_CONFIG, 'OPTIONS': modulestore_options }, - { - 'NAME': 'xml', - 'ENGINE': 'xmodule.modulestore.xml.XMLModuleStore', - 'OPTIONS': { - 'data_dir': DATA_DIR, - 'default_class': 'xmodule.hidden_module.HiddenDescriptor', - 'xblock_mixins': modulestore_options['xblock_mixins'], - } - }, ], 'xblock_mixins': modulestore_options['xblock_mixins'], } diff --git a/common/lib/xmodule/xmodule/modulestore/mixed.py b/common/lib/xmodule/xmodule/modulestore/mixed.py index adceb85aa6..78eb0fd36c 100644 --- a/common/lib/xmodule/xmodule/modulestore/mixed.py +++ b/common/lib/xmodule/xmodule/modulestore/mixed.py @@ -1,7 +1,7 @@ """ MixedModuleStore allows for aggregation between multiple modulestores. -In this way, courses can be served up both - say - XMLModuleStore or MongoModuleStore +In this way, courses can be served up via either SplitMongoModuleStore or MongoModuleStore. """ @@ -169,15 +169,6 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase): for store_settings in stores: key = store_settings['NAME'] - is_xml = 'XMLModuleStore' in store_settings['ENGINE'] - if is_xml: - # restrict xml to only load courses in mapping - store_settings['OPTIONS']['course_ids'] = [ - course_key.to_deprecated_string() - for course_key, store_key in self.mappings.iteritems() - if store_key == key - ] - store = create_modulestore_instance( store_settings['ENGINE'], self.contentstore, diff --git a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py index 5d5859fb90..101e909e96 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py @@ -140,28 +140,6 @@ def split_mongo_store_config(data_dir): return store -def xml_store_config(data_dir, source_dirs=None): - """ - Defines default module store using XMLModuleStore. - - Note: you should pass in a list of source_dirs that you care about, - otherwise all courses in the data_dir will be processed. - """ - store = { - 'default': { - 'NAME': 'xml', - 'ENGINE': 'xmodule.modulestore.xml.XMLModuleStore', - 'OPTIONS': { - 'data_dir': data_dir, - 'default_class': 'xmodule.hidden_module.HiddenDescriptor', - 'source_dirs': source_dirs, - } - } - } - - return store - - @patch('xmodule.modulestore.django.create_modulestore_instance', autospec=True) def drop_mongo_collections(mock_create): """ @@ -180,9 +158,6 @@ def drop_mongo_collections(mock_create): TEST_DATA_DIR = settings.COMMON_TEST_DATA_ROOT -# This is an XML only modulestore with only the toy course loaded -TEST_DATA_XML_MODULESTORE = xml_store_config(TEST_DATA_DIR, source_dirs=['toy']) - # This modulestore will provide both a mixed mongo editable modulestore, and # an XML store with just the toy course loaded. TEST_DATA_MIXED_TOY_MODULESTORE = mixed_store_config( diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py index dbe4a21f03..b296461818 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -110,15 +110,6 @@ class CommonMixedModuleStoreSetup(CourseComparisonTest): 'DOC_STORE_CONFIG': DOC_STORE_CONFIG, 'OPTIONS': modulestore_options }, - { - 'NAME': ModuleStoreEnum.Type.xml, - 'ENGINE': 'xmodule.modulestore.xml.XMLModuleStore', - 'OPTIONS': { - 'data_dir': DATA_DIR, - 'default_class': 'xmodule.hidden_module.HiddenDescriptor', - 'xblock_mixins': modulestore_options['xblock_mixins'], - } - }, ], 'xblock_mixins': modulestore_options['xblock_mixins'], } diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index 0c1224667d..ff8a7e0b90 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -44,8 +44,8 @@ from openedx.core.lib.gating import api as gating_api from student.models import anonymous_id_for_user from xmodule.modulestore.tests.django_utils import ( TEST_DATA_MIXED_TOY_MODULESTORE, - TEST_DATA_XML_MODULESTORE, - SharedModuleStoreTestCase) + SharedModuleStoreTestCase +) from xmodule.lti_module import LTIDescriptor from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore @@ -1369,17 +1369,6 @@ class ViewInStudioTest(ModuleStoreTestCase): # pylint: disable=attribute-defined-outside-init self.child_module = self._get_module(course.id, child_descriptor, child_descriptor.location) - def setup_xml_course(self): - """ - Define the XML backed course to use. - Toy courses are already loaded in XML and mixed modulestores. - """ - course_key = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall') - location = course_key.make_usage_key('chapter', 'Overview') - descriptor = modulestore().get_item(location) - - self.module = self._get_module(course_key, descriptor, location) - @attr('shard_1') class MongoViewInStudioTest(ViewInStudioTest): @@ -1428,24 +1417,6 @@ class MixedViewInStudioTest(ViewInStudioTest): result_fragment = self.module.render(STUDENT_VIEW, context=self.default_context) self.assertNotIn('View Unit in Studio', result_fragment.content) - def test_view_in_studio_link_xml_backed(self): - """Course in XML only modulestore should not see 'View in Studio' links.""" - self.setup_xml_course() - result_fragment = self.module.render(STUDENT_VIEW, context=self.default_context) - self.assertNotIn('View Unit in Studio', result_fragment.content) - - -@attr('shard_1') -class XmlViewInStudioTest(ViewInStudioTest): - """Test the 'View in Studio' link visibility in an xml backed course.""" - MODULESTORE = TEST_DATA_XML_MODULESTORE - - def test_view_in_studio_link_xml_backed(self): - """Course in XML only modulestore should not see 'View in Studio' links.""" - self.setup_xml_course() - result_fragment = self.module.render(STUDENT_VIEW) - self.assertNotIn('View Unit in Studio', result_fragment.content) - @XBlock.tag("detached") class DetachedXBlock(XBlock): diff --git a/lms/djangoapps/courseware/tests/tests.py b/lms/djangoapps/courseware/tests/tests.py index 85cb97b423..32e658d9aa 100644 --- a/lms/djangoapps/courseware/tests/tests.py +++ b/lms/djangoapps/courseware/tests/tests.py @@ -10,7 +10,6 @@ from nose.plugins.attrib import attr from opaque_keys.edx.locations import SlashSeparatedCourseKey from courseware.tests.helpers import LoginEnrollmentTestCase -from xmodule.modulestore.tests.django_utils import TEST_DATA_XML_MODULESTORE as XML_MODULESTORE from xmodule.modulestore.tests.django_utils import TEST_DATA_MIXED_TOY_MODULESTORE as TOY_MODULESTORE from lms.djangoapps.lms_xblock.field_data import LmsFieldData from xmodule.error_module import ErrorDescriptor @@ -122,24 +121,6 @@ class PageLoaderTestCase(LoginEnrollmentTestCase): self.assertNotIsInstance(descriptor, ErrorDescriptor) -@attr('shard_1') -class TestXmlCoursesLoad(ModuleStoreTestCase, PageLoaderTestCase): - """ - Check that all pages in test courses load properly from XML. - """ - MODULESTORE = XML_MODULESTORE - - def setUp(self): - super(TestXmlCoursesLoad, self).setUp() - self.setup_user() - - def test_toy_course_loads(self): - # Load one of the XML based courses - # Our test mapping rules allow the MixedModuleStore - # to load this course from XML, not Mongo. - self.check_all_pages_load(SlashSeparatedCourseKey('edX', 'toy', '2012_Fall')) - - @attr('shard_1') class TestMongoCoursesLoad(ModuleStoreTestCase, PageLoaderTestCase): """ diff --git a/lms/envs/common.py b/lms/envs/common.py index 122143deeb..6f144030ea 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -735,14 +735,6 @@ MODULESTORE = { 'fs_root': DATA_DIR, 'render_template': 'edxmako.shortcuts.render_to_string', } - }, - { - 'NAME': 'xml', - 'ENGINE': 'xmodule.modulestore.xml.XMLModuleStore', - 'OPTIONS': { - 'data_dir': DATA_DIR, - 'default_class': 'xmodule.hidden_module.HiddenDescriptor', - } } ] } From c4535afb3fe7c06335ac2a0a96a6effa810120b7 Mon Sep 17 00:00:00 2001 From: John Eskew Date: Mon, 21 Dec 2015 14:29:26 -0500 Subject: [PATCH 02/22] Remove more XML modulestore test code. --- .../xmodule/modulestore/tests/django_utils.py | 5 +- .../tests/test_mixed_modulestore.py | 162 ++---------------- 2 files changed, 12 insertions(+), 155 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py index 101e909e96..861e03fe23 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py @@ -29,13 +29,13 @@ from openedx.core.djangoapps.bookmarks.signals import trigger_update_xblocks_cac class StoreConstructors(object): """Enumeration of store constructor types.""" - draft, split, xml = range(3) + draft, split = range(2) def mixed_store_config(data_dir, mappings, include_xml=False, xml_source_dirs=None, store_order=None): """ Return a `MixedModuleStore` configuration, which provides - access to both Mongo- and XML-backed courses. + access to both Mongo-backed courses. Args: data_dir (string): the directory from which to load XML-backed courses. @@ -70,7 +70,6 @@ def mixed_store_config(data_dir, mappings, include_xml=False, xml_source_dirs=No store_constructors = { StoreConstructors.split: split_mongo_store_config(data_dir)['default'], StoreConstructors.draft: draft_mongo_store_config(data_dir)['default'], - StoreConstructors.xml: xml_store_config(data_dir, source_dirs=xml_source_dirs)['default'], } store = { diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py index b296461818..b31785cfce 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -74,9 +74,6 @@ class CommonMixedModuleStoreSetup(CourseComparisonTest): RENDER_TEMPLATE = lambda t_n, d, ctx=None, nsp='main': '' MONGO_COURSEID = 'MITx/999/2013_Spring' - XML_COURSEID1 = 'edX/toy/2012_Fall' - XML_COURSEID2 = 'edX/simple/2012_Fall' - BAD_COURSE_ID = 'edX/simple' modulestore_options = { 'default_class': DEFAULT_CLASS, @@ -91,11 +88,7 @@ class CommonMixedModuleStoreSetup(CourseComparisonTest): 'collection': COLLECTION, 'asset_collection': ASSET_COLLECTION, } - MAPPINGS = { - XML_COURSEID1: 'xml', - XML_COURSEID2: 'xml', - BAD_COURSE_ID: 'xml', - } + MAPPINGS = {} OPTIONS = { 'stores': [ { @@ -148,7 +141,7 @@ class CommonMixedModuleStoreSetup(CourseComparisonTest): self.addTypeEqualityFunc(BlockUsageLocator, '_compare_ignore_version') self.addTypeEqualityFunc(CourseLocator, '_compare_ignore_version') # define attrs which get set in initdb to quell pylint - self.writable_chapter_location = self.store = self.fake_location = self.xml_chapter_location = None + self.writable_chapter_location = self.store = self.fake_location = None self.course_locations = {} self.user_id = ModuleStoreEnum.UserID.test @@ -275,7 +268,7 @@ class CommonMixedModuleStoreSetup(CourseComparisonTest): # convert to CourseKeys self.course_locations = { course_id: CourseLocator.from_string(course_id) - for course_id in [self.MONGO_COURSEID, self.XML_COURSEID1, self.XML_COURSEID2] + for course_id in [self.MONGO_COURSEID] } # and then to the root UsageKey self.course_locations = { @@ -286,10 +279,6 @@ class CommonMixedModuleStoreSetup(CourseComparisonTest): mongo_course_key = self.course_locations[self.MONGO_COURSEID].course_key self.fake_location = self.store.make_course_key(mongo_course_key.org, mongo_course_key.course, mongo_course_key.run).make_usage_key('vertical', 'fake') - self.xml_chapter_location = self.course_locations[self.XML_COURSEID1].replace( - category='chapter', name='Overview' - ) - self._create_course(self.course_locations[self.MONGO_COURSEID].course_key) self.assertEquals(default, self.store.get_modulestore_type(self.course.id)) @@ -337,12 +326,6 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): Make sure we get back the store type we expect for given mappings """ self.initdb(default_ms) - self.assertEqual(self.store.get_modulestore_type( - self._course_key_from_string(self.XML_COURSEID1)), ModuleStoreEnum.Type.xml - ) - self.assertEqual(self.store.get_modulestore_type( - self._course_key_from_string(self.XML_COURSEID2)), ModuleStoreEnum.Type.xml - ) self.assertEqual(self.store.get_modulestore_type( self._course_key_from_string(self.MONGO_COURSEID)), default_ms ) @@ -392,15 +375,10 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): self.initdb(default_ms) self._create_block_hierarchy() - self.assertTrue(self.store.has_item(self.course_locations[self.XML_COURSEID1])) - with check_mongo_calls(max_find.pop(0), max_send): self.assertTrue(self.store.has_item(self.problem_x1a_1)) # try negative cases - self.assertFalse(self.store.has_item( - self.course_locations[self.XML_COURSEID1].replace(name='not_findable', category='problem') - )) with check_mongo_calls(max_find.pop(0), max_send): self.assertFalse(self.store.has_item(self.fake_location)) @@ -420,16 +398,10 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): self.initdb(default_ms) self._create_block_hierarchy() - self.assertIsNotNone(self.store.get_item(self.course_locations[self.XML_COURSEID1])) - with check_mongo_calls(max_find.pop(0), max_send): self.assertIsNotNone(self.store.get_item(self.problem_x1a_1)) # try negative cases - with self.assertRaises(ItemNotFoundError): - self.store.get_item( - self.course_locations[self.XML_COURSEID1].replace(name='not_findable', category='problem') - ) with check_mongo_calls(max_find.pop(0), max_send): with self.assertRaises(ItemNotFoundError): self.store.get_item(self.fake_location) @@ -448,12 +420,6 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): self.initdb(default_ms) self._create_block_hierarchy() - course_locn = self.course_locations[self.XML_COURSEID1] - # NOTE: use get_course if you just want the course. get_items is expensive - modules = self.store.get_items(course_locn.course_key, qualifiers={'category': 'course'}) - self.assertEqual(len(modules), 1) - self.assertEqual(modules[0].location, course_locn) - course_locn = self.course_locations[self.MONGO_COURSEID] with check_mongo_calls(max_find, max_send): modules = self.store.get_items(course_locn.course_key, qualifiers={'category': 'problem'}) @@ -536,18 +502,10 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): @ddt.unpack def test_update_item(self, default_ms, max_find, max_send): """ - Update should fail for r/o dbs and succeed for r/w ones + Update should succeed for r/w dbs """ self.initdb(default_ms) self._create_block_hierarchy() - course = self.store.get_course(self.course_locations[self.XML_COURSEID1].course_key) - # if following raised, then the test is really a noop, change it - self.assertFalse(course.show_calculator, "Default changed making test meaningless") - course.show_calculator = True - with self.assertRaises(NotImplementedError): # ensure it doesn't allow writing - self.store.update_item(course, self.user_id) - - # now do it for a r/w db problem = self.store.get_item(self.problem_x1a_1) # if following raised, then the test is really a noop, change it self.assertNotEqual(problem.max_attempts, 2, "Default changed making test meaningless") @@ -944,10 +902,6 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): if default_ms == ModuleStoreEnum.Type.mongo and mongo_uses_error_check(self.store): max_find += 1 - # r/o try deleting the chapter (is here to ensure it can't be deleted) - with self.assertRaises(NotImplementedError): - self.store.delete_item(self.xml_chapter_location, self.user_id) - with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, self.writable_chapter_location.course_key): with check_mongo_calls(max_find, max_send): self.store.delete_item(self.writable_chapter_location, self.user_id) @@ -1066,14 +1020,12 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): @ddt.unpack def test_get_courses(self, default_ms, max_find, max_send): self.initdb(default_ms) - # we should have 3 total courses across all stores + # we should have one course across all stores with check_mongo_calls(max_find, max_send): courses = self.store.get_courses() course_ids = [course.location for course in courses] - self.assertEqual(len(courses), 3, "Not 3 courses: {}".format(course_ids)) + self.assertEqual(len(courses), 1, "Not one course: {}".format(course_ids)) self.assertIn(self.course_locations[self.MONGO_COURSEID], course_ids) - self.assertIn(self.course_locations[self.XML_COURSEID1], course_ids) - self.assertIn(self.course_locations[self.XML_COURSEID2], course_ids) with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred): draft_courses = self.store.get_courses(remove_branch=True) @@ -1102,30 +1054,6 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): mongo_course = self.store.get_course(self.course_locations[self.MONGO_COURSEID].course_key) self.assertEqual(len(mongo_course.children), 1) - def test_xml_get_courses(self): - """ - Test that the xml modulestore only loaded the courses from the maps. - """ - self.initdb(ModuleStoreEnum.Type.mongo) - xml_store = self.store._get_modulestore_by_type(ModuleStoreEnum.Type.xml) # pylint: disable=protected-access - courses = xml_store.get_courses() - self.assertEqual(len(courses), 2) - course_ids = [course.id for course in courses] - self.assertIn(self.course_locations[self.XML_COURSEID1].course_key, course_ids) - self.assertIn(self.course_locations[self.XML_COURSEID2].course_key, course_ids) - # this course is in the directory from which we loaded courses but not in the map - self.assertNotIn("edX/toy/TT_2012_Fall", course_ids) - - def test_xml_no_write(self): - """ - Test that the xml modulestore doesn't allow write ops. - """ - self.initdb(ModuleStoreEnum.Type.mongo) - xml_store = self.store._get_modulestore_by_type(ModuleStoreEnum.Type.xml) # pylint: disable=protected-access - # the important thing is not which exception it raises but that it raises an exception - with self.assertRaises(AttributeError): - xml_store.create_course("org", "course", "run", self.user_id) - # draft is 2: find out which ms owns course, get item # split: active_versions, structure, definition (to load course wiki string) @ddt.data((ModuleStoreEnum.Type.mongo, 2, 0), (ModuleStoreEnum.Type.split, 3, 0)) @@ -1140,9 +1068,6 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): course = self.store.get_item(self.course_locations[self.MONGO_COURSEID]) self.assertEqual(course.id, self.course_locations[self.MONGO_COURSEID].course_key) - course = self.store.get_item(self.course_locations[self.XML_COURSEID1]) - self.assertEqual(course.id, self.course_locations[self.XML_COURSEID1].course_key) - @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_get_library(self, default_ms): """ @@ -1181,9 +1106,6 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): parent = self.store.get_parent_location(self.problem_x1a_1) self.assertEqual(parent, self.vertical_x1a) - parent = self.store.get_parent_location(self.xml_chapter_location) - self.assertEqual(parent, self.course_locations[self.XML_COURSEID1]) - def verify_get_parent_locations_results(self, expected_results): """ Verifies the results of calling get_parent_locations matches expected_results. @@ -1364,34 +1286,6 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): with self.assertRaises(NoPathToItem): path_to_location(self.store, orphan) - def test_xml_path_to_location(self): - """ - Make sure that path_to_location works: should be passed a modulestore - with the toy and simple courses loaded. - """ - # only needs course_locations set - self.initdb(ModuleStoreEnum.Type.mongo) - course_key = self.course_locations[self.XML_COURSEID1].course_key - video_key = course_key.make_usage_key('video', 'Welcome') - chapter_key = course_key.make_usage_key('chapter', 'Overview') - should_work = ( - (video_key, - (course_key, "Overview", "Welcome", None, None, video_key)), - (chapter_key, - (course_key, "Overview", None, None, None, chapter_key)), - ) - - for location, expected in should_work: - self.assertEqual(path_to_location(self.store, location), expected) - - not_found = ( - course_key.make_usage_key('video', 'WelcomeX'), - course_key.make_usage_key('course', 'NotHome'), - ) - for location in not_found: - with self.assertRaises(ItemNotFoundError): - path_to_location(self.store, location) - def test_navigation_index(self): """ Make sure that navigation_index correctly parses the various position values that we might get from calls to @@ -1643,15 +1537,6 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): Test the get_courses_for_wiki method """ self.initdb(default_ms) - # Test XML wikis - wiki_courses = self.store.get_courses_for_wiki('toy') - self.assertEqual(len(wiki_courses), 1) - self.assertIn(self.course_locations[self.XML_COURSEID1].course_key, wiki_courses) - - wiki_courses = self.store.get_courses_for_wiki('simple') - self.assertEqual(len(wiki_courses), 1) - self.assertIn(self.course_locations[self.XML_COURSEID2].course_key, wiki_courses) - # Test Mongo wiki with check_mongo_calls(max_find, max_send): wiki_courses = self.store.get_courses_for_wiki('999') @@ -1986,14 +1871,13 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): wiki_courses = self.store.get_courses_for_wiki('999') self.assertEqual(len(wiki_courses), 0) - # but there should be two courses with wiki_slug 'simple' + # but there should be one course with wiki_slug 'simple' wiki_courses = self.store.get_courses_for_wiki('simple') - self.assertEqual(len(wiki_courses), 2) + self.assertEqual(len(wiki_courses), 1) self.assertIn( self.course_locations[self.MONGO_COURSEID].course_key.replace(branch=None), wiki_courses ) - self.assertIn(self.course_locations[self.XML_COURSEID2].course_key, wiki_courses) # configure mongo course to use unique wiki_slug. mongo_course = self.store.get_course(self.course_locations[self.MONGO_COURSEID].course_key) @@ -2008,15 +1892,11 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): ) # and NOT retriveable with its old wiki_slug wiki_courses = self.store.get_courses_for_wiki('simple') - self.assertEqual(len(wiki_courses), 1) + self.assertEqual(len(wiki_courses), 0) self.assertNotIn( self.course_locations[self.MONGO_COURSEID].course_key.replace(branch=None), wiki_courses ) - self.assertIn( - self.course_locations[self.XML_COURSEID2].course_key, - wiki_courses - ) @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_branch_setting(self, default_ms): @@ -2117,7 +1997,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): except NotImplementedError: self.assertEquals(store_type, ModuleStoreEnum.Type.xml) - @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split, ModuleStoreEnum.Type.xml) + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_default_store(self, default_ms): """ Test the default store context manager @@ -2139,9 +2019,6 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): self.verify_default_store(ModuleStoreEnum.Type.mongo) with self.store.default_store(ModuleStoreEnum.Type.split): self.verify_default_store(ModuleStoreEnum.Type.split) - with self.store.default_store(ModuleStoreEnum.Type.xml): - self.verify_default_store(ModuleStoreEnum.Type.xml) - self.verify_default_store(ModuleStoreEnum.Type.split) self.verify_default_store(ModuleStoreEnum.Type.mongo) def test_default_store_fake(self): @@ -2196,25 +2073,6 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): dest_store = self.store._get_modulestore_by_type(destination_modulestore) self.assertCoursesEqual(source_store, source_course_key, dest_store, dest_course_id) - def test_clone_xml_split(self): - """ - Can clone xml courses to split; so, test it. - """ - with MongoContentstoreBuilder().build() as contentstore: - # initialize the mixed modulestore - self._initialize_mixed(contentstore=contentstore, mappings={self.XML_COURSEID2: 'xml', }) - source_course_key = CourseKey.from_string(self.XML_COURSEID2) - with self.store.default_store(ModuleStoreEnum.Type.split): - dest_course_id = CourseLocator("org.other", "course.other", "run.other") - self.store.clone_course( - source_course_key, dest_course_id, ModuleStoreEnum.UserID.test - ) - - # pylint: disable=protected-access - source_store = self.store._get_modulestore_by_type(ModuleStoreEnum.Type.xml) - dest_store = self.store._get_modulestore_by_type(ModuleStoreEnum.Type.split) - self.assertCoursesEqual(source_store, source_course_key, dest_store, dest_course_id) - @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_bulk_operations_signal_firing(self, default): """ Signals should be fired right before bulk_operations() exits. """ From 9cc1d4f91dac844a9907a14b89ab5e06fc732585 Mon Sep 17 00:00:00 2001 From: John Eskew Date: Mon, 21 Dec 2015 14:53:35 -0500 Subject: [PATCH 03/22] Remove include_xml option. --- common/djangoapps/embargo/tests/test_api.py | 4 +--- .../xmodule/modulestore/tests/django_utils.py | 20 +++++++------------ .../commands/tests/test_dump_course.py | 1 - 3 files changed, 8 insertions(+), 17 deletions(-) diff --git a/common/djangoapps/embargo/tests/test_api.py b/common/djangoapps/embargo/tests/test_api.py index e48940fe10..52fb829596 100644 --- a/common/djangoapps/embargo/tests/test_api.py +++ b/common/djangoapps/embargo/tests/test_api.py @@ -34,9 +34,7 @@ from embargo.exceptions import InvalidAccessPoint from mock import patch -# Since we don't need any XML course fixtures, use a modulestore configuration -# that disables the XML modulestore. -MODULESTORE_CONFIG = mixed_store_config(settings.COMMON_TEST_DATA_ROOT, {}, include_xml=False) +MODULESTORE_CONFIG = mixed_store_config(settings.COMMON_TEST_DATA_ROOT, {}) @ddt.ddt diff --git a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py index 861e03fe23..286810069a 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py @@ -32,7 +32,7 @@ class StoreConstructors(object): draft, split = range(2) -def mixed_store_config(data_dir, mappings, include_xml=False, xml_source_dirs=None, store_order=None): +def mixed_store_config(data_dir, mappings, xml_source_dirs=None, store_order=None): """ Return a `MixedModuleStore` configuration, which provides access to both Mongo-backed courses. @@ -51,11 +51,9 @@ def mixed_store_config(data_dir, mappings, include_xml=False, xml_source_dirs=No Keyword Args: - include_xml (boolean): If True, include an XML modulestore in the configuration. xml_source_dirs (list): The directories containing XML courses to load from disk. note: For the courses to be loaded into the XML modulestore and accessible do the following: - * include_xml should be True * xml_source_dirs should be the list of directories (relative to data_dir) containing the courses you want to load * mappings should be configured, pointing the xml courses to the xml modulestore @@ -64,9 +62,6 @@ def mixed_store_config(data_dir, mappings, include_xml=False, xml_source_dirs=No if store_order is None: store_order = [StoreConstructors.draft, StoreConstructors.split] - if include_xml and StoreConstructors.xml not in store_order: - store_order.append(StoreConstructors.xml) - store_constructors = { StoreConstructors.split: split_mongo_store_config(data_dir)['default'], StoreConstructors.draft: draft_mongo_store_config(data_dir)['default'], @@ -160,25 +155,25 @@ TEST_DATA_DIR = settings.COMMON_TEST_DATA_ROOT # This modulestore will provide both a mixed mongo editable modulestore, and # an XML store with just the toy course loaded. TEST_DATA_MIXED_TOY_MODULESTORE = mixed_store_config( - TEST_DATA_DIR, {'edX/toy/2012_Fall': 'xml', }, include_xml=True, xml_source_dirs=['toy'] + TEST_DATA_DIR, {}, xml_source_dirs=['toy'] ) # This modulestore will provide both a mixed mongo editable modulestore, and # an XML store with common/test/data/2014 loaded, which is a course that is closed. TEST_DATA_MIXED_CLOSED_MODULESTORE = mixed_store_config( - TEST_DATA_DIR, {'edX/detached_pages/2014': 'xml', }, include_xml=True, xml_source_dirs=['2014'] + TEST_DATA_DIR, {}, xml_source_dirs=['2014'] ) # This modulestore will provide both a mixed mongo editable modulestore, and # an XML store with common/test/data/graded loaded, which is a course that is graded. TEST_DATA_MIXED_GRADED_MODULESTORE = mixed_store_config( - TEST_DATA_DIR, {'edX/graded/2012_Fall': 'xml', }, include_xml=True, xml_source_dirs=['graded'] + TEST_DATA_DIR, {}, xml_source_dirs=['graded'] ) # All store requests now go through mixed # Use this modulestore if you specifically want to test mongo and not a mocked modulestore. # This modulestore definition below will not load any xml courses. -TEST_DATA_MONGO_MODULESTORE = mixed_store_config(mkdtemp_clean(), {}, include_xml=False) +TEST_DATA_MONGO_MODULESTORE = mixed_store_config(mkdtemp_clean(), {}) # All store requests now go through mixed # Use this modulestore if you specifically want to test split-mongo and not a mocked modulestore. @@ -186,7 +181,6 @@ TEST_DATA_MONGO_MODULESTORE = mixed_store_config(mkdtemp_clean(), {}, include_xm TEST_DATA_SPLIT_MODULESTORE = mixed_store_config( mkdtemp_clean(), {}, - include_xml=False, store_order=[StoreConstructors.split, StoreConstructors.draft] ) @@ -239,7 +233,7 @@ class SharedModuleStoreTestCase(TestCase): In Django 1.8, we will be able to use setUpTestData() to do class level init for Django ORM models that will get cleaned up properly. """ - MODULESTORE = mixed_store_config(mkdtemp_clean(), {}, include_xml=False) + MODULESTORE = mixed_store_config(mkdtemp_clean(), {}) # Tell Django to clean out all databases, not just default multi_db = True @@ -403,7 +397,7 @@ class ModuleStoreTestCase(TestCase): your `setUp()` method. """ - MODULESTORE = mixed_store_config(mkdtemp_clean(), {}, include_xml=False) + MODULESTORE = mixed_store_config(mkdtemp_clean(), {}) # Tell Django to clean out all databases, not just default multi_db = True diff --git a/lms/djangoapps/courseware/management/commands/tests/test_dump_course.py b/lms/djangoapps/courseware/management/commands/tests/test_dump_course.py index bbff7ff15e..53a5b87a68 100644 --- a/lms/djangoapps/courseware/management/commands/tests/test_dump_course.py +++ b/lms/djangoapps/courseware/management/commands/tests/test_dump_course.py @@ -24,7 +24,6 @@ from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.xml_importer import import_course_from_xml DATA_DIR = settings.COMMON_TEST_DATA_ROOT -XML_COURSE_DIRS = ['toy', 'simple'] @attr('shard_1') From 4221ca6f2a32aca100a04c4fd555aa194067abdb Mon Sep 17 00:00:00 2001 From: John Eskew Date: Tue, 22 Dec 2015 10:18:10 -0500 Subject: [PATCH 04/22] Use ToyCourseFactory instead of the XMLModuleStore-backed toy course. --- openedx/core/djangoapps/course_groups/tests/test_cohorts.py | 5 +++-- .../djangoapps/course_groups/tests/test_partition_scheme.py | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/openedx/core/djangoapps/course_groups/tests/test_cohorts.py b/openedx/core/djangoapps/course_groups/tests/test_cohorts.py index 1a0986df9c..0e5f23a033 100644 --- a/openedx/core/djangoapps/course_groups/tests/test_cohorts.py +++ b/openedx/core/djangoapps/course_groups/tests/test_cohorts.py @@ -17,6 +17,7 @@ from student.models import CourseEnrollment from student.tests.factories import UserFactory from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import TEST_DATA_MIXED_TOY_MODULESTORE, ModuleStoreTestCase +from xmodule.modulestore.tests.factories import ToyCourseFactory from ..models import CourseUserGroup, CourseCohort, CourseUserGroupPartitionGroup from .. import cohorts @@ -145,7 +146,7 @@ class TestCohorts(ModuleStoreTestCase): Make sure that course is reloaded every time--clear out the modulestore. """ super(TestCohorts, self).setUp() - self.toy_course_key = SlashSeparatedCourseKey("edX", "toy", "2012_Fall") + self.toy_course_key = ToyCourseFactory.create().id def _create_cohort(self, course_id, cohort_name, assignment_type): """ @@ -740,7 +741,7 @@ class TestCohortsAndPartitionGroups(ModuleStoreTestCase): """ super(TestCohortsAndPartitionGroups, self).setUp() - self.test_course_key = SlashSeparatedCourseKey("edX", "toy", "2012_Fall") + self.test_course_key = ToyCourseFactory.create().id self.course = modulestore().get_course(self.test_course_key) self.first_cohort = CohortFactory(course_id=self.course.id, name="FirstCohort") diff --git a/openedx/core/djangoapps/course_groups/tests/test_partition_scheme.py b/openedx/core/djangoapps/course_groups/tests/test_partition_scheme.py index df76894633..f76d1ebec8 100644 --- a/openedx/core/djangoapps/course_groups/tests/test_partition_scheme.py +++ b/openedx/core/djangoapps/course_groups/tests/test_partition_scheme.py @@ -16,6 +16,7 @@ from student.tests.factories import UserFactory from xmodule.partitions.partitions import Group, UserPartition, UserPartitionError from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, TEST_DATA_MIXED_TOY_MODULESTORE +from xmodule.modulestore.tests.factories import ToyCourseFactory from opaque_keys.edx.locations import SlashSeparatedCourseKey from openedx.core.djangoapps.user_api.partition_schemes import RandomUserPartitionScheme @@ -40,7 +41,7 @@ class TestCohortPartitionScheme(ModuleStoreTestCase): """ super(TestCohortPartitionScheme, self).setUp() - self.course_key = SlashSeparatedCourseKey("edX", "toy", "2012_Fall") + self.course_key = ToyCourseFactory.create().id self.course = modulestore().get_course(self.course_key) config_course_cohorts(self.course, is_cohorted=True) @@ -286,7 +287,7 @@ class TestGetCohortedUserPartition(ModuleStoreTestCase): and a student for each test. """ super(TestGetCohortedUserPartition, self).setUp() - self.course_key = SlashSeparatedCourseKey("edX", "toy", "2012_Fall") + self.course_key = ToyCourseFactory.create().id self.course = modulestore().get_course(self.course_key) self.student = UserFactory.create() From 99ded6c7ee65744190bfd976a3a32d0919b79346 Mon Sep 17 00:00:00 2001 From: John Eskew Date: Tue, 22 Dec 2015 11:25:52 -0500 Subject: [PATCH 05/22] Import test course instead of using XML-backed course. --- lms/djangoapps/courseware/tests/test_about.py | 32 +++++++++++---- .../courseware/tests/test_course_info.py | 36 ++++++++++++----- .../courseware/tests/test_courses.py | 16 ++++++-- lms/djangoapps/courseware/tests/test_tabs.py | 40 ++++++++++++++----- 4 files changed, 93 insertions(+), 31 deletions(-) diff --git a/lms/djangoapps/courseware/tests/test_about.py b/lms/djangoapps/courseware/tests/test_about.py index 3ed5e67c39..1bed645f5d 100644 --- a/lms/djangoapps/courseware/tests/test_about.py +++ b/lms/djangoapps/courseware/tests/test_about.py @@ -15,6 +15,8 @@ from opaque_keys.edx.locations import SlashSeparatedCourseKey from course_modes.models import CourseMode from track.tests import EventTrackingTestCase from xmodule.modulestore.tests.django_utils import TEST_DATA_MIXED_CLOSED_MODULESTORE +from xmodule.modulestore.tests.utils import TEST_DATA_DIR +from xmodule.modulestore.xml_importer import import_course_from_xml from student.models import CourseEnrollment from student.tests.factories import AdminFactory, CourseEnrollmentAllowedFactory, UserFactory @@ -201,14 +203,30 @@ class AboutTestCaseXML(LoginEnrollmentTestCase, ModuleStoreTestCase): """ MODULESTORE = TEST_DATA_MIXED_CLOSED_MODULESTORE - # The following XML test course (which lives at common/test/data/2014) - # is closed; we're testing that an about page still appears when - # the course is already closed - xml_course_id = SlashSeparatedCourseKey('edX', 'detached_pages', '2014') + def setUp(self): + """ + Set up the tests + """ + super(AboutTestCaseXML, self).setUp() - # this text appears in that course's about page - # common/test/data/2014/about/overview.html - xml_data = "about page 463139" + # The following test course (which lives at common/test/data/2014) + # is closed; we're testing that an about page still appears when + # the course is already closed + self.xml_course_id = self.store.make_course_key('edX', 'detached_pages', '2014') + import_course_from_xml( + self.store, + 'test_user', + TEST_DATA_DIR, + source_dirs=['2014'], + static_content_store=None, + target_id=self.xml_course_id, + raise_on_failure=True, + create_if_not_present=True, + ) + + # this text appears in that course's about page + # common/test/data/2014/about/overview.html + self.xml_data = "about page 463139" @patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False}) def test_logged_in_xml(self): diff --git a/lms/djangoapps/courseware/tests/test_course_info.py b/lms/djangoapps/courseware/tests/test_course_info.py index f71db15908..99f34a3c84 100644 --- a/lms/djangoapps/courseware/tests/test_course_info.py +++ b/lms/djangoapps/courseware/tests/test_course_info.py @@ -17,9 +17,11 @@ from util.date_utils import strftime_localized from xmodule.modulestore.tests.django_utils import ( ModuleStoreTestCase, SharedModuleStoreTestCase, - TEST_DATA_SPLIT_MODULESTORE + TEST_DATA_SPLIT_MODULESTORE, + TEST_DATA_MIXED_CLOSED_MODULESTORE ) -from xmodule.modulestore.tests.django_utils import TEST_DATA_MIXED_CLOSED_MODULESTORE +from xmodule.modulestore.tests.utils import TEST_DATA_DIR +from xmodule.modulestore.xml_importer import import_course_from_xml from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, check_mongo_calls from student.models import CourseEnrollment from student.tests.factories import AdminFactory @@ -214,14 +216,30 @@ class CourseInfoTestCaseXML(LoginEnrollmentTestCase, ModuleStoreTestCase): """ MODULESTORE = TEST_DATA_MIXED_CLOSED_MODULESTORE - # The following XML test course (which lives at common/test/data/2014) - # is closed; we're testing that a course info page still appears when - # the course is already closed - xml_course_key = SlashSeparatedCourseKey('edX', 'detached_pages', '2014') + def setUp(self): + """ + Set up the tests + """ + super(CourseInfoTestCaseXML, self).setUp() - # this text appears in that course's course info page - # common/test/data/2014/info/updates.html - xml_data = "course info 463139" + # The following test course (which lives at common/test/data/2014) + # is closed; we're testing that a course info page still appears when + # the course is already closed + self.xml_course_key = self.store.make_course_key('edX', 'detached_pages', '2014') + import_course_from_xml( + self.store, + 'test_user', + TEST_DATA_DIR, + source_dirs=['2014'], + static_content_store=None, + target_id=self.xml_course_key, + raise_on_failure=True, + create_if_not_present=True, + ) + + # this text appears in that course's course info page + # common/test/data/2014/info/updates.html + self.xml_data = "course info 463139" @mock.patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False}) def test_logged_in_xml(self): diff --git a/lms/djangoapps/courseware/tests/test_courses.py b/lms/djangoapps/courseware/tests/test_courses.py index 32788027a6..fde9442fb3 100644 --- a/lms/djangoapps/courseware/tests/test_courses.py +++ b/lms/djangoapps/courseware/tests/test_courses.py @@ -32,9 +32,12 @@ from student.tests.factories import UserFactory from xmodule.modulestore.django import _get_modulestore_branch_setting, modulestore from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.xml_importer import import_course_from_xml -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from xmodule.modulestore.tests.django_utils import TEST_DATA_MIXED_TOY_MODULESTORE -from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, check_mongo_calls +from xmodule.modulestore.tests.django_utils import ( + ModuleStoreTestCase, TEST_DATA_MIXED_TOY_MODULESTORE +) +from xmodule.modulestore.tests.factories import ( + CourseFactory, ItemFactory, ToyCourseFactory, check_mongo_calls +) from xmodule.tests.xml import factories as xml from xmodule.tests.xml import XModuleXmlImportTest @@ -308,7 +311,12 @@ class XmlCoursesRenderTest(ModuleStoreTestCase): """Test methods related to rendering courses content for an XML course.""" MODULESTORE = TEST_DATA_MIXED_TOY_MODULESTORE - toy_course_key = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall') + def setUp(self): + """ + Make sure that course is reloaded every time--clear out the modulestore. + """ + super(XmlCoursesRenderTest, self).setUp() + self.toy_course_key = ToyCourseFactory.create().id def test_get_course_info_section_render(self): course = get_course_by_id(self.toy_course_key) diff --git a/lms/djangoapps/courseware/tests/test_tabs.py b/lms/djangoapps/courseware/tests/test_tabs.py index d94de34be5..abc34a64fb 100644 --- a/lms/djangoapps/courseware/tests/test_tabs.py +++ b/lms/djangoapps/courseware/tests/test_tabs.py @@ -27,10 +27,12 @@ from util.milestones_helpers import ( from milestones.tests.utils import MilestonesTestCaseMixin from xmodule import tabs as xmodule_tabs from xmodule.modulestore.tests.django_utils import ( - TEST_DATA_MIXED_TOY_MODULESTORE, TEST_DATA_MIXED_CLOSED_MODULESTORE, - SharedModuleStoreTestCase) -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase + ModuleStoreTestCase, + SharedModuleStoreTestCase +) from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory +from xmodule.modulestore.tests.utils import TEST_DATA_DIR +from xmodule.modulestore.xml_importer import import_course_from_xml class TabTestCase(SharedModuleStoreTestCase): @@ -289,15 +291,31 @@ class StaticTabDateTestCaseXML(LoginEnrollmentTestCase, ModuleStoreTestCase): MODULESTORE = TEST_DATA_MIXED_CLOSED_MODULESTORE - # The following XML test course (which lives at common/test/data/2014) - # is closed; we're testing that tabs still appear when - # the course is already closed - xml_course_key = SlashSeparatedCourseKey('edX', 'detached_pages', '2014') + def setUp(self): + """ + Set up the tests + """ + super(StaticTabDateTestCaseXML, self).setUp() - # this text appears in the test course's tab - # common/test/data/2014/tabs/8e4cce2b4aaf4ba28b1220804619e41f.html - xml_data = "static 463139" - xml_url = "8e4cce2b4aaf4ba28b1220804619e41f" + # The following XML test course (which lives at common/test/data/2014) + # is closed; we're testing that tabs still appear when + # the course is already closed + self.xml_course_key = self.store.make_course_key('edX', 'detached_pages', '2014') + import_course_from_xml( + self.store, + 'test_user', + TEST_DATA_DIR, + source_dirs=['2014'], + static_content_store=None, + target_id=self.xml_course_key, + raise_on_failure=True, + create_if_not_present=True, + ) + + # this text appears in the test course's tab + # common/test/data/2014/tabs/8e4cce2b4aaf4ba28b1220804619e41f.html + self.xml_data = "static 463139" + self.xml_url = "8e4cce2b4aaf4ba28b1220804619e41f" @patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False}) def test_logged_in_xml(self): From 20533de1456c36fd65a28c2ebf1a587d7e2881d2 Mon Sep 17 00:00:00 2001 From: John Eskew Date: Tue, 22 Dec 2015 14:48:19 -0500 Subject: [PATCH 06/22] Skip test with static tabs - seems broken. --- common/lib/xmodule/xmodule/modulestore/tests/factories.py | 1 + lms/djangoapps/courseware/tests/test_tabs.py | 5 ++--- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/factories.py b/common/lib/xmodule/xmodule/modulestore/tests/factories.py index 48c1cc7d59..b33ed1d65e 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/factories.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/factories.py @@ -233,6 +233,7 @@ class ToyCourseFactory(SampleCourseFactory): user_id, toy_course.id, "course_info", "handouts", fields={"data": "Sample"} ) + ## TODO: Broken! These static tabs are never added to course.tabs? store.create_item( user_id, toy_course.id, "static_tab", "resources", fields={"display_name": "Resources"}, diff --git a/lms/djangoapps/courseware/tests/test_tabs.py b/lms/djangoapps/courseware/tests/test_tabs.py index abc34a64fb..099732a4ec 100644 --- a/lms/djangoapps/courseware/tests/test_tabs.py +++ b/lms/djangoapps/courseware/tests/test_tabs.py @@ -7,6 +7,7 @@ from django.http import Http404 from mock import MagicMock, Mock, patch from nose.plugins.attrib import attr from opaque_keys.edx.locations import SlashSeparatedCourseKey +from unittest import skip from courseware.courses import get_course_by_id from courseware.tabs import ( @@ -241,9 +242,6 @@ class StaticTabDateTestCase(LoginEnrollmentTestCase, SharedModuleStoreTestCase): cls.course.save() cls.toy_course_key = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall') - def setUp(self): - super(StaticTabDateTestCase, self).setUp() - def test_logged_in(self): self.setup_user() url = reverse('static_tab', args=[self.course.id.to_deprecated_string(), 'new_tab']) @@ -263,6 +261,7 @@ class StaticTabDateTestCase(LoginEnrollmentTestCase, SharedModuleStoreTestCase): with self.assertRaises(Http404): static_tab(request, course_id='edX/toy', tab_slug='new_tab') + @skip("Broken! Never finds the 'resources' static tab when created by the ToyCourseFactory.") def test_get_static_tab_contents(self): self.setup_user() course = get_course_by_id(self.toy_course_key) From 74eb70aa128709f5ca3da154596c40675f9fd836 Mon Sep 17 00:00:00 2001 From: John Eskew Date: Tue, 22 Dec 2015 15:26:06 -0500 Subject: [PATCH 07/22] Fix toy course textbook test - use Mongo-backed test course. --- common/lib/xmodule/xmodule/course_module.py | 2 +- .../xmodule/xmodule/modulestore/tests/factories.py | 3 ++- lms/djangoapps/courseware/tests/tests.py | 12 +++++++----- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 93e1481417..218b200730 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -175,7 +175,7 @@ class CourseFields(object): scope=Scope.settings ) textbooks = TextbookList( - help=_("List of pairs of (title, url) for textbooks used in this course"), + help=_("List of Textbook objects with (title, url) for textbooks used in this course"), default=[], scope=Scope.content ) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/factories.py b/common/lib/xmodule/xmodule/modulestore/tests/factories.py index b33ed1d65e..66e4ea081d 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/factories.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/factories.py @@ -25,6 +25,7 @@ from xmodule.modulestore import prefer_xmodules, ModuleStoreEnum from xmodule.modulestore.tests.sample_courses import default_block_info_tree, TOY_BLOCK_INFO_TREE from xmodule.tabs import CourseTab from xmodule.x_module import DEPRECATION_VSCOMPAT_EVENT +from xmodule.course_module import Textbook class Dummy(object): @@ -190,7 +191,7 @@ class ToyCourseFactory(SampleCourseFactory): fields = { 'block_info_tree': TOY_BLOCK_INFO_TREE, - 'textbooks': [["Textbook", "path/to/a/text_book"]], + 'textbooks': [Textbook("Textbook", "path/to/a/text_book")], 'wiki_slug': "toy", 'graded': True, 'discussion_topics': {"General": {"id": "i4x-edX-toy-course-2012_Fall"}}, diff --git a/lms/djangoapps/courseware/tests/tests.py b/lms/djangoapps/courseware/tests/tests.py index 32e658d9aa..5189f143c7 100644 --- a/lms/djangoapps/courseware/tests/tests.py +++ b/lms/djangoapps/courseware/tests/tests.py @@ -10,11 +10,13 @@ from nose.plugins.attrib import attr from opaque_keys.edx.locations import SlashSeparatedCourseKey from courseware.tests.helpers import LoginEnrollmentTestCase -from xmodule.modulestore.tests.django_utils import TEST_DATA_MIXED_TOY_MODULESTORE as TOY_MODULESTORE from lms.djangoapps.lms_xblock.field_data import LmsFieldData from xmodule.error_module import ErrorDescriptor from xmodule.modulestore.django import modulestore -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.django_utils import ( + ModuleStoreTestCase, TEST_DATA_MIXED_TOY_MODULESTORE +) +from xmodule.modulestore.tests.factories import ToyCourseFactory @attr('shard_1') @@ -126,11 +128,12 @@ class TestMongoCoursesLoad(ModuleStoreTestCase, PageLoaderTestCase): """ Check that all pages in test courses load properly from Mongo. """ - MODULESTORE = TOY_MODULESTORE + MODULESTORE = TEST_DATA_MIXED_TOY_MODULESTORE def setUp(self): super(TestMongoCoursesLoad, self).setUp() self.setup_user() + self.toy_course_key = ToyCourseFactory.create().id @mock.patch('xmodule.course_module.requests.get') def test_toy_textbooks_loads(self, mock_get): @@ -139,8 +142,7 @@ class TestMongoCoursesLoad(ModuleStoreTestCase, PageLoaderTestCase): """).strip() - - location = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall').make_usage_key('course', '2012_Fall') + location = self.toy_course_key.make_usage_key('course', '2012_Fall') course = self.store.get_item(location) self.assertGreater(len(course.textbooks), 0) From 3fb437e5330e4a8b6563b68191dcbb91595716d6 Mon Sep 17 00:00:00 2001 From: John Eskew Date: Tue, 22 Dec 2015 16:28:35 -0500 Subject: [PATCH 08/22] Import test course instead of using XML-backed course. --- lms/djangoapps/django_comment_client/tests/test_models.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lms/djangoapps/django_comment_client/tests/test_models.py b/lms/djangoapps/django_comment_client/tests/test_models.py index 4093252990..17fa747e5e 100644 --- a/lms/djangoapps/django_comment_client/tests/test_models.py +++ b/lms/djangoapps/django_comment_client/tests/test_models.py @@ -5,9 +5,11 @@ from django.test.testcases import TestCase from nose.plugins.attrib import attr from opaque_keys.edx.keys import CourseKey -from xmodule.modulestore.tests.django_utils import TEST_DATA_MIXED_TOY_MODULESTORE import django_comment_common.models as models -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.django_utils import ( + TEST_DATA_MIXED_TOY_MODULESTORE, ModuleStoreTestCase +) +from xmodule.modulestore.tests.factories import ToyCourseFactory @attr('shard_1') @@ -23,7 +25,7 @@ class RoleClassTestCase(ModuleStoreTestCase): # For course ID, syntax edx/classname/classdate is important # because xmodel.course_module.id_to_location looks for a string to split - self.course_id = CourseKey.from_string("edX/toy/2012_Fall") + self.course_id = ToyCourseFactory.create().id self.student_role = models.Role.objects.get_or_create(name="Student", course_id=self.course_id)[0] self.student_role.add_permission("delete_thread") From 415ef3616f4ac3924c84d7a643b3333766400e40 Mon Sep 17 00:00:00 2001 From: John Eskew Date: Tue, 22 Dec 2015 16:36:40 -0500 Subject: [PATCH 09/22] Remove XML-backed course-specific test. --- lms/djangoapps/bulk_email/tests/test_forms.py | 26 ------------------- 1 file changed, 26 deletions(-) diff --git a/lms/djangoapps/bulk_email/tests/test_forms.py b/lms/djangoapps/bulk_email/tests/test_forms.py index 13180d4e52..f2b308ef8b 100644 --- a/lms/djangoapps/bulk_email/tests/test_forms.py +++ b/lms/djangoapps/bulk_email/tests/test_forms.py @@ -127,32 +127,6 @@ class CourseAuthorizationFormTest(ModuleStoreTestCase): form.save() -class CourseAuthorizationXMLFormTest(ModuleStoreTestCase): - """Check that XML courses cannot be authorized for email.""" - MODULESTORE = TEST_DATA_MIXED_TOY_MODULESTORE - - @patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': True}) - def test_xml_course_authorization(self): - course_id = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall') - # Assert this is an XML course - self.assertEqual(modulestore().get_modulestore_type(course_id), ModuleStoreEnum.Type.xml) - - form_data = {'course_id': course_id.to_deprecated_string(), 'email_enabled': True} - form = CourseAuthorizationAdminForm(data=form_data) - # Validation shouldn't work - self.assertFalse(form.is_valid()) - - msg = u"Course Email feature is only available for courses authored in Studio. " - msg += u'"{0}" appears to be an XML backed course.'.format(course_id.to_deprecated_string()) - self.assertEquals(msg, form._errors['course_id'][0]) # pylint: disable=protected-access - - with self.assertRaisesRegexp( - ValueError, - "The CourseAuthorization could not be created because the data didn't validate." - ): - form.save() - - class CourseEmailTemplateFormTest(ModuleStoreTestCase): """Test the CourseEmailTemplateForm that is used in the Django admin subsystem.""" From 47fdc841c09a03013640427db9c28fb334b01d73 Mon Sep 17 00:00:00 2001 From: John Eskew Date: Tue, 22 Dec 2015 16:36:53 -0500 Subject: [PATCH 10/22] Import test course instead of using XML-backed course. --- lms/djangoapps/django_comment_client/tests/test_utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/django_comment_client/tests/test_utils.py b/lms/djangoapps/django_comment_client/tests/test_utils.py index 677144e8da..1d3a123bba 100644 --- a/lms/djangoapps/django_comment_client/tests/test_utils.py +++ b/lms/djangoapps/django_comment_client/tests/test_utils.py @@ -25,7 +25,7 @@ from openedx.core.djangoapps.content.course_structures.models import CourseStruc from openedx.core.djangoapps.util.testing import ContentGroupTestCase from student.roles import CourseStaffRole from xmodule.modulestore import ModuleStoreEnum -from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, ToyCourseFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, TEST_DATA_MIXED_TOY_MODULESTORE from xmodule.modulestore.django import modulestore from opaque_keys.edx.locator import CourseLocator @@ -1255,7 +1255,7 @@ class IsCommentableCohortedTestCase(ModuleStoreTestCase): Make sure that course is reloaded every time--clear out the modulestore. """ super(IsCommentableCohortedTestCase, self).setUp() - self.toy_course_key = CourseLocator("edX", "toy", "2012_Fall", deprecated=True) + self.toy_course_key = ToyCourseFactory.create().id def test_is_commentable_cohorted(self): course = modulestore().get_course(self.toy_course_key) From 9cfe31a2f3edbaf50d933b7c4a8652fcbcb57438 Mon Sep 17 00:00:00 2001 From: John Eskew Date: Tue, 22 Dec 2015 17:16:33 -0500 Subject: [PATCH 11/22] Remove unused xml_source_dirs from mixed modulestore config. --- .../xmodule/xmodule/modulestore/tests/django_utils.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py index 286810069a..1a1298373c 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py @@ -32,7 +32,7 @@ class StoreConstructors(object): draft, split = range(2) -def mixed_store_config(data_dir, mappings, xml_source_dirs=None, store_order=None): +def mixed_store_config(data_dir, mappings, store_order=None): """ Return a `MixedModuleStore` configuration, which provides access to both Mongo-backed courses. @@ -51,13 +51,8 @@ def mixed_store_config(data_dir, mappings, xml_source_dirs=None, store_order=Non Keyword Args: - xml_source_dirs (list): The directories containing XML courses to load from disk. - - note: For the courses to be loaded into the XML modulestore and accessible do the following: - * xml_source_dirs should be the list of directories (relative to data_dir) - containing the courses you want to load - * mappings should be configured, pointing the xml courses to the xml modulestore - + store_order (list): List of StoreConstructors providing order of modulestores + to use in creating courses. """ if store_order is None: store_order = [StoreConstructors.draft, StoreConstructors.split] From 91c94977d51b87d6a0926118384019c23b69deca Mon Sep 17 00:00:00 2001 From: John Eskew Date: Wed, 23 Dec 2015 10:43:59 -0500 Subject: [PATCH 12/22] Unify usage of a single test mixed modulestore called: TEST_DATA_MIXED_MODULESTORE Remove these test mixed modulestores: TEST_DATA_MIXED_TOY_MODULESTORE TEST_DATA_MIXED_CLOSED_MODULESTORE TEST_DATA_MIXED_GRADED_MODULESTORE --- .../student/tests/test_bulk_email_settings.py | 4 ++-- .../xmodule/modulestore/tests/django_utils.py | 24 +++++++------------ lms/djangoapps/bulk_email/tests/test_forms.py | 1 - lms/djangoapps/courseware/tests/test_about.py | 4 ++-- .../courseware/tests/test_course_info.py | 4 ++-- .../courseware/tests/test_courses.py | 2 +- .../courseware/tests/test_module_render.py | 8 +++---- lms/djangoapps/courseware/tests/test_tabs.py | 11 +++++---- lms/djangoapps/courseware/tests/test_views.py | 4 ++-- lms/djangoapps/courseware/tests/tests.py | 4 ++-- .../tests/test_models.py | 4 ++-- .../django_comment_client/tests/test_utils.py | 4 ++-- lms/djangoapps/instructor/tests/test_email.py | 4 ++-- lms/lib/xblock/test/test_mixin.py | 4 ++-- .../course_groups/tests/test_cohorts.py | 6 ++--- .../tests/test_partition_scheme.py | 6 ++--- 16 files changed, 43 insertions(+), 51 deletions(-) diff --git a/common/djangoapps/student/tests/test_bulk_email_settings.py b/common/djangoapps/student/tests/test_bulk_email_settings.py index 522064df90..b2bca30f0b 100644 --- a/common/djangoapps/student/tests/test_bulk_email_settings.py +++ b/common/djangoapps/student/tests/test_bulk_email_settings.py @@ -13,7 +13,7 @@ from opaque_keys.edx.locations import SlashSeparatedCourseKey from student.tests.factories import UserFactory, CourseEnrollmentFactory from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase -from xmodule.modulestore.tests.django_utils import TEST_DATA_MIXED_TOY_MODULESTORE +from xmodule.modulestore.tests.django_utils import TEST_DATA_MIXED_MODULESTORE from xmodule.modulestore.tests.factories import CourseFactory # This import is for an lms djangoapp. @@ -90,7 +90,7 @@ class TestStudentDashboardEmailViewXMLBacked(SharedModuleStoreTestCase): """ Check for email view on student dashboard, with XML backed course. """ - MODULESTORE = TEST_DATA_MIXED_TOY_MODULESTORE + MODULESTORE = TEST_DATA_MIXED_MODULESTORE def setUp(self): super(TestStudentDashboardEmailViewXMLBacked, self).setUp() diff --git a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py index 1a1298373c..faac630bd1 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py @@ -147,22 +147,14 @@ def drop_mongo_collections(mock_create): TEST_DATA_DIR = settings.COMMON_TEST_DATA_ROOT -# This modulestore will provide both a mixed mongo editable modulestore, and -# an XML store with just the toy course loaded. -TEST_DATA_MIXED_TOY_MODULESTORE = mixed_store_config( - TEST_DATA_DIR, {}, xml_source_dirs=['toy'] -) - -# This modulestore will provide both a mixed mongo editable modulestore, and -# an XML store with common/test/data/2014 loaded, which is a course that is closed. -TEST_DATA_MIXED_CLOSED_MODULESTORE = mixed_store_config( - TEST_DATA_DIR, {}, xml_source_dirs=['2014'] -) - -# This modulestore will provide both a mixed mongo editable modulestore, and -# an XML store with common/test/data/graded loaded, which is a course that is graded. -TEST_DATA_MIXED_GRADED_MODULESTORE = mixed_store_config( - TEST_DATA_DIR, {}, xml_source_dirs=['graded'] +# This modulestore will provide a mixed mongo editable modulestore. +# If your test uses the 'toy' course, use the the ToyCourseFactory to construct it. +# If your test needs a closed course to test against, import the common/test/data/2014 +# test course into this modulestore. +# If your test needs a graded course to test against, import the common/test/data/graded +# test course into this modulestore. +TEST_DATA_MIXED_MODULESTORE = mixed_store_config( + TEST_DATA_DIR, {} ) # All store requests now go through mixed diff --git a/lms/djangoapps/bulk_email/tests/test_forms.py b/lms/djangoapps/bulk_email/tests/test_forms.py index f2b308ef8b..5d465a05ec 100644 --- a/lms/djangoapps/bulk_email/tests/test_forms.py +++ b/lms/djangoapps/bulk_email/tests/test_forms.py @@ -8,7 +8,6 @@ from nose.plugins.attrib import attr from bulk_email.models import CourseAuthorization, CourseEmailTemplate from bulk_email.forms import CourseAuthorizationAdminForm, CourseEmailTemplateForm -from xmodule.modulestore.tests.django_utils import TEST_DATA_MIXED_TOY_MODULESTORE from opaque_keys.edx.locations import SlashSeparatedCourseKey from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory diff --git a/lms/djangoapps/courseware/tests/test_about.py b/lms/djangoapps/courseware/tests/test_about.py index 1bed645f5d..5530cd9dcc 100644 --- a/lms/djangoapps/courseware/tests/test_about.py +++ b/lms/djangoapps/courseware/tests/test_about.py @@ -14,7 +14,7 @@ from opaque_keys.edx.locations import SlashSeparatedCourseKey from course_modes.models import CourseMode from track.tests import EventTrackingTestCase -from xmodule.modulestore.tests.django_utils import TEST_DATA_MIXED_CLOSED_MODULESTORE +from xmodule.modulestore.tests.django_utils import TEST_DATA_MIXED_MODULESTORE from xmodule.modulestore.tests.utils import TEST_DATA_DIR from xmodule.modulestore.xml_importer import import_course_from_xml @@ -201,7 +201,7 @@ class AboutTestCaseXML(LoginEnrollmentTestCase, ModuleStoreTestCase): """ Tests for the course about page """ - MODULESTORE = TEST_DATA_MIXED_CLOSED_MODULESTORE + MODULESTORE = TEST_DATA_MIXED_MODULESTORE def setUp(self): """ diff --git a/lms/djangoapps/courseware/tests/test_course_info.py b/lms/djangoapps/courseware/tests/test_course_info.py index 99f34a3c84..2c122ee714 100644 --- a/lms/djangoapps/courseware/tests/test_course_info.py +++ b/lms/djangoapps/courseware/tests/test_course_info.py @@ -18,7 +18,7 @@ from xmodule.modulestore.tests.django_utils import ( ModuleStoreTestCase, SharedModuleStoreTestCase, TEST_DATA_SPLIT_MODULESTORE, - TEST_DATA_MIXED_CLOSED_MODULESTORE + TEST_DATA_MIXED_MODULESTORE ) from xmodule.modulestore.tests.utils import TEST_DATA_DIR from xmodule.modulestore.xml_importer import import_course_from_xml @@ -214,7 +214,7 @@ class CourseInfoTestCaseXML(LoginEnrollmentTestCase, ModuleStoreTestCase): """ Tests for the Course Info page for an XML course """ - MODULESTORE = TEST_DATA_MIXED_CLOSED_MODULESTORE + MODULESTORE = TEST_DATA_MIXED_MODULESTORE def setUp(self): """ diff --git a/lms/djangoapps/courseware/tests/test_courses.py b/lms/djangoapps/courseware/tests/test_courses.py index fde9442fb3..6976e376ca 100644 --- a/lms/djangoapps/courseware/tests/test_courses.py +++ b/lms/djangoapps/courseware/tests/test_courses.py @@ -309,7 +309,7 @@ class CoursesRenderTest(ModuleStoreTestCase): @attr('shard_1') class XmlCoursesRenderTest(ModuleStoreTestCase): """Test methods related to rendering courses content for an XML course.""" - MODULESTORE = TEST_DATA_MIXED_TOY_MODULESTORE + MODULESTORE = TEST_DATA_MIXED_MODULESTORE def setUp(self): """ diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index ff8a7e0b90..2484c98ded 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -43,13 +43,13 @@ from openedx.core.lib.courses import course_image_url from openedx.core.lib.gating import api as gating_api from student.models import anonymous_id_for_user from xmodule.modulestore.tests.django_utils import ( - TEST_DATA_MIXED_TOY_MODULESTORE, - SharedModuleStoreTestCase + ModuleStoreTestCase, + SharedModuleStoreTestCase, + TEST_DATA_MIXED_MODULESTORE ) from xmodule.lti_module import LTIDescriptor from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import ItemFactory, CourseFactory, ToyCourseFactory, check_mongo_calls from xmodule.x_module import XModuleDescriptor, XModule, STUDENT_VIEW, CombinedSystem @@ -1403,7 +1403,7 @@ class MongoViewInStudioTest(ViewInStudioTest): class MixedViewInStudioTest(ViewInStudioTest): """Test the 'View in Studio' link visibility in a mixed mongo backed course.""" - MODULESTORE = TEST_DATA_MIXED_TOY_MODULESTORE + MODULESTORE = TEST_DATA_MIXED_MODULESTORE def test_view_in_studio_link_mongo_backed(self): """Mixed mongo courses that are mongo backed should see 'View in Studio' links.""" diff --git a/lms/djangoapps/courseware/tests/test_tabs.py b/lms/djangoapps/courseware/tests/test_tabs.py index 099732a4ec..f11d7c12d1 100644 --- a/lms/djangoapps/courseware/tests/test_tabs.py +++ b/lms/djangoapps/courseware/tests/test_tabs.py @@ -29,7 +29,8 @@ from milestones.tests.utils import MilestonesTestCaseMixin from xmodule import tabs as xmodule_tabs from xmodule.modulestore.tests.django_utils import ( ModuleStoreTestCase, - SharedModuleStoreTestCase + SharedModuleStoreTestCase, + TEST_DATA_MIXED_MODULESTORE ) from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.modulestore.tests.utils import TEST_DATA_DIR @@ -228,7 +229,7 @@ class TextbooksTestCase(TabTestCase): class StaticTabDateTestCase(LoginEnrollmentTestCase, SharedModuleStoreTestCase): """Test cases for Static Tab Dates.""" - MODULESTORE = TEST_DATA_MIXED_TOY_MODULESTORE + MODULESTORE = TEST_DATA_MIXED_MODULESTORE @classmethod def setUpClass(cls): @@ -288,7 +289,7 @@ class StaticTabDateTestCaseXML(LoginEnrollmentTestCase, ModuleStoreTestCase): Tests for the static tab dates of an XML course """ - MODULESTORE = TEST_DATA_MIXED_CLOSED_MODULESTORE + MODULESTORE = TEST_DATA_MIXED_MODULESTORE def setUp(self): """ @@ -338,7 +339,7 @@ class EntranceExamsTabsTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase, Mi """ Validate tab behavior when dealing with Entrance Exams """ - MODULESTORE = TEST_DATA_MIXED_CLOSED_MODULESTORE + MODULESTORE = TEST_DATA_MIXED_MODULESTORE @patch.dict('django.conf.settings.FEATURES', {'ENTRANCE_EXAMS': True, 'MILESTONES_APP': True}) def setUp(self): @@ -445,7 +446,7 @@ class TextBookCourseViewsTestCase(LoginEnrollmentTestCase, SharedModuleStoreTest """ Validate tab behavior when dealing with textbooks. """ - MODULESTORE = TEST_DATA_MIXED_TOY_MODULESTORE + MODULESTORE = TEST_DATA_MIXED_MODULESTORE @classmethod def setUpClass(cls): diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 5a21d32c4c..5a9ebf80d3 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -51,7 +51,7 @@ from util.url import reload_django_url_config from util.views import ensure_valid_course_key from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore -from xmodule.modulestore.tests.django_utils import TEST_DATA_MIXED_TOY_MODULESTORE +from xmodule.modulestore.tests.django_utils import TEST_DATA_MIXED_MODULESTORE from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, check_mongo_calls from openedx.core.djangoapps.credit.api import set_credit_requirements @@ -63,7 +63,7 @@ class TestJumpTo(ModuleStoreTestCase): """ Check the jumpto link for a course. """ - MODULESTORE = TEST_DATA_MIXED_TOY_MODULESTORE + MODULESTORE = TEST_DATA_MIXED_MODULESTORE def setUp(self): super(TestJumpTo, self).setUp() diff --git a/lms/djangoapps/courseware/tests/tests.py b/lms/djangoapps/courseware/tests/tests.py index 5189f143c7..d376896dcb 100644 --- a/lms/djangoapps/courseware/tests/tests.py +++ b/lms/djangoapps/courseware/tests/tests.py @@ -14,7 +14,7 @@ from lms.djangoapps.lms_xblock.field_data import LmsFieldData from xmodule.error_module import ErrorDescriptor from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import ( - ModuleStoreTestCase, TEST_DATA_MIXED_TOY_MODULESTORE + ModuleStoreTestCase, TEST_DATA_MIXED_MODULESTORE ) from xmodule.modulestore.tests.factories import ToyCourseFactory @@ -128,7 +128,7 @@ class TestMongoCoursesLoad(ModuleStoreTestCase, PageLoaderTestCase): """ Check that all pages in test courses load properly from Mongo. """ - MODULESTORE = TEST_DATA_MIXED_TOY_MODULESTORE + MODULESTORE = TEST_DATA_MIXED_MODULESTORE def setUp(self): super(TestMongoCoursesLoad, self).setUp() diff --git a/lms/djangoapps/django_comment_client/tests/test_models.py b/lms/djangoapps/django_comment_client/tests/test_models.py index 17fa747e5e..5429e68661 100644 --- a/lms/djangoapps/django_comment_client/tests/test_models.py +++ b/lms/djangoapps/django_comment_client/tests/test_models.py @@ -7,7 +7,7 @@ from opaque_keys.edx.keys import CourseKey import django_comment_common.models as models from xmodule.modulestore.tests.django_utils import ( - TEST_DATA_MIXED_TOY_MODULESTORE, ModuleStoreTestCase + TEST_DATA_MIXED_MODULESTORE, ModuleStoreTestCase ) from xmodule.modulestore.tests.factories import ToyCourseFactory @@ -17,7 +17,7 @@ class RoleClassTestCase(ModuleStoreTestCase): """ Tests for roles of the comment client service integration """ - MODULESTORE = TEST_DATA_MIXED_TOY_MODULESTORE + MODULESTORE = TEST_DATA_MIXED_MODULESTORE def setUp(self): super(RoleClassTestCase, self).setUp() diff --git a/lms/djangoapps/django_comment_client/tests/test_utils.py b/lms/djangoapps/django_comment_client/tests/test_utils.py index 1d3a123bba..dd14861c71 100644 --- a/lms/djangoapps/django_comment_client/tests/test_utils.py +++ b/lms/djangoapps/django_comment_client/tests/test_utils.py @@ -26,7 +26,7 @@ from openedx.core.djangoapps.util.testing import ContentGroupTestCase from student.roles import CourseStaffRole from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, ToyCourseFactory -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, TEST_DATA_MIXED_TOY_MODULESTORE +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, TEST_DATA_MIXED_MODULESTORE from xmodule.modulestore.django import modulestore from opaque_keys.edx.locator import CourseLocator from lms.djangoapps.teams.tests.factories import CourseTeamFactory @@ -1248,7 +1248,7 @@ class IsCommentableCohortedTestCase(ModuleStoreTestCase): Test the is_commentable_cohorted function. """ - MODULESTORE = TEST_DATA_MIXED_TOY_MODULESTORE + MODULESTORE = TEST_DATA_MIXED_MODULESTORE def setUp(self): """ diff --git a/lms/djangoapps/instructor/tests/test_email.py b/lms/djangoapps/instructor/tests/test_email.py index 7f1bc0633f..3d16b31c5a 100644 --- a/lms/djangoapps/instructor/tests/test_email.py +++ b/lms/djangoapps/instructor/tests/test_email.py @@ -12,7 +12,7 @@ from opaque_keys.edx.locations import SlashSeparatedCourseKey from bulk_email.models import CourseAuthorization from xmodule.modulestore.tests.django_utils import ( - TEST_DATA_MIXED_TOY_MODULESTORE, SharedModuleStoreTestCase + TEST_DATA_MIXED_MODULESTORE, SharedModuleStoreTestCase ) from student.tests.factories import AdminFactory from xmodule.modulestore.tests.factories import CourseFactory @@ -112,7 +112,7 @@ class TestNewInstructorDashboardEmailViewXMLBacked(SharedModuleStoreTestCase): Check for email view on the new instructor dashboard """ - MODULESTORE = TEST_DATA_MIXED_TOY_MODULESTORE + MODULESTORE = TEST_DATA_MIXED_MODULESTORE @classmethod def setUpClass(cls): diff --git a/lms/lib/xblock/test/test_mixin.py b/lms/lib/xblock/test/test_mixin.py index 64360c4a5f..6cb4d3c52c 100644 --- a/lms/lib/xblock/test/test_mixin.py +++ b/lms/lib/xblock/test/test_mixin.py @@ -6,7 +6,7 @@ import ddt from xblock.validation import ValidationMessage from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.tests.factories import CourseFactory, ToyCourseFactory, ItemFactory -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, TEST_DATA_MIXED_TOY_MODULESTORE +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, TEST_DATA_MIXED_MODULESTORE from xmodule.partitions.partitions import Group, UserPartition @@ -157,7 +157,7 @@ class XBlockGetParentTest(LmsXBlockMixinTestCase): Test that XBlock.get_parent returns correct results with each modulestore backend. """ - MODULESTORE = TEST_DATA_MIXED_TOY_MODULESTORE + MODULESTORE = TEST_DATA_MIXED_MODULESTORE @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split, ModuleStoreEnum.Type.xml) def test_parents(self, modulestore_type): diff --git a/openedx/core/djangoapps/course_groups/tests/test_cohorts.py b/openedx/core/djangoapps/course_groups/tests/test_cohorts.py index 0e5f23a033..0df8acedfd 100644 --- a/openedx/core/djangoapps/course_groups/tests/test_cohorts.py +++ b/openedx/core/djangoapps/course_groups/tests/test_cohorts.py @@ -16,7 +16,7 @@ from opaque_keys.edx.locations import SlashSeparatedCourseKey from student.models import CourseEnrollment from student.tests.factories import UserFactory from xmodule.modulestore.django import modulestore -from xmodule.modulestore.tests.django_utils import TEST_DATA_MIXED_TOY_MODULESTORE, ModuleStoreTestCase +from xmodule.modulestore.tests.django_utils import TEST_DATA_MIXED_MODULESTORE, ModuleStoreTestCase from xmodule.modulestore.tests.factories import ToyCourseFactory from ..models import CourseUserGroup, CourseCohort, CourseUserGroupPartitionGroup @@ -139,7 +139,7 @@ class TestCohorts(ModuleStoreTestCase): """ Test the cohorts feature """ - MODULESTORE = TEST_DATA_MIXED_TOY_MODULESTORE + MODULESTORE = TEST_DATA_MIXED_MODULESTORE def setUp(self): """ @@ -733,7 +733,7 @@ class TestCohortsAndPartitionGroups(ModuleStoreTestCase): """ Test Cohorts and Partitions Groups. """ - MODULESTORE = TEST_DATA_MIXED_TOY_MODULESTORE + MODULESTORE = TEST_DATA_MIXED_MODULESTORE def setUp(self): """ diff --git a/openedx/core/djangoapps/course_groups/tests/test_partition_scheme.py b/openedx/core/djangoapps/course_groups/tests/test_partition_scheme.py index f76d1ebec8..b6b1971747 100644 --- a/openedx/core/djangoapps/course_groups/tests/test_partition_scheme.py +++ b/openedx/core/djangoapps/course_groups/tests/test_partition_scheme.py @@ -15,7 +15,7 @@ from courseware.tests.test_masquerade import StaffMasqueradeTestCase from student.tests.factories import UserFactory from xmodule.partitions.partitions import Group, UserPartition, UserPartitionError from xmodule.modulestore.django import modulestore -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, TEST_DATA_MIXED_TOY_MODULESTORE +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, TEST_DATA_MIXED_MODULESTORE from xmodule.modulestore.tests.factories import ToyCourseFactory from opaque_keys.edx.locations import SlashSeparatedCourseKey @@ -32,7 +32,7 @@ class TestCohortPartitionScheme(ModuleStoreTestCase): """ Test the logic for linking a user to a partition group based on their cohort. """ - MODULESTORE = TEST_DATA_MIXED_TOY_MODULESTORE + MODULESTORE = TEST_DATA_MIXED_MODULESTORE def setUp(self): """ @@ -279,7 +279,7 @@ class TestGetCohortedUserPartition(ModuleStoreTestCase): """ Test that `get_cohorted_user_partition` returns the first user_partition with scheme `CohortPartitionScheme`. """ - MODULESTORE = TEST_DATA_MIXED_TOY_MODULESTORE + MODULESTORE = TEST_DATA_MIXED_MODULESTORE def setUp(self): """ From 8a9845c26ee5ffac3e428da9dc8e72c8d2d413c6 Mon Sep 17 00:00:00 2001 From: John Eskew Date: Wed, 23 Dec 2015 14:00:36 -0500 Subject: [PATCH 13/22] Remove ModuleStoreEnum.Type.xml --- .../commands/tests/test_create_course.py | 4 ---- common/djangoapps/static_replace/__init__.py | 3 +-- common/djangoapps/student/views.py | 1 - .../xmodule/xmodule/modulestore/__init__.py | 1 - .../tests/test_mixed_modulestore.py | 7 ++----- .../xmodule/modulestore/tests/test_xml.py | 4 ---- common/lib/xmodule/xmodule/modulestore/xml.py | 4 ++-- .../xmodule/modulestore/xml_exporter.py | 4 +--- lms/djangoapps/bulk_email/forms.py | 7 ------- lms/djangoapps/courseware/courses.py | 4 +--- lms/djangoapps/dashboard/sysadmin.py | 18 +----------------- lms/djangoapps/instructor/views/tools.py | 3 +-- lms/lib/xblock/test/test_mixin.py | 8 ++------ openedx/core/lib/courses.py | 2 +- openedx/core/lib/xblock_utils.py | 3 +-- 15 files changed, 13 insertions(+), 60 deletions(-) diff --git a/cms/djangoapps/contentstore/management/commands/tests/test_create_course.py b/cms/djangoapps/contentstore/management/commands/tests/test_create_course.py index 1bde9f7a07..5ae88a9345 100644 --- a/cms/djangoapps/contentstore/management/commands/tests/test_create_course.py +++ b/cms/djangoapps/contentstore/management/commands/tests/test_create_course.py @@ -29,10 +29,6 @@ class TestArgParsing(unittest.TestCase): with self.assertRaises(CommandError): self.command.handle("foo", "user@foo.org", "org", "course", "run") - def test_xml_store(self): - with self.assertRaises(CommandError): - self.command.handle(ModuleStoreEnum.Type.xml, "user@foo.org", "org", "course", "run") - def test_nonexistent_user_id(self): errstring = "No user 99 found" with self.assertRaisesRegexp(CommandError, errstring): diff --git a/common/djangoapps/static_replace/__init__.py b/common/djangoapps/static_replace/__init__.py index 84c81bd187..5a12317ea9 100644 --- a/common/djangoapps/static_replace/__init__.py +++ b/common/djangoapps/static_replace/__init__.py @@ -164,8 +164,7 @@ def replace_static_urls(text, data_directory=None, course_id=None, static_asset_ return original # if we're running with a MongoBacked store course_namespace is not None, then use studio style urls elif (not static_asset_path) \ - and course_id \ - and modulestore().get_modulestore_type(course_id) != ModuleStoreEnum.Type.xml: + and course_id: # first look in the static file pipeline and see if we are trying to reference # a piece of static content which is in the edx-platform repo (e.g. JS associated with an xmodule) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 7cad92aa31..1698119c3f 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -649,7 +649,6 @@ def dashboard(request): show_email_settings_for = frozenset( enrollment.course_id for enrollment in course_enrollments if ( settings.FEATURES['ENABLE_INSTRUCTOR_EMAIL'] and - modulestore().get_modulestore_type(enrollment.course_id) != ModuleStoreEnum.Type.xml and CourseAuthorization.instructor_email_enabled(enrollment.course_id) ) ) diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index 4d2b3ae714..6e0e8671e6 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -55,7 +55,6 @@ class ModuleStoreEnum(object): """ split = 'split' mongo = 'mongo' - xml = 'xml' class RevisionOption(object): """ diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py index b31785cfce..5f1c8c0fd9 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -1991,11 +1991,8 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): self.assertEquals(store.get_modulestore_type(), store_type) # verify store used for creating a course - try: - course = self.store.create_course("org", "course{}".format(uuid4().hex[:5]), "run", self.user_id) - self.assertEquals(course.system.modulestore.get_modulestore_type(), store_type) - except NotImplementedError: - self.assertEquals(store_type, ModuleStoreEnum.Type.xml) + course = self.store.create_course("org", "course{}".format(uuid4().hex[:5]), "run", self.user_id) + self.assertEquals(course.system.modulestore.get_modulestore_type(), store_type) @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_default_store(self, default_ms): diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py b/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py index f029de4283..4da26acfd0 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py @@ -31,10 +31,6 @@ class TestXMLModuleStore(unittest.TestCase): """ Test around the XML modulestore """ - def test_xml_modulestore_type(self): - store = XMLModuleStore(DATA_DIR, source_dirs=[]) - self.assertEqual(store.get_modulestore_type(), ModuleStoreEnum.Type.xml) - @patch('xmodule.tabs.CourseTabList.initialize_default', Mock()) def test_unicode_chars_in_xml_content(self): # edX/full/6.002_Spring_2012 has non-ASCII chars, and during diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index c6b7b748ed..01aab51a5b 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -875,7 +875,7 @@ class XMLModuleStore(ModuleStoreReadBase): Args: course_key: just for signature compatibility """ - return ModuleStoreEnum.Type.xml + return None #ModuleStoreEnum.Type.xml def get_courses_for_wiki(self, wiki_slug, **kwargs): """ @@ -893,7 +893,7 @@ class XMLModuleStore(ModuleStoreReadBase): Returns the course count """ - return {ModuleStoreEnum.Type.xml: True} + return {'xml': True} @contextmanager def branch_setting(self, branch_setting, course_id=None): # pylint: disable=unused-argument diff --git a/common/lib/xmodule/xmodule/modulestore/xml_exporter.py b/common/lib/xmodule/xmodule/modulestore/xml_exporter.py index 91066d485d..7468e6755a 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_exporter.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_exporter.py @@ -279,9 +279,7 @@ class CourseExportManager(ExportManager): policy = {'course/' + courselike.location.name: own_metadata(courselike)} course_policy.write(dumps(policy, cls=EdxJSONEncoder, sort_keys=True, indent=4)) - # xml backed courses don't support drafts! - if courselike.runtime.modulestore.get_modulestore_type() != ModuleStoreEnum.Type.xml: - _export_drafts(self.modulestore, self.courselike_key, export_fs, xml_centric_courselike_key) + _export_drafts(self.modulestore, self.courselike_key, export_fs, xml_centric_courselike_key) class LibraryExportManager(ExportManager): diff --git a/lms/djangoapps/bulk_email/forms.py b/lms/djangoapps/bulk_email/forms.py index 6fdec1459f..32d91443c5 100644 --- a/lms/djangoapps/bulk_email/forms.py +++ b/lms/djangoapps/bulk_email/forms.py @@ -100,11 +100,4 @@ class CourseAuthorizationAdminForm(forms.ModelForm): msg += 'Please recheck that you have supplied a valid course id.' raise forms.ValidationError(msg) - # Now, try and discern if it is a Studio course - HTML editor doesn't work with XML courses - is_studio_course = modulestore().get_modulestore_type(course_key) != ModuleStoreEnum.Type.xml - if not is_studio_course: - msg = "Course Email feature is only available for courses authored in Studio. " - msg += '"{0}" appears to be an XML backed course.'.format(course_key.to_deprecated_string()) - raise forms.ValidationError(msg) - return course_key diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 252bb598cf..1b63685156 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -453,10 +453,8 @@ def get_studio_url(course, page): Args: course (CourseDescriptor) """ - is_studio_course = course.course_edit_method == "Studio" - is_mongo_course = modulestore().get_modulestore_type(course.id) != ModuleStoreEnum.Type.xml studio_link = None - if is_studio_course and is_mongo_course: + if course.course_edit_method == "Studio": studio_link = get_cms_course_link(course, page) return studio_link diff --git a/lms/djangoapps/dashboard/sysadmin.py b/lms/djangoapps/dashboard/sysadmin.py index 3d89a86f77..4273035d9e 100644 --- a/lms/djangoapps/dashboard/sysadmin.py +++ b/lms/djangoapps/dashboard/sysadmin.py @@ -491,23 +491,7 @@ class Courses(SysadminDashboardView): escape(str(err)) ) - is_xml_course = (modulestore().get_modulestore_type(course_key) == ModuleStoreEnum.Type.xml) - if course_found and is_xml_course: - cdir = course.data_dir - self.def_ms.courses.pop(cdir) - - # now move the directory (don't actually delete it) - new_dir = "{course_dir}_deleted_{timestamp}".format( - course_dir=cdir, - timestamp=int(time.time()) - ) - os.rename(settings.DATA_DIR / cdir, settings.DATA_DIR / new_dir) - - self.msg += (u"Deleted " - u"{0} = {1} ({2})".format( - cdir, course.id, course.display_name)) - - elif course_found and not is_xml_course: + if course_found: # delete course that is stored with mongodb backend self.def_ms.delete_course(course.id, request.user.id) # don't delete user permission groups, though diff --git a/lms/djangoapps/instructor/views/tools.py b/lms/djangoapps/instructor/views/tools.py index d1bbb66269..1bfb96d910 100644 --- a/lms/djangoapps/instructor/views/tools.py +++ b/lms/djangoapps/instructor/views/tools.py @@ -66,10 +66,9 @@ def bulk_email_is_enabled_for_course(course_id): """ bulk_email_enabled_globally = (settings.FEATURES['ENABLE_INSTRUCTOR_EMAIL'] is True) - is_studio_course = (modulestore().get_modulestore_type(course_id) != ModuleStoreEnum.Type.xml) bulk_email_enabled_for_course = CourseAuthorization.instructor_email_enabled(course_id) - if bulk_email_enabled_globally and is_studio_course and bulk_email_enabled_for_course: + if bulk_email_enabled_globally and bulk_email_enabled_for_course: return True return False diff --git a/lms/lib/xblock/test/test_mixin.py b/lms/lib/xblock/test/test_mixin.py index 6cb4d3c52c..3b501b3737 100644 --- a/lms/lib/xblock/test/test_mixin.py +++ b/lms/lib/xblock/test/test_mixin.py @@ -159,19 +159,15 @@ class XBlockGetParentTest(LmsXBlockMixinTestCase): """ MODULESTORE = TEST_DATA_MIXED_MODULESTORE - @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split, ModuleStoreEnum.Type.xml) + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_parents(self, modulestore_type): with self.store.default_store(modulestore_type): # setting up our own local course tree here, since it needs to be # created with the correct modulestore type. - if modulestore_type == 'xml': - course_key = self.store.make_course_key('edX', 'toy', '2012_Fall') - else: - course_key = ToyCourseFactory.create(run='2012_Fall_copy').id + course_key = ToyCourseFactory.create().id course = self.store.get_course(course_key) - self.assertIsNone(course.get_parent()) def recurse(parent): diff --git a/openedx/core/lib/courses.py b/openedx/core/lib/courses.py index 2936cc93d8..ca5d3f13d8 100644 --- a/openedx/core/lib/courses.py +++ b/openedx/core/lib/courses.py @@ -13,7 +13,7 @@ from xmodule.modulestore import ModuleStoreEnum def course_image_url(course): """Try to look up the image url for the course. If it's not found, log an error and return the dead link""" - if course.static_asset_path or modulestore().get_modulestore_type(course.id) == ModuleStoreEnum.Type.xml: + if course.static_asset_path: # If we are a static course with the course_image attribute # set different than the default, return that path so that # courses can use custom course image paths, otherwise just diff --git a/openedx/core/lib/xblock_utils.py b/openedx/core/lib/xblock_utils.py index ab25b27639..2a0232436b 100644 --- a/openedx/core/lib/xblock_utils.py +++ b/openedx/core/lib/xblock_utils.py @@ -300,10 +300,9 @@ def add_staff_markup(user, has_instructor_access, disable_staff_debug_info, bloc # TODO: make this more general, eg use an XModule attribute instead if isinstance(block, VerticalBlock) and (not context or not context.get('child_of_vertical', False)): # check that the course is a mongo backed Studio course before doing work - is_mongo_course = modulestore().get_modulestore_type(block.location.course_key) != ModuleStoreEnum.Type.xml is_studio_course = block.course_edit_method == "Studio" - if is_studio_course and is_mongo_course: + if is_studio_course: # build edit link to unit in CMS. Can't use reverse here as lms doesn't load cms's urls.py edit_link = "//" + settings.CMS_BASE + '/container/' + unicode(block.location) From dce920fa70d6a01f8ddea015b36a59d554694e15 Mon Sep 17 00:00:00 2001 From: John Eskew Date: Mon, 28 Dec 2015 10:29:00 -0500 Subject: [PATCH 14/22] Fix PEP8 error --- common/lib/xmodule/xmodule/modulestore/xml.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 01aab51a5b..e839206250 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -875,7 +875,8 @@ class XMLModuleStore(ModuleStoreReadBase): Args: course_key: just for signature compatibility """ - return None #ModuleStoreEnum.Type.xml + # return ModuleStoreEnum.Type.xml + return None def get_courses_for_wiki(self, wiki_slug, **kwargs): """ From d1d449ee1d18744c1275913ba175f7cc074b6993 Mon Sep 17 00:00:00 2001 From: John Eskew Date: Fri, 4 Mar 2016 15:15:58 -0500 Subject: [PATCH 15/22] Remove comments that refer to XML courses. --- common/lib/xmodule/xmodule/modulestore/tests/django_utils.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py index faac630bd1..8e91f8d920 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py @@ -159,12 +159,10 @@ TEST_DATA_MIXED_MODULESTORE = mixed_store_config( # All store requests now go through mixed # Use this modulestore if you specifically want to test mongo and not a mocked modulestore. -# This modulestore definition below will not load any xml courses. TEST_DATA_MONGO_MODULESTORE = mixed_store_config(mkdtemp_clean(), {}) # All store requests now go through mixed # Use this modulestore if you specifically want to test split-mongo and not a mocked modulestore. -# This modulestore definition below will not load any xml courses. TEST_DATA_SPLIT_MODULESTORE = mixed_store_config( mkdtemp_clean(), {}, From e892b552bafbc93e5b3e2aea36dc200a242df4b9 Mon Sep 17 00:00:00 2001 From: John Eskew Date: Sat, 5 Mar 2016 11:03:07 -0500 Subject: [PATCH 16/22] Remove non-existent import. --- lms/djangoapps/courseware/tests/test_courses.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/courseware/tests/test_courses.py b/lms/djangoapps/courseware/tests/test_courses.py index 6976e376ca..b2ecc49e80 100644 --- a/lms/djangoapps/courseware/tests/test_courses.py +++ b/lms/djangoapps/courseware/tests/test_courses.py @@ -33,7 +33,8 @@ from xmodule.modulestore.django import _get_modulestore_branch_setting, modulest from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.xml_importer import import_course_from_xml from xmodule.modulestore.tests.django_utils import ( - ModuleStoreTestCase, TEST_DATA_MIXED_TOY_MODULESTORE + ModuleStoreTestCase, + TEST_DATA_MIXED_MODULESTORE ) from xmodule.modulestore.tests.factories import ( CourseFactory, ItemFactory, ToyCourseFactory, check_mongo_calls From 37822ee99fbbb49b864b084fe12682d33849068a Mon Sep 17 00:00:00 2001 From: John Eskew Date: Tue, 12 Apr 2016 13:42:32 -0400 Subject: [PATCH 17/22] Add XML_COURSE_DIRS back in. --- .../courseware/management/commands/tests/test_dump_course.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lms/djangoapps/courseware/management/commands/tests/test_dump_course.py b/lms/djangoapps/courseware/management/commands/tests/test_dump_course.py index 53a5b87a68..bbff7ff15e 100644 --- a/lms/djangoapps/courseware/management/commands/tests/test_dump_course.py +++ b/lms/djangoapps/courseware/management/commands/tests/test_dump_course.py @@ -24,6 +24,7 @@ from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.xml_importer import import_course_from_xml DATA_DIR = settings.COMMON_TEST_DATA_ROOT +XML_COURSE_DIRS = ['toy', 'simple'] @attr('shard_1') From bee0c52b362cfef703362ed5b2c5a16a06002393 Mon Sep 17 00:00:00 2001 From: John Eskew Date: Tue, 12 Apr 2016 15:16:00 -0400 Subject: [PATCH 18/22] Remove another XML course test. --- .../tests/test_mixed_modulestore.py | 3 +- .../courseware/tests/test_courses.py | 36 ++----------------- 2 files changed, 3 insertions(+), 36 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py index 5f1c8c0fd9..5932428a34 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -88,7 +88,6 @@ class CommonMixedModuleStoreSetup(CourseComparisonTest): 'collection': COLLECTION, 'asset_collection': ASSET_COLLECTION, } - MAPPINGS = {} OPTIONS = { 'stores': [ { @@ -241,7 +240,7 @@ class CommonMixedModuleStoreSetup(CourseComparisonTest): return self.store.has_changes(self.store.get_item(location)) # pylint: disable=dangerous-default-value - def _initialize_mixed(self, mappings=MAPPINGS, contentstore=None): + def _initialize_mixed(self, mappings={}, contentstore=None): """ initializes the mixed modulestore. """ diff --git a/lms/djangoapps/courseware/tests/test_courses.py b/lms/djangoapps/courseware/tests/test_courses.py index b2ecc49e80..a3cbf06be4 100644 --- a/lms/djangoapps/courseware/tests/test_courses.py +++ b/lms/djangoapps/courseware/tests/test_courses.py @@ -32,12 +32,9 @@ from student.tests.factories import UserFactory from xmodule.modulestore.django import _get_modulestore_branch_setting, modulestore from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.xml_importer import import_course_from_xml -from xmodule.modulestore.tests.django_utils import ( - ModuleStoreTestCase, - TEST_DATA_MIXED_MODULESTORE -) +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import ( - CourseFactory, ItemFactory, ToyCourseFactory, check_mongo_calls + CourseFactory, ItemFactory, check_mongo_calls ) from xmodule.tests.xml import factories as xml from xmodule.tests.xml import XModuleXmlImportTest @@ -307,35 +304,6 @@ class CoursesRenderTest(ModuleStoreTestCase): self.assertIn("this module is temporarily unavailable", course_about) -@attr('shard_1') -class XmlCoursesRenderTest(ModuleStoreTestCase): - """Test methods related to rendering courses content for an XML course.""" - MODULESTORE = TEST_DATA_MIXED_MODULESTORE - - def setUp(self): - """ - Make sure that course is reloaded every time--clear out the modulestore. - """ - super(XmlCoursesRenderTest, self).setUp() - self.toy_course_key = ToyCourseFactory.create().id - - def test_get_course_info_section_render(self): - course = get_course_by_id(self.toy_course_key) - request = get_request_for_user(UserFactory.create()) - - # Test render works okay. Note the href is different in XML courses. - course_info = get_course_info_section(request, request.user, course, 'handouts') - self.assertEqual(course_info, "Sample") - - # Test when render raises an exception - with mock.patch('courseware.courses.get_module') as mock_module_render: - mock_module_render.return_value = mock.MagicMock( - render=mock.Mock(side_effect=Exception('Render failed!')) - ) - course_info = get_course_info_section(request, request.user, course, 'handouts') - self.assertIn("this module is temporarily unavailable", course_info) - - @attr('shard_1') @ddt.ddt class CourseInstantiationTests(ModuleStoreTestCase): From 84da6e4c1faf0fa6a3591fd8a783821cb7445ec8 Mon Sep 17 00:00:00 2001 From: John Eskew Date: Thu, 21 Apr 2016 11:56:23 -0400 Subject: [PATCH 19/22] Address DaveO's comments. --- common/djangoapps/static_replace/__init__.py | 3 +-- .../xmodule/modulestore/tests/test_mixed_modulestore.py | 3 ++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/common/djangoapps/static_replace/__init__.py b/common/djangoapps/static_replace/__init__.py index 5a12317ea9..d98bea1b90 100644 --- a/common/djangoapps/static_replace/__init__.py +++ b/common/djangoapps/static_replace/__init__.py @@ -163,8 +163,7 @@ def replace_static_urls(text, data_directory=None, course_id=None, static_asset_ if settings.DEBUG and finders.find(rest, True): return original # if we're running with a MongoBacked store course_namespace is not None, then use studio style urls - elif (not static_asset_path) \ - and course_id: + elif (not static_asset_path) and course_id: # first look in the static file pipeline and see if we are trying to reference # a piece of static content which is in the edx-platform repo (e.g. JS associated with an xmodule) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py index 5932428a34..7944a27ef6 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -240,10 +240,11 @@ class CommonMixedModuleStoreSetup(CourseComparisonTest): return self.store.has_changes(self.store.get_item(location)) # pylint: disable=dangerous-default-value - def _initialize_mixed(self, mappings={}, contentstore=None): + def _initialize_mixed(self, mappings=None, contentstore=None): """ initializes the mixed modulestore. """ + mappings = mappings or {} self.store = MixedModuleStore( contentstore, create_modulestore_instance=create_modulestore_instance, mappings=mappings, From 88927c74d9222b69b264e7dad0c94a17784c7be0 Mon Sep 17 00:00:00 2001 From: John Eskew Date: Thu, 21 Apr 2016 14:39:29 -0400 Subject: [PATCH 20/22] Fix static tabs test. --- common/lib/xmodule/xmodule/modulestore/tests/factories.py | 1 - lms/djangoapps/courseware/tests/test_tabs.py | 8 +++----- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/factories.py b/common/lib/xmodule/xmodule/modulestore/tests/factories.py index 66e4ea081d..c7f6e91e68 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/factories.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/factories.py @@ -234,7 +234,6 @@ class ToyCourseFactory(SampleCourseFactory): user_id, toy_course.id, "course_info", "handouts", fields={"data": "Sample"} ) - ## TODO: Broken! These static tabs are never added to course.tabs? store.create_item( user_id, toy_course.id, "static_tab", "resources", fields={"display_name": "Resources"}, diff --git a/lms/djangoapps/courseware/tests/test_tabs.py b/lms/djangoapps/courseware/tests/test_tabs.py index f11d7c12d1..f7105cb3d6 100644 --- a/lms/djangoapps/courseware/tests/test_tabs.py +++ b/lms/djangoapps/courseware/tests/test_tabs.py @@ -241,7 +241,6 @@ class StaticTabDateTestCase(LoginEnrollmentTestCase, SharedModuleStoreTestCase): ) cls.course.tabs.append(xmodule_tabs.CourseTab.load('static_tab', name='New Tab', url_slug='new_tab')) cls.course.save() - cls.toy_course_key = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall') def test_logged_in(self): self.setup_user() @@ -262,16 +261,15 @@ class StaticTabDateTestCase(LoginEnrollmentTestCase, SharedModuleStoreTestCase): with self.assertRaises(Http404): static_tab(request, course_id='edX/toy', tab_slug='new_tab') - @skip("Broken! Never finds the 'resources' static tab when created by the ToyCourseFactory.") def test_get_static_tab_contents(self): self.setup_user() - course = get_course_by_id(self.toy_course_key) + course = get_course_by_id(self.course.id) request = get_request_for_user(self.user) - tab = xmodule_tabs.CourseTabList.get_tab_by_slug(course.tabs, 'resources') + tab = xmodule_tabs.CourseTabList.get_tab_by_slug(course.tabs, 'new_tab') # Test render works okay tab_content = get_static_tab_contents(request, course, tab) - self.assertIn(self.toy_course_key.to_deprecated_string(), tab_content) + self.assertIn(self.course.id.to_deprecated_string(), tab_content) self.assertIn('static_tab', tab_content) # Test when render raises an exception From 6c3cc453a8dad15e278f171da08e61af7fc6e9ce Mon Sep 17 00:00:00 2001 From: John Eskew Date: Thu, 21 Apr 2016 14:40:02 -0400 Subject: [PATCH 21/22] Remove unneeded list comprehension. --- .../tests/test_mixed_modulestore.py | 23 +++++++------------ 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py index 7944a27ef6..0b17c8dd43 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -265,21 +265,14 @@ class CommonMixedModuleStoreSetup(CourseComparisonTest): break self._initialize_mixed() - # convert to CourseKeys - self.course_locations = { - course_id: CourseLocator.from_string(course_id) - for course_id in [self.MONGO_COURSEID] - } - # and then to the root UsageKey - self.course_locations = { - course_id: course_key.make_usage_key('course', course_key.run) - for course_id, course_key in self.course_locations.iteritems() - } - - mongo_course_key = self.course_locations[self.MONGO_COURSEID].course_key - self.fake_location = self.store.make_course_key(mongo_course_key.org, mongo_course_key.course, mongo_course_key.run).make_usage_key('vertical', 'fake') - - self._create_course(self.course_locations[self.MONGO_COURSEID].course_key) + test_course_key = CourseLocator.from_string(self.MONGO_COURSEID) + test_course_key = test_course_key.make_usage_key('course', test_course_key.run).course_key + self.fake_location = self.store.make_course_key( + test_course_key.org, + test_course_key.course, + test_course_key.run + ).make_usage_key('vertical', 'fake') + self._create_course(test_course_key) self.assertEquals(default, self.store.get_modulestore_type(self.course.id)) From 8354e6d79cd16a7df6873028d8d17322bebe34a4 Mon Sep 17 00:00:00 2001 From: John Eskew Date: Wed, 27 Apr 2016 09:27:36 -0400 Subject: [PATCH 22/22] Fix quality issue. --- lms/djangoapps/courseware/tests/test_tabs.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lms/djangoapps/courseware/tests/test_tabs.py b/lms/djangoapps/courseware/tests/test_tabs.py index f7105cb3d6..c1597a2f52 100644 --- a/lms/djangoapps/courseware/tests/test_tabs.py +++ b/lms/djangoapps/courseware/tests/test_tabs.py @@ -7,7 +7,6 @@ from django.http import Http404 from mock import MagicMock, Mock, patch from nose.plugins.attrib import attr from opaque_keys.edx.locations import SlashSeparatedCourseKey -from unittest import skip from courseware.courses import get_course_by_id from courseware.tabs import (