test: remove unnecessary tempfile mocking

openedx · Jan 16, 2025 · 3f989f2 · 3f989f2
1 parent 296cd08
commit 3f989f2
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 20 deletions.
diff --git a/cms/djangoapps/contentstore/tasks.py b/cms/djangoapps/contentstore/tasks.py
@@ -1285,6 +1285,9 @@ def _check_broken_links(task_instance, user_id, course_key_string, language):
         broken_links_file = NamedTemporaryFile(prefix=file_name + '.', suffix='.json')
         LOGGER.debug(f'[Link Check] json file being generated at {broken_links_file.name}')
 
+        with open(broken_links_file.name, 'w') as file:
+            json.dump(broken_or_locked_urls, file, indent=4)
+
         _write_broken_links_to_file(broken_or_locked_urls, broken_links_file)
 
         artifact = UserTaskArtifact(status=task_instance.status, name='BrokenLinks')

diff --git a/cms/djangoapps/contentstore/tests/test_tasks.py b/cms/djangoapps/contentstore/tests/test_tasks.py
@@ -216,29 +216,29 @@ class MockCourseLinkCheckTask(Task):
     def __init__(self):
         self.status = mock.Mock()
 
-mock_urls = [
-    ["block-v1:edX+DemoX+Demo_Course+type@vertical+block@1", "http://example.com/valid"],
-    ["block-v1:edX+DemoX+Demo_Course+type@vertical+block@2", "http://example.com/invalid"]
-]
-
-expected_file_contents = [
-    ['block-v1:edX+DemoX+Demo_Course+type@vertical+block@1', 'http://example.com/valid', False],
-    ['block-v1:edX+DemoX+Demo_Course+type@vertical+block@2', 'http://example.com/invalid', False]
-]
-
 
 class CheckBrokenLinksTaskTest(ModuleStoreTestCase):
+    def setUp(self):
+        super().setUp()
+        self.mock_urls = [
+            ["block-v1:edX+DemoX+Demo_Course+type@vertical+block@1", "http://example.com/valid"],
+            ["block-v1:edX+DemoX+Demo_Course+type@vertical+block@2", "http://example.com/invalid"]
+        ]
+        self.expected_file_contents = [
+            ['block-v1:edX+DemoX+Demo_Course+type@vertical+block@1', 'http://example.com/valid', False],
+            ['block-v1:edX+DemoX+Demo_Course+type@vertical+block@2', 'http://example.com/invalid', False]
+        ]
+
     @mock.patch('cms.djangoapps.contentstore.tasks.UserTaskArtifact', autospec=True)
     @mock.patch('cms.djangoapps.contentstore.tasks.UserTaskStatus', autospec=True)
-    @mock.patch('cms.djangoapps.contentstore.tasks.NamedTemporaryFile', autospec=True)
     @mock.patch('cms.djangoapps.contentstore.tasks._scan_course_for_links')
     @mock.patch('cms.djangoapps.contentstore.tasks._save_broken_links_file', autospec=True)
     @mock.patch('cms.djangoapps.contentstore.tasks._write_broken_links_to_file', autospec=True)
     def test_check_broken_links_stores_broken_and_locked_urls(
         self,
         mock_write_links,
         mock_save_broken_links_file,
-        mock_scan_course_for_links, mock_named_temp_file,
+        mock_scan_course_for_links,
         _mock_user_task_status,
         mock_user_task_artifact
     ):
@@ -252,13 +252,8 @@ def test_check_broken_links_stores_broken_and_locked_urls(
         # Arrange
         mock_user = UserFactory.create(username='student', password='password')
         mock_course_key_string = "course-v1:edX+DemoX+Demo_Course"
-
-        ### Mock the NamedTemporaryFile
-        mock_broken_links_file = mock.MagicMock()
-        mock_broken_links_file.name = 'broken_links.json'
-        mock_named_temp_file.return_value.__enter__.return_value = mock_broken_links_file
         mock_task = MockCourseLinkCheckTask()
-        mock_scan_course_for_links.return_value = mock_urls
+        mock_scan_course_for_links.return_value = self.mock_urls
 
         # Act
         _check_broken_links(mock_task, mock_user.id, mock_course_key_string, 'en')  # pylint: disable=no-value-for-parameter
@@ -268,10 +263,10 @@ def test_check_broken_links_stores_broken_and_locked_urls(
         mock_user_task_artifact.assert_called_once_with(status=mock.ANY, name='BrokenLinks')
 
         ### Check that the correct links are written to the file
-        mock_write_links.assert_called_once_with(expected_file_contents, mock_named_temp_file.return_value)
+        mock_write_links.assert_called_once_with(self.expected_file_contents, mock.ANY)
 
         ### Check that _save_broken_links_file was called with the correct arguments
-        mock_save_broken_links_file.assert_called_once_with(mock_user_task_artifact.return_value, mock_named_temp_file.return_value)
+        mock_save_broken_links_file.assert_called_once_with(mock_user_task_artifact.return_value, mock.ANY)
 
 
     def test_user_does_not_exist_raises_exception(self):