Phill has proposed merging lp:~phill-ridout/openlp/pathlib3 into lp:openlp.
Requested reviews: Raoul Snyman (raoul-snyman) Tomas Groth (tomasgroth) Tim Bentley (trb143) For more details, see: https://code.launchpad.net/~phill-ridout/openlp/pathlib3/+merge/329481 Part 3, converted some more utility methods lp:~phill-ridout/openlp/pathlib3 (revision 2762) [SUCCESS] https://ci.openlp.io/job/Branch-01-Pull/2173/ [SUCCESS] https://ci.openlp.io/job/Branch-02-Functional-Tests/2077/ [SUCCESS] https://ci.openlp.io/job/Branch-03-Interface-Tests/1964/ [SUCCESS] https://ci.openlp.io/job/Branch-04a-Code_Analysis/1334/ [SUCCESS] https://ci.openlp.io/job/Branch-04b-Test_Coverage/1171/ [SUCCESS] https://ci.openlp.io/job/Branch-04c-Code_Analysis2/301/ [FAILURE] https://ci.openlp.io/job/Branch-05-AppVeyor-Tests/144/ Stopping after failure -- Your team OpenLP Core is subscribed to branch lp:openlp.
=== modified file 'openlp/core/__init__.py' --- openlp/core/__init__.py 2017-08-03 17:54:40 +0000 +++ openlp/core/__init__.py 2017-08-23 20:29:44 +0000 @@ -33,6 +33,7 @@ import shutil import sys import time +from pathlib import Path from traceback import format_exception from PyQt5 import QtCore, QtGui, QtWidgets @@ -346,15 +347,18 @@ """ Setup our logging using log_path - :param log_path: the path + :param pathlib.Path log_path: The file to save the log to + :return: None + :rtype: None """ check_directory_exists(log_path, True) - filename = os.path.join(log_path, 'openlp.log') - logfile = logging.FileHandler(filename, 'w', encoding="UTF-8") + file_path = log_path / 'openlp.log' + # TODO: FileHandler accepts a Path object in Py3.6 + logfile = logging.FileHandler(str(file_path), 'w', encoding='UTF-8') logfile.setFormatter(logging.Formatter('%(asctime)s %(name)-55s %(levelname)-8s %(message)s')) log.addHandler(logfile) if log.isEnabledFor(logging.DEBUG): - print('Logging to: {name}'.format(name=filename)) + print('Logging to: {name}'.format(name=file_path)) def main(args=None): @@ -390,24 +394,24 @@ application.setApplicationName('OpenLPPortable') Settings.setDefaultFormat(Settings.IniFormat) # Get location OpenLPPortable.ini - application_path = str(AppLocation.get_directory(AppLocation.AppDir)) - set_up_logging(os.path.abspath(os.path.join(application_path, '..', '..', 'Other'))) + portable_path = (AppLocation.get_directory(AppLocation.AppDir) / '..' / '..').resolve() + data_path = portable_path / 'Data' + set_up_logging(portable_path / 'Other') log.info('Running portable') - portable_settings_file = os.path.abspath(os.path.join(application_path, '..', '..', 'Data', 'OpenLP.ini')) + portable_settings_path = data_path / 'OpenLP.ini' # Make this our settings file - log.info('INI file: {name}'.format(name=portable_settings_file)) - Settings.set_filename(portable_settings_file) + log.info('INI file: {name}'.format(name=portable_settings_path)) + Settings.set_filename(str(portable_settings_path)) portable_settings = Settings() # Set our data path - data_path = os.path.abspath(os.path.join(application_path, '..', '..', 'Data',)) log.info('Data path: {name}'.format(name=data_path)) # Point to our data path - portable_settings.setValue('advanced/data path', data_path) + portable_settings.setValue('advanced/data path', str(data_path)) portable_settings.setValue('advanced/is portable', True) portable_settings.sync() else: application.setApplicationName('OpenLP') - set_up_logging(str(AppLocation.get_directory(AppLocation.CacheDir))) + set_up_logging(AppLocation.get_directory(AppLocation.CacheDir)) Registry.create() Registry().register('application', application) Registry().set_flag('no_web_server', args.no_web_server) === modified file 'openlp/core/common/__init__.py' --- openlp/core/common/__init__.py 2017-08-01 20:59:41 +0000 +++ openlp/core/common/__init__.py 2017-08-23 20:29:44 +0000 @@ -32,7 +32,6 @@ import traceback from chardet.universaldetector import UniversalDetector from ipaddress import IPv4Address, IPv6Address, AddressValueError -from pathlib import Path from shutil import which from subprocess import check_output, CalledProcessError, STDOUT @@ -65,17 +64,19 @@ def check_directory_exists(directory, do_not_log=False): """ - Check a theme directory exists and if not create it + Check a directory exists and if not create it - :param directory: The directory to make sure exists - :param do_not_log: To not log anything. This is need for the start up, when the log isn't ready. + :param pathlib.Path directory: The directory to make sure exists + :param bool do_not_log: To not log anything. This is need for the start up, when the log isn't ready. + :return: None + :rtype: None """ if not do_not_log: log.debug('check_directory_exists {text}'.format(text=directory)) try: - if not os.path.exists(directory): - os.makedirs(directory) - except IOError as e: + if not directory.exists(): + directory.mkdir(parents=True) + except IOError: if not do_not_log: log.exception('failed to check if directory exists or create directory') @@ -85,19 +86,15 @@ A utility function to find and load OpenLP extensions, such as plugins, presentation and media controllers and importers. - :param glob_pattern: A glob pattern used to find the extension(s) to be imported. Should be relative to the - application directory. i.e. openlp/plugins/*/*plugin.py - :type glob_pattern: str - - :param excluded_files: A list of file names to exclude that the glob pattern may find. - :type excluded_files: list of strings - + :param str glob_pattern: A glob pattern used to find the extension(s) to be imported. Should be relative to the + application directory. i.e. plugins/*/*plugin.py + :param list[str] excluded_files: A list of file names to exclude that the glob pattern may find. :return: None :rtype: None """ - base_dir_path = AppLocation.get_directory(AppLocation.AppDir).parent - for extension_path in base_dir_path.glob(glob_pattern): - extension_path = extension_path.relative_to(base_dir_path) + app_dir = AppLocation.get_directory(AppLocation.AppDir) + for extension_path in app_dir.glob(glob_pattern): + extension_path = extension_path.relative_to(app_dir) if extension_path.name in excluded_files: continue module_name = path_to_module(extension_path) @@ -106,21 +103,19 @@ except (ImportError, OSError): # On some platforms importing vlc.py might cause OSError exceptions. (e.g. Mac OS X) log.warning('Failed to import {module_name} on path {extension_path}' - .format(module_name=module_name, extension_path=str(extension_path))) + .format(module_name=module_name, extension_path=extension_path)) def path_to_module(path): """ Convert a path to a module name (i.e openlp.core.common) - :param path: The path to convert to a module name. - :type path: Path - + :param pathlib.Path path: The path to convert to a module name. :return: The module name. :rtype: str """ module_path = path.with_suffix('') - return '.'.join(module_path.parts) + return 'openlp.' + '.'.join(module_path.parts) def get_frozen_path(frozen_option, non_frozen_option): @@ -378,20 +373,22 @@ return os.path.split(path) -def delete_file(file_path_name): +def delete_file(file_path): """ Deletes a file from the system. - :param file_path_name: The file, including path, to delete. + :param pathlib.Path file_path: The file, including path, to delete. + :return: True if the deletion was successful, or the file never existed. False otherwise. + :rtype: bool """ - if not file_path_name: + if not file_path: return False try: - if os.path.exists(file_path_name): - os.remove(file_path_name) + if file_path.exists(): + file_path.unlink() return True except (IOError, OSError): - log.exception("Unable to delete file {text}".format(text=file_path_name)) + log.exception('Unable to delete file {file_path}'.format(file_path=file_path)) return False @@ -411,18 +408,19 @@ return IMAGES_FILTER -def is_not_image_file(file_name): +def is_not_image_file(file_path): """ Validate that the file is not an image file. - :param file_name: File name to be checked. + :param pathlib.Path file_path: The file to be checked. + :return: If the file is not an image + :rtype: bool """ - if not file_name: + if not (file_path and file_path.exists()): return True else: formats = [bytes(fmt).decode().lower() for fmt in QtGui.QImageReader.supportedImageFormats()] - file_part, file_extension = os.path.splitext(str(file_name)) - if file_extension[1:].lower() in formats and os.path.exists(file_name): + if file_path.suffix[1:].lower() in formats: return False return True @@ -431,10 +429,10 @@ """ Removes invalid characters from the given ``filename``. - :param filename: The "dirty" file name to clean. + :param str filename: The "dirty" file name to clean. + :return: The cleaned string + :rtype: str """ - if not isinstance(filename, str): - filename = str(filename, 'utf-8') return INVALID_FILE_CHARS.sub('_', CONTROL_CHARS.sub('', filename)) @@ -442,8 +440,9 @@ """ Function that checks whether a binary exists. - :param program_path: The full path to the binary to check. + :param pathlib.Path program_path: The full path to the binary to check. :return: program output to be parsed + :rtype: bytes """ log.debug('testing program_path: {text}'.format(text=program_path)) try: @@ -453,26 +452,27 @@ startupinfo.dwFlags |= STARTF_USESHOWWINDOW else: startupinfo = None - runlog = check_output([program_path, '--help'], stderr=STDOUT, startupinfo=startupinfo) + run_log = check_output([str(program_path), '--help'], stderr=STDOUT, startupinfo=startupinfo) except CalledProcessError as e: - runlog = e.output + run_log = e.output except Exception: trace_error_handler(log) - runlog = '' - log.debug('check_output returned: {text}'.format(text=runlog)) - return runlog - - -def get_file_encoding(filename): + run_log = '' + log.debug('check_output returned: {text}'.format(text=run_log)) + return run_log + + +def get_file_encoding(file_path): """ Utility function to incrementally detect the file encoding. - :param filename: Filename for the file to determine the encoding for. Str + :param pathlib.Path file_path: Filename for the file to determine the encoding for. :return: A dict with the keys 'encoding' and 'confidence' + :rtype: dict[str, float] """ detector = UniversalDetector() try: - with open(filename, 'rb') as detect_file: + with file_path.open('rb') as detect_file: while not detector.done: chunk = detect_file.read(1024) if not chunk: === modified file 'openlp/core/common/applocation.py' --- openlp/core/common/applocation.py 2017-08-02 06:09:38 +0000 +++ openlp/core/common/applocation.py 2017-08-23 20:29:44 +0000 @@ -58,9 +58,6 @@ CacheDir = 5 LanguageDir = 6 - # Base path where data/config/cache dir is located - BaseDir = None - @staticmethod def get_directory(dir_type=AppDir): """ @@ -78,8 +75,6 @@ return get_frozen_path(FROZEN_APP_PATH, APP_PATH) / 'plugins' elif dir_type == AppLocation.LanguageDir: return get_frozen_path(FROZEN_APP_PATH, _get_os_dir_path(dir_type)) / 'i18n' - elif dir_type == AppLocation.DataDir and AppLocation.BaseDir: - return Path(AppLocation.BaseDir, 'data') else: return _get_os_dir_path(dir_type) @@ -96,7 +91,7 @@ path = Path(Settings().value('advanced/data path')) else: path = AppLocation.get_directory(AppLocation.DataDir) - check_directory_exists(str(path)) + check_directory_exists(path) return path @staticmethod @@ -104,14 +99,10 @@ """ Get a list of files from the data files path. - :param section: Defaults to *None*. The section of code getting the files - used to load from a section's data - subdirectory. - :type section: None | str - - :param extension: Defaults to ''. The extension to search for. For example:: + :param None | str section: Defaults to *None*. The section of code getting the files - used to load from a + section's data subdirectory. + :param str extension: Defaults to ''. The extension to search for. For example:: '.png' - :type extension: str - :return: List of files found. :rtype: list[pathlib.Path] """ @@ -134,7 +125,7 @@ :rtype: pathlib.Path """ path = AppLocation.get_data_path() / section - check_directory_exists(str(path)) + check_directory_exists(path) return path @@ -143,14 +134,12 @@ Return a path based on which OS and environment we are running in. :param dir_type: AppLocation Enum of the requested path type - :type dir_type: AppLocation Enum - :return: The requested path :rtype: pathlib.Path """ # If running from source, return the language directory from the source directory if dir_type == AppLocation.LanguageDir: - directory = Path(os.path.abspath(os.path.join(os.path.dirname(openlp.__file__), '..', 'resources'))) + directory = Path(openlp.__file__, '..', '..').resolve() / 'resources' if directory.exists(): return directory if is_win(): @@ -158,14 +147,14 @@ if dir_type == AppLocation.DataDir: return openlp_folder_path / 'data' elif dir_type == AppLocation.LanguageDir: - return os.path.dirname(openlp.__file__) + return Path(openlp.__file__).parent return openlp_folder_path elif is_macosx(): openlp_folder_path = Path(os.getenv('HOME'), 'Library', 'Application Support', 'openlp') if dir_type == AppLocation.DataDir: return openlp_folder_path / 'Data' elif dir_type == AppLocation.LanguageDir: - return os.path.dirname(openlp.__file__) + return Path(openlp.__file__).parent return openlp_folder_path else: if dir_type == AppLocation.LanguageDir: === modified file 'openlp/core/common/settings.py' --- openlp/core/common/settings.py 2017-08-23 20:21:11 +0000 +++ openlp/core/common/settings.py 2017-08-23 20:29:44 +0000 @@ -502,31 +502,3 @@ if isinstance(default_value, int): return int(setting) return setting - - def get_files_from_config(self, plugin): - """ - This removes the settings needed for old way we saved files (e. g. the image paths for the image plugin). A list - of file paths are returned. - - **Note**: Only a list of paths is returned; this does not convert anything! - - :param plugin: The Plugin object.The caller has to convert/save the list himself; o - """ - files_list = [] - # We need QSettings instead of Settings here to bypass our central settings dict. - # Do NOT do this anywhere else! - settings = QtCore.QSettings(self.fileName(), Settings.IniFormat) - settings.beginGroup(plugin.settings_section) - if settings.contains('{name} count'.format(name=plugin.name)): - # Get the count. - list_count = int(settings.value('{name} count'.format(name=plugin.name), 0)) - if list_count: - for counter in range(list_count): - # The keys were named e. g.: "image 0" - item = settings.value('{name} {counter:d}'.format(name=plugin.name, counter=counter), '') - if item: - files_list.append(item) - settings.remove('{name} {counter:d}'.format(name=plugin.name, counter=counter)) - settings.remove('{name} count'.format(name=plugin.name)) - settings.endGroup() - return files_list === modified file 'openlp/core/lib/__init__.py' --- openlp/core/lib/__init__.py 2017-08-23 20:21:11 +0000 +++ openlp/core/lib/__init__.py 2017-08-23 20:29:44 +0000 @@ -83,30 +83,28 @@ Next = 3 -def get_text_file_string(text_file): +def get_text_file_string(text_file_path): """ - Open a file and return its content as unicode string. If the supplied file name is not a file then the function + Open a file and return its content as a string. If the supplied file path is not a file then the function returns False. If there is an error loading the file or the content can't be decoded then the function will return None. - :param text_file: The name of the file. - :return: The file as a single string + :param pathlib.Path text_file_path: The path to the file. + :return: The contents of the file, False if the file does not exist, or None if there is an Error reading or + decoding the file. + :rtype: str | False | None """ - if not os.path.isfile(text_file): + if not text_file_path.is_file(): return False - file_handle = None content = None try: - file_handle = open(text_file, 'r', encoding='utf-8') - if file_handle.read(3) != '\xEF\xBB\xBF': - # no BOM was found - file_handle.seek(0) - content = file_handle.read() + with text_file_path.open('r', encoding='utf-8') as file_handle: + if file_handle.read(3) != '\xEF\xBB\xBF': + # no BOM was found + file_handle.seek(0) + content = file_handle.read() except (IOError, UnicodeError): - log.exception('Failed to open text file {text}'.format(text=text_file)) - finally: - if file_handle: - file_handle.close() + log.exception('Failed to open text file {text}'.format(text=text_file_path)) return content === modified file 'openlp/core/lib/db.py' --- openlp/core/lib/db.py 2017-08-01 20:59:41 +0000 +++ openlp/core/lib/db.py 2017-08-23 20:29:44 +0000 @@ -274,9 +274,9 @@ :param db_file_name: The database file name. Defaults to None resulting in the plugin_name being used. """ if db_file_name: - db_file_path = os.path.join(str(AppLocation.get_section_data_path(plugin_name)), db_file_name) + db_file_path = AppLocation.get_section_data_path(plugin_name) / db_file_name else: - db_file_path = os.path.join(str(AppLocation.get_section_data_path(plugin_name)), plugin_name) + db_file_path = AppLocation.get_section_data_path(plugin_name) / plugin_name return delete_file(db_file_path) === modified file 'openlp/core/lib/pluginmanager.py' --- openlp/core/lib/pluginmanager.py 2017-08-01 20:59:41 +0000 +++ openlp/core/lib/pluginmanager.py 2017-08-23 20:29:44 +0000 @@ -69,7 +69,7 @@ """ Scan a directory for objects inheriting from the ``Plugin`` class. """ - glob_pattern = os.path.join('openlp', 'plugins', '*', '*plugin.py') + glob_pattern = os.path.join('plugins', '*', '*plugin.py') extension_loader(glob_pattern) plugin_classes = Plugin.__subclasses__() plugin_objects = [] === modified file 'openlp/core/lib/theme.py' --- openlp/core/lib/theme.py 2017-08-01 20:59:41 +0000 +++ openlp/core/lib/theme.py 2017-08-23 20:29:44 +0000 @@ -158,9 +158,8 @@ Initialise the theme object. """ # basic theme object with defaults - json_dir = os.path.join(str(AppLocation.get_directory(AppLocation.AppDir)), 'core', 'lib', 'json') - json_file = os.path.join(json_dir, 'theme.json') - jsn = get_text_file_string(json_file) + json_path = AppLocation.get_directory(AppLocation.AppDir) / 'core' / 'lib' / 'json' / 'theme.json' + jsn = get_text_file_string(json_path) jsn = json.loads(jsn) self.expand_json(jsn) self.background_filename = '' === modified file 'openlp/core/ui/firsttimeform.py' --- openlp/core/ui/firsttimeform.py 2017-08-03 17:54:40 +0000 +++ openlp/core/ui/firsttimeform.py 2017-08-23 20:29:44 +0000 @@ -29,8 +29,9 @@ import urllib.request import urllib.parse import urllib.error +from configparser import ConfigParser, MissingSectionHeaderError, NoOptionError, NoSectionError +from pathlib import Path from tempfile import gettempdir -from configparser import ConfigParser, MissingSectionHeaderError, NoSectionError, NoOptionError from PyQt5 import QtCore, QtWidgets @@ -282,7 +283,7 @@ self.no_internet_cancel_button.setVisible(False) # Check if this is a re-run of the wizard. self.has_run_wizard = Settings().value('core/has run wizard') - check_directory_exists(os.path.join(gettempdir(), 'openlp')) + check_directory_exists(Path(gettempdir(), 'openlp')) def update_screen_list_combo(self): """ === modified file 'openlp/core/ui/mainwindow.py' --- openlp/core/ui/mainwindow.py 2017-08-03 17:54:40 +0000 +++ openlp/core/ui/mainwindow.py 2017-08-23 20:29:44 +0000 @@ -30,6 +30,7 @@ from datetime import datetime from distutils import dir_util from distutils.errors import DistutilsFileError +from pathlib import Path from tempfile import gettempdir from PyQt5 import QtCore, QtGui, QtWidgets @@ -870,7 +871,7 @@ setting_sections.extend([plugin.name for plugin in self.plugin_manager.plugins]) # Copy the settings file to the tmp dir, because we do not want to change the original one. temp_directory = os.path.join(str(gettempdir()), 'openlp') - check_directory_exists(temp_directory) + check_directory_exists(Path(temp_directory)) temp_config = os.path.join(temp_directory, os.path.basename(import_file_name)) shutil.copyfile(import_file_name, temp_config) settings = Settings() === modified file 'openlp/core/ui/media/mediacontroller.py' --- openlp/core/ui/media/mediacontroller.py 2017-06-25 17:03:31 +0000 +++ openlp/core/ui/media/mediacontroller.py 2017-08-23 20:29:44 +0000 @@ -177,7 +177,7 @@ Check to see if we have any media Player's available. """ log.debug('_check_available_media_players') - controller_dir = os.path.join('openlp', 'core', 'ui', 'media') + controller_dir = os.path.join('core', 'ui', 'media') glob_pattern = os.path.join(controller_dir, '*player.py') extension_loader(glob_pattern, ['mediaplayer.py']) player_classes = MediaPlayer.__subclasses__() === modified file 'openlp/core/ui/printserviceform.py' --- openlp/core/ui/printserviceform.py 2017-08-01 20:59:41 +0000 +++ openlp/core/ui/printserviceform.py 2017-08-23 20:29:44 +0000 @@ -176,7 +176,7 @@ html_data = self._add_element('html') self._add_element('head', parent=html_data) self._add_element('title', self.title_line_edit.text(), html_data.head) - css_path = os.path.join(str(AppLocation.get_data_path()), 'serviceprint', 'service_print.css') + css_path = AppLocation.get_data_path() / 'serviceprint' / 'service_print.css' custom_css = get_text_file_string(css_path) if not custom_css: custom_css = DEFAULT_CSS === modified file 'openlp/core/ui/servicemanager.py' --- openlp/core/ui/servicemanager.py 2017-08-03 17:54:40 +0000 +++ openlp/core/ui/servicemanager.py 2017-08-23 20:29:44 +0000 @@ -28,6 +28,7 @@ import shutil import zipfile from datetime import datetime, timedelta +from pathlib import Path from tempfile import mkstemp from PyQt5 import QtCore, QtGui, QtWidgets @@ -587,7 +588,7 @@ audio_from = os.path.join(self.service_path, audio_from) save_file = os.path.join(self.service_path, audio_to) save_path = os.path.split(save_file)[0] - check_directory_exists(save_path) + check_directory_exists(Path(save_path)) if not os.path.exists(save_file): shutil.copy(audio_from, save_file) zip_file.write(audio_from, audio_to) @@ -614,7 +615,7 @@ success = False self.main_window.add_recent_file(path_file_name) self.set_modified(False) - delete_file(temp_file_name) + delete_file(Path(temp_file_name)) return success def save_local_file(self): @@ -669,7 +670,7 @@ return self.save_file_as() self.main_window.add_recent_file(path_file_name) self.set_modified(False) - delete_file(temp_file_name) + delete_file(Path(temp_file_name)) return success def save_file_as(self, field=None): @@ -774,7 +775,7 @@ self.set_file_name(file_name) self.main_window.display_progress_bar(len(items)) self.process_service_items(items) - delete_file(p_file) + delete_file(Path(p_file)) self.main_window.add_recent_file(file_name) self.set_modified(False) Settings().setValue('servicemanager/last file', file_name) @@ -1343,7 +1344,7 @@ Empties the service_path of temporary files on system exit. """ for file_name in os.listdir(self.service_path): - file_path = os.path.join(self.service_path, file_name) + file_path = Path(self.service_path, file_name) delete_file(file_path) if os.path.exists(os.path.join(self.service_path, 'audio')): shutil.rmtree(os.path.join(self.service_path, 'audio'), True) === modified file 'openlp/core/ui/themeform.py' --- openlp/core/ui/themeform.py 2017-08-07 20:50:01 +0000 +++ openlp/core/ui/themeform.py 2017-08-23 20:29:44 +0000 @@ -24,6 +24,7 @@ """ import logging import os +from pathlib import Path from PyQt5 import QtCore, QtGui, QtWidgets @@ -188,7 +189,8 @@ """ background_image = BackgroundType.to_string(BackgroundType.Image) if self.page(self.currentId()) == self.background_page and \ - self.theme.background_type == background_image and is_not_image_file(self.theme.background_filename): + self.theme.background_type == background_image and \ + is_not_image_file(Path(self.theme.background_filename)): QtWidgets.QMessageBox.critical(self, translate('OpenLP.ThemeWizard', 'Background Image Empty'), translate('OpenLP.ThemeWizard', 'You have not selected a ' 'background image. Please select one before continuing.')) === modified file 'openlp/core/ui/thememanager.py' --- openlp/core/ui/thememanager.py 2017-08-07 20:50:01 +0000 +++ openlp/core/ui/thememanager.py 2017-08-23 20:29:44 +0000 @@ -25,6 +25,7 @@ import os import zipfile import shutil +from pathlib import Path from xml.etree.ElementTree import ElementTree, XML from PyQt5 import QtCore, QtGui, QtWidgets @@ -161,9 +162,9 @@ Set up the theme path variables """ self.path = str(AppLocation.get_section_data_path(self.settings_section)) - check_directory_exists(self.path) + check_directory_exists(Path(self.path)) self.thumb_path = os.path.join(self.path, 'thumbnails') - check_directory_exists(self.thumb_path) + check_directory_exists(Path(self.thumb_path)) def check_list_state(self, item, field=None): """ @@ -355,8 +356,8 @@ """ self.theme_list.remove(theme) thumb = '{name}.png'.format(name=theme) - delete_file(os.path.join(self.path, thumb)) - delete_file(os.path.join(self.thumb_path, thumb)) + delete_file(Path(self.path, thumb)) + delete_file(Path(self.thumb_path, thumb)) try: # Windows is always unicode, so no need to encode filenames if is_win(): @@ -450,7 +451,7 @@ for theme_file in files: theme_file = os.path.join(self.path, str(theme_file)) self.unzip_theme(theme_file, self.path) - delete_file(theme_file) + delete_file(Path(theme_file)) files = AppLocation.get_files(self.settings_section, '.png') # No themes have been found so create one if not files: @@ -514,12 +515,12 @@ :return: The theme object. """ self.log_debug('get theme data for theme {name}'.format(name=theme_name)) - theme_file = os.path.join(self.path, str(theme_name), str(theme_name) + '.json') - theme_data = get_text_file_string(theme_file) + theme_file_path = Path(self.path, str(theme_name), '{file_name}.json'.format(file_name=theme_name)) + theme_data = get_text_file_string(theme_file_path) jsn = True if not theme_data: - theme_file = os.path.join(self.path, str(theme_name), str(theme_name) + '.xml') - theme_data = get_text_file_string(theme_file) + theme_file_path = theme_file_path.with_suffix('.xml') + theme_data = get_text_file_string(theme_file_path) jsn = False if not theme_data: self.log_debug('No theme data - using default theme') @@ -592,7 +593,7 @@ # is directory or preview file continue full_name = os.path.join(directory, out_name) - check_directory_exists(os.path.dirname(full_name)) + check_directory_exists(Path(os.path.dirname(full_name))) if os.path.splitext(name)[1].lower() == '.xml' or os.path.splitext(name)[1].lower() == '.json': file_xml = str(theme_zip.read(name), 'utf-8') out_file = open(full_name, 'w', encoding='utf-8') @@ -670,10 +671,10 @@ name = theme.theme_name theme_pretty = theme.export_theme() theme_dir = os.path.join(self.path, name) - check_directory_exists(theme_dir) + check_directory_exists(Path(theme_dir)) theme_file = os.path.join(theme_dir, name + '.json') if self.old_background_image and image_to != self.old_background_image: - delete_file(self.old_background_image) + delete_file(Path(self.old_background_image)) out_file = None try: out_file = open(theme_file, 'w', encoding='utf-8') === modified file 'openlp/plugins/bibles/lib/importers/csvbible.py' --- openlp/plugins/bibles/lib/importers/csvbible.py 2016-12-31 11:01:36 +0000 +++ openlp/plugins/bibles/lib/importers/csvbible.py 2017-08-23 20:29:44 +0000 @@ -51,6 +51,7 @@ """ import csv from collections import namedtuple +from pathlib import Path from openlp.core.common import get_file_encoding, translate from openlp.core.lib.exceptions import ValidationError @@ -100,7 +101,7 @@ :return: An iterable yielding namedtuples of type results_tuple """ try: - encoding = get_file_encoding(filename)['encoding'] + encoding = get_file_encoding(Path(filename))['encoding'] with open(filename, 'r', encoding=encoding, newline='') as csv_file: csv_reader = csv.reader(csv_file, delimiter=',', quotechar='"') return [results_tuple(*line) for line in csv_reader] === modified file 'openlp/plugins/bibles/lib/manager.py' --- openlp/plugins/bibles/lib/manager.py 2017-08-01 20:59:41 +0000 +++ openlp/plugins/bibles/lib/manager.py 2017-08-23 20:29:44 +0000 @@ -22,6 +22,7 @@ import logging import os +from pathlib import Path from openlp.core.common import AppLocation, OpenLPMixin, RegistryProperties, Settings, translate, delete_file, UiStrings from openlp.plugins.bibles.lib import LanguageSelection, parse_reference @@ -137,7 +138,7 @@ # Remove corrupted files. if name is None: bible.session.close_all() - delete_file(os.path.join(self.path, filename)) + delete_file(Path(self.path, filename)) continue log.debug('Bible Name: "{name}"'.format(name=name)) self.db_cache[name] = bible @@ -185,7 +186,7 @@ bible = self.db_cache[name] bible.session.close_all() bible.session = None - return delete_file(os.path.join(bible.path, bible.file)) + return delete_file(Path(bible.path, bible.file)) def get_bibles(self): """ === modified file 'openlp/plugins/images/lib/mediaitem.py' --- openlp/plugins/images/lib/mediaitem.py 2017-08-03 17:54:40 +0000 +++ openlp/plugins/images/lib/mediaitem.py 2017-08-23 20:29:44 +0000 @@ -22,6 +22,7 @@ import logging import os +from pathlib import Path from PyQt5 import QtCore, QtGui, QtWidgets @@ -99,7 +100,7 @@ self.list_view.setIndentation(self.list_view.default_indentation) self.list_view.allow_internal_dnd = True self.service_path = os.path.join(str(AppLocation.get_section_data_path(self.settings_section)), 'thumbnails') - check_directory_exists(self.service_path) + check_directory_exists(Path(self.service_path)) # Load images from the database self.load_full_list( self.manager.get_all_objects(ImageFilenames, order_by_ref=ImageFilenames.filename), initial_load=True) @@ -210,8 +211,8 @@ """ images = self.manager.get_all_objects(ImageFilenames, ImageFilenames.group_id == image_group.id) for image in images: - delete_file(os.path.join(self.service_path, os.path.split(image.filename)[1])) - delete_file(self.generate_thumbnail_path(image)) + delete_file(Path(self.service_path, os.path.split(image.filename)[1])) + delete_file(Path(self.generate_thumbnail_path(image))) self.manager.delete_object(ImageFilenames, image.id) image_groups = self.manager.get_all_objects(ImageGroups, ImageGroups.parent_id == image_group.id) for group in image_groups: @@ -233,8 +234,8 @@ if row_item: item_data = row_item.data(0, QtCore.Qt.UserRole) if isinstance(item_data, ImageFilenames): - delete_file(os.path.join(self.service_path, row_item.text(0))) - delete_file(self.generate_thumbnail_path(item_data)) + delete_file(Path(self.service_path, row_item.text(0))) + delete_file(Path(self.generate_thumbnail_path(item_data))) if item_data.group_id == 0: self.list_view.takeTopLevelItem(self.list_view.indexOfTopLevelItem(row_item)) else: === modified file 'openlp/plugins/media/lib/mediaitem.py' --- openlp/plugins/media/lib/mediaitem.py 2017-08-03 17:54:40 +0000 +++ openlp/plugins/media/lib/mediaitem.py 2017-08-23 20:29:44 +0000 @@ -22,6 +22,7 @@ import logging import os +from pathlib import Path from PyQt5 import QtCore, QtWidgets @@ -301,7 +302,7 @@ """ self.list_view.clear() self.service_path = os.path.join(str(AppLocation.get_section_data_path(self.settings_section)), 'thumbnails') - check_directory_exists(self.service_path) + check_directory_exists(Path(self.service_path)) self.load_list(Settings().value(self.settings_section + '/media files')) self.rebuild_players() === modified file 'openlp/plugins/media/mediaplugin.py' --- openlp/plugins/media/mediaplugin.py 2017-08-03 17:54:40 +0000 +++ openlp/plugins/media/mediaplugin.py 2017-08-23 20:29:44 +0000 @@ -26,6 +26,7 @@ import logging import os import re +from pathlib import Path from PyQt5 import QtCore @@ -165,8 +166,7 @@ :param program_path:The full path to the binary to check. :return: If exists or not """ - program_type = None - runlog = check_binary_exists(program_path) + runlog = check_binary_exists(Path(program_path)) # Analyse the output to see it the program is mediainfo for line in runlog.splitlines(): decoded_line = line.decode() === modified file 'openlp/plugins/presentations/lib/impresscontroller.py' --- openlp/plugins/presentations/lib/impresscontroller.py 2017-05-14 10:11:10 +0000 +++ openlp/plugins/presentations/lib/impresscontroller.py 2017-08-23 20:29:44 +0000 @@ -34,6 +34,7 @@ import logging import os import time +from pathlib import Path from openlp.core.common import is_win, Registry, get_uno_command, get_uno_instance, delete_file @@ -275,7 +276,7 @@ try: doc.storeToURL(url_path, properties) self.convert_thumbnail(path, index + 1) - delete_file(path) + delete_file(Path(path)) except ErrorCodeIOException as exception: log.exception('ERROR! ErrorCodeIOException {error:d}'.format(error=exception.ErrCode)) except: === modified file 'openlp/plugins/presentations/lib/pdfcontroller.py' --- openlp/plugins/presentations/lib/pdfcontroller.py 2017-08-01 20:59:41 +0000 +++ openlp/plugins/presentations/lib/pdfcontroller.py 2017-08-23 20:29:44 +0000 @@ -23,6 +23,7 @@ import os import logging import re +from pathlib import Path from shutil import which from subprocess import check_output, CalledProcessError @@ -69,7 +70,7 @@ :return: Type of the binary, 'gs' if ghostscript, 'mudraw' if mudraw, None if invalid. """ program_type = None - runlog = check_binary_exists(program_path) + runlog = check_binary_exists(Path(program_path)) # Analyse the output to see it the program is mudraw, ghostscript or neither for line in runlog.splitlines(): decoded_line = line.decode() === modified file 'openlp/plugins/presentations/lib/presentationcontroller.py' --- openlp/plugins/presentations/lib/presentationcontroller.py 2017-08-01 20:59:41 +0000 +++ openlp/plugins/presentations/lib/presentationcontroller.py 2017-08-23 20:29:44 +0000 @@ -23,6 +23,7 @@ import logging import os import shutil +from pathlib import Path from PyQt5 import QtCore @@ -98,7 +99,7 @@ """ self.slide_number = 0 self.file_path = name - check_directory_exists(self.get_thumbnail_folder()) + check_directory_exists(Path(self.get_thumbnail_folder())) def load_presentation(self): """ @@ -419,8 +420,8 @@ self.thumbnail_folder = os.path.join( str(AppLocation.get_section_data_path(self.settings_section)), 'thumbnails') self.thumbnail_prefix = 'slide' - check_directory_exists(self.thumbnail_folder) - check_directory_exists(self.temp_folder) + check_directory_exists(Path(self.thumbnail_folder)) + check_directory_exists(Path(self.temp_folder)) def enabled(self): """ === modified file 'openlp/plugins/presentations/presentationplugin.py' --- openlp/plugins/presentations/presentationplugin.py 2017-06-09 15:56:40 +0000 +++ openlp/plugins/presentations/presentationplugin.py 2017-08-23 20:29:44 +0000 @@ -125,7 +125,7 @@ Check to see if we have any presentation software available. If not do not install the plugin. """ log.debug('check_pre_conditions') - controller_dir = os.path.join('openlp', 'plugins', 'presentations', 'lib') + controller_dir = os.path.join('plugins', 'presentations', 'lib') glob_pattern = os.path.join(controller_dir, '*controller.py') extension_loader(glob_pattern, ['presentationcontroller.py']) controller_classes = PresentationController.__subclasses__() === modified file 'openlp/plugins/songs/forms/editsongform.py' --- openlp/plugins/songs/forms/editsongform.py 2017-08-11 20:47:52 +0000 +++ openlp/plugins/songs/forms/editsongform.py 2017-08-23 20:29:44 +0000 @@ -1071,7 +1071,7 @@ log.debug(audio_files) save_path = os.path.join(str(AppLocation.get_section_data_path(self.media_item.plugin.name)), 'audio', str(self.song.id)) - check_directory_exists(save_path) + check_directory_exists(Path(save_path)) self.song.media_files = [] files = [] for row in range(self.audio_list_widget.count()): === modified file 'openlp/plugins/songs/lib/importers/songbeamer.py' --- openlp/plugins/songs/lib/importers/songbeamer.py 2017-05-11 19:53:47 +0000 +++ openlp/plugins/songs/lib/importers/songbeamer.py 2017-08-23 20:29:44 +0000 @@ -27,6 +27,7 @@ import re import base64 import math +from pathlib import Path from openlp.plugins.songs.lib import VerseType from openlp.plugins.songs.lib.importers.songimport import SongImport @@ -122,7 +123,7 @@ file_name = os.path.split(import_file)[1] if os.path.isfile(import_file): # Detect the encoding - self.input_file_encoding = get_file_encoding(import_file)['encoding'] + self.input_file_encoding = get_file_encoding(Path(import_file))['encoding'] # The encoding should only be ANSI (cp1252), UTF-8, Unicode, Big-Endian-Unicode. # So if it doesn't start with 'u' we default to cp1252. See: # https://forum.songbeamer.com/viewtopic.php?p=419&sid=ca4814924e37c11e4438b7272a98b6f2 === modified file 'openlp/plugins/songs/lib/importers/songimport.py' --- openlp/plugins/songs/lib/importers/songimport.py 2017-08-01 20:59:41 +0000 +++ openlp/plugins/songs/lib/importers/songimport.py 2017-08-23 20:29:44 +0000 @@ -24,6 +24,7 @@ import re import shutil import os +from pathlib import Path from PyQt5 import QtCore @@ -423,7 +424,7 @@ if not hasattr(self, 'save_path'): self.save_path = os.path.join(str(AppLocation.get_section_data_path(self.import_wizard.plugin.name)), 'audio', str(song_id)) - check_directory_exists(self.save_path) + check_directory_exists(Path(self.save_path)) if not filename.startswith(self.save_path): old_file, filename = filename, os.path.join(self.save_path, os.path.split(filename)[1]) shutil.copyfile(old_file, filename) === modified file 'openlp/plugins/songs/lib/mediaitem.py' --- openlp/plugins/songs/lib/mediaitem.py 2017-08-03 17:54:40 +0000 +++ openlp/plugins/songs/lib/mediaitem.py 2017-08-23 20:29:44 +0000 @@ -23,6 +23,7 @@ import logging import os import shutil +from pathlib import Path from PyQt5 import QtCore, QtWidgets from sqlalchemy.sql import and_, or_ @@ -89,7 +90,7 @@ for i, bga in enumerate(item.background_audio): dest_file = os.path.join( str(AppLocation.get_section_data_path(self.plugin.name)), 'audio', str(song.id), os.path.split(bga)[1]) - check_directory_exists(os.path.split(dest_file)[0]) + check_directory_exists(Path(os.path.split(dest_file)[0])) shutil.copyfile(os.path.join(str(AppLocation.get_section_data_path('servicemanager')), bga), dest_file) song.media_files.append(MediaFile.populate(weight=i, file_name=dest_file)) self.plugin.manager.save_object(song, True) @@ -535,7 +536,7 @@ if len(old_song.media_files) > 0: save_path = os.path.join( str(AppLocation.get_section_data_path(self.plugin.name)), 'audio', str(new_song.id)) - check_directory_exists(save_path) + check_directory_exists(Path(save_path)) for media_file in old_song.media_files: new_media_file_name = os.path.join(save_path, os.path.basename(media_file.file_name)) shutil.copyfile(media_file.file_name, new_media_file_name) === modified file 'openlp/plugins/songs/lib/openlyricsexport.py' --- openlp/plugins/songs/lib/openlyricsexport.py 2016-12-31 11:01:36 +0000 +++ openlp/plugins/songs/lib/openlyricsexport.py 2017-08-23 20:29:44 +0000 @@ -25,6 +25,7 @@ """ import logging import os +from pathlib import Path from lxml import etree @@ -47,7 +48,7 @@ self.manager = parent.plugin.manager self.songs = songs self.save_path = save_path - check_directory_exists(self.save_path) + check_directory_exists(Path(self.save_path)) def do_export(self): """ === modified file 'openlp/plugins/songusage/forms/songusagedetailform.py' --- openlp/plugins/songusage/forms/songusagedetailform.py 2017-08-04 17:40:57 +0000 +++ openlp/plugins/songusage/forms/songusagedetailform.py 2017-08-23 20:29:44 +0000 @@ -22,6 +22,7 @@ import logging import os +from pathlib import Path from PyQt5 import QtCore, QtWidgets from sqlalchemy.sql import and_ @@ -78,7 +79,7 @@ ' song usage report. \nPlease select an existing path on your computer.') ) return - check_directory_exists(path) + check_directory_exists(Path(path)) file_name = translate('SongUsagePlugin.SongUsageDetailForm', 'usage_detail_{old}_{new}.txt' ).format(old=self.from_date_calendar.selectedDate().toString('ddMMyyyy'), === modified file 'tests/functional/openlp_core_common/test_applocation.py' --- tests/functional/openlp_core_common/test_applocation.py 2017-08-01 20:59:41 +0000 +++ tests/functional/openlp_core_common/test_applocation.py 2017-08-23 20:29:44 +0000 @@ -43,14 +43,12 @@ """ with patch('openlp.core.common.applocation.Settings') as mocked_class, \ patch('openlp.core.common.AppLocation.get_directory') as mocked_get_directory, \ - patch('openlp.core.common.applocation.check_directory_exists') as mocked_check_directory_exists, \ - patch('openlp.core.common.applocation.os') as mocked_os: + patch('openlp.core.common.applocation.check_directory_exists') as mocked_check_directory_exists: # GIVEN: A mocked out Settings class and a mocked out AppLocation.get_directory() mocked_settings = mocked_class.return_value mocked_settings.contains.return_value = False - mocked_get_directory.return_value = os.path.join('test', 'dir') + mocked_get_directory.return_value = Path('test', 'dir') mocked_check_directory_exists.return_value = True - mocked_os.path.normpath.return_value = os.path.join('test', 'dir') # WHEN: we call AppLocation.get_data_path() data_path = AppLocation.get_data_path() @@ -58,8 +56,8 @@ # THEN: check that all the correct methods were called, and the result is correct mocked_settings.contains.assert_called_with('advanced/data path') mocked_get_directory.assert_called_with(AppLocation.DataDir) - mocked_check_directory_exists.assert_called_with(os.path.join('test', 'dir')) - self.assertEqual(os.path.join('test', 'dir'), data_path, 'Result should be "test/dir"') + mocked_check_directory_exists.assert_called_with(Path('test', 'dir')) + self.assertEqual(Path('test', 'dir'), data_path, 'Result should be "test/dir"') def test_get_data_path_with_custom_location(self): """ @@ -125,7 +123,7 @@ data_path = AppLocation.get_section_data_path('section') # THEN: check that all the correct methods were called, and the result is correct - mocked_check_directory_exists.assert_called_with(os.path.join('test', 'dir', 'section')) + mocked_check_directory_exists.assert_called_with(Path('test', 'dir', 'section')) self.assertEqual(Path('test', 'dir', 'section'), data_path, 'Result should be "test/dir/section"') def test_get_directory_for_app_dir(self): === modified file 'tests/functional/openlp_core_common/test_common.py' --- tests/functional/openlp_core_common/test_common.py 2017-08-01 20:59:41 +0000 +++ tests/functional/openlp_core_common/test_common.py 2017-08-23 20:29:44 +0000 @@ -35,44 +35,70 @@ """ A test suite to test out various functions in the openlp.core.common module. """ - def test_check_directory_exists(self): - """ - Test the check_directory_exists() function - """ - with patch('openlp.core.lib.os.path.exists') as mocked_exists, \ - patch('openlp.core.lib.os.makedirs') as mocked_makedirs: - # GIVEN: A directory to check and a mocked out os.makedirs and os.path.exists - directory_to_check = 'existing/directory' + def test_check_directory_exists_dir_exists(self): + """ + Test the check_directory_exists() function when the path already exists + """ + # GIVEN: A `Path` to check with patched out mkdir and exists methods + with patch.object(Path, 'exists') as mocked_exists, \ + patch.object(Path, 'mkdir') as mocked_mkdir, \ + patch('openlp.core.common.log'): - # WHEN: os.path.exists returns True and we check to see if the directory exists + # WHEN: `check_directory_exists` is called and the path exists mocked_exists.return_value = True - check_directory_exists(directory_to_check) - - # THEN: Only os.path.exists should have been called - mocked_exists.assert_called_with(directory_to_check) - self.assertIsNot(mocked_makedirs.called, 'os.makedirs should not have been called') - - # WHEN: os.path.exists returns False and we check the directory exists + check_directory_exists(Path('existing', 'directory')) + + # THEN: The function should not attempt to create the directory + mocked_exists.assert_called_with() + self.assertFalse(mocked_mkdir.called) + + def test_check_directory_exists_dir_doesnt_exists(self): + """ + Test the check_directory_exists() function when the path does not already exist + """ + # GIVEN: A `Path` to check with patched out mkdir and exists methods + with patch.object(Path, 'exists') as mocked_exists, \ + patch.object(Path, 'mkdir') as mocked_mkdir, \ + patch('openlp.core.common.log'): + + # WHEN: `check_directory_exists` is called and the path does not exist mocked_exists.return_value = False - check_directory_exists(directory_to_check) - - # THEN: Both the mocked functions should have been called - mocked_exists.assert_called_with(directory_to_check) - mocked_makedirs.assert_called_with(directory_to_check) - - # WHEN: os.path.exists raises an IOError + check_directory_exists(Path('existing', 'directory')) + + # THEN: The directory should have been created + mocked_exists.assert_called_with() + mocked_mkdir.assert_called_with(parents=True) + + def test_check_directory_exists_dir_io_error(self): + """ + Test the check_directory_exists() when an IOError is raised + """ + # GIVEN: A `Path` to check with patched out mkdir and exists methods + with patch.object(Path, 'exists') as mocked_exists, \ + patch.object(Path, 'mkdir'), \ + patch('openlp.core.common.log') as mocked_logger: + + # WHEN: An IOError is raised when checking the if the path exists. mocked_exists.side_effect = IOError() - check_directory_exists(directory_to_check) - - # THEN: We shouldn't get an exception though the mocked exists has been called - mocked_exists.assert_called_with(directory_to_check) + check_directory_exists(Path('existing', 'directory')) + + # THEN: The Error should have been logged + mocked_logger.exception.assert_called_once_with('failed to check if directory exists or create directory') + + def test_check_directory_exists_dir_value_error(self): + """ + Test the check_directory_exists() when an error other than IOError is raised + """ + # GIVEN: A `Path` to check with patched out mkdir and exists methods + with patch.object(Path, 'exists') as mocked_exists, \ + patch.object(Path, 'mkdir'), \ + patch('openlp.core.common.log'): # WHEN: Some other exception is raised mocked_exists.side_effect = ValueError() - # THEN: check_directory_exists raises an exception - mocked_exists.assert_called_with(directory_to_check) - self.assertRaises(ValueError, check_directory_exists, directory_to_check) + # THEN: `check_directory_exists` raises an exception + self.assertRaises(ValueError, check_directory_exists, Path('existing', 'directory')) def test_extension_loader_no_files_found(self): """ @@ -80,7 +106,7 @@ """ # GIVEN: A mocked `Path.glob` method which does not match any files with patch('openlp.core.common.AppLocation.get_directory', return_value=Path('/', 'app', 'dir', 'openlp')), \ - patch.object(common.Path, 'glob', return_value=[]), \ + patch.object(Path, 'glob', return_value=[]), \ patch('openlp.core.common.importlib.import_module') as mocked_import_module: # WHEN: Calling `extension_loader` @@ -95,7 +121,7 @@ """ # GIVEN: A mocked `Path.glob` method which returns a list of files with patch('openlp.core.common.AppLocation.get_directory', return_value=Path('/', 'app', 'dir', 'openlp')), \ - patch.object(common.Path, 'glob', return_value=[ + patch.object(Path, 'glob', return_value=[ Path('/', 'app', 'dir', 'openlp', 'import_dir', 'file1.py'), Path('/', 'app', 'dir', 'openlp', 'import_dir', 'file2.py'), Path('/', 'app', 'dir', 'openlp', 'import_dir', 'file3.py'), @@ -115,7 +141,7 @@ """ # GIVEN: A mocked `import_module` which raises an `ImportError` with patch('openlp.core.common.AppLocation.get_directory', return_value=Path('/', 'app', 'dir', 'openlp')), \ - patch.object(common.Path, 'glob', return_value=[ + patch.object(Path, 'glob', return_value=[ Path('/', 'app', 'dir', 'openlp', 'import_dir', 'file1.py')]), \ patch('openlp.core.common.importlib.import_module', side_effect=ImportError()), \ patch('openlp.core.common.log') as mocked_logger: @@ -132,7 +158,7 @@ """ # GIVEN: A mocked `SourceFileLoader` which raises an `OSError` with patch('openlp.core.common.AppLocation.get_directory', return_value=Path('/', 'app', 'dir', 'openlp')), \ - patch.object(common.Path, 'glob', return_value=[ + patch.object(Path, 'glob', return_value=[ Path('/', 'app', 'dir', 'openlp', 'import_dir', 'file1.py')]), \ patch('openlp.core.common.importlib.import_module', side_effect=OSError()), \ patch('openlp.core.common.log') as mocked_logger: @@ -174,7 +200,7 @@ Test `path_to_module` when supplied with a `Path` object """ # GIVEN: A `Path` object - path = Path('openlp/core/ui/media/webkitplayer.py') + path = Path('core', 'ui', 'media', 'webkitplayer.py') # WHEN: Calling path_to_module with the `Path` object result = path_to_module(path) === modified file 'tests/functional/openlp_core_common/test_init.py' --- tests/functional/openlp_core_common/test_init.py 2017-04-24 05:17:55 +0000 +++ tests/functional/openlp_core_common/test_init.py 2017-08-23 20:29:44 +0000 @@ -24,6 +24,7 @@ """ import os from io import BytesIO +from pathlib import Path from unittest import TestCase from unittest.mock import MagicMock, PropertyMock, call, patch @@ -296,10 +297,10 @@ """ # GIVEN: A blank path # WEHN: Calling delete_file - result = delete_file('') + result = delete_file(None) # THEN: delete_file should return False - self.assertFalse(result, "delete_file should return False when called with ''") + self.assertFalse(result, "delete_file should return False when called with None") def test_delete_file_path_success(self): """ @@ -309,84 +310,87 @@ with patch('openlp.core.common.os', **{'path.exists.return_value': False}): # WHEN: Calling delete_file with a file path - result = delete_file('path/file.ext') + result = delete_file(Path('path', 'file.ext')) # THEN: delete_file should return True self.assertTrue(result, 'delete_file should return True when it successfully deletes a file') def test_delete_file_path_no_file_exists(self): """ - Test the delete_file function when the file to remove does not exist + Test the `delete_file` function when the file to remove does not exist """ - # GIVEN: A mocked os which returns False when os.path.exists is called - with patch('openlp.core.common.os', **{'path.exists.return_value': False}): - - # WHEN: Calling delete_file with a file path - result = delete_file('path/file.ext') - - # THEN: delete_file should return True + # GIVEN: A patched `exists` methods on the Path object, which returns False + with patch.object(Path, 'exists', return_value=False), \ + patch.object(Path, 'unlink') as mocked_unlink: + + # WHEN: Calling `delete_file with` a file path + result = delete_file(Path('path', 'file.ext')) + + # THEN: The function should not attempt to delete the file and it should return True + self.assertFalse(mocked_unlink.called) self.assertTrue(result, 'delete_file should return True when the file doesnt exist') def test_delete_file_path_exception(self): """ - Test the delete_file function when os.remove raises an exception + Test the delete_file function when an exception is raised """ - # GIVEN: A mocked os which returns True when os.path.exists is called and raises an OSError when os.remove is + # GIVEN: A test `Path` object with a patched exists method which raises an OSError # called. - with patch('openlp.core.common.os', **{'path.exists.return_value': True, 'path.exists.side_effect': OSError}), \ + with patch.object(Path, 'exists') as mocked_exists, \ patch('openlp.core.common.log') as mocked_log: - - # WHEN: Calling delete_file with a file path - result = delete_file('path/file.ext') - - # THEN: delete_file should log and exception and return False - self.assertEqual(mocked_log.exception.call_count, 1) - self.assertFalse(result, 'delete_file should return False when os.remove raises an OSError') - - def test_get_file_name_encoding_done_test(self): + mocked_exists.side_effect = OSError + + # WHEN: Calling delete_file with a the test Path object + result = delete_file(Path('path', 'file.ext')) + + # THEN: The exception should be logged and `delete_file` should return False + self.assertTrue(mocked_log.exception.called) + self.assertFalse(result, 'delete_file should return False when an OSError is raised') + + def test_get_file_encoding_done_test(self): """ Test get_file_encoding when the detector sets done to True """ # GIVEN: A mocked UniversalDetector instance with done attribute set to True after first iteration with patch('openlp.core.common.UniversalDetector') as mocked_universal_detector, \ - patch('builtins.open', return_value=BytesIO(b"data" * 260)) as mocked_open: + patch.object(Path, 'open', return_value=BytesIO(b"data" * 260)) as mocked_open: encoding_result = {'encoding': 'UTF-8', 'confidence': 0.99} mocked_universal_detector_inst = MagicMock(result=encoding_result) type(mocked_universal_detector_inst).done = PropertyMock(side_effect=[False, True]) mocked_universal_detector.return_value = mocked_universal_detector_inst # WHEN: Calling get_file_encoding - result = get_file_encoding('file name') + result = get_file_encoding(Path('file name')) # THEN: The feed method of UniversalDetector should only br called once before returning a result - mocked_open.assert_called_once_with('file name', 'rb') + mocked_open.assert_called_once_with('rb') self.assertEqual(mocked_universal_detector_inst.feed.mock_calls, [call(b"data" * 256)]) mocked_universal_detector_inst.close.assert_called_once_with() self.assertEqual(result, encoding_result) - def test_get_file_name_encoding_eof_test(self): + def test_get_file_encoding_eof_test(self): """ Test get_file_encoding when the end of the file is reached """ # GIVEN: A mocked UniversalDetector instance which isn't set to done and a mocked open, with 1040 bytes of test # data (enough to run the iterator twice) with patch('openlp.core.common.UniversalDetector') as mocked_universal_detector, \ - patch('builtins.open', return_value=BytesIO(b"data" * 260)) as mocked_open: + patch.object(Path, 'open', return_value=BytesIO(b"data" * 260)) as mocked_open: encoding_result = {'encoding': 'UTF-8', 'confidence': 0.99} mocked_universal_detector_inst = MagicMock(mock=mocked_universal_detector, **{'done': False, 'result': encoding_result}) mocked_universal_detector.return_value = mocked_universal_detector_inst # WHEN: Calling get_file_encoding - result = get_file_encoding('file name') + result = get_file_encoding(Path('file name')) # THEN: The feed method of UniversalDetector should have been called twice before returning a result - mocked_open.assert_called_once_with('file name', 'rb') + mocked_open.assert_called_once_with('rb') self.assertEqual(mocked_universal_detector_inst.feed.mock_calls, [call(b"data" * 256), call(b"data" * 4)]) mocked_universal_detector_inst.close.assert_called_once_with() self.assertEqual(result, encoding_result) - def test_get_file_name_encoding_oserror_test(self): + def test_get_file_encoding_oserror_test(self): """ Test get_file_encoding when the end of the file is reached """ @@ -397,7 +401,7 @@ patch('openlp.core.common.log') as mocked_log: # WHEN: Calling get_file_encoding - result = get_file_encoding('file name') + result = get_file_encoding(Path('file name')) # THEN: log.exception should be called and get_file_encoding should return None mocked_log.exception.assert_called_once_with('Error detecting file encoding') === modified file 'tests/functional/openlp_core_lib/test_db.py' --- tests/functional/openlp_core_lib/test_db.py 2017-06-09 13:45:18 +0000 +++ tests/functional/openlp_core_lib/test_db.py 2017-08-23 20:29:44 +0000 @@ -24,6 +24,7 @@ """ import os import shutil +from pathlib import Path from tempfile import mkdtemp from unittest import TestCase @@ -129,10 +130,10 @@ # GIVEN: Mocked out AppLocation class and delete_file method, a test plugin name and a db location with patch('openlp.core.lib.db.AppLocation') as MockedAppLocation, \ patch('openlp.core.lib.db.delete_file') as mocked_delete_file: - MockedAppLocation.get_section_data_path.return_value = 'test-dir' + MockedAppLocation.get_section_data_path.return_value = Path('test-dir') mocked_delete_file.return_value = True test_plugin = 'test' - test_location = os.path.join('test-dir', test_plugin) + test_location = Path('test-dir', test_plugin) # WHEN: delete_database is run without a database file result = delete_database(test_plugin) @@ -149,11 +150,11 @@ # GIVEN: Mocked out AppLocation class and delete_file method, a test plugin name and a db location with patch('openlp.core.lib.db.AppLocation') as MockedAppLocation, \ patch('openlp.core.lib.db.delete_file') as mocked_delete_file: - MockedAppLocation.get_section_data_path.return_value = 'test-dir' + MockedAppLocation.get_section_data_path.return_value = Path('test-dir') mocked_delete_file.return_value = False test_plugin = 'test' test_db_file = 'mydb.sqlite' - test_location = os.path.join('test-dir', test_db_file) + test_location = Path('test-dir', test_db_file) # WHEN: delete_database is run without a database file result = delete_database(test_plugin, test_db_file) === removed file 'tests/functional/openlp_core_lib/test_file_dialog.py' --- tests/functional/openlp_core_lib/test_file_dialog.py 2017-08-07 21:12:42 +0000 +++ tests/functional/openlp_core_lib/test_file_dialog.py 1970-01-01 00:00:00 +0000 @@ -1,45 +0,0 @@ -# -*- coding: utf-8 -*- -# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4 - -############################################################################### -# OpenLP - Open Source Lyrics Projection # -# --------------------------------------------------------------------------- # -# Copyright (c) 2008-2017 OpenLP Developers # -# --------------------------------------------------------------------------- # -# This program is free software; you can redistribute it and/or modify it # -# under the terms of the GNU General Public License as published by the Free # -# Software Foundation; version 2 of the License. # -# # -# This program is distributed in the hope that it will be useful, but WITHOUT # -# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or # -# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for # -# more details. # -# # -# You should have received a copy of the GNU General Public License along # -# with this program; if not, write to the Free Software Foundation, Inc., 59 # -# Temple Place, Suite 330, Boston, MA 02111-1307 USA # -############################################################################### -""" -Package to test the openlp.core.ui.lib.filedialog package. -""" -from unittest import TestCase -from unittest.mock import MagicMock, patch - - -class TestFileDialog(TestCase): - """ - Test the functions in the :mod:`filedialog` module. - """ - def setUp(self): - self.os_patcher = patch('openlp.core.ui.lib.filedialog.os') - self.qt_gui_patcher = patch('openlp.core.ui.lib.filedialog.QtWidgets') - self.ui_strings_patcher = patch('openlp.core.ui.lib.filedialog.UiStrings') - self.mocked_os = self.os_patcher.start() - self.mocked_qt_gui = self.qt_gui_patcher.start() - self.mocked_ui_strings = self.ui_strings_patcher.start() - self.mocked_parent = MagicMock() - - def tearDown(self): - self.os_patcher.stop() - self.qt_gui_patcher.stop() - self.ui_strings_patcher.stop() === modified file 'tests/functional/openlp_core_lib/test_lib.py' --- tests/functional/openlp_core_lib/test_lib.py 2017-08-23 20:21:11 +0000 +++ tests/functional/openlp_core_lib/test_lib.py 2017-08-23 20:29:44 +0000 @@ -24,6 +24,7 @@ """ import os from datetime import datetime, timedelta +from pathlib import Path from unittest import TestCase from unittest.mock import MagicMock, patch @@ -148,35 +149,34 @@ """ Test the get_text_file_string() function when a file does not exist """ - with patch('openlp.core.lib.os.path.isfile') as mocked_isfile: - # GIVEN: A mocked out isfile which returns true, and a text file name - filename = 'testfile.txt' - mocked_isfile.return_value = False + # GIVEN: A patched is_file which returns False, and a file path + with patch.object(Path, 'is_file', return_value=False): + file_path = Path('testfile.txt') # WHEN: get_text_file_string is called - result = get_text_file_string(filename) + result = get_text_file_string(file_path) # THEN: The result should be False - mocked_isfile.assert_called_with(filename) + file_path.is_file.assert_called_with() self.assertFalse(result, 'False should be returned if no file exists') def test_get_text_file_string_read_error(self): """ Test the get_text_file_string() method when a read error happens """ - with patch('openlp.core.lib.os.path.isfile') as mocked_isfile, \ - patch('openlp.core.lib.open', create=True) as mocked_open: - # GIVEN: A mocked-out open() which raises an exception and isfile returns True - filename = 'testfile.txt' - mocked_isfile.return_value = True - mocked_open.side_effect = IOError() + # GIVEN: A patched open which raises an exception and is_file which returns True + with patch.object(Path, 'is_file'), \ + patch.object(Path, 'open'): + file_path = Path('testfile.txt') + file_path.is_file.return_value = True + file_path.open.side_effect = IOError() # WHEN: get_text_file_string is called - result = get_text_file_string(filename) + result = get_text_file_string(file_path) # THEN: None should be returned - mocked_isfile.assert_called_with(filename) - mocked_open.assert_called_with(filename, 'r', encoding='utf-8') + file_path.is_file.assert_called_once_with() + file_path.open.assert_called_once_with('r', encoding='utf-8') self.assertIsNone(result, 'None should be returned if the file cannot be opened') def test_get_text_file_string_decode_error(self): === modified file 'tests/functional/openlp_core_ui/test_firsttimeform.py' --- tests/functional/openlp_core_ui/test_firsttimeform.py 2017-04-24 05:17:55 +0000 +++ tests/functional/openlp_core_ui/test_firsttimeform.py 2017-08-23 20:29:44 +0000 @@ -25,6 +25,7 @@ import os import tempfile import urllib +from pathlib import Path from unittest import TestCase from unittest.mock import MagicMock, patch @@ -116,7 +117,7 @@ mocked_settings.value.return_value = True MockedSettings.return_value = mocked_settings mocked_gettempdir.return_value = 'temp' - expected_temp_path = os.path.join('temp', 'openlp') + expected_temp_path = Path('temp', 'openlp') # WHEN: The set_defaults() method is run frw.set_defaults() === modified file 'tests/functional/openlp_core_ui/test_thememanager.py' --- tests/functional/openlp_core_ui/test_thememanager.py 2017-06-01 06:18:47 +0000 +++ tests/functional/openlp_core_ui/test_thememanager.py 2017-08-23 20:29:44 +0000 @@ -90,7 +90,7 @@ # theme, check_directory_exists and thememanager-attributes. with patch('builtins.open') as mocked_open, \ patch('openlp.core.ui.thememanager.shutil.copyfile') as mocked_copyfile, \ - patch('openlp.core.ui.thememanager.check_directory_exists') as mocked_check_directory_exists: + patch('openlp.core.ui.thememanager.check_directory_exists'): mocked_open.return_value = MagicMock() theme_manager = ThemeManager(None) theme_manager.old_background_image = None @@ -118,7 +118,7 @@ # theme, check_directory_exists and thememanager-attributes. with patch('builtins.open') as mocked_open, \ patch('openlp.core.ui.thememanager.shutil.copyfile') as mocked_copyfile, \ - patch('openlp.core.ui.thememanager.check_directory_exists') as mocked_check_directory_exists: + patch('openlp.core.ui.thememanager.check_directory_exists'): mocked_open.return_value = MagicMock() theme_manager = ThemeManager(None) theme_manager.old_background_image = None === modified file 'tests/functional/openlp_plugins/bibles/test_manager.py' --- tests/functional/openlp_plugins/bibles/test_manager.py 2016-12-31 11:01:36 +0000 +++ tests/functional/openlp_plugins/bibles/test_manager.py 2017-08-23 20:29:44 +0000 @@ -22,6 +22,7 @@ """ This module contains tests for the manager submodule of the Bibles plugin. """ +from pathlib import Path from unittest import TestCase from unittest.mock import MagicMock, patch @@ -50,7 +51,6 @@ """ # GIVEN: An instance of BibleManager and a mocked bible with patch.object(BibleManager, 'reload_bibles'), \ - patch('openlp.plugins.bibles.lib.manager.os.path.join', side_effect=lambda x, y: '{}/{}'.format(x, y)),\ patch('openlp.plugins.bibles.lib.manager.delete_file', return_value=True) as mocked_delete_file: instance = BibleManager(MagicMock()) # We need to keep a reference to the mock for close_all as it gets set to None later on! @@ -66,4 +66,4 @@ self.assertTrue(result) mocked_close_all.assert_called_once_with() self.assertIsNone(mocked_bible.session) - mocked_delete_file.assert_called_once_with('bibles/KJV.sqlite') + mocked_delete_file.assert_called_once_with(Path('bibles', 'KJV.sqlite')) === modified file 'tests/functional/openlp_plugins/media/test_mediaplugin.py' --- tests/functional/openlp_plugins/media/test_mediaplugin.py 2017-06-06 20:58:12 +0000 +++ tests/functional/openlp_plugins/media/test_mediaplugin.py 2017-08-23 20:29:44 +0000 @@ -38,20 +38,18 @@ def setUp(self): Registry.create() - @patch(u'openlp.plugins.media.mediaplugin.Plugin.initialise') + @patch('openlp.plugins.media.mediaplugin.Plugin.initialise') def test_initialise(self, mocked_initialise): """ Test that the initialise() method overwrites the built-in one, but still calls it """ - # GIVEN: A media plugin instance and a mocked settings object + # GIVEN: A media plugin instance media_plugin = MediaPlugin() - mocked_settings = MagicMock() - mocked_settings.get_files_from_config.return_value = True # Not the real value, just need something "true-ish" # WHEN: initialise() is called media_plugin.initialise() - # THEN: The settings should be upgraded and the base initialise() method should be called + # THEN: The the base initialise() method should be called mocked_initialise.assert_called_with() def test_about_text(self): === modified file 'tests/functional/openlp_plugins/presentations/test_presentationcontroller.py' --- tests/functional/openlp_plugins/presentations/test_presentationcontroller.py 2017-05-30 18:42:35 +0000 +++ tests/functional/openlp_plugins/presentations/test_presentationcontroller.py 2017-08-23 20:29:44 +0000 @@ -24,6 +24,7 @@ classes and related methods. """ import os +from pathlib import Path from unittest import TestCase from unittest.mock import MagicMock, mock_open, patch @@ -38,7 +39,8 @@ """ def setUp(self): self.get_thumbnail_folder_patcher = \ - patch('openlp.plugins.presentations.lib.presentationcontroller.PresentationDocument.get_thumbnail_folder') + patch('openlp.plugins.presentations.lib.presentationcontroller.PresentationDocument.get_thumbnail_folder', + return_value=Path()) self.get_thumbnail_folder_patcher.start() mocked_plugin = MagicMock() mocked_plugin.settings_section = 'presentations' @@ -225,7 +227,7 @@ PresentationDocument(self.mock_controller, 'Name') # THEN: check_directory_exists should have been called with 'returned/path/' - self.mock_check_directory_exists.assert_called_once_with('returned/path/') + self.mock_check_directory_exists.assert_called_once_with(Path('returned', 'path')) self._setup_patcher.start() === modified file 'tests/interfaces/openlp_core_common/test_utils.py' --- tests/interfaces/openlp_core_common/test_utils.py 2016-12-31 11:01:36 +0000 +++ tests/interfaces/openlp_core_common/test_utils.py 2017-08-23 20:29:44 +0000 @@ -22,7 +22,7 @@ """ Functional tests to test the AppLocation class and related methods. """ -import os +from pathlib import Path from unittest import TestCase from openlp.core.common import is_not_image_file @@ -59,7 +59,7 @@ Test the method handles an image file """ # Given and empty string - file_name = os.path.join(TEST_RESOURCES_PATH, 'church.jpg') + file_name = Path(TEST_RESOURCES_PATH, 'church.jpg') # WHEN testing for it result = is_not_image_file(file_name) @@ -72,7 +72,7 @@ Test the method handles a non image file """ # Given and empty string - file_name = os.path.join(TEST_RESOURCES_PATH, 'serviceitem_custom_1.osj') + file_name = Path(TEST_RESOURCES_PATH, 'serviceitem_custom_1.osj') # WHEN testing for it result = is_not_image_file(file_name)
_______________________________________________ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp