From 83e9f2e29c041591ed403d1ae842277dbd7966b4 Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Thu, 13 Jun 2024 11:28:09 +0300 Subject: [PATCH 1/9] test: [AXM-636] Add some tests to cover Offline Mode --- .../tests/test_assets_management.py | 454 ++++++++++++++++++ .../tests/test_html_manipulator.py | 3 + .../features/offline_mode/tests/test_tasks.py | 76 +++ .../features/offline_mode/tests/test_utils.py | 248 ++++++++++ 4 files changed, 781 insertions(+) create mode 100644 openedx/features/offline_mode/tests/test_assets_management.py create mode 100644 openedx/features/offline_mode/tests/test_html_manipulator.py create mode 100644 openedx/features/offline_mode/tests/test_tasks.py create mode 100644 openedx/features/offline_mode/tests/test_utils.py diff --git a/openedx/features/offline_mode/tests/test_assets_management.py b/openedx/features/offline_mode/tests/test_assets_management.py new file mode 100644 index 000000000000..e70f1b277bbb --- /dev/null +++ b/openedx/features/offline_mode/tests/test_assets_management.py @@ -0,0 +1,454 @@ +""" +Tests for the testing utility functions for managing assets and files for Offline Mode. +""" + +from datetime import datetime +from unittest import TestCase +from unittest.mock import MagicMock, Mock, call, patch + +from django.conf import settings +from path import Path +from pytz import UTC + +from openedx.features.offline_mode.assets_management import ( + block_storage_path, + create_subdirectories_for_asset, + get_offline_block_content_path, + get_static_file_path, + is_modified, + remove_old_files, + save_asset_file, + save_mathjax_to_xblock_assets, +) +from openedx.features.offline_mode.constants import MATHJAX_CDN_URL, MATHJAX_STATIC_PATH +from xmodule.modulestore.exceptions import ItemNotFoundError + + +class AssetsManagementTestCase(TestCase): + """ + Test case for the testing utility functions for managing assets and files. + """ + + def test_get_static_file_path(self) -> None: + relative_path_mock = 'relative_path_mock' + expected_result = Path(f'{settings.STATIC_ROOT}/{relative_path_mock}') + + result = get_static_file_path(relative_path_mock) + + self.assertEqual(result, expected_result) + + @patch('openedx.features.offline_mode.assets_management.open') + @patch('openedx.features.offline_mode.assets_management.create_subdirectories_for_asset') + @patch('openedx.features.offline_mode.assets_management.os.path.join') + @patch('openedx.features.offline_mode.assets_management.AssetManager.find') + @patch('openedx.features.offline_mode.assets_management.StaticContent.get_asset_key_from_path') + def test_save_asset_file_if_filename_contains_slash( + self, + get_asset_key_from_path_mock: MagicMock, + asset_manager_find_mock: MagicMock, + os_path_join_mock: MagicMock, + create_subdirectories_for_asset_mock: MagicMock, + context_manager_mock: MagicMock, + ) -> None: + temp_dir_mock = 'temp_dir_mock' + xblock_mock = Mock() + path_mock = 'path_mock' + filename_mock = 'assets/filename_mock' + + save_asset_file(temp_dir_mock, xblock_mock, path_mock, filename_mock) + + get_asset_key_from_path_mock.assert_called_once_with( + xblock_mock.location.course_key, filename_mock.split('/')[-1] + ) + asset_manager_find_mock.assert_called_once_with(get_asset_key_from_path_mock.return_value) + os_path_join_mock.assert_called_once_with(temp_dir_mock, filename_mock) + create_subdirectories_for_asset_mock.assert_called_once_with(os_path_join_mock.return_value) + context_manager_mock.assert_called_once_with(os_path_join_mock.return_value, 'wb') + context_manager_mock.return_value.__enter__.return_value.write.assert_called_once_with( + asset_manager_find_mock.return_value.data + ) + + @patch('openedx.features.offline_mode.assets_management.open') + @patch('openedx.features.offline_mode.assets_management.create_subdirectories_for_asset') + @patch('openedx.features.offline_mode.assets_management.os.path.join') + @patch('openedx.features.offline_mode.assets_management.read_static_file') + @patch('openedx.features.offline_mode.assets_management.get_static_file_path') + def test_save_asset_file_no_slash_in_filename( + self, + get_static_file_path_mock: MagicMock, + read_static_file_mock: MagicMock, + os_path_join_mock: MagicMock, + create_subdirectories_for_asset_mock: MagicMock, + context_manager_mock: MagicMock, + ) -> None: + temp_dir_mock = 'temp_dir_mock' + xblock_mock = Mock() + path_mock = 'path_mock' + filename_mock = 'filename_mock' + + save_asset_file(temp_dir_mock, xblock_mock, path_mock, filename_mock) + + get_static_file_path_mock.assert_called_once_with(filename_mock) + read_static_file_mock.assert_called_once_with(get_static_file_path_mock.return_value) + os_path_join_mock.assert_called_once_with( + temp_dir_mock, 'assets', filename_mock, + ) + create_subdirectories_for_asset_mock.assert_called_once_with(os_path_join_mock.return_value) + context_manager_mock.assert_called_once_with(os_path_join_mock.return_value, 'wb') + context_manager_mock.return_value.__enter__.return_value.write.assert_called_once_with( + read_static_file_mock.return_value + ) + + @patch('openedx.features.offline_mode.assets_management.log.info') + @patch( + 'openedx.features.offline_mode.assets_management.get_static_file_path', side_effect=ItemNotFoundError + ) + def test_save_asset_file_can_not_find( + self, + get_static_file_path_mock: MagicMock, + log_info_mock: MagicMock, + ) -> None: + temp_dir_mock = 'temp_dir_mock' + xblock_mock = Mock() + path_mock = 'path_mock' + filename_mock = 'filename_mock' + + save_asset_file(temp_dir_mock, xblock_mock, path_mock, filename_mock) + + get_static_file_path_mock.assert_called_once_with(filename_mock) + log_info_mock.assert_called_once_with(f'Asset not found: {filename_mock}') + + @patch('openedx.features.offline_mode.assets_management.os') + def test_create_subdirectories_for_asset_subdirectories_does_not_exist(self, os_mock: MagicMock) -> None: + file_path_mock = 'file/path/mock/' + os_mock.path.exists.return_value = False + + expected_os_path_join_call_args_list = [ + call('/', 'file'), + call(os_mock.path.join.return_value, 'path'), + call(os_mock.path.join.return_value, 'mock'), + ] + expected_os_mock_mkdir_call_args_list = [ + call(os_mock.path.join.return_value), + call(os_mock.path.join.return_value), + call(os_mock.path.join.return_value), + ] + + create_subdirectories_for_asset(file_path_mock) + + self.assertListEqual(os_mock.path.join.call_args_list, expected_os_path_join_call_args_list) + self.assertListEqual(os_mock.mkdir.call_args_list, expected_os_mock_mkdir_call_args_list) + + @patch('openedx.features.offline_mode.assets_management.os') + def test_create_subdirectories_for_asset_subdirectories_exist(self, os_mock: MagicMock) -> None: + file_path_mock = 'file/path/mock/' + + expected_os_path_join_call_args_list = [ + call('/', 'file'), + call(os_mock.path.join.return_value, 'path'), + call(os_mock.path.join.return_value, 'mock'), + ] + + create_subdirectories_for_asset(file_path_mock) + + self.assertListEqual(os_mock.path.join.call_args_list, expected_os_path_join_call_args_list) + os_mock.mkdir.assert_not_called() + + @patch('openedx.features.offline_mode.assets_management.log') + @patch('openedx.features.offline_mode.assets_management.shutil.rmtree') + @patch('openedx.features.offline_mode.assets_management.default_storage.exists', side_effect=[False, False]) + @patch('openedx.features.offline_mode.assets_management.os') + @patch('openedx.features.offline_mode.assets_management.block_storage_path') + def test_remove_old_files_delete_only_assets_dir( + self, + block_storage_path_mock: MagicMock, + os_mock: MagicMock, + default_storage_exists_mock: MagicMock, + shutil_rmtree_mock: MagicMock, + logger_mock: MagicMock, + ) -> None: + xblock_mock = Mock() + + expected_os_mock_path_join_call_args = [ + call(block_storage_path_mock.return_value, 'assets'), + call(block_storage_path_mock.return_value, 'index.html'), + call(block_storage_path_mock.return_value, f'{xblock_mock.location.block_id}.zip'), + ] + expected_log_info = [ + call(f'Successfully deleted the directory: {os_mock.path.join.return_value}') + ] + expected_default_storage_exists_mock_call_args = [ + call(os_mock.path.join.return_value), call(os_mock.path.join.return_value) + ] + + remove_old_files(xblock_mock) + + block_storage_path_mock.assert_called_once_with(xblock_mock) + self.assertListEqual(os_mock.path.join.call_args_list, expected_os_mock_path_join_call_args) + os_mock.path.isdir.assert_called_once_with(os_mock.path.join.return_value) + shutil_rmtree_mock.assert_called_once_with(os_mock.path.join.return_value) + self.assertListEqual(logger_mock.info.call_args_list, expected_log_info) + self.assertListEqual( + default_storage_exists_mock.call_args_list, expected_default_storage_exists_mock_call_args + ) + os_mock.remove.assert_not_called() + + @patch('openedx.features.offline_mode.assets_management.log') + @patch('openedx.features.offline_mode.assets_management.shutil.rmtree') + @patch('openedx.features.offline_mode.assets_management.default_storage.exists', side_effect=[True, False]) + @patch('openedx.features.offline_mode.assets_management.os') + @patch('openedx.features.offline_mode.assets_management.block_storage_path') + def test_remove_old_files_delete_assets_dir_and_index( + self, + block_storage_path_mock: MagicMock, + os_mock: MagicMock, + default_storage_exists_mock: MagicMock, + shutil_rmtree_mock: MagicMock, + logger_mock: MagicMock, + ) -> None: + xblock_mock = Mock() + + expected_os_mock_path_join_call_args = [ + call(block_storage_path_mock.return_value, 'assets'), + call(block_storage_path_mock.return_value, 'index.html'), + call(block_storage_path_mock.return_value, f'{xblock_mock.location.block_id}.zip'), + ] + expected_log_info = [ + call(f'Successfully deleted the directory: {os_mock.path.join.return_value}'), + call(f'Successfully deleted the file: {os_mock.path.join.return_value}'), + ] + expected_default_storage_exists_mock_call_args = [ + call(os_mock.path.join.return_value), call(os_mock.path.join.return_value) + ] + expected_os_mock_remove_call_args = [call(os_mock.path.join.return_value)] + + remove_old_files(xblock_mock) + + block_storage_path_mock.assert_called_once_with(xblock_mock) + self.assertListEqual(os_mock.path.join.call_args_list, expected_os_mock_path_join_call_args) + os_mock.path.isdir.assert_called_once_with(os_mock.path.join.return_value) + shutil_rmtree_mock.assert_called_once_with(os_mock.path.join.return_value) + self.assertListEqual(logger_mock.info.call_args_list, expected_log_info) + self.assertListEqual( + default_storage_exists_mock.call_args_list, expected_default_storage_exists_mock_call_args + ) + self.assertListEqual(os_mock.remove.call_args_list, expected_os_mock_remove_call_args) + + @patch('openedx.features.offline_mode.assets_management.log') + @patch('openedx.features.offline_mode.assets_management.shutil.rmtree') + @patch('openedx.features.offline_mode.assets_management.default_storage.exists', side_effect=[True, True]) + @patch('openedx.features.offline_mode.assets_management.os') + @patch('openedx.features.offline_mode.assets_management.block_storage_path') + def test_remove_old_files_delete_assets_dir_and_index_and_offline_content_zip_file( + self, + block_storage_path_mock: MagicMock, + os_mock: MagicMock, + default_storage_exists_mock: MagicMock, + shutil_rmtree_mock: MagicMock, + logger_mock: MagicMock, + ) -> None: + xblock_mock = Mock() + + expected_os_mock_path_join_call_args = [ + call(block_storage_path_mock.return_value, 'assets'), + call(block_storage_path_mock.return_value, 'index.html'), + call(block_storage_path_mock.return_value, f'{xblock_mock.location.block_id}.zip') + ] + expected_log_info = [ + call(f'Successfully deleted the directory: {os_mock.path.join.return_value}'), + call(f'Successfully deleted the file: {os_mock.path.join.return_value}'), + call(f'Successfully deleted the file: {os_mock.path.join.return_value}'), + ] + expected_default_storage_exists_mock_call_args = [ + call(os_mock.path.join.return_value), call(os_mock.path.join.return_value) + ] + expected_os_mock_remove_call_args = [ + call(os_mock.path.join.return_value), call(os_mock.path.join.return_value) + ] + + remove_old_files(xblock_mock) + + block_storage_path_mock.assert_called_once_with(xblock_mock) + self.assertListEqual(os_mock.path.join.call_args_list, expected_os_mock_path_join_call_args) + os_mock.path.isdir.assert_called_once_with(os_mock.path.join.return_value) + shutil_rmtree_mock.assert_called_once_with(os_mock.path.join.return_value) + self.assertListEqual(logger_mock.info.call_args_list, expected_log_info) + self.assertListEqual( + default_storage_exists_mock.call_args_list, expected_default_storage_exists_mock_call_args + ) + self.assertListEqual(os_mock.remove.call_args_list, expected_os_mock_remove_call_args) + + @patch('openedx.features.offline_mode.assets_management.log.error') + @patch('openedx.features.offline_mode.assets_management.block_storage_path', side_effect=OSError) + def test_remove_old_files_os_error( + self, + block_storage_path_mock: MagicMock, + log_error_mock: MagicMock, + ) -> None: + xblock_mock = Mock() + + remove_old_files(xblock_mock) + + block_storage_path_mock.assert_called_once_with(xblock_mock) + log_error_mock.assert_called_once_with('Error occurred while deleting the files or directory: ') + + @patch('openedx.features.offline_mode.assets_management.default_storage.exists') + @patch('openedx.features.offline_mode.assets_management.os.path.join', return_value='offline_zip_path_mock') + @patch('openedx.features.offline_mode.assets_management.block_storage_path') + def test_get_offline_block_content_path_offline_content_exists( + self, + block_storage_path_mock: MagicMock, + os_path_join_mock: MagicMock, + default_storage_exists_mock: MagicMock, + ) -> None: + xblock_mock = Mock() + + result = get_offline_block_content_path(xblock_mock) + + block_storage_path_mock.assert_called_once_with(usage_key=xblock_mock.location) + os_path_join_mock.assert_called_once_with( + block_storage_path_mock.return_value, f'{xblock_mock.location.block_id}.zip' + ) + default_storage_exists_mock.assert_called_once_with(os_path_join_mock.return_value) + self.assertEqual(result, 'offline_zip_path_mock') + + @patch('openedx.features.offline_mode.assets_management.default_storage.exists', return_value=False) + @patch('openedx.features.offline_mode.assets_management.os.path.join', return_value='offline_zip_path_mock') + @patch('openedx.features.offline_mode.assets_management.block_storage_path') + def test_get_offline_block_content_path_does_not_exist( + self, + block_storage_path_mock: MagicMock, + os_path_join_mock: MagicMock, + default_storage_exists_mock: MagicMock, + ) -> None: + xblock_mock = Mock() + + result = get_offline_block_content_path(xblock_mock) + + block_storage_path_mock.assert_called_once_with(usage_key=xblock_mock.location) + os_path_join_mock.assert_called_once_with( + block_storage_path_mock.return_value, f'{xblock_mock.location.block_id}.zip' + ) + default_storage_exists_mock.assert_called_once_with(os_path_join_mock.return_value) + self.assertEqual(result, None) + + def test_block_storage_path_exists(self) -> None: + xblock_mock = Mock(location=Mock(course_key='course_key_mock')) + + result = block_storage_path(xblock_mock) + + self.assertEqual(result, 'course_key_mock/') + + def test_block_storage_path_does_not_exists(self) -> None: + result = block_storage_path() + + self.assertEqual(result, '') + + @patch( + 'openedx.features.offline_mode.assets_management.default_storage.get_created_time', + return_value=datetime(2024, 6, 12, tzinfo=UTC) + ) + @patch('openedx.features.offline_mode.assets_management.block_storage_path') + @patch('openedx.features.offline_mode.assets_management.os.path.join') + def test_is_modified_true( + self, + os_path_join_mock: MagicMock, + block_storage_path_mock: MagicMock, + get_created_time_mock: MagicMock, + ) -> None: + xblock_mock = Mock(published_on=datetime(2024, 6, 13, tzinfo=UTC)) + + result = is_modified(xblock_mock) + + os_path_join_mock.assert_called_once_with( + block_storage_path_mock.return_value, f'{xblock_mock.location.block_id}.zip') + get_created_time_mock.assert_called_once_with(os_path_join_mock.return_value) + self.assertEqual(result, True) + + @patch( + 'openedx.features.offline_mode.assets_management.default_storage.get_created_time', + return_value=datetime(2024, 6, 12, tzinfo=UTC) + ) + @patch('openedx.features.offline_mode.assets_management.block_storage_path') + @patch('openedx.features.offline_mode.assets_management.os.path.join') + def test_is_modified_false( + self, + os_path_join_mock: MagicMock, + block_storage_path_mock: MagicMock, + get_created_time_mock: MagicMock, + ) -> None: + xblock_mock = Mock(published_on=datetime(2024, 6, 1, tzinfo=UTC)) + + result = is_modified(xblock_mock) + + os_path_join_mock.assert_called_once_with( + block_storage_path_mock.return_value, f'{xblock_mock.location.block_id}.zip') + get_created_time_mock.assert_called_once_with(os_path_join_mock.return_value) + self.assertEqual(result, False) + + @patch( + 'openedx.features.offline_mode.assets_management.default_storage.get_created_time', + side_effect=OSError + ) + @patch('openedx.features.offline_mode.assets_management.block_storage_path') + @patch('openedx.features.offline_mode.assets_management.os.path.join') + def test_is_modified_os_error( + self, + os_path_join_mock: MagicMock, + block_storage_path_mock: MagicMock, + get_created_time_mock: MagicMock, + ) -> None: + xblock_mock = Mock() + + result = is_modified(xblock_mock) + + os_path_join_mock.assert_called_once_with( + block_storage_path_mock.return_value, f'{xblock_mock.location.block_id}.zip') + get_created_time_mock.assert_called_once_with(os_path_join_mock.return_value) + self.assertEqual(result, True) + + @patch('openedx.features.offline_mode.assets_management.log.info') + @patch('openedx.features.offline_mode.assets_management.open') + @patch('openedx.features.offline_mode.assets_management.requests.get') + @patch('openedx.features.offline_mode.assets_management.os') + def test_save_mathjax_to_xblock_assets_successfully( + self, + os_mock: MagicMock, + requests_get_mock: MagicMock, + context_manager_mock: MagicMock, + logger_mock: MagicMock, + ) -> None: + temp_dir_mock = 'temp_dir_mock' + os_mock.path.exists.return_value = False + + save_mathjax_to_xblock_assets(temp_dir_mock) + + os_mock.path.join.assert_called_once_with(temp_dir_mock, MATHJAX_STATIC_PATH) + os_mock.path.exists.assert_called_once_with(os_mock.path.join.return_value) + requests_get_mock.assert_called_once_with(MATHJAX_CDN_URL) + context_manager_mock.assert_called_once_with(os_mock.path.join.return_value, 'wb') + context_manager_mock.return_value.__enter__.return_value.write.assert_called_once_with( + requests_get_mock.return_value.content + ) + logger_mock.assert_called_once_with(f'Successfully saved MathJax to {os_mock.path.join.return_value}') + + @patch('openedx.features.offline_mode.assets_management.log.info') + @patch('openedx.features.offline_mode.assets_management.open') + @patch('openedx.features.offline_mode.assets_management.requests.get') + @patch('openedx.features.offline_mode.assets_management.os') + def test_save_mathjax_to_xblock_assets_already_exists( + self, + os_mock: MagicMock, + requests_get_mock: MagicMock, + context_manager_mock: MagicMock, + logger_mock: MagicMock, + ) -> None: + temp_dir_mock = 'temp_dir_mock' + + save_mathjax_to_xblock_assets(temp_dir_mock) + + os_mock.path.join.assert_called_once_with(temp_dir_mock, MATHJAX_STATIC_PATH) + os_mock.path.exists.assert_called_once_with(os_mock.path.join.return_value) + requests_get_mock.assert_not_called() + context_manager_mock.assert_not_called() + logger_mock.assert_not_called() diff --git a/openedx/features/offline_mode/tests/test_html_manipulator.py b/openedx/features/offline_mode/tests/test_html_manipulator.py new file mode 100644 index 000000000000..cacb7546549f --- /dev/null +++ b/openedx/features/offline_mode/tests/test_html_manipulator.py @@ -0,0 +1,3 @@ +""" +Tests for the testing methods for prepare HTML content for offline using. +""" diff --git a/openedx/features/offline_mode/tests/test_tasks.py b/openedx/features/offline_mode/tests/test_tasks.py new file mode 100644 index 000000000000..cd079a20fe80 --- /dev/null +++ b/openedx/features/offline_mode/tests/test_tasks.py @@ -0,0 +1,76 @@ +""" +Tests for the testing Offline Mode tacks. +""" + +from unittest import TestCase +from unittest.mock import MagicMock, Mock, call, patch + +from opaque_keys.edx.keys import CourseKey, UsageKey +from openedx.features.offline_mode.constants import OFFLINE_SUPPORTED_XBLOCKS +from openedx.features.offline_mode.tasks import ( + generate_offline_content_for_block, + generate_offline_content_for_course, +) + + +class GenerateOfflineContentTasksTestCase(TestCase): + """ + Test case for the testing generating offline content tacks. + """ + + @patch('openedx.features.offline_mode.tasks.generate_offline_content') + @patch('openedx.features.offline_mode.tasks.modulestore') + def test_generate_offline_content_for_block( + self, + modulestore_mock: MagicMock, + generate_offline_content_mock: MagicMock, + ) -> None: + block_id_mock = 'block-v1:a+a+a+type@problem+block@fb81e4dbfd4945cb9318d6bc460a956c' + html_data_mock = 'html_markup_data_mock' + + generate_offline_content_for_block(block_id_mock, html_data_mock) + + modulestore_mock.assert_called_once_with() + modulestore_mock.return_value.get_item.assert_called_once_with(UsageKey.from_string(block_id_mock)) + generate_offline_content_mock.assert_called_once_with( + modulestore_mock.return_value.get_item.return_value, html_data_mock + ) + + @patch('openedx.features.offline_mode.tasks.generate_offline_content_for_block') + @patch('openedx.features.offline_mode.tasks.XBlockRenderer') + @patch('openedx.features.offline_mode.tasks.modulestore') + def test_generate_offline_content_for_course_supported_block_types( + self, + modulestore_mock: MagicMock, + xblock_renderer_mock: MagicMock, + generate_offline_content_for_block_mock: MagicMock, + ) -> None: + course_id_mock = 'course-v1:a+a+a' + xblock_location_mock = 'xblock_location_mock' + html_data_mock = 'html_markup_data_mock' + modulestore_mock.return_value.get_items.return_value = [Mock(location=xblock_location_mock)] + xblock_renderer_mock.return_value.render_xblock_from_lms.return_value = html_data_mock + expected_call_args_for_modulestore_get_items = [ + call(CourseKey.from_string(course_id_mock), qualifiers={'category': offline_supported_block_type}) + for offline_supported_block_type in OFFLINE_SUPPORTED_XBLOCKS + ] + expected_call_args_for_xblock_renderer_mock = [ + call(xblock_location_mock) for _ in OFFLINE_SUPPORTED_XBLOCKS + ] + expected_call_args_for_generate_offline_content_for_block_mock = [ + call([xblock_location_mock, html_data_mock]) for _ in OFFLINE_SUPPORTED_XBLOCKS + ] + + generate_offline_content_for_course(course_id_mock) + + self.assertEqual(modulestore_mock.call_count, len(OFFLINE_SUPPORTED_XBLOCKS)) + self.assertListEqual( + modulestore_mock.return_value.get_items.call_args_list, expected_call_args_for_modulestore_get_items + ) + self.assertListEqual( + xblock_renderer_mock.call_args_list, expected_call_args_for_xblock_renderer_mock + ) + self.assertListEqual( + generate_offline_content_for_block_mock.apply_async.call_args_list, + expected_call_args_for_generate_offline_content_for_block_mock + ) diff --git a/openedx/features/offline_mode/tests/test_utils.py b/openedx/features/offline_mode/tests/test_utils.py new file mode 100644 index 000000000000..f520df2c1047 --- /dev/null +++ b/openedx/features/offline_mode/tests/test_utils.py @@ -0,0 +1,248 @@ +""" +Tests for the testing Offline Mode utils. +""" + +from unittest import TestCase +from unittest.mock import MagicMock, Mock, call, patch + +from openedx.features.offline_mode.utils import ( + add_files_to_zip_recursively, + create_zip_file, + generate_offline_content, + save_xblock_html, +) + + +class OfflineModeUtilsTestCase(TestCase): + """ + Test case for the testing Offline Mode utils. + """ + + @patch('openedx.features.offline_mode.utils.shutil.rmtree') + @patch('openedx.features.offline_mode.utils.create_zip_file') + @patch('openedx.features.offline_mode.utils.save_xblock_html') + @patch('openedx.features.offline_mode.utils.mkdtemp') + @patch('openedx.features.offline_mode.utils.remove_old_files') + @patch('openedx.features.offline_mode.utils.block_storage_path') + @patch('openedx.features.offline_mode.utils.is_modified') + def test_generate_offline_content_for_modified_xblock( + self, + is_modified_mock: MagicMock, + block_storage_path_mock: MagicMock, + remove_old_files_mock: MagicMock, + mkdtemp_mock: MagicMock, + save_xblock_html_mock: MagicMock, + create_zip_file_mock: MagicMock, + shutil_rmtree_mock: MagicMock, + ) -> None: + xblock_mock = Mock() + html_data_mock = 'html_markup_data_mock' + + generate_offline_content(xblock_mock, html_data_mock) + + is_modified_mock.assert_called_once_with(xblock_mock) + block_storage_path_mock.assert_called_once_with(xblock_mock) + remove_old_files_mock.assert_called_once_with(xblock_mock) + mkdtemp_mock.assert_called_once_with() + save_xblock_html_mock.assert_called_once_with(mkdtemp_mock.return_value, xblock_mock, html_data_mock) + create_zip_file_mock.assert_called_once_with( + mkdtemp_mock.return_value, + block_storage_path_mock.return_value, + f'{xblock_mock.location.block_id}.zip' + ) + shutil_rmtree_mock.assert_called_once_with(mkdtemp_mock.return_value, ignore_errors=True) + + @patch('openedx.features.offline_mode.utils.shutil.rmtree') + @patch('openedx.features.offline_mode.utils.create_zip_file') + @patch('openedx.features.offline_mode.utils.save_xblock_html') + @patch('openedx.features.offline_mode.utils.mkdtemp') + @patch('openedx.features.offline_mode.utils.remove_old_files') + @patch('openedx.features.offline_mode.utils.block_storage_path') + @patch('openedx.features.offline_mode.utils.is_modified', return_value=False) + def test_generate_offline_content_is_not_modified( + self, + is_modified_mock: MagicMock, + block_storage_path_mock: MagicMock, + remove_old_files_mock: MagicMock, + mkdtemp_mock: MagicMock, + save_xblock_html_mock: MagicMock, + create_zip_file_mock: MagicMock, + shutil_rmtree_mock: MagicMock, + ) -> None: + xblock_mock = Mock() + html_data_mock = 'html_markup_data_mock' + + generate_offline_content(xblock_mock, html_data_mock) + + is_modified_mock.assert_called_once_with(xblock_mock) + block_storage_path_mock.assert_not_called() + remove_old_files_mock.assert_not_called() + mkdtemp_mock.assert_not_called() + save_xblock_html_mock.assert_not_called() + create_zip_file_mock.assert_not_called() + shutil_rmtree_mock.assert_not_called() + + @patch('openedx.features.offline_mode.utils.os.path.join') + @patch('openedx.features.offline_mode.utils.open') + @patch('openedx.features.offline_mode.utils.HtmlManipulator') + def test_save_xblock_html( + self, + html_manipulator_mock: MagicMock, + context_manager_mock: MagicMock, + os_path_join_mock: MagicMock, + ) -> None: + tmp_dir_mock = Mock() + xblock_mock = Mock() + html_data_mock = 'html_markup_data_mock' + + save_xblock_html(tmp_dir_mock, xblock_mock, html_data_mock) + + html_manipulator_mock.assert_called_once_with(xblock_mock, html_data_mock, tmp_dir_mock) + html_manipulator_mock.return_value.process_html.assert_called_once_with() + context_manager_mock.assert_called_once_with(os_path_join_mock.return_value, 'w') + os_path_join_mock.assert_called_once_with(tmp_dir_mock, 'index.html') + context_manager_mock.return_value.__enter__.return_value.write.assert_called_once_with( + html_manipulator_mock.return_value.process_html.return_value + ) + + @patch('openedx.features.offline_mode.utils.log.info') + @patch('openedx.features.offline_mode.utils.add_files_to_zip_recursively') + @patch('openedx.features.offline_mode.utils.ZipFile') + @patch('openedx.features.offline_mode.utils.default_storage') + @patch('openedx.features.offline_mode.utils.os') + def test_create_zip_file_os_path_exists( + self, + os_mock: MagicMock, + default_storage_mock: MagicMock, + zip_file_context_manager: MagicMock, + add_files_to_zip_recursively_mock: MagicMock, + log_info_mock: MagicMock, + ) -> None: + temp_dir_mock = 'temp_dir_mock' + base_path_mock = 'base_path_mock' + file_name_mock = 'file_name_mock' + + create_zip_file(temp_dir_mock, base_path_mock, file_name_mock) + + os_mock.path.exists.assert_called_once_with(default_storage_mock.path.return_value) + os_mock.makedirs.assert_not_called() + self.assertListEqual( + default_storage_mock.path.call_args_list, + [call(base_path_mock), call(base_path_mock + file_name_mock)] + ) + zip_file_context_manager.assert_called_once_with(default_storage_mock.path.return_value, 'w') + zip_file_context_manager.return_value.__enter__.return_value.write.assert_called_once_with( + os_mock.path.join.return_value, 'index.html' + ) + add_files_to_zip_recursively_mock.assert_called_once_with( + zip_file_context_manager.return_value.__enter__.return_value, + current_base_path=os_mock.path.join.return_value, + current_path_in_zip='assets', + ) + log_info_mock.assert_called_once_with( + f'Offline content for {file_name_mock} has been generated.' + ) + + @patch('openedx.features.offline_mode.utils.log.info') + @patch('openedx.features.offline_mode.utils.add_files_to_zip_recursively') + @patch('openedx.features.offline_mode.utils.ZipFile') + @patch('openedx.features.offline_mode.utils.default_storage') + @patch('openedx.features.offline_mode.utils.os') + def test_create_zip_file_os_path_does_not_exists( + self, + os_mock: MagicMock, + default_storage_mock: MagicMock, + zip_file_context_manager: MagicMock, + add_files_to_zip_recursively_mock: MagicMock, + log_info_mock: MagicMock, + ) -> None: + temp_dir_mock = 'temp_dir_mock' + base_path_mock = 'base_path_mock' + file_name_mock = 'file_name_mock' + os_mock.path.exists.return_value = False + + create_zip_file(temp_dir_mock, base_path_mock, file_name_mock) + + os_mock.path.exists.assert_called_once_with(default_storage_mock.path.return_value) + os_mock.makedirs.assert_called_once_with(default_storage_mock.path.return_value) + self.assertListEqual( + default_storage_mock.path.call_args_list, + [call(base_path_mock), call(base_path_mock), call(base_path_mock + file_name_mock)] + ) + zip_file_context_manager.assert_called_once_with(default_storage_mock.path.return_value, 'w') + zip_file_context_manager.return_value.__enter__.return_value.write.assert_called_once_with( + os_mock.path.join.return_value, 'index.html' + ) + add_files_to_zip_recursively_mock.assert_called_once_with( + zip_file_context_manager.return_value.__enter__.return_value, + current_base_path=os_mock.path.join.return_value, + current_path_in_zip='assets', + ) + log_info_mock.assert_called_once_with( + f'Offline content for {file_name_mock} has been generated.' + ) + + @patch('openedx.features.offline_mode.utils.os') + def test_add_files_to_zip_recursively_successfully_for_file( + self, + os_mock: MagicMock, + ): + zip_file_mock = Mock() + current_base_path_mock = 'current_base_path_mock' + current_path_in_zip_mock = 'current_path_in_zip_mock' + resource_path_mock = 'resource_path_mock' + os_mock.listdir.return_value = [resource_path_mock] + + expected_os_mock_path_join_calls = [ + call(current_base_path_mock, resource_path_mock), + call(current_path_in_zip_mock, resource_path_mock) + ] + + add_files_to_zip_recursively(zip_file_mock, current_base_path_mock, current_path_in_zip_mock) + + os_mock.listdir.assert_called_once_with(current_base_path_mock) + self.assertListEqual(os_mock.path.join.call_args_list, expected_os_mock_path_join_calls) + zip_file_mock.write.assert_called_once_with(os_mock.path.join.return_value, os_mock.path.join.return_value) + + @patch('openedx.features.offline_mode.utils.add_files_to_zip_recursively') + @patch('openedx.features.offline_mode.utils.os') + def test_add_files_to_zip_recursively_successfully_recursively_path( + self, + os_mock: MagicMock, + add_files_to_zip_recursively_mock: MagicMock, + ): + zip_file_mock = Mock() + current_base_path_mock = 'current_base_path_mock' + current_path_in_zip_mock = 'current_path_in_zip_mock' + resource_path_mock = 'resource_path_mock' + os_mock.listdir.return_value = [resource_path_mock] + os_mock.path.isfile.return_value = False + + expected_os_mock_path_join_calls = [ + call(current_base_path_mock, resource_path_mock), + call(current_path_in_zip_mock, resource_path_mock) + ] + + add_files_to_zip_recursively(zip_file_mock, current_base_path_mock, current_path_in_zip_mock) + + os_mock.listdir.assert_called_once_with(current_base_path_mock) + self.assertListEqual(os_mock.path.join.call_args_list, expected_os_mock_path_join_calls) + add_files_to_zip_recursively_mock.assert_called_once_with( + zip_file_mock, os_mock.path.join.return_value, os_mock.path.join.return_value + ) + + @patch('openedx.features.offline_mode.utils.log.error') + @patch('openedx.features.offline_mode.utils.os.listdir', side_effect=OSError) + def test_add_files_to_zip_recursively_with_os_error( + self, + os_mock: MagicMock, + log_error_mock: MagicMock, + ): + zip_file_mock = Mock() + current_base_path_mock = 'current_base_path_mock' + current_path_in_zip_mock = 'current_path_in_zip_mock' + + add_files_to_zip_recursively(zip_file_mock, current_base_path_mock, current_path_in_zip_mock) + + os_mock.assert_called_once_with(current_base_path_mock) + log_error_mock.assert_called_once_with(f'Error while reading the directory: {current_base_path_mock}') From e7427d89ae4527d0cc83270ca0c8cb0449de944d Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Thu, 13 Jun 2024 15:55:59 +0300 Subject: [PATCH 2/9] test: [AXM-636] Covered the all Ofline Mode app with tests --- .../tests/test_html_manipulator.py | 115 ++++++++++++++++++ .../offline_mode/tests/test_renderer.py | 62 ++++++++++ 2 files changed, 177 insertions(+) create mode 100644 openedx/features/offline_mode/tests/test_renderer.py diff --git a/openedx/features/offline_mode/tests/test_html_manipulator.py b/openedx/features/offline_mode/tests/test_html_manipulator.py index cacb7546549f..e7eb65e787a0 100644 --- a/openedx/features/offline_mode/tests/test_html_manipulator.py +++ b/openedx/features/offline_mode/tests/test_html_manipulator.py @@ -1,3 +1,118 @@ """ Tests for the testing methods for prepare HTML content for offline using. """ + +from bs4 import BeautifulSoup +from unittest import TestCase +from unittest.mock import MagicMock, Mock, patch + +from openedx.features.offline_mode.constants import MATHJAX_CDN_URL, MATHJAX_STATIC_PATH +from openedx.features.offline_mode.html_manipulator import HtmlManipulator + + +class HtmlManipulatorTestCase(TestCase): + """ + Test case for the testing `HtmlManipulator` methods. + """ + + @patch('openedx.features.offline_mode.html_manipulator.HtmlManipulator._replace_iframe') + @patch('openedx.features.offline_mode.html_manipulator.BeautifulSoup', return_value='soup_mock') + @patch('openedx.features.offline_mode.html_manipulator.HtmlManipulator._replace_mathjax_link') + @patch('openedx.features.offline_mode.html_manipulator.HtmlManipulator._replace_static_links') + @patch('openedx.features.offline_mode.html_manipulator.HtmlManipulator._replace_asset_links') + def test_process_html( + self, + replace_asset_links_mock: MagicMock, + replace_static_links_mock: MagicMock, + replace_mathjax_link_mock: MagicMock, + beautiful_soup_mock: MagicMock, + replace_iframe_mock: MagicMock, + ) -> None: + html_data_mock = 'html_data_mock' + xblock_mock = Mock() + temp_dir_mock = 'temp_dir_mock' + html_manipulator = HtmlManipulator(xblock_mock, html_data_mock, temp_dir_mock) + expected_result = 'soup_mock' + + result = html_manipulator.process_html() + + replace_asset_links_mock.assert_called_once_with() + replace_static_links_mock.assert_called_once_with() + replace_mathjax_link_mock.assert_called_once_with() + beautiful_soup_mock.assert_called_once_with(html_manipulator.html_data, 'html.parser') + replace_iframe_mock.assert_called_once_with(beautiful_soup_mock.return_value) + self.assertEqual(result, expected_result) + + @patch('openedx.features.offline_mode.html_manipulator.save_mathjax_to_xblock_assets') + def test_replace_mathjax_link(self, save_mathjax_to_xblock_assets: MagicMock) -> None: + html_data_mock = f'' + xblock_mock = Mock() + temp_dir_mock = 'temp_dir_mock' + html_manipulator = HtmlManipulator(xblock_mock, html_data_mock, temp_dir_mock) + + expected_html_data_after_replacing = f'' + + self.assertEqual(html_manipulator.html_data, html_data_mock) + + html_manipulator._replace_mathjax_link() + + save_mathjax_to_xblock_assets.assert_called_once_with(html_manipulator.temp_dir) + self.assertEqual(html_manipulator.html_data, expected_html_data_after_replacing) + + @patch('openedx.features.offline_mode.html_manipulator.save_asset_file') + def test_replace_static_links(self, save_asset_file_mock: MagicMock) -> None: + html_data_mock = '
' + xblock_mock = Mock() + temp_dir_mock = 'temp_dir_mock' + html_manipulator = HtmlManipulator(xblock_mock, html_data_mock, temp_dir_mock) + + expected_html_data_after_replacing = ( + '
' + ) + + self.assertEqual(html_manipulator.html_data, html_data_mock) + + html_manipulator._replace_static_links() + + save_asset_file_mock.assert_called_once_with( + html_manipulator.temp_dir, + html_manipulator.xblock, + '/static/images/professor-sandel.jpg', + 'images/professor-sandel.jpg', + ) + self.assertEqual(html_manipulator.html_data, expected_html_data_after_replacing) + + @patch('openedx.features.offline_mode.html_manipulator.save_asset_file') + def test_replace_asset_links(self, save_asset_file_mock: MagicMock) -> None: + html_data_mock = '
' + xblock_mock = Mock() + temp_dir_mock = 'temp_dir_mock' + html_manipulator = HtmlManipulator(xblock_mock, html_data_mock, temp_dir_mock) + + expected_html_data_after_replacing = ( + '
' + ) + + self.assertEqual(html_manipulator.html_data, html_data_mock) + + html_manipulator._replace_asset_links() + + save_asset_file_mock.assert_called_once_with( + html_manipulator.temp_dir, + html_manipulator.xblock, + '/assets/images/professor-sandel.jpg', + 'assets/images/professor-sandel.jpg', + ) + self.assertEqual(html_manipulator.html_data, expected_html_data_after_replacing) + + def test_replace_iframe(self): + html_data_mock = """ + + """ + soup = BeautifulSoup(html_data_mock, 'html.parser') + expected_html_markup = """

