From 9196db5af0d4c83370b8fcd4f27e39bdf843fa82 Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Sat, 18 Nov 2017 11:23:15 +0000 Subject: [PATCH 1/7] Pathlib refactors and test fixes --- openlp/core/api/deploy.py | 15 ++-- openlp/core/api/endpoint/controller.py | 11 +-- openlp/core/api/endpoint/pluginhelpers.py | 15 ++-- openlp/core/api/http/endpoint.py | 12 +-- openlp/core/api/http/wsgiapp.py | 8 +- openlp/core/common/path.py | 3 + openlp/core/display/renderer.py | 4 +- openlp/core/lib/__init__.py | 9 +- openlp/core/lib/plugin.py | 17 +++- openlp/core/ui/maindisplay.py | 2 + openlp/core/ui/mainwindow.py | 7 +- openlp/core/ui/thememanager.py | 13 +-- openlp/core/version.py | 11 +-- openlp/plugins/images/lib/mediaitem.py | 2 +- openlp/plugins/presentations/lib/mediaitem.py | 4 +- .../lib/presentationcontroller.py | 2 +- .../functional/openlp_core/api/test_deploy.py | 26 +++--- tests/functional/openlp_core/lib/test_lib.py | 86 ++++++++++--------- .../openlp_core/ui/test_thememanager.py | 8 +- .../presentations/test_pdfcontroller.py | 20 ++--- .../openlp_core/lib/test_pluginmanager.py | 7 +- 21 files changed, 143 insertions(+), 139 deletions(-) diff --git a/openlp/core/api/deploy.py b/openlp/core/api/deploy.py index 0419b45db..b64cc40d5 100644 --- a/openlp/core/api/deploy.py +++ b/openlp/core/api/deploy.py @@ -22,7 +22,6 @@ """ Download and "install" the remote web client """ -import os from zipfile import ZipFile from openlp.core.common.applocation import AppLocation @@ -30,18 +29,18 @@ from openlp.core.common.registry import Registry from openlp.core.common.httputils import url_get_file, get_web_page, get_url_file_size -def deploy_zipfile(app_root, zip_name): +def deploy_zipfile(app_root_path, zip_name): """ Process the downloaded zip file and add to the correct directory - :param zip_name: the zip file to be processed - :param app_root: the directory where the zip get expanded to + :param str zip_name: the zip file name to be processed + :param openlp.core.common.path.Path app_root_path: The directory to expand the zip to :return: None """ - zip_file = os.path.join(app_root, zip_name) - web_zip = ZipFile(zip_file) - web_zip.extractall(app_root) + zip_path = app_root_path / zip_name + web_zip = ZipFile(str(zip_path)) + web_zip.extractall(str(app_root_path)) def download_sha256(): @@ -67,4 +66,4 @@ def download_and_check(callback=None): if url_get_file(callback, 'https://get.openlp.org/webclient/site.zip', AppLocation.get_section_data_path('remotes') / 'site.zip', sha256=sha256): - deploy_zipfile(str(AppLocation.get_section_data_path('remotes')), 'site.zip') + deploy_zipfile(AppLocation.get_section_data_path('remotes'), 'site.zip') diff --git a/openlp/core/api/endpoint/controller.py b/openlp/core/api/endpoint/controller.py index 8ecfdb732..13e8d1681 100644 --- a/openlp/core/api/endpoint/controller.py +++ b/openlp/core/api/endpoint/controller.py @@ -28,6 +28,7 @@ import json from openlp.core.api.http.endpoint import Endpoint from openlp.core.api.http import requires_auth from openlp.core.common.applocation import AppLocation +from openlp.core.common.path import Path from openlp.core.common.registry import Registry from openlp.core.common.settings import Settings from openlp.core.lib import ItemCapabilities, create_thumb @@ -66,12 +67,12 @@ def controller_text(request): elif current_item.is_image() and not frame.get('image', '') and Settings().value('api/thumbnails'): item['tag'] = str(index + 1) thumbnail_path = os.path.join('images', 'thumbnails', frame['title']) - full_thumbnail_path = str(AppLocation.get_data_path() / thumbnail_path) + full_thumbnail_path = AppLocation.get_data_path() / thumbnail_path # Create thumbnail if it doesn't exists - if not os.path.exists(full_thumbnail_path): - create_thumb(current_item.get_frame_path(index), full_thumbnail_path, False) - Registry().get('image_manager').add_image(full_thumbnail_path, frame['title'], None, 88, 88) - item['img'] = urllib.request.pathname2url(os.path.sep + thumbnail_path) + if not full_thumbnail_path.exists(): + create_thumb(Path(current_item.get_frame_path(index)), full_thumbnail_path, False) + Registry().get('image_manager').add_image(str(full_thumbnail_path), frame['title'], None, 88, 88) + item['img'] = urllib.request.pathname2url(os.path.sep + str(thumbnail_path)) item['text'] = str(frame['title']) item['html'] = str(frame['title']) else: diff --git a/openlp/core/api/endpoint/pluginhelpers.py b/openlp/core/api/endpoint/pluginhelpers.py index 9377bde6a..b8f606fbc 100644 --- a/openlp/core/api/endpoint/pluginhelpers.py +++ b/openlp/core/api/endpoint/pluginhelpers.py @@ -19,7 +19,6 @@ # with this program; if not, write to the Free Software Foundation, Inc., 59 # # Temple Place, Suite 330, Boston, MA 02111-1307 USA # ############################################################################### -import os import json import re import urllib @@ -103,7 +102,7 @@ def display_thumbnails(request, controller_name, log, dimensions, file_name, sli :param controller_name: which controller is requesting the image :param log: the logger object :param dimensions: the image size eg 88x88 - :param file_name: the file name of the image + :param str file_name: the file name of the image :param slide: the individual image name :return: """ @@ -124,12 +123,10 @@ def display_thumbnails(request, controller_name, log, dimensions, file_name, sli if controller_name and file_name: file_name = urllib.parse.unquote(file_name) if '..' not in file_name: # no hacking please + full_path = AppLocation.get_section_data_path(controller_name) / 'thumbnails' / file_name if slide: - full_path = str(AppLocation.get_section_data_path(controller_name) / 'thumbnails' / file_name / slide) - else: - full_path = str(AppLocation.get_section_data_path(controller_name) / 'thumbnails' / file_name) - if os.path.exists(full_path): - path, just_file_name = os.path.split(full_path) - Registry().get('image_manager').add_image(full_path, just_file_name, None, width, height) - image = Registry().get('image_manager').get_image(full_path, just_file_name, width, height) + full_path = full_path / slide + if full_path.exists(): + Registry().get('image_manager').add_image(full_path, full_path.name, None, width, height) + image = Registry().get('image_manager').get_image(full_path, full_path.name, width, height) return Response(body=image_to_byte(image, False), status=200, content_type='image/png', charset='utf8') diff --git a/openlp/core/api/http/endpoint.py b/openlp/core/api/http/endpoint.py index fe2b11d9a..011b7e73a 100644 --- a/openlp/core/api/http/endpoint.py +++ b/openlp/core/api/http/endpoint.py @@ -22,8 +22,6 @@ """ The Endpoint class, which provides plugins with a way to serve their own portion of the API """ -import os - from mako.template import Template from openlp.core.common.applocation import AppLocation @@ -67,13 +65,17 @@ class Endpoint(object): def render_template(self, filename, **kwargs): """ Render a mako template + + :param str filename: The file name of the template to render. + :return: The rendered template object. + :rtype: mako.template.Template """ - root = str(AppLocation.get_section_data_path('remotes')) + root_path = AppLocation.get_section_data_path('remotes') if not self.template_dir: raise Exception('No template directory specified') - path = os.path.join(root, self.template_dir, filename) + path = root_path / self.template_dir / filename if self.static_dir: kwargs['static_url'] = '/{prefix}/static'.format(prefix=self.url_prefix) kwargs['static_url'] = kwargs['static_url'].replace('//', '/') kwargs['assets_url'] = '/assets' - return Template(filename=path, input_encoding='utf-8').render(**kwargs) + return Template(filename=str(path), input_encoding='utf-8').render(**kwargs) diff --git a/openlp/core/api/http/wsgiapp.py b/openlp/core/api/http/wsgiapp.py index f948d4096..4c863b7cb 100644 --- a/openlp/core/api/http/wsgiapp.py +++ b/openlp/core/api/http/wsgiapp.py @@ -25,7 +25,6 @@ App stuff """ import json import logging -import os import re from webob import Request, Response @@ -138,12 +137,11 @@ class WSGIApplication(object): Add a static directory as a route """ if route not in self.static_routes: - root = str(AppLocation.get_section_data_path('remotes')) - static_path = os.path.abspath(os.path.join(root, static_dir)) - if not os.path.exists(static_path): + static_path = AppLocation.get_section_data_path('remotes') / static_dir + if not static_path.exists(): log.error('Static path "%s" does not exist. Skipping creating static route/', static_path) return - self.static_routes[route] = DirectoryApp(static_path) + self.static_routes[route] = DirectoryApp(str(static_path.resolve())) def dispatch(self, request): """ diff --git a/openlp/core/common/path.py b/openlp/core/common/path.py index 6b89acfb5..05d2ea4eb 100644 --- a/openlp/core/common/path.py +++ b/openlp/core/common/path.py @@ -69,6 +69,9 @@ 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 replace_params(args, kwargs, params): """ diff --git a/openlp/core/display/renderer.py b/openlp/core/display/renderer.py index 9dd7f3d1f..836e5edea 100644 --- a/openlp/core/display/renderer.py +++ b/openlp/core/display/renderer.py @@ -198,6 +198,7 @@ class Renderer(RegistryBase, LogMixin, RegistryProperties): :param theme_data: The theme to generated a preview for. :param force_page: Flag to tell message lines per page need to be generated. + :rtype: QtGui.QPixmap """ # save value for use in format_slide self.force_page = force_page @@ -222,8 +223,7 @@ class Renderer(RegistryBase, LogMixin, RegistryProperties): self.display.build_html(service_item) raw_html = service_item.get_rendered_frame(0) self.display.text(raw_html, False) - preview = self.display.preview() - return preview + return self.display.preview() self.force_page = False def format_slide(self, text, item): diff --git a/openlp/core/lib/__init__.py b/openlp/core/lib/__init__.py index 9bab13b71..49bae26eb 100644 --- a/openlp/core/lib/__init__.py +++ b/openlp/core/lib/__init__.py @@ -179,8 +179,9 @@ def create_thumb(image_path, thumb_path, return_icon=True, size=None): height of 88 is used. :return: The final icon. """ - ext = os.path.splitext(thumb_path)[1].lower() - reader = QtGui.QImageReader(image_path) + # TODO: To path object + thumb_path = Path(thumb_path) + reader = QtGui.QImageReader(str(image_path)) if size is None: # No size given; use default height of 88 if reader.size().isEmpty(): @@ -207,10 +208,10 @@ def create_thumb(image_path, thumb_path, return_icon=True, size=None): # Invalid; use default height of 88 reader.setScaledSize(QtCore.QSize(int(ratio * 88), 88)) thumb = reader.read() - thumb.save(thumb_path, ext[1:]) + thumb.save(str(thumb_path), thumb_path.suffix[1:].lower()) if not return_icon: return - if os.path.exists(thumb_path): + if thumb_path.exists(): return build_icon(thumb_path) # Fallback for files with animation support. return build_icon(image_path) diff --git a/openlp/core/lib/plugin.py b/openlp/core/lib/plugin.py index f155b3ce7..7e3b2e416 100644 --- a/openlp/core/lib/plugin.py +++ b/openlp/core/lib/plugin.py @@ -139,10 +139,6 @@ class Plugin(QtCore.QObject, RegistryProperties): self.text_strings = {} self.set_plugin_text_strings() self.name_strings = self.text_strings[StringContent.Name] - if version: - self.version = version - else: - self.version = get_version()['version'] self.settings_section = self.name self.icon = None self.media_item_class = media_item_class @@ -162,6 +158,19 @@ class Plugin(QtCore.QObject, RegistryProperties): Settings.extend_default_settings(default_settings) Registry().register_function('{name}_add_service_item'.format(name=self.name), self.process_add_service_event) Registry().register_function('{name}_config_updated'.format(name=self.name), self.config_update) + self._setup(version) + + def _setup(self, version): + """ + Run some initial setup. This method is separate from __init__ in order to mock it out in tests. + + :param version: Defaults to *None*, which means that the same version number is used as OpenLP's version number. + :rtype: None + """ + if version: + self.version = version + else: + self.version = get_version()['version'] def check_pre_conditions(self): """ diff --git a/openlp/core/ui/maindisplay.py b/openlp/core/ui/maindisplay.py index ca634910e..daf6a165c 100644 --- a/openlp/core/ui/maindisplay.py +++ b/openlp/core/ui/maindisplay.py @@ -398,6 +398,8 @@ class MainDisplay(Display, LogMixin, RegistryProperties): def preview(self): """ Generates a preview of the image displayed. + + :rtype: QtGui.QPixmap """ was_visible = self.isVisible() self.application.process_events() diff --git a/openlp/core/ui/mainwindow.py b/openlp/core/ui/mainwindow.py index c0e704afb..60fabdf58 100644 --- a/openlp/core/ui/mainwindow.py +++ b/openlp/core/ui/mainwindow.py @@ -296,10 +296,9 @@ class Ui_MainWindow(object): # Give QT Extra Hint that this is an About Menu Item self.about_item.setMenuRole(QtWidgets.QAction.AboutRole) if is_win(): - self.local_help_file = os.path.join(str(AppLocation.get_directory(AppLocation.AppDir)), 'OpenLP.chm') + self.local_help_file = AppLocation.get_directory(AppLocation.AppDir) / 'OpenLP.chm' elif is_macosx(): - self.local_help_file = os.path.join(str(AppLocation.get_directory(AppLocation.AppDir)), - '..', 'Resources', 'OpenLP.help') + self.local_help_file = AppLocation.get_directory(AppLocation.AppDir) / '..' / 'Resources' / 'OpenLP.help' self.user_manual_item = create_action(main_window, 'userManualItem', icon=':/system/system_help_contents.png', can_shortcuts=True, category=UiStrings().Help, triggers=self.on_help_clicked) @@ -760,7 +759,7 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties): Use the Online manual in other cases. (Linux) """ if is_macosx() or is_win(): - QtGui.QDesktopServices.openUrl(QtCore.QUrl.fromLocalFile(self.local_help_file)) + QtGui.QDesktopServices.openUrl(QtCore.QUrl.fromLocalFile(str(self.local_help_file))) else: import webbrowser webbrowser.open_new('http://manual.openlp.org/') diff --git a/openlp/core/ui/thememanager.py b/openlp/core/ui/thememanager.py index 1b39e5fec..fa8a9e4d9 100644 --- a/openlp/core/ui/thememanager.py +++ b/openlp/core/ui/thememanager.py @@ -497,12 +497,12 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R name = translate('OpenLP.ThemeManager', '{name} (default)').format(name=text_name) else: name = text_name - thumb = self.thumb_path / '{name}.png'.format(name=text_name) + thumb_path = self.thumb_path / '{name}.png'.format(name=text_name) item_name = QtWidgets.QListWidgetItem(name) - if validate_thumb(theme_path, thumb): - icon = build_icon(thumb) + if validate_thumb(theme_path, thumb_path): + icon = build_icon(thumb_path) else: - icon = create_thumb(str(theme_path), str(thumb)) + icon = create_thumb(theme_path, thumb_path) item_name.setIcon(icon) item_name.setData(QtCore.Qt.UserRole, text_name) self.theme_list_widget.addItem(item_name) @@ -692,7 +692,7 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R sample_path_name.unlink() frame.save(str(sample_path_name), 'png') thumb_path = self.thumb_path / '{name}.png'.format(name=theme_name) - create_thumb(str(sample_path_name), str(thumb_path), False) + create_thumb(sample_path_name, thumb_path, False) def update_preview_images(self): """ @@ -710,7 +710,8 @@ 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. + :param force_page: Flag to tell message lines per page need to be generated.7 + :rtype: QtGui.QPixmap """ return self.renderer.generate_preview(theme_data, force_page) diff --git a/openlp/core/version.py b/openlp/core/version.py index 314c4865f..6ba1b9ecc 100644 --- a/openlp/core/version.py +++ b/openlp/core/version.py @@ -23,7 +23,6 @@ The :mod:`openlp.core.version` module downloads the version details for OpenLP. """ import logging -import os import platform import sys import time @@ -176,18 +175,12 @@ def get_version(): full_version = '{tag}-bzr{tree}'.format(tag=tag_version.strip(), tree=tree_revision.strip()) else: # We're not running the development version, let's use the file. - file_path = str(AppLocation.get_directory(AppLocation.VersionDir)) - file_path = os.path.join(file_path, '.version') - version_file = None + file_path = AppLocation.get_directory(AppLocation.VersionDir) / '.version' try: - version_file = open(file_path, 'r') - full_version = str(version_file.read()).rstrip() + full_version = file_path.read_text().rstrip() except OSError: log.exception('Error in version file.') full_version = '0.0.0-bzr000' - finally: - if version_file: - version_file.close() bits = full_version.split('-') APPLICATION_VERSION = { 'full': full_version, diff --git a/openlp/plugins/images/lib/mediaitem.py b/openlp/plugins/images/lib/mediaitem.py index de347f8ce..552a63d44 100644 --- a/openlp/plugins/images/lib/mediaitem.py +++ b/openlp/plugins/images/lib/mediaitem.py @@ -377,7 +377,7 @@ class ImageMediaItem(MediaManagerItem): if validate_thumb(image.file_path, thumbnail_path): icon = build_icon(thumbnail_path) else: - icon = create_thumb(str(image.file_path), str(thumbnail_path)) + icon = create_thumb(image.file_path,thumbnail_path) item_name = QtWidgets.QTreeWidgetItem([file_name]) item_name.setText(0, file_name) item_name.setIcon(0, icon) diff --git a/openlp/plugins/presentations/lib/mediaitem.py b/openlp/plugins/presentations/lib/mediaitem.py index f58ba4861..923953040 100644 --- a/openlp/plugins/presentations/lib/mediaitem.py +++ b/openlp/plugins/presentations/lib/mediaitem.py @@ -198,10 +198,10 @@ class PresentationMediaItem(MediaManagerItem): if not (preview_path and preview_path.exists()): icon = build_icon(':/general/general_delete.png') else: - if validate_thumb(Path(preview_path), Path(thumbnail_path)): + if validate_thumb(preview_path, thumbnail_path): icon = build_icon(thumbnail_path) else: - icon = create_thumb(str(preview_path), str(thumbnail_path)) + icon = create_thumb(preview_path, thumbnail_path) else: if initial_load: icon = build_icon(':/general/general_delete.png') diff --git a/openlp/plugins/presentations/lib/presentationcontroller.py b/openlp/plugins/presentations/lib/presentationcontroller.py index bb424c9fa..db5786626 100644 --- a/openlp/plugins/presentations/lib/presentationcontroller.py +++ b/openlp/plugins/presentations/lib/presentationcontroller.py @@ -267,7 +267,7 @@ class PresentationDocument(object): return if image_path.is_file(): thumb_path = self.get_thumbnail_path(index, False) - create_thumb(str(image_path), str(thumb_path), False, QtCore.QSize(-1, 360)) + create_thumb(image_path, thumb_path, False, QtCore.QSize(-1, 360)) def get_thumbnail_path(self, slide_no, check_exists=False): """ diff --git a/tests/functional/openlp_core/api/test_deploy.py b/tests/functional/openlp_core/api/test_deploy.py index be36fb9c7..702e2a35d 100644 --- a/tests/functional/openlp_core/api/test_deploy.py +++ b/tests/functional/openlp_core/api/test_deploy.py @@ -19,15 +19,13 @@ # with this program; if not, write to the Free Software Foundation, Inc., 59 # # Temple Place, Suite 330, Boston, MA 02111-1307 USA # ############################################################################### - -import os -import shutil from tempfile import mkdtemp from unittest import TestCase from openlp.core.api.deploy import deploy_zipfile +from openlp.core.common.path import Path, copyfile -TEST_PATH = os.path.abspath(os.path.join(os.path.dirname(__file__), '..', '..', '..', 'resources')) +TEST_PATH = (Path(__file__).parent / '..' / '..' / '..' / 'resources').resolve() class TestRemoteDeploy(TestCase): @@ -39,25 +37,25 @@ class TestRemoteDeploy(TestCase): """ Setup for tests """ - self.app_root = mkdtemp() + self.app_root_path = Path(mkdtemp()) def tearDown(self): """ Clean up after tests """ - shutil.rmtree(self.app_root) + self.app_root_path.rmtree() def test_deploy_zipfile(self): """ Remote Deploy tests - test the dummy zip file is processed correctly """ # GIVEN: A new downloaded zip file - aa = TEST_PATH - zip_file = os.path.join(TEST_PATH, 'remotes', 'site.zip') - app_root = os.path.join(self.app_root, 'site.zip') - shutil.copyfile(zip_file, app_root) - # WHEN: I process the zipfile - deploy_zipfile(self.app_root, 'site.zip') + zip_path = TEST_PATH / 'remotes' / 'site.zip' + app_root_path = self.app_root_path / 'site.zip' + copyfile(zip_path, app_root_path) - # THEN test if www directory has been created - self.assertTrue(os.path.isdir(os.path.join(self.app_root, 'www')), 'We should have a www directory') + # WHEN: I process the zipfile + deploy_zipfile(self.app_root_path, 'site.zip') + + # THEN: test if www directory has been created + self.assertTrue((self.app_root_path / 'www').is_dir(), 'We should have a www directory') diff --git a/tests/functional/openlp_core/lib/test_lib.py b/tests/functional/openlp_core/lib/test_lib.py index 1352b5da5..67649fa49 100644 --- a/tests/functional/openlp_core/lib/test_lib.py +++ b/tests/functional/openlp_core/lib/test_lib.py @@ -274,32 +274,32 @@ class TestLib(TestCase): Test the create_thumb() function with a given size. """ # GIVEN: An image to create a thumb of. - image_path = os.path.join(TEST_PATH, 'church.jpg') - thumb_path = os.path.join(TEST_PATH, 'church_thumb.jpg') + image_path = Path(TEST_PATH, 'church.jpg') + thumb_path = Path(TEST_PATH, 'church_thumb.jpg') thumb_size = QtCore.QSize(10, 20) # Remove the thumb so that the test actually tests if the thumb will be created. Maybe it was not deleted in the # last test. try: - os.remove(thumb_path) + thumb_path.unlink() except: pass # Only continue when the thumb does not exist. - self.assertFalse(os.path.exists(thumb_path), 'Test was not run, because the thumb already exists.') + self.assertFalse(thumb_path.exists(), 'Test was not run, because the thumb already exists.') # WHEN: Create the thumb. icon = create_thumb(image_path, thumb_path, size=thumb_size) # THEN: Check if the thumb was created and scaled to the given size. - self.assertTrue(os.path.exists(thumb_path), 'Test was not ran, because the thumb already exists') + self.assertTrue(thumb_path.exists(), 'Test was not ran, because the thumb already exists') self.assertIsInstance(icon, QtGui.QIcon, 'The icon should be a QIcon') self.assertFalse(icon.isNull(), 'The icon should not be null') - self.assertEqual(thumb_size, QtGui.QImageReader(thumb_path).size(), 'The thumb should have the given size') + self.assertEqual(thumb_size, QtGui.QImageReader(str(thumb_path)).size(), 'The thumb should have the given size') # Remove the thumb so that the test actually tests if the thumb will be created. try: - os.remove(thumb_path) + thumb_path.unlink() except: pass @@ -308,14 +308,14 @@ class TestLib(TestCase): Test the create_thumb() function with no size specified. """ # GIVEN: An image to create a thumb of. - image_path = os.path.join(TEST_PATH, 'church.jpg') - thumb_path = os.path.join(TEST_PATH, 'church_thumb.jpg') + image_path = Path(TEST_PATH, 'church.jpg') + thumb_path = Path(TEST_PATH, 'church_thumb.jpg') expected_size = QtCore.QSize(63, 88) # Remove the thumb so that the test actually tests if the thumb will be created. Maybe it was not deleted in the # last test. try: - os.remove(thumb_path) + thumb_path.unlink() except: pass @@ -326,14 +326,15 @@ class TestLib(TestCase): icon = create_thumb(image_path, thumb_path) # THEN: Check if the thumb was created, retaining its aspect ratio. - self.assertTrue(os.path.exists(thumb_path), 'Test was not ran, because the thumb already exists') + self.assertTrue(thumb_path.exists(), 'Test was not ran, because the thumb already exists') self.assertIsInstance(icon, QtGui.QIcon, 'The icon should be a QIcon') self.assertFalse(icon.isNull(), 'The icon should not be null') - self.assertEqual(expected_size, QtGui.QImageReader(thumb_path).size(), 'The thumb should have the given size') + self.assertEqual(expected_size, QtGui.QImageReader(str(thumb_path)).size(), + 'The thumb should have the given size') # Remove the thumb so that the test actually tests if the thumb will be created. try: - os.remove(thumb_path) + thumb_path.unlink() except: pass @@ -342,33 +343,34 @@ class TestLib(TestCase): Test the create_thumb() function with invalid size specified. """ # GIVEN: An image to create a thumb of. - image_path = os.path.join(TEST_PATH, 'church.jpg') - thumb_path = os.path.join(TEST_PATH, 'church_thumb.jpg') + image_path = Path(TEST_PATH, 'church.jpg') + thumb_path = Path(TEST_PATH, 'church_thumb.jpg') thumb_size = QtCore.QSize(-1, -1) expected_size = QtCore.QSize(63, 88) # Remove the thumb so that the test actually tests if the thumb will be created. Maybe it was not deleted in the # last test. try: - os.remove(thumb_path) + thumb_path.unlink() except: pass # Only continue when the thumb does not exist. - self.assertFalse(os.path.exists(thumb_path), 'Test was not run, because the thumb already exists.') + self.assertFalse(thumb_path.exists(), 'Test was not run, because the thumb already exists.') # WHEN: Create the thumb. icon = create_thumb(image_path, thumb_path, size=thumb_size) # THEN: Check if the thumb was created, retaining its aspect ratio. - self.assertTrue(os.path.exists(thumb_path), 'Test was not ran, because the thumb already exists') + self.assertTrue(thumb_path.exists(), 'Test was not ran, because the thumb already exists') self.assertIsInstance(icon, QtGui.QIcon, 'The icon should be a QIcon') self.assertFalse(icon.isNull(), 'The icon should not be null') - self.assertEqual(expected_size, QtGui.QImageReader(thumb_path).size(), 'The thumb should have the given size') + self.assertEqual(expected_size, QtGui.QImageReader(str(thumb_path)).size(), + 'The thumb should have the given size') # Remove the thumb so that the test actually tests if the thumb will be created. try: - os.remove(thumb_path) + thumb_path.unlink() except: pass @@ -377,33 +379,33 @@ class TestLib(TestCase): Test the create_thumb() function with a size of only width specified. """ # GIVEN: An image to create a thumb of. - image_path = os.path.join(TEST_PATH, 'church.jpg') - thumb_path = os.path.join(TEST_PATH, 'church_thumb.jpg') + image_path = Path(TEST_PATH, 'church.jpg') + thumb_path = Path(TEST_PATH, 'church_thumb.jpg') thumb_size = QtCore.QSize(100, -1) expected_size = QtCore.QSize(100, 137) # Remove the thumb so that the test actually tests if the thumb will be created. Maybe it was not deleted in the # last test. try: - os.remove(thumb_path) + thumb_path.unlink() except: pass # Only continue when the thumb does not exist. - self.assertFalse(os.path.exists(thumb_path), 'Test was not run, because the thumb already exists.') + self.assertFalse(thumb_path.exists(), 'Test was not run, because the thumb already exists.') # WHEN: Create the thumb. icon = create_thumb(image_path, thumb_path, size=thumb_size) # THEN: Check if the thumb was created, retaining its aspect ratio. - self.assertTrue(os.path.exists(thumb_path), 'Test was not ran, because the thumb already exists') + self.assertTrue(thumb_path.exists(), 'Test was not ran, because the thumb already exists') self.assertIsInstance(icon, QtGui.QIcon, 'The icon should be a QIcon') self.assertFalse(icon.isNull(), 'The icon should not be null') - self.assertEqual(expected_size, QtGui.QImageReader(thumb_path).size(), 'The thumb should have the given size') + self.assertEqual(expected_size, QtGui.QImageReader(str(thumb_path)).size(), 'The thumb should have the given size') # Remove the thumb so that the test actually tests if the thumb will be created. try: - os.remove(thumb_path) + thumb_path.unlink() except: pass @@ -412,33 +414,33 @@ class TestLib(TestCase): Test the create_thumb() function with a size of only height specified. """ # GIVEN: An image to create a thumb of. - image_path = os.path.join(TEST_PATH, 'church.jpg') - thumb_path = os.path.join(TEST_PATH, 'church_thumb.jpg') + image_path = Path(TEST_PATH, 'church.jpg') + thumb_path = Path(TEST_PATH, 'church_thumb.jpg') thumb_size = QtCore.QSize(-1, 100) expected_size = QtCore.QSize(72, 100) # Remove the thumb so that the test actually tests if the thumb will be created. Maybe it was not deleted in the # last test. try: - os.remove(thumb_path) + thumb_path.unlink() except: pass # Only continue when the thumb does not exist. - self.assertFalse(os.path.exists(thumb_path), 'Test was not run, because the thumb already exists.') + self.assertFalse(thumb_path.exists(), 'Test was not run, because the thumb already exists.') # WHEN: Create the thumb. icon = create_thumb(image_path, thumb_path, size=thumb_size) # THEN: Check if the thumb was created, retaining its aspect ratio. - self.assertTrue(os.path.exists(thumb_path), 'Test was not ran, because the thumb already exists') + self.assertTrue(thumb_path.exists(), 'Test was not ran, because the thumb already exists') self.assertIsInstance(icon, QtGui.QIcon, 'The icon should be a QIcon') self.assertFalse(icon.isNull(), 'The icon should not be null') - self.assertEqual(expected_size, QtGui.QImageReader(thumb_path).size(), 'The thumb should have the given size') + self.assertEqual(expected_size, QtGui.QImageReader(str(thumb_path)).size(), 'The thumb should have the given size') # Remove the thumb so that the test actually tests if the thumb will be created. try: - os.remove(thumb_path) + thumb_path.unlink() except: pass @@ -447,8 +449,8 @@ class TestLib(TestCase): Test the create_thumb() function with a size of only height specified. """ # GIVEN: An image to create a thumb of. - image_path = os.path.join(TEST_PATH, 'church.jpg') - thumb_path = os.path.join(TEST_PATH, 'church_thumb.jpg') + image_path = Path(TEST_PATH, 'church.jpg') + thumb_path = Path(TEST_PATH, 'church_thumb.jpg') thumb_size = QtCore.QSize(-1, 100) expected_size_1 = QtCore.QSize(88, 88) expected_size_2 = QtCore.QSize(100, 100) @@ -456,12 +458,12 @@ class TestLib(TestCase): # Remove the thumb so that the test actually tests if the thumb will be created. Maybe it was not deleted in the # last test. try: - os.remove(thumb_path) + thumb_path.unlink() except: pass # Only continue when the thumb does not exist. - self.assertFalse(os.path.exists(thumb_path), 'Test was not run, because the thumb already exists.') + self.assertFalse(thumb_path.exists(), 'Test was not run, because the thumb already exists.') # WHEN: Create the thumb. with patch('openlp.core.lib.QtGui.QImageReader.size') as mocked_size: @@ -469,10 +471,10 @@ class TestLib(TestCase): icon = create_thumb(image_path, thumb_path, size=None) # THEN: Check if the thumb was created with aspect ratio of 1. - self.assertTrue(os.path.exists(thumb_path), 'Test was not ran, because the thumb already exists') + self.assertTrue(thumb_path.exists(), 'Test was not ran, because the thumb already exists') self.assertIsInstance(icon, QtGui.QIcon, 'The icon should be a QIcon') self.assertFalse(icon.isNull(), 'The icon should not be null') - self.assertEqual(expected_size_1, QtGui.QImageReader(thumb_path).size(), 'The thumb should have the given size') + self.assertEqual(expected_size_1, QtGui.QImageReader(str(thumb_path)).size(), 'The thumb should have the given size') # WHEN: Create the thumb. with patch('openlp.core.lib.QtGui.QImageReader.size') as mocked_size: @@ -482,11 +484,11 @@ class TestLib(TestCase): # THEN: Check if the thumb was created with aspect ratio of 1. self.assertIsInstance(icon, QtGui.QIcon, 'The icon should be a QIcon') self.assertFalse(icon.isNull(), 'The icon should not be null') - self.assertEqual(expected_size_2, QtGui.QImageReader(thumb_path).size(), 'The thumb should have the given size') + self.assertEqual(expected_size_2, QtGui.QImageReader(str(thumb_path)).size(), 'The thumb should have the given size') # Remove the thumb so that the test actually tests if the thumb will be created. try: - os.remove(thumb_path) + thumb_path.unlink() except: pass diff --git a/tests/functional/openlp_core/ui/test_thememanager.py b/tests/functional/openlp_core/ui/test_thememanager.py index 62bf980b4..f44f558b0 100644 --- a/tests/functional/openlp_core/ui/test_thememanager.py +++ b/tests/functional/openlp_core/ui/test_thememanager.py @@ -199,16 +199,16 @@ class TestThemeManager(TestCase): theme_manager._create_theme_from_xml = MagicMock() theme_manager.generate_and_save_image = MagicMock() theme_manager.theme_path = None - folder = Path(mkdtemp()) + folder_path = Path(mkdtemp()) theme_file = Path(TEST_RESOURCES_PATH, 'themes', 'Moss_on_tree.otz') # WHEN: We try to unzip it - theme_manager.unzip_theme(theme_file, folder) + theme_manager.unzip_theme(theme_file, folder_path) # THEN: Files should be unpacked - self.assertTrue((folder / 'Moss on tree' / 'Moss on tree.xml').exists()) + self.assertTrue((folder_path / 'Moss on tree' / 'Moss on tree.xml').exists()) self.assertEqual(mocked_critical_error_message_box.call_count, 0, 'No errors should have happened') - shutil.rmtree(str(folder)) + folder_path.rmtree() def test_unzip_theme_invalid_version(self): """ diff --git a/tests/functional/openlp_plugins/presentations/test_pdfcontroller.py b/tests/functional/openlp_plugins/presentations/test_pdfcontroller.py index a7281e062..9bd492983 100644 --- a/tests/functional/openlp_plugins/presentations/test_pdfcontroller.py +++ b/tests/functional/openlp_plugins/presentations/test_pdfcontroller.py @@ -67,10 +67,10 @@ class TestPdfController(TestCase, TestMixin): self.desktop.screenGeometry.return_value = SCREEN['size'] self.screens = ScreenList.create(self.desktop) Settings().extend_default_settings(__default_settings__) - self.temp_folder = Path(mkdtemp()) - self.thumbnail_folder = Path(mkdtemp()) + self.temp_folder_path = Path(mkdtemp()) + self.thumbnail_folder_path = Path(mkdtemp()) self.mock_plugin = MagicMock() - self.mock_plugin.settings_section = self.temp_folder + self.mock_plugin.settings_section = self.temp_folder_path def tearDown(self): """ @@ -78,8 +78,8 @@ class TestPdfController(TestCase, TestMixin): """ del self.screens self.destroy_settings() - shutil.rmtree(str(self.thumbnail_folder)) - shutil.rmtree(str(self.temp_folder)) + self.thumbnail_folder_path.rmtree() + self.temp_folder_path.rmtree() def test_constructor(self): """ @@ -105,8 +105,8 @@ class TestPdfController(TestCase, TestMixin): controller = PdfController(plugin=self.mock_plugin) if not controller.check_available(): raise SkipTest('Could not detect mudraw or ghostscript, so skipping PDF test') - controller.temp_folder = self.temp_folder - controller.thumbnail_folder = self.thumbnail_folder + controller.temp_folder = self.temp_folder_path + controller.thumbnail_folder = self.thumbnail_folder_path document = PdfDocument(controller, test_file) loaded = document.load_presentation() @@ -125,14 +125,14 @@ class TestPdfController(TestCase, TestMixin): controller = PdfController(plugin=self.mock_plugin) if not controller.check_available(): raise SkipTest('Could not detect mudraw or ghostscript, so skipping PDF test') - controller.temp_folder = self.temp_folder - controller.thumbnail_folder = self.thumbnail_folder + controller.temp_folder = self.temp_folder_path + controller.thumbnail_folder = self.thumbnail_folder_path document = PdfDocument(controller, test_file) loaded = document.load_presentation() # THEN: The load should succeed and pictures should be created and have been scales to fit the screen self.assertTrue(loaded, 'The loading of the PDF should succeed.') - image = QtGui.QImage(os.path.join(str(self.temp_folder), 'pdf_test1.pdf', 'mainslide001.png')) + image = QtGui.QImage(os.path.join(str(self.temp_folder_path), 'pdf_test1.pdf', 'mainslide001.png')) # Based on the converter used the resolution will differ a bit if controller.gsbin: self.assertEqual(760, image.height(), 'The height should be 760') diff --git a/tests/interfaces/openlp_core/lib/test_pluginmanager.py b/tests/interfaces/openlp_core/lib/test_pluginmanager.py index 2e7d979a6..588c15520 100644 --- a/tests/interfaces/openlp_core/lib/test_pluginmanager.py +++ b/tests/interfaces/openlp_core/lib/test_pluginmanager.py @@ -23,7 +23,6 @@ Package to test the openlp.core.lib.pluginmanager package. """ import sys -import shutil import gc from tempfile import mkdtemp from unittest import TestCase @@ -50,8 +49,8 @@ class TestPluginManager(TestCase, TestMixin): """ self.setup_application() self.build_settings() - self.temp_dir = Path(mkdtemp('openlp')) - Settings().setValue('advanced/data path', self.temp_dir) + self.temp_dir_path = Path(mkdtemp('openlp')) + Settings().setValue('advanced/data path', self.temp_dir_path) Registry.create() Registry().register('service_list', MagicMock()) self.main_window = QtWidgets.QMainWindow() @@ -64,7 +63,7 @@ class TestPluginManager(TestCase, TestMixin): # On windows we need to manually garbage collect to close sqlalchemy files # to avoid errors when temporary files are deleted. gc.collect() - shutil.rmtree(str(self.temp_dir)) + self.temp_dir_path.rmtree() @patch('openlp.plugins.songusage.lib.db.init_schema') @patch('openlp.plugins.songs.lib.db.init_schema') From f07d6e736c855fb3ae51644420af523da57fc960 Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Sat, 18 Nov 2017 22:37:24 +0000 Subject: [PATCH 2/7] Moar pathlib refactors --- openlp/core/api/endpoint/core.py | 12 ---- openlp/core/lib/mediamanageritem.py | 42 ++++++------- openlp/core/lib/pluginmanager.py | 3 +- openlp/core/ui/maindisplay.py | 4 +- openlp/core/ui/mainwindow.py | 59 ++++++++----------- .../presentations/presentationplugin.py | 9 +-- 6 files changed, 53 insertions(+), 76 deletions(-) diff --git a/openlp/core/api/endpoint/core.py b/openlp/core/api/endpoint/core.py index 2988e03aa..8b9bcc4c0 100644 --- a/openlp/core/api/endpoint/core.py +++ b/openlp/core/api/endpoint/core.py @@ -172,15 +172,3 @@ def main_image(request): 'slide_image': 'data:image/png;base64,' + str(image_to_byte(live_controller.slide_image)) } return {'results': result} - - -def get_content_type(file_name): - """ - Examines the extension of the file and determines what the content_type should be, defaults to text/plain - Returns the extension and the content_type - - :param file_name: name of file - """ - ext = os.path.splitext(file_name)[1] - content_type = FILE_TYPES.get(ext, 'text/plain') - return ext, content_type diff --git a/openlp/core/lib/mediamanageritem.py b/openlp/core/lib/mediamanageritem.py index cc884279c..8091b1a2e 100644 --- a/openlp/core/lib/mediamanageritem.py +++ b/openlp/core/lib/mediamanageritem.py @@ -318,10 +318,10 @@ class MediaManagerItem(QtWidgets.QWidget, RegistryProperties): self, self.on_new_prompt, Settings().value(self.settings_section + '/last directory'), self.on_new_file_masks) - log.info('New files(s) {file_paths}'.format(file_paths=file_paths)) + log.info('New file(s) {file_paths}'.format(file_paths=file_paths)) if file_paths: self.application.set_busy_cursor() - self.validate_and_load([path_to_str(path) for path in file_paths]) + self.validate_and_load(file_paths) self.application.set_normal_cursor() def load_file(self, data): @@ -330,23 +330,24 @@ class MediaManagerItem(QtWidgets.QWidget, RegistryProperties): :param data: A dictionary containing the list of files to be loaded and the target """ - new_files = [] + new_file_paths = [] error_shown = False for file_name in data['files']: - file_type = file_name.split('.')[-1] - if file_type.lower() not in self.on_new_file_masks: + file_path = str_to_path(file_name) # TODO: + if file_path.suffix[1:].lower() not in self.on_new_file_masks: if not error_shown: - critical_error_message_box(translate('OpenLP.MediaManagerItem', 'Invalid File Type'), - translate('OpenLP.MediaManagerItem', - 'Invalid File {name}.\n' - 'Suffix not supported').format(name=file_name)) + critical_error_message_box( + translate('OpenLP.MediaManagerItem', 'Invalid File Type'), + translate('OpenLP.MediaManagerItem', + 'Invalid File {file_path}.\nFile extension not supported').format( + file_path=file_path)) error_shown = True else: - new_files.append(file_name) - if new_files: + new_file_paths.append(file_path) + if new_file_paths: if 'target' in data: - self.validate_and_load(new_files, data['target']) - self.validate_and_load(new_files) + self.validate_and_load(new_file_paths, data['target']) + self.validate_and_load(new_file_paths) def dnd_move_internal(self, target): """ @@ -356,31 +357,30 @@ class MediaManagerItem(QtWidgets.QWidget, RegistryProperties): """ pass - def validate_and_load(self, files, target_group=None): + def validate_and_load(self, file_paths, target_group=None): """ Process a list for files either from the File Dialog or from Drag and Drop - :param files: The files to be loaded. + :param list[openlp.core.common.path.Path] file_paths: The files to be loaded. :param target_group: The QTreeWidgetItem of the group that will be the parent of the added files """ full_list = [] for count in range(self.list_view.count()): - full_list.append(self.list_view.item(count).data(QtCore.Qt.UserRole)) + full_list.append(self.list_view.item(count).data(QtCore.Qt.UserRole)) # TODO: Path objects duplicates_found = False files_added = False - for file_path in files: - if file_path in full_list: + for file_path in file_paths: + if path_to_str(file_path) in full_list: duplicates_found = True else: files_added = True - full_list.append(file_path) + full_list.append(path_to_str(file_path)) if full_list and files_added: if target_group is None: self.list_view.clear() self.load_list(full_list, target_group) - last_dir = os.path.split(files[0])[0] - Settings().setValue(self.settings_section + '/last directory', Path(last_dir)) + Settings().setValue(self.settings_section + '/last directory', file_paths[0].parent) Settings().setValue('{section}/{section} files'.format(section=self.settings_section), self.get_file_list()) if duplicates_found: critical_error_message_box(UiStrings().Duplicate, diff --git a/openlp/core/lib/pluginmanager.py b/openlp/core/lib/pluginmanager.py index 061788b25..d4a02f8c0 100644 --- a/openlp/core/lib/pluginmanager.py +++ b/openlp/core/lib/pluginmanager.py @@ -43,8 +43,7 @@ class PluginManager(RegistryBase, LogMixin, RegistryProperties): """ super(PluginManager, self).__init__(parent) self.log_info('Plugin manager Initialising') - self.base_path = os.path.abspath(str(AppLocation.get_directory(AppLocation.PluginsDir))) - self.log_debug('Base path {path}'.format(path=self.base_path)) + self.log_debug('Base path {path}'.format(path=AppLocation.get_directory(AppLocation.PluginsDir))) self.plugins = [] self.log_info('Plugin manager Initialised') diff --git a/openlp/core/ui/maindisplay.py b/openlp/core/ui/maindisplay.py index daf6a165c..80cccdb4c 100644 --- a/openlp/core/ui/maindisplay.py +++ b/openlp/core/ui/maindisplay.py @@ -29,7 +29,6 @@ Some of the code for this form is based on the examples at: """ import html import logging -import os from PyQt5 import QtCore, QtWidgets, QtWebKit, QtWebKitWidgets, QtGui, QtMultimedia @@ -490,8 +489,7 @@ class MainDisplay(Display, LogMixin, RegistryProperties): service_item = ServiceItem() service_item.title = 'webkit' service_item.processor = 'webkit' - path = os.path.join(str(AppLocation.get_section_data_path('themes')), - self.service_item.theme_data.theme_name) + path = str(AppLocation.get_section_data_path('themes') / self.service_item.theme_data.theme_name) service_item.add_from_command(path, path_to_str(self.service_item.theme_data.background_filename), ':/media/slidecontroller_multimedia.png') diff --git a/openlp/core/ui/mainwindow.py b/openlp/core/ui/mainwindow.py index 60fabdf58..5cdee786e 100644 --- a/openlp/core/ui/mainwindow.py +++ b/openlp/core/ui/mainwindow.py @@ -649,8 +649,8 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties): self.application.process_events() plugin.first_time() self.application.process_events() - temp_dir = os.path.join(str(gettempdir()), 'openlp') - shutil.rmtree(temp_dir, True) + temp_path = Path(gettempdir(), 'openlp') + temp_path.rmtree(True) def on_first_time_wizard_clicked(self): """ @@ -1219,7 +1219,7 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties): settings.remove('custom slide') settings.remove('service') settings.beginGroup(self.general_settings_section) - self.recent_files = [path_to_str(file_path) for file_path in settings.value('recent files')] + self.recent_files = settings.value('recent files') settings.endGroup() settings.beginGroup(self.ui_settings_section) self.move(settings.value('main window position')) @@ -1243,7 +1243,7 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties): log.debug('Saving QSettings') settings = Settings() settings.beginGroup(self.general_settings_section) - settings.setValue('recent files', [str_to_path(file) for file in self.recent_files]) + settings.setValue('recent files', self.recent_files) settings.endGroup() settings.beginGroup(self.ui_settings_section) settings.setValue('main window position', self.pos()) @@ -1259,26 +1259,24 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties): Updates the recent file menu with the latest list of service files accessed. """ recent_file_count = Settings().value('advanced/recent file count') - existing_recent_files = [recentFile for recentFile in self.recent_files if os.path.isfile(str(recentFile))] - recent_files_to_display = existing_recent_files[0:recent_file_count] self.recent_files_menu.clear() - for file_id, filename in enumerate(recent_files_to_display): - log.debug('Recent file name: {name}'.format(name=filename)) + count = 0 + for recent_path in self.recent_files: + if not recent_path.is_file(): + continue + count += 1 + log.debug('Recent file name: {name}'.format(name=recent_path)) action = create_action(self, '', - text='&{n} {name}'.format(n=file_id + 1, - name=os.path.splitext(os.path.basename(str(filename)))[0]), - data=filename, - triggers=self.service_manager_contents.on_recent_service_clicked) + text='&{n} {name}'.format(n=count, name=recent_path.name), + data=recent_path, triggers=self.service_manager_contents.on_recent_service_clicked) self.recent_files_menu.addAction(action) - clear_recent_files_action = create_action(self, '', - text=translate('OpenLP.MainWindow', 'Clear List', 'Clear List of ' - 'recent files'), - statustip=translate('OpenLP.MainWindow', 'Clear the list of recent ' - 'files.'), - enabled=bool(self.recent_files), - triggers=self.clear_recent_file_menu) + if count == recent_file_count: + break + clear_recent_files_action = \ + create_action(self, '', text=translate('OpenLP.MainWindow', 'Clear List', 'Clear List of recent files'), + statustip=translate('OpenLP.MainWindow', 'Clear the list of recent files.'), + enabled=bool(self.recent_files), triggers=self.clear_recent_file_menu) add_actions(self.recent_files_menu, (None, clear_recent_files_action)) - clear_recent_files_action.setEnabled(bool(self.recent_files)) def add_recent_file(self, filename): """ @@ -1290,20 +1288,13 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties): # actually stored in the settings therefore the default value of 20 will # always be used. max_recent_files = Settings().value('advanced/max recent files') - if filename: - # Add some cleanup to reduce duplication in the recent file list - filename = os.path.abspath(filename) - # abspath() only capitalises the drive letter if it wasn't provided - # in the given filename which then causes duplication. - if filename[1:3] == ':\\': - filename = filename[0].upper() + filename[1:] - if filename in self.recent_files: - self.recent_files.remove(filename) - if not isinstance(self.recent_files, list): - self.recent_files = [self.recent_files] - self.recent_files.insert(0, filename) - while len(self.recent_files) > max_recent_files: - self.recent_files.pop() + file_path = Path(filename) + # Some cleanup to reduce duplication in the recent file list + file_path = file_path.resolve() + if file_path in self.recent_files: + self.recent_files.remove(file_path) + self.recent_files.insert(0, file_path) + self.recent_files = self.recent_files[:max_recent_files] def clear_recent_file_menu(self): """ diff --git a/openlp/plugins/presentations/presentationplugin.py b/openlp/plugins/presentations/presentationplugin.py index 5e32af7b6..4c4ba5253 100644 --- a/openlp/plugins/presentations/presentationplugin.py +++ b/openlp/plugins/presentations/presentationplugin.py @@ -31,6 +31,7 @@ from PyQt5 import QtCore from openlp.core.api.http import register_endpoint from openlp.core.common import extension_loader from openlp.core.common.i18n import translate +from openlp.core.common.path import path_to_str from openlp.core.common.settings import Settings from openlp.core.lib import Plugin, StringContent, build_icon from openlp.plugins.presentations.endpoint import api_presentations_endpoint, presentations_endpoint @@ -144,12 +145,12 @@ class PresentationPlugin(Plugin): # TODO: Can be removed when the upgrade path to OpenLP 3.0 is no longer needed, also ensure code in # PresentationDocument.get_thumbnail_folder and PresentationDocument.get_temp_folder is removed super().app_startup() - files_from_config = Settings().value('presentations/presentations files') - for file in files_from_config: - self.media_item.clean_up_thumbnails(file, clean_for_update=True) + presentation_paths = Settings().value('presentations/presentations files') + for path in presentation_paths: + self.media_item.clean_up_thumbnails(path, clean_for_update=True) self.media_item.list_view.clear() Settings().setValue('presentations/thumbnail_scheme', 'md5') - self.media_item.validate_and_load(files_from_config) + self.media_item.validate_and_load(presentation_paths) @staticmethod def about(): From 1b168dd7bfef74f739c144ccfdcf1cddc58aae7b Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Sat, 18 Nov 2017 23:14:28 +0000 Subject: [PATCH 3/7] More pathlib refactors --- openlp/core/common/__init__.py | 11 ----- openlp/core/ui/servicemanager.py | 16 ++++--- .../openlp_core/common/test_init.py | 43 +------------------ .../openlp_core/ui/test_servicemanager.py | 4 +- 4 files changed, 12 insertions(+), 62 deletions(-) diff --git a/openlp/core/common/__init__.py b/openlp/core/common/__init__.py index d280cbde2..99e222041 100644 --- a/openlp/core/common/__init__.py +++ b/openlp/core/common/__init__.py @@ -318,17 +318,6 @@ def get_filesystem_encoding(): return encoding -def split_filename(path): - """ - Return a list of the parts in a given path. - """ - path = os.path.abspath(path) - if not os.path.isfile(path): - return path, '' - else: - return os.path.split(path) - - def delete_file(file_path): """ Deletes a file from the system. diff --git a/openlp/core/ui/servicemanager.py b/openlp/core/ui/servicemanager.py index 81edd4930..9c943afaf 100644 --- a/openlp/core/ui/servicemanager.py +++ b/openlp/core/ui/servicemanager.py @@ -32,7 +32,7 @@ from tempfile import mkstemp from PyQt5 import QtCore, QtGui, QtWidgets -from openlp.core.common import ThemeLevel, split_filename, delete_file +from openlp.core.common import ThemeLevel, delete_file from openlp.core.common.actions import ActionList, CategoryOrder from openlp.core.common.applocation import AppLocation from openlp.core.common.i18n import UiStrings, format_time, translate @@ -319,7 +319,7 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi self.service_id = 0 # is a new service and has not been saved self._modified = False - self._file_name = '' + self._service_path = None self.service_has_all_original_files = True self.list_double_clicked = False @@ -366,7 +366,7 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi :param openlp.core.common.path.Path file_path: The service file name :rtype: None """ - self._file_name = path_to_str(file_path) + self._service_path = file_path self.main_window.set_service_modified(self.is_modified(), self.short_file_name()) Settings().setValue('servicemanager/last file', file_path) if file_path and file_path.suffix == '.oszl': @@ -377,14 +377,16 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi def file_name(self): """ Return the current file name including path. + + :rtype: openlp.core.common.path.Path """ - return self._file_name + return self._service_path def short_file_name(self): """ Return the current file name, excluding the path. """ - return split_filename(self._file_name)[1] + return self._service_path.name def reset_supported_suffixes(self): """ @@ -706,13 +708,13 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi default_file_path = directory_path / default_file_path lite_filter = translate('OpenLP.ServiceManager', 'OpenLP Service Files - lite (*.oszl)') packaged_filter = translate('OpenLP.ServiceManager', 'OpenLP Service Files (*.osz)') - if self._file_name.endswith('oszl'): + if self._service_path and self._service_path.suffix == '.oszl': default_filter = lite_filter else: default_filter = packaged_filter # SaveAs from osz to oszl is not valid as the files will be deleted on exit which is not sensible or usable in # the long term. - if self._file_name.endswith('oszl') or self.service_has_all_original_files: + if self._service_path and self._service_path.suffix == '.oszl' or self.service_has_all_original_files: file_path, filter_used = FileDialog.getSaveFileName( self.main_window, UiStrings().SaveService, default_file_path, '{packaged};; {lite}'.format(packaged=packaged_filter, lite=lite_filter), diff --git a/tests/functional/openlp_core/common/test_init.py b/tests/functional/openlp_core/common/test_init.py index 5acfe59bb..9b86c99aa 100644 --- a/tests/functional/openlp_core/common/test_init.py +++ b/tests/functional/openlp_core/common/test_init.py @@ -28,7 +28,7 @@ from unittest import TestCase from unittest.mock import MagicMock, PropertyMock, call, patch from openlp.core.common import add_actions, clean_filename, delete_file, get_file_encoding, get_filesystem_encoding, \ - get_uno_command, get_uno_instance, split_filename + get_uno_command, get_uno_instance from openlp.core.common.path import Path from tests.helpers.testmixin import TestMixin @@ -236,47 +236,6 @@ class TestInit(TestCase, TestMixin): mocked_getdefaultencoding.assert_called_with() self.assertEqual('utf-8', result, 'The result should be "utf-8"') - def test_split_filename_with_file_path(self): - """ - Test the split_filename() function with a path to a file - """ - # GIVEN: A path to a file. - if os.name == 'nt': - file_path = 'C:\\home\\user\\myfile.txt' - wanted_result = ('C:\\home\\user', 'myfile.txt') - else: - file_path = '/home/user/myfile.txt' - wanted_result = ('/home/user', 'myfile.txt') - with patch('openlp.core.common.os.path.isfile') as mocked_is_file: - mocked_is_file.return_value = True - - # WHEN: Split the file name. - result = split_filename(file_path) - - # THEN: A tuple should be returned. - self.assertEqual(wanted_result, result, 'A tuple with the dir and file name should have been returned') - - def test_split_filename_with_dir_path(self): - """ - Test the split_filename() function with a path to a directory - """ - # GIVEN: A path to a dir. - if os.name == 'nt': - file_path = 'C:\\home\\user\\mydir' - wanted_result = ('C:\\home\\user\\mydir', '') - else: - file_path = '/home/user/mydir' - wanted_result = ('/home/user/mydir', '') - with patch('openlp.core.common.os.path.isfile') as mocked_is_file: - mocked_is_file.return_value = False - - # WHEN: Split the file name. - result = split_filename(file_path) - - # THEN: A tuple should be returned. - self.assertEqual(wanted_result, result, - 'A two-entry tuple with the directory and file name (empty) should have been returned.') - def test_clean_filename(self): """ Test the clean_filename() function diff --git a/tests/functional/openlp_core/ui/test_servicemanager.py b/tests/functional/openlp_core/ui/test_servicemanager.py index adf241d35..3c0958506 100644 --- a/tests/functional/openlp_core/ui/test_servicemanager.py +++ b/tests/functional/openlp_core/ui/test_servicemanager.py @@ -637,7 +637,7 @@ class TestServiceManager(TestCase): Registry().register('main_window', mocked_main_window) Registry().register('application', MagicMock()) service_manager = ServiceManager(None) - service_manager._file_name = os.path.join('temp', 'filename.osz') + service_manager._service_path = os.path.join('temp', 'filename.osz') service_manager._save_lite = False service_manager.service_items = [] service_manager.service_theme = 'Default' @@ -666,7 +666,7 @@ class TestServiceManager(TestCase): Registry().register('main_window', mocked_main_window) Registry().register('application', MagicMock()) service_manager = ServiceManager(None) - service_manager._file_name = os.path.join('temp', 'filename.osz') + service_manager._service_path = os.path.join('temp', 'filename.osz') service_manager._save_lite = False service_manager.service_items = [] service_manager.service_theme = 'Default' From a864dbbbc99822cd79fbadef3a8e19b701bf5a11 Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Sun, 19 Nov 2017 21:57:38 +0000 Subject: [PATCH 4/7] Minor tidyups --- openlp/core/ui/mainwindow.py | 2 +- openlp/core/ui/servicemanager.py | 2 +- openlp/plugins/media/mediaplugin.py | 9 +++++---- openlp/plugins/presentations/presentationplugin.py | 1 - 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/openlp/core/ui/mainwindow.py b/openlp/core/ui/mainwindow.py index 7c3881203..312e9d445 100644 --- a/openlp/core/ui/mainwindow.py +++ b/openlp/core/ui/mainwindow.py @@ -40,7 +40,7 @@ from openlp.core.common import is_win, is_macosx, add_actions from openlp.core.common.actions import ActionList, CategoryOrder from openlp.core.common.applocation import AppLocation from openlp.core.common.i18n import LanguageManager, UiStrings, translate -from openlp.core.common.path import Path, copyfile, create_paths, path_to_str, str_to_path +from openlp.core.common.path import Path, copyfile, create_paths from openlp.core.common.mixins import RegistryProperties from openlp.core.common.registry import Registry from openlp.core.common.settings import Settings diff --git a/openlp/core/ui/servicemanager.py b/openlp/core/ui/servicemanager.py index 9c943afaf..e6fb34e2f 100644 --- a/openlp/core/ui/servicemanager.py +++ b/openlp/core/ui/servicemanager.py @@ -37,7 +37,7 @@ from openlp.core.common.actions import ActionList, CategoryOrder from openlp.core.common.applocation import AppLocation from openlp.core.common.i18n import UiStrings, format_time, translate from openlp.core.common.mixins import LogMixin, RegistryProperties -from openlp.core.common.path import Path, create_paths, path_to_str, str_to_path +from openlp.core.common.path import Path, create_paths, str_to_path from openlp.core.common.registry import Registry, RegistryBase from openlp.core.common.settings import Settings from openlp.core.lib import ServiceItem, ItemCapabilities, PluginStatus, build_icon diff --git a/openlp/plugins/media/mediaplugin.py b/openlp/plugins/media/mediaplugin.py index 5efe8c910..461e78746 100644 --- a/openlp/plugins/media/mediaplugin.py +++ b/openlp/plugins/media/mediaplugin.py @@ -78,10 +78,10 @@ class MediaPlugin(Plugin): """ log.debug('check_installed Mediainfo') # Try to find mediainfo in the path - exists = process_check_binary('mediainfo') + exists = process_check_binary(Path('mediainfo')) # If mediainfo is not in the path, try to find it in the application folder if not exists: - exists = process_check_binary(os.path.join(str(AppLocation.get_directory(AppLocation.AppDir)), 'mediainfo')) + exists = process_check_binary(AppLocation.get_directory(AppLocation.AppDir) / 'mediainfo') return exists def app_startup(self): @@ -165,10 +165,11 @@ def process_check_binary(program_path): """ Function that checks whether a binary MediaInfo is present - :param program_path:The full path to the binary to check. + :param openlp.core.common.path.Path program_path:The full path to the binary to check. :return: If exists or not + :rtype: bool """ - runlog = check_binary_exists(Path(program_path)) + runlog = check_binary_exists(program_path) # Analyse the output to see it the program is mediainfo for line in runlog.splitlines(): decoded_line = line.decode() diff --git a/openlp/plugins/presentations/presentationplugin.py b/openlp/plugins/presentations/presentationplugin.py index 4c4ba5253..a68a22176 100644 --- a/openlp/plugins/presentations/presentationplugin.py +++ b/openlp/plugins/presentations/presentationplugin.py @@ -31,7 +31,6 @@ from PyQt5 import QtCore from openlp.core.api.http import register_endpoint from openlp.core.common import extension_loader from openlp.core.common.i18n import translate -from openlp.core.common.path import path_to_str from openlp.core.common.settings import Settings from openlp.core.lib import Plugin, StringContent, build_icon from openlp.plugins.presentations.endpoint import api_presentations_endpoint, presentations_endpoint From 7d0b84126962632f46f54ae08f1859db3fbce1f2 Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Mon, 20 Nov 2017 21:57:34 +0000 Subject: [PATCH 5/7] tidyups --- openlp/core/common/path.py | 28 ++++++------------- openlp/core/lib/mediamanageritem.py | 4 +-- openlp/core/ui/thememanager.py | 8 +++--- .../lib/presentationcontroller.py | 6 ++-- .../openlp_core/common/test_path.py | 20 +++++++------ .../songs/test_openlyricsexport.py | 5 ++-- 6 files changed, 32 insertions(+), 39 deletions(-) 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): """ From 97e6e759bd457828c12ba8344d5d38e004f948ee Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Tue, 21 Nov 2017 07:15:05 +0000 Subject: [PATCH 6/7] test fix --- tests/functional/openlp_core/lib/test_lib.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/openlp_core/lib/test_lib.py b/tests/functional/openlp_core/lib/test_lib.py index 67649fa49..15126c14e 100644 --- a/tests/functional/openlp_core/lib/test_lib.py +++ b/tests/functional/openlp_core/lib/test_lib.py @@ -320,7 +320,7 @@ class TestLib(TestCase): pass # Only continue when the thumb does not exist. - self.assertFalse(os.path.exists(thumb_path), 'Test was not run, because the thumb already exists.') + self.assertFalse(thumb_path.exists(), 'Test was not run, because the thumb already exists.') # WHEN: Create the thumb. icon = create_thumb(image_path, thumb_path) From bd2efc8ec028fda8879a6a9e6c0be006ff3ea4aa Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Tue, 21 Nov 2017 07:23:02 +0000 Subject: [PATCH 7/7] PEP8 --- openlp/plugins/images/lib/mediaitem.py | 2 +- tests/functional/openlp_core/lib/test_lib.py | 12 ++++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/openlp/plugins/images/lib/mediaitem.py b/openlp/plugins/images/lib/mediaitem.py index 552a63d44..25a8d2353 100644 --- a/openlp/plugins/images/lib/mediaitem.py +++ b/openlp/plugins/images/lib/mediaitem.py @@ -377,7 +377,7 @@ class ImageMediaItem(MediaManagerItem): if validate_thumb(image.file_path, thumbnail_path): icon = build_icon(thumbnail_path) else: - icon = create_thumb(image.file_path,thumbnail_path) + icon = create_thumb(image.file_path, thumbnail_path) item_name = QtWidgets.QTreeWidgetItem([file_name]) item_name.setText(0, file_name) item_name.setIcon(0, icon) diff --git a/tests/functional/openlp_core/lib/test_lib.py b/tests/functional/openlp_core/lib/test_lib.py index 15126c14e..e9788060e 100644 --- a/tests/functional/openlp_core/lib/test_lib.py +++ b/tests/functional/openlp_core/lib/test_lib.py @@ -401,7 +401,8 @@ class TestLib(TestCase): self.assertTrue(thumb_path.exists(), 'Test was not ran, because the thumb already exists') self.assertIsInstance(icon, QtGui.QIcon, 'The icon should be a QIcon') self.assertFalse(icon.isNull(), 'The icon should not be null') - self.assertEqual(expected_size, QtGui.QImageReader(str(thumb_path)).size(), 'The thumb should have the given size') + self.assertEqual( + expected_size, QtGui.QImageReader(str(thumb_path)).size(), 'The thumb should have the given size') # Remove the thumb so that the test actually tests if the thumb will be created. try: @@ -436,7 +437,8 @@ class TestLib(TestCase): self.assertTrue(thumb_path.exists(), 'Test was not ran, because the thumb already exists') self.assertIsInstance(icon, QtGui.QIcon, 'The icon should be a QIcon') self.assertFalse(icon.isNull(), 'The icon should not be null') - self.assertEqual(expected_size, QtGui.QImageReader(str(thumb_path)).size(), 'The thumb should have the given size') + self.assertEqual( + expected_size, QtGui.QImageReader(str(thumb_path)).size(), 'The thumb should have the given size') # Remove the thumb so that the test actually tests if the thumb will be created. try: @@ -474,7 +476,8 @@ class TestLib(TestCase): self.assertTrue(thumb_path.exists(), 'Test was not ran, because the thumb already exists') self.assertIsInstance(icon, QtGui.QIcon, 'The icon should be a QIcon') self.assertFalse(icon.isNull(), 'The icon should not be null') - self.assertEqual(expected_size_1, QtGui.QImageReader(str(thumb_path)).size(), 'The thumb should have the given size') + self.assertEqual( + expected_size_1, QtGui.QImageReader(str(thumb_path)).size(), 'The thumb should have the given size') # WHEN: Create the thumb. with patch('openlp.core.lib.QtGui.QImageReader.size') as mocked_size: @@ -484,7 +487,8 @@ class TestLib(TestCase): # THEN: Check if the thumb was created with aspect ratio of 1. self.assertIsInstance(icon, QtGui.QIcon, 'The icon should be a QIcon') self.assertFalse(icon.isNull(), 'The icon should not be null') - self.assertEqual(expected_size_2, QtGui.QImageReader(str(thumb_path)).size(), 'The thumb should have the given size') + self.assertEqual( + expected_size_2, QtGui.QImageReader(str(thumb_path)).size(), 'The thumb should have the given size') # Remove the thumb so that the test actually tests if the thumb will be created. try: