Merge pull request #12985 from edx/zoldak/EV-56

Make the temp dir for instructor dashboard local files uniquely named EV-56

lms/djangoapps/instructor_task/tests/test_base.py

@@ -4,8 +4,9 @@ Base test classes for LMS instructor-initiated background tasks
 """
 import os
 import json
-from mock import Mock
+from mock import Mock, patch
 import shutil
+from tempfile import mkdtemp
 import unicodecsv
 from uuid import uuid4
 
@@ -290,15 +291,30 @@ class TestReportMixin(object):
     """
     Cleans up after tests that place files in the reports directory.
     """
-    def tearDown(self):
-        report_store = ReportStore.from_config(config_name='GRADES_DOWNLOAD')
-        try:
-            reports_download_path = report_store.storage.path('')
-        except NotImplementedError:
-            pass  # storage backend does not use the local filesystem
-        else:
-            if os.path.exists(reports_download_path):
-                shutil.rmtree(reports_download_path)
+    def setUp(self):
+
+        def clean_up_tmpdir():
+            """Remove temporary directory created for instructor task models."""
+            if os.path.exists(self.tmp_dir):
+                shutil.rmtree(self.tmp_dir)
+
+        super(TestReportMixin, self).setUp()
+
+        # Ensure that working with the temp directories in tests is thread safe
+        # by creating a unique temporary directory for each testcase.
+        self.tmp_dir = mkdtemp()
+
+        mock_grades_download = {'STORAGE_TYPE': 'localfs', 'BUCKET': 'test-grades', 'ROOT_PATH': self.tmp_dir}
+        self.grades_patch = patch.dict('django.conf.settings.GRADES_DOWNLOAD', mock_grades_download)
+        self.grades_patch.start()
+        self.addCleanup(self.grades_patch.stop)
+
+        mock_fin_report = {'STORAGE_TYPE': 'localfs', 'BUCKET': 'test-financial-reports', 'ROOT_PATH': self.tmp_dir}
+        self.reports_patch = patch.dict('django.conf.settings.FINANCIAL_REPORTS', mock_fin_report)
+        self.reports_patch.start()
+        self.addCleanup(self.reports_patch.stop)
+
+        self.addCleanup(clean_up_tmpdir)
 
     def verify_rows_in_csv(self, expected_rows, file_index=0, verify_order=True, ignore_other_columns=False):
         """

lms/djangoapps/instructor_task/tests/test_models.py

@@ -1,7 +1,7 @@
 """
 Tests for instructor_task/models.py.
 """
-
+import copy
 from cStringIO import StringIO
 import time
 
@@ -16,21 +16,6 @@ from instructor_task.tests.test_base import TestReportMixin
 from opaque_keys.edx.locator import CourseLocator
 
 
-LOCAL_SETTINGS = {
-    'STORAGE_KWARGS': {
-        'location': settings.GRADES_DOWNLOAD['ROOT_PATH'],
-    },
-}
-
-S3_SETTINGS = {
-    'STORAGE_CLASS': 'storages.backends.s3boto.S3BotoStorage',
-    'STORAGE_KWARGS': {
-        'bucket': settings.GRADES_DOWNLOAD['BUCKET'],
-        'location': settings.GRADES_DOWNLOAD['ROOT_PATH'],
-    },
-}
-
-
 class ReportStoreTestMixin(object):
     """
     Mixin for report store tests.
@@ -92,7 +77,6 @@ class S3ReportStoreTestCase(MockS3Mixin, ReportStoreTestMixin, TestReportMixin,
         return ReportStore.from_config(config_name='GRADES_DOWNLOAD')
 
 
-@override_settings(GRADES_DOWNLOAD=LOCAL_SETTINGS)
 class DjangoStorageReportStoreLocalTestCase(ReportStoreTestMixin, TestReportMixin, SimpleTestCase):
     """
     Test the DjangoStorageReportStore implementation using the local
@@ -103,10 +87,12 @@ class DjangoStorageReportStoreLocalTestCase(ReportStoreTestMixin, TestReportMixi
         Create and return a DjangoStorageReportStore configured to use the
         local filesystem for storage.
         """
-        return ReportStore.from_config(config_name='GRADES_DOWNLOAD')
+        test_settings = copy.deepcopy(settings.GRADES_DOWNLOAD)
+        test_settings['STORAGE_KWARGS'] = {'location': settings.GRADES_DOWNLOAD['ROOT_PATH']}
+        with override_settings(GRADES_DOWNLOAD=test_settings):
+            return ReportStore.from_config(config_name='GRADES_DOWNLOAD')
 
 
-@override_settings(GRADES_DOWNLOAD=S3_SETTINGS)
 class DjangoStorageReportStoreS3TestCase(MockS3Mixin, ReportStoreTestMixin, TestReportMixin, SimpleTestCase):
     """
     Test the DjangoStorageReportStore implementation using S3 stubs.
@@ -116,6 +102,13 @@ class DjangoStorageReportStoreS3TestCase(MockS3Mixin, ReportStoreTestMixin, Test
         Create and return a DjangoStorageReportStore configured to use S3 for
         storage.
         """
-        connection = boto.connect_s3()
-        connection.create_bucket(settings.GRADES_DOWNLOAD['STORAGE_KWARGS']['bucket'])
-        return ReportStore.from_config(config_name='GRADES_DOWNLOAD')
+        test_settings = copy.deepcopy(settings.GRADES_DOWNLOAD)
+        test_settings['STORAGE_CLASS'] = 'storages.backends.s3boto.S3BotoStorage'
+        test_settings['STORAGE_KWARGS'] = {
+            'bucket': settings.GRADES_DOWNLOAD['BUCKET'],
+            'location': settings.GRADES_DOWNLOAD['ROOT_PATH'],
+        }
+        with override_settings(GRADES_DOWNLOAD=test_settings):
+            connection = boto.connect_s3()
+            connection.create_bucket(settings.GRADES_DOWNLOAD['STORAGE_KWARGS']['bucket'])
+            return ReportStore.from_config(config_name='GRADES_DOWNLOAD')

lms/envs/test.py

@@ -70,10 +70,6 @@ FEATURES['ENABLE_VERIFIED_CERTIFICATES'] = True
 FEATURES['ENABLE_GRADE_DOWNLOADS'] = True
 FEATURES['ALLOW_COURSE_STAFF_GRADE_DOWNLOADS'] = True
 
-GRADES_DOWNLOAD['ROOT_PATH'] += "-{}".format(os.getpid())
-FINANCIAL_REPORTS['ROOT_PATH'] += "-{}".format(os.getpid())
-
-
 # Toggles embargo on for testing
 FEATURES['EMBARGO'] = True