${_('YouTube Video')}

""" + + HtmlManipulator._replace_iframe(soup) + + self.assertEqual(f'{soup.find_all("p")[0]}', expected_html_markup) diff --git a/openedx/features/offline_mode/tests/test_renderer.py b/openedx/features/offline_mode/tests/test_renderer.py new file mode 100644 index 000000000000..fa0a16ce62b0 --- /dev/null +++ b/openedx/features/offline_mode/tests/test_renderer.py @@ -0,0 +1,62 @@ +""" +Tests for the testing xBlock renderers for Offline Mode. +""" + +from openedx.features.offline_mode.renderer import XBlockRenderer +from xmodule.capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory + + +class XBlockRendererTestCase(ModuleStoreTestCase): + """ + Test case for the testing `XBlockRenderer`. + """ + + def setup_course(self): + """ + Helper method to create the course. + """ + default_store = self.store.default_modulestore.get_modulestore_type() + with self.store.default_store(default_store): + self.course = CourseFactory.create() + chapter = BlockFactory.create(parent=self.course, category='chapter') + problem_xml = MultipleChoiceResponseXMLFactory().build_xml( + question_text='The correct answer is Choice 2', + choices=[False, False, True, False], + choice_names=['choice_0', 'choice_1', 'choice_2', 'choice_3'] + ) + self.vertical_block = BlockFactory.create( + parent_location=chapter.location, + category='vertical', + display_name="Vertical" + ) + self.html_block = BlockFactory.create( + parent=self.vertical_block, + category='html', + data="

Test HTML Content

" + ) + self.problem_block = BlockFactory.create( + parent=self.vertical_block, + category='problem', + display_name='Problem', + data=problem_xml + ) + + def test_render_xblock_from_lms_html_block(self): + self.setup_course() + xblock_renderer = XBlockRenderer(str(self.html_block.location), user=self.user) + + result = xblock_renderer.render_xblock_from_lms() + + self.assertIsNotNone(result) + self.assertEqual(type(result), str) + + def test_render_xblock_from_lms_problem_block(self): + self.setup_course() + xblock_renderer = XBlockRenderer(str(self.problem_block.location), user=self.user) + + result = xblock_renderer.render_xblock_from_lms() + + self.assertIsNotNone(result) + self.assertEqual(type(result), str) From b16d6750aa30c5d2b3efca1ece175bf19e78f1e2 Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Thu, 13 Jun 2024 17:19:38 +0300 Subject: [PATCH 3/9] test: [AXM-636] Add test to Blocks Info API --- .../tests/test_course_info_views.py | 32 ++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/mobile_api/tests/test_course_info_views.py b/lms/djangoapps/mobile_api/tests/test_course_info_views.py index ca4750d121bf..5f9a48b96616 100644 --- a/lms/djangoapps/mobile_api/tests/test_course_info_views.py +++ b/lms/djangoapps/mobile_api/tests/test_course_info_views.py @@ -1,7 +1,7 @@ """ Tests for course_info """ -from unittest.mock import patch +from unittest.mock import MagicMock, patch import ddt @@ -450,3 +450,33 @@ def test_extend_sequential_info_with_assignment_progress_for_other_types(self, b self.assertEqual(response.status_code, status.HTTP_200_OK) for block_info in response.data['blocks'].values(): self.assertNotEqual('assignment_progress', block_info) + + @patch('lms.djangoapps.mobile_api.course_info.views.default_storage') + @patch('lms.djangoapps.mobile_api.course_info.views.get_offline_block_content_path') + @patch('lms.djangoapps.mobile_api.course_info.views.is_offline_mode_enabled') + def test_extend_block_info_with_offline_data( + self, + is_offline_mode_enabled_mock: MagicMock, + get_offline_block_content_path_mock: MagicMock, + default_storage_mock: MagicMock, + ) -> None: + url = reverse('blocks_info_in_course', kwargs={'api_version': 'v4'}) + offline_content_path_mock = '/offline_content_path_mock/' + created_time_mock = 'created_time_mock' + size_mock = 'size_mock' + get_offline_block_content_path_mock.return_value = offline_content_path_mock + default_storage_mock.get_created_time.return_value = created_time_mock + default_storage_mock.size.return_value = size_mock + + expected_offline_download_data = { + 'file_url': offline_content_path_mock, + 'last_modified': created_time_mock, + 'file_size': size_mock + } + + response = self.verify_response(url=url) + + is_offline_mode_enabled_mock.assert_called_once_with(self.course.course_id) + self.assertEqual(response.status_code, status.HTTP_200_OK) + for block_info in response.data['blocks'].values(): + self.assertDictEqual(block_info['offline_download'], expected_offline_download_data) From 1dceee1d0a502d4232677e4933d65c0169352d20 Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Thu, 13 Jun 2024 17:29:30 +0300 Subject: [PATCH 4/9] style: [AXM-636] Improve code style --- .../features/offline_mode/tests/test_html_manipulator.py | 8 ++++---- openedx/features/offline_mode/tests/test_renderer.py | 8 ++++---- openedx/features/offline_mode/tests/test_tasks.py | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/openedx/features/offline_mode/tests/test_html_manipulator.py b/openedx/features/offline_mode/tests/test_html_manipulator.py index e7eb65e787a0..d085a0ebf8c3 100644 --- a/openedx/features/offline_mode/tests/test_html_manipulator.py +++ b/openedx/features/offline_mode/tests/test_html_manipulator.py @@ -54,7 +54,7 @@ def test_replace_mathjax_link(self, save_mathjax_to_xblock_assets: MagicMock) -> self.assertEqual(html_manipulator.html_data, html_data_mock) - html_manipulator._replace_mathjax_link() + html_manipulator._replace_mathjax_link() # lint-amnesty, pylint: disable=protected-access save_mathjax_to_xblock_assets.assert_called_once_with(html_manipulator.temp_dir) self.assertEqual(html_manipulator.html_data, expected_html_data_after_replacing) @@ -72,7 +72,7 @@ def test_replace_static_links(self, save_asset_file_mock: MagicMock) -> None: self.assertEqual(html_manipulator.html_data, html_data_mock) - html_manipulator._replace_static_links() + html_manipulator._replace_static_links() # lint-amnesty, pylint: disable=protected-access save_asset_file_mock.assert_called_once_with( html_manipulator.temp_dir, @@ -95,7 +95,7 @@ def test_replace_asset_links(self, save_asset_file_mock: MagicMock) -> None: self.assertEqual(html_manipulator.html_data, html_data_mock) - html_manipulator._replace_asset_links() + html_manipulator._replace_asset_links() # lint-amnesty, pylint: disable=protected-access save_asset_file_mock.assert_called_once_with( html_manipulator.temp_dir, @@ -113,6 +113,6 @@ def test_replace_iframe(self): soup = BeautifulSoup(html_data_mock, 'html.parser') expected_html_markup = """

