HEAD

nose2.cfg

@@ -1,5 +1,5 @@
 [unittest]
-verbose = true
+verbose = True
 plugins = nose2.plugins.mp
 
 [log-capture]
@@ -9,14 +9,19 @@ filter = -nose
 log-level = ERROR
 
 [test-result]
-always-on = true
-descriptions = true
+always-on = True
+descriptions = True
 
 [coverage]
-always-on = true
+always-on = True
 coverage = openlp
 coverage-report = html
 
 [multiprocess]
-always-on = false
+always-on = False
 processes = 4
+
+[output-buffer]
+always-on = True
+stderr = True
+stdout = False

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

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:

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

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

openlp/core/api/endpoint/remote.py

@@ -27,7 +27,7 @@ from openlp.core.api.endpoint.core import TRANSLATED_STRINGS
 
 log = logging.getLogger(__name__)
 
-remote_endpoint = Endpoint('remote', template_dir='remotes', static_dir='remotes')
+remote_endpoint = Endpoint('remote', template_dir='remotes')
 
 
 @remote_endpoint.route('{view}')

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)

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

openlp/core/common/__init__.py

@@ -43,9 +43,13 @@ log = logging.getLogger(__name__ + '.__init__')
 
 FIRST_CAMEL_REGEX = re.compile('(.)([A-Z][a-z]+)')
 SECOND_CAMEL_REGEX = re.compile('([a-z0-9])([A-Z])')
-CONTROL_CHARS = re.compile(r'[\x00-\x1F\x7F-\x9F]', re.UNICODE)
-INVALID_FILE_CHARS = re.compile(r'[\\/:\*\?"<>\|\+\[\]%]', re.UNICODE)
+CONTROL_CHARS = re.compile(r'[\x00-\x1F\x7F-\x9F]')
+INVALID_FILE_CHARS = re.compile(r'[\\/:\*\?"<>\|\+\[\]%]')
 IMAGES_FILTER = None
+REPLACMENT_CHARS_MAP = str.maketrans({'\u2018': '\'', '\u2019': '\'', '\u201c': '"', '\u201d': '"', '\u2026': '...',
+                                      '\u2013': '-', '\u2014': '-', '\v': '\n\n', '\f': '\n\n'})
+NEW_LINE_REGEX = re.compile(r' ?(\r\n?|\n) ?')
+WHITESPACE_REGEX = re.compile(r'[ \t]+')
 
 
 def trace_error_handler(logger):
@@ -314,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.
@@ -339,7 +332,7 @@ def delete_file(file_path):
         if file_path.exists():
             file_path.unlink()
         return True
-    except (IOError, OSError):
+    except OSError:
         log.exception('Unable to delete file {file_path}'.format(file_path=file_path))
         return False
 
@@ -436,3 +429,17 @@ def get_file_encoding(file_path):
         return detector.result
     except OSError:
         log.exception('Error detecting file encoding')
+
+
+def normalize_str(irreg_str):
+    """
+    Normalize the supplied string. Remove unicode control chars and tidy up white space.
+
+    :param str irreg_str: The string to normalize.
+    :return: The normalized string
+    :rtype: str
+    """
+    irreg_str = irreg_str.translate(REPLACMENT_CHARS_MAP)
+    irreg_str = CONTROL_CHARS.sub('', irreg_str)
+    irreg_str = NEW_LINE_REGEX.sub('\n', irreg_str)
+    return WHITESPACE_REGEX.sub(' ', irreg_str)

openlp/core/common/applocation.py

@@ -83,7 +83,7 @@ class AppLocation(object):
         """
         # Check if we have a different data location.
         if Settings().contains('advanced/data path'):
-            path = Settings().value('advanced/data path')
+            path = Path(Settings().value('advanced/data path'))
         else:
             path = AppLocation.get_directory(AppLocation.DataDir)
             create_paths(path)

openlp/core/common/httputils.py

@@ -97,8 +97,8 @@ def get_web_page(url, headers=None, update_openlp=False, proxies=None):
             response = requests.get(url, headers=headers, proxies=proxies, timeout=float(CONNECTION_TIMEOUT))
             log.debug('Downloaded page {url}'.format(url=response.url))
             break
-        except IOError:
-            # For now, catch IOError. All requests errors inherit from IOError
+        except OSError:
+            # For now, catch OSError. All requests errors inherit from OSError
             log.exception('Unable to connect to {url}'.format(url=url))
             response = None
             if retries >= CONNECTION_RETRIES:
@@ -127,7 +127,7 @@ def get_url_file_size(url):
         try:
             response = requests.head(url, timeout=float(CONNECTION_TIMEOUT), allow_redirects=True)
             return int(response.headers['Content-Length'])
-        except IOError:
+        except OSError:
             if retries > CONNECTION_RETRIES:
                 raise ConnectionError('Unable to download {url}'.format(url=url))
             else:
@@ -173,7 +173,7 @@ def url_get_file(callback, url, file_path, sha256=None):
                     file_path.unlink()
                 return False
             break
-        except IOError:
+        except OSError:
             trace_error_handler(log)
             if retries > CONNECTION_RETRIES:
                 if file_path.exists():

openlp/core/common/i18n.py

@@ -53,7 +53,7 @@ def translate(context, text, comment=None, qt_translate=QtCore.QCoreApplication.
 
 Language = namedtuple('Language', ['id', 'name', 'code'])
 ICU_COLLATOR = None
-DIGITS_OR_NONDIGITS = re.compile(r'\d+|\D+', re.UNICODE)
+DIGITS_OR_NONDIGITS = re.compile(r'\d+|\D+')
 LANGUAGES = sorted([
     Language(1, translate('common.languages', '(Afan) Oromo', 'Language code: om'), 'om'),
     Language(2, translate('common.languages', 'Abkhazian', 'Language code: ab'), 'ab'),

openlp/core/common/mixins.py

@@ -101,6 +101,20 @@ class RegistryProperties(object):
     """
     This adds registry components to classes to use at run time.
     """
+    _application = None
+    _plugin_manager = None
+    _image_manager = None
+    _media_controller = None
+    _service_manager = None
+    _preview_controller = None
+    _live_controller = None
+    _main_window = None
+    _renderer = None
+    _theme_manager = None
+    _settings_form = None
+    _alerts_manager = None
+    _projector_manager = None
+
     @property
     def application(self):
         """

openlp/core/common/path.py

@@ -69,6 +69,16 @@ class Path(PathVariant):
                 path = path.relative_to(base_path)
         return {'__Path__': path.parts}
 
+    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):
     """
@@ -153,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.
@@ -233,7 +226,7 @@ def create_paths(*paths, **kwargs):
         try:
             if not path.exists():
                 path.mkdir(parents=True)
-        except IOError:
+        except OSError:
             if not kwargs.get('do_not_log', False):
                 log.exception('failed to check if directory exists or create directory')
 

openlp/core/common/registry.py

@@ -151,8 +151,9 @@ class Registry(object):
                     trace_error_handler(log)
                     log.exception('Exception for function {function}'.format(function=function))
         else:
-            trace_error_handler(log)
-            log.exception('Event {event} called but not registered'.format(event=event))
+            if log.getEffectiveLevel() == logging.DEBUG:
+                trace_error_handler(log)
+                log.exception('Event {event} called but not registered'.format(event=event))
         return results
 
     def get_flag(self, key):

openlp/core/common/settings.py

@@ -40,7 +40,7 @@ __version__ = 2
 
 # Fix for bug #1014422.
 X11_BYPASS_DEFAULT = True
-if is_linux():
+if is_linux():                                                                              # pragma: no cover
     # Default to False on Gnome.
     X11_BYPASS_DEFAULT = bool(not os.environ.get('GNOME_DESKTOP_SESSION_ID'))
     # Default to False on Xfce.
@@ -230,11 +230,14 @@ class Settings(QtCore.QSettings):
         'projector/source dialog type': 0  # Source select dialog box type
     }
     __file_path__ = ''
+    # Settings upgrades prior to 3.0
     __setting_upgrade_1__ = [
-        # Changed during 2.2.x development.
         ('songs/search as type', 'advanced/search as type', []),
         ('media/players', 'media/players_temp', [(media_players_conv, None)]),  # Convert phonon to system
         ('media/players_temp', 'media/players', []),  # Move temp setting from above to correct setting
+    ]
+    # Settings upgrades for 3.0 (aka 2.6)
+    __setting_upgrade_2__ = [
         ('advanced/default color', 'core/logo background color', []),  # Default image renamed + moved to general > 2.4.
         ('advanced/default image', 'core/logo file', []),  # Default image renamed + moved to general after 2.4.
         ('remotes/https enabled', '', []),
@@ -255,9 +258,7 @@ class Settings(QtCore.QSettings):
         # Last search type was renamed to last used search type in 2.6 since Bible search value type changed in 2.6.
         ('songs/last search type', 'songs/last used search type', []),
         ('bibles/last search type', '', []),
-        ('custom/last search type', 'custom/last used search type', [])]
-
-    __setting_upgrade_2__ = [
+        ('custom/last search type', 'custom/last used search type', []),
         # The following changes are being made for the conversion to using Path objects made in 2.6 development
         ('advanced/data path', 'advanced/data path', [(str_to_path, None)]),
         ('crashreport/last directory', 'crashreport/last directory', [(str_to_path, None)]),
@@ -280,6 +281,9 @@ class Settings(QtCore.QSettings):
         ('presentations/last directory', 'presentations/last directory', [(str_to_path, None)]),
         ('images/last directory', 'images/last directory', [(str_to_path, None)]),
         ('media/last directory', 'media/last directory', [(str_to_path, None)]),
+        ('songuasge/db password', 'songusage/db password', []),
+        ('songuasge/db hostname', 'songusage/db hostname', []),
+        ('songuasge/db database', 'songusage/db database', []),
         (['core/monitor', 'core/x position', 'core/y position', 'core/height', 'core/width', 'core/override',
           'core/display on monitor'], 'core/monitors', [(upgrade_monitor, [1, 0, 0, None, None, False, False])])
     ]
@@ -490,32 +494,38 @@ class Settings(QtCore.QSettings):
         for version in range(current_version, __version__):
             version += 1
             upgrade_list = getattr(self, '__setting_upgrade_{version}__'.format(version=version))
-            for old_key, new_key, rules in upgrade_list:
+            for old_keys, new_key, rules in upgrade_list:
                 # Once removed we don't have to do this again. - Can be removed once fully switched to the versioning
                 # system.
-                if not self.contains(old_key):
+                if not isinstance(old_keys, (tuple, list)):
+                    old_keys = [old_keys]
+                if any([not self.contains(old_key) for old_key in old_keys]):
+                    log.warning('One of {} does not exist, skipping upgrade'.format(old_keys))
                     continue
                 if new_key:
                     # Get the value of the old_key.
-                    old_value = super(Settings, self).value(old_key)
+                    old_values = [super(Settings, self).value(old_key) for old_key in old_keys]
                     # When we want to convert the value, we have to figure out the default value (because we cannot get
                     # the default value from the central settings dict.
                     if rules:
-                        default_value = rules[0][1]
-                        old_value = self._convert_value(old_value, default_value)
+                        default_values = rules[0][1]
+                        if not isinstance(default_values, (list, tuple)):
+                            default_values = [default_values]
+                        old_values = [self._convert_value(old_value, default_value)
+                                      for old_value, default_value in zip(old_values, default_values)]
                     # Iterate over our rules and check what the old_value should be "converted" to.
-                    for new, old in rules:
+                    new_value = None
+                    for new_rule, old_rule in rules:
                         # If the value matches with the condition (rule), then use the provided value. This is used to
                         # convert values. E. g. an old value 1 results in True, and 0 in False.
-                        if callable(new):
-                            old_value = new(old_value)
-                        elif old == old_value:
-                            old_value = new
+                        if callable(new_rule):
+                            new_value = new_rule(*old_values)
+                        elif old_rule in old_values:
+                            new_value = new_rule
                             break
-                    self.setValue(new_key, old_value)
-                if new_key != old_key:
-                    self.remove(old_key)
-        self.setValue('settings/version', version)
+                    self.setValue(new_key, new_value)
+                [self.remove(old_key) for old_key in old_keys if old_key != new_key]
+            self.setValue('settings/version', version)
 
     def value(self, key):
         """

openlp/core/lib/__init__.py

@@ -99,7 +99,7 @@ def get_text_file_string(text_file_path):
                 # no BOM was found
                 file_handle.seek(0)
             content = file_handle.read()
-    except (IOError, UnicodeError):
+    except (OSError, UnicodeError):
         log.exception('Failed to open text file {text}'.format(text=text_file_path))
     return content
 
@@ -174,8 +174,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():
@@ -202,10 +203,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)
@@ -312,6 +313,3 @@ from .serviceitem import ServiceItem, ServiceItemType, ItemCapabilities
 from .htmlbuilder import build_html, build_lyrics_format_css, build_lyrics_outline_css, build_chords_css
 from .imagemanager import ImageManager
 from .mediamanageritem import MediaManagerItem
-from .projector.db import ProjectorDB, Projector
-from .projector.pjlink import PJLink
-from .projector.constants import PJLINK_PORT, ERROR_MSG, ERROR_STRING

openlp/core/lib/mediamanageritem.py

@@ -92,7 +92,7 @@ class MediaManagerItem(QtWidgets.QWidget, RegistryProperties):
         Run some initial setup. This method is separate from __init__ in order to mock it out in tests.
         """
         self.hide()
-        self.whitespace = re.compile(r'[\W_]+', re.UNICODE)
+        self.whitespace = re.compile(r'[\W_]+')
         visible_title = self.plugin.get_string(StringContent.VisibleName)
         self.title = str(visible_title['title'])
         Registry().register(self.plugin.name, self)
@@ -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,21 +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)
+            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:
-            self.validate_and_load(new_files, data['target'])
+                new_file_paths.append(file_path)
+        if new_file_paths:
+            if 'target' in data:
+                self.validate_and_load(new_file_paths, data['target'])
+            self.validate_and_load(new_file_paths)
 
     def dnd_move_internal(self, target):
         """
@@ -354,12 +357,12 @@ 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 = []
@@ -367,18 +370,17 @@ class MediaManagerItem(QtWidgets.QWidget, RegistryProperties):
             full_list.append(self.list_view.item(count).data(QtCore.Qt.UserRole))
         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,

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

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

openlp/core/projectors/__init__.py

@@ -20,11 +20,13 @@
 # Temple Place, Suite 330, Boston, MA 02111-1307 USA                          #
 ###############################################################################
 """
-    :mod:`openlp.core.ui.projector`
+    :mod:`openlp.core.projectors`
 
-    Initialization for the openlp.core.ui.projector modules.
+    Initialization for the openlp.core.projectors modules.
 """
 
+from openlp.core.projectors.constants import PJLINK_PORT, ERROR_MSG, ERROR_STRING
+
 
 class DialogSourceStyle(object):
     """

openlp/core/projectors/db.py

@@ -43,8 +43,8 @@ from sqlalchemy.ext.declarative import declarative_base, declared_attr
 from sqlalchemy.orm import relationship
 
 from openlp.core.lib.db import Manager, init_db, init_url
-from openlp.core.lib.projector.constants import PJLINK_DEFAULT_CODES
-from openlp.core.lib.projector import upgrade
+from openlp.core.projectors.constants import PJLINK_DEFAULT_CODES
+from openlp.core.projectors import upgrade
 
 Base = declarative_base(MetaData())
 

openlp/core/projectors/editform.py

@@ -30,8 +30,8 @@ from PyQt5 import QtCore, QtWidgets
 from openlp.core.common import verify_ip_address
 from openlp.core.common.i18n import translate
 from openlp.core.lib import build_icon
-from openlp.core.lib.projector.db import Projector
-from openlp.core.lib.projector.constants import PJLINK_PORT
+from openlp.core.projectors.db import Projector
+from openlp.core.projectors.constants import PJLINK_PORT
 
 log = logging.getLogger(__name__)
 log.debug('editform loaded')

openlp/core/projectors/manager.py

@@ -34,14 +34,14 @@ from openlp.core.common.mixins import LogMixin, RegistryProperties
 from openlp.core.common.registry import RegistryBase
 from openlp.core.common.settings import Settings
 from openlp.core.lib.ui import create_widget_action
-from openlp.core.lib.projector import DialogSourceStyle
-from openlp.core.lib.projector.constants import ERROR_MSG, ERROR_STRING, E_AUTHENTICATION, E_ERROR, \
+from openlp.core.projectors import DialogSourceStyle
+from openlp.core.projectors.constants import ERROR_MSG, ERROR_STRING, E_AUTHENTICATION, E_ERROR, \
     E_NETWORK, E_NOT_CONNECTED, E_UNKNOWN_SOCKET_ERROR, STATUS_STRING, S_CONNECTED, S_CONNECTING, S_COOLDOWN, \
     S_INITIALIZE, S_NOT_CONNECTED, S_OFF, S_ON, S_STANDBY, S_WARMUP
-from openlp.core.lib.projector.db import ProjectorDB
-from openlp.core.lib.projector.pjlink import PJLink, PJLinkUDP
-from openlp.core.ui.projector.editform import ProjectorEditForm
-from openlp.core.ui.projector.sourceselectform import SourceSelectTabs, SourceSelectSingle
+from openlp.core.projectors.db import ProjectorDB
+from openlp.core.projectors.pjlink import PJLink, PJLinkUDP
+from openlp.core.projectors.editform import ProjectorEditForm
+from openlp.core.projectors.sourceselectform import SourceSelectTabs, SourceSelectSingle
 from openlp.core.widgets.toolbar import OpenLPToolbar
 
 log = logging.getLogger(__name__)
@@ -518,7 +518,7 @@ class ProjectorManager(QtWidgets.QWidget, RegistryBase, UiProjectorManager, LogM
         projector.thread.quit()
         new_list = []
         for item in self.projector_list:
-            if item.link.dbid == projector.link.dbid:
+            if item.link.db_item.id == projector.link.db_item.id:
                 continue
             new_list.append(item)
         self.projector_list = new_list
@@ -672,14 +672,16 @@ class ProjectorManager(QtWidgets.QWidget, RegistryBase, UiProjectorManager, LogM
                                                                        data=projector.model_filter)
             count = 1
             for item in projector.link.lamp:
+                if item['On'] is None:
+                    status = translate('OpenLP.ProjectorManager', 'Unavailable')
+                elif item['On']:
+                    status = translate('OpenLP.ProjectorManager', 'ON')
+                else:
+                    status = translate('OpenLP.ProjectorManager', 'OFF')
                 message += '<b>{title} {count}</b> {status} '.format(title=translate('OpenLP.ProjectorManager',
                                                                                      'Lamp'),
                                                                      count=count,
-                                                                     status=translate('OpenLP.ProjectorManager',
-                                                                                      'ON')
-                                                                     if item['On']
-                                                                     else translate('OpenLP.ProjectorManager',
-                                                                                    'OFF'))
+                                                                     status=status)
 
                 message += '<b>{title}</b>: {hours}<br />'.format(title=translate('OpenLP.ProjectorManager', 'Hours'),
                                                                   hours=item['Hours'])
@@ -730,7 +732,6 @@ class ProjectorManager(QtWidgets.QWidget, RegistryBase, UiProjectorManager, LogM
         thread.started.connect(item.link.thread_started)
         thread.finished.connect(item.link.thread_stopped)
         thread.finished.connect(thread.deleteLater)
-        item.link.projectorNetwork.connect(self.update_status)
         item.link.changeStatus.connect(self.update_status)
         item.link.projectorAuthentication.connect(self.authentication_error)
         item.link.projectorNoAuthentication.connect(self.no_authentication_error)

openlp/core/projectors/pjlink.py

@@ -54,7 +54,7 @@ from PyQt5 import QtCore, QtNetwork
 
 from openlp.core.common import qmd5_hash
 from openlp.core.common.i18n import translate
-from openlp.core.lib.projector.constants import CONNECTION_ERRORS, CR, ERROR_MSG, ERROR_STRING, \
+from openlp.core.projectors.constants import CONNECTION_ERRORS, CR, ERROR_MSG, ERROR_STRING, \
     E_AUTHENTICATION, E_CONNECTION_REFUSED, E_GENERAL, E_INVALID_DATA, E_NETWORK, E_NOT_CONNECTED, E_OK, \
     E_PARAMETER, E_PROJECTOR, E_SOCKET_TIMEOUT, E_UNAVAILABLE, E_UNDEFINED, PJLINK_ERRORS, PJLINK_ERST_DATA, \
     PJLINK_ERST_STATUS, PJLINK_MAX_PACKET, PJLINK_PORT, PJLINK_POWR_STATUS, PJLINK_VALID_CMD, \
@@ -402,17 +402,20 @@ class PJLinkCommands(object):
         :param data: Lamp(s) status.
         """
         lamps = []
-        data_dict = data.split()
-        while data_dict:
-            try:
-                fill = {'Hours': int(data_dict[0]), 'On': False if data_dict[1] == '0' else True}
-            except ValueError:
-                # In case of invalid entry
-                log.warning('({ip}) process_lamp(): Invalid data "{data}"'.format(ip=self.ip, data=data))
-                return
-            lamps.append(fill)
-            data_dict.pop(0)  # Remove lamp hours
-            data_dict.pop(0)  # Remove lamp on/off
+        lamp_list = data.split()
+        if len(lamp_list) < 2:
+            lamps.append({'Hours': int(lamp_list[0]), 'On': None})
+        else:
+            while lamp_list:
+                try:
+                    fill = {'Hours': int(lamp_list[0]), 'On': False if lamp_list[1] == '0' else True}
+                except ValueError:
+                    # In case of invalid entry
+                    log.warning('({ip}) process_lamp(): Invalid data "{data}"'.format(ip=self.ip, data=data))
+                    return
+                lamps.append(fill)
+                lamp_list.pop(0)  # Remove lamp hours
+                lamp_list.pop(0)  # Remove lamp on/off
         self.lamp = lamps
         return
 
@@ -520,7 +523,6 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands):
     """
     # Signals sent by this module
     changeStatus = QtCore.pyqtSignal(str, int, str)
-    projectorNetwork = QtCore.pyqtSignal(int)  # Projector network activity
     projectorStatus = QtCore.pyqtSignal(int)  # Status update
     projectorAuthentication = QtCore.pyqtSignal(str)  # Authentication error
     projectorNoAuthentication = QtCore.pyqtSignal(str)  # PIN set and no authentication needed
@@ -846,7 +848,6 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands):
             log.debug('({ip}) get_socket(): No data available (-1)'.format(ip=self.ip))
             return self.receive_data_signal()
         self.socket_timer.stop()
-        self.projectorNetwork.emit(S_NETWORK_RECEIVED)
         return self.get_data(buff=read, ip=self.ip)
 
     def get_data(self, buff, ip):
@@ -925,7 +926,6 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands):
         if cmd not in PJLINK_VALID_CMD:
             log.error('({ip}) send_command(): Invalid command requested - ignoring.'.format(ip=self.ip))
             return
-        self.projectorNetwork.emit(S_NETWORK_SENDING)
         log.debug('({ip}) send_command(): Building cmd="{command}" opts="{data}"{salt}'.format(ip=self.ip,
                                                                                                command=cmd,
                                                                                                data=opts,
@@ -996,7 +996,6 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands):
         log.debug('({ip}) _send_string(): Sending "{data}"'.format(ip=self.ip, data=out.strip()))
         log.debug('({ip}) _send_string(): Queue = {data}'.format(ip=self.ip, data=self.send_queue))
         self.socket_timer.start()
-        self.projectorNetwork.emit(S_NETWORK_SENDING)
         sent = self.write(out.encode('{string_encoding}'.format(string_encoding='utf-8' if utf8 else 'ascii')))
         self.waitForBytesWritten(2000)  # 2 seconds should be enough
         if sent == -1:

openlp/core/projectors/sourceselectform.py

@@ -31,8 +31,8 @@ from PyQt5 import QtCore, QtWidgets
 from openlp.core.common import is_macosx
 from openlp.core.common.i18n import translate
 from openlp.core.lib import build_icon
-from openlp.core.lib.projector.db import ProjectorSource
-from openlp.core.lib.projector.constants import PJLINK_DEFAULT_SOURCES, PJLINK_DEFAULT_CODES
+from openlp.core.projectors.db import ProjectorSource
+from openlp.core.projectors.constants import PJLINK_DEFAULT_SOURCES, PJLINK_DEFAULT_CODES
 
 log = logging.getLogger(__name__)
 

openlp/core/projectors/tab.py

@@ -29,7 +29,7 @@ from PyQt5 import QtWidgets
 from openlp.core.common.i18n import UiStrings, translate
 from openlp.core.common.settings import Settings
 from openlp.core.lib import SettingsTab
-from openlp.core.lib.projector import DialogSourceStyle
+from openlp.core.projectors import DialogSourceStyle
 
 log = logging.getLogger(__name__)
 log.debug('projectortab module loaded')

openlp/core/ui/__init__.py

@@ -113,9 +113,10 @@ from .formattingtagcontroller import FormattingTagController
 from .shortcutlistform import ShortcutListForm
 from .servicemanager import ServiceManager
 from .thememanager import ThemeManager
-from .projector.manager import ProjectorManager
-from .projector.tab import ProjectorTab
-from .projector.editform import ProjectorEditForm
+
+from openlp.core.projectors.editform import ProjectorEditForm
+from openlp.core.projectors.manager import ProjectorManager
+from openlp.core.projectors.tab import ProjectorTab
 
 __all__ = ['SplashScreen', 'AboutForm', 'SettingsForm', 'MainDisplay', 'SlideController', 'ServiceManager', 'ThemeForm',
            'ThemeManager', 'ServiceItemEditForm', 'FirstTimeForm', 'FirstTimeLanguageForm', 'Display', 'AudioPlayer',

openlp/core/ui/exceptionform.py

@@ -108,7 +108,7 @@ class ExceptionForm(QtWidgets.QDialog, Ui_ExceptionDialog, RegistryProperties):
             try:
                 with file_path.open('w') as report_file:
                     report_file.write(report_text)
-            except IOError:
+            except OSError:
                 log.exception('Failed to write crash report')
 
     def on_send_report_button_clicked(self):

openlp/core/ui/firsttimewizard.py

@@ -261,8 +261,8 @@ class UiFirstTimeWizard(object):
         self.alert_check_box.setText(translate('OpenLP.FirstTimeWizard',
                                                'Alerts – Display informative messages while showing other slides'))
         self.projectors_check_box.setText(translate('OpenLP.FirstTimeWizard',
-                                                    'Projectors – Control PJLink compatible projects on your network'
-                                                    ' from OpenLP'))
+                                                    'Projector Controller – Control PJLink compatible projects on your'
+                                                    ' network from OpenLP'))
         self.no_internet_page.setTitle(translate('OpenLP.FirstTimeWizard', 'No Internet Connection'))
         self.no_internet_page.setSubTitle(
             translate('OpenLP.FirstTimeWizard', 'Unable to detect an Internet connection.'))

openlp/core/ui/formattingtagcontroller.py

@@ -43,7 +43,7 @@ class FormattingTagController(object):
             r'(?P<tag>[^\s/!\?>]+)(?:\s+[^\s=]+="[^"]*")*\s*(?P<empty>/)?'
             r'|(?P<cdata>!\[CDATA\[(?:(?!\]\]>).)*\]\])'
             r'|(?P<procinst>\?(?:(?!\?>).)*\?)'
-            r'|(?P<comment>!--(?:(?!-->).)*--))>', re.UNICODE)
+            r'|(?P<comment>!--(?:(?!-->).)*--))>')
         self.html_regex = re.compile(r'^(?:[^<>]*%s)*[^<>]*$' % self.html_tag_regex.pattern)
 
     def pre_save(self):

openlp/core/ui/mainwindow.py

@@ -40,24 +40,24 @@ 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
 from openlp.core.display.screens import ScreenList
 from openlp.core.lib import PluginManager, ImageManager, PluginStatus, build_icon
 from openlp.core.lib.ui import create_action
+from openlp.core.projectors.manager import ProjectorManager
 from openlp.core.ui import AboutForm, SettingsForm, ServiceManager, ThemeManager, PluginForm, ShortcutListForm, \
     FormattingTagForm
 from openlp.core.ui.slidecontroller import LiveController, PreviewController
 from openlp.core.ui.firsttimeform import FirstTimeForm
-from openlp.core.widgets.dialogs import FileDialog
-from openlp.core.widgets.docks import OpenLPDockWidget, MediaDockManager
 from openlp.core.ui.media import MediaController
 from openlp.core.ui.printserviceform import PrintServiceForm
-from openlp.core.ui.projector.manager import ProjectorManager
 from openlp.core.ui.style import PROGRESSBAR_STYLE, get_library_stylesheet
 from openlp.core.version import get_version
+from openlp.core.widgets.dialogs import FileDialog
+from openlp.core.widgets.docks import OpenLPDockWidget, MediaDockManager
 
 
 log = logging.getLogger(__name__)
@@ -180,7 +180,7 @@ class Ui_MainWindow(object):
                                             triggers=self.service_manager_contents.on_load_service_clicked)
         self.file_save_item = create_action(main_window, 'fileSaveItem', icon=':/general/general_save.png',
                                             can_shortcuts=True, category=UiStrings().File,
-                                            triggers=self.service_manager_contents.save_file)
+                                            triggers=self.service_manager_contents.decide_save_method)
         self.file_save_as_item = create_action(main_window, 'fileSaveAsItem', can_shortcuts=True,
                                                category=UiStrings().File,
                                                triggers=self.service_manager_contents.save_file_as)
@@ -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)
@@ -375,7 +374,7 @@ class Ui_MainWindow(object):
         self.media_manager_dock.setWindowTitle(translate('OpenLP.MainWindow', 'Library'))
         self.service_manager_dock.setWindowTitle(translate('OpenLP.MainWindow', 'Service'))
         self.theme_manager_dock.setWindowTitle(translate('OpenLP.MainWindow', 'Themes'))
-        self.projector_manager_dock.setWindowTitle(translate('OpenLP.MainWindow', 'Projectors'))
+        self.projector_manager_dock.setWindowTitle(translate('OpenLP.MainWindow', 'Projector Controller'))
         self.file_new_item.setText(translate('OpenLP.MainWindow', '&New Service'))
         self.file_new_item.setToolTip(UiStrings().NewService)
         self.file_new_item.setStatusTip(UiStrings().CreateService)
@@ -407,7 +406,7 @@ class Ui_MainWindow(object):
             translate('OpenLP.MainWindow', 'Import settings from a *.config file previously exported from '
                                            'this or another machine.'))
         self.settings_import_item.setText(translate('OpenLP.MainWindow', 'Settings'))
-        self.view_projector_manager_item.setText(translate('OpenLP.MainWindow', '&Projectors'))
+        self.view_projector_manager_item.setText(translate('OpenLP.MainWindow', '&Projector Controller'))
         self.view_projector_manager_item.setToolTip(translate('OpenLP.MainWindow', 'Hide or show Projectors.'))
         self.view_projector_manager_item.setStatusTip(translate('OpenLP.MainWindow',
                                                                 'Toggle visibility of the Projectors.'))
@@ -649,8 +648,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):
         """
@@ -759,7 +758,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/')
@@ -1219,7 +1218,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 +1242,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 +1258,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 +1287,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):
         """
@@ -1366,7 +1356,7 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties):
                               '- Please wait for copy to finish').format(path=self.new_data_path))
                 dir_util.copy_tree(str(old_data_path), str(self.new_data_path))
                 log.info('Copy successful')
-            except (IOError, os.error, DistutilsFileError) as why:
+            except (OSError, DistutilsFileError) as why:
                 self.application.set_normal_cursor()
                 log.exception('Data copy failed {err}'.format(err=str(why)))
                 err_text = translate('OpenLP.MainWindow',

openlp/core/ui/media/vendor/mediainfoWrapper.py

@@ -25,10 +25,8 @@ information related to the rwquested media.
 """
 import json
 import os
-from subprocess import Popen
-from tempfile import mkstemp
+from subprocess import check_output
 
-import six
 from bs4 import BeautifulSoup, NavigableString
 
 ENV_DICT = os.environ
@@ -80,7 +78,7 @@ class Track(object):
 
     def to_data(self):
         data = {}
-        for k, v in six.iteritems(self.__dict__):
+        for k, v in self.__dict__.items():
             if k != 'xml_dom_fragment':
                 data[k] = v
         return data
@@ -100,20 +98,10 @@ class MediaInfoWrapper(object):
 
     @staticmethod
     def parse(filename, environment=ENV_DICT):
-        command = ["mediainfo", "-f", "--Output=XML", filename]
-        fileno_out, fname_out = mkstemp(suffix=".xml", prefix="media-")
-        fileno_err, fname_err = mkstemp(suffix=".err", prefix="media-")
-        fp_out = os.fdopen(fileno_out, 'r+b')
-        fp_err = os.fdopen(fileno_err, 'r+b')
-        p = Popen(command, stdout=fp_out, stderr=fp_err, env=environment)
-        p.wait()
-        fp_out.seek(0)
-
-        xml_dom = MediaInfoWrapper.parse_xml_data_into_dom(fp_out.read())
-        fp_out.close()
-        fp_err.close()
-        os.unlink(fname_out)
-        os.unlink(fname_err)
+        xml = check_output(['mediainfo', '-f', '--Output=XML', '--Inform=OLDXML', filename])
+        if not xml.startswith(b'<?xml'):
+            xml = check_output(['mediainfo', '-f', '--Output=XML', filename])
+        xml_dom = MediaInfoWrapper.parse_xml_data_into_dom(xml)
         return MediaInfoWrapper(xml_dom)
 
     def _populate_tracks(self):

openlp/core/ui/servicemanager.py

@@ -32,12 +32,12 @@ 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
 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
@@ -193,18 +193,6 @@ class Ui_ServiceManager(object):
             text=translate('OpenLP.ServiceManager', 'Move to &bottom'), icon=':/services/service_bottom.png',
             tooltip=translate('OpenLP.ServiceManager', 'Move item to the end of the service.'),
             can_shortcuts=True, category=UiStrings().Service, triggers=self.on_service_end)
-        self.down_action = self.order_toolbar.add_toolbar_action(
-            'down',
-            text=translate('OpenLP.ServiceManager', 'Move &down'), can_shortcuts=True,
-            tooltip=translate('OpenLP.ServiceManager', 'Moves the selection down the window.'), visible=False,
-            triggers=self.on_move_selection_down)
-        action_list.add_action(self.down_action)
-        self.up_action = self.order_toolbar.add_toolbar_action(
-            'up',
-            text=translate('OpenLP.ServiceManager', 'Move up'), can_shortcuts=True,
-            tooltip=translate('OpenLP.ServiceManager', 'Moves the selection up the window.'), visible=False,
-            triggers=self.on_move_selection_up)
-        action_list.add_action(self.up_action)
         self.order_toolbar.addSeparator()
         self.delete_action = self.order_toolbar.add_toolbar_action(
             'delete', can_shortcuts=True,
@@ -228,7 +216,7 @@ class Ui_ServiceManager(object):
             text=translate('OpenLP.ServiceManager', 'Go Live'), icon=':/general/general_live.png',
             tooltip=translate('OpenLP.ServiceManager', 'Send the selected item to Live.'),
             category=UiStrings().Service,
-            triggers=self.make_live)
+            triggers=self.on_make_live_action_triggered)
         self.layout.addWidget(self.order_toolbar)
         # Connect up our signals and slots
         self.theme_combo_box.activated.connect(self.on_theme_combo_box_selected)
@@ -300,8 +288,8 @@ class Ui_ServiceManager(object):
         self.theme_menu = QtWidgets.QMenu(translate('OpenLP.ServiceManager', '&Change Item Theme'))
         self.menu.addMenu(self.theme_menu)
         self.service_manager_list.addActions([self.move_down_action, self.move_up_action, self.make_live_action,
-                                              self.move_top_action, self.move_bottom_action, self.up_action,
-                                              self.down_action, self.expand_action, self.collapse_action])
+                                              self.move_top_action, self.move_bottom_action, self.expand_action,
+                                              self.collapse_action])
         Registry().register_function('theme_update_list', self.update_theme_list)
         Registry().register_function('config_screen_changed', self.regenerate_service_items)
         Registry().register_function('theme_update_global', self.theme_change)
@@ -331,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
 
@@ -378,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':
@@ -389,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):
         """
@@ -474,6 +464,12 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi
         Load a recent file as the service triggered by mainwindow recent service list.
         :param field:
         """
+        if self.is_modified():
+            result = self.save_modified_service()
+            if result == QtWidgets.QMessageBox.Cancel:
+                return False
+            elif result == QtWidgets.QMessageBox.Save:
+                self.decide_save_method()
         sender = self.sender()
         self.load_file(sender.data())
 
@@ -603,7 +599,7 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi
                 if not os.path.exists(save_file):
                     shutil.copy(audio_from, save_file)
                 zip_file.write(audio_from, audio_to)
-        except IOError:
+        except OSError:
             self.log_exception('Failed to save service to disk: {name}'.format(name=temp_file_name))
             self.main_window.error_message(translate('OpenLP.ServiceManager', 'Error Saving File'),
                                            translate('OpenLP.ServiceManager', 'There was an error saving your file.'))
@@ -664,7 +660,7 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi
             zip_file = zipfile.ZipFile(temp_file_name, 'w', zipfile.ZIP_STORED, True)
             # First we add service contents.
             zip_file.writestr(service_file_name, service_content)
-        except IOError:
+        except OSError:
             self.log_exception('Failed to save service to disk: {name}'.format(name=temp_file_name))
             self.main_window.error_message(translate('OpenLP.ServiceManager', 'Error Saving File'),
                                            translate('OpenLP.ServiceManager', 'There was an error saving your file.'))
@@ -710,20 +706,29 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi
         directory_path = Settings().value(self.main_window.service_manager_settings_section + '/last directory')
         if directory_path:
             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._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,
-                translate('OpenLP.ServiceManager',
-                          'OpenLP Service Files (*.osz);; OpenLP Service Files - lite (*.oszl)'))
+                '{packaged};; {lite}'.format(packaged=packaged_filter, lite=lite_filter),
+                default_filter)
         else:
             file_path, filter_used = FileDialog.getSaveFileName(
-                self.main_window, UiStrings().SaveService, file_path,
-                translate('OpenLP.ServiceManager', 'OpenLP Service Files (*.osz);;'))
+                self.main_window, UiStrings().SaveService, default_file_path,
+                '{packaged};;'.format(packaged=packaged_filter))
         if not file_path:
             return False
-        file_path.with_suffix('.osz')
+        if filter_used == lite_filter:
+            file_path = file_path.with_suffix('.oszl')
+        else:
+            file_path = file_path.with_suffix('.osz')
         self.set_file_name(file_path)
         self.decide_save_method()
 
@@ -791,11 +796,11 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi
             else:
                 critical_error_message_box(message=translate('OpenLP.ServiceManager', 'File is not a valid service.'))
                 self.log_error('File contains no service data')
-        except (IOError, NameError):
+        except (OSError, NameError):
             self.log_exception('Problem loading service file {name}'.format(name=file_name))
             critical_error_message_box(message=translate('OpenLP.ServiceManager',
                                        'File could not be opened because it is corrupt.'))
-        except zipfile.BadZipfile:
+        except zipfile.BadZipFile:
             if os.path.getsize(file_name) == 0:
                 self.log_exception('Service file is zero sized: {name}'.format(name=file_name))
                 QtWidgets.QMessageBox.information(self, translate('OpenLP.ServiceManager', 'Empty File'),
@@ -1657,14 +1662,15 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi
                 if start_pos == -1:
                     return
                 if item is None:
-                    end_pos = len(self.service_items)
+                    end_pos = len(self.service_items) - 1
                 else:
                     end_pos = get_parent_item_data(item) - 1
                 service_item = self.service_items[start_pos]
-                self.service_items.remove(service_item)
-                self.service_items.insert(end_pos, service_item)
-                self.repaint_service_list(end_pos, child)
-                self.set_modified()
+                if start_pos != end_pos:
+                    self.service_items.remove(service_item)
+                    self.service_items.insert(end_pos, service_item)
+                    self.repaint_service_list(end_pos, child)
+                    self.set_modified()
             else:
                 # we are not over anything so drop
                 replace = False
@@ -1730,6 +1736,15 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi
         self.service_items[item]['service_item'].update_theme(theme)
         self.regenerate_service_items(True)
 
+    def on_make_live_action_triggered(self, checked):
+        """
+        Handle `make_live_action` when the action is triggered.
+
+        :param bool checked: Not Used.
+        :rtype: None
+        """
+        self.make_live()
+
     def get_drop_position(self):
         """
         Getter for drop_position. Used in: MediaManagerItem

openlp/core/ui/settingsform.py

@@ -30,9 +30,9 @@ from openlp.core.api import ApiTab
 from openlp.core.common.mixins import RegistryProperties
 from openlp.core.common.registry import Registry
 from openlp.core.lib import build_icon
+from openlp.core.projectors.tab import ProjectorTab
 from openlp.core.ui import AdvancedTab, GeneralTab, ThemesTab
 from openlp.core.ui.media import PlayerTab
-from openlp.core.ui.projector.tab import ProjectorTab
 from openlp.core.ui.settingsdialog import Ui_SettingsDialog
 
 log = logging.getLogger(__name__)

openlp/core/ui/slidecontroller.py

@@ -228,6 +228,9 @@ class SlideController(QtWidgets.QWidget, LogMixin, RegistryProperties):
             self.hide_menu.setPopupMode(QtWidgets.QToolButton.MenuButtonPopup)
             self.hide_menu.setMenu(QtWidgets.QMenu(translate('OpenLP.SlideController', 'Hide'), self.toolbar))
             self.toolbar.add_toolbar_widget(self.hide_menu)
+            self.toolbar.add_toolbar_action('goPreview', icon=':/general/general_live.png',
+                                            tooltip=translate('OpenLP.SlideController', 'Move to preview.'),
+                                            triggers=self.on_go_preview)
             # The order of the blank to modes in Shortcuts list comes from here.
             self.desktop_screen_enable = create_action(self, 'desktopScreenEnable',
                                                        text=translate('OpenLP.SlideController', 'Show Desktop'),
@@ -1408,6 +1411,15 @@ class SlideController(QtWidgets.QWidget, LogMixin, RegistryProperties):
                 self.live_controller.add_service_manager_item(self.service_item, row)
             self.live_controller.preview_widget.setFocus()
 
+    def on_go_preview(self, field=None):
+        """
+        If live copy slide item to preview controller from live Controller
+        """
+        row = self.preview_widget.current_slide_number()
+        if -1 < row < self.preview_widget.slide_count():
+            self.preview_controller.add_service_manager_item(self.service_item, row)
+            self.preview_controller.preview_widget.setFocus()
+
     def on_media_start(self, item):
         """
         Respond to the arrival of a media service item

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):
@@ -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)
@@ -604,7 +604,7 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R
                     else:
                         with full_name.open('wb') as out_file:
                             out_file.write(theme_zip.read(zipped_file))
-        except (IOError, zipfile.BadZipfile):
+        except (OSError, zipfile.BadZipFile):
             self.log_exception('Importing theme from zip failed {name}'.format(name=file_path))
             raise ValidationError
         except ValidationError:
@@ -667,7 +667,7 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R
         theme_path = theme_dir / '{file_name}.json'.format(file_name=name)
         try:
                 theme_path.write_text(theme_pretty)
-        except IOError:
+        except OSError:
             self.log_exception('Saving theme to file failed')
         if image_source_path and image_destination_path:
             if self.old_background_image_path and image_destination_path != self.old_background_image_path:
@@ -675,7 +675,7 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R
             if image_source_path != image_destination_path:
                 try:
                     copyfile(image_source_path, image_destination_path)
-                except IOError:
+                except OSError:
                     self.log_exception('Failed to save theme image')
         self.generate_and_save_image(name, theme)
 
@@ -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):
         """
@@ -711,6 +711,7 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R
 
         :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
         """
         return self.renderer.generate_preview(theme_data, force_page)
 

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
@@ -110,7 +109,7 @@ class VersionWorker(QtCore.QObject):
                 remote_version = response.text
                 log.debug('New version found: %s', remote_version)
                 break
-            except IOError:
+            except OSError:
                 log.exception('Unable to connect to OpenLP server to download version file')
                 retries += 1
         else:
@@ -190,18 +189,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()
-        except IOError:
+            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,

openlp/core/widgets/edits.py

@@ -27,6 +27,7 @@ import re
 
 from PyQt5 import QtCore, QtGui, QtWidgets
 
+from openlp.core.common import CONTROL_CHARS
 from openlp.core.common.i18n import UiStrings, translate
 from openlp.core.common.path import Path, path_to_str, str_to_path
 from openlp.core.common.settings import Settings
@@ -241,7 +242,7 @@ class PathEdit(QtWidgets.QWidget):
         self.line_edit.editingFinished.connect(self.on_line_edit_editing_finished)
         self.update_button_tool_tips()
 
-    @property
+    @QtCore.pyqtProperty('QVariant')
     def path(self):
         """
         A property getter method to return the selected path.
@@ -349,7 +350,7 @@ class PathEdit(QtWidgets.QWidget):
         :rtype: None
         """
         if self._path != path:
-            self.path = path
+            self._path = path
             self.pathChanged.emit(path)
 
 
@@ -470,12 +471,21 @@ class SpellTextEdit(QtWidgets.QPlainTextEdit):
                     cursor.insertText(html['start tag'])
                     cursor.insertText(html['end tag'])
 
+    def insertFromMimeData(self, source):
+        """
+        Reimplement `insertFromMimeData` so that we can remove any control characters
+
+        :param QtCore.QMimeData source: The mime data to insert
+        :rtype: None
+        """
+        self.insertPlainText(CONTROL_CHARS.sub('', source.text()))
+
 
 class Highlighter(QtGui.QSyntaxHighlighter):
     """
     Provides a text highlighter for pointing out spelling errors in text.
     """
-    WORDS = r'(?iu)[\w\']+'
+    WORDS = r'(?i)[\w\']+'
 
     def __init__(self, *args):
         """

openlp/core/widgets/views.py

@@ -336,7 +336,7 @@ class ListWidgetWithDnD(QtWidgets.QListWidget):
                     for file in listing:
                         files.append(os.path.join(local_file, file))
             Registry().execute('{mime_data}_dnd'.format(mime_data=self.mime_data_text),
-                               {'files': files, 'target': self.itemAt(event.pos())})
+                               {'files': files})
         else:
             event.ignore()
 

openlp/plugins/bibles/forms/bibleimportform.py

@@ -23,7 +23,6 @@
 The bible import functions for OpenLP
 """
 import logging
-import os
 import urllib.error
 from lxml import etree
 
@@ -41,6 +40,7 @@ from openlp.core.common.settings import Settings
 from openlp.core.lib.db import delete_database
 from openlp.core.lib.exceptions import ValidationError
 from openlp.core.lib.ui import critical_error_message_box
+from openlp.core.widgets.edits import PathEdit
 from openlp.core.widgets.wizard import OpenLPWizard, WizardStrings
 from openlp.plugins.bibles.lib.db import clean_filename
 from openlp.plugins.bibles.lib.importers.http import CWExtract, BGExtract, BSExtract
@@ -122,15 +122,9 @@ class BibleImportForm(OpenLPWizard):
         Set up the signals used in the bible importer.
         """
         self.web_source_combo_box.currentIndexChanged.connect(self.on_web_source_combo_box_index_changed)
-        self.osis_browse_button.clicked.connect(self.on_osis_browse_button_clicked)
-        self.csv_books_button.clicked.connect(self.on_csv_books_browse_button_clicked)
-        self.csv_verses_button.clicked.connect(self.on_csv_verses_browse_button_clicked)
-        self.open_song_browse_button.clicked.connect(self.on_open_song_browse_button_clicked)
-        self.zefania_browse_button.clicked.connect(self.on_zefania_browse_button_clicked)
-        self.wordproject_browse_button.clicked.connect(self.on_wordproject_browse_button_clicked)
         self.web_update_button.clicked.connect(self.on_web_update_button_clicked)
-        self.sword_browse_button.clicked.connect(self.on_sword_browse_button_clicked)
-        self.sword_zipbrowse_button.clicked.connect(self.on_sword_zipbrowse_button_clicked)
+        self.sword_folder_path_edit.pathChanged.connect(self.on_sword_folder_path_edit_path_changed)
+        self.sword_zipfile_path_edit.pathChanged.connect(self.on_sword_zipfile_path_edit_path_changed)
 
     def add_custom_pages(self):
         """
@@ -161,17 +155,12 @@ class BibleImportForm(OpenLPWizard):
         self.osis_layout.setObjectName('OsisLayout')
         self.osis_file_label = QtWidgets.QLabel(self.osis_widget)
         self.osis_file_label.setObjectName('OsisFileLabel')
-        self.osis_file_layout = QtWidgets.QHBoxLayout()
-        self.osis_file_layout.setObjectName('OsisFileLayout')
-        self.osis_file_edit = QtWidgets.QLineEdit(self.osis_widget)
-        self.osis_file_edit.setSizePolicy(QtWidgets.QSizePolicy.Maximum, QtWidgets.QSizePolicy.Preferred)
-        self.osis_file_edit.setObjectName('OsisFileEdit')
-        self.osis_file_layout.addWidget(self.osis_file_edit)
-        self.osis_browse_button = QtWidgets.QToolButton(self.osis_widget)
-        self.osis_browse_button.setIcon(self.open_icon)
-        self.osis_browse_button.setObjectName('OsisBrowseButton')
-        self.osis_file_layout.addWidget(self.osis_browse_button)
-        self.osis_layout.addRow(self.osis_file_label, self.osis_file_layout)
+        self.osis_path_edit = PathEdit(
+            self.osis_widget,
+            default_path=Settings().value('bibles/last directory import'),
+            dialog_caption=WizardStrings.OpenTypeFile.format(file_type=WizardStrings.OSIS),
+            show_revert=False)
+        self.osis_layout.addRow(self.osis_file_label, self.osis_path_edit)
         self.osis_layout.setItem(1, QtWidgets.QFormLayout.LabelRole, self.spacer)
         self.select_stack.addWidget(self.osis_widget)
         self.csv_widget = QtWidgets.QWidget(self.select_page)
@@ -181,30 +170,27 @@ class BibleImportForm(OpenLPWizard):
         self.csv_layout.setObjectName('CsvLayout')
         self.csv_books_label = QtWidgets.QLabel(self.csv_widget)
         self.csv_books_label.setObjectName('CsvBooksLabel')
-        self.csv_books_layout = QtWidgets.QHBoxLayout()
-        self.csv_books_layout.setObjectName('CsvBooksLayout')
-        self.csv_books_edit = QtWidgets.QLineEdit(self.csv_widget)
-        self.csv_books_edit.setSizePolicy(QtWidgets.QSizePolicy.Maximum, QtWidgets.QSizePolicy.Preferred)
-        self.csv_books_edit.setObjectName('CsvBooksEdit')
-        self.csv_books_layout.addWidget(self.csv_books_edit)
-        self.csv_books_button = QtWidgets.QToolButton(self.csv_widget)
-        self.csv_books_button.setIcon(self.open_icon)
-        self.csv_books_button.setObjectName('CsvBooksButton')
-        self.csv_books_layout.addWidget(self.csv_books_button)
-        self.csv_layout.addRow(self.csv_books_label, self.csv_books_layout)
+        self.csv_books_path_edit = PathEdit(
+            self.csv_widget,
+            default_path=Settings().value('bibles/last directory import'),
+            dialog_caption=WizardStrings.OpenTypeFile.format(file_type=WizardStrings.CSV),
+            show_revert=False,
+        )
+        self.csv_books_path_edit.filters = \
+            '{name} (*.csv)'.format(name=translate('BiblesPlugin.ImportWizardForm', 'CSV File'))
+        self.csv_layout.addRow(self.csv_books_label, self.csv_books_path_edit)
         self.csv_verses_label = QtWidgets.QLabel(self.csv_widget)
         self.csv_verses_label.setObjectName('CsvVersesLabel')
-        self.csv_verses_layout = QtWidgets.QHBoxLayout()
-        self.csv_verses_layout.setObjectName('CsvVersesLayout')
-        self.csv_verses_edit = QtWidgets.QLineEdit(self.csv_widget)
-        self.csv_verses_edit.setSizePolicy(QtWidgets.QSizePolicy.Maximum, QtWidgets.QSizePolicy.Preferred)
-        self.csv_verses_edit.setObjectName('CsvVersesEdit')
-        self.csv_verses_layout.addWidget(self.csv_verses_edit)
-        self.csv_verses_button = QtWidgets.QToolButton(self.csv_widget)
-        self.csv_verses_button.setIcon(self.open_icon)
-        self.csv_verses_button.setObjectName('CsvVersesButton')
-        self.csv_verses_layout.addWidget(self.csv_verses_button)
-        self.csv_layout.addRow(self.csv_verses_label, self.csv_verses_layout)
+        self.csv_verses_path_edit = PathEdit(
+            self.csv_widget,
+            default_path=Settings().value('bibles/last directory import'),
+            dialog_caption=WizardStrings.OpenTypeFile.format(file_type=WizardStrings.CSV),
+            show_revert=False,
+        )
+        self.csv_verses_path_edit.filters = \
+            '{name} (*.csv)'.format(name=translate('BiblesPlugin.ImportWizardForm', 'CSV File'))
+        self.csv_layout.addRow(self.csv_books_label, self.csv_books_path_edit)
+        self.csv_layout.addRow(self.csv_verses_label, self.csv_verses_path_edit)
         self.csv_layout.setItem(3, QtWidgets.QFormLayout.LabelRole, self.spacer)
         self.select_stack.addWidget(self.csv_widget)
         self.open_song_widget = QtWidgets.QWidget(self.select_page)
@@ -214,17 +200,13 @@ class BibleImportForm(OpenLPWizard):
         self.open_song_layout.setObjectName('OpenSongLayout')
         self.open_song_file_label = QtWidgets.QLabel(self.open_song_widget)
         self.open_song_file_label.setObjectName('OpenSongFileLabel')
-        self.open_song_file_layout = QtWidgets.QHBoxLayout()
-        self.open_song_file_layout.setObjectName('OpenSongFileLayout')
-        self.open_song_file_edit = QtWidgets.QLineEdit(self.open_song_widget)
-        self.open_song_file_edit.setSizePolicy(QtWidgets.QSizePolicy.Maximum, QtWidgets.QSizePolicy.Preferred)
-        self.open_song_file_edit.setObjectName('OpenSongFileEdit')
-        self.open_song_file_layout.addWidget(self.open_song_file_edit)
-        self.open_song_browse_button = QtWidgets.QToolButton(self.open_song_widget)
-        self.open_song_browse_button.setIcon(self.open_icon)
-        self.open_song_browse_button.setObjectName('OpenSongBrowseButton')
-        self.open_song_file_layout.addWidget(self.open_song_browse_button)
-        self.open_song_layout.addRow(self.open_song_file_label, self.open_song_file_layout)
+        self.open_song_path_edit = PathEdit(
+            self.open_song_widget,
+            default_path=Settings().value('bibles/last directory import'),
+            dialog_caption=WizardStrings.OpenTypeFile.format(file_type=WizardStrings.OS),
+            show_revert=False,
+        )
+        self.open_song_layout.addRow(self.open_song_file_label, self.open_song_path_edit)
         self.open_song_layout.setItem(1, QtWidgets.QFormLayout.LabelRole, self.spacer)
         self.select_stack.addWidget(self.open_song_widget)
         self.web_tab_widget = QtWidgets.QTabWidget(self.select_page)
@@ -292,17 +274,14 @@ class BibleImportForm(OpenLPWizard):
         self.zefania_layout.setObjectName('ZefaniaLayout')
         self.zefania_file_label = QtWidgets.QLabel(self.zefania_widget)
         self.zefania_file_label.setObjectName('ZefaniaFileLabel')
-        self.zefania_file_layout = QtWidgets.QHBoxLayout()
-        self.zefania_file_layout.setObjectName('ZefaniaFileLayout')
-        self.zefania_file_edit = QtWidgets.QLineEdit(self.zefania_widget)
-        self.zefania_file_edit.setObjectName('ZefaniaFileEdit')
-        self.zefania_file_layout.addWidget(self.zefania_file_edit)
-        self.zefania_browse_button = QtWidgets.QToolButton(self.zefania_widget)
-        self.zefania_browse_button.setIcon(self.open_icon)
-        self.zefania_browse_button.setObjectName('ZefaniaBrowseButton')
-        self.zefania_file_layout.addWidget(self.zefania_browse_button)
-        self.zefania_layout.addRow(self.zefania_file_label, self.zefania_file_layout)
-        self.zefania_layout.setItem(5, QtWidgets.QFormLayout.LabelRole, self.spacer)
+        self.zefania_path_edit = PathEdit(
+            self.zefania_widget,
+            default_path=Settings().value('bibles/last directory import'),
+            dialog_caption=WizardStrings.OpenTypeFile.format(file_type=WizardStrings.ZEF),
+            show_revert=False,
+        )
+        self.zefania_layout.addRow(self.zefania_file_label, self.zefania_path_edit)
+        self.zefania_layout.setItem(2, QtWidgets.QFormLayout.LabelRole, self.spacer)
         self.select_stack.addWidget(self.zefania_widget)
         self.sword_widget = QtWidgets.QWidget(self.select_page)
         self.sword_widget.setObjectName('SwordWidget')
@@ -318,16 +297,13 @@ class BibleImportForm(OpenLPWizard):
         self.sword_folder_label = QtWidgets.QLabel(self.sword_folder_tab)
         self.sword_folder_label.setObjectName('SwordSourceLabel')
         self.sword_folder_label.setObjectName('SwordFolderLabel')
-        self.sword_folder_edit = QtWidgets.QLineEdit(self.sword_folder_tab)
-        self.sword_folder_edit.setSizePolicy(QtWidgets.QSizePolicy.Maximum, QtWidgets.QSizePolicy.Preferred)
-        self.sword_folder_edit.setObjectName('SwordFolderEdit')
-        self.sword_browse_button = QtWidgets.QToolButton(self.sword_folder_tab)
-        self.sword_browse_button.setIcon(self.open_icon)
-        self.sword_browse_button.setObjectName('SwordBrowseButton')
-        self.sword_folder_layout = QtWidgets.QHBoxLayout()
-        self.sword_folder_layout.addWidget(self.sword_folder_edit)
-        self.sword_folder_layout.addWidget(self.sword_browse_button)
-        self.sword_folder_tab_layout.addRow(self.sword_folder_label, self.sword_folder_layout)
+        self.sword_folder_path_edit = PathEdit(
+            self.sword_folder_tab,
+            default_path=Settings().value('bibles/last directory import'),
+            dialog_caption=WizardStrings.OpenTypeFile.format(file_type=WizardStrings.SWORD),
+            show_revert=False,
+        )
+        self.sword_folder_tab_layout.addRow(self.sword_folder_label, self.sword_folder_path_edit)
         self.sword_bible_label = QtWidgets.QLabel(self.sword_folder_tab)
         self.sword_bible_label.setObjectName('SwordBibleLabel')
         self.sword_bible_combo_box = QtWidgets.QComboBox(self.sword_folder_tab)
@@ -342,16 +318,13 @@ class BibleImportForm(OpenLPWizard):
         self.sword_zip_layout.setObjectName('SwordZipLayout')
         self.sword_zipfile_label = QtWidgets.QLabel(self.sword_zip_tab)
         self.sword_zipfile_label.setObjectName('SwordZipFileLabel')
-        self.sword_zipfile_edit = QtWidgets.QLineEdit(self.sword_zip_tab)
-        self.sword_zipfile_edit.setSizePolicy(QtWidgets.QSizePolicy.Maximum, QtWidgets.QSizePolicy.Preferred)
-        self.sword_zipfile_edit.setObjectName('SwordZipFileEdit')
-        self.sword_zipbrowse_button = QtWidgets.QToolButton(self.sword_zip_tab)
-        self.sword_zipbrowse_button.setIcon(self.open_icon)
-        self.sword_zipbrowse_button.setObjectName('SwordZipBrowseButton')
-        self.sword_zipfile_layout = QtWidgets.QHBoxLayout()
-        self.sword_zipfile_layout.addWidget(self.sword_zipfile_edit)
-        self.sword_zipfile_layout.addWidget(self.sword_zipbrowse_button)
-        self.sword_zip_layout.addRow(self.sword_zipfile_label, self.sword_zipfile_layout)
+        self.sword_zipfile_path_edit = PathEdit(
+            self.sword_zip_tab,
+            default_path=Settings().value('bibles/last directory import'),
+            dialog_caption=WizardStrings.OpenTypeFile.format(file_type=WizardStrings.SWORD),
+            show_revert=False,
+        )
+        self.sword_zip_layout.addRow(self.sword_zipfile_label, self.sword_zipfile_path_edit)
         self.sword_zipbible_label = QtWidgets.QLabel(self.sword_folder_tab)
         self.sword_zipbible_label.setObjectName('SwordZipBibleLabel')
         self.sword_zipbible_combo_box = QtWidgets.QComboBox(self.sword_zip_tab)
@@ -372,18 +345,13 @@ class BibleImportForm(OpenLPWizard):
         self.wordproject_layout.setObjectName('WordProjectLayout')
         self.wordproject_file_label = QtWidgets.QLabel(self.wordproject_widget)
         self.wordproject_file_label.setObjectName('WordProjectFileLabel')
-        self.wordproject_file_layout = QtWidgets.QHBoxLayout()
-        self.wordproject_file_layout.setObjectName('WordProjectFileLayout')
-        self.wordproject_file_edit = QtWidgets.QLineEdit(self.wordproject_widget)
-        self.wordproject_file_edit.setSizePolicy(QtWidgets.QSizePolicy.Maximum, QtWidgets.QSizePolicy.Preferred)
-        self.wordproject_file_edit.setObjectName('WordProjectFileEdit')
-        self.wordproject_file_layout.addWidget(self.wordproject_file_edit)
-        self.wordproject_browse_button = QtWidgets.QToolButton(self.wordproject_widget)
-        self.wordproject_browse_button.setIcon(self.open_icon)
-        self.wordproject_browse_button.setObjectName('WordProjectBrowseButton')
-        self.wordproject_file_layout.addWidget(self.wordproject_browse_button)
-        self.wordproject_layout.addRow(self.wordproject_file_label, self.wordproject_file_layout)
-        self.wordproject_layout.setItem(5, QtWidgets.QFormLayout.LabelRole, self.spacer)
+        self.wordproject_path_edit = PathEdit(
+            self.wordproject_widget,
+            default_path=Settings().value('bibles/last directory import'),
+            dialog_caption=WizardStrings.OpenTypeFile.format(file_type=WizardStrings.WordProject),
+            show_revert=False)
+        self.wordproject_layout.addRow(self.wordproject_file_label, self.wordproject_path_edit)
+        self.wordproject_layout.setItem(1, QtWidgets.QFormLayout.LabelRole, self.spacer)
         self.select_stack.addWidget(self.wordproject_widget)
         self.select_page_layout.addLayout(self.select_stack)
         self.addPage(self.select_page)
@@ -468,8 +436,6 @@ class BibleImportForm(OpenLPWizard):
         self.sword_bible_label.setText(translate('BiblesPlugin.ImportWizardForm', 'Bibles:'))
         self.sword_folder_label.setText(translate('BiblesPlugin.ImportWizardForm', 'SWORD data folder:'))
         self.sword_zipfile_label.setText(translate('BiblesPlugin.ImportWizardForm', 'SWORD zip-file:'))
-        self.sword_folder_edit.setPlaceholderText(translate('BiblesPlugin.ImportWizardForm',
-                                                            'Defaults to the standard SWORD data folder'))
         self.sword_zipbible_label.setText(translate('BiblesPlugin.ImportWizardForm', 'Bibles:'))
         self.sword_tab_widget.setTabText(self.sword_tab_widget.indexOf(self.sword_folder_tab),
                                          translate('BiblesPlugin.ImportWizardForm', 'Import from folder'))
@@ -518,7 +484,7 @@ class BibleImportForm(OpenLPWizard):
             if self.field('source_format') == BibleFormat.OSIS:
                 if not self.field('osis_location'):
                     critical_error_message_box(UiStrings().NFSs, WizardStrings.YouSpecifyFile % WizardStrings.OSIS)
-                    self.osis_file_edit.setFocus()
+                    self.osis_path_edit.setFocus()
                     return False
             elif self.field('source_format') == BibleFormat.CSV:
                 if not self.field('csv_booksfile'):
@@ -538,18 +504,18 @@ class BibleImportForm(OpenLPWizard):
             elif self.field('source_format') == BibleFormat.OpenSong:
                 if not self.field('opensong_file'):
                     critical_error_message_box(UiStrings().NFSs, WizardStrings.YouSpecifyFile % WizardStrings.OS)
-                    self.open_song_file_edit.setFocus()
+                    self.open_song_path_edit.setFocus()
                     return False
             elif self.field('source_format') == BibleFormat.Zefania:
                 if not self.field('zefania_file'):
                     critical_error_message_box(UiStrings().NFSs, WizardStrings.YouSpecifyFile % WizardStrings.ZEF)
-                    self.zefania_file_edit.setFocus()
+                    self.zefania_path_edit.setFocus()
                     return False
             elif self.field('source_format') == BibleFormat.WordProject:
                 if not self.field('wordproject_file'):
                     critical_error_message_box(UiStrings().NFSs,
                                                WizardStrings.YouSpecifyFile % WizardStrings.WordProject)
-                    self.wordproject_file_edit.setFocus()
+                    self.wordproject_path_edit.setFocus()
                     return False
             elif self.field('source_format') == BibleFormat.WebDownload:
                 # If count is 0 the bible list has not yet been downloaded
@@ -563,7 +529,7 @@ class BibleImportForm(OpenLPWizard):
                     if not self.field('sword_folder_path') and self.sword_bible_combo_box.count() == 0:
                         critical_error_message_box(UiStrings().NFSs,
                                                    WizardStrings.YouSpecifyFolder % WizardStrings.SWORD)
-                        self.sword_folder_edit.setFocus()
+                        self.sword_folder_path_edit.setFocus()
                         return False
                     key = self.sword_bible_combo_box.itemData(self.sword_bible_combo_box.currentIndex())
                     if 'description' in self.pysword_folder_modules_json[key]:
@@ -575,7 +541,7 @@ class BibleImportForm(OpenLPWizard):
                 elif self.sword_tab_widget.currentIndex() == self.sword_tab_widget.indexOf(self.sword_zip_tab):
                     if not self.field('sword_zip_path'):
                         critical_error_message_box(UiStrings().NFSs, WizardStrings.YouSpecifyFile % WizardStrings.SWORD)
-                        self.sword_zipfile_edit.setFocus()
+                        self.sword_zipfile_path_edit.setFocus()
                         return False
                     key = self.sword_zipbible_combo_box.itemData(self.sword_zipbible_combo_box.currentIndex())
                     if 'description' in self.pysword_zip_modules_json[key]:
@@ -586,7 +552,6 @@ class BibleImportForm(OpenLPWizard):
         elif self.currentPage() == self.license_details_page:
             license_version = self.field('license_version')
             license_copyright = self.field('license_copyright')
-            path = str(AppLocation.get_section_data_path('bibles'))
             if not license_version:
                 critical_error_message_box(
                     UiStrings().EmptyField,
@@ -608,7 +573,7 @@ class BibleImportForm(OpenLPWizard):
                               'existing one.'))
                 self.version_name_edit.setFocus()
                 return False
-            elif os.path.exists(os.path.join(path, clean_filename(license_version))):
+            elif (AppLocation.get_section_data_path('bibles') / clean_filename(license_version)).exists():
                 critical_error_message_box(
                     translate('BiblesPlugin.ImportWizardForm', 'Bible Exists'),
                     translate('BiblesPlugin.ImportWizardForm', 'This Bible already exists. Please import '
@@ -631,57 +596,6 @@ class BibleImportForm(OpenLPWizard):
             bibles.sort(key=get_locale_key)
             self.web_translation_combo_box.addItems(bibles)
 
-    def on_osis_browse_button_clicked(self):
-        """
-        Show the file open dialog for the OSIS file.
-        """
-        self.get_file_name(WizardStrings.OpenTypeFile % WizardStrings.OSIS, self.osis_file_edit,
-                           'last directory import')
-
-    def on_csv_books_browse_button_clicked(self):
-        """
-        Show the file open dialog for the books CSV file.
-        """
-        # TODO: Verify format() with varible template
-        self.get_file_name(
-            WizardStrings.OpenTypeFile % WizardStrings.CSV,
-            self.csv_books_edit,
-            'last directory import',
-            '{name} (*.csv)'.format(name=translate('BiblesPlugin.ImportWizardForm', 'CSV File')))
-
-    def on_csv_verses_browse_button_clicked(self):
-        """
-        Show the file open dialog for the verses CSV file.
-        """
-        # TODO: Verify format() with variable template
-        self.get_file_name(WizardStrings.OpenTypeFile % WizardStrings.CSV, self.csv_verses_edit,
-                           'last directory import',
-                           '{name} (*.csv)'.format(name=translate('BiblesPlugin.ImportWizardForm', 'CSV File')))
-
-    def on_open_song_browse_button_clicked(self):
-        """
-        Show the file open dialog for the OpenSong file.
-        """
-        # TODO: Verify format() with variable template
-        self.get_file_name(WizardStrings.OpenTypeFile % WizardStrings.OS, self.open_song_file_edit,
-                           'last directory import')
-
-    def on_zefania_browse_button_clicked(self):
-        """
-        Show the file open dialog for the Zefania file.
-        """
-        # TODO: Verify format() with variable template
-        self.get_file_name(WizardStrings.OpenTypeFile % WizardStrings.ZEF, self.zefania_file_edit,
-                           'last directory import')
-
-    def on_wordproject_browse_button_clicked(self):
-        """
-        Show the file open dialog for the WordProject file.
-        """
-        # TODO: Verify format() with variable template
-        self.get_file_name(WizardStrings.OpenTypeFile % WizardStrings.WordProject, self.wordproject_file_edit,
-                           'last directory import')
-
     def on_web_update_button_clicked(self):
         """
         Download list of bibles from Crosswalk, BibleServer and BibleGateway.
@@ -718,15 +632,13 @@ class BibleImportForm(OpenLPWizard):
         self.web_update_button.setEnabled(True)
         self.web_progress_bar.setVisible(False)
 
-    def on_sword_browse_button_clicked(self):
+    def on_sword_folder_path_edit_path_changed(self, new_path):
         """
         Show the file open dialog for the SWORD folder.
         """
-        self.get_folder(WizardStrings.OpenTypeFolder % WizardStrings.SWORD, self.sword_folder_edit,
-                        'last directory import')
-        if self.sword_folder_edit.text():
+        if new_path:
             try:
-                self.pysword_folder_modules = modules.SwordModules(self.sword_folder_edit.text())
+                self.pysword_folder_modules = modules.SwordModules(str(new_path))
                 self.pysword_folder_modules_json = self.pysword_folder_modules.parse_modules()
                 bible_keys = self.pysword_folder_modules_json.keys()
                 self.sword_bible_combo_box.clear()
@@ -735,15 +647,13 @@ class BibleImportForm(OpenLPWizard):
             except:
                 self.sword_bible_combo_box.clear()
 
-    def on_sword_zipbrowse_button_clicked(self):
+    def on_sword_zipfile_path_edit_path_changed(self, new_path):
         """
         Show the file open dialog for a SWORD zip-file.
         """
-        self.get_file_name(WizardStrings.OpenTypeFile % WizardStrings.SWORD, self.sword_zipfile_edit,
-                           'last directory import')
-        if self.sword_zipfile_edit.text():
+        if new_path:
             try:
-                self.pysword_zip_modules = modules.SwordModules(self.sword_zipfile_edit.text())
+                self.pysword_zip_modules = modules.SwordModules(str(new_path))
                 self.pysword_zip_modules_json = self.pysword_zip_modules.parse_modules()
                 bible_keys = self.pysword_zip_modules_json.keys()
                 self.sword_zipbible_combo_box.clear()
@@ -757,16 +667,16 @@ class BibleImportForm(OpenLPWizard):
         Register the bible import wizard fields.
         """
         self.select_page.registerField('source_format', self.format_combo_box)
-        self.select_page.registerField('osis_location', self.osis_file_edit)
-        self.select_page.registerField('csv_booksfile', self.csv_books_edit)
-        self.select_page.registerField('csv_versefile', self.csv_verses_edit)
-        self.select_page.registerField('opensong_file', self.open_song_file_edit)
-        self.select_page.registerField('zefania_file', self.zefania_file_edit)
-        self.select_page.registerField('wordproject_file', self.wordproject_file_edit)
+        self.select_page.registerField('osis_location', self.osis_path_edit, 'path', PathEdit.pathChanged)
+        self.select_page.registerField('csv_booksfile', self.csv_books_path_edit, 'path', PathEdit.pathChanged)
+        self.select_page.registerField('csv_versefile', self.csv_verses_path_edit, 'path', PathEdit.pathChanged)
+        self.select_page.registerField('opensong_file', self.open_song_path_edit, 'path', PathEdit.pathChanged)
+        self.select_page.registerField('zefania_file', self.zefania_path_edit, 'path', PathEdit.pathChanged)
+        self.select_page.registerField('wordproject_file', self.wordproject_path_edit, 'path', PathEdit.pathChanged)
         self.select_page.registerField('web_location', self.web_source_combo_box)
         self.select_page.registerField('web_biblename', self.web_translation_combo_box)
-        self.select_page.registerField('sword_folder_path', self.sword_folder_edit)
-        self.select_page.registerField('sword_zip_path', self.sword_zipfile_edit)
+        self.select_page.registerField('sword_folder_path', self.sword_folder_path_edit, 'path', PathEdit.pathChanged)
+        self.select_page.registerField('sword_zip_path', self.sword_zipfile_path_edit, 'path', PathEdit.pathChanged)
         self.select_page.registerField('proxy_server', self.web_server_edit)
         self.select_page.registerField('proxy_username', self.web_user_edit)
         self.select_page.registerField('proxy_password', self.web_password_edit)
@@ -785,13 +695,13 @@ class BibleImportForm(OpenLPWizard):
         self.finish_button.setVisible(False)
         self.cancel_button.setVisible(True)
         self.setField('source_format', 0)
-        self.setField('osis_location', '')
-        self.setField('csv_booksfile', '')
-        self.setField('csv_versefile', '')
-        self.setField('opensong_file', '')
-        self.setField('zefania_file', '')
-        self.setField('sword_folder_path', '')
-        self.setField('sword_zip_path', '')
+        self.setField('osis_location', None)
+        self.setField('csv_booksfile', None)
+        self.setField('csv_versefile', None)
+        self.setField('opensong_file', None)
+        self.setField('zefania_file', None)
+        self.setField('sword_folder_path', None)
+        self.setField('sword_zip_path', None)
         self.setField('web_location', WebDownload.Crosswalk)
         self.setField('web_biblename', self.web_translation_combo_box.currentIndex())
         self.setField('proxy_server', settings.value('proxy address'))
@@ -833,16 +743,16 @@ class BibleImportForm(OpenLPWizard):
         if bible_type == BibleFormat.OSIS:
             # Import an OSIS bible.
             importer = self.manager.import_bible(BibleFormat.OSIS, name=license_version,
-                                                 filename=self.field('osis_location'))
+                                                 file_path=self.field('osis_location'))
         elif bible_type == BibleFormat.CSV:
             # Import a CSV bible.
             importer = self.manager.import_bible(BibleFormat.CSV, name=license_version,
-                                                 booksfile=self.field('csv_booksfile'),
-                                                 versefile=self.field('csv_versefile'))
+                                                 books_path=self.field('csv_booksfile'),
+                                                 verse_path=self.field('csv_versefile'))
         elif bible_type == BibleFormat.OpenSong:
             # Import an OpenSong bible.
             importer = self.manager.import_bible(BibleFormat.OpenSong, name=license_version,
-                                                 filename=self.field('opensong_file'))
+                                                 file_path=self.field('opensong_file'))
         elif bible_type == BibleFormat.WebDownload:
             # Import a bible from the web.
             self.progress_bar.setMaximum(1)
@@ -861,11 +771,11 @@ class BibleImportForm(OpenLPWizard):
         elif bible_type == BibleFormat.Zefania:
             # Import a Zefania bible.
             importer = self.manager.import_bible(BibleFormat.Zefania, name=license_version,
-                                                 filename=self.field('zefania_file'))
+                                                 file_path=self.field('zefania_file'))
         elif bible_type == BibleFormat.WordProject:
             # Import a WordProject bible.
             importer = self.manager.import_bible(BibleFormat.WordProject, name=license_version,
-                                                 filename=self.field('wordproject_file'))
+                                                 file_path=self.field('wordproject_file'))
         elif bible_type == BibleFormat.SWORD:
             # Import a SWORD bible.
             if self.sword_tab_widget.currentIndex() == self.sword_tab_widget.indexOf(self.sword_folder_tab):

openlp/plugins/bibles/forms/booknameform.py

@@ -113,8 +113,7 @@ class BookNameForm(QDialog, Ui_BookNameDialog):
             cor_book = self.corresponding_combo_box.currentText()
             for character in '\\.^$*+?{}[]()':
                 cor_book = cor_book.replace(character, '\\' + character)
-            books = [key for key in list(self.book_names.keys()) if re.match(cor_book, str(self.book_names[key]),
-                                                                             re.UNICODE)]
+            books = [key for key in list(self.book_names.keys()) if re.match(cor_book, str(self.book_names[key]))]
             books = [_f for _f in map(BiblesResourcesDB.get_book, books) if _f]
             if books:
                 self.book_id = books[0]['id']

openlp/plugins/bibles/lib/__init__.py

@@ -224,13 +224,13 @@ def update_reference_separators():
     range_regex = '(?:(?P<from_chapter>[0-9]+){sep_v})?' \
         '(?P<from_verse>[0-9]+)(?P<range_to>{sep_r}(?:(?:(?P<to_chapter>' \
         '[0-9]+){sep_v})?(?P<to_verse>[0-9]+)|{sep_e})?)?'.format_map(REFERENCE_SEPARATORS)
-    REFERENCE_MATCHES['range'] = re.compile(r'^\s*{range}\s*$'.format(range=range_regex), re.UNICODE)
-    REFERENCE_MATCHES['range_separator'] = re.compile(REFERENCE_SEPARATORS['sep_l'], re.UNICODE)
+    REFERENCE_MATCHES['range'] = re.compile(r'^\s*{range}\s*$'.format(range=range_regex))
+    REFERENCE_MATCHES['range_separator'] = re.compile(REFERENCE_SEPARATORS['sep_l'])
     # full reference match: <book>(<range>(,(?!$)|(?=$)))+
     REFERENCE_MATCHES['full'] = \
         re.compile(r'^\s*(?!\s)(?P<book>[\d]*[.]?[^\d\.]+)\.*(?<!\s)\s*'
                    r'(?P<ranges>(?:{range_regex}(?:{sep_l}(?!\s*$)|(?=\s*$)))+)\s*$'.format(
-                       range_regex=range_regex, sep_l=REFERENCE_SEPARATORS['sep_l']), re.UNICODE)
+                       range_regex=range_regex, sep_l=REFERENCE_SEPARATORS['sep_l']))
 
 
 def get_reference_separator(separator_type):

openlp/plugins/bibles/lib/bibleimport.py

@@ -37,23 +37,23 @@ class BibleImport(BibleDB, LogMixin, RegistryProperties):
     """
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
-        self.filename = kwargs['filename'] if 'filename' in kwargs else None
+        self.file_path = kwargs.get('file_path')
         self.wizard = None
         self.stop_import_flag = False
         Registry().register_function('openlp_stop_wizard', self.stop_import)
 
     @staticmethod
-    def is_compressed(file):
+    def is_compressed(file_path):
         """
         Check if the supplied file is compressed
 
-        :param file: A path to the file to check
+        :param file_path: A path to the file to check
         """
-        if is_zipfile(file):
+        if is_zipfile(str(file_path)):
             critical_error_message_box(
                 message=translate('BiblesPlugin.BibleImport',
                                   'The file "{file}" you supplied is compressed. You must decompress it before import.'
-                                  ).format(file=file))
+                                  ).format(file=file_path))
             return True
         return False
 
@@ -143,24 +143,24 @@ class BibleImport(BibleDB, LogMixin, RegistryProperties):
             self.log_debug('No book name supplied. Falling back to guess_id')
             book_ref_id = guess_id
         if not book_ref_id:
-            raise ValidationError(msg='Could not resolve book_ref_id in "{}"'.format(self.filename))
+            raise ValidationError(msg='Could not resolve book_ref_id in "{}"'.format(self.file_path))
         book_details = BiblesResourcesDB.get_book_by_id(book_ref_id)
         if book_details is None:
             raise ValidationError(msg='book_ref_id: {book_ref} Could not be found in the BibleResourcesDB while '
-                                      'importing {file}'.format(book_ref=book_ref_id, file=self.filename))
+                                      'importing {file}'.format(book_ref=book_ref_id, file=self.file_path))
         return self.create_book(name, book_ref_id, book_details['testament_id'])
 
-    def parse_xml(self, filename, use_objectify=False, elements=None, tags=None):
+    def parse_xml(self, file_path, use_objectify=False, elements=None, tags=None):
         """
         Parse and clean the supplied file by removing any elements or tags we don't use.
-        :param filename: The filename of the xml file to parse. Str
+        :param file_path: The filename of the xml file to parse. Str
         :param use_objectify: Use the objectify parser rather than the etree parser. (Bool)
         :param elements: A tuple of element names (Str) to remove along with their content.
         :param tags: A tuple of element names (Str) to remove, preserving their content.
         :return: The root element of the xml document
         """
         try:
-            with open(filename, 'rb') as import_file:
+            with file_path.open('rb') as import_file:
                 # NOTE: We don't need to do any of the normal encoding detection here, because lxml does it's own
                 # encoding detection, and the two mechanisms together interfere with each other.
                 if not use_objectify:
@@ -207,17 +207,17 @@ class BibleImport(BibleDB, LogMixin, RegistryProperties):
         self.log_debug('Stopping import')
         self.stop_import_flag = True
 
-    def validate_xml_file(self, filename, tag):
+    def validate_xml_file(self, file_path, tag):
         """
         Validate the supplied file
 
-        :param filename: The supplied file
+        :param file_path: The supplied file
         :param tag: The expected root tag type
         :return: True if valid. ValidationError is raised otherwise.
         """
-        if BibleImport.is_compressed(filename):
+        if BibleImport.is_compressed(file_path):
             raise ValidationError(msg='Compressed file')
-        bible = self.parse_xml(filename, use_objectify=True)
+        bible = self.parse_xml(file_path, use_objectify=True)
         if bible is None:
             raise ValidationError(msg='Error when opening file')
         root_tag = bible.tag.lower()

openlp/plugins/bibles/lib/biblestab.py

@@ -41,11 +41,11 @@ class BiblesTab(SettingsTab):
     """
     log.info('Bible Tab loaded')
 
-    def _init_(self, parent, title, visible_title, icon_path):
+    def _init_(self, *args, **kwargs):
         self.paragraph_style = True
         self.show_new_chapters = False
         self.display_style = 0
-        super(BiblesTab, self).__init__(parent, title, visible_title, icon_path)
+        super().__init__(*args, **kwargs)
 
     def setupUi(self):
         self.setObjectName('BiblesTab')

openlp/plugins/bibles/lib/db.py

@@ -35,6 +35,7 @@ from sqlalchemy.orm.exc import UnmappedClassError
 from openlp.core.common import clean_filename
 from openlp.core.common.applocation import AppLocation
 from openlp.core.common.i18n import translate
+from openlp.core.common.path import Path
 from openlp.core.lib.db import BaseModel, init_db, Manager
 from openlp.core.lib.ui import critical_error_message_box
 from openlp.plugins.bibles.lib import BibleStrings, LanguageSelection, upgrade
@@ -129,10 +130,15 @@ class BibleDB(Manager):
         :param parent:
         :param kwargs:
             ``path``
-                The path to the bible database file.
+                The path to the bible database file. Type: openlp.core.common.path.Path
 
             ``name``
                 The name of the database. This is also used as the file name for SQLite databases.
+
+            ``file``
+                Type: openlp.core.common.path.Path
+
+        :rtype: None
         """
         log.info('BibleDB loaded')
         self._setup(parent, **kwargs)
@@ -145,20 +151,20 @@ class BibleDB(Manager):
         self.session = None
         if 'path' not in kwargs:
             raise KeyError('Missing keyword argument "path".')
+        self.path = kwargs['path']
         if 'name' not in kwargs and 'file' not in kwargs:
             raise KeyError('Missing keyword argument "name" or "file".')
         if 'name' in kwargs:
             self.name = kwargs['name']
             if not isinstance(self.name, str):
                 self.name = str(self.name, 'utf-8')
-            self.file = clean_filename(self.name) + '.sqlite'
+            # TODO: To path object
+            file_path = Path(clean_filename(self.name) + '.sqlite')
         if 'file' in kwargs:
-            self.file = kwargs['file']
-        Manager.__init__(self, 'bibles', init_schema, self.file, upgrade)
+            file_path = kwargs['file']
+        Manager.__init__(self, 'bibles', init_schema, file_path, upgrade)
         if self.session and 'file' in kwargs:
                 self.get_name()
-        if 'path' in kwargs:
-            self.path = kwargs['path']
         self._is_web_bible = None
 
     def get_name(self):
@@ -307,8 +313,7 @@ class BibleDB(Manager):
         book_escaped = book
         for character in RESERVED_CHARACTERS:
             book_escaped = book_escaped.replace(character, '\\' + character)
-        regex_book = re.compile('\\s*{book}\\s*'.format(book='\\s*'.join(book_escaped.split())),
-                                re.UNICODE | re.IGNORECASE)
+        regex_book = re.compile('\\s*{book}\\s*'.format(book='\\s*'.join(book_escaped.split())), re.IGNORECASE)
         if language_selection == LanguageSelection.Bible:
             db_book = self.get_book(book)
             if db_book:
@@ -471,9 +476,9 @@ class BiblesResourcesDB(QtCore.QObject, Manager):
         Return the cursor object. Instantiate one if it doesn't exist yet.
         """
         if BiblesResourcesDB.cursor is None:
-            file_path = os.path.join(str(AppLocation.get_directory(AppLocation.PluginsDir)),
-                                     'bibles', 'resources', 'bibles_resources.sqlite')
-            conn = sqlite3.connect(file_path)
+            file_path = \
+                AppLocation.get_directory(AppLocation.PluginsDir) / 'bibles' / 'resources' / 'bibles_resources.sqlite'
+            conn = sqlite3.connect(str(file_path))
             BiblesResourcesDB.cursor = conn.cursor()
         return BiblesResourcesDB.cursor
 
@@ -759,17 +764,13 @@ class AlternativeBookNamesDB(QtCore.QObject, Manager):
         If necessary loads up the database and creates the tables if the database doesn't exist.
         """
         if AlternativeBookNamesDB.cursor is None:
-            file_path = os.path.join(
-                str(AppLocation.get_directory(AppLocation.DataDir)), 'bibles', 'alternative_book_names.sqlite')
-            if not os.path.exists(file_path):
+            file_path = AppLocation.get_directory(AppLocation.DataDir) / 'bibles' / 'alternative_book_names.sqlite'
+            AlternativeBookNamesDB.conn = sqlite3.connect(str(file_path))
+            if not file_path.exists():
                 # create new DB, create table alternative_book_names
-                AlternativeBookNamesDB.conn = sqlite3.connect(file_path)
                 AlternativeBookNamesDB.conn.execute(
                     'CREATE TABLE alternative_book_names(id INTEGER NOT NULL, '
                     'book_reference_id INTEGER, language_id INTEGER, name VARCHAR(50), PRIMARY KEY (id))')
-            else:
-                # use existing DB
-                AlternativeBookNamesDB.conn = sqlite3.connect(file_path)
             AlternativeBookNamesDB.cursor = AlternativeBookNamesDB.conn.cursor()
         return AlternativeBookNamesDB.cursor
 

openlp/plugins/bibles/lib/importers/__init__.py

@@ -19,3 +19,6 @@
 # with this program; if not, write to the Free Software Foundation, Inc., 59  #
 # Temple Place, Suite 330, Boston, MA 02111-1307 USA                          #
 ###############################################################################
+"""
+The :mod:`~openlp.plugins.bibles.lib.importers` module contains importers for the Bibles plugin.
+"""

openlp/plugins/bibles/lib/importers/csvbible.py

@@ -73,8 +73,8 @@ class CSVBible(BibleImport):
         """
         super().__init__(*args, **kwargs)
         self.log_info(self.__class__.__name__)
-        self.books_file = kwargs['booksfile']
-        self.verses_file = kwargs['versefile']
+        self.books_path = kwargs['books_path']
+        self.verses_path = kwargs['verse_path']
 
     @staticmethod
     def get_book_name(name, books):
@@ -92,21 +92,22 @@ class CSVBible(BibleImport):
         return book_name
 
     @staticmethod
-    def parse_csv_file(filename, results_tuple):
+    def parse_csv_file(file_path, results_tuple):
         """
         Parse the supplied CSV file.
 
-        :param filename: The name of the file to parse. Str
-        :param results_tuple: The namedtuple to use to store the results. namedtuple
-        :return: An iterable yielding namedtuples of type results_tuple
+        :param openlp.core.common.path.Path file_path: The name of the file to parse.
+        :param namedtuple results_tuple: The namedtuple to use to store the results.
+        :return: An list of namedtuples of type results_tuple
+        :rtype: list[namedtuple]
         """
         try:
-            encoding = get_file_encoding(Path(filename))['encoding']
-            with open(filename, 'r', encoding=encoding, newline='') as csv_file:
+            encoding = get_file_encoding(file_path)['encoding']
+            with file_path.open('r', encoding=encoding, newline='') as csv_file:
                 csv_reader = csv.reader(csv_file, delimiter=',', quotechar='"')
                 return [results_tuple(*line) for line in csv_reader]
         except (OSError, csv.Error):
-            raise ValidationError(msg='Parsing "{file}" failed'.format(file=filename))
+            raise ValidationError(msg='Parsing "{file}" failed'.format(file=file_path))
 
     def process_books(self, books):
         """
@@ -159,12 +160,12 @@ class CSVBible(BibleImport):
         self.language_id = self.get_language(bible_name)
         if not self.language_id:
             return False
-        books = self.parse_csv_file(self.books_file, Book)
+        books = self.parse_csv_file(self.books_path, Book)
         self.wizard.progress_bar.setValue(0)
         self.wizard.progress_bar.setMinimum(0)
         self.wizard.progress_bar.setMaximum(len(books))
         book_list = self.process_books(books)
-        verses = self.parse_csv_file(self.verses_file, Verse)
+        verses = self.parse_csv_file(self.verses_path, Verse)
         self.wizard.progress_bar.setValue(0)
         self.wizard.progress_bar.setMaximum(len(books) + 1)
         self.process_verses(verses, book_list)

openlp/plugins/bibles/lib/importers/opensong.py

@@ -46,7 +46,8 @@ def parse_chapter_number(number, previous_number):
 
     :param number: The raw data from the xml
     :param previous_number: The previous chapter number
-    :return: Number of current chapter. (Int)
+    :return: Number of current chapter.
+    :rtype: int
     """
     if number:
         return int(number.split()[-1])
@@ -132,13 +133,13 @@ class OpenSongBible(BibleImport):
         :param bible_name: The name of the bible being imported
         :return: True if import completed, False if import was unsuccessful
         """
-        self.log_debug('Starting OpenSong import from "{name}"'.format(name=self.filename))
-        self.validate_xml_file(self.filename, 'bible')
-        bible = self.parse_xml(self.filename, use_objectify=True)
+        self.log_debug('Starting OpenSong import from "{name}"'.format(name=self.file_path))
+        self.validate_xml_file(self.file_path, 'bible')
+        bible = self.parse_xml(self.file_path, use_objectify=True)
         if bible is None:
             return False
         # No language info in the opensong format, so ask the user
-        self.language_id = self.get_language_id(bible_name=self.filename)
+        self.language_id = self.get_language_id(bible_name=str(self.file_path))
         if not self.language_id:
             return False
         self.process_books(bible.b)

openlp/plugins/bibles/lib/importers/osis.py

@@ -159,14 +159,14 @@ class OSISBible(BibleImport):
         """
         Loads a Bible from file.
         """
-        self.log_debug('Starting OSIS import from "{name}"'.format(name=self.filename))
-        self.validate_xml_file(self.filename, '{http://www.bibletechnologies.net/2003/osis/namespace}osis')
-        bible = self.parse_xml(self.filename, elements=REMOVABLE_ELEMENTS, tags=REMOVABLE_TAGS)
+        self.log_debug('Starting OSIS import from "{name}"'.format(name=self.file_path))
+        self.validate_xml_file(self.file_path, '{http://www.bibletechnologies.net/2003/osis/namespace}osis')
+        bible = self.parse_xml(self.file_path, elements=REMOVABLE_ELEMENTS, tags=REMOVABLE_TAGS)
         if bible is None:
             return False
         # Find bible language
         language = bible.xpath("//ns:osisText/@xml:lang", namespaces=NS)
-        self.language_id = self.get_language_id(language[0] if language else None, bible_name=self.filename)
+        self.language_id = self.get_language_id(language[0] if language else None, bible_name=str(self.file_path))
         if not self.language_id:
             return False
         self.process_books(bible)

openlp/plugins/bibles/lib/importers/sword.py

@@ -60,7 +60,7 @@ class SwordBible(BibleImport):
             bible = pysword_modules.get_bible_from_module(self.sword_key)
             language = pysword_module_json['lang']
             language = language[language.find('.') + 1:]
-            language_id = self.get_language_id(language, bible_name=self.filename)
+            language_id = self.get_language_id(language, bible_name=str(self.file_path))
             if not language_id:
                 return False
             books = bible.get_structure().get_books()

openlp/plugins/bibles/lib/importers/wordproject.py

@@ -19,15 +19,14 @@
 # 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 re
 import logging
-from codecs import open as copen
+import re
 from tempfile import TemporaryDirectory
 from zipfile import ZipFile
 
 from bs4 import BeautifulSoup, Tag, NavigableString
 
+from openlp.core.common.path import Path
 from openlp.plugins.bibles.lib.bibleimport import BibleImport
 
 BOOK_NUMBER_PATTERN = re.compile(r'\[(\d+)\]')
@@ -51,9 +50,9 @@ class WordProjectBible(BibleImport):
         Unzip the file to a temporary directory
         """
         self.tmp = TemporaryDirectory()
