diff --git a/cms/djangoapps/contentstore/management/commands/tests/test_cleanup_assets.py b/cms/djangoapps/contentstore/management/commands/tests/test_cleanup_assets.py index 4824779dab..0930c6571d 100644 --- a/cms/djangoapps/contentstore/management/commands/tests/test_cleanup_assets.py +++ b/cms/djangoapps/contentstore/management/commands/tests/test_cleanup_assets.py @@ -11,6 +11,9 @@ from xmodule.modulestore.django import modulestore from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.mongo.base import location_to_query from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.utils import ( + add_temp_files_from_dict, remove_temp_files_from_list, DOT_FILES_DICT +) from xmodule.modulestore.xml_importer import import_course_from_xml from django.conf import settings @@ -21,13 +24,16 @@ class ExportAllCourses(ModuleStoreTestCase): """ Tests assets cleanup for all courses. """ + course_dir = TEST_DATA_DIR / "course_ignore" + def setUp(self): """ Common setup. """ super(ExportAllCourses, self).setUp() - self.content_store = contentstore() # pylint: disable=protected-access self.module_store = modulestore()._get_modulestore_by_type(ModuleStoreEnum.Type.mongo) + self.addCleanup(remove_temp_files_from_list, DOT_FILES_DICT.keys(), self.course_dir / "static") + add_temp_files_from_dict(DOT_FILES_DICT, self.course_dir / "static") def test_export_all_courses(self): """ @@ -38,13 +44,13 @@ class ExportAllCourses(ModuleStoreTestCase): self.module_store, '**replace_user**', TEST_DATA_DIR, - ['dot-underscore'], + ['course_ignore'], static_content_store=self.content_store, do_import_static=True, verbose=True ) - course = self.module_store.get_course(CourseKey.from_string('/'.join(['edX', 'dot-underscore', '2014_Fall']))) + course = self.module_store.get_course(CourseKey.from_string('/'.join(['edX', 'course_ignore', '2014_Fall']))) self.assertIsNotNone(course) # check that there are two assets ['example.txt', '.example.txt'] in contentstore for imported course diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py b/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py index 6f3838c593..482e70f850 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py @@ -15,6 +15,9 @@ from xmodule.tests import DATA_DIR from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import CourseLocator from xmodule.modulestore.tests.test_modulestore import check_has_course_method +from xmodule.modulestore.tests.utils import ( + add_temp_files_from_dict, remove_temp_files_from_list, TILDA_FILES_DICT +) def glob_tildes_at_end(path): @@ -57,16 +60,6 @@ class TestXMLModuleStore(TestCase): errors = modulestore.get_course_errors(CourseKey.from_string("edX/toy/2012_Fall")) 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, source_dirs=['tilde'], load_error_modules=False) - about_location = CourseKey.from_string('edX/tilde/2012_Fall').make_usage_key( - 'about', 'index', - ) - about_module = modulestore.get_item(about_location) - self.assertIn("GREEN", about_module.data) - self.assertNotIn("RED", about_module.data) - def test_get_courses_for_wiki(self): """ Test the get_courses_for_wiki method @@ -147,3 +140,23 @@ class TestXMLModuleStore(TestCase): other_parent = store.get_item(other_parent_loc) # children rather than get_children b/c the instance returned by get_children != shared_item self.assertIn(shared_item_loc, other_parent.children) + + +class TestModuleStoreIgnore(TestXMLModuleStore): + shard = 2 + course_dir = DATA_DIR / "course_ignore" + + def setUp(self): + super(TestModuleStoreIgnore, self).setUp() + self.addCleanup(remove_temp_files_from_list, TILDA_FILES_DICT.keys(), self.course_dir / "static") + add_temp_files_from_dict(TILDA_FILES_DICT, self.course_dir / "static") + + @patch("xmodule.modulestore.xml.glob.glob", side_effect=glob_tildes_at_end) + def test_tilde_files_ignored(self, _fake_glob): + modulestore = XMLModuleStore(DATA_DIR, source_dirs=['course_ignore'], load_error_modules=False) + about_location = CourseKey.from_string('edX/course_ignore/2014_Fall').make_usage_key( + 'about', 'index', + ) + about_module = modulestore.get_item(about_location) + self.assertIn("GREEN", about_module.data) + self.assertNotIn("RED", about_module.data) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/utils.py b/common/lib/xmodule/xmodule/modulestore/tests/utils.py index 2ae0b06cd5..e729143872 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/utils.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/utils.py @@ -1,6 +1,8 @@ """ Helper classes and methods for running modulestore tests without Django. """ +import io +import os import random from contextlib import contextmanager, nested @@ -9,7 +11,6 @@ from path import Path as path from shutil import rmtree from tempfile import mkdtemp from unittest import TestCase -import os from xmodule.x_module import XModuleMixin from xmodule.contentstore.mongo import MongoContentStore @@ -73,6 +74,27 @@ def mock_tab_from_json(tab_dict): return tab_dict +def add_temp_files_from_dict(file_dict, dir): + """ + Takes in a dict formatted as: { file_name: content }, and adds files to directory + """ + for file_name in file_dict: + with io.open("{}/{}".format(dir, file_name), "w") as opened_file: + content = file_dict[file_name] + if content: + opened_file.write(unicode(content)) + + +def remove_temp_files_from_list(file_list, dir): + """ + Takes in a list of file names and removes them from dir if they exist + """ + for file_name in file_list: + file_path = "{}/{}".format(dir, file_name) + if os.path.exists(file_path): + os.remove(file_path) + + class MixedSplitTestCase(TestCase): """ Stripped-down version of ModuleStoreTestCase that can be used without Django @@ -471,6 +493,14 @@ SHORT_NAME_MAP = dict(zip(MODULESTORE_SETUPS, MODULESTORE_SHORTNAMES)) CONTENTSTORE_SETUPS = (MongoContentstoreBuilder(),) +DOT_FILES_DICT = { + ".DS_Store": None, + ".example.txt": "BLUE", +} +TILDA_FILES_DICT = { + "example.txt~": "RED" +} + class PureModulestoreTestCase(TestCase): """ diff --git a/common/lib/xmodule/xmodule/tests/test_import_static.py b/common/lib/xmodule/xmodule/tests/test_import_static.py index 86c2cecb9b..7db84f5c42 100644 --- a/common/lib/xmodule/xmodule/tests/test_import_static.py +++ b/common/lib/xmodule/xmodule/tests/test_import_static.py @@ -2,44 +2,40 @@ Tests that check that we ignore the appropriate files when importing courses. """ import unittest +import ddt from mock import Mock +import os from xmodule.modulestore.xml_importer import StaticContentImporter from opaque_keys.edx.locator import CourseLocator from xmodule.tests import DATA_DIR +from xmodule.modulestore.tests.utils import ( + add_temp_files_from_dict, remove_temp_files_from_list, DOT_FILES_DICT, TILDA_FILES_DICT +) class IgnoredFilesTestCase(unittest.TestCase): "Tests for ignored files" shard = 1 + course_dir = DATA_DIR / "course_ignore" + dict_list = [DOT_FILES_DICT, TILDA_FILES_DICT] - def test_ignore_tilde_static_files(self): - course_dir = DATA_DIR / "tilde" - course_id = CourseLocator("edX", "tilde", "Fall_2012") + def setUp(self): + super(IgnoredFilesTestCase, self).setUp() + for dictionary in self.dict_list: + self.addCleanup(remove_temp_files_from_list, dictionary.keys(), self.course_dir / "static") + add_temp_files_from_dict(dictionary, self.course_dir / "static") + + def test_sample_static_files(self): + """ + Test for to ensure Mac OS metadata files (filename starts with "._") as well + as files ending with "~" get ignored, while files starting with "." are not. + """ + course_id = CourseLocator("edX", "course_ignore", "2014_Fall") content_store = Mock() content_store.generate_thumbnail.return_value = ("content", "location") static_content_importer = StaticContentImporter( static_content_store=content_store, - course_data_path=course_dir, - target_id=course_id - ) - static_content_importer.import_static_content_directory() - 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"]) - - def test_ignore_dot_underscore_static_files(self): - """ - Test for ignored Mac OS metadata files (filename starts with "._") - """ - course_dir = DATA_DIR / "dot-underscore" - course_id = CourseLocator("edX", "dot-underscore", "2014_Fall") - content_store = Mock() - content_store.generate_thumbnail.return_value = ("content", "location") - static_content_importer = StaticContentImporter( - static_content_store=content_store, - course_data_path=course_dir, + course_data_path=self.course_dir, target_id=course_id ) static_content_importer.import_static_content_directory() @@ -47,7 +43,8 @@ class IgnoredFilesTestCase(unittest.TestCase): name_val = {sc.name: sc.data for sc in saved_static_content} self.assertIn("example.txt", name_val) self.assertIn(".example.txt", name_val) - self.assertNotIn("._example.txt", name_val) - self.assertNotIn(".DS_Store", name_val) self.assertIn("GREEN", name_val["example.txt"]) self.assertIn("BLUE", name_val[".example.txt"]) + self.assertNotIn("._example.txt", name_val) + self.assertNotIn(".DS_Store", name_val) + self.assertNotIn("example.txt~", name_val) diff --git a/common/test/data/course_ignore/README.md b/common/test/data/course_ignore/README.md new file mode 100644 index 0000000000..ac85b344ec --- /dev/null +++ b/common/test/data/course_ignore/README.md @@ -0,0 +1,6 @@ +IGNORING FILES + +This course gets used to simulate imports of a course that include unnecessary files +inside the static assets directory, such as metadata files from Mac OS (filename starts with ._), +or backup files from an editor (filename ends with ~). These files do not belong with the content +so they should not be imported, and any existing instances should be removed. diff --git a/common/test/data/dot-underscore/about/index.html b/common/test/data/course_ignore/about/index.html similarity index 100% rename from common/test/data/dot-underscore/about/index.html rename to common/test/data/course_ignore/about/index.html diff --git a/common/test/data/course_ignore/course.xml b/common/test/data/course_ignore/course.xml new file mode 100644 index 0000000000..7761c9811a --- /dev/null +++ b/common/test/data/course_ignore/course.xml @@ -0,0 +1 @@ + diff --git a/common/test/data/dot-underscore/course/2014_Fall.xml b/common/test/data/course_ignore/course/2014_Fall.xml similarity index 100% rename from common/test/data/dot-underscore/course/2014_Fall.xml rename to common/test/data/course_ignore/course/2014_Fall.xml diff --git a/common/test/data/dot-underscore/static/example.txt b/common/test/data/course_ignore/static/example.txt similarity index 100% rename from common/test/data/dot-underscore/static/example.txt rename to common/test/data/course_ignore/static/example.txt diff --git a/common/test/data/dot-underscore/README.md b/common/test/data/dot-underscore/README.md deleted file mode 100644 index dc54082345..0000000000 --- a/common/test/data/dot-underscore/README.md +++ /dev/null @@ -1,6 +0,0 @@ -IGNORE MAC METADATA FILES - -This course simulates an import of a course from a Mac OS that has some unnessary -metadata files (filename starts with ._) in assets (static/._example.txt). These -files do not belong with the content so skip them on import and also do a -cleanup for such already added assets. diff --git a/common/test/data/dot-underscore/course.xml b/common/test/data/dot-underscore/course.xml deleted file mode 100644 index 9cec88e74f..0000000000 --- a/common/test/data/dot-underscore/course.xml +++ /dev/null @@ -1 +0,0 @@ - diff --git a/common/test/data/dot-underscore/static/.DS_Store b/common/test/data/dot-underscore/static/.DS_Store deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/common/test/data/dot-underscore/static/.example.txt b/common/test/data/dot-underscore/static/.example.txt deleted file mode 100644 index cb9e9c6ba5..0000000000 --- a/common/test/data/dot-underscore/static/.example.txt +++ /dev/null @@ -1 +0,0 @@ -BLUE \ No newline at end of file diff --git a/common/test/data/tilde/README.md b/common/test/data/tilde/README.md deleted file mode 100644 index 7f24e3e75e..0000000000 --- a/common/test/data/tilde/README.md +++ /dev/null @@ -1,9 +0,0 @@ -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 deleted file mode 100644 index d991f425fb..0000000000 --- a/common/test/data/tilde/about/index.html +++ /dev/null @@ -1 +0,0 @@ -GREEN diff --git a/common/test/data/tilde/about/index.html~ b/common/test/data/tilde/about/index.html~ deleted file mode 100644 index f24fc29aa6..0000000000 --- a/common/test/data/tilde/about/index.html~ +++ /dev/null @@ -1 +0,0 @@ -RED diff --git a/common/test/data/tilde/course.xml b/common/test/data/tilde/course.xml deleted file mode 100644 index 0489d7eed4..0000000000 --- a/common/test/data/tilde/course.xml +++ /dev/null @@ -1 +0,0 @@ - diff --git a/common/test/data/tilde/course/2012_Fall.xml b/common/test/data/tilde/course/2012_Fall.xml deleted file mode 100644 index c9d2e8702d..0000000000 --- a/common/test/data/tilde/course/2012_Fall.xml +++ /dev/null @@ -1,2 +0,0 @@ - - diff --git a/common/test/data/tilde/static/example.txt b/common/test/data/tilde/static/example.txt deleted file mode 100644 index d991f425fb..0000000000 --- a/common/test/data/tilde/static/example.txt +++ /dev/null @@ -1 +0,0 @@ -GREEN diff --git a/common/test/data/tilde/static/example.txt~ b/common/test/data/tilde/static/example.txt~ deleted file mode 100644 index f24fc29aa6..0000000000 --- a/common/test/data/tilde/static/example.txt~ +++ /dev/null @@ -1 +0,0 @@ -RED