${_('YouTube Video')}

""" - HtmlManipulator._replace_iframe(soup) + HtmlManipulator._replace_iframe(soup) # lint-amnesty, pylint: disable=protected-access self.assertEqual(f'{soup.find_all("p")[0]}', expected_html_markup) diff --git a/openedx/features/offline_mode/tests/test_renderer.py b/openedx/features/offline_mode/tests/test_renderer.py index fa0a16ce62b0..e9afe0a62507 100644 --- a/openedx/features/offline_mode/tests/test_renderer.py +++ b/openedx/features/offline_mode/tests/test_renderer.py @@ -19,24 +19,24 @@ def setup_course(self): """ default_store = self.store.default_modulestore.get_modulestore_type() with self.store.default_store(default_store): - self.course = CourseFactory.create() + self.course = CourseFactory.create() # lint-amnesty, pylint: disable=attribute-defined-outside-init chapter = BlockFactory.create(parent=self.course, category='chapter') problem_xml = MultipleChoiceResponseXMLFactory().build_xml( question_text='The correct answer is Choice 2', choices=[False, False, True, False], choice_names=['choice_0', 'choice_1', 'choice_2', 'choice_3'] ) - self.vertical_block = BlockFactory.create( + self.vertical_block = BlockFactory.create( # lint-amnesty, pylint: disable=attribute-defined-outside-init parent_location=chapter.location, category='vertical', display_name="Vertical" ) - self.html_block = BlockFactory.create( + self.html_block = BlockFactory.create( # lint-amnesty, pylint: disable=attribute-defined-outside-init parent=self.vertical_block, category='html', data="

Test HTML Content