-        zip_file = ZipFile(os.path.abspath(self.filename))
-        zip_file.extractall(self.tmp.name)
-        self.base_dir = os.path.join(self.tmp.name, os.path.splitext(os.path.basename(self.filename))[0])
+        with ZipFile(str(self.file_path)) as zip_file:
+            zip_file.extractall(self.tmp.name)
+        self.base_path = Path(self.tmp.name, self.file_path.stem)
 
     def process_books(self):
         """
@@ -62,8 +61,7 @@ class WordProjectBible(BibleImport):
         :param bible_data: parsed xml
         :return: None
         """
-        with copen(os.path.join(self.base_dir, 'index.htm'), encoding='utf-8', errors='ignore') as index_file:
-            page = index_file.read()
+        page = (self.base_path / 'index.htm').read_text(encoding='utf-8', errors='ignore')
         soup = BeautifulSoup(page, 'lxml')
         bible_books = soup.find('div', 'textOptions').find_all('li')
         book_count = len(bible_books)
@@ -93,9 +91,7 @@ class WordProjectBible(BibleImport):
         :return: None
         """
         log.debug(book_link)
-        book_file = os.path.join(self.base_dir, os.path.normpath(book_link))
-        with copen(book_file, encoding='utf-8', errors='ignore') as f:
-            page = f.read()
+        page = (self.base_path / book_link).read_text(encoding='utf-8', errors='ignore')
         soup = BeautifulSoup(page, 'lxml')
         header_div = soup.find('div', 'textHeader')
         chapters_p = header_div.find('p')
@@ -114,9 +110,8 @@ class WordProjectBible(BibleImport):
         """
         Get the verses for a particular book
         """
-        chapter_file_name = os.path.join(self.base_dir, '{:02d}'.format(book_number), '{}.htm'.format(chapter_number))
-        with copen(chapter_file_name, encoding='utf-8', errors='ignore') as chapter_file:
-            page = chapter_file.read()
+        chapter_file_path = self.base_path / '{:02d}'.format(book_number) / '{}.htm'.format(chapter_number)
+        page = chapter_file_path.read_text(encoding='utf-8', errors='ignore')
         soup = BeautifulSoup(page, 'lxml')
         text_body = soup.find('div', 'textBody')
         if text_body:
@@ -158,9 +153,9 @@ class WordProjectBible(BibleImport):
         """
         Loads a Bible from file.
         """
-        self.log_debug('Starting WordProject import from "{name}"'.format(name=self.filename))
+        self.log_debug('Starting WordProject import from "{name}"'.format(name=self.file_path))
         self._unzip_file()
-        self.language_id = self.get_language_id(None, bible_name=self.filename)
+        self.language_id = self.get_language_id(None, bible_name=str(self.file_path))
         result = False
         if self.language_id:
             self.process_books()

openlp/plugins/bibles/lib/importers/zefania.py

@@ -45,13 +45,13 @@ class ZefaniaBible(BibleImport):
         """
         Loads a Bible from file.
         """
-        log.debug('Starting Zefania import from "{name}"'.format(name=self.filename))
+        log.debug('Starting Zefania import from "{name}"'.format(name=self.file_path))
         success = True
         try:
-            xmlbible = self.parse_xml(self.filename, elements=REMOVABLE_ELEMENTS, tags=REMOVABLE_TAGS)
+            xmlbible = self.parse_xml(self.file_path, elements=REMOVABLE_ELEMENTS, tags=REMOVABLE_TAGS)
             # Find bible language
             language = xmlbible.xpath("/XMLBIBLE/INFORMATION/language/text()")
-            language_id = self.get_language_id(language[0] if language else None, bible_name=self.filename)
+            language_id = self.get_language_id(language[0] if language else None, bible_name=str(self.file_path))
             if not language_id:
                 return False
             no_of_books = int(xmlbible.xpath('count(//BIBLEBOOK)'))
@@ -69,7 +69,7 @@ class ZefaniaBible(BibleImport):
                     log.debug('Could not find a name, will use number, basically a guess.')
                     book_ref_id = int(bnumber)
                 if not book_ref_id:
-                    log.error('Importing books from "{name}" failed'.format(name=self.filename))
+                    log.error('Importing books from "{name}" failed'.format(name=self.file_path))
                     return False
                 book_details = BiblesResourcesDB.get_book_by_id(book_ref_id)
                 db_book = self.create_book(book_details['name'], book_ref_id, book_details['testament_id'])

openlp/plugins/bibles/lib/manager.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 logging
 
 from openlp.core.common import delete_file
@@ -115,7 +114,7 @@ class BibleManager(LogMixin, RegistryProperties):
         self.settings_section = 'bibles'
         self.web = 'Web'
         self.db_cache = None
-        self.path = str(AppLocation.get_section_data_path(self.settings_section))
+        self.path = AppLocation.get_section_data_path(self.settings_section)
         self.proxy_name = Settings().value(self.settings_section + '/proxy name')
         self.suffix = '.sqlite'
         self.import_wizard = None
@@ -128,20 +127,20 @@ class BibleManager(LogMixin, RegistryProperties):
         of HTTPBible is loaded instead of the BibleDB class.
         """
         log.debug('Reload bibles')
-        files = [str(file) for file in AppLocation.get_files(self.settings_section, self.suffix)]
-        if 'alternative_book_names.sqlite' in files:
-            files.remove('alternative_book_names.sqlite')
-        log.debug('Bible Files {text}'.format(text=files))
+        file_paths = AppLocation.get_files(self.settings_section, self.suffix)
+        if Path('alternative_book_names.sqlite') in file_paths:
+            file_paths.remove(Path('alternative_book_names.sqlite'))
+        log.debug('Bible Files {text}'.format(text=file_paths))
         self.db_cache = {}
-        for filename in files:
-            bible = BibleDB(self.parent, path=self.path, file=filename)
+        for file_path in file_paths:
+            bible = BibleDB(self.parent, path=self.path, file=file_path)
             if not bible.session:
                 continue
             name = bible.get_name()
             # Remove corrupted files.
             if name is None:
                 bible.session.close_all()
-                delete_file(Path(self.path, filename))
+                delete_file(self.path / file_path)
                 continue
             log.debug('Bible Name: "{name}"'.format(name=name))
             self.db_cache[name] = bible
@@ -150,7 +149,7 @@ class BibleManager(LogMixin, RegistryProperties):
                 source = self.db_cache[name].get_object(BibleMeta, 'download_source')
                 download_name = self.db_cache[name].get_object(BibleMeta, 'download_name').value
                 meta_proxy = self.db_cache[name].get_object(BibleMeta, 'proxy_server')
-                web_bible = HTTPBible(self.parent, path=self.path, file=filename, download_source=source.value,
+                web_bible = HTTPBible(self.parent, path=self.path, file=file_path, download_source=source.value,
                                       download_name=download_name)
                 if meta_proxy:
                     web_bible.proxy_server = meta_proxy.value

openlp/plugins/custom/lib/db.py

@@ -26,7 +26,7 @@ the Custom plugin
 from sqlalchemy import Column, Table, types
 from sqlalchemy.orm import mapper
 
-from openlp.core.common.i18n import get_locale_key
+from openlp.core.common.i18n import get_natural_key
 from openlp.core.lib.db import BaseModel, init_db
 
 
@@ -36,10 +36,10 @@ class CustomSlide(BaseModel):
     """
     # By default sort the customs by its title considering language specific characters.
     def __lt__(self, other):
-        return get_locale_key(self.title) < get_locale_key(other.title)
+        return get_natural_key(self.title) < get_natural_key(other.title)
 
     def __eq__(self, other):
-        return get_locale_key(self.title) == get_locale_key(other.title)
+        return get_natural_key(self.title) == get_natural_key(other.title)
 
     def __hash__(self):
         """

openlp/plugins/images/lib/mediaitem.py

@@ -26,7 +26,7 @@ from PyQt5 import QtCore, QtGui, QtWidgets
 
 from openlp.core.common import delete_file, get_images_filter
 from openlp.core.common.applocation import AppLocation
-from openlp.core.common.i18n import UiStrings, translate, get_locale_key
+from openlp.core.common.i18n import UiStrings, translate, get_natural_key
 from openlp.core.common.path import Path, create_paths
 from openlp.core.common.registry import Registry
 from openlp.core.common.settings import Settings
@@ -81,8 +81,12 @@ class ImageMediaItem(MediaManagerItem):
         self.add_group_action.setToolTip(UiStrings().AddGroupDot)
         self.replace_action.setText(UiStrings().ReplaceBG)
         self.replace_action.setToolTip(UiStrings().ReplaceLiveBG)
+        self.replace_action_context.setText(UiStrings().ReplaceBG)
+        self.replace_action_context.setToolTip(UiStrings().ReplaceLiveBG)
         self.reset_action.setText(UiStrings().ResetBG)
         self.reset_action.setToolTip(UiStrings().ResetLiveBG)
+        self.reset_action_context.setText(UiStrings().ResetBG)
+        self.reset_action_context.setToolTip(UiStrings().ResetLiveBG)
 
     def required_icons(self):
         """
@@ -184,6 +188,13 @@ class ImageMediaItem(MediaManagerItem):
             self.list_view,
             text=translate('ImagePlugin', 'Add new image(s)'),
             icon=':/general/general_open.png', triggers=self.on_file_click)
+        create_widget_action(self.list_view, separator=True)
+        self.replace_action_context = create_widget_action(
+            self.list_view, text=UiStrings().ReplaceBG, icon=':/slides/slide_theme.png',
+            triggers=self.on_replace_click)
+        self.reset_action_context = create_widget_action(
+            self.list_view, text=UiStrings().ReplaceLiveBG, icon=':/system/system_close.png',
+            visible=False, triggers=self.on_reset_click)
 
     def add_start_header_bar(self):
         """
@@ -271,7 +282,7 @@ class ImageMediaItem(MediaManagerItem):
         :param parent_group_id: The ID of the group that will be added recursively.
         """
         image_groups = self.manager.get_all_objects(ImageGroups, ImageGroups.parent_id == parent_group_id)
-        image_groups.sort(key=lambda group_object: get_locale_key(group_object.group_name))
+        image_groups.sort(key=lambda group_object: get_natural_key(group_object.group_name))
         folder_icon = build_icon(':/images/image_group.png')
         for image_group in image_groups:
             group = QtWidgets.QTreeWidgetItem()
@@ -298,7 +309,7 @@ class ImageMediaItem(MediaManagerItem):
             combobox.clear()
             combobox.top_level_group_added = False
         image_groups = self.manager.get_all_objects(ImageGroups, ImageGroups.parent_id == parent_group_id)
-        image_groups.sort(key=lambda group_object: get_locale_key(group_object.group_name))
+        image_groups.sort(key=lambda group_object: get_natural_key(group_object.group_name))
         for image_group in image_groups:
             combobox.addItem(prefix + image_group.group_name, image_group.id)
             self.fill_groups_combobox(combobox, image_group.id, prefix + '   ')
@@ -355,7 +366,7 @@ class ImageMediaItem(MediaManagerItem):
             self.expand_group(open_group.id)
         # Sort the images by its filename considering language specific.
         # characters.
-        images.sort(key=lambda image_object: get_locale_key(image_object.file_path.name))
+        images.sort(key=lambda image_object: get_natural_key(image_object.file_path.name))
         for image in images:
             log.debug('Loading image: {name}'.format(name=image.file_path))
             file_name = image.file_path.name
@@ -390,6 +401,7 @@ class ImageMediaItem(MediaManagerItem):
         :param files: A List of strings containing the filenames of the files to be loaded
         :param target_group: The QTreeWidgetItem of the group that will be the parent of the added files
         """
+        file_paths = [Path(file) for file in file_paths]
         self.application.set_normal_cursor()
         self.load_list(file_paths, target_group)
         last_dir = file_paths[0].parent
@@ -532,9 +544,9 @@ class ImageMediaItem(MediaManagerItem):
                 group_items.append(item)
             if isinstance(item.data(0, QtCore.Qt.UserRole), ImageFilenames):
                 image_items.append(item)
-        group_items.sort(key=lambda item: get_locale_key(item.text(0)))
+        group_items.sort(key=lambda item: get_natural_key(item.text(0)))
         target_group.addChildren(group_items)
-        image_items.sort(key=lambda item: get_locale_key(item.text(0)))
+        image_items.sort(key=lambda item: get_natural_key(item.text(0)))
         target_group.addChildren(image_items)
 
     def generate_slide_data(self, service_item, item=None, xml_version=False, remote=False,
@@ -658,6 +670,7 @@ class ImageMediaItem(MediaManagerItem):
         Called to reset the Live background with the image selected.
         """
         self.reset_action.setVisible(False)
+        self.reset_action_context.setVisible(False)
         self.live_controller.display.reset_image()
 
     def live_theme_changed(self):
@@ -665,6 +678,7 @@ class ImageMediaItem(MediaManagerItem):
         Triggered by the change of theme in the slide controller.
         """
         self.reset_action.setVisible(False)
+        self.reset_action_context.setVisible(False)
 
     def on_replace_click(self):
         """
@@ -682,6 +696,7 @@ class ImageMediaItem(MediaManagerItem):
             if file_path.exists():
                 if self.live_controller.display.direct_image(str(file_path), background):
                     self.reset_action.setVisible(True)
+                    self.reset_action_context.setVisible(True)
                 else:
                     critical_error_message_box(
                         UiStrings().LiveBGError,

openlp/plugins/media/lib/mediaitem.py

@@ -26,7 +26,7 @@ import os
 from PyQt5 import QtCore, QtWidgets
 
 from openlp.core.common.applocation import AppLocation
-from openlp.core.common.i18n import UiStrings, translate, get_locale_key
+from openlp.core.common.i18n import UiStrings, translate, get_natural_key
 from openlp.core.common.path import Path, path_to_str, create_paths
 from openlp.core.common.mixins import RegistryProperties
 from openlp.core.common.registry import Registry
@@ -176,7 +176,7 @@ class MediaMediaItem(MediaManagerItem, RegistryProperties):
     def add_custom_context_actions(self):
         create_widget_action(self.list_view, separator=True)
         self.replace_action_context = create_widget_action(
-            self.list_view, text=UiStrings().ReplaceBG, icon=':/slides/slide_blank.png',
+            self.list_view, text=UiStrings().ReplaceBG, icon=':/slides/slide_theme.png',
             triggers=self.on_replace_click)
         self.reset_action_context = create_widget_action(
             self.list_view, text=UiStrings().ReplaceLiveBG, icon=':/system/system_close.png',
@@ -362,7 +362,7 @@ class MediaMediaItem(MediaManagerItem, RegistryProperties):
         :param media: The media
         :param target_group:
         """
-        media.sort(key=lambda file_name: get_locale_key(os.path.split(str(file_name))[1]))
+        media.sort(key=lambda file_name: get_natural_key(os.path.split(str(file_name))[1]))
         for track in media:
             track_info = QtCore.QFileInfo(track)
             item_name = None
@@ -404,7 +404,7 @@ class MediaMediaItem(MediaManagerItem, RegistryProperties):
         :return: The media list
         """
         media_file_paths = Settings().value(self.settings_section + '/media files')
-        media_file_paths.sort(key=lambda file_path: get_locale_key(file_path.name))
+        media_file_paths.sort(key=lambda file_path: get_natural_key(file_path.name))
         if media_type == MediaType.Audio:
             extension = self.media_controller.audio_extensions_list
         else:

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

openlp/plugins/presentations/lib/mediaitem.py

@@ -23,7 +23,7 @@ import logging
 
 from PyQt5 import QtCore, QtGui, QtWidgets
 
-from openlp.core.common.i18n import UiStrings, translate, get_locale_key
+from openlp.core.common.i18n import UiStrings, translate, get_natural_key
 from openlp.core.common.path import Path, path_to_str, str_to_path
 from openlp.core.common.registry import Registry
 from openlp.core.common.settings import Settings
@@ -165,7 +165,7 @@ class PresentationMediaItem(MediaManagerItem):
         if not initial_load:
             self.main_window.display_progress_bar(len(file_paths))
         # Sort the presentations by its filename considering language specific characters.
-        file_paths.sort(key=lambda file_path: get_locale_key(file_path.name))
+        file_paths.sort(key=lambda file_path: get_natural_key(file_path.name))
         for file_path in file_paths:
             if not initial_load:
                 self.main_window.increment_progress_bar()
@@ -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')
@@ -243,7 +243,7 @@ class PresentationMediaItem(MediaManagerItem):
         """
         Clean up the files created such as thumbnails
 
-        :param openlp.core.common.path.Path file_path: File path of the presention to clean up after
+        :param openlp.core.common.path.Path file_path: File path of the presentation to clean up after
         :param bool clean_for_update: Only clean thumbnails if update is needed
         :rtype: None
         """

openlp/plugins/presentations/lib/messagelistener.py

@@ -191,7 +191,7 @@ class Controller(object):
         """
         Based on the handler passed at startup triggers the previous slide event.
         """
-        log.debug('Live = {live}, previous'.formta(live=self.is_live))
+        log.debug('Live = {live}, previous'.format(live=self.is_live))
         if not self.doc:
             return
         if not self.is_live:

openlp/plugins/presentations/lib/pdfcontroller.py

@@ -19,8 +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 logging
 import re
 from subprocess import check_output, CalledProcessError

openlp/plugins/presentations/lib/pptviewcontroller.py

@@ -70,7 +70,7 @@ class PptviewController(PresentationController):
             try:
                 self.start_process()
                 return self.process.CheckInstalled()
-            except WindowsError:
+            except OSError:
                 return False
 
         def start_process(self):

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')
 
@@ -139,7 +139,8 @@ class PresentationDocument(object):
         :return: The path to the thumbnail
         :rtype: openlp.core.common.path.Path
         """
-        # TODO: If statement can be removed when the upgrade path from 2.0.x to 2.2.x is no longer needed
+        # TODO: Can be removed when the upgrade path to OpenLP 3.0 is no longer needed, also ensure code in
+        #       get_temp_folder and PresentationPluginapp_startup is removed
         if Settings().value('presentations/thumbnail_scheme') == 'md5':
             folder = md5_hash(bytes(self.file_path))
         else:
@@ -153,7 +154,8 @@ class PresentationDocument(object):
         :return: The path to the temporary file folder
         :rtype: openlp.core.common.path.Path
         """
-        # TODO: If statement can be removed when the upgrade path from 2.0.x to 2.2.x is no longer needed
+        # TODO: Can be removed when the upgrade path to OpenLP 3.0 is no longer needed, also ensure code in
+        #       get_thumbnail_folder and PresentationPluginapp_startup is removed
         if Settings().value('presentations/thumbnail_scheme') == 'md5':
             folder = md5_hash(bytes(self.file_path))
         else:
@@ -265,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):
         """

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.settings import Settings
 from openlp.core.lib import Plugin, StringContent, build_icon
 from openlp.plugins.presentations.endpoint import api_presentations_endpoint, presentations_endpoint
 from openlp.plugins.presentations.lib import PresentationController, PresentationMediaItem, PresentationTab
@@ -136,6 +137,20 @@ class PresentationPlugin(Plugin):
             self.register_controllers(controller)
         return bool(self.controllers)
 
+    def app_startup(self):
+        """
+        Perform tasks on application startup.
+        """
+        # 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()
+        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(presentation_paths)
+
     @staticmethod
     def about():
         """

openlp/plugins/songs/forms/editsongform.py

@@ -105,9 +105,9 @@ class EditSongForm(QtWidgets.QDialog, Ui_EditSongDialog, RegistryProperties):
         self.topics_list_view.setSortingEnabled(False)
         self.topics_list_view.setAlternatingRowColors(True)
         self.audio_list_widget.setAlternatingRowColors(True)
-        self.find_verse_split = re.compile('---\[\]---\n', re.UNICODE)
-        self.whitespace = re.compile(r'\W+', re.UNICODE)
-        self.find_tags = re.compile(u'\{/?\w+\}', re.UNICODE)
+        self.find_verse_split = re.compile('---\[\]---\n')
+        self.whitespace = re.compile(r'\W+')
+        self.find_tags = re.compile(r'\{/?\w+\}')
 
     def _load_objects(self, cls, combo, cache):
         """

openlp/plugins/songs/lib/__init__.py

@@ -24,7 +24,6 @@ The :mod:`~openlp.plugins.songs.lib` module contains a number of library functio
 """
 
 import logging
-import os
 import re
 
 from PyQt5 import QtWidgets
@@ -39,8 +38,8 @@ from openlp.plugins.songs.lib.ui import SongStrings
 
 log = logging.getLogger(__name__)
 
-WHITESPACE = re.compile(r'[\W_]+', re.UNICODE)
-APOSTROPHE = re.compile('[\'`’ʻ′]', re.UNICODE)
+WHITESPACE = re.compile(r'[\W_]+')
+APOSTROPHE = re.compile(r'[\'`’ʻ′]')
 # PATTERN will look for the next occurence of one of these symbols:
 #   \controlword - optionally preceded by \*, optionally followed by a number
 #   \'## - where ## is a pair of hex digits, representing a single character

openlp/plugins/songs/lib/importers/easyslides.py

@@ -25,6 +25,7 @@ import re
 
 from lxml import etree, objectify
 
+from openlp.core.common import normalize_str
 from openlp.plugins.songs.lib import VerseType
 from openlp.plugins.songs.lib.importers.songimport import SongImport
 
@@ -225,7 +226,7 @@ class EasySlidesImport(SongImport):
                 verses[reg].setdefault(vt, {})
                 verses[reg][vt].setdefault(vn, {})
                 verses[reg][vt][vn].setdefault(inst, [])
-                verses[reg][vt][vn][inst].append(self.tidy_text(line))
+                verses[reg][vt][vn][inst].append(normalize_str(line))
         # done parsing
         versetags = []
         # we use our_verse_order to ensure, we insert lyrics in the same order

openlp/plugins/songs/lib/importers/mediashout.py

@@ -101,7 +101,7 @@ class MediaShoutImport(SongImport):
             self.song_book_name = song.SongID
         for verse in verses:
             tag = VERSE_TAGS[verse.Type] + str(verse.Number) if verse.Type < len(VERSE_TAGS) else 'O'
-            self.add_verse(self.tidy_text(verse.Text), tag)
+            self.add_verse(verse.Text, tag)
         for order in verse_order:
             if order.Type < len(VERSE_TAGS):
                 self.verse_order_list.append(VERSE_TAGS[order.Type] + str(order.Number))

openlp/plugins/songs/lib/importers/openoffice.py

@@ -24,7 +24,7 @@ import time
 
 from PyQt5 import QtCore
 
-from openlp.core.common import is_win, get_uno_command, get_uno_instance
+from openlp.core.common import get_uno_command, get_uno_instance, is_win, normalize_str
 from openlp.core.common.i18n import translate
 from .songimport import SongImport
 
@@ -241,7 +241,7 @@ class OpenOfficeImport(SongImport):
 
         :param text: The text.
         """
-        song_texts = self.tidy_text(text).split('\f')
+        song_texts = normalize_str(text).split('\f')
         self.set_defaults()
         for song_text in song_texts:
             if song_text.strip():

openlp/plugins/songs/lib/importers/opensong.py

@@ -25,6 +25,7 @@ import re
 from lxml import objectify
 from lxml.etree import Error, LxmlError
 
+from openlp.core.common import normalize_str
 from openlp.core.common.i18n import translate
 from openlp.core.common.settings import Settings
 from openlp.plugins.songs.lib import VerseType
@@ -262,7 +263,7 @@ class OpenSongImport(SongImport):
                                                               post=this_line[offset + column:])
                     offset += len(chord) + 2
             # Tidy text and remove the ____s from extended words
-            this_line = self.tidy_text(this_line)
+            this_line = normalize_str(this_line)
             this_line = this_line.replace('_', '')
             this_line = this_line.replace('||', '\n[---]\n')
             this_line = this_line.strip()

openlp/plugins/songs/lib/importers/songimport.py

@@ -25,6 +25,7 @@ import re
 
 from PyQt5 import QtCore
 
+from openlp.core.common import normalize_str
 from openlp.core.common.applocation import AppLocation
 from openlp.core.common.i18n import translate
 from openlp.core.common.path import copyfile, create_paths
@@ -130,26 +131,6 @@ class SongImport(QtCore.QObject):
     def register(self, import_wizard):
         self.import_wizard = import_wizard
 
-    def tidy_text(self, text):
-        """
-        Get rid of some dodgy unicode and formatting characters we're not interested in. Some can be converted to ascii.
-        """
-        text = text.replace('\u2018', '\'')
-        text = text.replace('\u2019', '\'')
-        text = text.replace('\u201c', '"')
-        text = text.replace('\u201d', '"')
-        text = text.replace('\u2026', '...')
-        text = text.replace('\u2013', '-')
-        text = text.replace('\u2014', '-')
-        # Replace vertical tab with 2 linebreaks
-        text = text.replace('\v', '\n\n')
-        # Replace form feed (page break) with 2 linebreaks
-        text = text.replace('\f', '\n\n')
-        # Remove surplus blank lines, spaces, trailing/leading spaces
-        text = re.sub(r'[ \t]+', ' ', text)
-        text = re.sub(r' ?(\r\n?|\n) ?', '\n', text)
-        return text
-
     def process_song_text(self, text):
         """
         Process the song text from import
@@ -368,7 +349,7 @@ class SongImport(QtCore.QObject):
                 verse_tag = VerseType.tags[VerseType.Other]
                 log.info('Versetype {old} changing to {new}'.format(old=verse_def, new=new_verse_def))
                 verse_def = new_verse_def
-            sxml.add_verse_to_lyrics(verse_tag, verse_def[1:], verse_text, lang)
+            sxml.add_verse_to_lyrics(verse_tag, verse_def[1:], normalize_str(verse_text), lang)
         song.lyrics = str(sxml.extract_xml(), 'utf-8')
         if not self.verse_order_list and self.verse_order_list_generated_useful:
             self.verse_order_list = self.verse_order_list_generated

openlp/plugins/songs/lib/importers/songsoffellowship.py

@@ -194,7 +194,6 @@ class SongsOfFellowshipImport(OpenOfficeImport):
         :param text_portion: A Piece of text
         """
         text = text_portion.getString()
-        text = self.tidy_text(text)
         if text.strip() == '':
             return text
         if text_portion.CharWeight == BOLD:

openlp/plugins/songs/lib/importers/zionworx.py

@@ -30,9 +30,6 @@ from openlp.plugins.songs.lib.importers.songimport import SongImport
 
 log = logging.getLogger(__name__)
 
-# Used to strip control chars (except 10=LF, 13=CR)
-CONTROL_CHARS_MAP = dict.fromkeys(list(range(10)) + [11, 12] + list(range(14, 32)) + [127])
-
 
 class ZionWorxImport(SongImport):
     """
@@ -95,12 +92,12 @@ class ZionWorxImport(SongImport):
                     return
                 self.set_defaults()
                 try:
-                    self.title = self._decode(record['Title1'])
+                    self.title = record['Title1']
                     if record['Title2']:
-                        self.alternate_title = self._decode(record['Title2'])
-                    self.parse_author(self._decode(record['Writer']))
-                    self.add_copyright(self._decode(record['Copyright']))
-                    lyrics = self._decode(record['Lyrics'])
+                        self.alternate_title = record['Title2']
+                    self.parse_author(record['Writer'])
+                    self.add_copyright(record['Copyright'])
+                    lyrics = record['Lyrics']
                 except UnicodeDecodeError as e:
                     self.log_error(translate('SongsPlugin.ZionWorxImport', 'Record {index}').format(index=index),
                                    translate('SongsPlugin.ZionWorxImport', 'Decoding error: {error}').format(error=e))
@@ -122,10 +119,3 @@ class ZionWorxImport(SongImport):
                 if not self.finish():
                     self.log_error(translate('SongsPlugin.ZionWorxImport', 'Record %d') % index +
                                    (': "' + title + '"' if title else ''))
-
-    def _decode(self, str):
-        """
-        Strips all control characters (except new lines).
-        """
-        # ZionWorx has no option for setting the encoding for its songs, so we assume encoding is always the same.
-        return str.translate(CONTROL_CHARS_MAP)

openlp/plugins/songs/lib/openlyricsxml.py

@@ -281,7 +281,7 @@ class OpenLyrics(object):
         # Process the formatting tags.
         # Have we any tags in song lyrics?
         tags_element = None
-        match = re.search('\{/?\w+\}', song.lyrics, re.UNICODE)
+        match = re.search(r'\{/?\w+\}', song.lyrics)
         if match:
             # Named 'format_' - 'format' is built-in function in Python.
             format_ = etree.SubElement(song_xml, 'format')

openlp/plugins/songusage/forms/songusagedetailform.py

@@ -54,8 +54,14 @@ class SongUsageDetailForm(QtWidgets.QDialog, Ui_SongUsageDetailDialog, RegistryP
         """
         We need to set up the screen
         """
-        self.from_date_calendar.setSelectedDate(Settings().value(self.plugin.settings_section + '/from date'))
-        self.to_date_calendar.setSelectedDate(Settings().value(self.plugin.settings_section + '/to date'))
+        to_date = Settings().value(self.plugin.settings_section + '/to date')
+        if not (isinstance(to_date, QtCore.QDate) and to_date.isValid()):
+            to_date = QtCore.QDate.currentDate()
+        from_date = Settings().value(self.plugin.settings_section + '/from date')
+        if not (isinstance(from_date, QtCore.QDate) and from_date.isValid()):
+            from_date = to_date.addYears(-1)
+        self.from_date_calendar.setSelectedDate(from_date)
+        self.to_date_calendar.setSelectedDate(to_date)
         self.report_path_edit.path = Settings().value(self.plugin.settings_section + '/last directory export')
 
     def on_report_path_edit_path_changed(self, file_path):

openlp/plugins/songusage/songusageplugin.py

@@ -38,20 +38,17 @@ from openlp.plugins.songusage.lib.db import init_schema, SongUsageItem
 
 log = logging.getLogger(__name__)
 
-YEAR = QtCore.QDate().currentDate().year()
-if QtCore.QDate().currentDate().month() < 9:
-    YEAR -= 1
-
+TODAY = QtCore.QDate.currentDate()
 
 __default_settings__ = {
     'songusage/db type': 'sqlite',
     'songusage/db username': '',
-    'songuasge/db password': '',
-    'songuasge/db hostname': '',
-    'songuasge/db database': '',
+    'songusage/db password': '',
+    'songusage/db hostname': '',
+    'songusage/db database': '',
     'songusage/active': False,
-    'songusage/to date': QtCore.QDate(YEAR, 8, 31),
-    'songusage/from date': QtCore.QDate(YEAR - 1, 9, 1),
+    'songusage/to date': TODAY,
+    'songusage/from date': TODAY.addYears(-1),
     'songusage/last directory export': None
 }
 

scripts/jenkins_script.py

@@ -21,35 +21,35 @@
 # Temple Place, Suite 330, Boston, MA 02111-1307 USA                          #
 ###############################################################################
 """
-This script helps to trigger builds of branches. To use it you have to install the jenkins-webapi package:
+This script helps to trigger builds of branches. To use it you have to install the python-jenkins module. On Fedora
+and Ubuntu/Debian, it is available as the ``python3-jenkins`` package::
 
-    pip3 install jenkins-webapi
+    $ sudo dnf/apt install python3-jenkins
 
-You probably want to create an alias. Add this to your ~/.bashrc file and then logout and login (to apply the alias):
+To make it easier to run you may want to create a shell script or an alias. To create an alias, add this to your
+``~/.bashrc`` (or ``~/.zshrc``) file and then log out and log back in again (to apply the alias)::
 
-    alias ci="python3 ./scripts/jenkins_script.py TOKEN"
+    alias ci="python3 /path/to/openlp_root/scripts/jenkins_script.py -u USERNAME -p PASSWORD"
 
-You can look up the token in the Branch-01-Pull job configuration or ask in IRC.
+To create a shell script, create the following file in a location in your ``$PATH`` (I called mine ``ci``)::
+
+    #!/bin/bash
+    python3 /path/to/openlp_root/scripts/jenkins_script.py -u USERNAME -p PASSWORD
+
+``USERNAME`` is your Jenkins username, and ``PASSWORD`` is your Jenkins password or personal token.
+
+An older version of this script used to use a shared TOKEN, but this has been replaced with the username and password.
 """
-
+import os
 import re
-import sys
 import time
-from optparse import OptionParser
+from argparse import ArgumentParser
 from subprocess import Popen, PIPE
-import warnings
 
-from requests.exceptions import HTTPError
 from jenkins import Jenkins
 
-
 JENKINS_URL = 'https://ci.openlp.io/'
 REPO_REGEX = r'(.*/+)(~.*)'
-# Allows us to black list token. So when we change the token, we can display a proper message to the user.
-OLD_TOKENS = []
-
-# Disable the InsecureRequestWarning we get from urllib3, because we're not verifying our own self-signed certificate
-warnings.simplefilter('ignore')
 
 
 class OpenLPJobs(object):
@@ -85,13 +85,23 @@ class JenkinsTrigger(object):
     :param token: The token we need to trigger the build. If you do not have this token, ask in IRC.
     """
 
-    def __init__(self, token):
+    def __init__(self, username, password, can_use_colour):
         """
         Create the JenkinsTrigger instance.
         """
-        self.token = token
+        self.jobs = {}
+        self.can_use_colour = can_use_colour and not os.name.startswith('nt')
         self.repo_name = get_repo_name()
-        self.jenkins_instance = Jenkins(JENKINS_URL)
+        self.server = Jenkins(JENKINS_URL, username=username, password=password)
+
+    def fetch_jobs(self):
+        """
+        Get the job info for all the jobs
+        """
+        for job_name in OpenLPJobs.Jobs:
+            job_info = self.server.get_job_info(job_name)
+            self.jobs[job_name] = job_info
+            self.jobs[job_name]['nextBuildUrl'] = '{url}{nextBuildNumber}/'.format(**job_info)
 
     def trigger_build(self):
         """
@@ -102,15 +112,15 @@ class JenkinsTrigger(object):
         # We just want the name (not the email).
         name = ' '.join(raw_output.decode().split()[:-1])
         cause = 'Build triggered by %s (%s)' % (name, self.repo_name)
-        self.jenkins_instance.job(OpenLPJobs.Branch_Pull).build({'BRANCH_NAME': self.repo_name, 'cause': cause},
-                                                                token=self.token)
+        self.fetch_jobs()
+        self.server.build_job(OpenLPJobs.Branch_Pull, {'BRANCH_NAME': self.repo_name, 'cause': cause})
 
     def print_output(self):
         """
         Print the status information of the build triggered.
         """
         print('Add this to your merge proposal:')
-        print('--------------------------------')
+        print('-' * 80)
         bzr = Popen(('bzr', 'revno'), stdout=PIPE, stderr=PIPE)
         raw_output, error = bzr.communicate()
         revno = raw_output.decode().strip()
@@ -118,7 +128,10 @@ class JenkinsTrigger(object):
 
         for job in OpenLPJobs.Jobs:
             if not self.__print_build_info(job):
-                print('Stopping after failure')
+                if self.current_build:
+                    print('Stopping after failure, see {}console for more details'.format(self.current_build['url']))
+                else:
+                    print('Stopping after failure')
                 break
 
     def open_browser(self):
@@ -129,6 +142,20 @@ class JenkinsTrigger(object):
         # Open the url
         Popen(('xdg-open', url), stderr=PIPE)
 
+    def _get_build_info(self, job_name, build_number):
+        """
+        Get the build info from the server. This method will check the queue and wait for the build.
+        """
+        queue_info = self.server.get_queue_info()
+        tries = 0
+        while queue_info and tries < 50:
+            tries += 1
+            time.sleep(0.5)
+            queue_info = self.server.get_queue_info()
+        if tries >= 50:
+            raise Exception('Build has not started yet, it may be stuck in the queue.')
+        return self.server.get_build_info(job_name, build_number)
+
     def __print_build_info(self, job_name):
         """
         This helper method prints the job information of the given ``job_name``
@@ -136,21 +163,24 @@ class JenkinsTrigger(object):
         :param job_name: The name of the job we want the information from. For example *Branch-01-Pull*. Use the class
          variables from the :class:`OpenLPJobs` class.
         """
+        job = self.jobs[job_name]
+        print('{:<70} [WAITING]'.format(job['nextBuildUrl']), end='', flush=True)
+        self.current_build = self._get_build_info(job_name, job['nextBuildNumber'])
+        print('\b\b\b\b\b\b\b\b\b[RUNNING]', end='', flush=True)
         is_success = False
-        job = self.jenkins_instance.job(job_name)
-        while job.info['inQueue']:
-            time.sleep(1)
-        build = job.last_build
-        build.wait()
-        if build.info['result'] == 'SUCCESS':
-            # Make 'SUCCESS' green.
-            result_string = '%s%s%s' % (Colour.GREEN_START, build.info['result'], Colour.GREEN_END)
-            is_success = True
-        else:
-            # Make 'FAILURE' red.
-            result_string = '%s%s%s' % (Colour.RED_START, build.info['result'], Colour.RED_END)
-        url = build.info['url']
-        print('[%s] %s' % (result_string, url))
+        while self.current_build['building'] is True:
+            time.sleep(0.5)
+            self.current_build = self.server.get_build_info(job_name, job['nextBuildNumber'])
+        result_string = self.current_build['result']
+        is_success = result_string == 'SUCCESS'
+        if self.can_use_colour:
+            if is_success:
+                # Make 'SUCCESS' green.
+                result_string = '{}{}{}'.format(Colour.GREEN_START, result_string, Colour.GREEN_END)
+            else:
+                # Make 'FAILURE' red.
+                result_string = '{}{}{}'.format(Colour.RED_START, result_string, Colour.RED_END)
+        print('\b\b\b\b\b\b\b\b\b[{:>7}]'.format(result_string))
         return is_success
 
 
@@ -186,36 +216,29 @@ def get_repo_name():
 
 
 def main():
-    usage = 'Usage: python %prog TOKEN [options]'
+    """
+    Run the script
+    """
+    parser = ArgumentParser()
+    parser.add_argument('-d', '--disable-output', action='store_true', default=False, help='Disable output')
+    parser.add_argument('-b', '--open-browser', action='store_true', default=False,
+                        help='Opens the jenkins page in your browser')
+    parser.add_argument('-n', '--no-colour', action='store_true', default=False,
+                        help='Disable coloured output (always disabled on Windows)')
+    parser.add_argument('-u', '--username', required=True, help='Your Jenkins username')
+    parser.add_argument('-p', '--password', required=True, help='Your Jenkins password or personal token')
+    args = parser.parse_args()
 
-    parser = OptionParser(usage=usage)
-    parser.add_option('-d', '--disable-output', dest='enable_output', action='store_false', default=True,
-                      help='Disable output.')
-    parser.add_option('-b', '--open-browser', dest='open_browser', action='store_true', default=False,
-                      help='Opens the jenkins page in your browser.')
-    options, args = parser.parse_args(sys.argv)
-
-    if len(args) == 2:
-        if not get_repo_name():
-            print('Not a branch. Have you pushed it to launchpad? Did you cd to the branch?')
-            return
-        token = args[-1]
-        if token in OLD_TOKENS:
-            print('Your token is not valid anymore. Get the most recent one.')
-            return
-        jenkins_trigger = JenkinsTrigger(token)
-        try:
-            jenkins_trigger.trigger_build()
-        except HTTPError:
-            print('Wrong token.')
-            return
-        # Open the browser before printing the output.
-        if options.open_browser:
-            jenkins_trigger.open_browser()
-        if options.enable_output:
-            jenkins_trigger.print_output()
-    else:
-        parser.print_help()
+    if not get_repo_name():
+        print('Not a branch. Have you pushed it to launchpad? Did you cd to the branch?')
+        return
+    jenkins_trigger = JenkinsTrigger(args.username, args.password, not args.no_colour)
+    jenkins_trigger.trigger_build()
+    # Open the browser before printing the output.
+    if args.open_browser:
+        jenkins_trigger.open_browser()
+    if not args.disable_output:
+        jenkins_trigger.print_output()
 
 
 if __name__ == '__main__':

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

tests/functional/openlp_core/common/test_actions.py

@@ -153,6 +153,7 @@ class TestActionList(TestCase, TestMixin):
         """
         Prepare the tests
         """
+        self.setup_application()
         self.action_list = ActionList.get_instance()
         self.build_settings()
         self.settings = Settings()

tests/functional/openlp_core/common/test_httputils.py

@@ -233,7 +233,7 @@ class TestHttpUtils(TestCase, TestMixin):
         Test socket timeout gets caught
         """
         # GIVEN: Mocked urlopen to fake a network disconnect in the middle of a download
-        mocked_requests.get.side_effect = IOError
+        mocked_requests.get.side_effect = OSError
 
         # WHEN: Attempt to retrieve a file
         url_get_file(MagicMock(), url='http://localhost/test', file_path=Path(self.tempfile))

tests/functional/openlp_core/common/test_i18n.py

@@ -155,7 +155,7 @@ def test_check_same_instance():
     assert first_instance is second_instance, 'Two UiStrings objects should be the same instance'
 
 
-def test_translate(self):
+def test_translate():
     """
     Test the translate() function
     """

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

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):
         """
@@ -371,13 +375,13 @@ class TestPath(TestCase):
     @patch('openlp.core.common.path.log')
     def test_create_paths_dir_io_error(self, mocked_logger):
         """
-        Test the create_paths() when an IOError is raised
+        Test the create_paths() when an OSError is raised
         """
         # GIVEN: A `Path` to check with patched out mkdir and exists methods
         mocked_path = MagicMock()
-        mocked_path.exists.side_effect = IOError('Cannot make directory')
+        mocked_path.exists.side_effect = OSError('Cannot make directory')
 
-        # WHEN: An IOError is raised when checking the if the path exists.
+        # WHEN: An OSError is raised when checking the if the path exists.
         create_paths(mocked_path)
 
         # THEN: The Error should have been logged
@@ -385,7 +389,7 @@ class TestPath(TestCase):
 
     def test_create_paths_dir_value_error(self):
         """
-        Test the create_paths() when an error other than IOError is raised
+        Test the create_paths() when an error other than OSError is raised
         """
         # GIVEN: A `Path` to check with patched out mkdir and exists methods
         mocked_path = MagicMock()

tests/functional/openlp_core/common/test_settings.py

@@ -22,10 +22,12 @@
 """
 Package to test the openlp.core.lib.settings package.
 """
+from pathlib import Path
 from unittest import TestCase
-from unittest.mock import patch
+from unittest.mock import call, patch
 
-from openlp.core.common.settings import Settings
+from openlp.core.common import settings
+from openlp.core.common.settings import Settings, media_players_conv
 
 from tests.helpers.testmixin import TestMixin
 
@@ -47,28 +49,58 @@ class TestSettings(TestCase, TestMixin):
         """
         self.destroy_settings()
 
-    def test_settings_basic(self):
-        """
-        Test the Settings creation and its default usage
-        """
-        # GIVEN: A new Settings setup
+    def test_media_players_conv(self):
+        """Test the media players conversion function"""
+        # GIVEN: A list of media players
+        media_players = 'phonon,webkit,vlc'
+
+        # WHEN: The media converter function is called
+        result = media_players_conv(media_players)
+
+        # THEN: The list should have been converted correctly
+        assert result == 'system,webkit,vlc'
+
+    def test_default_value(self):
+        """Test reading a setting that doesn't exist yet"""
+        # GIVEN: A setting that doesn't exist yet
 
         # WHEN reading a setting for the first time
         default_value = Settings().value('core/has run wizard')
 
         # THEN the default value is returned
-        self.assertFalse(default_value, 'The default value should be False')
+        assert default_value is False, 'The default value should be False'
 
+    def test_save_new_value(self):
+        """Test saving a new setting"""
+        # GIVEN: A setting that hasn't been saved yet
         # WHEN a new value is saved into config
         Settings().setValue('core/has run wizard', True)
 
         # THEN the new value is returned when re-read
-        self.assertTrue(Settings().value('core/has run wizard'), 'The saved value should have been returned')
+        assert Settings().value('core/has run wizard') is True, 'The saved value should have been returned'
+
+    def test_set_up_default_values(self):
+        """Test that the default values are updated"""
+        # GIVEN: A Settings object with defaults
+        # WHEN: set_up_default_values() is called
+        Settings.set_up_default_values()
+
+        # THEN: The default values should have been added to the dictionary
+        assert 'advanced/default service name' in Settings.__default_settings__
+
+    def test_get_default_value(self):
+        """Test that the default value for a setting is returned"""
+        # GIVEN: A Settings class with a default value
+        Settings.__default_settings__['test/moo'] = 'baa'
+
+        # WHEN: get_default_value() is called
+        result = Settings().get_default_value('test/moo')
+
+        # THEN: The correct default value should be returned
+        assert result == 'baa'
 
     def test_settings_override(self):
