From 8c8810ee2c6d4e7cfea9b0a6c3a4aa275e6fd54b Mon Sep 17 00:00:00 2001 From: Ned Batchelder Date: Sat, 21 Mar 2015 17:26:25 -0400 Subject: [PATCH 1/4] Put gitignores for test_root/uploads in one place --- .gitignore | 2 -- test_root/uploads/.gitignore | 1 + 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index e05cae25c6..e8cd280ccd 100644 --- a/.gitignore +++ b/.gitignore @@ -59,8 +59,6 @@ jscover.log.* .tddium* common/test/data/test_unicode/static/ django-pyfs -test_root/uploads/*.txt -test_root/uploads/badges/*.png ### Installation artifacts *.egg-info diff --git a/test_root/uploads/.gitignore b/test_root/uploads/.gitignore index 39fc5e825c..409deae7b2 100644 --- a/test_root/uploads/.gitignore +++ b/test_root/uploads/.gitignore @@ -1,3 +1,4 @@ *.csv *.jpg *.png +*.txt From cb0624bfa1a69fd919345f2716d21c53c6738787 Mon Sep 17 00:00:00 2001 From: Ned Batchelder Date: Sat, 21 Mar 2015 19:24:56 -0400 Subject: [PATCH 2/4] Correct parent references in one test. --- lms/djangoapps/instructor_task/tests/test_tasks.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/instructor_task/tests/test_tasks.py b/lms/djangoapps/instructor_task/tests/test_tasks.py index 7d5448ee1d..2ec7ef7ec0 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks.py @@ -40,10 +40,10 @@ class TestTaskFailure(Exception): class TestInstructorTasks(InstructorTaskModuleTestCase): def setUp(self): - super(InstructorTaskModuleTestCase, self).setUp() + super(TestInstructorTasks, self).setUp() self.initialize_course() self.instructor = self.create_instructor('instructor') - self.location = InstructorTaskModuleTestCase.problem_location(PROBLEM_URL_NAME) + self.location = self.problem_location(PROBLEM_URL_NAME) def _create_input_entry(self, student_ident=None, use_problem_url=True, course_id=None): """Creates a InstructorTask entry for testing.""" From e6e5a8d812173c92fb63e40bb8564a598994d547 Mon Sep 17 00:00:00 2001 From: Ned Batchelder Date: Sun, 22 Mar 2015 07:26:21 -0400 Subject: [PATCH 3/4] Convert some try/finally to addCleanup --- .../xmodule/modulestore/tests/test_mongo.py | 42 ++++++++----------- .../commands/tests/test_dump_course.py | 10 ++--- .../tests/test_email_opt_in_list.py | 6 +-- 3 files changed, 22 insertions(+), 36 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py index 8d8f28b0d8..4397312c2f 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py @@ -571,12 +571,10 @@ class TestMongoModuleStore(TestMongoModuleStoreBase): self.content_store.find(location) root_dir = path(mkdtemp()) - try: - export_course_to_xml(self.draft_store, self.content_store, course_key, root_dir, 'test_export') - assert_true(path(root_dir / 'test_export/static/images/course_image.jpg').isfile()) - assert_true(path(root_dir / 'test_export/static/images_course_image.jpg').isfile()) - finally: - shutil.rmtree(root_dir) + self.addCleanup(shutil.rmtree, root_dir) + export_course_to_xml(self.draft_store, self.content_store, course_key, root_dir, 'test_export') + self.assertTrue(path(root_dir / 'test_export/static/images/course_image.jpg').isfile()) + self.assertTrue(path(root_dir / 'test_export/static/images_course_image.jpg').isfile()) @patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json) def test_export_course_image_nondefault(self, _from_json): @@ -588,12 +586,10 @@ class TestMongoModuleStore(TestMongoModuleStoreBase): assert_true(course.course_image, 'just_a_test.jpg') root_dir = path(mkdtemp()) - try: - export_course_to_xml(self.draft_store, self.content_store, course.id, root_dir, 'test_export') - assert_true(path(root_dir / 'test_export/static/just_a_test.jpg').isfile()) - assert_false(path(root_dir / 'test_export/static/images/course_image.jpg').isfile()) - finally: - shutil.rmtree(root_dir) + self.addCleanup(shutil.rmtree, root_dir) + export_course_to_xml(self.draft_store, self.content_store, course.id, root_dir, 'test_export') + self.assertTrue(path(root_dir / 'test_export/static/just_a_test.jpg').isfile()) + self.assertFalse(path(root_dir / 'test_export/static/images/course_image.jpg').isfile()) def test_course_without_image(self): """ @@ -602,12 +598,10 @@ class TestMongoModuleStore(TestMongoModuleStoreBase): """ course = self.draft_store.get_course(SlashSeparatedCourseKey('edX', 'simple_with_draft', '2012_Fall')) root_dir = path(mkdtemp()) - try: - export_course_to_xml(self.draft_store, self.content_store, course.id, root_dir, 'test_export') - assert_false(path(root_dir / 'test_export/static/images/course_image.jpg').isfile()) - assert_false(path(root_dir / 'test_export/static/images_course_image.jpg').isfile()) - finally: - shutil.rmtree(root_dir) + self.addCleanup(shutil.rmtree, root_dir) + export_course_to_xml(self.draft_store, self.content_store, course.id, root_dir, 'test_export') + self.assertFalse(path(root_dir / 'test_export/static/images/course_image.jpg').isfile()) + self.assertFalse(path(root_dir / 'test_export/static/images_course_image.jpg').isfile()) def _create_test_tree(self, name, user_id=None): """ @@ -728,15 +722,13 @@ class TestMongoModuleStore(TestMongoModuleStoreBase): self.assertEqual(unicode(component.link_to_location), unicode(problem_location)) root_dir = path(mkdtemp()) + self.addCleanup(shutil.rmtree, root_dir) # export_course_to_xml should work. - try: - export_course_to_xml( - self.draft_store, self.content_store, interface_location.course_key, - root_dir, 'test_export' - ) - finally: - shutil.rmtree(root_dir) + export_course_to_xml( + self.draft_store, self.content_store, interface_location.course_key, + root_dir, 'test_export' + ) def test_draft_modulestore_create_child_with_position(self): """ 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 9a04bb2505..ee10293ad7 100644 --- a/lms/djangoapps/courseware/management/commands/tests/test_dump_course.py +++ b/lms/djangoapps/courseware/management/commands/tests/test_dump_course.py @@ -175,14 +175,12 @@ class CommandsTestBase(ModuleStoreTestCase): def test_export_course(self): tmp_dir = path(mkdtemp()) + self.addCleanup(shutil.rmtree, tmp_dir) filename = tmp_dir / 'test.tar.gz' - try: - self.run_export_course(filename) - with tarfile.open(filename) as tar_file: - self.check_export_file(tar_file) - finally: - shutil.rmtree(tmp_dir) + self.run_export_course(filename) + with tarfile.open(filename) as tar_file: + self.check_export_file(tar_file) def test_export_course_stdout(self): output = self.run_export_course('-') diff --git a/openedx/core/djangoapps/user_api/management/tests/test_email_opt_in_list.py b/openedx/core/djangoapps/user_api/management/tests/test_email_opt_in_list.py index a776687292..f3c320c0db 100644 --- a/openedx/core/djangoapps/user_api/management/tests/test_email_opt_in_list.py +++ b/openedx/core/djangoapps/user_api/management/tests/test_email_opt_in_list.py @@ -331,11 +331,7 @@ class EmailOptInListTest(ModuleStoreTestCase): # Create a temporary directory for the output # Delete it when we're finished temp_dir_path = tempfile.mkdtemp() - - def _cleanup(): # pylint: disable=missing-docstring - shutil.rmtree(temp_dir_path) - - self.addCleanup(_cleanup) + self.addCleanup(shutil.rmtree, temp_dir_path) # Sanitize the arguments if other_names is None: From 514d85c1838c9e935086c522e2e233e62aa95f75 Mon Sep 17 00:00:00 2001 From: Ned Batchelder Date: Thu, 2 Jul 2015 19:31:26 -0400 Subject: [PATCH 4/4] Fix other temp dirs that are not cleaned up properly --- .../xmodule/xmodule/modulestore/tests/django_utils.py | 9 +++++---- .../modulestore/tests/test_modulestore_settings.py | 5 +++-- lms/envs/test.py | 5 +++-- 3 files changed, 11 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 1998ae2978..2a05094abc 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py @@ -4,7 +4,6 @@ Modulestore configuration for test cases. """ import datetime import pytz -from tempfile import mkdtemp from uuid import uuid4 from mock import patch @@ -16,6 +15,8 @@ from django.test.utils import override_settings from request_cache.middleware import RequestCache from courseware.field_overrides import OverrideFieldData # pylint: disable=import-error +from openedx.core.lib.tempdir import mkdtemp_clean + from xmodule.contentstore.django import _CONTENTSTORE from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore, clear_existing_modulestores @@ -184,13 +185,13 @@ TEST_DATA_MIXED_GRADED_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(), {}, include_xml=False) +TEST_DATA_MONGO_MODULESTORE = mixed_store_config(mkdtemp_clean(), {}, include_xml=False) # 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(), + mkdtemp_clean(), {}, include_xml=False, store_order=[StoreConstructors.split, StoreConstructors.draft] @@ -235,7 +236,7 @@ class ModuleStoreTestCase(TestCase): your `setUp()` method. """ - MODULESTORE = mixed_store_config(mkdtemp(), {}, include_xml=False) + MODULESTORE = mixed_store_config(mkdtemp_clean(), {}, include_xml=False) def setUp(self, **kwargs): """ diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_modulestore_settings.py b/common/lib/xmodule/xmodule/modulestore/tests/test_modulestore_settings.py index dea9ee8d52..35b232e083 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_modulestore_settings.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_modulestore_settings.py @@ -3,7 +3,8 @@ Tests for testing the modulestore settings migration code. """ import copy import ddt -from tempfile import mkdtemp + +from openedx.core.lib.tempdir import mkdtemp_clean from unittest import TestCase from xmodule.modulestore.modulestore_settings import ( @@ -37,7 +38,7 @@ class ModuleStoreSettingsMigration(TestCase): "collection": "modulestore", "db": "edxapp", "default_class": "xmodule.hidden_module.HiddenDescriptor", - "fs_root": mkdtemp(), + "fs_root": mkdtemp_clean(), "host": "localhost", "password": "password", "port": 27017, diff --git a/lms/envs/test.py b/lms/envs/test.py index 8c7cc0b0ca..9cf5251a37 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -21,10 +21,11 @@ sessions. Assumes structure: from .common import * import os from path import path -from tempfile import mkdtemp from uuid import uuid4 from warnings import filterwarnings, simplefilter +from openedx.core.lib.tempdir import mkdtemp_clean + # Silence noisy logs to make troubleshooting easier when tests fail. import logging LOG_OVERRIDES = [ @@ -151,7 +152,7 @@ update_module_store_settings( 'fs_root': TEST_ROOT / "data", }, xml_store_options={ - 'data_dir': mkdtemp(dir=TEST_ROOT), # never inadvertently load all the XML courses + 'data_dir': mkdtemp_clean(dir=TEST_ROOT), # never inadvertently load all the XML courses }, doc_store_settings={ 'host': MONGO_HOST,