" ) - self.problem_block = BlockFactory.create( + self.problem_block = BlockFactory.create( # lint-amnesty, pylint: disable=attribute-defined-outside-init parent=self.vertical_block, category='problem', display_name='Problem', diff --git a/openedx/features/offline_mode/tests/test_tasks.py b/openedx/features/offline_mode/tests/test_tasks.py index cd079a20fe80..eaa503498193 100644 --- a/openedx/features/offline_mode/tests/test_tasks.py +++ b/openedx/features/offline_mode/tests/test_tasks.py @@ -3,7 +3,7 @@ """ from unittest import TestCase -from unittest.mock import MagicMock, Mock, call, patch +from unittest.mock import MagicMock, Mock, call, patch from opaque_keys.edx.keys import CourseKey, UsageKey from openedx.features.offline_mode.constants import OFFLINE_SUPPORTED_XBLOCKS From 687b79bfd9e1376d9e4762ad1752f3f98c91feed Mon Sep 17 00:00:00 2001 From: Kyrylo Kireiev <90455454+KyryloKireiev@users.noreply.github.com> Date: Mon, 17 Jun 2024 15:01:16 +0300 Subject: [PATCH 5/9] style: [AXM-636] Ivan's code improving Co-authored-by: Ivan Niedielnitsev <81557788+NiedielnitsevIvan@users.noreply.github.com> --- openedx/features/offline_mode/tests/test_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openedx/features/offline_mode/tests/test_utils.py b/openedx/features/offline_mode/tests/test_utils.py index f520df2c1047..e21c1a911e6a 100644 --- a/openedx/features/offline_mode/tests/test_utils.py +++ b/openedx/features/offline_mode/tests/test_utils.py @@ -24,7 +24,7 @@ class OfflineModeUtilsTestCase(TestCase): @patch('openedx.features.offline_mode.utils.mkdtemp') @patch('openedx.features.offline_mode.utils.remove_old_files') @patch('openedx.features.offline_mode.utils.block_storage_path') - @patch('openedx.features.offline_mode.utils.is_modified') + @patch('openedx.features.offline_mode.utils.is_modified', return_value=True) def test_generate_offline_content_for_modified_xblock( self, is_modified_mock: MagicMock, From a7dc6a2a7db1b599758f681eb273694ae0f7d021 Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Tue, 18 Jun 2024 16:15:00 +0300 Subject: [PATCH 6/9] refactor: [AXM-636] Refactor tests, add new tests --- .../tests/test_course_info_views.py | 27 ++++++++++++++ .../tests/test_html_manipulator.py | 8 ++--- .../offline_mode/tests/test_renderer.py | 8 +++-- .../features/offline_mode/tests/test_tasks.py | 35 +++++++++++++++++-- 4 files changed, 70 insertions(+), 8 deletions(-) diff --git a/lms/djangoapps/mobile_api/tests/test_course_info_views.py b/lms/djangoapps/mobile_api/tests/test_course_info_views.py index 5f9a48b96616..9d4c24f1bc18 100644 --- a/lms/djangoapps/mobile_api/tests/test_course_info_views.py +++ b/lms/djangoapps/mobile_api/tests/test_course_info_views.py @@ -480,3 +480,30 @@ def test_extend_block_info_with_offline_data( self.assertEqual(response.status_code, status.HTTP_200_OK) for block_info in response.data['blocks'].values(): self.assertDictEqual(block_info['offline_download'], expected_offline_download_data) + + @patch('lms.djangoapps.mobile_api.course_info.views.is_offline_mode_enabled') + @ddt.data( + ('v0.5', True), + ('v0.5', False), + ('v1', True), + ('v1', False), + ('v2', True), + ('v2', False), + ('v3', True), + ('v3', False), + ) + @ddt.unpack + def test_not_extend_block_info_with_offline_data_for_version_less_v4_and_any_waffle_flag( + self, + api_version: str, + offline_mode_waffle_flag_mock: MagicMock, + is_offline_mode_enabled_mock: MagicMock, + ) -> None: + url = reverse('blocks_info_in_course', kwargs={'api_version': api_version}) + is_offline_mode_enabled_mock.return_value = offline_mode_waffle_flag_mock + + response = self.verify_response(url=url) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + for block_info in response.data['blocks'].values(): + self.assertNotIn('offline_download', block_info) diff --git a/openedx/features/offline_mode/tests/test_html_manipulator.py b/openedx/features/offline_mode/tests/test_html_manipulator.py index d085a0ebf8c3..a3e8c776ff32 100644 --- a/openedx/features/offline_mode/tests/test_html_manipulator.py +++ b/openedx/features/offline_mode/tests/test_html_manipulator.py @@ -84,13 +84,13 @@ def test_replace_static_links(self, save_asset_file_mock: MagicMock) -> None: @patch('openedx.features.offline_mode.html_manipulator.save_asset_file') def test_replace_asset_links(self, save_asset_file_mock: MagicMock) -> None: - html_data_mock = '

' + html_data_mock = '/assets/courseware/v1/5b628a18f2ee3303081ffe4d6ab64ee4/asset-v1:OpenedX+DemoX+DemoCourse+type@asset+block/Pendleton_Sinking_Ship.jpeg' # lint-amnesty, pylint: disable=line-too-long xblock_mock = Mock() temp_dir_mock = 'temp_dir_mock' html_manipulator = HtmlManipulator(xblock_mock, html_data_mock, temp_dir_mock) expected_html_data_after_replacing = ( - '
' + 'assets/courseware/v1/5b628a18f2ee3303081ffe4d6ab64ee4/asset-v1:OpenedX+DemoX+DemoCourse+type@asset+block/Pendleton_Sinking_Ship.jpeg' # lint-amnesty, pylint: disable=line-too-long ) self.assertEqual(html_manipulator.html_data, html_data_mock) @@ -100,8 +100,8 @@ def test_replace_asset_links(self, save_asset_file_mock: MagicMock) -> None: save_asset_file_mock.assert_called_once_with( html_manipulator.temp_dir, html_manipulator.xblock, - '/assets/images/professor-sandel.jpg', - 'assets/images/professor-sandel.jpg', + html_data_mock, + expected_html_data_after_replacing, ) self.assertEqual(html_manipulator.html_data, expected_html_data_after_replacing) diff --git a/openedx/features/offline_mode/tests/test_renderer.py b/openedx/features/offline_mode/tests/test_renderer.py index e9afe0a62507..91975c50990c 100644 --- a/openedx/features/offline_mode/tests/test_renderer.py +++ b/openedx/features/offline_mode/tests/test_renderer.py @@ -34,12 +34,13 @@ def setup_course(self): self.html_block = BlockFactory.create( # lint-amnesty, pylint: disable=attribute-defined-outside-init parent=self.vertical_block, category='html', - data="

Test HTML Content

" + display_name='HTML xblock for Offline', + data='

Test HTML Content

' ) self.problem_block = BlockFactory.create( # lint-amnesty, pylint: disable=attribute-defined-outside-init parent=self.vertical_block, category='problem', - display_name='Problem', + display_name='Problem xblock for Offline', data=problem_xml ) @@ -51,6 +52,8 @@ def test_render_xblock_from_lms_html_block(self): self.assertIsNotNone(result) self.assertEqual(type(result), str) + self.assertIn('HTML xblock for Offline', result) + self.assertIn('

Test HTML Content

', result) def test_render_xblock_from_lms_problem_block(self): self.setup_course() @@ -60,3 +63,4 @@ def test_render_xblock_from_lms_problem_block(self): self.assertIsNotNone(result) self.assertEqual(type(result), str) + self.assertIn('Problem xblock for Offline', result) diff --git a/openedx/features/offline_mode/tests/test_tasks.py b/openedx/features/offline_mode/tests/test_tasks.py index eaa503498193..a4a4a8ea6c1b 100644 --- a/openedx/features/offline_mode/tests/test_tasks.py +++ b/openedx/features/offline_mode/tests/test_tasks.py @@ -6,11 +6,12 @@ from unittest.mock import MagicMock, Mock, call, patch from opaque_keys.edx.keys import CourseKey, UsageKey -from openedx.features.offline_mode.constants import OFFLINE_SUPPORTED_XBLOCKS +from openedx.features.offline_mode.constants import MAX_RETRY_ATTEMPTS, OFFLINE_SUPPORTED_XBLOCKS from openedx.features.offline_mode.tasks import ( generate_offline_content_for_block, generate_offline_content_for_course, ) +from xmodule.modulestore.exceptions import ItemNotFoundError class GenerateOfflineContentTasksTestCase(TestCase): @@ -20,7 +21,7 @@ class GenerateOfflineContentTasksTestCase(TestCase): @patch('openedx.features.offline_mode.tasks.generate_offline_content') @patch('openedx.features.offline_mode.tasks.modulestore') - def test_generate_offline_content_for_block( + def test_generate_offline_content_for_block_success( self, modulestore_mock: MagicMock, generate_offline_content_mock: MagicMock, @@ -36,6 +37,36 @@ def test_generate_offline_content_for_block( modulestore_mock.return_value.get_item.return_value, html_data_mock ) + @patch('openedx.features.offline_mode.tasks.generate_offline_content') + @patch('openedx.features.offline_mode.tasks.modulestore', side_effect=ItemNotFoundError) + def test_generate_offline_content_for_block_with_exception_in_modulestore( + self, + modulestore_mock: MagicMock, + generate_offline_content_mock: MagicMock, + ) -> None: + block_id_mock = 'block-v1:a+a+a+type@problem+block@fb81e4dbfd4945cb9318d6bc460a956c' + html_data_mock = 'html_markup_data_mock' + + generate_offline_content_for_block.delay(block_id_mock, html_data_mock) + + self.assertEqual(modulestore_mock.call_count, MAX_RETRY_ATTEMPTS + 1) + generate_offline_content_mock.assert_not_called() + + @patch('openedx.features.offline_mode.tasks.generate_offline_content', side_effect=FileNotFoundError) + @patch('openedx.features.offline_mode.tasks.modulestore') + def test_generate_offline_content_for_block_with_exception_in_offline_content_generation( + self, + modulestore_mock: MagicMock, + generate_offline_content_mock: MagicMock, + ) -> None: + block_id_mock = 'block-v1:a+a+a+type@problem+block@fb81e4dbfd4945cb9318d6bc460a956c' + html_data_mock = 'html_markup_data_mock' + + generate_offline_content_for_block.delay(block_id_mock, html_data_mock) + + self.assertEqual(modulestore_mock.call_count, MAX_RETRY_ATTEMPTS + 1) + self.assertEqual(generate_offline_content_mock.call_count, MAX_RETRY_ATTEMPTS + 1) + @patch('openedx.features.offline_mode.tasks.generate_offline_content_for_block') @patch('openedx.features.offline_mode.tasks.XBlockRenderer') @patch('openedx.features.offline_mode.tasks.modulestore') From 34aa2cf8fbc5322ccfc5a4910fbf7a3f67a7231a Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Fri, 21 Jun 2024 12:41:17 +0300 Subject: [PATCH 7/9] test: [AXM-653] Fix tests, refactor, add new functional tests --- .../offline_mode/assets_management.py | 3 +- .../tests/test_assets_management.py | 162 +++------- .../offline_mode/tests/test_renderer.py | 39 +-- .../tests/test_storage_management.py | 302 ++++++++++++++++++ .../features/offline_mode/tests/test_tasks.py | 53 ++- .../features/offline_mode/tests/test_utils.py | 248 -------------- 6 files changed, 397 insertions(+), 410 deletions(-) create mode 100644 openedx/features/offline_mode/tests/test_storage_management.py delete mode 100644 openedx/features/offline_mode/tests/test_utils.py diff --git a/openedx/features/offline_mode/assets_management.py b/openedx/features/offline_mode/assets_management.py index 225990449e5f..6e02a28ae5c8 100644 --- a/openedx/features/offline_mode/assets_management.py +++ b/openedx/features/offline_mode/assets_management.py @@ -39,7 +39,7 @@ def read_static_file(path): def save_asset_file(temp_dir, xblock, path, filename): """ - Saves an asset file to the default storage. + Saves an asset file to the temporary directory. If the filename contains a '/', it reads the static file directly from the file system. Otherwise, it fetches the asset from the AssetManager. @@ -89,7 +89,6 @@ def clean_outdated_xblock_files(xblock): base_path = block_storage_path(xblock) offline_zip_path = os.path.join(base_path, f'{xblock.location.block_id}.zip') - # Delete the 'offline_content.zip' file if it exists if default_storage.exists(offline_zip_path): default_storage.delete(offline_zip_path) log.info(f"Successfully deleted the file: {offline_zip_path}") diff --git a/openedx/features/offline_mode/tests/test_assets_management.py b/openedx/features/offline_mode/tests/test_assets_management.py index e70f1b277bbb..e386d10e2b57 100644 --- a/openedx/features/offline_mode/tests/test_assets_management.py +++ b/openedx/features/offline_mode/tests/test_assets_management.py @@ -2,21 +2,24 @@ Tests for the testing utility functions for managing assets and files for Offline Mode. """ +import os + from datetime import datetime from unittest import TestCase from unittest.mock import MagicMock, Mock, call, patch +from botocore.exceptions import ClientError from django.conf import settings from path import Path from pytz import UTC from openedx.features.offline_mode.assets_management import ( block_storage_path, + clean_outdated_xblock_files, create_subdirectories_for_asset, get_offline_block_content_path, get_static_file_path, is_modified, - remove_old_files, save_asset_file, save_mathjax_to_xblock_assets, ) @@ -99,14 +102,14 @@ def test_save_asset_file_no_slash_in_filename( read_static_file_mock.return_value ) - @patch('openedx.features.offline_mode.assets_management.log.info') + @patch('openedx.features.offline_mode.assets_management.log.warning') @patch( 'openedx.features.offline_mode.assets_management.get_static_file_path', side_effect=ItemNotFoundError ) def test_save_asset_file_can_not_find( self, get_static_file_path_mock: MagicMock, - log_info_mock: MagicMock, + log_warning_mock: MagicMock, ) -> None: temp_dir_mock = 'temp_dir_mock' xblock_mock = Mock() @@ -116,7 +119,9 @@ def test_save_asset_file_can_not_find( save_asset_file(temp_dir_mock, xblock_mock, path_mock, filename_mock) get_static_file_path_mock.assert_called_once_with(filename_mock) - log_info_mock.assert_called_once_with(f'Asset not found: {filename_mock}') + log_warning_mock.assert_called_once_with( + f'Asset not found: {filename_mock}, during offline content generation.' + ) @patch('openedx.features.offline_mode.assets_management.os') def test_create_subdirectories_for_asset_subdirectories_does_not_exist(self, os_mock: MagicMock) -> None: @@ -155,142 +160,73 @@ def test_create_subdirectories_for_asset_subdirectories_exist(self, os_mock: Mag os_mock.mkdir.assert_not_called() @patch('openedx.features.offline_mode.assets_management.log') - @patch('openedx.features.offline_mode.assets_management.shutil.rmtree') - @patch('openedx.features.offline_mode.assets_management.default_storage.exists', side_effect=[False, False]) - @patch('openedx.features.offline_mode.assets_management.os') + @patch('openedx.features.offline_mode.assets_management.default_storage') @patch('openedx.features.offline_mode.assets_management.block_storage_path') - def test_remove_old_files_delete_only_assets_dir( + def test_clean_outdated_xblock_files_successful( self, block_storage_path_mock: MagicMock, - os_mock: MagicMock, - default_storage_exists_mock: MagicMock, - shutil_rmtree_mock: MagicMock, + default_storage_mock: MagicMock, logger_mock: MagicMock, ) -> None: xblock_mock = Mock() - - expected_os_mock_path_join_call_args = [ - call(block_storage_path_mock.return_value, 'assets'), - call(block_storage_path_mock.return_value, 'index.html'), - call(block_storage_path_mock.return_value, f'{xblock_mock.location.block_id}.zip'), - ] - expected_log_info = [ - call(f'Successfully deleted the directory: {os_mock.path.join.return_value}') - ] - expected_default_storage_exists_mock_call_args = [ - call(os_mock.path.join.return_value), call(os_mock.path.join.return_value) - ] - - remove_old_files(xblock_mock) - - block_storage_path_mock.assert_called_once_with(xblock_mock) - self.assertListEqual(os_mock.path.join.call_args_list, expected_os_mock_path_join_call_args) - os_mock.path.isdir.assert_called_once_with(os_mock.path.join.return_value) - shutil_rmtree_mock.assert_called_once_with(os_mock.path.join.return_value) - self.assertListEqual(logger_mock.info.call_args_list, expected_log_info) - self.assertListEqual( - default_storage_exists_mock.call_args_list, expected_default_storage_exists_mock_call_args + default_storage_mock.exists.return_value = True + expected_offline_zip_path = os.path.join( + block_storage_path_mock.return_value, f'{xblock_mock.location.block_id}.zip' ) - os_mock.remove.assert_not_called() - - @patch('openedx.features.offline_mode.assets_management.log') - @patch('openedx.features.offline_mode.assets_management.shutil.rmtree') - @patch('openedx.features.offline_mode.assets_management.default_storage.exists', side_effect=[True, False]) - @patch('openedx.features.offline_mode.assets_management.os') - @patch('openedx.features.offline_mode.assets_management.block_storage_path') - def test_remove_old_files_delete_assets_dir_and_index( - self, - block_storage_path_mock: MagicMock, - os_mock: MagicMock, - default_storage_exists_mock: MagicMock, - shutil_rmtree_mock: MagicMock, - logger_mock: MagicMock, - ) -> None: - xblock_mock = Mock() - - expected_os_mock_path_join_call_args = [ - call(block_storage_path_mock.return_value, 'assets'), - call(block_storage_path_mock.return_value, 'index.html'), - call(block_storage_path_mock.return_value, f'{xblock_mock.location.block_id}.zip'), - ] - expected_log_info = [ - call(f'Successfully deleted the directory: {os_mock.path.join.return_value}'), - call(f'Successfully deleted the file: {os_mock.path.join.return_value}'), - ] - expected_default_storage_exists_mock_call_args = [ - call(os_mock.path.join.return_value), call(os_mock.path.join.return_value) - ] - expected_os_mock_remove_call_args = [call(os_mock.path.join.return_value)] - remove_old_files(xblock_mock) + clean_outdated_xblock_files(xblock_mock) block_storage_path_mock.assert_called_once_with(xblock_mock) - self.assertListEqual(os_mock.path.join.call_args_list, expected_os_mock_path_join_call_args) - os_mock.path.isdir.assert_called_once_with(os_mock.path.join.return_value) - shutil_rmtree_mock.assert_called_once_with(os_mock.path.join.return_value) - self.assertListEqual(logger_mock.info.call_args_list, expected_log_info) - self.assertListEqual( - default_storage_exists_mock.call_args_list, expected_default_storage_exists_mock_call_args - ) - self.assertListEqual(os_mock.remove.call_args_list, expected_os_mock_remove_call_args) + default_storage_mock.exists.assert_called_once_with(expected_offline_zip_path) + default_storage_mock.delete.assert_called_once_with(expected_offline_zip_path) + logger_mock.info.assert_called_once_with(f'Successfully deleted the file: {expected_offline_zip_path}') @patch('openedx.features.offline_mode.assets_management.log') - @patch('openedx.features.offline_mode.assets_management.shutil.rmtree') - @patch('openedx.features.offline_mode.assets_management.default_storage.exists', side_effect=[True, True]) - @patch('openedx.features.offline_mode.assets_management.os') + @patch('openedx.features.offline_mode.assets_management.default_storage') @patch('openedx.features.offline_mode.assets_management.block_storage_path') - def test_remove_old_files_delete_assets_dir_and_index_and_offline_content_zip_file( + def test_clean_outdated_xblock_files_does_not_exist( self, block_storage_path_mock: MagicMock, - os_mock: MagicMock, - default_storage_exists_mock: MagicMock, - shutil_rmtree_mock: MagicMock, + default_storage_mock: MagicMock, logger_mock: MagicMock, ) -> None: xblock_mock = Mock() + default_storage_mock.exists.return_value = False + expected_offline_zip_path = os.path.join( + block_storage_path_mock.return_value, f'{xblock_mock.location.block_id}.zip' + ) - expected_os_mock_path_join_call_args = [ - call(block_storage_path_mock.return_value, 'assets'), - call(block_storage_path_mock.return_value, 'index.html'), - call(block_storage_path_mock.return_value, f'{xblock_mock.location.block_id}.zip') - ] - expected_log_info = [ - call(f'Successfully deleted the directory: {os_mock.path.join.return_value}'), - call(f'Successfully deleted the file: {os_mock.path.join.return_value}'), - call(f'Successfully deleted the file: {os_mock.path.join.return_value}'), - ] - expected_default_storage_exists_mock_call_args = [ - call(os_mock.path.join.return_value), call(os_mock.path.join.return_value) - ] - expected_os_mock_remove_call_args = [ - call(os_mock.path.join.return_value), call(os_mock.path.join.return_value) - ] - - remove_old_files(xblock_mock) + clean_outdated_xblock_files(xblock_mock) block_storage_path_mock.assert_called_once_with(xblock_mock) - self.assertListEqual(os_mock.path.join.call_args_list, expected_os_mock_path_join_call_args) - os_mock.path.isdir.assert_called_once_with(os_mock.path.join.return_value) - shutil_rmtree_mock.assert_called_once_with(os_mock.path.join.return_value) - self.assertListEqual(logger_mock.info.call_args_list, expected_log_info) - self.assertListEqual( - default_storage_exists_mock.call_args_list, expected_default_storage_exists_mock_call_args - ) - self.assertListEqual(os_mock.remove.call_args_list, expected_os_mock_remove_call_args) + default_storage_mock.exists.assert_called_once_with(expected_offline_zip_path) + default_storage_mock.delete.assert_not_called() + logger_mock.info.assert_not_called() @patch('openedx.features.offline_mode.assets_management.log.error') - @patch('openedx.features.offline_mode.assets_management.block_storage_path', side_effect=OSError) - def test_remove_old_files_os_error( + @patch('openedx.features.offline_mode.assets_management.default_storage.exists') + @patch('openedx.features.offline_mode.assets_management.block_storage_path') + def test_remove_old_files_client_error( self, block_storage_path_mock: MagicMock, + default_storage_exists_mock: MagicMock, log_error_mock: MagicMock, ) -> None: xblock_mock = Mock() + default_storage_exists_mock.side_effect = ClientError( + operation_name='InvalidKeyPair.Duplicate', error_response={ + 'Error': {'Code': 'Duplicate', 'Message': 'Invalid File Path'} + } + ) + expected_error_message = ( + 'An error occurred (Duplicate) when calling the InvalidKeyPair.Duplicate operation: Invalid File Path' + ) - remove_old_files(xblock_mock) - + clean_outdated_xblock_files(xblock_mock) block_storage_path_mock.assert_called_once_with(xblock_mock) - log_error_mock.assert_called_once_with('Error occurred while deleting the files or directory: ') + log_error_mock.assert_called_once_with( + f'Error occurred while deleting the files or directory: {expected_error_message}' + ) @patch('openedx.features.offline_mode.assets_management.default_storage.exists') @patch('openedx.features.offline_mode.assets_management.os.path.join', return_value='offline_zip_path_mock') @@ -345,7 +281,7 @@ def test_block_storage_path_does_not_exists(self) -> None: self.assertEqual(result, '') @patch( - 'openedx.features.offline_mode.assets_management.default_storage.get_created_time', + 'openedx.features.offline_mode.assets_management.default_storage.get_modified_time', return_value=datetime(2024, 6, 12, tzinfo=UTC) ) @patch('openedx.features.offline_mode.assets_management.block_storage_path') @@ -366,7 +302,7 @@ def test_is_modified_true( self.assertEqual(result, True) @patch( - 'openedx.features.offline_mode.assets_management.default_storage.get_created_time', + 'openedx.features.offline_mode.assets_management.default_storage.get_modified_time', return_value=datetime(2024, 6, 12, tzinfo=UTC) ) @patch('openedx.features.offline_mode.assets_management.block_storage_path') @@ -387,7 +323,7 @@ def test_is_modified_false( self.assertEqual(result, False) @patch( - 'openedx.features.offline_mode.assets_management.default_storage.get_created_time', + 'openedx.features.offline_mode.assets_management.default_storage.get_modified_time', side_effect=OSError ) @patch('openedx.features.offline_mode.assets_management.block_storage_path') diff --git a/openedx/features/offline_mode/tests/test_renderer.py b/openedx/features/offline_mode/tests/test_renderer.py index 91975c50990c..b46d75431cf4 100644 --- a/openedx/features/offline_mode/tests/test_renderer.py +++ b/openedx/features/offline_mode/tests/test_renderer.py @@ -3,49 +3,15 @@ """ from openedx.features.offline_mode.renderer import XBlockRenderer -from xmodule.capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory +from openedx.features.offline_mode.tests.base import CourseForOfflineTestCase -class XBlockRendererTestCase(ModuleStoreTestCase): +class XBlockRendererTestCase(CourseForOfflineTestCase): """ Test case for the testing `XBlockRenderer`. """ - def setup_course(self): - """ - Helper method to create the course. - """ - default_store = self.store.default_modulestore.get_modulestore_type() - with self.store.default_store(default_store): - self.course = CourseFactory.create() # lint-amnesty, pylint: disable=attribute-defined-outside-init - chapter = BlockFactory.create(parent=self.course, category='chapter') - problem_xml = MultipleChoiceResponseXMLFactory().build_xml( - question_text='The correct answer is Choice 2', - choices=[False, False, True, False], - choice_names=['choice_0', 'choice_1', 'choice_2', 'choice_3'] - ) - self.vertical_block = BlockFactory.create( # lint-amnesty, pylint: disable=attribute-defined-outside-init - parent_location=chapter.location, - category='vertical', - display_name="Vertical" - ) - self.html_block = BlockFactory.create( # lint-amnesty, pylint: disable=attribute-defined-outside-init - parent=self.vertical_block, - category='html', - display_name='HTML xblock for Offline', - data='

