From 583b36938e569f37fe52ff95ba6b5720a4a67730 Mon Sep 17 00:00:00 2001 From: Usama Sadiq Date: Tue, 24 May 2022 11:44:45 +0500 Subject: [PATCH] fix: fix ResourceWarnings (#30317) --- .../views/tests/test_import_export.py | 20 ++-- lms/djangoapps/badges/tests/test_models.py | 112 +++++++++++------- .../courseware/tests/test_video_mongo.py | 16 +-- .../lms_xblock/test/test_runtime.py | 9 +- .../enterprise_support/tests/test_admin.py | 14 ++- 5 files changed, 99 insertions(+), 72 deletions(-) diff --git a/cms/djangoapps/contentstore/views/tests/test_import_export.py b/cms/djangoapps/contentstore/views/tests/test_import_export.py index 0364a7b179..280ea4d469 100644 --- a/cms/djangoapps/contentstore/views/tests/test_import_export.py +++ b/cms/djangoapps/contentstore/views/tests/test_import_export.py @@ -861,15 +861,17 @@ class ExportTestCase(CourseTestCase): root_dir = path(tempfile.mkdtemp()) try: export_library_to_xml(self.store, contentstore(), lib_key, root_dir, name) - lib_xml = lxml.etree.XML(open(root_dir / name / LIBRARY_ROOT).read()) - self.assertEqual(lib_xml.get('org'), lib_key.org) - self.assertEqual(lib_xml.get('library'), lib_key.library) - block = lib_xml.find('video') - self.assertIsNotNone(block) - self.assertEqual(block.get('url_name'), video_block.url_name) - video_xml = lxml.etree.XML(open(root_dir / name / 'video' / video_block.url_name + '.xml').read()) - self.assertEqual(video_xml.tag, 'video') - self.assertEqual(video_xml.get('youtube_id_1_0'), youtube_id) + with open(root_dir / name / LIBRARY_ROOT) as xml_root: + lib_xml = lxml.etree.XML(xml_root.read()) + self.assertEqual(lib_xml.get('org'), lib_key.org) + self.assertEqual(lib_xml.get('library'), lib_key.library) + block = lib_xml.find('video') + self.assertIsNotNone(block) + self.assertEqual(block.get('url_name'), video_block.url_name) + with open(root_dir / name / 'video' / video_block.url_name + '.xml') as xml_block: + video_xml = lxml.etree.XML(xml_block.read()) + self.assertEqual(video_xml.tag, 'video') + self.assertEqual(video_xml.get('youtube_id_1_0'), youtube_id) finally: shutil.rmtree(root_dir / name) diff --git a/lms/djangoapps/badges/tests/test_models.py b/lms/djangoapps/badges/tests/test_models.py index 4957614d10..e57c2e1b23 100644 --- a/lms/djangoapps/badges/tests/test_models.py +++ b/lms/djangoapps/badges/tests/test_models.py @@ -3,6 +3,7 @@ Tests for the Badges app models. """ +import contextlib from unittest.mock import Mock, patch import pytest @@ -28,11 +29,13 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase # lint-a from xmodule.modulestore.tests.factories import CourseFactory # lint-amnesty, pylint: disable=wrong-import-order +@contextlib.contextmanager def get_image(name): """ Get one of the test images from the test data directory. """ - return ImageFile(open(f'{TEST_DATA_DIR}/badges/{name}.png', mode='rb')) # lint-amnesty, pylint: disable=bad-option-value, open-builtin + with open(f'{TEST_DATA_DIR}/badges/{name}.png', mode='rb') as fimage: + yield ImageFile(fimage) @override_settings(MEDIA_ROOT=TEST_DATA_ROOT) @@ -49,16 +52,21 @@ class BadgeImageConfigurationTest(TestCase): """ Verify that creating two configurations as default is not permitted. """ - CourseCompleteImageConfiguration(mode='test', icon=get_image('good'), default=True).save() - pytest.raises(ValidationError, CourseCompleteImageConfiguration(mode='test2', icon=get_image('good'), - default=True).full_clean) + with get_image('good') as image_handle: + CourseCompleteImageConfiguration(mode='test', icon=ImageFile(image_handle), default=True).save() + with get_image('good') as image_handle: + pytest.raises(ValidationError, CourseCompleteImageConfiguration(mode='test2', icon=ImageFile(image_handle), + default=True).full_clean) def test_runs_validator(self): """ Verify that the image validator is triggered when cleaning the model. """ - pytest.raises(ValidationError, CourseCompleteImageConfiguration(mode='test2', icon=get_image('unbalanced')) - .full_clean) + with get_image('unbalanced') as image_handle: + pytest.raises( + ValidationError, + CourseCompleteImageConfiguration(mode='test2', icon=ImageFile(image_handle)).full_clean + ) class DummyBackend: @@ -102,10 +110,11 @@ class BadgeClassTest(ModuleStoreTestCase): """ premade_badge_class = BadgeClassFactory.create() # Ignore additional parameters. This class already exists. - badge_class = BadgeClass.get_badge_class( - slug='test_slug', issuing_component='test_component', description='Attempted override', - criteria='test', display_name='Testola', image_file_handle=get_image('good') - ) + with get_image('good') as image_handle: + badge_class = BadgeClass.get_badge_class( + slug='test_slug', issuing_component='test_component', description='Attempted override', + criteria='test', display_name='Testola', image_file_handle=image_handle + ) # These defaults are set on the factory. assert badge_class.criteria == 'https://example.com/syllabus' assert badge_class.display_name == 'Test Badge' @@ -119,15 +128,17 @@ class BadgeClassTest(ModuleStoreTestCase): """ course_key = CourseFactory.create().location.course_key premade_badge_class = BadgeClassFactory.create(course_id=course_key) - badge_class = BadgeClass.get_badge_class( - slug='test_slug', issuing_component='test_component', description='Attempted override', - criteria='test', display_name='Testola', image_file_handle=get_image('good') - ) - course_badge_class = BadgeClass.get_badge_class( - slug='test_slug', issuing_component='test_component', description='Attempted override', - criteria='test', display_name='Testola', image_file_handle=get_image('good'), - course_id=course_key, - ) + with get_image('good') as image_handle: + badge_class = BadgeClass.get_badge_class( + slug='test_slug', issuing_component='test_component', description='Attempted override', + criteria='test', display_name='Testola', image_file_handle=image_handle + ) + with get_image('good') as image_handle: + course_badge_class = BadgeClass.get_badge_class( + slug='test_slug', issuing_component='test_component', description='Attempted override', + criteria='test', display_name='Testola', image_file_handle=image_handle, + course_id=course_key, + ) assert badge_class.id != course_badge_class.id assert course_badge_class.id == premade_badge_class.id @@ -138,21 +149,23 @@ class BadgeClassTest(ModuleStoreTestCase): """ course_key = CourseFactory.create(metadata={'issue_badges': False}).location.course_key with pytest.raises(CourseBadgesDisabledError): - BadgeClass.get_badge_class( - slug='test_slug', issuing_component='test_component', description='Attempted override', - criteria='test', display_name='Testola', image_file_handle=get_image('good'), - course_id=course_key, - ) + with get_image('good') as image_handle: + BadgeClass.get_badge_class( + slug='test_slug', issuing_component='test_component', description='Attempted override', + criteria='test', display_name='Testola', image_file_handle=image_handle, + course_id=course_key, + ) def test_get_badge_class_create(self): """ Verify fetching a badge creates it if it doesn't yet exist. """ - badge_class = BadgeClass.get_badge_class( - slug='new_slug', issuing_component='new_component', description='This is a test', - criteria='https://example.com/test_criteria', display_name='Super Badge', - image_file_handle=get_image('good') - ) + with get_image('good') as image_handle: + badge_class = BadgeClass.get_badge_class( + slug='new_slug', issuing_component='new_component', description='This is a test', + criteria='https://example.com/test_criteria', display_name='Super Badge', + image_file_handle=image_handle + ) # This should have been saved before being passed back. assert badge_class.id assert badge_class.slug == 'new_slug' @@ -183,21 +196,24 @@ class BadgeClassTest(ModuleStoreTestCase): Verify handing a broken image to get_badge_class raises a validation error upon creation. """ # TODO Test should be updated, this doc doesn't makes sense, the object eventually gets created - self.assertRaises( - ValidationError, - BadgeClass.get_badge_class, - slug='new_slug', issuing_component='new_component', description='This is a test', - criteria='https://example.com/test_criteria', display_name='Super Badge', - image_file_handle=get_image('unbalanced') - ) + with get_image('unbalanced') as image_handle: + self.assertRaises( + ValidationError, + BadgeClass.get_badge_class, + slug='new_slug', issuing_component='new_component', description='This is a test', + criteria='https://example.com/test_criteria', display_name='Super Badge', + image_file_handle=image_handle + ) def test_get_badge_class_data_validate(self): """ Verify handing incomplete data for required fields when making a badge class raises an Integrity error. """ - image = get_image('good') with pytest.raises(IntegrityError), self.allow_transaction_exception(): - BadgeClass.get_badge_class(slug='new_slug', issuing_component='new_component', image_file_handle=image) + with get_image('good') as image_handle: + BadgeClass.get_badge_class( + slug='new_slug', issuing_component='new_component', image_file_handle=image_handle + ) def test_get_for_user(self): """ @@ -229,8 +245,13 @@ class BadgeClassTest(ModuleStoreTestCase): """ Verify that the image validator is triggered when cleaning the model. """ - pytest.raises(ValidationError, BadgeClass(slug='test', issuing_component='test2', criteria='test3', - description='test4', image=get_image('unbalanced')).full_clean) + with get_image('unbalanced') as image_handle: + pytest.raises( + ValidationError, + BadgeClass( + slug='test', issuing_component='test2', criteria='test3', + description='test4', image=ImageFile(image_handle)).full_clean + ) class BadgeAssertionTest(ModuleStoreTestCase): @@ -272,18 +293,19 @@ class ValidBadgeImageTest(TestCase): """ Verify that saving a valid badge image is no problem. """ - validate_badge_image(get_image('good')) + with get_image('good') as image_handle: + validate_badge_image(ImageFile(image_handle)) def test_unbalanced_image(self): """ Verify that setting an image with an uneven width and height raises an error. """ - unbalanced = ImageFile(get_image('unbalanced')) - self.assertRaises(ValidationError, validate_badge_image, unbalanced) + with get_image('unbalanced') as image_handle: + self.assertRaises(ValidationError, validate_badge_image, ImageFile(image_handle)) def test_large_image(self): """ Verify that setting an image that is too big raises an error. """ - large = get_image('large') - self.assertRaises(ValidationError, validate_badge_image, large) + with get_image('large') as image_handle: + self.assertRaises(ValidationError, validate_badge_image, ImageFile(image_handle)) diff --git a/lms/djangoapps/courseware/tests/test_video_mongo.py b/lms/djangoapps/courseware/tests/test_video_mongo.py index 129471f92e..c84bf675b7 100644 --- a/lms/djangoapps/courseware/tests/test_video_mongo.py +++ b/lms/djangoapps/courseware/tests/test_video_mongo.py @@ -1693,10 +1693,10 @@ class VideoBlockTest(TestCase, VideoBlockTestBase): assert [transcript_file_name] == self.file_system.listdir(EXPORT_IMPORT_STATIC_DIR) # Also verify the content of created transcript file. - expected_transcript_content = File(open(expected_transcript_path)).read() - transcript = get_video_transcript_data(video_id=self.descriptor.edx_video_id, language_code=language_code) - - assert transcript['content'].decode('utf-8') == expected_transcript_content + with open(expected_transcript_path) as transcript_path: + expected_transcript_content = File(transcript_path).read() + transcript = get_video_transcript_data(video_id=self.descriptor.edx_video_id, language_code=language_code) + assert transcript['content'].decode('utf-8') == expected_transcript_content @ddt.data( (['en', 'da'], 'test_sub', ''), @@ -1754,10 +1754,10 @@ class VideoBlockTest(TestCase, VideoBlockTestBase): combine(self.temp_dir, EXPORT_IMPORT_COURSE_DIR), combine(EXPORT_IMPORT_STATIC_DIR, expected_transcripts[language]) ) - expected_transcript_content = File(open(expected_transcript_path)).read() - transcript = get_video_transcript_data(video_id=self.descriptor.edx_video_id, language_code=language) - - assert transcript['content'].decode('utf-8') == expected_transcript_content + with open(expected_transcript_path) as transcript_path: + expected_transcript_content = File(transcript_path).read() + transcript = get_video_transcript_data(video_id=self.descriptor.edx_video_id, language_code=language) + assert transcript['content'].decode('utf-8') == expected_transcript_content def test_export_val_data_not_found(self): """ diff --git a/lms/djangoapps/lms_xblock/test/test_runtime.py b/lms/djangoapps/lms_xblock/test/test_runtime.py index d85c9e5816..ca0ceb8910 100644 --- a/lms/djangoapps/lms_xblock/test/test_runtime.py +++ b/lms/djangoapps/lms_xblock/test/test_runtime.py @@ -207,10 +207,11 @@ class TestBadgingService(ModuleStoreTestCase): premade_badge_class = BadgeClassFactory.create() # Ignore additional parameters. This class already exists. # We should get back the first class we created, rather than a new one. - badge_class = badge_service.get_badge_class( - slug='test_slug', issuing_component='test_component', description='Attempted override', - criteria='test', display_name='Testola', image_file_handle=get_image('good') - ) + with get_image('good') as image_handle: + badge_class = badge_service.get_badge_class( + slug='test_slug', issuing_component='test_component', description='Attempted override', + criteria='test', display_name='Testola', image_file_handle=image_handle + ) # These defaults are set on the factory. assert badge_class.criteria == 'https://example.com/syllabus' assert badge_class.display_name == 'Test Badge' diff --git a/openedx/features/enterprise_support/tests/test_admin.py b/openedx/features/enterprise_support/tests/test_admin.py index 4946701cc9..b5495670d1 100644 --- a/openedx/features/enterprise_support/tests/test_admin.py +++ b/openedx/features/enterprise_support/tests/test_admin.py @@ -93,16 +93,18 @@ class EnrollmentAttributeOverrideViewTest(ModuleStoreTestCase): Tests that HTTP POST is working as expected when creating new attributes and updating. """ csv_path = self.create_csv() - post_data = {'csv_file': open(csv_path)} - response = self.client.post(self.view_url, data=post_data) - assert response.status_code == 302 + with open(csv_path) as csv_file: + post_data = {'csv_file': csv_file} + response = self.client.post(self.view_url, data=post_data) + assert response.status_code == 302 self.verify_enrollment_attributes() # override existing csv_path = self.create_csv(data=self.csv_data_for_existing_attributes) - post_data = {'csv_file': open(csv_path)} - response = self.client.post(self.view_url, data=post_data) - assert response.status_code == 302 + with open(csv_path) as csv_file: + post_data = {'csv_file': csv_file} + response = self.client.post(self.view_url, data=post_data) + assert response.status_code == 302 self.verify_enrollment_attributes(data=self.csv_data_for_existing_attributes) def test_post_with_no_csv(self):