From 3647c17911a5cec77711ad4962fe1b6c2741466c Mon Sep 17 00:00:00 2001 From: Jesse Zoldak Date: Tue, 12 Jul 2016 16:43:50 -0400 Subject: [PATCH 1/2] Make the temp dir for instructor dashboard local files uniquely named. EV-56 --- .../instructor_task/tests/test_base.py | 36 +++++++++++++------ lms/envs/test.py | 4 --- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/lms/djangoapps/instructor_task/tests/test_base.py b/lms/djangoapps/instructor_task/tests/test_base.py index 3bedf84816..5ca8eb0d6a 100644 --- a/lms/djangoapps/instructor_task/tests/test_base.py +++ b/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): """ diff --git a/lms/envs/test.py b/lms/envs/test.py index b2dea916b8..0b73ce4cef 100644 --- a/lms/envs/test.py +++ b/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 From 5600a9bd0cd707e6c10a6a455235c4535a48daf8 Mon Sep 17 00:00:00 2001 From: Jesse Zoldak Date: Fri, 15 Jul 2016 16:59:53 -0400 Subject: [PATCH 2/2] Fix instructor_task model tests to use unique temp dirs --- .../instructor_task/tests/test_models.py | 37 ++++++++----------- 1 file changed, 15 insertions(+), 22 deletions(-) diff --git a/lms/djangoapps/instructor_task/tests/test_models.py b/lms/djangoapps/instructor_task/tests/test_models.py index ba21840861..e1bda80ddd 100644 --- a/lms/djangoapps/instructor_task/tests/test_models.py +++ b/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')