Test HTML Content

' - ) - self.problem_block = BlockFactory.create( # lint-amnesty, pylint: disable=attribute-defined-outside-init - parent=self.vertical_block, - category='problem', - display_name='Problem xblock for Offline', - data=problem_xml - ) - def test_render_xblock_from_lms_html_block(self): - self.setup_course() xblock_renderer = XBlockRenderer(str(self.html_block.location), user=self.user) result = xblock_renderer.render_xblock_from_lms() @@ -56,7 +22,6 @@ def test_render_xblock_from_lms_html_block(self): self.assertIn('

Test HTML Content

', result) def test_render_xblock_from_lms_problem_block(self): - self.setup_course() xblock_renderer = XBlockRenderer(str(self.problem_block.location), user=self.user) result = xblock_renderer.render_xblock_from_lms() diff --git a/openedx/features/offline_mode/tests/test_storage_management.py b/openedx/features/offline_mode/tests/test_storage_management.py new file mode 100644 index 000000000000..c226c20ac1f8 --- /dev/null +++ b/openedx/features/offline_mode/tests/test_storage_management.py @@ -0,0 +1,302 @@ +""" +Tests for the testing Offline Mode storage management. +""" + +import os +import shutil +from unittest import TestCase +from unittest.mock import MagicMock, Mock, call, patch + +from django.http.response import Http404 + +from openedx.features.offline_mode.constants import MATHJAX_STATIC_PATH +from openedx.features.offline_mode.storage_management import OfflineContentGenerator +from openedx.features.offline_mode.tests.base import CourseForOfflineTestCase + + +class OfflineContentGeneratorTestCase(TestCase): + """ + Test case for the testing Offline Mode utils. + """ + + @patch('openedx.features.offline_mode.storage_management.shutil.rmtree') + @patch('openedx.features.offline_mode.storage_management.OfflineContentGenerator.create_zip_file') + @patch('openedx.features.offline_mode.storage_management.OfflineContentGenerator.save_xblock_html') + @patch('openedx.features.offline_mode.storage_management.mkdtemp') + @patch('openedx.features.offline_mode.storage_management.clean_outdated_xblock_files') + @patch('openedx.features.offline_mode.storage_management.block_storage_path') + @patch('openedx.features.offline_mode.storage_management.is_modified', return_value=True) + def test_generate_offline_content_for_modified_xblock( + self, + is_modified_mock: MagicMock, + block_storage_path_mock: MagicMock, + clean_outdated_xblock_files_mock: MagicMock, + mkdtemp_mock: MagicMock, + save_xblock_html_mock: MagicMock, + create_zip_file_mock: MagicMock, + shutil_rmtree_mock: MagicMock, + ) -> None: + xblock_mock = Mock() + html_data_mock = 'html_markup_data_mock' + + OfflineContentGenerator(xblock_mock, html_data_mock).generate_offline_content() + + is_modified_mock.assert_called_once_with(xblock_mock) + block_storage_path_mock.assert_called_once_with(xblock_mock) + clean_outdated_xblock_files_mock.assert_called_once_with(xblock_mock) + mkdtemp_mock.assert_called_once_with() + save_xblock_html_mock.assert_called_once_with(mkdtemp_mock.return_value) + create_zip_file_mock.assert_called_once_with( + mkdtemp_mock.return_value, + block_storage_path_mock.return_value, + f'{xblock_mock.location.block_id}.zip' + ) + shutil_rmtree_mock.assert_called_once_with(mkdtemp_mock.return_value, ignore_errors=True) + + @patch('openedx.features.offline_mode.storage_management.shutil.rmtree') + @patch('openedx.features.offline_mode.storage_management.OfflineContentGenerator.create_zip_file') + @patch('openedx.features.offline_mode.storage_management.OfflineContentGenerator.save_xblock_html') + @patch('openedx.features.offline_mode.storage_management.mkdtemp') + @patch('openedx.features.offline_mode.storage_management.clean_outdated_xblock_files') + @patch('openedx.features.offline_mode.storage_management.block_storage_path') + @patch('openedx.features.offline_mode.storage_management.is_modified', return_value=False) + def test_generate_offline_content_is_not_modified( + self, + is_modified_mock: MagicMock, + block_storage_path_mock: MagicMock, + clean_outdated_xblock_files_mock: MagicMock, + mkdtemp_mock: MagicMock, + save_xblock_html_mock: MagicMock, + create_zip_file_mock: MagicMock, + shutil_rmtree_mock: MagicMock, + ) -> None: + xblock_mock = Mock() + html_data_mock = 'html_markup_data_mock' + + OfflineContentGenerator(xblock_mock, html_data_mock).generate_offline_content() + + is_modified_mock.assert_called_once_with(xblock_mock) + block_storage_path_mock.assert_not_called() + clean_outdated_xblock_files_mock.assert_not_called() + mkdtemp_mock.assert_not_called() + save_xblock_html_mock.assert_not_called() + create_zip_file_mock.assert_not_called() + shutil_rmtree_mock.assert_not_called() + + @patch('openedx.features.offline_mode.storage_management.shutil.rmtree') + @patch('openedx.features.offline_mode.storage_management.log.error') + @patch( + 'openedx.features.offline_mode.storage_management.OfflineContentGenerator.create_zip_file', + side_effect=Http404, + ) + @patch('openedx.features.offline_mode.storage_management.OfflineContentGenerator.save_xblock_html') + @patch('openedx.features.offline_mode.storage_management.mkdtemp') + @patch('openedx.features.offline_mode.storage_management.clean_outdated_xblock_files') + @patch('openedx.features.offline_mode.storage_management.block_storage_path') + @patch('openedx.features.offline_mode.storage_management.is_modified', return_value=True) + def test_generate_offline_content_is_not_modified_with_http_404_status( + self, + is_modified_mock: MagicMock, + block_storage_path_mock: MagicMock, + clean_outdated_xblock_files_mock: MagicMock, + mkdtemp_mock: MagicMock, + save_xblock_html_mock: MagicMock, + create_zip_file_mock: MagicMock, + logger_mock: MagicMock, + shutil_rmtree_mock: MagicMock, + ) -> None: + xblock_mock = Mock() + html_data_mock = 'html_markup_data_mock' + + OfflineContentGenerator(xblock_mock, html_data_mock).generate_offline_content() + + is_modified_mock.assert_called_once_with(xblock_mock) + block_storage_path_mock.assert_called_once_with(xblock_mock) + clean_outdated_xblock_files_mock.assert_called_once_with(xblock_mock) + mkdtemp_mock.assert_called_once_with() + save_xblock_html_mock.assert_called_once_with(mkdtemp_mock.return_value) + create_zip_file_mock.assert_called_once_with( + mkdtemp_mock.return_value, + block_storage_path_mock.return_value, + f'{xblock_mock.location.block_id}.zip' + ) + logger_mock.assert_called_once_with( + f'Block {xblock_mock.location.block_id} cannot be fetched from course' + f' {xblock_mock.location.course_key} during offline content generation.' + ) + shutil_rmtree_mock.assert_called_once_with(mkdtemp_mock.return_value, ignore_errors=True) + + @patch('openedx.features.offline_mode.storage_management.os.path.join') + @patch('openedx.features.offline_mode.storage_management.open') + @patch('openedx.features.offline_mode.storage_management.HtmlManipulator') + def test_save_xblock_html( + self, + html_manipulator_mock: MagicMock, + context_manager_mock: MagicMock, + os_path_join_mock: MagicMock, + ) -> None: + tmp_dir_mock = Mock() + xblock_mock = Mock() + html_data_mock = 'html_markup_data_mock' + + OfflineContentGenerator(xblock_mock, html_data_mock).save_xblock_html(tmp_dir_mock) + + html_manipulator_mock.assert_called_once_with(xblock_mock, html_data_mock, tmp_dir_mock) + html_manipulator_mock.return_value.process_html.assert_called_once_with() + context_manager_mock.assert_called_once_with(os_path_join_mock.return_value, 'w') + os_path_join_mock.assert_called_once_with(tmp_dir_mock, 'index.html') + context_manager_mock.return_value.__enter__.return_value.write.assert_called_once_with( + html_manipulator_mock.return_value.process_html.return_value + ) + + @patch('openedx.features.offline_mode.storage_management.log.info') + @patch('openedx.features.offline_mode.storage_management.ContentFile') + @patch('openedx.features.offline_mode.storage_management.open') + @patch('openedx.features.offline_mode.storage_management.get_storage') + @patch('openedx.features.offline_mode.storage_management.OfflineContentGenerator.add_files_to_zip_recursively') + @patch('openedx.features.offline_mode.storage_management.ZipFile') + def test_create_zip_file( + self, + zip_file_context_manager: MagicMock, + add_files_to_zip_recursively_mock: MagicMock, + storage_mock: MagicMock, + open_context_manager_mock: MagicMock, + content_file_mock: MagicMock, + log_info_mock: MagicMock, + ) -> None: + xblock_mock = Mock() + html_data_mock = 'html_markup_data_mock' + temp_dir_mock = 'temp_dir_mock' + base_path_mock = 'base_path_mock' + file_name_mock = 'file_name_mock' + + OfflineContentGenerator(xblock_mock, html_data_mock).create_zip_file( + temp_dir_mock, base_path_mock, file_name_mock + ) + + zip_file_context_manager.assert_called_once_with(os.path.join(temp_dir_mock, file_name_mock), 'w') + zip_file_context_manager.return_value.__enter__.return_value.write.assert_called_once_with( + os.path.join(temp_dir_mock, 'index.html'), 'index.html' + ) + add_files_to_zip_recursively_mock.assert_called_once_with( + zip_file_context_manager.return_value.__enter__.return_value, + current_base_path=os.path.join(temp_dir_mock, 'assets'), + current_path_in_zip='assets', + ) + open_context_manager_mock.assert_called_once_with(os.path.join(temp_dir_mock, file_name_mock), 'rb') + content_file_mock.assert_called_once_with( + open_context_manager_mock.return_value.__enter__.return_value.read.return_value + ) + storage_mock.return_value.save.assert_called_once_with( + os.path.join(base_path_mock + file_name_mock), content_file_mock.return_value + ) + log_info_mock.assert_called_once_with( + f'Offline content for {file_name_mock} has been generated.' + ) + + @patch('openedx.features.offline_mode.storage_management.os') + def test_add_files_to_zip_recursively_successfully_for_file( + self, + os_mock: MagicMock, + ) -> None: + xblock_mock = Mock() + html_data_mock = 'html_markup_data_mock' + zip_file_mock = Mock() + current_base_path_mock = 'current_base_path_mock' + current_path_in_zip_mock = 'current_path_in_zip_mock' + resource_path_mock = 'resource_path_mock' + os_mock.listdir.return_value = [resource_path_mock] + + expected_os_mock_path_join_calls = [ + call(current_base_path_mock, resource_path_mock), + call(current_path_in_zip_mock, resource_path_mock) + ] + + OfflineContentGenerator(xblock_mock, html_data_mock).add_files_to_zip_recursively( + zip_file_mock, current_base_path_mock, current_path_in_zip_mock + ) + + os_mock.listdir.assert_called_once_with(current_base_path_mock) + self.assertListEqual(os_mock.path.join.call_args_list, expected_os_mock_path_join_calls) + zip_file_mock.write.assert_called_once_with(os_mock.path.join.return_value, os_mock.path.join.return_value) + + @patch('openedx.features.offline_mode.storage_management.OfflineContentGenerator.add_files_to_zip_recursively') + @patch('openedx.features.offline_mode.storage_management.os.listdir') + def test_add_files_to_zip_recursively_successfully_recursively_path( + self, + os_listdir_mock: MagicMock, + add_files_to_zip_recursively_mock: MagicMock, + ) -> None: + xblock_mock = Mock() + html_data_mock = 'html_markup_data_mock' + zip_file_mock = Mock() + current_base_path_mock = 'current_base_path_mock' + current_path_in_zip_mock = 'current_path_in_zip_mock' + resource_path_mock = 'resource_path_mock' + os_listdir_mock.listdir.return_value = [resource_path_mock] + + OfflineContentGenerator(xblock_mock, html_data_mock).add_files_to_zip_recursively( + zip_file_mock, current_base_path_mock, current_path_in_zip_mock + ) + + add_files_to_zip_recursively_mock.assert_called_once_with( + zip_file_mock, current_base_path_mock, current_path_in_zip_mock + ) + + @patch('openedx.features.offline_mode.storage_management.log.error') + @patch('openedx.features.offline_mode.storage_management.os.listdir', side_effect=OSError) + def test_add_files_to_zip_recursively_with_os_error( + self, + os_mock: MagicMock, + log_error_mock: MagicMock, + ) -> None: + xblock_mock = Mock() + html_data_mock = 'html_markup_data_mock' + zip_file_mock = Mock() + current_base_path_mock = 'current_base_path_mock' + current_path_in_zip_mock = 'current_path_in_zip_mock' + + OfflineContentGenerator(xblock_mock, html_data_mock).add_files_to_zip_recursively( + zip_file_mock, current_base_path_mock, current_path_in_zip_mock + ) + + os_mock.assert_called_once_with(current_base_path_mock) + log_error_mock.assert_called_once_with(f'Error while reading the directory: {current_base_path_mock}') + + +class OfflineContentGeneratorFunctionalTestCase(CourseForOfflineTestCase): + """ + Tests creating Offline Content in storage. + """ + + def setUp(self): + super().setUp() + self.html_data = '

Test HTML Content

' # lint-amnesty, pylint: disable=attribute-defined-outside-init + + @patch('openedx.features.offline_mode.html_manipulator.save_mathjax_to_xblock_assets') + def test_generate_offline_content(self, save_mathjax_to_xblock_assets_mock): + OfflineContentGenerator(self.html_block, self.html_data).generate_offline_content() + + expected_offline_content_path = 'test_root/uploads/course-v1:RaccoonGang+1+2024/HTML_xblock_for_Offline.zip' + + save_mathjax_to_xblock_assets_mock.assert_called_once() + self.assertTrue(os.path.exists(expected_offline_content_path)) + shutil.rmtree('test_root/uploads/course-v1:RaccoonGang+1+2024', ignore_errors=True) + + def test_save_xblock_html_to_temp_dir(self): + shutil.rmtree('test_root/assets', ignore_errors=True) + temp_dir = 'test_root/' + os.makedirs('test_root/assets/js/') + OfflineContentGenerator(self.html_block, self.html_data).save_xblock_html(temp_dir) + + expected_index_html_path = 'test_root/index.html' + expected_mathjax_static_path = os.path.join(temp_dir, MATHJAX_STATIC_PATH) + + self.assertTrue(os.path.exists(expected_index_html_path)) + self.assertTrue(os.path.exists(expected_mathjax_static_path)) + with open(expected_index_html_path, 'r') as content: + html_data = content.read() + self.assertIn(self.html_data, html_data) + + shutil.rmtree('test_root/assets', ignore_errors=True) + os.remove(expected_index_html_path) diff --git a/openedx/features/offline_mode/tests/test_tasks.py b/openedx/features/offline_mode/tests/test_tasks.py index a4a4a8ea6c1b..75512d8e8b96 100644 --- a/openedx/features/offline_mode/tests/test_tasks.py +++ b/openedx/features/offline_mode/tests/test_tasks.py @@ -19,12 +19,12 @@ class GenerateOfflineContentTasksTestCase(TestCase): Test case for the testing generating offline content tacks. """ - @patch('openedx.features.offline_mode.tasks.generate_offline_content') + @patch('openedx.features.offline_mode.tasks.OfflineContentGenerator') @patch('openedx.features.offline_mode.tasks.modulestore') def test_generate_offline_content_for_block_success( self, modulestore_mock: MagicMock, - generate_offline_content_mock: MagicMock, + offline_content_generator_mock: MagicMock, ) -> None: block_id_mock = 'block-v1:a+a+a+type@problem+block@fb81e4dbfd4945cb9318d6bc460a956c' html_data_mock = 'html_markup_data_mock' @@ -33,16 +33,17 @@ def test_generate_offline_content_for_block_success( modulestore_mock.assert_called_once_with() modulestore_mock.return_value.get_item.assert_called_once_with(UsageKey.from_string(block_id_mock)) - generate_offline_content_mock.assert_called_once_with( + offline_content_generator_mock.assert_called_once_with( modulestore_mock.return_value.get_item.return_value, html_data_mock ) + offline_content_generator_mock.return_value.generate_offline_content.assert_called_once_with() - @patch('openedx.features.offline_mode.tasks.generate_offline_content') + @patch('openedx.features.offline_mode.tasks.OfflineContentGenerator') @patch('openedx.features.offline_mode.tasks.modulestore', side_effect=ItemNotFoundError) def test_generate_offline_content_for_block_with_exception_in_modulestore( self, modulestore_mock: MagicMock, - generate_offline_content_mock: MagicMock, + offline_content_generator_mock: MagicMock, ) -> None: block_id_mock = 'block-v1:a+a+a+type@problem+block@fb81e4dbfd4945cb9318d6bc460a956c' html_data_mock = 'html_markup_data_mock' @@ -50,14 +51,14 @@ def test_generate_offline_content_for_block_with_exception_in_modulestore( generate_offline_content_for_block.delay(block_id_mock, html_data_mock) self.assertEqual(modulestore_mock.call_count, MAX_RETRY_ATTEMPTS + 1) - generate_offline_content_mock.assert_not_called() + offline_content_generator_mock.assert_not_called() - @patch('openedx.features.offline_mode.tasks.generate_offline_content', side_effect=FileNotFoundError) + @patch('openedx.features.offline_mode.tasks.OfflineContentGenerator', side_effect=FileNotFoundError) @patch('openedx.features.offline_mode.tasks.modulestore') def test_generate_offline_content_for_block_with_exception_in_offline_content_generation( self, modulestore_mock: MagicMock, - generate_offline_content_mock: MagicMock, + offline_content_generator_mock: MagicMock, ) -> None: block_id_mock = 'block-v1:a+a+a+type@problem+block@fb81e4dbfd4945cb9318d6bc460a956c' html_data_mock = 'html_markup_data_mock' @@ -65,7 +66,7 @@ def test_generate_offline_content_for_block_with_exception_in_offline_content_ge generate_offline_content_for_block.delay(block_id_mock, html_data_mock) self.assertEqual(modulestore_mock.call_count, MAX_RETRY_ATTEMPTS + 1) - self.assertEqual(generate_offline_content_mock.call_count, MAX_RETRY_ATTEMPTS + 1) + self.assertEqual(offline_content_generator_mock.call_count, MAX_RETRY_ATTEMPTS + 1) @patch('openedx.features.offline_mode.tasks.generate_offline_content_for_block') @patch('openedx.features.offline_mode.tasks.XBlockRenderer') @@ -79,7 +80,9 @@ def test_generate_offline_content_for_course_supported_block_types( course_id_mock = 'course-v1:a+a+a' xblock_location_mock = 'xblock_location_mock' html_data_mock = 'html_markup_data_mock' - modulestore_mock.return_value.get_items.return_value = [Mock(location=xblock_location_mock)] + modulestore_mock.return_value.get_items.return_value = [ + Mock(location=xblock_location_mock, closed=Mock(return_value=False)) + ] xblock_renderer_mock.return_value.render_xblock_from_lms.return_value = html_data_mock expected_call_args_for_modulestore_get_items = [ call(CourseKey.from_string(course_id_mock), qualifiers={'category': offline_supported_block_type}) @@ -105,3 +108,33 @@ def test_generate_offline_content_for_course_supported_block_types( generate_offline_content_for_block_mock.apply_async.call_args_list, expected_call_args_for_generate_offline_content_for_block_mock ) + + @patch('openedx.features.offline_mode.tasks.generate_offline_content_for_block') + @patch('openedx.features.offline_mode.tasks.XBlockRenderer') + @patch('openedx.features.offline_mode.tasks.modulestore') + def test_generate_offline_content_for_course_supported_block_types_closed_xblock( + self, + modulestore_mock: MagicMock, + xblock_renderer_mock: MagicMock, + generate_offline_content_for_block_mock: MagicMock, + ) -> None: + course_id_mock = 'course-v1:a+a+a' + xblock_location_mock = 'xblock_location_mock' + html_data_mock = 'html_markup_data_mock' + modulestore_mock.return_value.get_items.return_value = [ + Mock(location=xblock_location_mock, closed=Mock(return_value=True)) + ] + xblock_renderer_mock.return_value.render_xblock_from_lms.return_value = html_data_mock + expected_call_args_for_modulestore_get_items = [ + call(CourseKey.from_string(course_id_mock), qualifiers={'category': offline_supported_block_type}) + for offline_supported_block_type in OFFLINE_SUPPORTED_XBLOCKS + ] + + generate_offline_content_for_course(course_id_mock) + + self.assertEqual(modulestore_mock.call_count, len(OFFLINE_SUPPORTED_XBLOCKS)) + self.assertListEqual( + modulestore_mock.return_value.get_items.call_args_list, expected_call_args_for_modulestore_get_items + ) + xblock_renderer_mock.assert_not_called() + generate_offline_content_for_block_mock.assert_not_called() diff --git a/openedx/features/offline_mode/tests/test_utils.py b/openedx/features/offline_mode/tests/test_utils.py deleted file mode 100644 index e21c1a911e6a..000000000000 --- a/openedx/features/offline_mode/tests/test_utils.py +++ /dev/null @@ -1,248 +0,0 @@ -""" -Tests for the testing Offline Mode utils. -""" - -from unittest import TestCase -from unittest.mock import MagicMock, Mock, call, patch - -from openedx.features.offline_mode.utils import ( - add_files_to_zip_recursively, - create_zip_file, - generate_offline_content, - save_xblock_html, -) - - -class OfflineModeUtilsTestCase(TestCase): - """ - Test case for the testing Offline Mode utils. - """ - - @patch('openedx.features.offline_mode.utils.shutil.rmtree') - @patch('openedx.features.offline_mode.utils.create_zip_file') - @patch('openedx.features.offline_mode.utils.save_xblock_html') - @patch('openedx.features.offline_mode.utils.mkdtemp') - @patch('openedx.features.offline_mode.utils.remove_old_files') - @patch('openedx.features.offline_mode.utils.block_storage_path') - @patch('openedx.features.offline_mode.utils.is_modified', return_value=True) - def test_generate_offline_content_for_modified_xblock( - self, - is_modified_mock: MagicMock, - block_storage_path_mock: MagicMock, - remove_old_files_mock: MagicMock, - mkdtemp_mock: MagicMock, - save_xblock_html_mock: MagicMock, - create_zip_file_mock: MagicMock, - shutil_rmtree_mock: MagicMock, - ) -> None: - xblock_mock = Mock() - html_data_mock = 'html_markup_data_mock' - - generate_offline_content(xblock_mock, html_data_mock) - - is_modified_mock.assert_called_once_with(xblock_mock) - block_storage_path_mock.assert_called_once_with(xblock_mock) - remove_old_files_mock.assert_called_once_with(xblock_mock) - mkdtemp_mock.assert_called_once_with() - save_xblock_html_mock.assert_called_once_with(mkdtemp_mock.return_value, xblock_mock, html_data_mock) - create_zip_file_mock.assert_called_once_with( - mkdtemp_mock.return_value, - block_storage_path_mock.return_value, - f'{xblock_mock.location.block_id}.zip' - ) - shutil_rmtree_mock.assert_called_once_with(mkdtemp_mock.return_value, ignore_errors=True) - - @patch('openedx.features.offline_mode.utils.shutil.rmtree') - @patch('openedx.features.offline_mode.utils.create_zip_file') - @patch('openedx.features.offline_mode.utils.save_xblock_html') - @patch('openedx.features.offline_mode.utils.mkdtemp') - @patch('openedx.features.offline_mode.utils.remove_old_files') - @patch('openedx.features.offline_mode.utils.block_storage_path') - @patch('openedx.features.offline_mode.utils.is_modified', return_value=False) - def test_generate_offline_content_is_not_modified( - self, - is_modified_mock: MagicMock, - block_storage_path_mock: MagicMock, - remove_old_files_mock: MagicMock, - mkdtemp_mock: MagicMock, - save_xblock_html_mock: MagicMock, - create_zip_file_mock: MagicMock, - shutil_rmtree_mock: MagicMock, - ) -> None: - xblock_mock = Mock() - html_data_mock = 'html_markup_data_mock' - - generate_offline_content(xblock_mock, html_data_mock) - - is_modified_mock.assert_called_once_with(xblock_mock) - block_storage_path_mock.assert_not_called() - remove_old_files_mock.assert_not_called() - mkdtemp_mock.assert_not_called() - save_xblock_html_mock.assert_not_called() - create_zip_file_mock.assert_not_called() - shutil_rmtree_mock.assert_not_called() - - @patch('openedx.features.offline_mode.utils.os.path.join') - @patch('openedx.features.offline_mode.utils.open') - @patch('openedx.features.offline_mode.utils.HtmlManipulator') - def test_save_xblock_html( - self, - html_manipulator_mock: MagicMock, - context_manager_mock: MagicMock, - os_path_join_mock: MagicMock, - ) -> None: - tmp_dir_mock = Mock() - xblock_mock = Mock() - html_data_mock = 'html_markup_data_mock' - - save_xblock_html(tmp_dir_mock, xblock_mock, html_data_mock) - - html_manipulator_mock.assert_called_once_with(xblock_mock, html_data_mock, tmp_dir_mock) - html_manipulator_mock.return_value.process_html.assert_called_once_with() - context_manager_mock.assert_called_once_with(os_path_join_mock.return_value, 'w') - os_path_join_mock.assert_called_once_with(tmp_dir_mock, 'index.html') - context_manager_mock.return_value.__enter__.return_value.write.assert_called_once_with( - html_manipulator_mock.return_value.process_html.return_value - ) - - @patch('openedx.features.offline_mode.utils.log.info') - @patch('openedx.features.offline_mode.utils.add_files_to_zip_recursively') - @patch('openedx.features.offline_mode.utils.ZipFile') - @patch('openedx.features.offline_mode.utils.default_storage') - @patch('openedx.features.offline_mode.utils.os') - def test_create_zip_file_os_path_exists( - self, - os_mock: MagicMock, - default_storage_mock: MagicMock, - zip_file_context_manager: MagicMock, - add_files_to_zip_recursively_mock: MagicMock, - log_info_mock: MagicMock, - ) -> None: - temp_dir_mock = 'temp_dir_mock' - base_path_mock = 'base_path_mock' - file_name_mock = 'file_name_mock' - - create_zip_file(temp_dir_mock, base_path_mock, file_name_mock) - - os_mock.path.exists.assert_called_once_with(default_storage_mock.path.return_value) - os_mock.makedirs.assert_not_called() - self.assertListEqual( - default_storage_mock.path.call_args_list, - [call(base_path_mock), call(base_path_mock + file_name_mock)] - ) - zip_file_context_manager.assert_called_once_with(default_storage_mock.path.return_value, 'w') - zip_file_context_manager.return_value.__enter__.return_value.write.assert_called_once_with( - os_mock.path.join.return_value, 'index.html' - ) - add_files_to_zip_recursively_mock.assert_called_once_with( - zip_file_context_manager.return_value.__enter__.return_value, - current_base_path=os_mock.path.join.return_value, - current_path_in_zip='assets', - ) - log_info_mock.assert_called_once_with( - f'Offline content for {file_name_mock} has been generated.' - ) - - @patch('openedx.features.offline_mode.utils.log.info') - @patch('openedx.features.offline_mode.utils.add_files_to_zip_recursively') - @patch('openedx.features.offline_mode.utils.ZipFile') - @patch('openedx.features.offline_mode.utils.default_storage') - @patch('openedx.features.offline_mode.utils.os') - def test_create_zip_file_os_path_does_not_exists( - self, - os_mock: MagicMock, - default_storage_mock: MagicMock, - zip_file_context_manager: MagicMock, - add_files_to_zip_recursively_mock: MagicMock, - log_info_mock: MagicMock, - ) -> None: - temp_dir_mock = 'temp_dir_mock' - base_path_mock = 'base_path_mock' - file_name_mock = 'file_name_mock' - os_mock.path.exists.return_value = False - - create_zip_file(temp_dir_mock, base_path_mock, file_name_mock) - - os_mock.path.exists.assert_called_once_with(default_storage_mock.path.return_value) - os_mock.makedirs.assert_called_once_with(default_storage_mock.path.return_value) - self.assertListEqual( - default_storage_mock.path.call_args_list, - [call(base_path_mock), call(base_path_mock), call(base_path_mock + file_name_mock)] - ) - zip_file_context_manager.assert_called_once_with(default_storage_mock.path.return_value, 'w') - zip_file_context_manager.return_value.__enter__.return_value.write.assert_called_once_with( - os_mock.path.join.return_value, 'index.html' - ) - add_files_to_zip_recursively_mock.assert_called_once_with( - zip_file_context_manager.return_value.__enter__.return_value, - current_base_path=os_mock.path.join.return_value, - current_path_in_zip='assets', - ) - log_info_mock.assert_called_once_with( - f'Offline content for {file_name_mock} has been generated.' - ) - - @patch('openedx.features.offline_mode.utils.os') - def test_add_files_to_zip_recursively_successfully_for_file( - self, - os_mock: MagicMock, - ): - zip_file_mock = Mock() - current_base_path_mock = 'current_base_path_mock' - current_path_in_zip_mock = 'current_path_in_zip_mock' - resource_path_mock = 'resource_path_mock' - os_mock.listdir.return_value = [resource_path_mock] - - expected_os_mock_path_join_calls = [ - call(current_base_path_mock, resource_path_mock), - call(current_path_in_zip_mock, resource_path_mock) - ] - - add_files_to_zip_recursively(zip_file_mock, current_base_path_mock, current_path_in_zip_mock) - - os_mock.listdir.assert_called_once_with(current_base_path_mock) - self.assertListEqual(os_mock.path.join.call_args_list, expected_os_mock_path_join_calls) - zip_file_mock.write.assert_called_once_with(os_mock.path.join.return_value, os_mock.path.join.return_value) - - @patch('openedx.features.offline_mode.utils.add_files_to_zip_recursively') - @patch('openedx.features.offline_mode.utils.os') - def test_add_files_to_zip_recursively_successfully_recursively_path( - self, - os_mock: MagicMock, - add_files_to_zip_recursively_mock: MagicMock, - ): - zip_file_mock = Mock() - current_base_path_mock = 'current_base_path_mock' - current_path_in_zip_mock = 'current_path_in_zip_mock' - resource_path_mock = 'resource_path_mock' - os_mock.listdir.return_value = [resource_path_mock] - os_mock.path.isfile.return_value = False - - expected_os_mock_path_join_calls = [ - call(current_base_path_mock, resource_path_mock), - call(current_path_in_zip_mock, resource_path_mock) - ] - - add_files_to_zip_recursively(zip_file_mock, current_base_path_mock, current_path_in_zip_mock) - - os_mock.listdir.assert_called_once_with(current_base_path_mock) - self.assertListEqual(os_mock.path.join.call_args_list, expected_os_mock_path_join_calls) - add_files_to_zip_recursively_mock.assert_called_once_with( - zip_file_mock, os_mock.path.join.return_value, os_mock.path.join.return_value - ) - - @patch('openedx.features.offline_mode.utils.log.error') - @patch('openedx.features.offline_mode.utils.os.listdir', side_effect=OSError) - def test_add_files_to_zip_recursively_with_os_error( - self, - os_mock: MagicMock, - log_error_mock: MagicMock, - ): - zip_file_mock = Mock() - current_base_path_mock = 'current_base_path_mock' - current_path_in_zip_mock = 'current_path_in_zip_mock' - - add_files_to_zip_recursively(zip_file_mock, current_base_path_mock, current_path_in_zip_mock) - - os_mock.assert_called_once_with(current_base_path_mock) - log_error_mock.assert_called_once_with(f'Error while reading the directory: {current_base_path_mock}') From 8cb8daa8433a8d95803ff5913d11c7d60d3c6413 Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Fri, 21 Jun 2024 12:46:56 +0300 Subject: [PATCH 8/9] fix: [AXM-653] Commit forgotten base file --- openedx/features/offline_mode/tests/base.py | 47 +++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 openedx/features/offline_mode/tests/base.py diff --git a/openedx/features/offline_mode/tests/base.py b/openedx/features/offline_mode/tests/base.py new file mode 100644 index 000000000000..f8692400f267 --- /dev/null +++ b/openedx/features/offline_mode/tests/base.py @@ -0,0 +1,47 @@ +""" +Tests for the testing xBlock renderers for Offline Mode. +""" + +from xmodule.capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory + + +class CourseForOfflineTestCase(ModuleStoreTestCase): + """ + Base class for creation course for Offline Mode testing. + """ + + def setUp(self): + super().setUp() + default_store = self.store.default_modulestore.get_modulestore_type() + with self.store.default_store(default_store): + self.course = CourseFactory.create( # lint-amnesty, pylint: disable=attribute-defined-outside-init + display_name='Offline Course', + org='RaccoonGang', + number='1', + run='2024', + ) + chapter = BlockFactory.create(parent=self.course, category='chapter') + problem_xml = MultipleChoiceResponseXMLFactory().build_xml( + question_text='The correct answer is Choice 2', + choices=[False, False, True, False], + choice_names=['choice_0', 'choice_1', 'choice_2', 'choice_3'] + ) + self.vertical_block = BlockFactory.create( # lint-amnesty, pylint: disable=attribute-defined-outside-init + parent_location=chapter.location, + category='vertical', + display_name='Vertical' + ) + self.html_block = BlockFactory.create( # lint-amnesty, pylint: disable=attribute-defined-outside-init + parent=self.vertical_block, + category='html', + display_name='HTML xblock for Offline', + data='

Test HTML Content

' + ) + self.problem_block = BlockFactory.create( # lint-amnesty, pylint: disable=attribute-defined-outside-init + parent=self.vertical_block, + category='problem', + display_name='Problem xblock for Offline', + data=problem_xml + ) From fc5be32f05086c287dbfa598ef6b7201e14d5586 Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Sun, 23 Jun 2024 23:40:51 +0300 Subject: [PATCH 9/9] test: [AXM-653] Fix old tests, add new tests --- .../tests/test_course_info_views.py | 48 ++++++--- .../tests/test_html_manipulator.py | 47 ++++++++- .../tests/test_storage_management.py | 98 ++++++------------- .../features/offline_mode/tests/test_tasks.py | 59 +++++------ 4 files changed, 145 insertions(+), 107 deletions(-) diff --git a/lms/djangoapps/mobile_api/tests/test_course_info_views.py b/lms/djangoapps/mobile_api/tests/test_course_info_views.py index 9d4c24f1bc18..091ce38561fb 100644 --- a/lms/djangoapps/mobile_api/tests/test_course_info_views.py +++ b/lms/djangoapps/mobile_api/tests/test_course_info_views.py @@ -17,11 +17,13 @@ from common.djangoapps.student.tests.factories import UserFactory # pylint: disable=unused-import from common.djangoapps.util.course import get_link_for_about_page from lms.djangoapps.mobile_api.testutils import MobileAPITestCase, MobileAuthTestMixin, MobileCourseAccessTestMixin -from lms.djangoapps.mobile_api.utils import API_V1, API_V05 +from lms.djangoapps.mobile_api.utils import API_V05, API_V1, API_V2, API_V3, API_V4 from lms.djangoapps.mobile_api.course_info.views import BlocksInfoInCourseView from lms.djangoapps.course_api.blocks.tests.test_views import TestBlocksInCourseView from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.features.course_experience import ENABLE_COURSE_GOALS +from openedx.features.offline_mode.constants import DEFAULT_OFFLINE_SUPPORTED_XBLOCKS +from openedx.features.offline_mode.toggles import ENABLE_OFFLINE_MODE from xmodule.html_block import CourseInfoBlock # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order @@ -460,12 +462,12 @@ def test_extend_block_info_with_offline_data( get_offline_block_content_path_mock: MagicMock, default_storage_mock: MagicMock, ) -> None: - url = reverse('blocks_info_in_course', kwargs={'api_version': 'v4'}) + url = reverse('blocks_info_in_course', kwargs={'api_version': API_V4}) offline_content_path_mock = '/offline_content_path_mock/' created_time_mock = 'created_time_mock' size_mock = 'size_mock' get_offline_block_content_path_mock.return_value = offline_content_path_mock - default_storage_mock.get_created_time.return_value = created_time_mock + default_storage_mock.get_modified_time.return_value = created_time_mock default_storage_mock.size.return_value = size_mock expected_offline_download_data = { @@ -483,14 +485,14 @@ def test_extend_block_info_with_offline_data( @patch('lms.djangoapps.mobile_api.course_info.views.is_offline_mode_enabled') @ddt.data( - ('v0.5', True), - ('v0.5', False), - ('v1', True), - ('v1', False), - ('v2', True), - ('v2', False), - ('v3', True), - ('v3', False), + (API_V05, True), + (API_V05, False), + (API_V1, True), + (API_V1, False), + (API_V2, True), + (API_V2, False), + (API_V3, True), + (API_V3, False), ) @ddt.unpack def test_not_extend_block_info_with_offline_data_for_version_less_v4_and_any_waffle_flag( @@ -507,3 +509,27 @@ def test_not_extend_block_info_with_offline_data_for_version_less_v4_and_any_waf self.assertEqual(response.status_code, status.HTTP_200_OK) for block_info in response.data['blocks'].values(): self.assertNotIn('offline_download', block_info) + + @override_waffle_flag(ENABLE_OFFLINE_MODE, active=True) + @patch('openedx.features.offline_mode.html_manipulator.save_mathjax_to_xblock_assets') + def test_create_offline_content_integration_test(self, save_mathjax_to_xblock_assets_mock: MagicMock) -> None: + UserFactory.create(username='offline_mode_worker', password='password', is_staff=True) + handle_course_published_url = reverse('offline_mode:handle_course_published') + self.client.login(username='offline_mode_worker', password='password') + + handler_response = self.client.post(handle_course_published_url, {'course_id': str(self.course.id)}) + self.assertEqual(handler_response.status_code, status.HTTP_200_OK) + + url = reverse('blocks_info_in_course', kwargs={'api_version': API_V4}) + + response = self.verify_response(url=url) + self.assertEqual(response.status_code, status.HTTP_200_OK) + for block_info in response.data['blocks'].values(): + if block_type := block_info.get('type'): + if block_type in DEFAULT_OFFLINE_SUPPORTED_XBLOCKS: + expected_offline_content_url = f'/uploads/{self.course.id}/{block_info["block_id"]}.zip' + self.assertIn('offline_download', block_info) + self.assertIn('file_url', block_info['offline_download']) + self.assertIn('last_modified', block_info['offline_download']) + self.assertIn('file_size', block_info['offline_download']) + self.assertEqual(expected_offline_content_url, block_info['offline_download']['file_url']) diff --git a/openedx/features/offline_mode/tests/test_html_manipulator.py b/openedx/features/offline_mode/tests/test_html_manipulator.py index a3e8c776ff32..cf280900177e 100644 --- a/openedx/features/offline_mode/tests/test_html_manipulator.py +++ b/openedx/features/offline_mode/tests/test_html_manipulator.py @@ -4,7 +4,7 @@ from bs4 import BeautifulSoup from unittest import TestCase -from unittest.mock import MagicMock, Mock, patch +from unittest.mock import MagicMock, Mock, call, patch from openedx.features.offline_mode.constants import MATHJAX_CDN_URL, MATHJAX_STATIC_PATH from openedx.features.offline_mode.html_manipulator import HtmlManipulator @@ -17,6 +17,8 @@ class HtmlManipulatorTestCase(TestCase): @patch('openedx.features.offline_mode.html_manipulator.HtmlManipulator._replace_iframe') @patch('openedx.features.offline_mode.html_manipulator.BeautifulSoup', return_value='soup_mock') + @patch('openedx.features.offline_mode.html_manipulator.HtmlManipulator._copy_platform_fonts') + @patch('openedx.features.offline_mode.html_manipulator.HtmlManipulator._replace_external_links') @patch('openedx.features.offline_mode.html_manipulator.HtmlManipulator._replace_mathjax_link') @patch('openedx.features.offline_mode.html_manipulator.HtmlManipulator._replace_static_links') @patch('openedx.features.offline_mode.html_manipulator.HtmlManipulator._replace_asset_links') @@ -25,6 +27,8 @@ def test_process_html( replace_asset_links_mock: MagicMock, replace_static_links_mock: MagicMock, replace_mathjax_link_mock: MagicMock, + replace_external_links: MagicMock, + copy_platform_fonts: MagicMock, beautiful_soup_mock: MagicMock, replace_iframe_mock: MagicMock, ) -> None: @@ -39,6 +43,8 @@ def test_process_html( replace_asset_links_mock.assert_called_once_with() replace_static_links_mock.assert_called_once_with() replace_mathjax_link_mock.assert_called_once_with() + replace_external_links.assert_called_once_with() + copy_platform_fonts.assert_called_once_with() beautiful_soup_mock.assert_called_once_with(html_manipulator.html_data, 'html.parser') replace_iframe_mock.assert_called_once_with(beautiful_soup_mock.return_value) self.assertEqual(result, expected_result) @@ -116,3 +122,42 @@ def test_replace_iframe(self): HtmlManipulator._replace_iframe(soup) # lint-amnesty, pylint: disable=protected-access self.assertEqual(f'{soup.find_all("p")[0]}', expected_html_markup) + + @patch('openedx.features.offline_mode.html_manipulator.save_external_file') + def test_replace_external_links(self, save_external_file_mock: MagicMock) -> None: + xblock_mock = Mock() + temp_dir_mock = 'temp_dir_mock' + html_data_mock = """ + Example Image + + """ + + html_manipulator = HtmlManipulator(xblock_mock, html_data_mock, temp_dir_mock) + html_manipulator._replace_external_links() # lint-amnesty, pylint: disable=protected-access + + self.assertEqual(save_external_file_mock.call_count, 2) + + @patch('openedx.features.offline_mode.html_manipulator.uuid.uuid4') + @patch('openedx.features.offline_mode.html_manipulator.save_external_file') + def test_replace_external_link( + self, + save_external_file_mock: MagicMock, + uuid_mock: MagicMock, + ) -> None: + xblock_mock = Mock() + temp_dir_mock = 'temp_dir_mock' + html_data_mock = 'html_data_mock' + external_url_mock = 'https://cdn.example.com/image.jpg' + uuid_result_mock = '123e4567-e89b-12d3-a456-426655440000' + uuid_mock.return_value = uuid_result_mock + mock_match = MagicMock() + mock_match.group.side_effect = [external_url_mock, 'jpg'] + + expected_result = 'assets/external/123e4567-e89b-12d3-a456-426655440000.jpg' + expected_save_external_file_args = [call(temp_dir_mock, external_url_mock, expected_result)] + + html_manipulator = HtmlManipulator(xblock_mock, html_data_mock, temp_dir_mock) + result = html_manipulator._replace_external_link(mock_match) # lint-amnesty, pylint: disable=protected-access + + self.assertListEqual(save_external_file_mock.call_args_list, expected_save_external_file_args) + self.assertEqual(result, expected_result) diff --git a/openedx/features/offline_mode/tests/test_storage_management.py b/openedx/features/offline_mode/tests/test_storage_management.py index c226c20ac1f8..f77f2ba614cd 100644 --- a/openedx/features/offline_mode/tests/test_storage_management.py +++ b/openedx/features/offline_mode/tests/test_storage_management.py @@ -18,91 +18,62 @@ class OfflineContentGeneratorTestCase(TestCase): """ Test case for the testing Offline Mode utils. """ - - @patch('openedx.features.offline_mode.storage_management.shutil.rmtree') - @patch('openedx.features.offline_mode.storage_management.OfflineContentGenerator.create_zip_file') - @patch('openedx.features.offline_mode.storage_management.OfflineContentGenerator.save_xblock_html') - @patch('openedx.features.offline_mode.storage_management.mkdtemp') - @patch('openedx.features.offline_mode.storage_management.clean_outdated_xblock_files') - @patch('openedx.features.offline_mode.storage_management.block_storage_path') - @patch('openedx.features.offline_mode.storage_management.is_modified', return_value=True) - def test_generate_offline_content_for_modified_xblock( - self, - is_modified_mock: MagicMock, - block_storage_path_mock: MagicMock, - clean_outdated_xblock_files_mock: MagicMock, - mkdtemp_mock: MagicMock, - save_xblock_html_mock: MagicMock, - create_zip_file_mock: MagicMock, - shutil_rmtree_mock: MagicMock, - ) -> None: + @patch('openedx.features.offline_mode.storage_management.XBlockRenderer') + def test_render_block_html_data_successful(self, xblock_renderer_mock: MagicMock) -> None: xblock_mock = Mock() html_data_mock = 'html_markup_data_mock' - OfflineContentGenerator(xblock_mock, html_data_mock).generate_offline_content() + result = OfflineContentGenerator(xblock_mock, html_data_mock).render_block_html_data() - is_modified_mock.assert_called_once_with(xblock_mock) - block_storage_path_mock.assert_called_once_with(xblock_mock) - clean_outdated_xblock_files_mock.assert_called_once_with(xblock_mock) - mkdtemp_mock.assert_called_once_with() - save_xblock_html_mock.assert_called_once_with(mkdtemp_mock.return_value) - create_zip_file_mock.assert_called_once_with( - mkdtemp_mock.return_value, - block_storage_path_mock.return_value, - f'{xblock_mock.location.block_id}.zip' + xblock_renderer_mock.assert_called_once_with(str(xblock_mock.location)) + xblock_renderer_mock.return_value.render_xblock_from_lms.assert_called_once_with() + self.assertEqual(result, xblock_renderer_mock.return_value.render_xblock_from_lms.return_value) + + @patch('openedx.features.offline_mode.storage_management.XBlockRenderer') + def test_render_block_html_data_successful_no_html_data(self, xblock_renderer_mock: MagicMock) -> None: + xblock_mock = Mock() + expected_xblock_renderer_args_list = [call(str(xblock_mock.location)), call(str(xblock_mock.location))] + + result = OfflineContentGenerator(xblock_mock).render_block_html_data() + + self.assertListEqual(xblock_renderer_mock.call_args_list, expected_xblock_renderer_args_list) + self.assertListEqual( + xblock_renderer_mock.return_value.render_xblock_from_lms.call_args_list, [call(), call()] ) - shutil_rmtree_mock.assert_called_once_with(mkdtemp_mock.return_value, ignore_errors=True) + self.assertEqual(result, xblock_renderer_mock.return_value.render_xblock_from_lms.return_value) - @patch('openedx.features.offline_mode.storage_management.shutil.rmtree') - @patch('openedx.features.offline_mode.storage_management.OfflineContentGenerator.create_zip_file') - @patch('openedx.features.offline_mode.storage_management.OfflineContentGenerator.save_xblock_html') - @patch('openedx.features.offline_mode.storage_management.mkdtemp') - @patch('openedx.features.offline_mode.storage_management.clean_outdated_xblock_files') - @patch('openedx.features.offline_mode.storage_management.block_storage_path') - @patch('openedx.features.offline_mode.storage_management.is_modified', return_value=False) - def test_generate_offline_content_is_not_modified( + @patch('openedx.features.offline_mode.storage_management.log.error') + @patch('openedx.features.offline_mode.storage_management.XBlockRenderer', side_effect=Http404) + def test_render_block_html_data_http404( self, - is_modified_mock: MagicMock, - block_storage_path_mock: MagicMock, - clean_outdated_xblock_files_mock: MagicMock, - mkdtemp_mock: MagicMock, - save_xblock_html_mock: MagicMock, - create_zip_file_mock: MagicMock, - shutil_rmtree_mock: MagicMock, + xblock_renderer_mock: MagicMock, + logger_mock: MagicMock, ) -> None: xblock_mock = Mock() html_data_mock = 'html_markup_data_mock' - OfflineContentGenerator(xblock_mock, html_data_mock).generate_offline_content() + with self.assertRaises(Http404): + OfflineContentGenerator(xblock_mock, html_data_mock).render_block_html_data() - is_modified_mock.assert_called_once_with(xblock_mock) - block_storage_path_mock.assert_not_called() - clean_outdated_xblock_files_mock.assert_not_called() - mkdtemp_mock.assert_not_called() - save_xblock_html_mock.assert_not_called() - create_zip_file_mock.assert_not_called() - shutil_rmtree_mock.assert_not_called() + xblock_renderer_mock.assert_called_once_with(str(xblock_mock.location)) + logger_mock.assert_called_once_with( + f'Block {str(xblock_mock.location)} cannot be fetched from course' + f' {xblock_mock.location.course_key} during offline content generation.' + ) @patch('openedx.features.offline_mode.storage_management.shutil.rmtree') - @patch('openedx.features.offline_mode.storage_management.log.error') - @patch( - 'openedx.features.offline_mode.storage_management.OfflineContentGenerator.create_zip_file', - side_effect=Http404, - ) + @patch('openedx.features.offline_mode.storage_management.OfflineContentGenerator.create_zip_file') @patch('openedx.features.offline_mode.storage_management.OfflineContentGenerator.save_xblock_html') @patch('openedx.features.offline_mode.storage_management.mkdtemp') @patch('openedx.features.offline_mode.storage_management.clean_outdated_xblock_files') @patch('openedx.features.offline_mode.storage_management.block_storage_path') - @patch('openedx.features.offline_mode.storage_management.is_modified', return_value=True) - def test_generate_offline_content_is_not_modified_with_http_404_status( + def test_generate_offline_content_for_modified_xblock( self, - is_modified_mock: MagicMock, block_storage_path_mock: MagicMock, clean_outdated_xblock_files_mock: MagicMock, mkdtemp_mock: MagicMock, save_xblock_html_mock: MagicMock, create_zip_file_mock: MagicMock, - logger_mock: MagicMock, shutil_rmtree_mock: MagicMock, ) -> None: xblock_mock = Mock() @@ -110,7 +81,6 @@ def test_generate_offline_content_is_not_modified_with_http_404_status( OfflineContentGenerator(xblock_mock, html_data_mock).generate_offline_content() - is_modified_mock.assert_called_once_with(xblock_mock) block_storage_path_mock.assert_called_once_with(xblock_mock) clean_outdated_xblock_files_mock.assert_called_once_with(xblock_mock) mkdtemp_mock.assert_called_once_with() @@ -120,10 +90,6 @@ def test_generate_offline_content_is_not_modified_with_http_404_status( block_storage_path_mock.return_value, f'{xblock_mock.location.block_id}.zip' ) - logger_mock.assert_called_once_with( - f'Block {xblock_mock.location.block_id} cannot be fetched from course' - f' {xblock_mock.location.course_key} during offline content generation.' - ) shutil_rmtree_mock.assert_called_once_with(mkdtemp_mock.return_value, ignore_errors=True) @patch('openedx.features.offline_mode.storage_management.os.path.join') diff --git a/openedx/features/offline_mode/tests/test_tasks.py b/openedx/features/offline_mode/tests/test_tasks.py index 75512d8e8b96..ef468d825d7b 100644 --- a/openedx/features/offline_mode/tests/test_tasks.py +++ b/openedx/features/offline_mode/tests/test_tasks.py @@ -5,15 +5,17 @@ from unittest import TestCase from unittest.mock import MagicMock, Mock, call, patch +from ddt import data, ddt, unpack +from django.http.response import Http404 from opaque_keys.edx.keys import CourseKey, UsageKey from openedx.features.offline_mode.constants import MAX_RETRY_ATTEMPTS, OFFLINE_SUPPORTED_XBLOCKS from openedx.features.offline_mode.tasks import ( generate_offline_content_for_block, generate_offline_content_for_course, ) -from xmodule.modulestore.exceptions import ItemNotFoundError +@ddt class GenerateOfflineContentTasksTestCase(TestCase): """ Test case for the testing generating offline content tacks. @@ -27,33 +29,29 @@ def test_generate_offline_content_for_block_success( offline_content_generator_mock: MagicMock, ) -> None: block_id_mock = 'block-v1:a+a+a+type@problem+block@fb81e4dbfd4945cb9318d6bc460a956c' - html_data_mock = 'html_markup_data_mock' - generate_offline_content_for_block(block_id_mock, html_data_mock) + generate_offline_content_for_block(block_id_mock) modulestore_mock.assert_called_once_with() modulestore_mock.return_value.get_item.assert_called_once_with(UsageKey.from_string(block_id_mock)) - offline_content_generator_mock.assert_called_once_with( - modulestore_mock.return_value.get_item.return_value, html_data_mock - ) + offline_content_generator_mock.assert_called_once_with(modulestore_mock.return_value.get_item.return_value) offline_content_generator_mock.return_value.generate_offline_content.assert_called_once_with() @patch('openedx.features.offline_mode.tasks.OfflineContentGenerator') - @patch('openedx.features.offline_mode.tasks.modulestore', side_effect=ItemNotFoundError) + @patch('openedx.features.offline_mode.tasks.modulestore', side_effect=Http404) def test_generate_offline_content_for_block_with_exception_in_modulestore( self, modulestore_mock: MagicMock, offline_content_generator_mock: MagicMock, ) -> None: block_id_mock = 'block-v1:a+a+a+type@problem+block@fb81e4dbfd4945cb9318d6bc460a956c' - html_data_mock = 'html_markup_data_mock' - generate_offline_content_for_block.delay(block_id_mock, html_data_mock) + generate_offline_content_for_block.delay(block_id_mock) self.assertEqual(modulestore_mock.call_count, MAX_RETRY_ATTEMPTS + 1) offline_content_generator_mock.assert_not_called() - @patch('openedx.features.offline_mode.tasks.OfflineContentGenerator', side_effect=FileNotFoundError) + @patch('openedx.features.offline_mode.tasks.OfflineContentGenerator', side_effect=Http404) @patch('openedx.features.offline_mode.tasks.modulestore') def test_generate_offline_content_for_block_with_exception_in_offline_content_generation( self, @@ -61,38 +59,36 @@ def test_generate_offline_content_for_block_with_exception_in_offline_content_ge offline_content_generator_mock: MagicMock, ) -> None: block_id_mock = 'block-v1:a+a+a+type@problem+block@fb81e4dbfd4945cb9318d6bc460a956c' - html_data_mock = 'html_markup_data_mock' - generate_offline_content_for_block.delay(block_id_mock, html_data_mock) + generate_offline_content_for_block.delay(block_id_mock) self.assertEqual(modulestore_mock.call_count, MAX_RETRY_ATTEMPTS + 1) self.assertEqual(offline_content_generator_mock.call_count, MAX_RETRY_ATTEMPTS + 1) @patch('openedx.features.offline_mode.tasks.generate_offline_content_for_block') - @patch('openedx.features.offline_mode.tasks.XBlockRenderer') + @patch('openedx.features.offline_mode.tasks.is_modified') @patch('openedx.features.offline_mode.tasks.modulestore') def test_generate_offline_content_for_course_supported_block_types( self, modulestore_mock: MagicMock, - xblock_renderer_mock: MagicMock, + is_modified_mock: MagicMock, generate_offline_content_for_block_mock: MagicMock, ) -> None: course_id_mock = 'course-v1:a+a+a' xblock_location_mock = 'xblock_location_mock' - html_data_mock = 'html_markup_data_mock' modulestore_mock.return_value.get_items.return_value = [ Mock(location=xblock_location_mock, closed=Mock(return_value=False)) ] - xblock_renderer_mock.return_value.render_xblock_from_lms.return_value = html_data_mock + expected_call_args_for_modulestore_get_items = [ call(CourseKey.from_string(course_id_mock), qualifiers={'category': offline_supported_block_type}) for offline_supported_block_type in OFFLINE_SUPPORTED_XBLOCKS ] - expected_call_args_for_xblock_renderer_mock = [ - call(xblock_location_mock) for _ in OFFLINE_SUPPORTED_XBLOCKS + expected_call_args_is_modified_mock = [ + call(modulestore_mock.return_value.get_items.return_value[0]) for _ in OFFLINE_SUPPORTED_XBLOCKS ] expected_call_args_for_generate_offline_content_for_block_mock = [ - call([xblock_location_mock, html_data_mock]) for _ in OFFLINE_SUPPORTED_XBLOCKS + call([xblock_location_mock]) for _ in OFFLINE_SUPPORTED_XBLOCKS ] generate_offline_content_for_course(course_id_mock) @@ -101,30 +97,36 @@ def test_generate_offline_content_for_course_supported_block_types( self.assertListEqual( modulestore_mock.return_value.get_items.call_args_list, expected_call_args_for_modulestore_get_items ) - self.assertListEqual( - xblock_renderer_mock.call_args_list, expected_call_args_for_xblock_renderer_mock - ) + self.assertListEqual(is_modified_mock.call_args_list, expected_call_args_is_modified_mock) self.assertListEqual( generate_offline_content_for_block_mock.apply_async.call_args_list, expected_call_args_for_generate_offline_content_for_block_mock ) @patch('openedx.features.offline_mode.tasks.generate_offline_content_for_block') - @patch('openedx.features.offline_mode.tasks.XBlockRenderer') + @patch('openedx.features.offline_mode.tasks.is_modified') @patch('openedx.features.offline_mode.tasks.modulestore') - def test_generate_offline_content_for_course_supported_block_types_closed_xblock( + @data( + (False, False), + (True, False), + (False, True), + ) + @unpack + def test_generate_offline_content_for_course_supported_block_types_for_closed_or_not_modified_xblock( self, + is_modified_value_mock: bool, + is_closed_value_mock: bool, modulestore_mock: MagicMock, - xblock_renderer_mock: MagicMock, + is_modified_mock: MagicMock, generate_offline_content_for_block_mock: MagicMock, ) -> None: course_id_mock = 'course-v1:a+a+a' xblock_location_mock = 'xblock_location_mock' - html_data_mock = 'html_markup_data_mock' modulestore_mock.return_value.get_items.return_value = [ - Mock(location=xblock_location_mock, closed=Mock(return_value=True)) + Mock(location=xblock_location_mock, closed=Mock(return_value=is_closed_value_mock)) ] - xblock_renderer_mock.return_value.render_xblock_from_lms.return_value = html_data_mock + is_modified_mock.return_value = is_modified_value_mock + expected_call_args_for_modulestore_get_items = [ call(CourseKey.from_string(course_id_mock), qualifiers={'category': offline_supported_block_type}) for offline_supported_block_type in OFFLINE_SUPPORTED_XBLOCKS @@ -136,5 +138,4 @@ def test_generate_offline_content_for_course_supported_block_types_closed_xblock self.assertListEqual( modulestore_mock.return_value.get_items.call_args_list, expected_call_args_for_modulestore_get_items ) - xblock_renderer_mock.assert_not_called() generate_offline_content_for_block_mock.assert_not_called()