-        """
-        Test the Settings creation and its override usage
-        """
+        """Test the Settings creation and its override usage"""
         # GIVEN: an override for the settings
         screen_settings = {
             'test/extend': 'very wide',
@@ -79,18 +111,22 @@ class TestSettings(TestCase, TestMixin):
         extend = Settings().value('test/extend')
 
         # THEN the default value is returned
-        self.assertEqual('very wide', extend, 'The default value of "very wide" should be returned')
+        assert extend == 'very wide', 'The default value of "very wide" should be returned'
+
+    def test_save_existing_setting(self):
+        """Test that saving an existing setting returns the new value"""
+        # GIVEN: An existing setting
+        Settings().extend_default_settings({'test/existing value': None})
+        Settings().setValue('test/existing value', 'old value')
 
         # WHEN a new value is saved into config
-        Settings().setValue('test/extend', 'very short')
+        Settings().setValue('test/existing value', 'new value')
 
         # THEN the new value is returned when re-read
-        self.assertEqual('very short', Settings().value('test/extend'), 'The saved value should be returned')
+        assert Settings().value('test/existing value') == 'new value', 'The saved value should be returned'
 
     def test_settings_override_with_group(self):
-        """
-        Test the Settings creation and its override usage - with groups
-        """
+        """Test the Settings creation and its override usage - with groups"""
         # GIVEN: an override for the settings
         screen_settings = {
             'test/extend': 'very wide',
@@ -112,9 +148,7 @@ class TestSettings(TestCase, TestMixin):
         self.assertEqual('very short', Settings().value('test/extend'), 'The saved value should be returned')
 
     def test_settings_nonexisting(self):
-        """
-        Test the Settings on query for non-existing value
-        """
+        """Test the Settings on query for non-existing value"""
         # GIVEN: A new Settings setup
         with self.assertRaises(KeyError) as cm:
             # WHEN reading a setting that doesn't exists
@@ -124,9 +158,7 @@ class TestSettings(TestCase, TestMixin):
         self.assertEqual(str(cm.exception), "'core/does not exists'", 'We should get an exception')
 
     def test_extend_default_settings(self):
-        """
-        Test that the extend_default_settings method extends the default settings
-        """
+        """Test that the extend_default_settings method extends the default settings"""
         # GIVEN: A patched __default_settings__ dictionary
         with patch.dict(Settings.__default_settings__,
                         {'test/setting 1': 1, 'test/setting 2': 2, 'test/setting 3': 3}, True):
@@ -138,3 +170,125 @@ class TestSettings(TestCase, TestMixin):
             self.assertEqual(
                 Settings.__default_settings__, {'test/setting 1': 1, 'test/setting 2': 2, 'test/setting 3': 4,
                                                 'test/extended 1': 1, 'test/extended 2': 2})
+
+    @patch('openlp.core.common.settings.QtCore.QSettings.contains')
+    @patch('openlp.core.common.settings.QtCore.QSettings.value')
+    @patch('openlp.core.common.settings.QtCore.QSettings.setValue')
+    @patch('openlp.core.common.settings.QtCore.QSettings.remove')
+    def test_upgrade_single_setting(self, mocked_remove, mocked_setValue, mocked_value, mocked_contains):
+        """Test that the upgrade mechanism for settings works correctly for single value upgrades"""
+        # GIVEN: A settings object with an upgrade step to take (99, so that we don't interfere with real ones)
+        local_settings = Settings()
+        local_settings.__setting_upgrade_99__ = [
+            ('single/value', 'single/new value', [(str, '')])
+        ]
+        settings.__version__ = 99
+        mocked_value.side_effect = [98, 10]
+        mocked_contains.return_value = True
+
+        # WHEN: upgrade_settings() is called
+        local_settings.upgrade_settings()
+
+        # THEN: The correct calls should have been made with the correct values
+        assert mocked_value.call_count == 2, 'Settings().value() should have been called twice'
+        assert mocked_value.call_args_list == [call('settings/version', 0), call('single/value')]
+        assert mocked_setValue.call_count == 2, 'Settings().setValue() should have been called twice'
+        assert mocked_setValue.call_args_list == [call('single/new value', '10'), call('settings/version', 99)]
+        mocked_contains.assert_called_once_with('single/value')
+        mocked_remove.assert_called_once_with('single/value')
+
+    @patch('openlp.core.common.settings.QtCore.QSettings.contains')
+    @patch('openlp.core.common.settings.QtCore.QSettings.value')
+    @patch('openlp.core.common.settings.QtCore.QSettings.setValue')
+    @patch('openlp.core.common.settings.QtCore.QSettings.remove')
+    def test_upgrade_setting_value(self, mocked_remove, mocked_setValue, mocked_value, mocked_contains):
+        """Test that the upgrade mechanism for settings correctly uses the new value when it's not a function"""
+        # GIVEN: A settings object with an upgrade step to take (99, so that we don't interfere with real ones)
+        local_settings = Settings()
+        local_settings.__setting_upgrade_99__ = [
+            ('values/old value', 'values/new value', [(True, 1)])
+        ]
+        settings.__version__ = 99
+        mocked_value.side_effect = [98, 1]
+        mocked_contains.return_value = True
+
+        # WHEN: upgrade_settings() is called
+        local_settings.upgrade_settings()
+
+        # THEN: The correct calls should have been made with the correct values
+        assert mocked_value.call_count == 2, 'Settings().value() should have been called twice'
+        assert mocked_value.call_args_list == [call('settings/version', 0), call('values/old value')]
+        assert mocked_setValue.call_count == 2, 'Settings().setValue() should have been called twice'
+        assert mocked_setValue.call_args_list == [call('values/new value', True), call('settings/version', 99)]
+        mocked_contains.assert_called_once_with('values/old value')
+        mocked_remove.assert_called_once_with('values/old value')
+
+    @patch('openlp.core.common.settings.QtCore.QSettings.contains')
+    @patch('openlp.core.common.settings.QtCore.QSettings.value')
+    @patch('openlp.core.common.settings.QtCore.QSettings.setValue')
+    @patch('openlp.core.common.settings.QtCore.QSettings.remove')
+    def test_upgrade_multiple_one_invalid(self, mocked_remove, mocked_setValue, mocked_value, mocked_contains):
+        """Test that the upgrade mechanism for settings works correctly for multiple values where one is invalid"""
+        # GIVEN: A settings object with an upgrade step to take
+        local_settings = Settings()
+        local_settings.__setting_upgrade_99__ = [
+            (['multiple/value 1', 'multiple/value 2'], 'single/new value', [])
+        ]
+        settings.__version__ = 99
+        mocked_value.side_effect = [98, 10]
+        mocked_contains.side_effect = [True, False]
+
+        # WHEN: upgrade_settings() is called
+        local_settings.upgrade_settings()
+
+        # THEN: The correct calls should have been made with the correct values
+        mocked_value.assert_called_once_with('settings/version', 0)
+        mocked_setValue.assert_called_once_with('settings/version', 99)
+        assert mocked_contains.call_args_list == [call('multiple/value 1'), call('multiple/value 2')]
+
+    def test_can_upgrade(self):
+        """Test the Settings.can_upgrade() method"""
+        # GIVEN: A Settings object
+        local_settings = Settings()
+
+        # WHEN: can_upgrade() is run
+        result = local_settings.can_upgrade()
+
+        # THEN: The result should be True
+        assert result is True, 'The settings should be upgradeable'
+
+    def test_convert_value_setting_none_str(self):
+        """Test the Settings._convert_value() method when a setting is None and the default value is a string"""
+        # GIVEN: A settings object
+        # WHEN: _convert_value() is run
+        result = Settings()._convert_value(None, 'string')
+
+        # THEN: The result should be an empty string
+        assert result == '', 'The result should be an empty string'
+
+    def test_convert_value_setting_none_list(self):
+        """Test the Settings._convert_value() method when a setting is None and the default value is a list"""
+        # GIVEN: A settings object
+        # WHEN: _convert_value() is run
+        result = Settings()._convert_value(None, [None])
+
+        # THEN: The result should be an empty list
+        assert result == [], 'The result should be an empty list'
+
+    def test_convert_value_setting_json_Path(self):
+        """Test the Settings._convert_value() method when a setting is JSON and represents a Path object"""
+        # GIVEN: A settings object
+        # WHEN: _convert_value() is run
+        result = Settings()._convert_value('{"__Path__": ["openlp", "core"]}', None)
+
+        # THEN: The result should be a Path object
+        assert isinstance(result, Path), 'The result should be a Path object'
+
+    def test_convert_value_setting_bool_str(self):
+        """Test the Settings._convert_value() method when a setting is supposed to be a boolean"""
+        # GIVEN: A settings object
+        # WHEN: _convert_value() is run
+        result = Settings()._convert_value('false', True)
+
+        # THEN: The result should be False
+        assert result is False, 'The result should be False'

tests/functional/openlp_core/lib/test_lib.py

@@ -167,7 +167,7 @@ class TestLib(TestCase):
                 patch.object(Path, 'open'):
             file_path = Path('testfile.txt')
             file_path.is_file.return_value = True
-            file_path.open.side_effect = IOError()
+            file_path.open.side_effect = OSError()
 
             # WHEN: get_text_file_string is called
             result = get_text_file_string(file_path)
@@ -273,32 +273,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
 
@@ -307,32 +307,33 @@ 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
 
         # 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)
 
         # 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
 
@@ -341,33 +342,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
 
@@ -376,33 +378,34 @@ 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
 
@@ -411,33 +414,34 @@ 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
 
@@ -446,8 +450,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)
@@ -455,12 +459,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:
@@ -468,10 +472,11 @@ 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:
@@ -481,11 +486,12 @@ 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
 

tests/functional/openlp_core/projectors/__init__.py

@@ -20,5 +20,5 @@
 # Temple Place, Suite 330, Boston, MA 02111-1307 USA                          #
 ###############################################################################
 """
-The Projector driver module.
+Module-level functions for the functional test suite
 """

tests/functional/openlp_core/projectors/test_projector_bugfixes_01.py

@@ -0,0 +1,134 @@
+# -*- coding: utf-8 -*-
+# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4
+
+###############################################################################
+# OpenLP - Open Source Lyrics Projection                                      #
+# --------------------------------------------------------------------------- #
+# Copyright (c) 2008-2015 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.projectors.pjlink base package.
+"""
+from unittest import TestCase
+from unittest.mock import patch
+
+from openlp.core.projectors.db import Projector
+from openlp.core.projectors.pjlink import PJLink
+
+from tests.resources.projector.data import TEST_PIN, TEST_CONNECT_AUTHENTICATE, TEST_HASH, TEST1_DATA
+
+
+class TestPJLinkBugs(TestCase):
+    """
+    Tests for the PJLink module bugfixes
+    """
+    def setUp(self):
+        '''
+        Initialization
+        '''
+        self.pjlink_test = PJLink(Projector(**TEST1_DATA), no_poll=True)
+
+    def tearDown(self):
+        '''
+        Cleanups
+        '''
+        self.pjlink_test = None
+
+    def test_bug_1550891_process_clss_nonstandard_reply_1(self):
+        """
+        Bugfix 1550891: CLSS request returns non-standard reply with Optoma/Viewsonic projector
+        """
+        # GIVEN: Test object
+        pjlink = self.pjlink_test
+
+        # WHEN: Process non-standard reply
+        pjlink.process_clss('Class 1')
+
+        # THEN: Projector class should be set with proper value
+        self.assertEqual(pjlink.pjlink_class, '1',
+                         'Non-standard class reply should have set class=1')
+
+    def test_bug_1550891_process_clss_nonstandard_reply_2(self):
+        """
+        Bugfix 1550891: CLSS request returns non-standard reply with BenQ projector
+        """
+        # GIVEN: Test object
+        pjlink = self.pjlink_test
+
+        # WHEN: Process non-standard reply
+        pjlink.process_clss('Version2')
+
+        # THEN: Projector class should be set with proper value
+        # NOTE: At this time BenQ is Class 1, but we're trying a different value to verify
+        self.assertEqual(pjlink.pjlink_class, '2',
+                         'Non-standard class reply should have set class=2')
+
+    def test_bug_1593882_no_pin_authenticated_connection(self):
+        """
+        Test bug 1593882 no pin and authenticated request exception
+        """
+        # GIVEN: Test object and mocks
+        mock_socket_timer = patch.object(self.pjlink_test, 'socket_timer').start()
+        mock_timer = patch.object(self.pjlink_test, 'timer').start()
+        mock_authentication = patch.object(self.pjlink_test, 'projectorAuthentication').start()
+        mock_ready_read = patch.object(self.pjlink_test, 'waitForReadyRead').start()
+        mock_send_command = patch.object(self.pjlink_test, 'send_command').start()
+        pjlink = self.pjlink_test
+        pjlink.pin = None
+        mock_ready_read.return_value = True
+
+        # WHEN: call with authentication request and pin not set
+        pjlink.check_login(data=TEST_CONNECT_AUTHENTICATE)
+
+        # THEN: 'No Authentication' signal should have been sent
+        mock_authentication.emit.assert_called_with(pjlink.ip)
+
+    def test_bug_1593883_pjlink_authentication(self):
+        """
+        Test bugfix 1593883 pjlink authentication
+        """
+        # GIVEN: Test object and data
+        mock_socket_timer = patch.object(self.pjlink_test, 'socket_timer').start()
+        mock_timer = patch.object(self.pjlink_test, 'timer').start()
+        mock_send_command = patch.object(self.pjlink_test, 'write').start()
+        mock_state = patch.object(self.pjlink_test, 'state').start()
+        mock_waitForReadyRead = patch.object(self.pjlink_test, 'waitForReadyRead').start()
+        pjlink = self.pjlink_test
+        pjlink.pin = TEST_PIN
+        mock_state.return_value = pjlink.ConnectedState
+        mock_waitForReadyRead.return_value = True
+
+        # WHEN: Athenticated connection is called
+        pjlink.check_login(data=TEST_CONNECT_AUTHENTICATE)
+
+        # THEN: send_command should have the proper authentication
+        self.assertEqual("{test}".format(test=mock_send_command.call_args),
+                         "call(b'{hash}%1CLSS ?\\r')".format(hash=TEST_HASH))
+
+    def test_bug_1734275_process_lamp_nonstandard_reply(self):
+        """
+        Test bugfix 17342785 non-standard LAMP response
+        """
+        # GIVEN: Test object
+        pjlink = self.pjlink_test
+
+        # WHEN: Process lamp command called with only hours and no lamp power state
+        pjlink.process_lamp("45")
+
+        # THEN: Lamp should show hours as 45 and lamp power as Unavailable
+        self.assertEqual(len(pjlink.lamp), 1, 'There should only be 1 lamp available')
+        self.assertEqual(pjlink.lamp[0]['Hours'], 45, 'Lamp hours should have equalled 45')
+        self.assertIsNone(pjlink.lamp[0]['On'], 'Lamp power should be "None"')

tests/functional/openlp_core/projectors/test_projector_constants.py

@@ -20,7 +20,7 @@
 # Temple Place, Suite 330, Boston, MA 02111-1307 USA                          #
 ###############################################################################
 """
-Package to test the openlp.core.lib.projector.constants package.
+Package to test the openlp.core.projectors.constants module.
 """
 from unittest import TestCase
 
@@ -37,7 +37,7 @@ class TestProjectorConstants(TestCase):
         from tests.resources.projector.data import TEST_VIDEO_CODES
 
         # WHEN: Import projector PJLINK_DEFAULT_CODES
-        from openlp.core.lib.projector.constants import PJLINK_DEFAULT_CODES
+        from openlp.core.projectors.constants import PJLINK_DEFAULT_CODES
 
         # THEN: Verify dictionary was build correctly
         self.assertEqual(PJLINK_DEFAULT_CODES, TEST_VIDEO_CODES, 'PJLink video strings should match')

tests/functional/openlp_core/projectors/test_projector_db.py

@@ -20,7 +20,7 @@
 # Temple Place, Suite 330, Boston, MA 02111-1307 USA                          #
 ###############################################################################
 """
-Package to test the openlp.core.ui.projectordb  find, edit, delete
+Package to test the openlp.core.projectors.db module.
 record functions.
 
 PREREQUISITE: add_record() and get_all() functions validated.
@@ -32,10 +32,10 @@ from tempfile import mkdtemp
 from unittest import TestCase
 from unittest.mock import patch
 
-from openlp.core.lib.projector import upgrade
 from openlp.core.lib.db import upgrade_db
-from openlp.core.lib.projector.constants import PJLINK_PORT
-from openlp.core.lib.projector.db import Manufacturer, Model, Projector, ProjectorDB, ProjectorSource, Source
+from openlp.core.projectors import upgrade
+from openlp.core.projectors.constants import PJLINK_PORT
+from openlp.core.projectors.db import Manufacturer, Model, Projector, ProjectorDB, ProjectorSource, Source
 
 from tests.resources.projector.data import TEST_DB_PJLINK1, TEST_DB, TEST1_DATA, TEST2_DATA, TEST3_DATA
 from tests.utils.constants import TEST_RESOURCES_PATH
@@ -129,7 +129,7 @@ class TestProjectorDB(TestCase):
     """
     Test case for ProjectorDB
     """
-    @patch('openlp.core.lib.projector.db.init_url')
+    @patch('openlp.core.projectors.db.init_url')
     def setUp(self, mocked_init_url):
         """
         Set up anything necessary for all tests

tests/functional/openlp_core/projectors/test_projector_pjlink_base.py

@@ -20,16 +20,16 @@
 # Temple Place, Suite 330, Boston, MA 02111-1307 USA                          #
 ###############################################################################
 """
-Package to test the openlp.core.lib.projector.pjlink base package.
+Package to test the openlp.core.projectors.pjlink base package.
 """
 from unittest import TestCase
 from unittest.mock import call, patch, MagicMock
 
-from openlp.core.lib.projector.db import Projector
-from openlp.core.lib.projector.pjlink import PJLink
-from openlp.core.lib.projector.constants import E_PARAMETER, ERROR_STRING, S_ON, S_CONNECTED
+from openlp.core.projectors.constants import E_PARAMETER, ERROR_STRING, S_ON, S_CONNECTED
+from openlp.core.projectors.db import Projector
+from openlp.core.projectors.pjlink import PJLink
 
-from tests.resources.projector.data import TEST_PIN, TEST_SALT, TEST_CONNECT_AUTHENTICATE, TEST_HASH, TEST1_DATA
+from tests.resources.projector.data import TEST_PIN, TEST_SALT, TEST_CONNECT_AUTHENTICATE, TEST1_DATA
 
 pjlink_test = PJLink(Projector(**TEST1_DATA), no_poll=True)
 
@@ -79,58 +79,6 @@ class TestPJLinkBase(TestCase):
                                        'change_status should have been called with "{}"'.format(
                                            ERROR_STRING[E_PARAMETER]))
 
-    @patch.object(pjlink_test, 'send_command')
-    @patch.object(pjlink_test, 'waitForReadyRead')
-    @patch.object(pjlink_test, 'projectorAuthentication')
-    @patch.object(pjlink_test, 'timer')
-    @patch.object(pjlink_test, 'socket_timer')
-    def test_bug_1593882_no_pin_authenticated_connection(self,
-                                                         mock_socket_timer,
-                                                         mock_timer,
-                                                         mock_authentication,
-                                                         mock_ready_read,
-                                                         mock_send_command):
-        """
-        Test bug 1593882 no pin and authenticated request exception
-        """
-        # GIVEN: Test object and mocks
-        pjlink = pjlink_test
-        pjlink.pin = None
-        mock_ready_read.return_value = True
-
-        # WHEN: call with authentication request and pin not set
-        pjlink.check_login(data=TEST_CONNECT_AUTHENTICATE)
-
-        # THEN: 'No Authentication' signal should have been sent
-        mock_authentication.emit.assert_called_with(pjlink.ip)
-
-    @patch.object(pjlink_test, 'waitForReadyRead')
-    @patch.object(pjlink_test, 'state')
-    @patch.object(pjlink_test, '_send_command')
-    @patch.object(pjlink_test, 'timer')
-    @patch.object(pjlink_test, 'socket_timer')
-    def test_bug_1593883_pjlink_authentication(self,
-                                               mock_socket_timer,
-                                               mock_timer,
-                                               mock_send_command,
-                                               mock_state,
-                                               mock_waitForReadyRead):
-        """
-        Test bugfix 1593883 pjlink authentication
-        """
-        # GIVEN: Test object and data
-        pjlink = pjlink_test
-        pjlink.pin = TEST_PIN
-        mock_state.return_value = pjlink.ConnectedState
-        mock_waitForReadyRead.return_value = True
-
-        # WHEN: Athenticated connection is called
-        pjlink.check_login(data=TEST_CONNECT_AUTHENTICATE)
-
-        # THEN: send_command should have the proper authentication
-        self.assertEqual("{test}".format(test=mock_send_command.call_args),
-                         "call(data='{hash}%1CLSS ?\\r')".format(hash=TEST_HASH))
-
     @patch.object(pjlink_test, 'disconnect_from_host')
     def test_socket_abort(self, mock_disconnect):
         """

tests/functional/openlp_core/projectors/test_projector_pjlink_cmd_routing.py

@@ -20,20 +20,20 @@
 # Temple Place, Suite 330, Boston, MA 02111-1307 USA                          #
 ###############################################################################
 """
-Package to test the openlp.core.lib.projector.pjlink class command routing.
+Package to test the openlp.core.projectors.pjlink command routing.
 """
 
 from unittest import TestCase
 from unittest.mock import patch, MagicMock
 
-import openlp.core.lib.projector.pjlink
-from openlp.core.lib.projector.db import Projector
-from openlp.core.lib.projector.pjlink import PJLink
-from openlp.core.lib.projector.constants import PJLINK_ERRORS, \
+import openlp.core.projectors.pjlink
+from openlp.core.projectors.constants import PJLINK_ERRORS, \
     E_AUTHENTICATION, E_PARAMETER, E_PROJECTOR, E_UNAVAILABLE, E_UNDEFINED
+from openlp.core.projectors.db import Projector
+from openlp.core.projectors.pjlink import PJLink
 
 '''
-from openlp.core.lib.projector.constants import ERROR_STRING, PJLINK_ERST_DATA, PJLINK_ERST_STATUS, \
+from openlp.core.projectors.constants import ERROR_STRING, PJLINK_ERST_DATA, PJLINK_ERST_STATUS, \
     PJLINK_POWR_STATUS, PJLINK_VALID_CMD, E_WARN, E_ERROR, S_OFF, S_STANDBY, S_ON
 '''
 from tests.resources.projector.data import TEST_PIN, TEST1_DATA
@@ -46,7 +46,7 @@ class TestPJLinkRouting(TestCase):
     """
     Tests for the PJLink module command routing
     """
-    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    @patch.object(openlp.core.projectors.pjlink, 'log')
     def test_process_command_call_clss(self, mock_log):
         """
         Test process_command calls proper function
@@ -66,7 +66,7 @@ class TestPJLinkRouting(TestCase):
         mock_process_clss.assert_called_with('1')
 
     @patch.object(pjlink_test, 'change_status')
-    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    @patch.object(openlp.core.projectors.pjlink, 'log')
     def test_process_command_err1(self, mock_log, mock_change_status):
         """
         Test ERR1 - Undefined projector function
@@ -85,7 +85,7 @@ class TestPJLinkRouting(TestCase):
         mock_log.error.assert_called_with(log_text)
 
     @patch.object(pjlink_test, 'change_status')
-    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    @patch.object(openlp.core.projectors.pjlink, 'log')
     def test_process_command_err2(self, mock_log, mock_change_status):
         """
         Test ERR2 - Parameter Error
@@ -104,7 +104,7 @@ class TestPJLinkRouting(TestCase):
         mock_log.error.assert_called_with(log_text)
 
     @patch.object(pjlink_test, 'change_status')
-    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    @patch.object(openlp.core.projectors.pjlink, 'log')
     def test_process_command_err3(self, mock_log, mock_change_status):
         """
         Test ERR3 - Unavailable error
@@ -123,7 +123,7 @@ class TestPJLinkRouting(TestCase):
         mock_log.error.assert_called_with(log_text)
 
     @patch.object(pjlink_test, 'change_status')
-    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    @patch.object(openlp.core.projectors.pjlink, 'log')
     def test_process_command_err4(self, mock_log, mock_change_status):
         """
         Test ERR3 - Unavailable error
@@ -144,7 +144,7 @@ class TestPJLinkRouting(TestCase):
     @patch.object(pjlink_test, 'projectorAuthentication')
     @patch.object(pjlink_test, 'change_status')
     @patch.object(pjlink_test, 'disconnect_from_host')
-    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    @patch.object(openlp.core.projectors.pjlink, 'log')
     def test_process_command_erra(self, mock_log, mock_disconnect, mock_change_status, mock_err_authenticate):
         """
         Test ERRA - Authentication Error
@@ -163,7 +163,7 @@ class TestPJLinkRouting(TestCase):
         mock_change_status.assert_called_once_with(E_AUTHENTICATION)
         mock_log.error.assert_called_with(log_text)
 
-    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    @patch.object(openlp.core.projectors.pjlink, 'log')
     def test_process_command_future(self, mock_log):
         """
         Test command valid but no method to process yet
@@ -184,7 +184,7 @@ class TestPJLinkRouting(TestCase):
         mock_log.warning.assert_called_once_with(log_text)
 
     @patch.object(pjlink_test, 'pjlink_functions')
-    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    @patch.object(openlp.core.projectors.pjlink, 'log')
     def test_process_command_invalid(self, mock_log, mock_functions):
         """
         Test not a valid command
@@ -203,7 +203,7 @@ class TestPJLinkRouting(TestCase):
         mock_log.error.assert_called_once_with(log_text)
 
     @patch.object(pjlink_test, 'pjlink_functions')
-    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    @patch.object(openlp.core.projectors.pjlink, 'log')
     def test_process_command_ok(self, mock_log, mock_functions):
         """
         Test command returned success

tests/functional/openlp_core/projectors/test_projector_pjlink_commands.py

@@ -20,18 +20,18 @@
 # Temple Place, Suite 330, Boston, MA 02111-1307 USA                          #
 ###############################################################################
 """
-Package to test the openlp.core.lib.projector.pjlink commands package.
+Package to test the openlp.core.projectors.pjlink commands package.
 """
 from unittest import TestCase
 from unittest.mock import patch
 
-import openlp.core.lib.projector.pjlink
-from openlp.core.lib.projector.db import Projector
-from openlp.core.lib.projector.pjlink import PJLink
-from openlp.core.lib.projector.constants import ERROR_STRING, PJLINK_ERST_DATA, PJLINK_ERST_STATUS, \
+import openlp.core.projectors.pjlink
+from openlp.core.projectors.constants import ERROR_STRING, PJLINK_ERST_DATA, PJLINK_ERST_STATUS, \
     PJLINK_POWR_STATUS, \
     E_ERROR, E_NOT_CONNECTED, E_SOCKET_ADDRESS_NOT_AVAILABLE, E_UNKNOWN_SOCKET_ERROR, E_WARN, \
     S_CONNECTED, S_OFF, S_ON, S_NOT_CONNECTED, S_CONNECTING, S_STANDBY
+from openlp.core.projectors.db import Projector
+from openlp.core.projectors.pjlink import PJLink
 
 from tests.resources.projector.data import TEST_PIN, TEST1_DATA
 
@@ -50,7 +50,7 @@ class TestPJLinkCommands(TestCase):
     Tests for the PJLink module
     """
     @patch.object(pjlink_test, 'changeStatus')
-    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    @patch.object(openlp.core.projectors.pjlink, 'log')
     def test_projector_change_status_connection_error(self, mock_log, mock_change_status):
         """
         Test change_status with connection error
@@ -74,7 +74,7 @@ class TestPJLinkCommands(TestCase):
         self.assertEqual(mock_log.debug.call_count, 3, 'Debug log should have been called 3 times')
 
     @patch.object(pjlink_test, 'changeStatus')
-    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    @patch.object(openlp.core.projectors.pjlink, 'log')
     def test_projector_change_status_connection_status_connecting(self, mock_log, mock_change_status):
         """
         Test change_status with connection status
@@ -97,7 +97,7 @@ class TestPJLinkCommands(TestCase):
         self.assertEqual(mock_log.debug.call_count, 3, 'Debug log should have been called 3 times')
 
     @patch.object(pjlink_test, 'changeStatus')
-    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    @patch.object(openlp.core.projectors.pjlink, 'log')
     def test_projector_change_status_connection_status_connected(self, mock_log, mock_change_status):
         """
         Test change_status with connection status
@@ -120,7 +120,7 @@ class TestPJLinkCommands(TestCase):
         self.assertEqual(mock_log.debug.call_count, 3, 'Debug log should have been called 3 times')
 
     @patch.object(pjlink_test, 'changeStatus')
-    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    @patch.object(openlp.core.projectors.pjlink, 'log')
     def test_projector_change_status_connection_status_with_message(self, mock_log, mock_change_status):
         """
         Test change_status with connection status
@@ -144,7 +144,7 @@ class TestPJLinkCommands(TestCase):
         self.assertEqual(mock_log.debug.call_count, 3, 'Debug log should have been called 3 times')
 
     @patch.object(pjlink_test, 'send_command')
-    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    @patch.object(openlp.core.projectors.pjlink, 'log')
     def test_projector_get_av_mute_status(self, mock_log, mock_send_command):
         """
         Test sending command to retrieve shutter/audio state
@@ -164,7 +164,7 @@ class TestPJLinkCommands(TestCase):
         mock_send_command.assert_called_once_with(cmd=test_data)
 
     @patch.object(pjlink_test, 'send_command')
-    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    @patch.object(openlp.core.projectors.pjlink, 'log')
     def test_projector_get_available_inputs(self, mock_log, mock_send_command):
         """
         Test sending command to retrieve avaliable inputs
@@ -184,7 +184,7 @@ class TestPJLinkCommands(TestCase):
         mock_send_command.assert_called_once_with(cmd=test_data)
 
     @patch.object(pjlink_test, 'send_command')
-    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    @patch.object(openlp.core.projectors.pjlink, 'log')
     def test_projector_get_error_status(self, mock_log, mock_send_command):
         """
         Test sending command to retrieve projector error status
@@ -204,7 +204,7 @@ class TestPJLinkCommands(TestCase):
         mock_send_command.assert_called_once_with(cmd=test_data)
 
     @patch.object(pjlink_test, 'send_command')
-    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    @patch.object(openlp.core.projectors.pjlink, 'log')
     def test_projector_get_input_source(self, mock_log, mock_send_command):
         """
         Test sending command to retrieve current input
@@ -224,7 +224,7 @@ class TestPJLinkCommands(TestCase):
         mock_send_command.assert_called_once_with(cmd=test_data)
 
     @patch.object(pjlink_test, 'send_command')
-    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    @patch.object(openlp.core.projectors.pjlink, 'log')
     def test_projector_get_lamp_status(self, mock_log, mock_send_command):
         """
         Test sending command to retrieve lamp(s) status
@@ -244,7 +244,7 @@ class TestPJLinkCommands(TestCase):
         mock_send_command.assert_called_once_with(cmd=test_data)
 
     @patch.object(pjlink_test, 'send_command')
-    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    @patch.object(openlp.core.projectors.pjlink, 'log')
     def test_projector_get_manufacturer(self, mock_log, mock_send_command):
         """
         Test sending command to retrieve manufacturer name
@@ -264,7 +264,7 @@ class TestPJLinkCommands(TestCase):
         mock_send_command.assert_called_once_with(cmd=test_data)
 
     @patch.object(pjlink_test, 'send_command')
-    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    @patch.object(openlp.core.projectors.pjlink, 'log')
     def test_projector_get_model(self, mock_log, mock_send_command):
         """
         Test sending command to get model information
@@ -284,7 +284,7 @@ class TestPJLinkCommands(TestCase):
         mock_send_command.assert_called_once_with(cmd=test_data)
 
     @patch.object(pjlink_test, 'send_command')
-    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    @patch.object(openlp.core.projectors.pjlink, 'log')
     def test_projector_get_name(self, mock_log, mock_send_command):
         """
         Test sending command to get user-assigned name
@@ -304,7 +304,7 @@ class TestPJLinkCommands(TestCase):
         mock_send_command.assert_called_once_with(cmd=test_data)
 
     @patch.object(pjlink_test, 'send_command')
-    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    @patch.object(openlp.core.projectors.pjlink, 'log')
     def test_projector_get_other_info(self, mock_log, mock_send_command):
         """
         Test sending command to retrieve other information
@@ -324,7 +324,7 @@ class TestPJLinkCommands(TestCase):
         mock_send_command.assert_called_once_with(cmd=test_data)
 
     @patch.object(pjlink_test, 'send_command')
-    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    @patch.object(openlp.core.projectors.pjlink, 'log')
     def test_projector_get_power_status(self, mock_log, mock_send_command):
         """
         Test sending command to retrieve current power state
@@ -570,36 +570,7 @@ class TestPJLinkCommands(TestCase):
         self.assertEqual(pjlink.pjlink_class, '2',
                          'Projector should have set class=2')
 
-    def test_projector_process_clss_nonstandard_reply_optoma(self):
-        """
-        Bugfix 1550891: CLSS request returns non-standard reply with Optoma projector
-        """
-        # GIVEN: Test object
-        pjlink = pjlink_test
-
-        # WHEN: Process non-standard reply
-        pjlink.process_clss('Class 1')
-
-        # THEN: Projector class should be set with proper value
-        self.assertEqual(pjlink.pjlink_class, '1',
-                         'Non-standard class reply should have set class=1')
-
-    def test_projector_process_clss_nonstandard_reply_benq(self):
-        """
-        Bugfix 1550891: CLSS request returns non-standard reply with BenQ projector
-        """
-        # GIVEN: Test object
-        pjlink = pjlink_test
-
-        # WHEN: Process non-standard reply
-        pjlink.process_clss('Version2')
-
-        # THEN: Projector class should be set with proper value
-        # NOTE: At this time BenQ is Class 1, but we're trying a different value to verify
-        self.assertEqual(pjlink.pjlink_class, '2',
-                         'Non-standard class reply should have set class=2')
-
-    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    @patch.object(openlp.core.projectors.pjlink, 'log')
     def test_projector_process_clss_invalid_nan(self, mock_log):
         """
         Test CLSS reply has no class number
@@ -616,7 +587,7 @@ class TestPJLinkCommands(TestCase):
                          'Non-standard class reply should have set class=1')
         mock_log.error.assert_called_once_with(log_text)
 
-    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    @patch.object(openlp.core.projectors.pjlink, 'log')
     def test_projector_process_clss_invalid_no_version(self, mock_log):
         """
         Test CLSS reply has no class number
@@ -648,7 +619,7 @@ class TestPJLinkCommands(TestCase):
         # THEN: PJLink instance errors should be None
         self.assertIsNone(pjlink.projector_errors, 'projector_errors should have been set to None')
 
-    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    @patch.object(openlp.core.projectors.pjlink, 'log')
     def test_projector_process_erst_data_invalid_length(self, mock_log):
         """
         Test test_projector_process_erst_data_invalid_length
@@ -666,7 +637,7 @@ class TestPJLinkCommands(TestCase):
         self.assertTrue(mock_log.warning.called, 'Warning should have been logged')
         mock_log.warning.assert_called_once_with(log_text)
 
-    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    @patch.object(openlp.core.projectors.pjlink, 'log')
     def test_projector_process_erst_data_invalid_nan(self, mock_log):
         """
         Test test_projector_process_erst_data_invalid_nan
@@ -764,7 +735,7 @@ class TestPJLinkCommands(TestCase):
         self.assertEqual(pjlink.source, '1', 'Input source should be set to "1"')
 
     @patch.object(pjlink_test, 'projectorUpdateIcons')
-    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    @patch.object(openlp.core.projectors.pjlink, 'log')
     def test_projector_process_inst(self, mock_log, mock_UpdateIcons):
         """
         Test saving video source available information
@@ -787,7 +758,7 @@ class TestPJLinkCommands(TestCase):
         mock_log.debug.assert_called_once_with(log_data)
         self.assertTrue(mock_UpdateIcons.emit.called, 'Update Icons should have been called')
 
-    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    @patch.object(openlp.core.projectors.pjlink, 'log')
     def test_projector_process_lamp_invalid(self, mock_log):
         """
         Test status multiple lamp on/off and hours
@@ -858,7 +829,7 @@ class TestPJLinkCommands(TestCase):
         self.assertEqual(pjlink.lamp[0]['Hours'], 22222,
                          'Lamp hours should have been set to 22222')
 
-    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    @patch.object(openlp.core.projectors.pjlink, 'log')
     def test_projector_process_name(self, mock_log):
         """
         Test saving NAME data from projector
@@ -1040,7 +1011,7 @@ class TestPJLinkCommands(TestCase):
         self.assertNotEquals(pjlink.serial_no, test_number,
                              'Projector serial number should NOT have been set')
 
-    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    @patch.object(openlp.core.projectors.pjlink, 'log')
     def test_projector_process_sver(self, mock_log):
         """
         Test invalid software version information - too long
@@ -1061,7 +1032,7 @@ class TestPJLinkCommands(TestCase):
         self.assertIsNone(pjlink.sw_version_received, 'Received software version should not have changed')
         mock_log.debug.assert_called_once_with(test_log)
 
-    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    @patch.object(openlp.core.projectors.pjlink, 'log')
     def test_projector_process_sver_changed(self, mock_log):
         """
         Test invalid software version information - Received different than saved
@@ -1086,7 +1057,7 @@ class TestPJLinkCommands(TestCase):
         # There was 4 calls, but only the last one is checked with this method
         mock_log.warning.assert_called_with(test_log)
 
-    @patch.object(openlp.core.lib.projector.pjlink, 'log')
+    @patch.object(openlp.core.projectors.pjlink, 'log')
     def test_projector_process_sver_invalid(self, mock_log):
         """
         Test invalid software version information - too long

tests/functional/openlp_core/ui/test_first_time.py

@@ -40,7 +40,7 @@ class TestFirstTimeWizard(TestMixin, TestCase):
         Test get_web_page will attempt CONNECTION_RETRIES+1 connections - bug 1409031
         """
         # GIVEN: Initial settings and mocks
-        mocked_requests.get.side_effect = IOError('Unable to connect')
+        mocked_requests.get.side_effect = OSError('Unable to connect')
 
         # WHEN: A webpage is requested
         try:

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'

tests/functional/openlp_core/ui/test_slidecontroller.py

@@ -208,6 +208,33 @@ class TestSlideController(TestCase):
         mocked_on_theme_display.assert_called_once_with(False)
         mocked_on_hide_display.assert_called_once_with(False)
 
+    def test_on_go_live_preview_controller(self):
+        """
+        Test that when the on_go_preview() method is called the message is sent to the preview controller and focus is
+        set correctly.
+        """
+        # GIVEN: A new SlideController instance and plugin preview then pressing go live should respond
+        mocked_display = MagicMock()
+        mocked_preview_controller = MagicMock()
+        mocked_preview_widget = MagicMock()
+        mocked_service_item = MagicMock()
+        mocked_service_item.from_service = False
+        mocked_preview_widget.current_slide_number.return_value = 1
+        mocked_preview_widget.slide_count = MagicMock(return_value=2)
+        mocked_preview_controller.preview_widget = MagicMock()
+        Registry.create()
+        Registry().register('preview_controller', mocked_preview_controller)
+        slide_controller = SlideController(None)
+        slide_controller.service_item = mocked_service_item
+        slide_controller.preview_widget = mocked_preview_widget
+        slide_controller.display = mocked_display
+
+        # WHEN: on_go_live() is called
+        slide_controller.on_go_preview()
+
+        # THEN: the preview controller should have the service item and the focus set to live
+        mocked_preview_controller.preview_widget.setFocus.assert_called_once_with()
+
     def test_on_go_live_live_controller(self):
         """
         Test that when the on_go_live() method is called the message is sent to the live controller and focus is

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

tests/functional/openlp_core/widgets/test_views.py

@@ -627,4 +627,3 @@ class TestTreeWidgetWithDnD(TestCase):
         assert widget.allow_internal_dnd is False
         assert widget.indentation() == 0
         assert widget.isAnimated() is True
-

tests/functional/openlp_plugins/bibles/test_bibleimport.py

@@ -30,6 +30,7 @@ from lxml import etree, objectify
 from PyQt5.QtWidgets import QDialog
 
 from openlp.core.common.i18n import Language
+from openlp.core.common.path import Path
 from openlp.core.lib.exceptions import ValidationError
 from openlp.plugins.bibles.lib.bibleimport import BibleImport
 from openlp.plugins.bibles.lib.db import BibleDB
@@ -48,7 +49,7 @@ class TestBibleImport(TestCase):
             b'    <data><unsupported>Test<x>data</x><y>to</y>discard</unsupported></data>\n'
             b'</root>'
         )
