diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py b/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py index 9e15440f11..71274aae5f 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py @@ -1,16 +1,37 @@ +""" +Tests around our XML modulestore, including importing +well-formed and not-well-formed XML. +""" import os.path +import unittest +from glob import glob +from mock import patch from nose.tools import assert_raises, assert_equals # pylint: disable=E0611 from xmodule.course_module import CourseDescriptor from xmodule.modulestore.xml import XMLModuleStore -from xmodule.modulestore import XML_MODULESTORE_TYPE +from xmodule.modulestore import Location, XML_MODULESTORE_TYPE from .test_modulestore import check_path_to_location from xmodule.tests import DATA_DIR -class TestXMLModuleStore(object): +def glob_tildes_at_end(path): + """ + A wrapper for the `glob.glob` function, but it always returns + files that end in a tilde (~) at the end of the list of results. + """ + result = glob(path) + with_tildes = [f for f in result if f.endswith("~")] + no_tildes = [f for f in result if not f.endswith("~")] + return no_tildes + with_tildes + + +class TestXMLModuleStore(unittest.TestCase): + """ + Test around the XML modulestore + """ def test_path_to_location(self): """Make sure that path_to_location works properly""" @@ -42,3 +63,18 @@ class TestXMLModuleStore(object): location = CourseDescriptor.id_to_location("edX/toy/2012_Fall") errors = modulestore.get_item_errors(location) assert errors == [] + + @patch("xmodule.modulestore.xml.glob.glob", side_effect=glob_tildes_at_end) + def test_tilde_files_ignored(self, _fake_glob): + modulestore = XMLModuleStore(DATA_DIR, course_dirs=['tilde'], load_error_modules=False) + course_module = modulestore.modules['edX/tilde/2012_Fall'] + about_location = Location({ + 'tag': 'i4x', + 'org': 'edX', + 'course': 'tilde', + 'category': 'about', + 'name': 'index', + }) + about_module = course_module[about_location] + self.assertIn("GREEN", about_module.data) + self.assertNotIn("RED", about_module.data) diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 0cdaf7f7e3..5b55fa1807 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -405,8 +405,9 @@ class XMLModuleStore(ModuleStoreReadBase): try: course_descriptor = self.load_course(course_dir, errorlog.tracker) except Exception as e: - msg = "ERROR: Failed to load course '{0}': {1}".format(course_dir.encode("utf-8"), - unicode(e)) + msg = "ERROR: Failed to load course '{0}': {1}".format( + course_dir.encode("utf-8"), unicode(e) + ) log.exception(msg) errorlog.tracker(msg) @@ -568,6 +569,9 @@ class XMLModuleStore(ModuleStoreReadBase): if not os.path.isfile(filepath): continue + if filepath.endswith('~'): # skip *~ files + continue + with open(filepath) as f: try: html = f.read().decode('utf-8') diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index 47dedecc6b..03be931d2b 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -39,6 +39,12 @@ def import_static_content( for filename in filenames: content_path = os.path.join(dirname, filename) + + if filename.endswith('~'): + if verbose: + log.debug('skipping static content %s...', content_path) + continue + if verbose: log.debug('importing static content %s...', content_path) diff --git a/common/lib/xmodule/xmodule/tests/test_import_static.py b/common/lib/xmodule/xmodule/tests/test_import_static.py new file mode 100644 index 0000000000..f19d41aaf4 --- /dev/null +++ b/common/lib/xmodule/xmodule/tests/test_import_static.py @@ -0,0 +1,23 @@ +""" +Tests that check that we ignore the appropriate files when importing courses. +""" +import unittest +from mock import Mock +from xmodule.modulestore import Location +from xmodule.modulestore.xml_importer import import_static_content +from xmodule.tests import DATA_DIR + + +class IgnoredFilesTestCase(unittest.TestCase): + "Tests for ignored files" + def test_ignore_tilde_static_files(self): + course_dir = DATA_DIR / "tilde" + loc = Location("edX", "tilde", "Fall_2012") + content_store = Mock() + content_store.generate_thumbnail.return_value = ("content", "location") + import_static_content(Mock(), Mock(), course_dir, content_store, loc) + saved_static_content = [call[0][0] for call in content_store.save.call_args_list] + name_val = {sc.name: sc.data for sc in saved_static_content} + self.assertIn("example.txt", name_val) + self.assertNotIn("example.txt~", name_val) + self.assertIn("GREEN", name_val["example.txt"]) diff --git a/common/test/data/tilde/README.md b/common/test/data/tilde/README.md new file mode 100644 index 0000000000..7f24e3e75e --- /dev/null +++ b/common/test/data/tilde/README.md @@ -0,0 +1,9 @@ +DO NOT DELETE TILDE FILES + +This course simulates an export that has been edited by hand, where the editor's +text editor has left behind some backup files (about/index.html~ and +static/example.txt~). Normally, we do not commit files that end with tildes to +the repository, for precisely this reason -- they are backup files, and do +not belong with the content. However, in this case, we *need* these backup files +to be committed to the repository, so that we can exercise logic in the codebase +that checks for these sort of editor backup files and skips them on export. diff --git a/common/test/data/tilde/about/index.html b/common/test/data/tilde/about/index.html new file mode 100644 index 0000000000..d991f425fb --- /dev/null +++ b/common/test/data/tilde/about/index.html @@ -0,0 +1 @@ +GREEN diff --git a/common/test/data/tilde/about/index.html~ b/common/test/data/tilde/about/index.html~ new file mode 100644 index 0000000000..f24fc29aa6 --- /dev/null +++ b/common/test/data/tilde/about/index.html~ @@ -0,0 +1 @@ +RED diff --git a/common/test/data/tilde/course.xml b/common/test/data/tilde/course.xml new file mode 100644 index 0000000000..0489d7eed4 --- /dev/null +++ b/common/test/data/tilde/course.xml @@ -0,0 +1 @@ + diff --git a/common/test/data/tilde/course/2012_Fall.xml b/common/test/data/tilde/course/2012_Fall.xml new file mode 100644 index 0000000000..c9d2e8702d --- /dev/null +++ b/common/test/data/tilde/course/2012_Fall.xml @@ -0,0 +1,2 @@ + + diff --git a/common/test/data/tilde/static/example.txt b/common/test/data/tilde/static/example.txt new file mode 100644 index 0000000000..d991f425fb --- /dev/null +++ b/common/test/data/tilde/static/example.txt @@ -0,0 +1 @@ +GREEN diff --git a/common/test/data/tilde/static/example.txt~ b/common/test/data/tilde/static/example.txt~ new file mode 100644 index 0000000000..f24fc29aa6 --- /dev/null +++ b/common/test/data/tilde/static/example.txt~ @@ -0,0 +1 @@ +RED