tidyups

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):
+    def rmtree(self, ignore_errors=False, onerror=None):
-        shutil.rmtree(str(self), *args, **kwargs)
+        """
+        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.

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:

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)

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')
 

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`
+        # GIVEN: A mocked :func:`shutil.rmtree` and a test Path object.
-        with patch('openlp.core.common.path.shutil.rmtree', return_value='') as mocked_shutil_rmtree:
+        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):
         """

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):
         """