From bf0e5a270183b90becc56ec7f709deef7bb26fd6 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Mon, 11 Mar 2013 21:14:24 -0400 Subject: [PATCH] do a bunch of pylint improvements --- .../contentstore/tests/test_contentstore.py | 159 +++++++++--------- 1 file changed, 78 insertions(+), 81 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 58197b9762..99ef1169b1 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -9,10 +9,8 @@ from tempdir import mkdtemp_clean import json from fs.osfs import OSFS import copy -from mock import Mock -from json import dumps, loads +from json import loads -from student.models import Registration from django.contrib.auth.models import User from cms.djangoapps.contentstore.utils import get_modulestore @@ -22,12 +20,11 @@ from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.modulestore import Location from xmodule.modulestore.store_utilities import clone_course from xmodule.modulestore.store_utilities import delete_course -from xmodule.modulestore.django import modulestore, _MODULESTORES +from xmodule.modulestore.django import modulestore from xmodule.contentstore.django import contentstore from xmodule.templates import update_templates from xmodule.modulestore.xml_exporter import export_to_xml from xmodule.modulestore.xml_importer import import_from_xml -from xmodule.templates import update_templates from xmodule.capa_module import CapaDescriptor from xmodule.course_module import CourseDescriptor @@ -81,8 +78,8 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): def test_static_tab_reordering(self): import_from_xml(modulestore(), 'common/test/data/', ['full']) - ms = modulestore('direct') - course = ms.get_item(Location(['i4x', 'edX', 'full', 'course', '6.002_Spring_2012', None])) + module_store = modulestore('direct') + course = module_store.get_item(Location(['i4x', 'edX', 'full', 'course', '6.002_Spring_2012', None])) # reverse the ordering reverse_tabs = [] @@ -90,9 +87,9 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): if tab['type'] == 'static_tab': reverse_tabs.insert(0, 'i4x://edX/full/static_tab/{0}'.format(tab['url_slug'])) - resp = self.client.post(reverse('reorder_static_tabs'), json.dumps({'tabs': reverse_tabs}), "application/json") + self.client.post(reverse('reorder_static_tabs'), json.dumps({'tabs': reverse_tabs}), "application/json") - course = ms.get_item(Location(['i4x', 'edX', 'full', 'course', '6.002_Spring_2012', None])) + course = module_store.get_item(Location(['i4x', 'edX', 'full', 'course', '6.002_Spring_2012', None])) # compare to make sure that the tabs information is in the expected order after the server call course_tabs = [] @@ -105,28 +102,29 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): def test_delete(self): import_from_xml(modulestore(), 'common/test/data/', ['full']) - ms = modulestore('direct') - course = ms.get_item(Location(['i4x', 'edX', 'full', 'course', '6.002_Spring_2012', None])) + module_store = modulestore('direct') - sequential = ms.get_item(Location(['i4x', 'edX', 'full', 'sequential','Administrivia_and_Circuit_Elements', None])) + sequential = module_store.get_item(Location(['i4x', 'edX', 'full', 'sequential', 'Administrivia_and_Circuit_Elements', None])) - chapter = ms.get_item(Location(['i4x', 'edX', 'full', 'chapter','Week_1', None])) + chapter = module_store.get_item(Location(['i4x', 'edX', 'full', 'chapter','Week_1', None])) # make sure the parent no longer points to the child object which was deleted self.assertTrue(sequential.location.url() in chapter.definition['children']) - resp = self.client.post(reverse('delete_item'), json.dumps({'id': sequential.location.url(), 'delete_children':'true'}), "application/json") + self.client.post(reverse('delete_item'), + json.dumps({'id': sequential.location.url(), 'delete_children':'true'}), + "application/json") - bFound = False + found = False try: - sequential = ms.get_item(Location(['i4x', 'edX', 'full', 'sequential','Administrivia_and_Circuit_Elements', None])) - bFound = True + module_store.get_item(Location(['i4x', 'edX', 'full', 'sequential', 'Administrivia_and_Circuit_Elements', None])) + found = True except ItemNotFoundError: pass - self.assertFalse(bFound) + self.assertFalse(found) - chapter = ms.get_item(Location(['i4x', 'edX', 'full', 'chapter','Week_1', None])) + chapter = module_store.get_item(Location(['i4x', 'edX', 'full', 'chapter','Week_1', None])) # make sure the parent no longer points to the child object which was deleted self.assertFalse(sequential.location.url() in chapter.definition['children']) @@ -139,22 +137,22 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): while there is a base definition in /about/effort.html ''' import_from_xml(modulestore(), 'common/test/data/', ['full']) - ms = modulestore('direct') - effort = ms.get_item(Location(['i4x', 'edX', 'full', 'about', 'effort', None])) + module_store = modulestore('direct') + effort = module_store.get_item(Location(['i4x', 'edX', 'full', 'about', 'effort', None])) self.assertEqual(effort.definition['data'], '6 hours') # this one should be in a non-override folder - effort = ms.get_item(Location(['i4x', 'edX', 'full', 'about', 'end_date', None])) + effort = module_store.get_item(Location(['i4x', 'edX', 'full', 'about', 'end_date', None])) self.assertEqual(effort.definition['data'], 'TBD') def test_remove_hide_progress_tab(self): import_from_xml(modulestore(), 'common/test/data/', ['full']) - ms = modulestore('direct') - cs = contentstore() + module_store = modulestore('direct') + content_store = contentstore() source_location = CourseDescriptor.id_to_location('edX/full/6.002_Spring_2012') - course = ms.get_item(source_location) + course = module_store.get_item(source_location) self.assertNotIn('hide_progress_tab', course.metadata) def test_clone_course(self): @@ -173,19 +171,19 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): data = parse_json(resp) self.assertEqual(data['id'], 'i4x://MITx/999/course/Robot_Super_Course') - ms = modulestore('direct') - cs = contentstore() + module_store = modulestore('direct') + content_store = contentstore() source_location = CourseDescriptor.id_to_location('edX/full/6.002_Spring_2012') dest_location = CourseDescriptor.id_to_location('MITx/999/Robot_Super_Course') - clone_course(ms, cs, source_location, dest_location) + clone_course(module_store, content_store, source_location, dest_location) # now loop through all the units in the course and verify that the clone can render them, which # means the objects are at least present - items = ms.get_items(Location(['i4x', 'edX', 'full', 'vertical', None])) + items = module_store.get_items(Location(['i4x', 'edX', 'full', 'vertical', None])) self.assertGreater(len(items), 0) - clone_items = ms.get_items(Location(['i4x', 'MITx', '999', 'vertical', None])) + clone_items = module_store.get_items(Location(['i4x', 'MITx', '999', 'vertical', None])) self.assertGreater(len(clone_items), 0) for descriptor in items: new_loc = descriptor.location._replace(org='MITx', course='999') @@ -196,14 +194,14 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): def test_delete_course(self): import_from_xml(modulestore(), 'common/test/data/', ['full']) - ms = modulestore('direct') - cs = contentstore() + module_store = modulestore('direct') + content_store = contentstore() location = CourseDescriptor.id_to_location('edX/full/6.002_Spring_2012') - delete_course(ms, cs, location, commit=True) + delete_course(module_store, content_store, location, commit=True) - items = ms.get_items(Location(['i4x', 'edX', 'full', 'vertical', None])) + items = module_store.get_items(Location(['i4x', 'edX', 'full', 'vertical', None])) self.assertEqual(len(items), 0) def verify_content_existence(self, modulestore, root_dir, location, dirname, category_name, filename_suffix=''): @@ -218,10 +216,10 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): self.assertTrue(fs.exists(item.location.name + filename_suffix)) def test_export_course(self): - ms = modulestore('direct') - cs = contentstore() + module_store = modulestore('direct') + content_store = contentstore() - import_from_xml(ms, 'common/test/data/', ['full']) + import_from_xml(module_store, 'common/test/data/', ['full']) location = CourseDescriptor.id_to_location('edX/full/6.002_Spring_2012') root_dir = path(mkdtemp_clean()) @@ -229,43 +227,43 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): print 'Exporting to tempdir = {0}'.format(root_dir) # export out to a tempdir - export_to_xml(ms, cs, location, root_dir, 'test_export') + export_to_xml(module_store, content_store, location, root_dir, 'test_export') # check for static tabs - self.verify_content_existence(ms, root_dir, location, 'tabs', 'static_tab', '.html') + self.verify_content_existence(module_store, root_dir, location, 'tabs', 'static_tab', '.html') # check for custom_tags - self.verify_content_existence(ms, root_dir, location, 'info', 'course_info', '.html') + self.verify_content_existence(module_store, root_dir, location, 'info', 'course_info', '.html') # check for custom_tags - self.verify_content_existence(ms, root_dir, location, 'custom_tags', 'custom_tag_template') + self.verify_content_existence(module_store, root_dir, location, 'custom_tags', 'custom_tag_template') # check for graiding_policy.json fs = OSFS(root_dir / 'test_export/policies/6.002_Spring_2012') self.assertTrue(fs.exists('grading_policy.json')) - course = ms.get_item(location) + course = module_store.get_item(location) # compare what's on disk compared to what we have in our course - with fs.open('grading_policy.json','r') as grading_policy: - on_disk = loads(grading_policy.read()) + with fs.open('grading_policy.json', 'r') as grading_policy: + on_disk = loads(grading_policy.read()) self.assertEqual(on_disk, course.definition['data']['grading_policy']) #check for policy.json self.assertTrue(fs.exists('policy.json')) # compare what's on disk to what we have in the course module - with fs.open('policy.json','r') as course_policy: + with fs.open('policy.json', 'r') as course_policy: on_disk = loads(course_policy.read()) self.assertIn('course/6.002_Spring_2012', on_disk) self.assertEqual(on_disk['course/6.002_Spring_2012'], course.metadata) # remove old course - delete_course(ms, cs, location) + delete_course(module_store, content_store, location) # reimport - import_from_xml(ms, root_dir, ['test_export']) + import_from_xml(module_store, root_dir, ['test_export']) - items = ms.get_items(Location(['i4x', 'edX', 'full', 'vertical', None])) + items = module_store.get_items(Location(['i4x', 'edX', 'full', 'vertical', None])) self.assertGreater(len(items), 0) for descriptor in items: print "Checking {0}....".format(descriptor.location.url()) @@ -275,11 +273,11 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): shutil.rmtree(root_dir) def test_course_handouts_rewrites(self): - ms = modulestore('direct') - cs = contentstore() + module_store = modulestore('direct') + content_store = contentstore() # import a test course - import_from_xml(ms, 'common/test/data/', ['full']) + import_from_xml(module_store, 'common/test/data/', ['full']) handout_location = Location(['i4x', 'edX', 'full', 'course_info', 'handouts']) @@ -294,32 +292,32 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): self.assertContains(resp, '/c4x/edX/full/asset/handouts_schematic_tutorial.pdf') def test_export_course_with_unknown_metadata(self): - ms = modulestore('direct') - cs = contentstore() + module_store = modulestore('direct') + content_store = contentstore() - import_from_xml(ms, 'common/test/data/', ['full']) + import_from_xml(module_store, 'common/test/data/', ['full']) location = CourseDescriptor.id_to_location('edX/full/6.002_Spring_2012') root_dir = path(mkdtemp_clean()) - course = ms.get_item(location) + course = module_store.get_item(location) # add a bool piece of unknown metadata so we can verify we don't throw an exception course.metadata['new_metadata'] = True - ms.update_metadata(location, course.metadata) + module_store.update_metadata(location, course.metadata) print 'Exporting to tempdir = {0}'.format(root_dir) # export out to a tempdir - bExported = False + exported = False try: - export_to_xml(ms, cs, location, root_dir, 'test_export') - bExported = True + export_to_xml(module_store, content_store, location, root_dir, 'test_export') + exported = True except Exception: pass - self.assertTrue(bExported) + self.assertTrue(exported) class ContentStoreTest(ModuleStoreTestCase): """ @@ -458,7 +456,7 @@ class ContentStoreTest(ModuleStoreTestCase): def test_capa_module(self): """Test that a problem treats markdown specially.""" - course = CourseFactory.create(org='MITx', course='999', display_name='Robot Super Course') + CourseFactory.create(org='MITx', course='999', display_name='Robot Super Course') problem_data = { 'parent_location': 'i4x://MITx/999/course/Robot_Super_Course', @@ -480,10 +478,10 @@ class ContentStoreTest(ModuleStoreTestCase): def test_import_metadata_with_attempts_empty_string(self): import_from_xml(modulestore(), 'common/test/data/', ['simple']) - ms = modulestore('direct') + module_store = modulestore('direct') did_load_item = False try: - ms.get_item(Location(['i4x', 'edX', 'simple', 'problem', 'ps01-simple', None])) + module_store.get_item(Location(['i4x', 'edX', 'simple', 'problem', 'ps01-simple', None])) did_load_item = True except ItemNotFoundError: pass @@ -494,10 +492,10 @@ class ContentStoreTest(ModuleStoreTestCase): def test_metadata_inheritance(self): import_from_xml(modulestore(), 'common/test/data/', ['full']) - ms = modulestore('direct') - course = ms.get_item(Location(['i4x', 'edX', 'full', 'course', '6.002_Spring_2012', None])) + module_store = modulestore('direct') + course = module_store.get_item(Location(['i4x', 'edX', 'full', 'course', '6.002_Spring_2012', None])) - verticals = ms.get_items(['i4x', 'edX', 'full', 'vertical', None, None]) + verticals = module_store.get_items(['i4x', 'edX', 'full', 'vertical', None, None]) # let's assert on the metadata_inheritance on an existing vertical for vertical in verticals: @@ -508,15 +506,15 @@ class ContentStoreTest(ModuleStoreTestCase): new_component_location = Location('i4x', 'edX', 'full', 'html', 'new_component') source_template_location = Location('i4x', 'edx', 'templates', 'html', 'Blank_HTML_Page') - + # crate a new module and add it as a child to a vertical - ms.clone_item(source_template_location, new_component_location) + module_store.clone_item(source_template_location, new_component_location) parent = verticals[0] - ms.update_children(parent.location, parent.definition.get('children', []) + [new_component_location.url()]) + module_store.update_children(parent.location, parent.definition.get('children', []) + [new_component_location.url()]) # flush the cache - ms.get_cached_metadata_inheritance_tree(new_component_location, -1) - new_module = ms.get_item(new_component_location) + module_store.get_cached_metadata_inheritance_tree(new_component_location, -1) + new_module = module_store.get_item(new_component_location) # check for grace period definition which should be defined at the course level self.assertIn('graceperiod', new_module.metadata) @@ -529,11 +527,11 @@ class ContentStoreTest(ModuleStoreTestCase): # now let's define an override at the leaf node level # new_module.metadata['graceperiod'] = '1 day' - ms.update_metadata(new_module.location, new_module.metadata) + module_store.update_metadata(new_module.location, new_module.metadata) # flush the cache and refetch - ms.get_cached_metadata_inheritance_tree(new_component_location, -1) - new_module = ms.get_item(new_component_location) + module_store.get_cached_metadata_inheritance_tree(new_component_location, -1) + new_module = module_store.get_item(new_component_location) self.assertIn('graceperiod', new_module.metadata) self.assertEqual('1 day', new_module.metadata['graceperiod']) @@ -542,15 +540,15 @@ class ContentStoreTest(ModuleStoreTestCase): class TemplateTestCase(ModuleStoreTestCase): def test_template_cleanup(self): - ms = modulestore('direct') + module_store = modulestore('direct') # insert a bogus template in the store bogus_template_location = Location('i4x', 'edx', 'templates', 'html', 'bogus') source_template_location = Location('i4x', 'edx', 'templates', 'html', 'Blank_HTML_Page') - - ms.clone_item(source_template_location, bogus_template_location) - verify_create = ms.get_item(bogus_template_location) + module_store.clone_item(source_template_location, bogus_template_location) + + verify_create = module_store.get_item(bogus_template_location) self.assertIsNotNone(verify_create) # now run cleanup @@ -559,10 +557,9 @@ class TemplateTestCase(ModuleStoreTestCase): # now try to find dangling template, it should not be in DB any longer asserted = False try: - verify_create = ms.get_item(bogus_template_location) + verify_create = module_store.get_item(bogus_template_location) except ItemNotFoundError: asserted = True - self.assertTrue(asserted) - + self.assertTrue(asserted)