-        self.open_patcher = patch('builtins.open')
+        self.open_patcher = patch.object(Path, 'open')
         self.addCleanup(self.open_patcher.stop)
         self.mocked_open = self.open_patcher.start()
         self.critical_error_message_box_patcher = \
@@ -74,8 +75,8 @@ class TestBibleImport(TestCase):
         # WHEN: Creating an instance of BibleImport with no key word arguments
         instance = BibleImport(MagicMock())
 
-        # THEN: The filename attribute should be None
-        assert instance.filename is None
+        # THEN: The file_path attribute should be None
+        assert instance.file_path is None
         assert isinstance(instance, BibleDB)
 
     def test_init_kwargs_set(self):
@@ -84,11 +85,11 @@ class TestBibleImport(TestCase):
         """
         # GIVEN: A patched BibleDB._setup, BibleImport class and mocked parent
         # WHEN: Creating an instance of BibleImport with selected key word arguments
-        kwargs = {'filename': 'bible.xml'}
+        kwargs = {'file_path': 'bible.xml'}
         instance = BibleImport(MagicMock(), **kwargs)
 
-        # THEN: The filename keyword should be set to bible.xml
-        assert instance.filename == 'bible.xml'
+        # THEN: The file_path keyword should be set to bible.xml
+        assert instance.file_path == 'bible.xml'
         assert isinstance(instance, BibleDB)
 
     @patch.object(BibleDB, '_setup')
@@ -361,7 +362,7 @@ class TestBibleImport(TestCase):
         instance.wizard = MagicMock()
 
         # WHEN: Calling parse_xml
-        result = instance.parse_xml('file.tst')
+        result = instance.parse_xml(Path('file.tst'))
 
         # THEN: The result returned should contain the correct data, and should be an instance of eetree_Element
         assert etree.tostring(result) == b'<root>\n    <data><div>Test<p>data</p><a>to</a>keep</div></data>\n' \
@@ -378,7 +379,7 @@ class TestBibleImport(TestCase):
         instance.wizard = MagicMock()
 
         # WHEN: Calling parse_xml
-        result = instance.parse_xml('file.tst', use_objectify=True)
+        result = instance.parse_xml(Path('file.tst'), use_objectify=True)
 
         # THEN: The result returned should contain the correct data, and should be an instance of ObjectifiedElement
         assert etree.tostring(result) == b'<root><data><div>Test<p>data</p><a>to</a>keep</div></data>' \
@@ -396,7 +397,7 @@ class TestBibleImport(TestCase):
         instance.wizard = MagicMock()
 
         # WHEN: Calling parse_xml, with a test file
-        result = instance.parse_xml('file.tst', elements=elements)
+        result = instance.parse_xml(Path('file.tst'), elements=elements)
 
         # THEN: The result returned should contain the correct data
         assert etree.tostring(result) == \
@@ -413,7 +414,7 @@ class TestBibleImport(TestCase):
         instance.wizard = MagicMock()
 
         # WHEN: Calling parse_xml, with a test file
-        result = instance.parse_xml('file.tst', tags=tags)
+        result = instance.parse_xml(Path('file.tst'), tags=tags)
 
         # THEN: The result returned should contain the correct data
         assert etree.tostring(result) == b'<root>\n    <data>Testdatatokeep</data>\n    <data><unsupported>Test' \
@@ -431,7 +432,7 @@ class TestBibleImport(TestCase):
         instance.wizard = MagicMock()
 
         # WHEN: Calling parse_xml, with a test file
-        result = instance.parse_xml('file.tst', elements=elements, tags=tags)
+        result = instance.parse_xml(Path('file.tst'), elements=elements, tags=tags)
 
         # THEN: The result returned should contain the correct data
         assert etree.tostring(result) == b'<root>\n    <data>Testdatatokeep</data>\n    <data/>\n</root>'
@@ -446,10 +447,10 @@ class TestBibleImport(TestCase):
         exception.filename = 'file.tst'
         exception.strerror = 'No such file or directory'
         self.mocked_open.side_effect = exception
-        importer = BibleImport(MagicMock(), path='.', name='.', filename='')
+        importer = BibleImport(MagicMock(), path='.', name='.', file_path=None)
 
         # WHEN: Calling parse_xml
-        result = importer.parse_xml('file.tst')
+        result = importer.parse_xml(Path('file.tst'))
 
         # THEN: parse_xml should have caught the error, informed the user and returned None
         mocked_log_exception.assert_called_once_with('Opening file.tst failed.')
@@ -468,10 +469,10 @@ class TestBibleImport(TestCase):
         exception.filename = 'file.tst'
         exception.strerror = 'Permission denied'
         self.mocked_open.side_effect = exception
-        importer = BibleImport(MagicMock(), path='.', name='.', filename='')
+        importer = BibleImport(MagicMock(), path='.', name='.', file_path=None)
 
         # WHEN: Calling parse_xml
-        result = importer.parse_xml('file.tst')
+        result = importer.parse_xml(Path('file.tst'))
 
         # THEN: parse_xml should have caught the error, informed the user and returned None
         mocked_log_exception.assert_called_once_with('Opening file.tst failed.')
@@ -485,7 +486,7 @@ class TestBibleImport(TestCase):
         Test set_current_chapter
         """
         # GIVEN: An instance of BibleImport and a mocked wizard
-        importer = BibleImport(MagicMock(), path='.', name='.', filename='')
+        importer = BibleImport(MagicMock(), path='.', name='.', file_path=None)
         importer.wizard = MagicMock()
 
         # WHEN: Calling set_current_chapter
@@ -500,7 +501,7 @@ class TestBibleImport(TestCase):
         Test that validate_xml_file raises a ValidationError when is_compressed returns True
         """
         # GIVEN: A mocked parse_xml which returns None
-        importer = BibleImport(MagicMock(), path='.', name='.', filename='')
+        importer = BibleImport(MagicMock(), path='.', name='.', file_path=None)
 
         # WHEN: Calling is_compressed
         # THEN: ValidationError should be raised, with the message 'Compressed file'
@@ -515,7 +516,7 @@ class TestBibleImport(TestCase):
         Test that validate_xml_file raises a ValidationError when parse_xml returns None
         """
         # GIVEN: A mocked parse_xml which returns None
-        importer = BibleImport(MagicMock(), path='.', name='.', filename='')
+        importer = BibleImport(MagicMock(), path='.', name='.', file_path=None)
 
         # WHEN: Calling validate_xml_file
         # THEN: ValidationError should be raised, with the message 'Error when opening file'
@@ -531,7 +532,7 @@ class TestBibleImport(TestCase):
         Test that validate_xml_file returns True with valid XML
         """
         # GIVEN: Some test data with an OpenSong Bible "bible" root tag
-        importer = BibleImport(MagicMock(), path='.', name='.', filename='')
+        importer = BibleImport(MagicMock(), path='.', name='.', file_path=None)
 
         # WHEN: Calling validate_xml_file
         result = importer.validate_xml_file('file.name', 'bible')
@@ -546,7 +547,7 @@ class TestBibleImport(TestCase):
         Test that validate_xml_file raises a ValidationError with an OpenSong root tag
         """
         # GIVEN: Some test data with an Zefania root tag and an instance of BibleImport
-        importer = BibleImport(MagicMock(), path='.', name='.', filename='')
+        importer = BibleImport(MagicMock(), path='.', name='.', file_path=None)
 
         # WHEN: Calling validate_xml_file
         # THEN: ValidationError should be raised, and the critical error message box should was called informing
@@ -566,7 +567,7 @@ class TestBibleImport(TestCase):
         # GIVEN: Some test data with an Zefania root tag and an instance of BibleImport
         mocked_parse_xml.return_value = objectify.fromstring(
             '<osis xmlns=\'http://www.bibletechnologies.net/2003/OSIS/namespace\'></osis>')
-        importer = BibleImport(MagicMock(), path='.', name='.', filename='')
+        importer = BibleImport(MagicMock(), path='.', name='.', file_path=None)
 
         # WHEN: Calling validate_xml_file
         # THEN: ValidationError should be raised, and the critical error message box should was called informing
@@ -584,7 +585,7 @@ class TestBibleImport(TestCase):
         Test that validate_xml_file raises a ValidationError with an Zefania root tag
         """
         # GIVEN: Some test data with an Zefania root tag and an instance of BibleImport
-        importer = BibleImport(MagicMock(), path='.', name='.', filename='')
+        importer = BibleImport(MagicMock(), path='.', name='.', file_path=None)
 
         # WHEN: Calling validate_xml_file
         # THEN: ValidationError should be raised, and the critical error message box should was called informing
@@ -602,7 +603,7 @@ class TestBibleImport(TestCase):
         Test that validate_xml_file raises a ValidationError with an unknown root tag
         """
         # GIVEN: Some test data with an unknown root tag and an instance of BibleImport
-        importer = BibleImport(MagicMock(), path='.', name='.', filename='')
+        importer = BibleImport(MagicMock(), path='.', name='.', file_path=None)
 
         # WHEN: Calling validate_xml_file
         # THEN: ValidationError should be raised, and the critical error message box should was called informing