diff --git a/openlp/core/common/path.py b/openlp/core/common/path.py index 05d2ea4eb..0e4b45c2a 100644 --- a/openlp/core/common/path.py +++ b/openlp/core/common/path.py @@ -69,8 +69,15 @@ class Path(PathVariant): path = path.relative_to(base_path) return {'__Path__': path.parts} - def rmtree(self, *args, **kwargs): - shutil.rmtree(str(self), *args, **kwargs) + def rmtree(self, ignore_errors=False, onerror=None): + """ + Provide an interface to :func:`shutil.rmtree` + + :param bool ignore_errors: Ignore errors + :param onerror: Handler function to handle any errors + :rtype: None + """ + shutil.rmtree(str(self), ignore_errors, onerror) def replace_params(args, kwargs, params): @@ -156,23 +163,6 @@ def copytree(*args, **kwargs): return str_to_path(shutil.copytree(*args, **kwargs)) -def rmtree(*args, **kwargs): - """ - Wraps :func:shutil.rmtree` so that we can accept Path objects. - - :param openlp.core.common.path.Path path: Takes a Path object which is then converted to a str object - :return: Passes the return from :func:`shutil.rmtree` back - :rtype: None - - See the following link for more information on the other parameters: - https://docs.python.org/3/library/shutil.html#shutil.rmtree - """ - - args, kwargs = replace_params(args, kwargs, ((0, 'path', path_to_str),)) - - return shutil.rmtree(*args, **kwargs) - - def which(*args, **kwargs): """ Wraps :func:shutil.which` so that it return a Path objects. diff --git a/openlp/core/lib/mediamanageritem.py b/openlp/core/lib/mediamanageritem.py index 8091b1a2e..55306267c 100644 --- a/openlp/core/lib/mediamanageritem.py +++ b/openlp/core/lib/mediamanageritem.py @@ -333,7 +333,7 @@ class MediaManagerItem(QtWidgets.QWidget, RegistryProperties): new_file_paths = [] error_shown = False for file_name in data['files']: - file_path = str_to_path(file_name) # TODO: + file_path = str_to_path(file_name) if file_path.suffix[1:].lower() not in self.on_new_file_masks: if not error_shown: critical_error_message_box( @@ -367,7 +367,7 @@ class MediaManagerItem(QtWidgets.QWidget, RegistryProperties): """ full_list = [] for count in range(self.list_view.count()): - full_list.append(self.list_view.item(count).data(QtCore.Qt.UserRole)) # TODO: Path objects + full_list.append(self.list_view.item(count).data(QtCore.Qt.UserRole)) duplicates_found = False files_added = False for file_path in file_paths: diff --git a/openlp/core/ui/thememanager.py b/openlp/core/ui/thememanager.py index fa8a9e4d9..fae75f81d 100644 --- a/openlp/core/ui/thememanager.py +++ b/openlp/core/ui/thememanager.py @@ -32,7 +32,7 @@ from openlp.core.common import delete_file from openlp.core.common.applocation import AppLocation from openlp.core.common.i18n import UiStrings, translate, get_locale_key from openlp.core.common.mixins import LogMixin, RegistryProperties -from openlp.core.common.path import Path, copyfile, create_paths, path_to_str, rmtree +from openlp.core.common.path import Path, copyfile, create_paths, path_to_str from openlp.core.common.registry import Registry, RegistryBase from openlp.core.common.settings import Settings from openlp.core.lib import ImageSource, ValidationError, get_text_file_string, build_icon, \ @@ -376,7 +376,7 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R delete_file(self.theme_path / thumb) delete_file(self.thumb_path / thumb) try: - rmtree(self.theme_path / theme) + (self.theme_path / theme).rmtree() except OSError: self.log_exception('Error deleting theme {name}'.format(name=theme)) @@ -431,7 +431,7 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R 'The theme_name export failed because this error occurred: {err}') .format(err=ose.strerror)) if theme_path.exists(): - rmtree(theme_path, True) + theme_path.rmtree(ignore_errors=True) return False def on_import_theme(self, checked=None): @@ -710,7 +710,7 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R Call the renderer to build a Sample Image :param theme_data: The theme to generated a preview for. - :param force_page: Flag to tell message lines per page need to be generated.7 + :param force_page: Flag to tell message lines per page need to be generated. :rtype: QtGui.QPixmap """ return self.renderer.generate_preview(theme_data, force_page) diff --git a/openlp/plugins/presentations/lib/presentationcontroller.py b/openlp/plugins/presentations/lib/presentationcontroller.py index db5786626..b54d8afa9 100644 --- a/openlp/plugins/presentations/lib/presentationcontroller.py +++ b/openlp/plugins/presentations/lib/presentationcontroller.py @@ -25,7 +25,7 @@ from PyQt5 import QtCore from openlp.core.common import md5_hash from openlp.core.common.applocation import AppLocation -from openlp.core.common.path import Path, create_paths, rmtree +from openlp.core.common.path import Path, create_paths from openlp.core.common.registry import Registry from openlp.core.common.settings import Settings from openlp.core.lib import create_thumb, validate_thumb @@ -126,9 +126,9 @@ class PresentationDocument(object): thumbnail_folder_path = self.get_thumbnail_folder() temp_folder_path = self.get_temp_folder() if thumbnail_folder_path.exists(): - rmtree(thumbnail_folder_path) + thumbnail_folder_path.rmtree() if temp_folder_path.exists(): - rmtree(temp_folder_path) + temp_folder_path.rmtree() except OSError: log.exception('Failed to delete presentation controller files') diff --git a/tests/functional/openlp_core/common/test_path.py b/tests/functional/openlp_core/common/test_path.py index 2ec89771b..498b7aaa0 100644 --- a/tests/functional/openlp_core/common/test_path.py +++ b/tests/functional/openlp_core/common/test_path.py @@ -26,7 +26,7 @@ import os from unittest import TestCase from unittest.mock import ANY, MagicMock, patch -from openlp.core.common.path import Path, copy, copyfile, copytree, create_paths, path_to_str, replace_params, rmtree, \ +from openlp.core.common.path import Path, copy, copyfile, copytree, create_paths, path_to_str, replace_params, \ str_to_path, which @@ -172,31 +172,35 @@ class TestShutil(TestCase): """ Test :func:`rmtree` """ - # GIVEN: A mocked :func:`shutil.rmtree` + # GIVEN: A mocked :func:`shutil.rmtree` and a test Path object with patch('openlp.core.common.path.shutil.rmtree', return_value=None) as mocked_shutil_rmtree: + path = Path('test', 'path') # WHEN: Calling :func:`openlp.core.common.path.rmtree` with the path parameter as Path object type - result = rmtree(Path('test', 'path')) + result = path.rmtree() # THEN: :func:`shutil.rmtree` should have been called with the str equivalents of the Path object. - mocked_shutil_rmtree.assert_called_once_with(os.path.join('test', 'path')) + mocked_shutil_rmtree.assert_called_once_with( + os.path.join('test', 'path'), False, None) self.assertIsNone(result) def test_rmtree_optional_params(self): """ Test :func:`openlp.core.common.path.rmtree` when optional parameters are passed """ - # GIVEN: A mocked :func:`shutil.rmtree` - with patch('openlp.core.common.path.shutil.rmtree', return_value='') as mocked_shutil_rmtree: + # GIVEN: A mocked :func:`shutil.rmtree` and a test Path object. + with patch('openlp.core.common.path.shutil.rmtree', return_value=None) as mocked_shutil_rmtree: + path = Path('test', 'path') mocked_on_error = MagicMock() # WHEN: Calling :func:`openlp.core.common.path.rmtree` with :param:`ignore_errors` set to True and # :param:`onerror` set to a mocked object - rmtree(Path('test', 'path'), ignore_errors=True, onerror=mocked_on_error) + path.rmtree(ignore_errors=True, onerror=mocked_on_error) # THEN: :func:`shutil.rmtree` should have been called with the optional parameters, with out any of the # values being modified - mocked_shutil_rmtree.assert_called_once_with(ANY, ignore_errors=True, onerror=mocked_on_error) + mocked_shutil_rmtree.assert_called_once_with( + os.path.join('test', 'path'), True, mocked_on_error) def test_which_no_command(self): """ diff --git a/tests/functional/openlp_plugins/songs/test_openlyricsexport.py b/tests/functional/openlp_plugins/songs/test_openlyricsexport.py index 0fd4767db..85bf0818c 100644 --- a/tests/functional/openlp_plugins/songs/test_openlyricsexport.py +++ b/tests/functional/openlp_plugins/songs/test_openlyricsexport.py @@ -22,13 +22,12 @@ """ This module contains tests for the OpenLyrics song importer. """ -import shutil from tempfile import mkdtemp from unittest import TestCase from unittest.mock import MagicMock, patch from openlp.core.common.registry import Registry -from openlp.core.common.path import Path, rmtree +from openlp.core.common.path import Path from openlp.plugins.songs.lib.openlyricsexport import OpenLyricsExport from tests.helpers.testmixin import TestMixin @@ -49,7 +48,7 @@ class TestOpenLyricsExport(TestCase, TestMixin): """ Cleanup """ - rmtree(self.temp_folder) + self.temp_folder.rmtree() def test_export_same_filename(self): """