Merge lp:~raoul-snyman/openlp/better-threading into lp:openlp

[Raoul Snyman]

Major overhaul of how threading in OpenLP works. Rather than messing around with threads yourself, you create a worker object descended from ThreadWorker, implement start() (and stop() if it's a long-running thread), and run it using run_thread().

Changes related to thread API:

- WebSocket was refactored (mostly into the worker)
- HttpServer was refactored a bit
- CheckMediaWorker was refactored a bit
- Version check refactored
- SongSelect search refactored
- New _wait_for_threads() method in MainWindow
- Tidied up closeEvent in MainWindow a bit

Bugs fixed:

- Logs have returned to the cache dir when XDG is around
- Flipped the --no-web-server flag (now False is off, not on)
- Fixed a call to reload_bibles()

Other things done:

- Removed the --style option (it never worked)
- Renamed "url_get_file() to download_file()
- Standardised a callback object for download_file()

Add this to your merge proposal:
--------------------------------------------------------------------------------
lp:~raoul-snyman/openlp/better-threading (revision 2803)
https://ci.openlp.io/job/Branch-01-Pull/2413/ [SUCCESS]
https://ci.openlp.io/job/Branch-02a-Linux-Tests/2314/ [SUCCESS]
https://ci.openlp.io/job/Branch-02b-macOS-Tests/109/ [SUCCESS]
https://ci.openlp.io/job/Branch-03a-Build-Source/32/ [SUCCESS]
https://ci.openlp.io/job/Branch-03b-Build-macOS/31/ [SUCCESS]
https://ci.openlp.io/job/Branch-04a-Code-Analysis/1494/ [SUCCESS]
https://ci.openlp.io/job/Branch-04b-Test-Coverage/1307/ [SUCCESS]
https://ci.openlp.io/job/Branch-05-AppVeyor-Tests/258/ [FAILURE]
Stopping after failure

Failed builds:
 - Branch-05-AppVeyor-Tests #258: https://ci.openlp.io/job/Branch-05-AppVeyor-Tests/258/console

[Tim Bentley (trb143)]

Very nice.
3 Minor questions.

review: Needs Fixing

[Phill (phill-ridout)]

Just a few minor things

review: Needs Fixing

[Raoul Snyman (raoul-snyman)]

Replied to Tim's questions.

[lp:~raoul-snyman/openlp/better-threading]

2804. By Raoul Snyman on 2018-01-07

Fix some issues highlighted by Tim and Phill, and added a file that was erroneously removed

2805. By Raoul Snyman on 2018-01-07

Fix the tests I now added back in

Unmerged revisions

=== modified file 'openlp/core/api/deploy.py'
--- openlp/core/api/deploy.py	2017-12-29 09:15:48 +0000
+++ openlp/core/api/deploy.py	2018-01-07 18:07:40 +0000
@@ -25,7 +25,7 @@
 from zipfile import ZipFile
 
 from openlp.core.common.applocation import AppLocation
-from openlp.core.common.httputils import url_get_file, get_web_page, get_url_file_size
+from openlp.core.common.httputils import download_file, get_web_page, get_url_file_size
 from openlp.core.common.registry import Registry
 
 
@@ -65,7 +65,7 @@
     sha256, version = download_sha256()
     file_size = get_url_file_size('https://get.openlp.org/webclient/site.zip')
     callback.setRange(0, file_size)
-    if url_get_file(callback, 'https://get.openlp.org/webclient/site.zip',
-                    AppLocation.get_section_data_path('remotes') / 'site.zip',
-                    sha256=sha256):
+    if download_file(callback, 'https://get.openlp.org/webclient/site.zip',
+                     AppLocation.get_section_data_path('remotes') / 'site.zip',
+                     sha256=sha256):
         deploy_zipfile(AppLocation.get_section_data_path('remotes'), 'site.zip')

=== modified file 'openlp/core/api/http/server.py'
--- openlp/core/api/http/server.py	2017-12-29 09:15:48 +0000
+++ openlp/core/api/http/server.py	2018-01-07 18:07:40 +0000
@@ -27,7 +27,7 @@
 import time
 
 from PyQt5 import QtCore, QtWidgets
-from waitress import serve
+from waitress.server import create_server
 
 from openlp.core.api.deploy import download_and_check, download_sha256
 from openlp.core.api.endpoint.controller import controller_endpoint, api_controller_endpoint
@@ -44,23 +44,16 @@
 from openlp.core.common.path import create_paths
 from openlp.core.common.registry import Registry, RegistryBase
 from openlp.core.common.settings import Settings
+from openlp.core.threading import ThreadWorker, run_thread
 
 log = logging.getLogger(__name__)
 
 
-class HttpWorker(QtCore.QObject):
+class HttpWorker(ThreadWorker):
     """
     A special Qt thread class to allow the HTTP server to run at the same time as the UI.
     """
-    def __init__(self):
-        """
-        Constructor for the thread class.
-
-        :param server: The http server class.
-        """
-        super(HttpWorker, self).__init__()
-
-    def run(self):
+    def start(self):
         """
         Run the thread.
         """
@@ -68,12 +61,21 @@
         port = Settings().value('api/port')
         Registry().execute('get_website_version')
         try:
-            serve(application, host=address, port=port)
+            self.server = create_server(application, host=address, port=port)
+            self.server.run()
         except OSError:
             log.exception('An error occurred when serving the application.')
+        self.quit.emit()
 
     def stop(self):
-        pass
+        """
+        A method to stop the worker
+        """
+        if hasattr(self, 'server'):
+            # Loop through all the channels and close them to stop the server
+            for channel in self.server._map.values():
+                if hasattr(channel, 'close'):
+                    channel.close()
 
 
 class HttpServer(RegistryBase, RegistryProperties, LogMixin):
@@ -85,12 +87,9 @@
         Initialise the http server, and start the http server
         """
         super(HttpServer, self).__init__(parent)
-        if Registry().get_flag('no_web_server'):
-            self.worker = HttpWorker()
-            self.thread = QtCore.QThread()
-            self.worker.moveToThread(self.thread)
-            self.thread.started.connect(self.worker.run)
-            self.thread.start()
+        if not Registry().get_flag('no_web_server'):
+            worker = HttpWorker()
+            run_thread(worker, 'http_server')
             Registry().register_function('download_website', self.first_time)
             Registry().register_function('get_website_version', self.website_version)
         Registry().set_flag('website_version', '0.0')
@@ -167,7 +166,7 @@
         self.was_cancelled = False
         self.previous_size = 0
 
-    def _download_progress(self, count, block_size):
+    def update_progress(self, count, block_size):
         """
         Calculate and display the download progress.
         """

=== modified file 'openlp/core/api/websockets.py'
--- openlp/core/api/websockets.py	2017-12-29 09:15:48 +0000
+++ openlp/core/api/websockets.py	2018-01-07 18:07:40 +0000
@@ -28,37 +28,88 @@
 import logging
 import time
 
-import websockets
-from PyQt5 import QtCore
+from websockets import serve
 
 from openlp.core.common.mixins import LogMixin, RegistryProperties
 from openlp.core.common.registry import Registry
 from openlp.core.common.settings import Settings
+from openlp.core.threading import ThreadWorker, run_thread
 
 log = logging.getLogger(__name__)
 
 
-class WebSocketWorker(QtCore.QObject):
+async def handle_websocket(request, path):
+    """
+    Handle web socket requests and return the poll information
+
+    Check every 0.2 seconds to get the latest position and send if it changed. This only gets triggered when the first
+    client connects.
+
+    :param request: request from client
+    :param path: determines the endpoints supported
+    """
+    log.debug('WebSocket handler registered with client')
+    previous_poll = None
+    previous_main_poll = None
+    poller = Registry().get('poller')
+    if path == '/state':
+        while True:
+            current_poll = poller.poll()
+            if current_poll != previous_poll:
+                await request.send(json.dumps(current_poll).encode())
+                previous_poll = current_poll
+            await asyncio.sleep(0.2)
+    elif path == '/live_changed':
+        while True:
+            main_poll = poller.main_poll()
+            if main_poll != previous_main_poll:
+                await request.send(main_poll)
+                previous_main_poll = main_poll
+            await asyncio.sleep(0.2)
+
+
+class WebSocketWorker(ThreadWorker, RegistryProperties, LogMixin):
     """
     A special Qt thread class to allow the WebSockets server to run at the same time as the UI.
     """
-    def __init__(self, server):
-        """
-        Constructor for the thread class.
-
-        :param server: The http server class.
-        """
-        self.ws_server = server
-        super(WebSocketWorker, self).__init__()
-
-    def run(self):
-        """
-        Run the thread.
-        """
-        self.ws_server.start_server()
+    def start(self):
+        """
+        Run the worker.
+        """
+        address = Settings().value('api/ip address')
+        port = Settings().value('api/websocket port')
+        # Start the event loop
+        self.event_loop = asyncio.new_event_loop()
+        asyncio.set_event_loop(self.event_loop)
+        # Create the websocker server
+        loop = 1
+        self.server = None
+        while not self.server:
+            try:
+                self.server = serve(handle_websocket, address, port)
+                log.debug('WebSocket server started on {addr}:{port}'.format(addr=address, port=port))
+            except Exception:
+                log.exception('Failed to start WebSocket server')
+                loop += 1
+                time.sleep(0.1)
+            if not self.server and loop > 3:
+                log.error('Unable to start WebSocket server {addr}:{port}, giving up'.format(addr=address, port=port))
+        if self.server:
+            # If the websocket server exists, start listening
+            self.event_loop.run_until_complete(self.server)
+            self.event_loop.run_forever()
+        self.quit.emit()
 
     def stop(self):
-        self.ws_server.stop = True
+        """
+        Stop the websocket server
+        """
+        if hasattr(self.server, 'ws_server'):
+            self.server.ws_server.close()
+        elif hasattr(self.server, 'server'):
+            self.server.server.close()
+        self.event_loop.stop()
+        self.event_loop.close()
 
 
 class WebSocketServer(RegistryProperties, LogMixin):
@@ -70,74 +121,6 @@
         Initialise and start the WebSockets server
         """
         super(WebSocketServer, self).__init__()
-        if Registry().get_flag('no_web_server'):
-            self.settings_section = 'api'
-            self.worker = WebSocketWorker(self)
-            self.thread = QtCore.QThread()
-            self.worker.moveToThread(self.thread)
-            self.thread.started.connect(self.worker.run)
-            self.thread.start()
-
-    def start_server(self):
-        """
-        Start the correct server and save the handler
-        """
-        address = Settings().value(self.settings_section + '/ip address')
-        port = Settings().value(self.settings_section + '/websocket port')
-        self.start_websocket_instance(address, port)
-        # If web socket server start listening
-        if hasattr(self, 'ws_server') and self.ws_server:
-            event_loop = asyncio.new_event_loop()
-            asyncio.set_event_loop(event_loop)
-            event_loop.run_until_complete(self.ws_server)
-            event_loop.run_forever()
-        else:
-            log.debug('Failed to start ws server on port {port}'.format(port=port))
-
-    def start_websocket_instance(self, address, port):
-        """
-        Start the server
-
-        :param address: The server address
-        :param port: The run port
-        """
-        loop = 1
-        while loop < 4:
-            try:
-                self.ws_server = websockets.serve(self.handle_websocket, address, port)
-                log.debug("Web Socket Server started for class {address} {port}".format(address=address, port=port))
-                break
-            except Exception as e:
-                log.error('Failed to start ws server {why}'.format(why=e))
-                loop += 1
-                time.sleep(0.1)
-
-    @staticmethod
-    async def handle_websocket(request, path):
-        """
-        Handle web socket requests and return the poll information.
-        Check ever 0.2 seconds to get the latest position and send if changed.
-        Only gets triggered when 1st client attaches
-
-        :param request: request from client
-        :param path: determines the endpoints supported
-        :return:
-        """
-        log.debug("web socket handler registered with client")
-        previous_poll = None
-        previous_main_poll = None
-        poller = Registry().get('poller')
-        if path == '/state':
-            while True:
-                current_poll = poller.poll()
-                if current_poll != previous_poll:
-                    await request.send(json.dumps(current_poll).encode())
-                    previous_poll = current_poll
-                await asyncio.sleep(0.2)
-        elif path == '/live_changed':
-            while True:
-                main_poll = poller.main_poll()
-                if main_poll != previous_main_poll:
-                    await request.send(main_poll)
-                    previous_main_poll = main_poll
-                await asyncio.sleep(0.2)
+        if not Registry().get_flag('no_web_server'):
+            worker = WebSocketWorker()
+            run_thread(worker, 'websocket_server')

=== modified file 'openlp/core/app.py'
--- openlp/core/app.py	2017-12-29 09:15:48 +0000
+++ openlp/core/app.py	2018-01-07 18:07:40 +0000
@@ -304,8 +304,7 @@
                              'off a USB flash drive (not implemented).')
     parser.add_argument('-d', '--dev-version', dest='dev_version', action='store_true',
                         help='Ignore the version file and pull the version directly from Bazaar')
-    parser.add_argument('-s', '--style', dest='style', help='Set the Qt5 style (passed directly to Qt5).')
-    parser.add_argument('-w', '--no-web-server', dest='no_web_server', action='store_false',
+    parser.add_argument('-w', '--no-web-server', dest='no_web_server', action='store_true',
                         help='Turn off the Web and Socket Server ')
     parser.add_argument('rargs', nargs='?', default=[])
     # Parse command line options and deal with them. Use args supplied pragmatically if possible.
@@ -343,8 +342,6 @@
         log.setLevel(logging.WARNING)
     else:
         log.setLevel(logging.INFO)
-    if args and args.style:
-        qt_args.extend(['-style', args.style])
     # Throw the rest of the arguments at Qt, just in case.
     qt_args.extend(args.rargs)
     # Bug #1018855: Set the WM_CLASS property in X11
@@ -358,7 +355,7 @@
     application.setOrganizationDomain('openlp.org')
     application.setAttribute(QtCore.Qt.AA_UseHighDpiPixmaps, True)
     application.setAttribute(QtCore.Qt.AA_DontCreateNativeWidgetSiblings, True)
-    if args and args.portable:
+    if args.portable:
         application.setApplicationName('OpenLPPortable')
         Settings.setDefaultFormat(Settings.IniFormat)
         # Get location OpenLPPortable.ini

=== modified file 'openlp/core/common/applocation.py'
--- openlp/core/common/applocation.py	2017-12-29 09:15:48 +0000
+++ openlp/core/common/applocation.py	2018-01-07 18:07:40 +0000
@@ -157,7 +157,7 @@
                 return directory
             return Path('/usr', 'share', 'openlp')
         if XDG_BASE_AVAILABLE:
-            if dir_type == AppLocation.DataDir or dir_type == AppLocation.CacheDir:
+            if dir_type == AppLocation.DataDir:
                 return Path(BaseDirectory.xdg_data_home, 'openlp')
             elif dir_type == AppLocation.CacheDir:
                 return Path(BaseDirectory.xdg_cache_home, 'openlp')

=== modified file 'openlp/core/common/httputils.py'
--- openlp/core/common/httputils.py	2017-12-29 09:15:48 +0000
+++ openlp/core/common/httputils.py	2018-01-07 18:07:40 +0000
@@ -20,7 +20,7 @@
 # Temple Place, Suite 330, Boston, MA 02111-1307 USA                          #
 ###############################################################################
 """
-The :mod:`openlp.core.utils` module provides the utility libraries for OpenLP.
+The :mod:`openlp.core.common.httputils` module provides the utility methods for downloading stuff.
 """
 import hashlib
 import logging
@@ -104,7 +104,7 @@
             if retries >= CONNECTION_RETRIES:
                 raise ConnectionError('Unable to connect to {url}, see log for details'.format(url=url))
             retries += 1
-        except:
+        except:                                                                # noqa
             # Don't know what's happening, so reraise the original
             log.exception('Unknown error when trying to connect to {url}'.format(url=url))
             raise
@@ -136,12 +136,12 @@
                 continue
 
 
-def url_get_file(callback, url, file_path, sha256=None):
+def download_file(update_object, url, file_path, sha256=None):
     """"
     Download a file given a URL.  The file is retrieved in chunks, giving the ability to cancel the download at any
     point. Returns False on download error.
 
-    :param callback: the class which needs to be updated
+    :param update_object: the object which needs to be updated
     :param url: URL to download
     :param file_path: Destination file
     :param sha256: The check sum value to be checked against the download value
@@ -158,13 +158,14 @@
                     hasher = hashlib.sha256()
                 # Download until finished or canceled.
                 for chunk in response.iter_content(chunk_size=block_size):
-                    if callback.was_cancelled:
+                    if hasattr(update_object, 'was_cancelled') and update_object.was_cancelled:
                         break
                     saved_file.write(chunk)
                     if sha256:
                         hasher.update(chunk)
                     block_count += 1
-                    callback._download_progress(block_count, block_size)
+                    if hasattr(update_object, 'update_progress'):
+                        update_object.update_progress(block_count, block_size)
                 response.close()
             if sha256 and hasher.hexdigest() != sha256:
                 log.error('sha256 sums did not match for file %s, got %s, expected %s', file_path, hasher.hexdigest(),
@@ -183,7 +184,7 @@
                 retries += 1
                 time.sleep(0.1)
                 continue
-    if callback.was_cancelled and file_path.exists():
+    if hasattr(update_object, 'was_cancelled') and update_object.was_cancelled and file_path.exists():
         file_path.unlink()
     return True
 

=== modified file 'openlp/core/lib/imagemanager.py'
--- openlp/core/lib/imagemanager.py	2017-12-29 09:15:48 +0000
+++ openlp/core/lib/imagemanager.py	2018-01-07 18:07:40 +0000
@@ -35,13 +35,14 @@
 from openlp.core.common.settings import Settings
 from openlp.core.display.screens import ScreenList
 from openlp.core.lib import resize_image, image_to_byte
+from openlp.core.threading import ThreadWorker, run_thread
 
 log = logging.getLogger(__name__)
 
 
-class ImageThread(QtCore.QThread):
+class ImageWorker(ThreadWorker):
     """
-    A special Qt thread class to speed up the display of images. This is threaded so it loads the frames and generates
+    A thread worker class to speed up the display of images. This is threaded so it loads the frames and generates
     byte stream in background.
     """
     def __init__(self, manager):
@@ -51,14 +52,21 @@
         ``manager``
             The image manager.
         """
-        super(ImageThread, self).__init__(None)
+        super().__init__()
         self.image_manager = manager
 
-    def run(self):
+    def start(self):
         """
-        Run the thread.
+        Start the worker
         """
         self.image_manager.process()
+        self.quit.emit()
+
+    def stop(self):
+        """
+        Stop the worker
+        """
+        self.image_manager.stop_manager = True
 
 
 class Priority(object):
@@ -130,7 +138,7 @@
 
 class PriorityQueue(queue.PriorityQueue):
     """
-    Customised ``Queue.PriorityQueue``.
+    Customised ``queue.PriorityQueue``.
 
     Each item in the queue must be a tuple with three values. The first value is the :class:`Image`'s ``priority``
     attribute, the second value the :class:`Image`'s ``secondary_priority`` attribute. The last value the :class:`Image`
@@ -179,7 +187,6 @@
         self.width = current_screen['size'].width()
         self.height = current_screen['size'].height()
         self._cache = {}
-        self.image_thread = ImageThread(self)
         self._conversion_queue = PriorityQueue()
         self.stop_manager = False
         Registry().register_function('images_regenerate', self.process_updates)
@@ -230,9 +237,13 @@
         """
         Flush the queue to updated any data to update
         """
-        # We want only one thread.
-        if not self.image_thread.isRunning():
-            self.image_thread.start()
+        try:
+            worker = ImageWorker(self)
+            run_thread(worker, 'image_manager')
+        except KeyError:
+            # run_thread() will throw a KeyError if this thread already exists, so ignore it so that we don't
+            # try to start another thread when one is already running
+            pass
 
     def get_image(self, path, source, width=-1, height=-1):
         """
@@ -305,9 +316,7 @@
                 if image.path == path and image.timestamp != os.stat(path).st_mtime:
                     image.timestamp = os.stat(path).st_mtime
                     self._reset_image(image)
-        # We want only one thread.
-        if not self.image_thread.isRunning():
-            self.image_thread.start()
+        self.process_updates()
 
     def process(self):
         """

=== modified file 'openlp/core/projectors/manager.py'
--- openlp/core/projectors/manager.py	2018-01-03 00:35:14 +0000
+++ openlp/core/projectors/manager.py	2018-01-07 18:07:40 +0000
@@ -308,8 +308,7 @@
         self.settings_section = 'projector'
         self.projectordb = projectordb
         self.projector_list = []
-        self.pjlink_udp = PJLinkUDP()
-        self.pjlink_udp.projector_list = self.projector_list
+        self.pjlink_udp = PJLinkUDP(self.projector_list)
         self.source_select_form = None
 
     def bootstrap_initialise(self):

=== modified file 'openlp/core/projectors/pjlink.py'
--- openlp/core/projectors/pjlink.py	2018-01-03 00:35:14 +0000
+++ openlp/core/projectors/pjlink.py	2018-01-07 18:07:40 +0000
@@ -89,11 +89,11 @@
         'SRCH'   # Class 2  (reply is ACKN)
     ]
 
-    def __init__(self, port=PJLINK_PORT):
+    def __init__(self, projector_list, port=PJLINK_PORT):
         """
         Initialize socket
         """
-
+        self.projector_list = projector_list
         self.port = port
 
 

=== modified file 'openlp/core/threading.py'
--- openlp/core/threading.py	2017-12-29 09:15:48 +0000
+++ openlp/core/threading.py	2018-01-07 18:07:40 +0000
@@ -24,26 +24,41 @@
 """
 from PyQt5 import QtCore
 
-
-def run_thread(parent, worker, prefix='', auto_start=True):
+from openlp.core.common.registry import Registry
+
+
+class ThreadWorker(QtCore.QObject):
+    """
+    The :class:`~openlp.core.threading.ThreadWorker` class provides a base class for all worker objects
+    """
+    quit = QtCore.pyqtSignal()
+
+    def start(self):
+        """
+        The start method is how the worker runs. Basically, put your code here.
+        """
+        raise NotImplementedError('Your base class needs to override this method and run self.quit.emit() at the end.')
+
+
+def run_thread(worker, thread_name, can_start=True):
     """
     Create a thread and assign a worker to it. This removes a lot of boilerplate code from the codebase.
 
-    :param object parent: The parent object so that the thread and worker are not orphaned.
     :param QObject worker: A QObject-based worker object which does the actual work.
-    :param str prefix: A prefix to be applied to the attribute names.
-    :param bool auto_start: Automatically start the thread. Defaults to True.
+    :param str thread_name: The name of the thread, used to keep track of the thread.
+    :param bool can_start: Start the thread. Defaults to True.
     """
-    # Set up attribute names
-    thread_name = 'thread'
-    worker_name = 'worker'
-    if prefix:
-        thread_name = '_'.join([prefix, thread_name])
-        worker_name = '_'.join([prefix, worker_name])
+    if not thread_name:
+        raise ValueError('A thread_name is required when calling the "run_thread" function')
+    main_window = Registry().get('main_window')
+    if thread_name in main_window.threads:
+        raise KeyError('A thread with the name "{}" has already been created, please use another'.format(thread_name))
     # Create the thread and add the thread and the worker to the parent
     thread = QtCore.QThread()
-    setattr(parent, thread_name, thread)
-    setattr(parent, worker_name, worker)
+    main_window.threads[thread_name] = {
+        'thread': thread,
+        'worker': worker
+    }
     # Move the worker into the thread's context
     worker.moveToThread(thread)
     # Connect slots and signals
@@ -51,5 +66,46 @@
     worker.quit.connect(thread.quit)
     worker.quit.connect(worker.deleteLater)
     thread.finished.connect(thread.deleteLater)
-    if auto_start:
+    thread.finished.connect(make_remove_thread(thread_name))
+    if can_start:
         thread.start()
+
+
+def get_thread_worker(thread_name):
+    """
+    Get the worker by the thread name
+
+    :param str thread_name: The name of the thread
+    :returns: The worker for this thread name
+    """
+    return Registry().get('main_window').threads.get(thread_name)
+
+
+def is_thread_finished(thread_name):
+    """
+    Check if a thread is finished running.
+
+    :param str thread_name: The name of the thread
+    :returns: True if the thread is finished, False if it is still running
+    """
+    main_window = Registry().get('main_window')
+    return thread_name not in main_window.threads or main_window.threads[thread_name]['thread'].isFinished()
+
+
+def make_remove_thread(thread_name):
+    """
+    Create a function to remove the thread once the thread is finished.
+
+    :param str thread_name: The name of the thread which should be removed from the thread registry.
+    :returns: A function which will remove the thread from the thread registry.
+    """
+    def remove_thread():
+        """
+        Stop and remove a registered thread
+
+        :param str thread_name: The name of the thread to stop and remove
+        """
+        main_window = Registry().get('main_window')
+        if thread_name in main_window.threads:
+            del main_window.threads[thread_name]
+    return remove_thread

=== modified file 'openlp/core/ui/firsttimeform.py'
--- openlp/core/ui/firsttimeform.py	2017-12-29 09:15:48 +0000
+++ openlp/core/ui/firsttimeform.py	2018-01-07 18:07:40 +0000
@@ -23,8 +23,6 @@
 This module contains the first time wizard.
 """
 import logging
-import os
-import socket
 import time
 import urllib.error
 import urllib.parse
@@ -36,7 +34,7 @@
 
 from openlp.core.common import clean_button_text, trace_error_handler
 from openlp.core.common.applocation import AppLocation
-from openlp.core.common.httputils import get_web_page, get_url_file_size, url_get_file, CONNECTION_TIMEOUT
+from openlp.core.common.httputils import get_web_page, get_url_file_size, download_file
 from openlp.core.common.i18n import translate
 from openlp.core.common.mixins import RegistryProperties
 from openlp.core.common.path import Path, create_paths
@@ -44,46 +42,47 @@
 from openlp.core.common.settings import Settings
 from openlp.core.lib import PluginStatus, build_icon
 from openlp.core.lib.ui import critical_error_message_box
-from .firsttimewizard import UiFirstTimeWizard, FirstTimePage
+from openlp.core.threading import ThreadWorker, run_thread, get_thread_worker, is_thread_finished
+from openlp.core.ui.firsttimewizard import UiFirstTimeWizard, FirstTimePage
 
 log = logging.getLogger(__name__)
 
 
-class ThemeScreenshotWorker(QtCore.QObject):
+class ThemeScreenshotWorker(ThreadWorker):
     """
     This thread downloads a theme's screenshot
     """
     screenshot_downloaded = QtCore.pyqtSignal(str, str, str)
-    finished = QtCore.pyqtSignal()
 
     def __init__(self, themes_url, title, filename, sha256, screenshot):
         """
         Set up the worker object
         """
-        self.was_download_cancelled = False
+        self.was_cancelled = False
         self.themes_url = themes_url
         self.title = title
         self.filename = filename
         self.sha256 = sha256
         self.screenshot = screenshot
-        socket.setdefaulttimeout(CONNECTION_TIMEOUT)
-        super(ThemeScreenshotWorker, self).__init__()
+        super().__init__()
 
-    def run(self):
-        """
-        Overridden method to run the thread.
-        """
-        if self.was_download_cancelled:
+    def start(self):
+        """
+        Run the worker
+        """
+        if self.was_cancelled:
             return
         try:
-            urllib.request.urlretrieve('{host}{name}'.format(host=self.themes_url, name=self.screenshot),
-                                       os.path.join(gettempdir(), 'openlp', self.screenshot))
-            # Signal that the screenshot has been downloaded
-            self.screenshot_downloaded.emit(self.title, self.filename, self.sha256)
-        except:
+            download_path = Path(gettempdir()) / 'openlp' / self.screenshot
+            is_success = download_file(self, '{host}{name}'.format(host=self.themes_url, name=self.screenshot),
+                                       download_path)
+            if is_success and not self.was_cancelled:
+                # Signal that the screenshot has been downloaded
+                self.screenshot_downloaded.emit(self.title, self.filename, self.sha256)
+        except:                                                                 # noqa
             log.exception('Unable to download screenshot')
         finally:
-            self.finished.emit()
+            self.quit.emit()
 
     @QtCore.pyqtSlot(bool)
     def set_download_canceled(self, toggle):
@@ -145,12 +144,13 @@
             return FirstTimePage.Progress
         elif self.currentId() == FirstTimePage.Themes:
             self.application.set_busy_cursor()
-            while not all([thread.isFinished() for thread in self.theme_screenshot_threads]):
+            while not all([is_thread_finished(thread_name) for thread_name in self.theme_screenshot_threads]):
                 time.sleep(0.1)
                 self.application.process_events()
             # Build the screenshot icons, as this can not be done in the thread.
             self._build_theme_screenshots()
             self.application.set_normal_cursor()
+            self.theme_screenshot_threads = []
             return FirstTimePage.Defaults
         else:
             return self.get_next_page_id()
@@ -171,7 +171,6 @@
         self.screens = screens
         self.was_cancelled = False
         self.theme_screenshot_threads = []
-        self.theme_screenshot_workers = []
         self.has_run_wizard = False
 
     def _download_index(self):
@@ -256,14 +255,10 @@
                 sha256 = self.config.get('theme_{theme}'.format(theme=theme), 'sha256', fallback='')
                 screenshot = self.config.get('theme_{theme}'.format(theme=theme), 'screenshot')
                 worker = ThemeScreenshotWorker(self.themes_url, title, filename, sha256, screenshot)
-                self.theme_screenshot_workers.append(worker)
                 worker.screenshot_downloaded.connect(self.on_screenshot_downloaded)
-                thread = QtCore.QThread(self)
-                self.theme_screenshot_threads.append(thread)
-                thread.started.connect(worker.run)
-                worker.finished.connect(thread.quit)
-                worker.moveToThread(thread)
-                thread.start()
+                thread_name = 'theme_screenshot_{title}'.format(title=title)
+                run_thread(worker, thread_name)
+                self.theme_screenshot_threads.append(thread_name)
             self.application.process_events()
 
     def set_defaults(self):
@@ -353,12 +348,14 @@
         Process the triggering of the cancel button.
         """
         self.was_cancelled = True
-        if self.theme_screenshot_workers:
-            for worker in self.theme_screenshot_workers:
-                worker.set_download_canceled(True)
+        if self.theme_screenshot_threads:
+            for thread_name in self.theme_screenshot_threads:
+                worker = get_thread_worker(thread_name)
+                if worker:
+                    worker.set_download_canceled(True)
         # Was the thread created.
         if self.theme_screenshot_threads:
-            while any([thread.isRunning() for thread in self.theme_screenshot_threads]):
+            while any([not is_thread_finished(thread_name) for thread_name in self.theme_screenshot_threads]):
                 time.sleep(0.1)
         self.application.set_normal_cursor()
 
@@ -562,8 +559,8 @@
                 self._increment_progress_bar(self.downloading.format(name=filename), 0)
                 self.previous_size = 0
                 destination = songs_destination_path / str(filename)
-                if not url_get_file(self, '{path}{name}'.format(path=self.songs_url, name=filename),
-                                    destination, sha256):
+                if not download_file(self, '{path}{name}'.format(path=self.songs_url, name=filename),
+                                     destination, sha256):
                     missed_files.append('Song: {name}'.format(name=filename))
         # Download Bibles
         bibles_iterator = QtWidgets.QTreeWidgetItemIterator(self.bibles_tree_widget)
@@ -573,8 +570,8 @@
                 bible, sha256 = item.data(0, QtCore.Qt.UserRole)
                 self._increment_progress_bar(self.downloading.format(name=bible), 0)
                 self.previous_size = 0
-                if not url_get_file(self, '{path}{name}'.format(path=self.bibles_url, name=bible),
-                                    bibles_destination_path / bible, sha256):
+                if not download_file(self, '{path}{name}'.format(path=self.bibles_url, name=bible),
+                                     bibles_destination_path / bible, sha256):
                     missed_files.append('Bible: {name}'.format(name=bible))
             bibles_iterator += 1
         # Download themes
@@ -584,8 +581,8 @@
                 theme, sha256 = item.data(QtCore.Qt.UserRole)
                 self._increment_progress_bar(self.downloading.format(name=theme), 0)
                 self.previous_size = 0
-                if not url_get_file(self, '{path}{name}'.format(path=self.themes_url, name=theme),
-                                    themes_destination_path / theme, sha256):
+                if not download_file(self, '{path}{name}'.format(path=self.themes_url, name=theme),
+                                     themes_destination_path / theme, sha256):
                     missed_files.append('Theme: {name}'.format(name=theme))
         if missed_files:
             file_list = ''

=== modified file 'openlp/core/ui/mainwindow.py'
--- openlp/core/ui/mainwindow.py	2017-12-29 09:15:48 +0000
+++ openlp/core/ui/mainwindow.py	2018-01-07 18:07:40 +0000
@@ -24,7 +24,6 @@
 """
 import logging
 import sys
-import time
 from datetime import datetime
 from distutils import dir_util
 from distutils.errors import DistutilsFileError
@@ -478,8 +477,7 @@
         """
         super(MainWindow, self).__init__()
         Registry().register('main_window', self)
-        self.version_thread = None
-        self.version_worker = None
+        self.threads = {}
         self.clipboard = self.application.clipboard()
         self.arguments = ''.join(self.application.args)
         # Set up settings sections for the main application (not for use by plugins).
@@ -501,8 +499,8 @@
         Settings().set_up_default_values()
         self.about_form = AboutForm(self)
         MediaController()
-        websockets.WebSocketServer()
-        server.HttpServer()
+        self.ws_server = websockets.WebSocketServer()
+        self.http_server = server.HttpServer(self)
         SettingsForm(self)
         self.formatting_tag_form = FormattingTagForm(self)
         self.shortcut_form = ShortcutListForm(self)
@@ -549,6 +547,41 @@
         # Reset the cursor
         self.application.set_normal_cursor()
 
+    def _wait_for_threads(self):
+        """
+        Wait for the threads
+        """
+        # Sometimes the threads haven't finished, let's wait for them
+        wait_dialog = QtWidgets.QProgressDialog('Waiting for some things to finish...', '', 0, 0, self)
+        wait_dialog.setWindowModality(QtCore.Qt.WindowModal)
+        wait_dialog.setAutoClose(False)
+        wait_dialog.setCancelButton(None)
+        wait_dialog.show()
+        for thread_name in self.threads.keys():
+            log.debug('Waiting for thread %s', thread_name)
+            self.application.processEvents()
+            thread = self.threads[thread_name]['thread']
+            worker = self.threads[thread_name]['worker']
+            try:
+                if worker and hasattr(worker, 'stop'):
+                    # If the worker has a stop method, run it
+                    worker.stop()
+                if thread and thread.isRunning():
+                    # If the thread is running, let's wait 5 seconds for it
+                    retry = 0
+                    while thread.isRunning() and retry < 50:
+                        # Make the GUI responsive while we wait
+                        self.application.processEvents()
+                        thread.wait(100)
+                        retry += 1
+                    if thread.isRunning():
+                        # If the thread is still running after 5 seconds, kill it
+                        thread.terminate()
+            except RuntimeError:
+                # Ignore the RuntimeError that is thrown when Qt has already deleted the C++ thread object
+                pass
+        wait_dialog.close()
+
     def bootstrap_post_set_up(self):
         """
         process the bootstrap post setup request
@@ -695,7 +728,7 @@
         # Update the theme widget
         self.theme_manager_contents.load_themes()
         # Check if any Bibles downloaded.  If there are, they will be processed.
-        Registry().execute('bibles_load_list', True)
+        Registry().execute('bibles_load_list')
         self.application.set_normal_cursor()
 
     def is_display_blank(self):
@@ -1000,39 +1033,14 @@
         if not self.application.is_event_loop_active:
             event.ignore()
             return
-        # Sometimes the version thread hasn't finished, let's wait for it
-        try:
-            if self.version_thread and self.version_thread.isRunning():
-                wait_dialog = QtWidgets.QProgressDialog('Waiting for some things to finish...', '', 0, 0, self)
-                wait_dialog.setWindowModality(QtCore.Qt.WindowModal)
-                wait_dialog.setAutoClose(False)
-                wait_dialog.setCancelButton(None)
-                wait_dialog.show()
-                retry = 0
-                while self.version_thread.isRunning() and retry < 50:
-                    self.application.processEvents()
-                    self.version_thread.wait(100)
-                    retry += 1
-                if self.version_thread.isRunning():
-                    self.version_thread.terminate()
-                wait_dialog.close()
-        except RuntimeError:
-            # Ignore the RuntimeError that is thrown when Qt has already deleted the C++ thread object
-            pass
-        # If we just did a settings import, close without saving changes.
-        if self.settings_imported:
-            self.clean_up(False)
-            event.accept()
         if self.service_manager_contents.is_modified():
             ret = self.service_manager_contents.save_modified_service()
             if ret == QtWidgets.QMessageBox.Save:
                 if self.service_manager_contents.decide_save_method():
-                    self.clean_up()
                     event.accept()
                 else:
                     event.ignore()
             elif ret == QtWidgets.QMessageBox.Discard:
-                self.clean_up()
                 event.accept()
             else:
                 event.ignore()
@@ -1048,13 +1056,16 @@
                 close_button.setText(translate('OpenLP.MainWindow', '&Exit OpenLP'))
                 msg_box.setDefaultButton(QtWidgets.QMessageBox.Close)
                 if msg_box.exec() == QtWidgets.QMessageBox.Close:
-                    self.clean_up()
                     event.accept()
                 else:
                     event.ignore()
             else:
-                self.clean_up()
                 event.accept()
+        if event.isAccepted():
+            # Wait for all the threads to complete
+            self._wait_for_threads()
+            # If we just did a settings import, close without saving changes.
+            self.clean_up(save_settings=not self.settings_imported)
 
     def clean_up(self, save_settings=True):
         """
@@ -1062,9 +1073,6 @@
 
         :param save_settings: Switch to prevent saving settings. Defaults to **True**.
         """
-        self.image_manager.stop_manager = True
-        while self.image_manager.image_thread.isRunning():
-            time.sleep(0.1)
         if save_settings:
             if Settings().value('advanced/save current plugin'):
                 Settings().setValue('advanced/current media plugin', self.media_tool_box.currentIndex())

=== modified file 'openlp/core/ui/media/systemplayer.py'
--- openlp/core/ui/media/systemplayer.py	2017-12-29 09:15:48 +0000
+++ openlp/core/ui/media/systemplayer.py	2018-01-07 18:07:40 +0000
@@ -31,6 +31,7 @@
 from openlp.core.common.i18n import translate
 from openlp.core.ui.media import MediaState
 from openlp.core.ui.media.mediaplayer import MediaPlayer
+from openlp.core.threading import ThreadWorker, run_thread, is_thread_finished
 
 log = logging.getLogger(__name__)
 
@@ -293,39 +294,38 @@
         :param path: Path to file to be checked
         :return: True if file can be played otherwise False
         """
-        thread = QtCore.QThread()
         check_media_worker = CheckMediaWorker(path)
         check_media_worker.setVolume(0)
-        check_media_worker.moveToThread(thread)
-        check_media_worker.finished.connect(thread.quit)
-        thread.started.connect(check_media_worker.play)
-        thread.start()
-        while thread.isRunning():
+        run_thread(check_media_worker, 'check_media')
+        while not is_thread_finished('check_media'):
             self.application.processEvents()
         return check_media_worker.result
 
 
-class CheckMediaWorker(QtMultimedia.QMediaPlayer):
+class CheckMediaWorker(QtMultimedia.QMediaPlayer, ThreadWorker):
     """
     Class used to check if a media file is playable
     """
-    finished = QtCore.pyqtSignal()
-
     def __init__(self, path):
         super(CheckMediaWorker, self).__init__(None, QtMultimedia.QMediaPlayer.VideoSurface)
+        self.path = path
+
+    def start(self):
+        """
+        Start the thread worker
+        """
         self.result = None
-
         self.error.connect(functools.partial(self.signals, 'error'))
         self.mediaStatusChanged.connect(functools.partial(self.signals, 'media'))
-
-        self.setMedia(QtMultimedia.QMediaContent(QtCore.QUrl.fromLocalFile(path)))
+        self.setMedia(QtMultimedia.QMediaContent(QtCore.QUrl.fromLocalFile(self.path)))
+        self.play()
 
     def signals(self, origin, status):
         if origin == 'media' and status == self.BufferedMedia:
             self.result = True
             self.stop()
-            self.finished.emit()
+            self.quit.emit()
         elif origin == 'error' and status != self.NoError:
             self.result = False
             self.stop()
-            self.finished.emit()
+            self.quit.emit()

=== modified file 'openlp/core/version.py'
--- openlp/core/version.py	2018-01-02 21:00:54 +0000
+++ openlp/core/version.py	2018-01-07 18:07:40 +0000
@@ -35,7 +35,7 @@
 
 from openlp.core.common.applocation import AppLocation
 from openlp.core.common.settings import Settings
-from openlp.core.threading import run_thread
+from openlp.core.threading import ThreadWorker, run_thread
 
 log = logging.getLogger(__name__)
 
@@ -44,14 +44,13 @@
 CONNECTION_RETRIES = 2
 
 
-class VersionWorker(QtCore.QObject):
+class VersionWorker(ThreadWorker):
     """
     A worker class to fetch the version of OpenLP from the website. This is run from within a thread so that it
     doesn't affect the loading time of OpenLP.
     """
     new_version = QtCore.pyqtSignal(dict)
     no_internet = QtCore.pyqtSignal()
-    quit = QtCore.pyqtSignal()
 
     def __init__(self, last_check_date, current_version):
         """
@@ -110,22 +109,22 @@
     Settings().setValue('core/last version test', date.today().strftime('%Y-%m-%d'))
 
 
-def check_for_update(parent):
+def check_for_update(main_window):
     """
     Run a thread to download and check the version of OpenLP
 
-    :param MainWindow parent: The parent object for the thread. Usually the OpenLP main window.
+    :param MainWindow main_window: The OpenLP main window.
     """
     last_check_date = Settings().value('core/last version test')
     if date.today().strftime('%Y-%m-%d') <= last_check_date:
         log.debug('Version check skipped, last checked today')
         return
     worker = VersionWorker(last_check_date, get_version())
-    worker.new_version.connect(parent.on_new_version)
+    worker.new_version.connect(main_window.on_new_version)
     worker.quit.connect(update_check_date)
     # TODO: Use this to figure out if there's an Internet connection?
     # worker.no_internet.connect(parent.on_no_internet)
-    run_thread(parent, worker, 'version')
+    run_thread(worker, 'version')
 
 
 def get_version():

=== modified file 'openlp/plugins/songs/forms/songselectform.py'
--- openlp/plugins/songs/forms/songselectform.py	2017-12-29 09:15:48 +0000
+++ openlp/plugins/songs/forms/songselectform.py	2018-01-07 18:07:40 +0000
@@ -27,24 +27,23 @@
 
 from PyQt5 import QtCore, QtWidgets
 
-from openlp.core.common import is_win
 from openlp.core.common.i18n import translate
-from openlp.core.common.registry import Registry
+from openlp.core.common.mixins import RegistryProperties
 from openlp.core.common.settings import Settings
+from openlp.core.threading import ThreadWorker, run_thread
 from openlp.plugins.songs.forms.songselectdialog import Ui_SongSelectDialog
 from openlp.plugins.songs.lib.songselect import SongSelectImport
 
 log = logging.getLogger(__name__)
 
 
-class SearchWorker(QtCore.QObject):
+class SearchWorker(ThreadWorker):
     """
     Run the actual SongSelect search, and notify the GUI when we find each song.
     """
     show_info = QtCore.pyqtSignal(str, str)
     found_song = QtCore.pyqtSignal(dict)
     finished = QtCore.pyqtSignal()
-    quit = QtCore.pyqtSignal()
 
     def __init__(self, importer, search_text):
         super().__init__()
@@ -74,7 +73,7 @@
         self.found_song.emit(song)
 
 
-class SongSelectForm(QtWidgets.QDialog, Ui_SongSelectDialog):
+class SongSelectForm(QtWidgets.QDialog, Ui_SongSelectDialog, RegistryProperties):
     """
     The :class:`SongSelectForm` class is the SongSelect dialog.
     """
@@ -90,8 +89,6 @@
         """
         Initialise the SongSelectForm
         """
-        self.thread = None
-        self.worker = None
         self.song_count = 0
         self.song = None
         self.set_progress_visible(False)
@@ -311,17 +308,11 @@
         search_history = self.search_combobox.getItems()
         Settings().setValue(self.plugin.settings_section + '/songselect searches', '|'.join(search_history))
         # Create thread and run search
-        self.thread = QtCore.QThread()
-        self.worker = SearchWorker(self.song_select_importer, self.search_combobox.currentText())
-        self.worker.moveToThread(self.thread)
-        self.thread.started.connect(self.worker.start)
-        self.worker.show_info.connect(self.on_search_show_info)
-        self.worker.found_song.connect(self.on_search_found_song)
-        self.worker.finished.connect(self.on_search_finished)
-        self.worker.quit.connect(self.thread.quit)
-        self.worker.quit.connect(self.worker.deleteLater)
-        self.thread.finished.connect(self.thread.deleteLater)
-        self.thread.start()
+        worker = SearchWorker(self.song_select_importer, self.search_combobox.currentText())
+        worker.show_info.connect(self.on_search_show_info)
+        worker.found_song.connect(self.on_search_found_song)
+        worker.finished.connect(self.on_search_finished)
+        run_thread(worker, 'songselect')
 
     def on_stop_button_clicked(self):
         """
@@ -408,16 +399,3 @@
         """
         self.search_progress_bar.setVisible(is_visible)
         self.stop_button.setVisible(is_visible)
-
-    @property
-    def application(self):
-        """
-        Adds the openlp to the class dynamically.
-        Windows needs to access the application in a dynamic manner.
-        """
-        if is_win():
-            return Registry().get('application')
-        else:
-            if not hasattr(self, '_application'):
-                self._application = Registry().get('application')
-            return self._application

=== modified file 'tests/functional/openlp_core/api/http/test_http.py'
--- tests/functional/openlp_core/api/http/test_http.py	2017-12-29 09:15:48 +0000
+++ tests/functional/openlp_core/api/http/test_http.py	2018-01-07 18:07:40 +0000
@@ -42,8 +42,23 @@
         Registry().register('service_list', MagicMock())
 
     @patch('openlp.core.api.http.server.HttpWorker')
-    @patch('openlp.core.api.http.server.QtCore.QThread')
-    def test_server_start(self, mock_qthread, mock_thread):
+    @patch('openlp.core.api.http.server.run_thread')
+    def test_server_start(self, mocked_run_thread, MockHttpWorker):
+        """
+        Test the starting of the Waitress Server with the disable flag set off
+        """
+        # GIVEN: A new httpserver
+        # WHEN: I start the server
+        Registry().set_flag('no_web_server', False)
+        HttpServer()
+
+        # THEN: the api environment should have been created
+        assert mocked_run_thread.call_count == 1, 'The qthread should have been called once'
+        assert MockHttpWorker.call_count == 1, 'The http thread should have been called once'
+
+    @patch('openlp.core.api.http.server.HttpWorker')
+    @patch('openlp.core.api.http.server.run_thread')
+    def test_server_start_not_required(self, mocked_run_thread, MockHttpWorker):
         """
         Test the starting of the Waitress Server with the disable flag set off
         """
@@ -53,20 +68,5 @@
         HttpServer()
 
         # THEN: the api environment should have been created
-        assert mock_qthread.call_count == 1, 'The qthread should have been called once'
-        assert mock_thread.call_count == 1, 'The http thread should have been called once'
-
-    @patch('openlp.core.api.http.server.HttpWorker')
-    @patch('openlp.core.api.http.server.QtCore.QThread')
-    def test_server_start_not_required(self, mock_qthread, mock_thread):
-        """
-        Test the starting of the Waitress Server with the disable flag set off
-        """
-        # GIVEN: A new httpserver
-        # WHEN: I start the server
-        Registry().set_flag('no_web_server', False)
-        HttpServer()
-
-        # THEN: the api environment should have been created
-        assert mock_qthread.call_count == 0, 'The qthread should not have have been called'
-        assert mock_thread.call_count == 0, 'The http thread should not have been called'
+        assert mocked_run_thread.call_count == 0, 'The qthread should not have have been called'
+        assert MockHttpWorker.call_count == 0, 'The http thread should not have been called'

=== modified file 'tests/functional/openlp_core/api/test_websockets.py'
--- tests/functional/openlp_core/api/test_websockets.py	2017-12-29 09:15:48 +0000
+++ tests/functional/openlp_core/api/test_websockets.py	2018-01-07 18:07:40 +0000
@@ -63,34 +63,34 @@
         self.destroy_settings()
 
     @patch('openlp.core.api.websockets.WebSocketWorker')
-    @patch('openlp.core.api.websockets.QtCore.QThread')
-    def test_serverstart(self, mock_qthread, mock_worker):
+    @patch('openlp.core.api.websockets.run_thread')
+    def test_serverstart(self, mocked_run_thread, MockWebSocketWorker):
         """
         Test the starting of the WebSockets Server with the disabled flag set on
         """
         # GIVEN: A new httpserver
         # WHEN: I start the server
-        Registry().set_flag('no_web_server', True)
+        Registry().set_flag('no_web_server', False)
         WebSocketServer()
 
         # THEN: the api environment should have been created
-        assert mock_qthread.call_count == 1, 'The qthread should have been called once'
-        assert mock_worker.call_count == 1, 'The http thread should have been called once'
+        assert mocked_run_thread.call_count == 1, 'The qthread should have been called once'
+        assert MockWebSocketWorker.call_count == 1, 'The http thread should have been called once'
 
     @patch('openlp.core.api.websockets.WebSocketWorker')
-    @patch('openlp.core.api.websockets.QtCore.QThread')
-    def test_serverstart_not_required(self, mock_qthread, mock_worker):
+    @patch('openlp.core.api.websockets.run_thread')
+    def test_serverstart_not_required(self, mocked_run_thread, MockWebSocketWorker):
         """
         Test the starting of the WebSockets Server with the disabled flag set off
         """
         # GIVEN: A new httpserver and the server is not required
         # WHEN: I start the server
-        Registry().set_flag('no_web_server', False)
+        Registry().set_flag('no_web_server', True)
         WebSocketServer()
 
         # THEN: the api environment should have been created
-        assert mock_qthread.call_count == 0, 'The qthread should not have been called'
-        assert mock_worker.call_count == 0, 'The http thread should not have been called'
+        assert mocked_run_thread.call_count == 0, 'The qthread should not have been called'
+        assert MockWebSocketWorker.call_count == 0, 'The http thread should not have been called'
 
     def test_main_poll(self):
         """

=== modified file 'tests/functional/openlp_core/common/test_httputils.py'
--- tests/functional/openlp_core/common/test_httputils.py	2017-12-29 09:15:48 +0000
+++ tests/functional/openlp_core/common/test_httputils.py	2018-01-07 18:07:40 +0000
@@ -27,7 +27,7 @@
 from unittest import TestCase
 from unittest.mock import MagicMock, patch
 
-from openlp.core.common.httputils import get_user_agent, get_web_page, get_url_file_size, url_get_file
+from openlp.core.common.httputils import get_user_agent, get_web_page, get_url_file_size, download_file
 from openlp.core.common.path import Path
 from tests.helpers.testmixin import TestMixin
 
@@ -235,7 +235,7 @@
         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))
+        download_file(MagicMock(), url='http://localhost/test', file_path=Path(self.tempfile))
 
         # THEN: socket.timeout should have been caught
         # NOTE: Test is if $tmpdir/tempfile is still there, then test fails since ftw deletes bad downloaded files

=== modified file 'tests/functional/openlp_core/lib/test_image_manager.py'
--- tests/functional/openlp_core/lib/test_image_manager.py	2017-12-28 08:22:55 +0000
+++ tests/functional/openlp_core/lib/test_image_manager.py	2018-01-07 18:07:40 +0000
@@ -25,20 +25,113 @@
 import os
 import time
 from threading import Lock
-from unittest import TestCase
-from unittest.mock import patch
+from unittest import TestCase, skip
+from unittest.mock import MagicMock, patch
 
 from PyQt5 import QtGui
 
 from openlp.core.common.registry import Registry
 from openlp.core.display.screens import ScreenList
-from openlp.core.lib.imagemanager import ImageManager, Priority
+from openlp.core.lib.imagemanager import ImageWorker, ImageManager, Priority, PriorityQueue
 from tests.helpers.testmixin import TestMixin
 from tests.utils.constants import RESOURCE_PATH
 
 TEST_PATH = str(RESOURCE_PATH)
 
 
+class TestImageWorker(TestCase, TestMixin):
+    """
+    Test all the methods in the ImageWorker class
+    """
+    def test_init(self):
+        """
+        Test the constructor of the ImageWorker
+        """
+        # GIVEN: An ImageWorker class and a mocked ImageManager
+        mocked_image_manager = MagicMock()
+
+        # WHEN: Creating the ImageWorker
+        worker = ImageWorker(mocked_image_manager)
+
+        # THEN: The image_manager attribute should be set correctly
+        assert worker.image_manager is mocked_image_manager, \
+            'worker.image_manager should have been the mocked_image_manager'
+
+    @patch('openlp.core.lib.imagemanager.ThreadWorker.quit')
+    def test_start(self, mocked_quit):
+        """
+        Test that the start() method of the image worker calls the process method and then emits quit.
+        """
+        # GIVEN: A mocked image_manager and a new image worker
+        mocked_image_manager = MagicMock()
+        worker = ImageWorker(mocked_image_manager)
+
+        # WHEN: start() is called
+        worker.start()
+
+        # THEN: process() should have been called and quit should have been emitted
+        mocked_image_manager.process.assert_called_once_with()
+        mocked_quit.emit.assert_called_once_with()
+
+    def test_stop(self):
+        """
+        Test that the stop method does the right thing
+        """
+        # GIVEN: A mocked image_manager and a worker
+        mocked_image_manager = MagicMock()
+        worker = ImageWorker(mocked_image_manager)
+
+        # WHEN: The stop() method is called
+        worker.stop()
+
+        # THEN: The stop_manager attrivute should have been set to True
+        assert mocked_image_manager.stop_manager is True, 'mocked_image_manager.stop_manager should have been True'
+
+
+class TestPriorityQueue(TestCase, TestMixin):
+    """
+    Test the PriorityQueue class
+    """
+    @patch('openlp.core.lib.imagemanager.PriorityQueue.remove')
+    @patch('openlp.core.lib.imagemanager.PriorityQueue.put')
+    def test_modify_priority(self, mocked_put, mocked_remove):
+        """
+        Test the modify_priority() method of PriorityQueue
+        """
+        # GIVEN: An instance of a PriorityQueue and a mocked image
+        mocked_image = MagicMock()
+        mocked_image.priority = Priority.Normal
+        mocked_image.secondary_priority = Priority.Low
+        queue = PriorityQueue()
+
+        # WHEN: modify_priority is called with a mocked image and a new priority
+        queue.modify_priority(mocked_image, Priority.High)
+
+        # THEN: The remove() method should have been called, image priority updated and put() called
+        mocked_remove.assert_called_once_with(mocked_image)
+        assert mocked_image.priority == Priority.High, 'The priority should have been Priority.High'
+        mocked_put.assert_called_once_with((Priority.High, Priority.Low, mocked_image))
+
+    def test_remove(self):
+        """
+        Test the remove() method of PriorityQueue
+        """
+        # GIVEN: A PriorityQueue instance with a mocked image and queue
+        mocked_image = MagicMock()
+        mocked_image.priority = Priority.High
+        mocked_image.secondary_priority = Priority.Normal
+        queue = PriorityQueue()
+
+        # WHEN: An image is removed
+        with patch.object(queue, 'queue') as mocked_queue:
+            mocked_queue.__contains__.return_value = True
+            queue.remove(mocked_image)
+
+        # THEN: The mocked queue.remove() method should have been called
+        mocked_queue.remove.assert_called_once_with((Priority.High, Priority.Normal, mocked_image))
+
+
+@skip('Probably not going to use ImageManager in WebEngine/Reveal.js')
 class TestImageManager(TestCase, TestMixin):
 
     def setUp(self):
@@ -57,10 +150,10 @@
         Delete all the C++ objects at the end so that we don't have a segfault
         """
         self.image_manager.stop_manager = True
-        self.image_manager.image_thread.wait()
         del self.app
 
-    def test_basic_image_manager(self):
+    @patch('openlp.core.lib.imagemanager.run_thread')
+    def test_basic_image_manager(self, mocked_run_thread):
         """
         Test the Image Manager setup basic functionality
         """
@@ -86,7 +179,8 @@
             self.image_manager.get_image(TEST_PATH, 'church1.jpg')
         assert context.exception is not '', 'KeyError exception should have been thrown for missing image'
 
-    def test_different_dimension_image(self):
+    @patch('openlp.core.lib.imagemanager.run_thread')
+    def test_different_dimension_image(self, mocked_run_thread):
         """
         Test the Image Manager with dimensions
         """
@@ -118,57 +212,58 @@
             self.image_manager.get_image(full_path, 'church.jpg', 120, 120)
         assert context.exception is not '', 'KeyError exception should have been thrown for missing dimension'
 
-    def test_process_cache(self):
+    @patch('openlp.core.lib.imagemanager.resize_image')
+    @patch('openlp.core.lib.imagemanager.image_to_byte')
+    @patch('openlp.core.lib.imagemanager.run_thread')
+    def test_process_cache(self, mocked_run_thread, mocked_image_to_byte, mocked_resize_image):
         """
         Test the process_cache method
         """
-        with patch('openlp.core.lib.imagemanager.resize_image') as mocked_resize_image, \
-                patch('openlp.core.lib.imagemanager.image_to_byte') as mocked_image_to_byte:
-            # GIVEN: Mocked functions
-            mocked_resize_image.side_effect = self.mocked_resize_image
-            mocked_image_to_byte.side_effect = self.mocked_image_to_byte
-            image1 = 'church.jpg'
-            image2 = 'church2.jpg'
-            image3 = 'church3.jpg'
-            image4 = 'church4.jpg'
-
-            # WHEN: Add the images. Then get the lock (=queue can not be processed).
-            self.lock.acquire()
-            self.image_manager.add_image(TEST_PATH, image1, None)
-            self.image_manager.add_image(TEST_PATH, image2, None)
-
-            # THEN: All images have been added to the queue, and only the first image is not be in the list anymore, but
-            #  is being processed (see mocked methods/functions).
-            # Note: Priority.Normal means, that the resize_image() was not completed yet (because afterwards the #
-            # priority is adjusted to Priority.Lowest).
-            assert self.get_image_priority(image1) == Priority.Normal, "image1's priority should be 'Priority.Normal'"
-            assert self.get_image_priority(image2) == Priority.Normal, "image2's priority should be 'Priority.Normal'"
-
-            # WHEN: Add more images.
-            self.image_manager.add_image(TEST_PATH, image3, None)
-            self.image_manager.add_image(TEST_PATH, image4, None)
-            # Allow the queue to process.
-            self.lock.release()
-            # Request some "data".
-            self.image_manager.get_image_bytes(TEST_PATH, image4)
-            self.image_manager.get_image(TEST_PATH, image3)
-            # Now the mocked methods/functions do not have to sleep anymore.
-            self.sleep_time = 0
-            # Wait for the queue to finish.
-            while not self.image_manager._conversion_queue.empty():
-                time.sleep(0.1)
-            # Because empty() is not reliable, wait a litte; just to make sure.
+        # GIVEN: Mocked functions
+        mocked_resize_image.side_effect = self.mocked_resize_image
+        mocked_image_to_byte.side_effect = self.mocked_image_to_byte
+        image1 = 'church.jpg'
+        image2 = 'church2.jpg'
+        image3 = 'church3.jpg'
+        image4 = 'church4.jpg'
+
+        # WHEN: Add the images. Then get the lock (=queue can not be processed).
+        self.lock.acquire()
+        self.image_manager.add_image(TEST_PATH, image1, None)
+        self.image_manager.add_image(TEST_PATH, image2, None)
+
+        # THEN: All images have been added to the queue, and only the first image is not be in the list anymore, but
+        #  is being processed (see mocked methods/functions).
+        # Note: Priority.Normal means, that the resize_image() was not completed yet (because afterwards the #
+        # priority is adjusted to Priority.Lowest).
+        assert self.get_image_priority(image1) == Priority.Normal, "image1's priority should be 'Priority.Normal'"
+        assert self.get_image_priority(image2) == Priority.Normal, "image2's priority should be 'Priority.Normal'"
+
+        # WHEN: Add more images.
+        self.image_manager.add_image(TEST_PATH, image3, None)
+        self.image_manager.add_image(TEST_PATH, image4, None)
+        # Allow the queue to process.
+        self.lock.release()
+        # Request some "data".
+        self.image_manager.get_image_bytes(TEST_PATH, image4)
+        self.image_manager.get_image(TEST_PATH, image3)
+        # Now the mocked methods/functions do not have to sleep anymore.
+        self.sleep_time = 0
+        # Wait for the queue to finish.
+        while not self.image_manager._conversion_queue.empty():
             time.sleep(0.1)
-            # THEN: The images' priority reflect how they were processed.
-            assert self.image_manager._conversion_queue.qsize() == 0, "The queue should be empty."
-            assert self.get_image_priority(image1) == Priority.Lowest, \
-                "The image should have not been requested (=Lowest)"
-            assert self.get_image_priority(image2) == Priority.Lowest, \
-                "The image should have not been requested (=Lowest)"
-            assert self.get_image_priority(image3) == Priority.Low, \
-                "Only the QImage should have been requested (=Low)."
-            assert self.get_image_priority(image4) == Priority.Urgent, \
-                "The image bytes should have been requested (=Urgent)."
+        # Because empty() is not reliable, wait a litte; just to make sure.
+        time.sleep(0.1)
+        # THEN: The images' priority reflect how they were processed.
+        assert self.image_manager._conversion_queue.qsize() == 0, "The queue should be empty."
+        assert self.get_image_priority(image1) == Priority.Lowest, \
+            "The image should have not been requested (=Lowest)"
+        assert self.get_image_priority(image2) == Priority.Lowest, \
+            "The image should have not been requested (=Lowest)"
+        assert self.get_image_priority(image3) == Priority.Low, \
+            "Only the QImage should have been requested (=Low)."
+        assert self.get_image_priority(image4) == Priority.Urgent, \
+            "The image bytes should have been requested (=Urgent)."
 
     def get_image_priority(self, image):
         """

=== modified file 'tests/functional/openlp_core/test_app.py'
--- tests/functional/openlp_core/test_app.py	2017-12-29 09:15:48 +0000
+++ tests/functional/openlp_core/test_app.py	2018-01-07 18:07:40 +0000
@@ -36,14 +36,15 @@
     """
     # GIVEN: a a set of system arguments.
     sys.argv[1:] = []
+
     # WHEN: We we parse them to expand to options
-    args = parse_options(None)
+    args = parse_options()
+
     # THEN: the following fields will have been extracted.
     assert args.dev_version is False, 'The dev_version flag should be False'
     assert args.loglevel == 'warning', 'The log level should be set to warning'
     assert args.no_error_form is False, 'The no_error_form should be set to False'
     assert args.portable is False, 'The portable flag should be set to false'
-    assert args.style is None, 'There are no style flags to be processed'
     assert args.rargs == [], 'The service file should be blank'
 
 
@@ -53,14 +54,15 @@
     """
     # GIVEN: a a set of system arguments.
     sys.argv[1:] = ['-l debug']
+
     # WHEN: We we parse them to expand to options
-    args = parse_options(None)
+    args = parse_options()
+
     # THEN: the following fields will have been extracted.
     assert args.dev_version is False, 'The dev_version flag should be False'
     assert args.loglevel == ' debug', 'The log level should be set to debug'
     assert args.no_error_form is False, 'The no_error_form should be set to False'
     assert args.portable is False, 'The portable flag should be set to false'
-    assert args.style is None, 'There are no style flags to be processed'
     assert args.rargs == [], 'The service file should be blank'
 
 
@@ -70,14 +72,15 @@
     """
     # GIVEN: a a set of system arguments.
     sys.argv[1:] = ['--portable']
+
     # WHEN: We we parse them to expand to options
-    args = parse_options(None)
+    args = parse_options()
+
     # THEN: the following fields will have been extracted.
     assert args.dev_version is False, 'The dev_version flag should be False'
     assert args.loglevel == 'warning', 'The log level should be set to warning'
     assert args.no_error_form is False, 'The no_error_form should be set to False'
     assert args.portable is True, 'The portable flag should be set to true'
-    assert args.style is None, 'There are no style flags to be processed'
     assert args.rargs == [], 'The service file should be blank'
 
 
@@ -87,14 +90,15 @@
     """
     # GIVEN: a a set of system arguments.
     sys.argv[1:] = ['-l debug', '-d']
+
     # WHEN: We we parse them to expand to options
-    args = parse_options(None)
+    args = parse_options()
+
     # THEN: the following fields will have been extracted.
     assert args.dev_version is True, 'The dev_version flag should be True'
     assert args.loglevel == ' debug', 'The log level should be set to debug'
     assert args.no_error_form is False, 'The no_error_form should be set to False'
     assert args.portable is False, 'The portable flag should be set to false'
-    assert args.style is None, 'There are no style flags to be processed'
     assert args.rargs == [], 'The service file should be blank'
 
 
@@ -104,14 +108,15 @@
     """
     # GIVEN: a a set of system arguments.
     sys.argv[1:] = ['dummy_temp']
+
     # WHEN: We we parse them to expand to options
-    args = parse_options(None)
+    args = parse_options()
+
     # THEN: the following fields will have been extracted.
     assert args.dev_version is False, 'The dev_version flag should be False'
     assert args.loglevel == 'warning', 'The log level should be set to warning'
     assert args.no_error_form is False, 'The no_error_form should be set to False'
     assert args.portable is False, 'The portable flag should be set to false'
-    assert args.style is None, 'There are no style flags to be processed'
     assert args.rargs == 'dummy_temp', 'The service file should not be blank'
 
 
@@ -121,14 +126,15 @@
     """
     # GIVEN: a a set of system arguments.
     sys.argv[1:] = ['-l debug', 'dummy_temp']
+
     # WHEN: We we parse them to expand to options
-    args = parse_options(None)
+    args = parse_options()
+
     # THEN: the following fields will have been extracted.
     assert args.dev_version is False, 'The dev_version flag should be False'
     assert args.loglevel == ' debug', 'The log level should be set to debug'
     assert args.no_error_form is False, 'The no_error_form should be set to False'
     assert args.portable is False, 'The portable flag should be set to false'
-    assert args.style is None, 'There are no style flags to be processed'
     assert args.rargs == 'dummy_temp', 'The service file should not be blank'
 
 

=== added file 'tests/functional/openlp_core/test_threading.py'
--- tests/functional/openlp_core/test_threading.py	1970-01-01 00:00:00 +0000
+++ tests/functional/openlp_core/test_threading.py	2018-01-07 18:07:40 +0000
@@ -0,0 +1,89 @@
+# -*- coding: utf-8 -*-
+# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4
+
+###############################################################################
+# OpenLP - Open Source Lyrics Projection                                      #
+# --------------------------------------------------------------------------- #
+# Copyright (c) 2008-2018 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.threading package.
+"""
+from unittest.mock import MagicMock, call, patch
+
+from openlp.core.version import run_thread
+
+
+def test_run_thread_no_name():
+    """
+    Test that trying to run a thread without a name results in an exception being thrown
+    """
+    # GIVEN: A fake worker
+    # WHEN: run_thread() is called without a name
+    try:
+        run_thread(MagicMock(), '')
+        assert False, 'A ValueError should have been thrown to prevent blank names'
+    except ValueError:
+        # THEN: A ValueError should have been thrown
+        assert True, 'A ValueError was correctly thrown'
+
+
+@patch('openlp.core.threading.Registry')
+def test_run_thread_exists(MockRegistry):
+    """
+    Test that trying to run a thread with a name that already exists will throw a KeyError
+    """
+    # GIVEN: A mocked registry with a main window object
+    mocked_main_window = MagicMock()
+    mocked_main_window.threads = {'test_thread': MagicMock()}
+    MockRegistry.return_value.get.return_value = mocked_main_window
+
+    # WHEN: run_thread() is called
+    try:
+        run_thread(MagicMock(), 'test_thread')
+        assert False, 'A KeyError should have been thrown to show that a thread with this name already exists'
+    except KeyError:
+        assert True, 'A KeyError was correctly thrown'
+
+
+@patch('openlp.core.threading.QtCore.QThread')
+@patch('openlp.core.threading.Registry')
+def test_run_thread(MockRegistry, MockQThread):
+    """
+    Test that running a thread works correctly
+    """
+    # GIVEN: A mocked registry with a main window object
+    mocked_main_window = MagicMock()
+    mocked_main_window.threads = {}
+    MockRegistry.return_value.get.return_value = mocked_main_window
+
+    # WHEN: run_thread() is called
+    run_thread(MagicMock(), 'test_thread')
+
+    # THEN: The thread should be in the threads list and the correct methods should have been called
+    assert len(mocked_main_window.threads.keys()) == 1, 'There should be 1 item in the list of threads'
+    assert list(mocked_main_window.threads.keys()) == ['test_thread'], 'The test_thread item should be in the list'
+    mocked_worker = mocked_main_window.threads['test_thread']['worker']
+    mocked_thread = mocked_main_window.threads['test_thread']['thread']
+    mocked_worker.moveToThread.assert_called_once_with(mocked_thread)
+    mocked_thread.started.connect.assert_called_once_with(mocked_worker.start)
+    expected_quit_calls = [call(mocked_thread.quit), call(mocked_worker.deleteLater)]
+    assert mocked_worker.quit.connect.call_args_list == expected_quit_calls, \
+        'The workers quit signal should be connected twice'
+    assert mocked_thread.finished.connect.call_args_list[0] == call(mocked_thread.deleteLater), \
+        'The threads finished signal should be connected to its deleteLater slot'
+    assert mocked_thread.finished.connect.call_count == 2, 'The signal should have been connected twice'
+    mocked_thread.start.assert_called_once_with()

=== added file 'tests/functional/openlp_core/ui/media/test_systemplayer.py'
--- tests/functional/openlp_core/ui/media/test_systemplayer.py	1970-01-01 00:00:00 +0000
+++ tests/functional/openlp_core/ui/media/test_systemplayer.py	2018-01-07 18:07:40 +0000
@@ -0,0 +1,543 @@
+# -*- coding: utf-8 -*-
+# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4
+
+###############################################################################
+# OpenLP - Open Source Lyrics Projection                                      #
+# --------------------------------------------------------------------------- #
+# Copyright (c) 2008-2018 OpenLP Developers                                   #
+# --------------------------------------------------------------------------- #
+# This program is free software; you can redistribute it and/or modify it     #
+# under the terms of the GNU General Public License as published by the Free  #
+# Software Foundation; version 2 of the License.                              #
+#                                                                             #
+# This program is distributed in the hope that it will be useful, but WITHOUT #
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or       #
+# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for    #
+# more details.                                                               #
+#                                                                             #
+# You should have received a copy of the GNU General Public License along     #
+# with this program; if not, write to the Free Software Foundation, Inc., 59  #
+# Temple Place, Suite 330, Boston, MA 02111-1307 USA                          #
+###############################################################################
+"""
+Package to test the openlp.core.ui.media.systemplayer package.
+"""
+from unittest import TestCase
+from unittest.mock import MagicMock, call, patch
+
+from PyQt5 import QtCore, QtMultimedia
+
+from openlp.core.common.registry import Registry
+from openlp.core.ui.media import MediaState
+from openlp.core.ui.media.systemplayer import SystemPlayer, CheckMediaWorker, ADDITIONAL_EXT
+
+
+class TestSystemPlayer(TestCase):
+    """
+    Test the system media player
+    """
+    @patch('openlp.core.ui.media.systemplayer.mimetypes')
+    @patch('openlp.core.ui.media.systemplayer.QtMultimedia.QMediaPlayer')
+    def test_constructor(self, MockQMediaPlayer, mocked_mimetypes):
+        """
+        Test the SystemPlayer constructor
+        """
+        # GIVEN: The SystemPlayer class and a mockedQMediaPlayer
+        mocked_media_player = MagicMock()
+        mocked_media_player.supportedMimeTypes.return_value = [
+            'application/postscript',
+            'audio/aiff',
+            'audio/x-aiff',
+            'text/html',
+            'video/animaflex',
+            'video/x-ms-asf'
+        ]
+        mocked_mimetypes.guess_all_extensions.side_effect = [
+            ['.aiff'],
+            ['.aiff'],
+            ['.afl'],
+            ['.asf']
+        ]
+        MockQMediaPlayer.return_value = mocked_media_player
+
+        # WHEN: An object is created from it
+        player = SystemPlayer(self)
+
+        # THEN: The correct initial values should be set up
+        assert 'system' == player.name
+        assert 'System' == player.original_name
+        assert '&System' == player.display_name
+        assert self == player.parent
+        assert ADDITIONAL_EXT == player.additional_extensions
+        MockQMediaPlayer.assert_called_once_with(None, QtMultimedia.QMediaPlayer.VideoSurface)
+        mocked_mimetypes.init.assert_called_once_with()
+        mocked_media_player.service.assert_called_once_with()
+        mocked_media_player.supportedMimeTypes.assert_called_once_with()
+        assert ['*.aiff'] == player.audio_extensions_list
+        assert ['*.afl', '*.asf'] == player.video_extensions_list
+
+    @patch('openlp.core.ui.media.systemplayer.QtMultimediaWidgets.QVideoWidget')
+    @patch('openlp.core.ui.media.systemplayer.QtMultimedia.QMediaPlayer')
+    def test_setup(self, MockQMediaPlayer, MockQVideoWidget):
+        """
+        Test the setup() method of SystemPlayer
+        """
+        # GIVEN: A SystemPlayer instance and a mock display
+        player = SystemPlayer(self)
+        mocked_display = MagicMock()
+        mocked_display.size.return_value = [1, 2, 3, 4]
+        mocked_video_widget = MagicMock()
+        mocked_media_player = MagicMock()
+        MockQVideoWidget.return_value = mocked_video_widget
+        MockQMediaPlayer.return_value = mocked_media_player
+
+        # WHEN: setup() is run
+        player.setup(mocked_display)
+
+        # THEN: The player should have a display widget
+        MockQVideoWidget.assert_called_once_with(mocked_display)
+        assert mocked_video_widget == mocked_display.video_widget
+        mocked_display.size.assert_called_once_with()
+        mocked_video_widget.resize.assert_called_once_with([1, 2, 3, 4])
+        MockQMediaPlayer.assert_called_with(mocked_display)
+        assert mocked_media_player == mocked_display.media_player
+        mocked_media_player.setVideoOutput.assert_called_once_with(mocked_video_widget)
+        mocked_video_widget.raise_.assert_called_once_with()
+        mocked_video_widget.hide.assert_called_once_with()
+        assert player.has_own_widget is True
+
+    def test_disconnect_slots(self):
+        """
+        Test that we the disconnect slots method catches the TypeError
+        """
+        # GIVEN: A SystemPlayer class and a signal that throws a TypeError
+        player = SystemPlayer(self)
+        mocked_signal = MagicMock()
+        mocked_signal.disconnect.side_effect = \
+            TypeError('disconnect() failed between \'durationChanged\' and all its connections')
+
+        # WHEN: disconnect_slots() is called
+        player.disconnect_slots(mocked_signal)
+
+        # THEN: disconnect should have been called and the exception should have been ignored
+        mocked_signal.disconnect.assert_called_once_with()
+
+    def test_check_available(self):
+        """
+        Test the check_available() method on SystemPlayer
+        """
+        # GIVEN: A SystemPlayer instance
+        player = SystemPlayer(self)
+
+        # WHEN: check_available is run
+        result = player.check_available()
+
+        # THEN: it should be available
+        assert result is True
+
+    def test_load_valid_media(self):
+        """
+        Test the load() method of SystemPlayer with a valid media file
+        """
+        # GIVEN: A SystemPlayer instance and a mocked display
+        player = SystemPlayer(self)
+        mocked_display = MagicMock()
+        mocked_display.controller.media_info.volume = 1
+        mocked_display.controller.media_info.file_info.absoluteFilePath.return_value = '/path/to/file'
+
+        # WHEN: The load() method is run
+        with patch.object(player, 'check_media') as mocked_check_media, \
+                patch.object(player, 'volume') as mocked_volume:
+            mocked_check_media.return_value = True
+            result = player.load(mocked_display)
+
+        # THEN: the file is sent to the video widget
+        mocked_display.controller.media_info.file_info.absoluteFilePath.assert_called_once_with()
+        mocked_check_media.assert_called_once_with('/path/to/file')
+        mocked_display.media_player.setMedia.assert_called_once_with(
+            QtMultimedia.QMediaContent(QtCore.QUrl.fromLocalFile('/path/to/file')))
+        mocked_volume.assert_called_once_with(mocked_display, 1)
+        assert result is True
+
+    def test_load_invalid_media(self):
+        """
+        Test the load() method of SystemPlayer with an invalid media file
+        """
+        # GIVEN: A SystemPlayer instance and a mocked display
+        player = SystemPlayer(self)
+        mocked_display = MagicMock()
+        mocked_display.controller.media_info.volume = 1
+        mocked_display.controller.media_info.file_info.absoluteFilePath.return_value = '/path/to/file'
+
+        # WHEN: The load() method is run
+        with patch.object(player, 'check_media') as mocked_check_media, \
+                patch.object(player, 'volume'):
+            mocked_check_media.return_value = False
+            result = player.load(mocked_display)
+
+        # THEN: stuff
+        mocked_display.controller.media_info.file_info.absoluteFilePath.assert_called_once_with()
+        mocked_check_media.assert_called_once_with('/path/to/file')
+        assert result is False
+
+    def test_resize(self):
+        """
+        Test the resize() method of the SystemPlayer
+        """
+        # GIVEN: A SystemPlayer instance and a mocked display
+        player = SystemPlayer(self)
+        mocked_display = MagicMock()
+        mocked_display.size.return_value = [1, 2, 3, 4]
+
+        # WHEN: The resize() method is called
+        player.resize(mocked_display)
+
+        # THEN: The player is resized
+        mocked_display.size.assert_called_once_with()
+        mocked_display.video_widget.resize.assert_called_once_with([1, 2, 3, 4])
+
+    @patch('openlp.core.ui.media.systemplayer.functools')
+    def test_play_is_live(self, mocked_functools):
+        """
+        Test the play() method of the SystemPlayer on the live display
+        """
+        # GIVEN: A SystemPlayer instance and a mocked display
+        mocked_functools.partial.return_value = 'function'
+        player = SystemPlayer(self)
+        mocked_display = MagicMock()
+        mocked_display.controller.is_live = True
+        mocked_display.controller.media_info.start_time = 1
+        mocked_display.controller.media_info.volume = 1
+
+        # WHEN: play() is called
+        with patch.object(player, 'get_live_state') as mocked_get_live_state, \
+                patch.object(player, 'seek') as mocked_seek, \
+                patch.object(player, 'volume') as mocked_volume, \
+                patch.object(player, 'set_state') as mocked_set_state, \
+                patch.object(player, 'disconnect_slots') as mocked_disconnect_slots:
+            mocked_get_live_state.return_value = QtMultimedia.QMediaPlayer.PlayingState
+            result = player.play(mocked_display)
+
+        # THEN: the media file is played
+        mocked_get_live_state.assert_called_once_with()
+        mocked_display.media_player.play.assert_called_once_with()
+        mocked_seek.assert_called_once_with(mocked_display, 1000)
+        mocked_volume.assert_called_once_with(mocked_display, 1)
+        mocked_disconnect_slots.assert_called_once_with(mocked_display.media_player.durationChanged)
+        mocked_display.media_player.durationChanged.connect.assert_called_once_with('function')
+        mocked_set_state.assert_called_once_with(MediaState.Playing, mocked_display)
+        mocked_display.video_widget.raise_.assert_called_once_with()
+        assert result is True
+
+    @patch('openlp.core.ui.media.systemplayer.functools')
+    def test_play_is_preview(self, mocked_functools):
+        """
+        Test the play() method of the SystemPlayer on the preview display
+        """
+        # GIVEN: A SystemPlayer instance and a mocked display
+        mocked_functools.partial.return_value = 'function'
+        player = SystemPlayer(self)
+        mocked_display = MagicMock()
+        mocked_display.controller.is_live = False
+        mocked_display.controller.media_info.start_time = 1
+        mocked_display.controller.media_info.volume = 1
+
+        # WHEN: play() is called
+        with patch.object(player, 'get_preview_state') as mocked_get_preview_state, \
+                patch.object(player, 'seek') as mocked_seek, \
+                patch.object(player, 'volume') as mocked_volume, \
+                patch.object(player, 'set_state') as mocked_set_state:
+            mocked_get_preview_state.return_value = QtMultimedia.QMediaPlayer.PlayingState
+            result = player.play(mocked_display)
+
+        # THEN: the media file is played
+        mocked_get_preview_state.assert_called_once_with()
+        mocked_display.media_player.play.assert_called_once_with()
+        mocked_seek.assert_called_once_with(mocked_display, 1000)
+        mocked_volume.assert_called_once_with(mocked_display, 1)
+        mocked_display.media_player.durationChanged.connect.assert_called_once_with('function')
+        mocked_set_state.assert_called_once_with(MediaState.Playing, mocked_display)
+        mocked_display.video_widget.raise_.assert_called_once_with()
+        assert result is True
+
+    def test_pause_is_live(self):
+        """
+        Test the pause() method of the SystemPlayer on the live display
+        """
+        # GIVEN: A SystemPlayer instance
+        player = SystemPlayer(self)
+        mocked_display = MagicMock()
+        mocked_display.controller.is_live = True
+
+        # WHEN: The pause method is called
+        with patch.object(player, 'get_live_state') as mocked_get_live_state, \
+                patch.object(player, 'set_state') as mocked_set_state:
+            mocked_get_live_state.return_value = QtMultimedia.QMediaPlayer.PausedState
+            player.pause(mocked_display)
+
+        # THEN: The video is paused
+        mocked_display.media_player.pause.assert_called_once_with()
+        mocked_get_live_state.assert_called_once_with()
+        mocked_set_state.assert_called_once_with(MediaState.Paused, mocked_display)
+
+    def test_pause_is_preview(self):
+        """
+        Test the pause() method of the SystemPlayer on the preview display
+        """
+        # GIVEN: A SystemPlayer instance
+        player = SystemPlayer(self)
+        mocked_display = MagicMock()
+        mocked_display.controller.is_live = False
+
+        # WHEN: The pause method is called
+        with patch.object(player, 'get_preview_state') as mocked_get_preview_state, \
+                patch.object(player, 'set_state') as mocked_set_state:
+            mocked_get_preview_state.return_value = QtMultimedia.QMediaPlayer.PausedState
+            player.pause(mocked_display)
+
+        # THEN: The video is paused
+        mocked_display.media_player.pause.assert_called_once_with()
+        mocked_get_preview_state.assert_called_once_with()
+        mocked_set_state.assert_called_once_with(MediaState.Paused, mocked_display)
+
+    def test_stop(self):
+        """
+        Test the stop() method of the SystemPlayer
+        """
+        # GIVEN: A SystemPlayer instance
+        player = SystemPlayer(self)
+        mocked_display = MagicMock()
+
+        # WHEN: The stop method is called
+        with patch.object(player, 'set_visible') as mocked_set_visible, \
+                patch.object(player, 'set_state') as mocked_set_state:
+            player.stop(mocked_display)
+
+        # THEN: The video is stopped
+        mocked_display.media_player.stop.assert_called_once_with()
+        mocked_set_visible.assert_called_once_with(mocked_display, False)
+        mocked_set_state.assert_called_once_with(MediaState.Stopped, mocked_display)
+
+    def test_volume(self):
+        """
+        Test the volume() method of the SystemPlayer
+        """
+        # GIVEN: A SystemPlayer instance
+        player = SystemPlayer(self)
+        mocked_display = MagicMock()
+        mocked_display.has_audio = True
+
+        # WHEN: The stop method is called
+        player.volume(mocked_display, 2)
+
+        # THEN: The video is stopped
+        mocked_display.media_player.setVolume.assert_called_once_with(2)
+
+    def test_seek(self):
+        """
+        Test the seek() method of the SystemPlayer
+        """
+        # GIVEN: A SystemPlayer instance
+        player = SystemPlayer(self)
+        mocked_display = MagicMock()
+
+        # WHEN: The stop method is called
+        player.seek(mocked_display, 2)
+
+        # THEN: The video is stopped
+        mocked_display.media_player.setPosition.assert_called_once_with(2)
+
+    def test_reset(self):
+        """
+        Test the reset() method of the SystemPlayer
+        """
+        # GIVEN: A SystemPlayer instance
+        player = SystemPlayer(self)
+        mocked_display = MagicMock()
+
+        # WHEN: reset() is called
+        with patch.object(player, 'set_state') as mocked_set_state, \
+                patch.object(player, 'set_visible') as mocked_set_visible:
+            player.reset(mocked_display)
+
+        # THEN: The media player is reset
+        mocked_display.media_player.stop()
+        mocked_display.media_player.setMedia.assert_called_once_with(QtMultimedia.QMediaContent())
+        mocked_set_visible.assert_called_once_with(mocked_display, False)
+        mocked_display.video_widget.setVisible.assert_called_once_with(False)
+        mocked_set_state.assert_called_once_with(MediaState.Off, mocked_display)
+
+    def test_set_visible(self):
+        """
+        Test the set_visible() method on the SystemPlayer
+        """
+        # GIVEN: A SystemPlayer instance and a mocked display
+        player = SystemPlayer(self)
+        player.has_own_widget = True
+        mocked_display = MagicMock()
+
+        # WHEN: set_visible() is called
+        player.set_visible(mocked_display, True)
+
+        # THEN: The widget should be visible
+        mocked_display.video_widget.setVisible.assert_called_once_with(True)
+
+    def test_set_duration(self):
+        """
+        Test the set_duration() method of the SystemPlayer
+        """
+        # GIVEN: a mocked controller
+        mocked_controller = MagicMock()
+        mocked_controller.media_info.length = 5
+
+        # WHEN: The set_duration() is called. NB: the 10 here is ignored by the code
+        SystemPlayer.set_duration(mocked_controller, 10)
+
+        # THEN: The maximum length of the slider should be set
+        mocked_controller.seek_slider.setMaximum.assert_called_once_with(5)
+
+    def test_update_ui(self):
+        """
+        Test the update_ui() method on the SystemPlayer
+        """
+        # GIVEN: A SystemPlayer instance
+        player = SystemPlayer(self)
+        player.state = [MediaState.Playing, MediaState.Playing]
+        mocked_display = MagicMock()
+        mocked_display.media_player.state.return_value = QtMultimedia.QMediaPlayer.PausedState
+        mocked_display.controller.media_info.end_time = 1
+        mocked_display.media_player.position.return_value = 2
+        mocked_display.controller.seek_slider.isSliderDown.return_value = False
+
+        # WHEN: update_ui() is called
+        with patch.object(player, 'stop') as mocked_stop, \
+                patch.object(player, 'set_visible') as mocked_set_visible:
+            player.update_ui(mocked_display)
+
+        # THEN: The UI is updated
+        expected_stop_calls = [call(mocked_display)]
+        expected_position_calls = [call(), call()]
+        expected_block_signals_calls = [call(True), call(False)]
+        mocked_display.media_player.state.assert_called_once_with()
+        assert 1 == mocked_stop.call_count
+        assert expected_stop_calls == mocked_stop.call_args_list
+        assert 2 == mocked_display.media_player.position.call_count
+        assert expected_position_calls == mocked_display.media_player.position.call_args_list
+        mocked_set_visible.assert_called_once_with(mocked_display, False)
+        mocked_display.controller.seek_slider.isSliderDown.assert_called_once_with()
+        assert expected_block_signals_calls == mocked_display.controller.seek_slider.blockSignals.call_args_list
+        mocked_display.controller.seek_slider.setSliderPosition.assert_called_once_with(2)
+
+    def test_get_media_display_css(self):
+        """
+        Test the get_media_display_css() method of the SystemPlayer
+        """
+        # GIVEN: A SystemPlayer instance
+        player = SystemPlayer(self)
+
+        # WHEN: get_media_display_css() is called
+        result = player.get_media_display_css()
+
+        # THEN: The css should be empty
+        assert '' == result
+
+    @patch('openlp.core.ui.media.systemplayer.QtMultimedia.QMediaPlayer')
+    def test_get_info(self, MockQMediaPlayer):
+        """
+        Test the get_info() method of the SystemPlayer
+        """
+        # GIVEN: A SystemPlayer instance
+        mocked_media_player = MagicMock()
+        mocked_media_player.supportedMimeTypes.return_value = []
+        MockQMediaPlayer.return_value = mocked_media_player
+        player = SystemPlayer(self)
+
+        # WHEN: get_info() is called
+        result = player.get_info()
+
+        # THEN: The info should be correct
+        expected_info = 'This media player uses your operating system to provide media capabilities.<br/> ' \
+            '<strong>Audio</strong><br/>[]<br/><strong>Video</strong><br/>[]<br/>'
+        assert expected_info == result
+
+    @patch('openlp.core.ui.media.systemplayer.CheckMediaWorker')
+    @patch('openlp.core.ui.media.systemplayer.run_thread')
+    @patch('openlp.core.ui.media.systemplayer.is_thread_finished')
+    def test_check_media(self, mocked_is_thread_finished, mocked_run_thread, MockCheckMediaWorker):
+        """
+        Test the check_media() method of the SystemPlayer
+        """
+        # GIVEN: A SystemPlayer instance and a mocked thread
+        valid_file = '/path/to/video.ogv'
+        mocked_application = MagicMock()
+        Registry().create()
+        Registry().register('application', mocked_application)
+        player = SystemPlayer(self)
+        mocked_is_thread_finished.side_effect = [False, True]
+        mocked_check_media_worker = MagicMock()
+        mocked_check_media_worker.result = True
+        MockCheckMediaWorker.return_value = mocked_check_media_worker
+
+        # WHEN: check_media() is called with a valid media file
+        result = player.check_media(valid_file)
+
+        # THEN: It should return True
+        MockCheckMediaWorker.assert_called_once_with(valid_file)
+        mocked_check_media_worker.setVolume.assert_called_once_with(0)
+        mocked_run_thread.assert_called_once_with(mocked_check_media_worker, 'check_media')
+        mocked_is_thread_finished.assert_called_with('check_media')
+        assert mocked_is_thread_finished.call_count == 2, 'is_thread_finished() should have been called twice'
+        mocked_application.processEvents.assert_called_once_with()
+        assert result is True
+
+
+class TestCheckMediaWorker(TestCase):
+    """
+    Test the CheckMediaWorker class
+    """
+    def test_constructor(self):
+        """
+        Test the constructor of the CheckMediaWorker class
+        """
+        # GIVEN: A file path
+        path = 'file.ogv'
+
+        # WHEN: The CheckMediaWorker object is instantiated
+        worker = CheckMediaWorker(path)
+
+        # THEN: The correct values should be set up
+        assert worker is not None
+
+    def test_signals_media(self):
+        """
+        Test the signals() signal of the CheckMediaWorker class with a "media" origin
+        """
+        # GIVEN: A CheckMediaWorker instance
+        worker = CheckMediaWorker('file.ogv')
+
+        # WHEN: signals() is called with media and BufferedMedia
+        with patch.object(worker, 'stop') as mocked_stop, \
+                patch.object(worker, 'quit') as mocked_quit:
+            worker.signals('media', worker.BufferedMedia)
+
+        # THEN: The worker should exit and the result should be True
+        mocked_stop.assert_called_once_with()
+        mocked_quit.emit.assert_called_once_with()
+        assert worker.result is True
+
+    def test_signals_error(self):
+        """
+        Test the signals() signal of the CheckMediaWorker class with a "error" origin
+        """
+        # GIVEN: A CheckMediaWorker instance
+        worker = CheckMediaWorker('file.ogv')
+
+        # WHEN: signals() is called with error and BufferedMedia
+        with patch.object(worker, 'stop') as mocked_stop, \
+                patch.object(worker, 'quit') as mocked_quit:
+            worker.signals('error', None)
+
+        # THEN: The worker should exit and the result should be True
+        mocked_stop.assert_called_once_with()
+        mocked_quit.emit.assert_called_once_with()
+        assert worker.result is False

=== removed file 'tests/functional/openlp_core/ui/media/test_systemplayer.py'
--- tests/functional/openlp_core/ui/media/test_systemplayer.py	2017-12-29 09:15:48 +0000
+++ tests/functional/openlp_core/ui/media/test_systemplayer.py	1970-01-01 00:00:00 +0000
@@ -1,549 +0,0 @@
-# -*- coding: utf-8 -*-
-# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4
-
-###############################################################################
-# OpenLP - Open Source Lyrics Projection                                      #
-# --------------------------------------------------------------------------- #
-# Copyright (c) 2008-2018 OpenLP Developers                                   #
-# --------------------------------------------------------------------------- #
-# This program is free software; you can redistribute it and/or modify it     #
-# under the terms of the GNU General Public License as published by the Free  #
-# Software Foundation; version 2 of the License.                              #
-#                                                                             #
-# This program is distributed in the hope that it will be useful, but WITHOUT #
-# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or       #
-# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for    #
-# more details.                                                               #
-#                                                                             #
-# You should have received a copy of the GNU General Public License along     #
-# with this program; if not, write to the Free Software Foundation, Inc., 59  #
-# Temple Place, Suite 330, Boston, MA 02111-1307 USA                          #
-###############################################################################
-"""
-Package to test the openlp.core.ui.media.systemplayer package.
-"""
-from unittest import TestCase
-from unittest.mock import MagicMock, call, patch
-
-from PyQt5 import QtCore, QtMultimedia
-
-from openlp.core.common.registry import Registry
-from openlp.core.ui.media import MediaState
-from openlp.core.ui.media.systemplayer import SystemPlayer, CheckMediaWorker, ADDITIONAL_EXT
-
-
-class TestSystemPlayer(TestCase):
-    """
-    Test the system media player
-    """
-    @patch('openlp.core.ui.media.systemplayer.mimetypes')
-    @patch('openlp.core.ui.media.systemplayer.QtMultimedia.QMediaPlayer')
-    def test_constructor(self, MockQMediaPlayer, mocked_mimetypes):
-        """
-        Test the SystemPlayer constructor
-        """
-        # GIVEN: The SystemPlayer class and a mockedQMediaPlayer
-        mocked_media_player = MagicMock()
-        mocked_media_player.supportedMimeTypes.return_value = [
-            'application/postscript',
-            'audio/aiff',
-            'audio/x-aiff',
-            'text/html',
-            'video/animaflex',
-            'video/x-ms-asf'
-        ]
-        mocked_mimetypes.guess_all_extensions.side_effect = [
-            ['.aiff'],
-            ['.aiff'],
-            ['.afl'],
-            ['.asf']
-        ]
-        MockQMediaPlayer.return_value = mocked_media_player
-
-        # WHEN: An object is created from it
-        player = SystemPlayer(self)
-
-        # THEN: The correct initial values should be set up
-        assert 'system' == player.name
-        assert 'System' == player.original_name
-        assert '&System' == player.display_name
-        assert self == player.parent
-        assert ADDITIONAL_EXT == player.additional_extensions
-        MockQMediaPlayer.assert_called_once_with(None, QtMultimedia.QMediaPlayer.VideoSurface)
-        mocked_mimetypes.init.assert_called_once_with()
-        mocked_media_player.service.assert_called_once_with()
-        mocked_media_player.supportedMimeTypes.assert_called_once_with()
-        assert ['*.aiff'] == player.audio_extensions_list
-        assert ['*.afl', '*.asf'] == player.video_extensions_list
-
-    @patch('openlp.core.ui.media.systemplayer.QtMultimediaWidgets.QVideoWidget')
-    @patch('openlp.core.ui.media.systemplayer.QtMultimedia.QMediaPlayer')
-    def test_setup(self, MockQMediaPlayer, MockQVideoWidget):
-        """
-        Test the setup() method of SystemPlayer
-        """
-        # GIVEN: A SystemPlayer instance and a mock display
-        player = SystemPlayer(self)
-        mocked_display = MagicMock()
-        mocked_display.size.return_value = [1, 2, 3, 4]
-        mocked_video_widget = MagicMock()
-        mocked_media_player = MagicMock()
-        MockQVideoWidget.return_value = mocked_video_widget
-        MockQMediaPlayer.return_value = mocked_media_player
-
-        # WHEN: setup() is run
-        player.setup(mocked_display)
-
-        # THEN: The player should have a display widget
-        MockQVideoWidget.assert_called_once_with(mocked_display)
-        assert mocked_video_widget == mocked_display.video_widget
-        mocked_display.size.assert_called_once_with()
-        mocked_video_widget.resize.assert_called_once_with([1, 2, 3, 4])
-        MockQMediaPlayer.assert_called_with(mocked_display)
-        assert mocked_media_player == mocked_display.media_player
-        mocked_media_player.setVideoOutput.assert_called_once_with(mocked_video_widget)
-        mocked_video_widget.raise_.assert_called_once_with()
-        mocked_video_widget.hide.assert_called_once_with()
-        assert player.has_own_widget is True
-
-    def test_disconnect_slots(self):
-        """
-        Test that we the disconnect slots method catches the TypeError
-        """
-        # GIVEN: A SystemPlayer class and a signal that throws a TypeError
-        player = SystemPlayer(self)
-        mocked_signal = MagicMock()
-        mocked_signal.disconnect.side_effect = \
-            TypeError('disconnect() failed between \'durationChanged\' and all its connections')
-
-        # WHEN: disconnect_slots() is called
-        player.disconnect_slots(mocked_signal)
-
-        # THEN: disconnect should have been called and the exception should have been ignored
-        mocked_signal.disconnect.assert_called_once_with()
-
-    def test_check_available(self):
-        """
-        Test the check_available() method on SystemPlayer
-        """
-        # GIVEN: A SystemPlayer instance
-        player = SystemPlayer(self)
-
-        # WHEN: check_available is run
-        result = player.check_available()
-
-        # THEN: it should be available
-        assert result is True
-
-    def test_load_valid_media(self):
-        """
-        Test the load() method of SystemPlayer with a valid media file
-        """
-        # GIVEN: A SystemPlayer instance and a mocked display
-        player = SystemPlayer(self)
-        mocked_display = MagicMock()
-        mocked_display.controller.media_info.volume = 1
-        mocked_display.controller.media_info.file_info.absoluteFilePath.return_value = '/path/to/file'
-
-        # WHEN: The load() method is run
-        with patch.object(player, 'check_media') as mocked_check_media, \
-                patch.object(player, 'volume') as mocked_volume:
-            mocked_check_media.return_value = True
-            result = player.load(mocked_display)
-
-        # THEN: the file is sent to the video widget
-        mocked_display.controller.media_info.file_info.absoluteFilePath.assert_called_once_with()
-        mocked_check_media.assert_called_once_with('/path/to/file')
-        mocked_display.media_player.setMedia.assert_called_once_with(
-            QtMultimedia.QMediaContent(QtCore.QUrl.fromLocalFile('/path/to/file')))
-        mocked_volume.assert_called_once_with(mocked_display, 1)
-        assert result is True
-
-    def test_load_invalid_media(self):
-        """
-        Test the load() method of SystemPlayer with an invalid media file
-        """
-        # GIVEN: A SystemPlayer instance and a mocked display
-        player = SystemPlayer(self)
-        mocked_display = MagicMock()
-        mocked_display.controller.media_info.volume = 1
-        mocked_display.controller.media_info.file_info.absoluteFilePath.return_value = '/path/to/file'
-
-        # WHEN: The load() method is run
-        with patch.object(player, 'check_media') as mocked_check_media, \
-                patch.object(player, 'volume') as mocked_volume:
-            mocked_check_media.return_value = False
-            result = player.load(mocked_display)
-
-        # THEN: stuff
-        mocked_display.controller.media_info.file_info.absoluteFilePath.assert_called_once_with()
-        mocked_check_media.assert_called_once_with('/path/to/file')
-        assert result is False
-
-    def test_resize(self):
-        """
-        Test the resize() method of the SystemPlayer
-        """
-        # GIVEN: A SystemPlayer instance and a mocked display
-        player = SystemPlayer(self)
-        mocked_display = MagicMock()
-        mocked_display.size.return_value = [1, 2, 3, 4]
-
-        # WHEN: The resize() method is called
-        player.resize(mocked_display)
-
-        # THEN: The player is resized
-        mocked_display.size.assert_called_once_with()
-        mocked_display.video_widget.resize.assert_called_once_with([1, 2, 3, 4])
-
-    @patch('openlp.core.ui.media.systemplayer.functools')
-    def test_play_is_live(self, mocked_functools):
-        """
-        Test the play() method of the SystemPlayer on the live display
-        """
-        # GIVEN: A SystemPlayer instance and a mocked display
-        mocked_functools.partial.return_value = 'function'
-        player = SystemPlayer(self)
-        mocked_display = MagicMock()
-        mocked_display.controller.is_live = True
-        mocked_display.controller.media_info.start_time = 1
-        mocked_display.controller.media_info.volume = 1
-
-        # WHEN: play() is called
-        with patch.object(player, 'get_live_state') as mocked_get_live_state, \
-                patch.object(player, 'seek') as mocked_seek, \
-                patch.object(player, 'volume') as mocked_volume, \
-                patch.object(player, 'set_state') as mocked_set_state, \
-                patch.object(player, 'disconnect_slots') as mocked_disconnect_slots:
-            mocked_get_live_state.return_value = QtMultimedia.QMediaPlayer.PlayingState
-            result = player.play(mocked_display)
-
-        # THEN: the media file is played
-        mocked_get_live_state.assert_called_once_with()
-        mocked_display.media_player.play.assert_called_once_with()
-        mocked_seek.assert_called_once_with(mocked_display, 1000)
-        mocked_volume.assert_called_once_with(mocked_display, 1)
-        mocked_disconnect_slots.assert_called_once_with(mocked_display.media_player.durationChanged)
-        mocked_display.media_player.durationChanged.connect.assert_called_once_with('function')
-        mocked_set_state.assert_called_once_with(MediaState.Playing, mocked_display)
-        mocked_display.video_widget.raise_.assert_called_once_with()
-        assert result is True
-
-    @patch('openlp.core.ui.media.systemplayer.functools')
-    def test_play_is_preview(self, mocked_functools):
-        """
-        Test the play() method of the SystemPlayer on the preview display
-        """
-        # GIVEN: A SystemPlayer instance and a mocked display
-        mocked_functools.partial.return_value = 'function'
-        player = SystemPlayer(self)
-        mocked_display = MagicMock()
-        mocked_display.controller.is_live = False
-        mocked_display.controller.media_info.start_time = 1
-        mocked_display.controller.media_info.volume = 1
-
-        # WHEN: play() is called
-        with patch.object(player, 'get_preview_state') as mocked_get_preview_state, \
-                patch.object(player, 'seek') as mocked_seek, \
-                patch.object(player, 'volume') as mocked_volume, \
-                patch.object(player, 'set_state') as mocked_set_state:
-            mocked_get_preview_state.return_value = QtMultimedia.QMediaPlayer.PlayingState
-            result = player.play(mocked_display)
-
-        # THEN: the media file is played
-        mocked_get_preview_state.assert_called_once_with()
-        mocked_display.media_player.play.assert_called_once_with()
-        mocked_seek.assert_called_once_with(mocked_display, 1000)
-        mocked_volume.assert_called_once_with(mocked_display, 1)
-        mocked_display.media_player.durationChanged.connect.assert_called_once_with('function')
-        mocked_set_state.assert_called_once_with(MediaState.Playing, mocked_display)
-        mocked_display.video_widget.raise_.assert_called_once_with()
-        assert result is True
-
-    def test_pause_is_live(self):
-        """
-        Test the pause() method of the SystemPlayer on the live display
-        """
-        # GIVEN: A SystemPlayer instance
-        player = SystemPlayer(self)
-        mocked_display = MagicMock()
-        mocked_display.controller.is_live = True
-
-        # WHEN: The pause method is called
-        with patch.object(player, 'get_live_state') as mocked_get_live_state, \
-                patch.object(player, 'set_state') as mocked_set_state:
-            mocked_get_live_state.return_value = QtMultimedia.QMediaPlayer.PausedState
-            player.pause(mocked_display)
-
-        # THEN: The video is paused
-        mocked_display.media_player.pause.assert_called_once_with()
-        mocked_get_live_state.assert_called_once_with()
-        mocked_set_state.assert_called_once_with(MediaState.Paused, mocked_display)
-
-    def test_pause_is_preview(self):
-        """
-        Test the pause() method of the SystemPlayer on the preview display
-        """
-        # GIVEN: A SystemPlayer instance
-        player = SystemPlayer(self)
-        mocked_display = MagicMock()
-        mocked_display.controller.is_live = False
-
-        # WHEN: The pause method is called
-        with patch.object(player, 'get_preview_state') as mocked_get_preview_state, \
-                patch.object(player, 'set_state') as mocked_set_state:
-            mocked_get_preview_state.return_value = QtMultimedia.QMediaPlayer.PausedState
-            player.pause(mocked_display)
-
-        # THEN: The video is paused
-        mocked_display.media_player.pause.assert_called_once_with()
-        mocked_get_preview_state.assert_called_once_with()
-        mocked_set_state.assert_called_once_with(MediaState.Paused, mocked_display)
-
-    def test_stop(self):
-        """
-        Test the stop() method of the SystemPlayer
-        """
-        # GIVEN: A SystemPlayer instance
-        player = SystemPlayer(self)
-        mocked_display = MagicMock()
-
-        # WHEN: The stop method is called
-        with patch.object(player, 'set_visible') as mocked_set_visible, \
-                patch.object(player, 'set_state') as mocked_set_state:
-            player.stop(mocked_display)
-
-        # THEN: The video is stopped
-        mocked_display.media_player.stop.assert_called_once_with()
-        mocked_set_visible.assert_called_once_with(mocked_display, False)
-        mocked_set_state.assert_called_once_with(MediaState.Stopped, mocked_display)
-
-    def test_volume(self):
-        """
-        Test the volume() method of the SystemPlayer
-        """
-        # GIVEN: A SystemPlayer instance
-        player = SystemPlayer(self)
-        mocked_display = MagicMock()
-        mocked_display.has_audio = True
-
-        # WHEN: The stop method is called
-        player.volume(mocked_display, 2)
-
-        # THEN: The video is stopped
-        mocked_display.media_player.setVolume.assert_called_once_with(2)
-
-    def test_seek(self):
-        """
-        Test the seek() method of the SystemPlayer
-        """
-        # GIVEN: A SystemPlayer instance
-        player = SystemPlayer(self)
-        mocked_display = MagicMock()
-
-        # WHEN: The stop method is called
-        player.seek(mocked_display, 2)
-
-        # THEN: The video is stopped
-        mocked_display.media_player.setPosition.assert_called_once_with(2)
-
-    def test_reset(self):
-        """
-        Test the reset() method of the SystemPlayer
-        """
-        # GIVEN: A SystemPlayer instance
-        player = SystemPlayer(self)
-        mocked_display = MagicMock()
-
-        # WHEN: reset() is called
-        with patch.object(player, 'set_state') as mocked_set_state, \
-                patch.object(player, 'set_visible') as mocked_set_visible:
-            player.reset(mocked_display)
-
-        # THEN: The media player is reset
-        mocked_display.media_player.stop()
-        mocked_display.media_player.setMedia.assert_called_once_with(QtMultimedia.QMediaContent())
-        mocked_set_visible.assert_called_once_with(mocked_display, False)
-        mocked_display.video_widget.setVisible.assert_called_once_with(False)
-        mocked_set_state.assert_called_once_with(MediaState.Off, mocked_display)
-
-    def test_set_visible(self):
-        """
-        Test the set_visible() method on the SystemPlayer
-        """
-        # GIVEN: A SystemPlayer instance and a mocked display
-        player = SystemPlayer(self)
-        player.has_own_widget = True
-        mocked_display = MagicMock()
-
-        # WHEN: set_visible() is called
-        player.set_visible(mocked_display, True)
-
-        # THEN: The widget should be visible
-        mocked_display.video_widget.setVisible.assert_called_once_with(True)
-
-    def test_set_duration(self):
-        """
-        Test the set_duration() method of the SystemPlayer
-        """
-        # GIVEN: a mocked controller
-        mocked_controller = MagicMock()
-        mocked_controller.media_info.length = 5
-
-        # WHEN: The set_duration() is called. NB: the 10 here is ignored by the code
-        SystemPlayer.set_duration(mocked_controller, 10)
-
-        # THEN: The maximum length of the slider should be set
-        mocked_controller.seek_slider.setMaximum.assert_called_once_with(5)
-
-    def test_update_ui(self):
-        """
-        Test the update_ui() method on the SystemPlayer
-        """
-        # GIVEN: A SystemPlayer instance
-        player = SystemPlayer(self)
-        player.state = [MediaState.Playing, MediaState.Playing]
-        mocked_display = MagicMock()
-        mocked_display.media_player.state.return_value = QtMultimedia.QMediaPlayer.PausedState
-        mocked_display.controller.media_info.end_time = 1
-        mocked_display.media_player.position.return_value = 2
-        mocked_display.controller.seek_slider.isSliderDown.return_value = False
-
-        # WHEN: update_ui() is called
-        with patch.object(player, 'stop') as mocked_stop, \
-                patch.object(player, 'set_visible') as mocked_set_visible:
-            player.update_ui(mocked_display)
-
-        # THEN: The UI is updated
-        expected_stop_calls = [call(mocked_display)]
-        expected_position_calls = [call(), call()]
-        expected_block_signals_calls = [call(True), call(False)]
-        mocked_display.media_player.state.assert_called_once_with()
-        assert 1 == mocked_stop.call_count
-        assert expected_stop_calls == mocked_stop.call_args_list
-        assert 2 == mocked_display.media_player.position.call_count
-        assert expected_position_calls == mocked_display.media_player.position.call_args_list
-        mocked_set_visible.assert_called_once_with(mocked_display, False)
-        mocked_display.controller.seek_slider.isSliderDown.assert_called_once_with()
-        assert expected_block_signals_calls == mocked_display.controller.seek_slider.blockSignals.call_args_list
-        mocked_display.controller.seek_slider.setSliderPosition.assert_called_once_with(2)
-
-    def test_get_media_display_css(self):
-        """
-        Test the get_media_display_css() method of the SystemPlayer
-        """
-        # GIVEN: A SystemPlayer instance
-        player = SystemPlayer(self)
-
-        # WHEN: get_media_display_css() is called
-        result = player.get_media_display_css()
-
-        # THEN: The css should be empty
-        assert '' == result
-
-    @patch('openlp.core.ui.media.systemplayer.QtMultimedia.QMediaPlayer')
-    def test_get_info(self, MockQMediaPlayer):
-        """
-        Test the get_info() method of the SystemPlayer
-        """
-        # GIVEN: A SystemPlayer instance
-        mocked_media_player = MagicMock()
-        mocked_media_player.supportedMimeTypes.return_value = []
-        MockQMediaPlayer.return_value = mocked_media_player
-        player = SystemPlayer(self)
-
-        # WHEN: get_info() is called
-        result = player.get_info()
-
-        # THEN: The info should be correct
-        expected_info = 'This media player uses your operating system to provide media capabilities.<br/> ' \
-            '<strong>Audio</strong><br/>[]<br/><strong>Video</strong><br/>[]<br/>'
-        assert expected_info == result
-
-    @patch('openlp.core.ui.media.systemplayer.CheckMediaWorker')
-    @patch('openlp.core.ui.media.systemplayer.QtCore.QThread')
-    def test_check_media(self, MockQThread, MockCheckMediaWorker):
-        """
-        Test the check_media() method of the SystemPlayer
-        """
-        # GIVEN: A SystemPlayer instance and a mocked thread
-        valid_file = '/path/to/video.ogv'
-        mocked_application = MagicMock()
-        Registry().create()
-        Registry().register('application', mocked_application)
-        player = SystemPlayer(self)
-        mocked_thread = MagicMock()
-        mocked_thread.isRunning.side_effect = [True, False]
-        mocked_thread.quit = 'quit'  # actually supposed to be a slot, but it's all mocked out anyway
-        MockQThread.return_value = mocked_thread
-        mocked_check_media_worker = MagicMock()
-        mocked_check_media_worker.play = 'play'
-        mocked_check_media_worker.result = True
-        MockCheckMediaWorker.return_value = mocked_check_media_worker
-
-        # WHEN: check_media() is called with a valid media file
-        result = player.check_media(valid_file)
-
-        # THEN: It should return True
-        MockQThread.assert_called_once_with()
-        MockCheckMediaWorker.assert_called_once_with(valid_file)
-        mocked_check_media_worker.setVolume.assert_called_once_with(0)
-        mocked_check_media_worker.moveToThread.assert_called_once_with(mocked_thread)
-        mocked_check_media_worker.finished.connect.assert_called_once_with('quit')
-        mocked_thread.started.connect.assert_called_once_with('play')
-        mocked_thread.start.assert_called_once_with()
-        assert 2 == mocked_thread.isRunning.call_count
-        mocked_application.processEvents.assert_called_once_with()
-        assert result is True
-
-
-class TestCheckMediaWorker(TestCase):
-    """
-    Test the CheckMediaWorker class
-    """
-    def test_constructor(self):
-        """
-        Test the constructor of the CheckMediaWorker class
-        """
-        # GIVEN: A file path
-        path = 'file.ogv'
-
-        # WHEN: The CheckMediaWorker object is instantiated
-        worker = CheckMediaWorker(path)
-
-        # THEN: The correct values should be set up
-        assert worker is not None
-
-    def test_signals_media(self):
-        """
-        Test the signals() signal of the CheckMediaWorker class with a "media" origin
-        """
-        # GIVEN: A CheckMediaWorker instance
-        worker = CheckMediaWorker('file.ogv')
-
-        # WHEN: signals() is called with media and BufferedMedia
-        with patch.object(worker, 'stop') as mocked_stop, \
-                patch.object(worker, 'finished') as mocked_finished:
-            worker.signals('media', worker.BufferedMedia)
-
-        # THEN: The worker should exit and the result should be True
-        mocked_stop.assert_called_once_with()
-        mocked_finished.emit.assert_called_once_with()
-        assert worker.result is True
-
-    def test_signals_error(self):
-        """
-        Test the signals() signal of the CheckMediaWorker class with a "error" origin
-        """
-        # GIVEN: A CheckMediaWorker instance
-        worker = CheckMediaWorker('file.ogv')
-
-        # WHEN: signals() is called with error and BufferedMedia
-        with patch.object(worker, 'stop') as mocked_stop, \
-                patch.object(worker, 'finished') as mocked_finished:
-            worker.signals('error', None)
-
-        # THEN: The worker should exit and the result should be True
-        mocked_stop.assert_called_once_with()
-        mocked_finished.emit.assert_called_once_with()
-        assert worker.result is False

=== modified file 'tests/functional/openlp_core/ui/test_firsttimeform.py'
--- tests/functional/openlp_core/ui/test_firsttimeform.py	2017-12-28 08:22:55 +0000
+++ tests/functional/openlp_core/ui/test_firsttimeform.py	2018-01-07 18:07:40 +0000
@@ -92,7 +92,6 @@
         assert frw.web_access is True, 'The default value of self.web_access should be True'
         assert frw.was_cancelled is False, 'The default value of self.was_cancelled should be False'
         assert [] == frw.theme_screenshot_threads, 'The list of threads should be empty'
-        assert [] == frw.theme_screenshot_workers, 'The list of workers should be empty'
         assert frw.has_run_wizard is False, 'has_run_wizard should be False'
 
     def test_set_defaults(self):
@@ -155,32 +154,33 @@
             mocked_display_combo_box.count.assert_called_with()
             mocked_display_combo_box.setCurrentIndex.assert_called_with(1)
 
-    def test_on_cancel_button_clicked(self):
+    @patch('openlp.core.ui.firsttimeform.time')
+    @patch('openlp.core.ui.firsttimeform.get_thread_worker')
+    @patch('openlp.core.ui.firsttimeform.is_thread_finished')
+    def test_on_cancel_button_clicked(self, mocked_is_thread_finished, mocked_get_thread_worker, mocked_time):
         """
         Test that the cancel button click slot shuts down the threads correctly
         """
         # GIVEN: A FRW, some mocked threads and workers (that isn't quite done) and other mocked stuff
-        frw = FirstTimeForm(None)
-        frw.initialize(MagicMock())
         mocked_worker = MagicMock()
-        mocked_thread = MagicMock()
-        mocked_thread.isRunning.side_effect = [True, False]
-        frw.theme_screenshot_workers.append(mocked_worker)
-        frw.theme_screenshot_threads.append(mocked_thread)
-        with patch('openlp.core.ui.firsttimeform.time') as mocked_time, \
-                patch.object(frw.application, 'set_normal_cursor') as mocked_set_normal_cursor:
+        mocked_get_thread_worker.return_value = mocked_worker
+        mocked_is_thread_finished.side_effect = [False, True]
+        frw = FirstTimeForm(None)
+        frw.initialize(MagicMock())
+        frw.theme_screenshot_threads = ['test_thread']
+        with patch.object(frw.application, 'set_normal_cursor') as mocked_set_normal_cursor:
 
             # WHEN: on_cancel_button_clicked() is called
             frw.on_cancel_button_clicked()
 
             # THEN: The right things should be called in the right order
             assert frw.was_cancelled is True, 'The was_cancelled property should have been set to True'
+            mocked_get_thread_worker.assert_called_once_with('test_thread')
             mocked_worker.set_download_canceled.assert_called_with(True)
-            mocked_thread.isRunning.assert_called_with()
-            assert 2 == mocked_thread.isRunning.call_count, 'isRunning() should have been called twice'
-            mocked_time.sleep.assert_called_with(0.1)
-            assert 1 == mocked_time.sleep.call_count, 'sleep() should have only been called once'
-            mocked_set_normal_cursor.assert_called_with()
+            mocked_is_thread_finished.assert_called_with('test_thread')
+            assert mocked_is_thread_finished.call_count == 2, 'isRunning() should have been called twice'
+            mocked_time.sleep.assert_called_once_with(0.1)
+            mocked_set_normal_cursor.assert_called_once_with()
 
     def test_broken_config(self):
         """

=== modified file 'tests/functional/openlp_core/ui/test_mainwindow.py'
--- tests/functional/openlp_core/ui/test_mainwindow.py	2017-12-29 09:15:48 +0000
+++ tests/functional/openlp_core/ui/test_mainwindow.py	2018-01-07 18:07:40 +0000
@@ -60,9 +60,10 @@
         # Mock cursor busy/normal methods.
         self.app.set_busy_cursor = MagicMock()
         self.app.set_normal_cursor = MagicMock()
+        self.app.process_events = MagicMock()
         self.app.args = []
         Registry().register('application', self.app)
-        Registry().set_flag('no_web_server', False)
+        Registry().set_flag('no_web_server', True)
         self.add_toolbar_action_patcher = patch('openlp.core.ui.mainwindow.create_action')
         self.mocked_add_toolbar_action = self.add_toolbar_action_patcher.start()
         self.mocked_add_toolbar_action.side_effect = self._create_mock_action
@@ -74,8 +75,8 @@
         """
         Delete all the C++ objects and stop all the patchers
         """
+        del self.main_window
         self.add_toolbar_action_patcher.stop()
-        del self.main_window
 
     def test_cmd_line_file(self):
         """
@@ -92,20 +93,20 @@
         # THEN the service from the arguments is loaded
         mocked_load_file.assert_called_with(service)
 
-    def test_cmd_line_arg(self):
+    @patch('openlp.core.ui.servicemanager.ServiceManager.load_file')
+    def test_cmd_line_arg(self, mocked_load_file):
         """
         Test that passing a non service file does nothing.
         """
         # GIVEN a non service file as an argument to openlp
         service = os.path.join('openlp.py')
         self.main_window.arguments = [service]
-        with patch('openlp.core.ui.servicemanager.ServiceManager.load_file') as mocked_load_file:
-
-            # WHEN the argument is processed
-            self.main_window.open_cmd_line_files("")
-
-            # THEN the file should not be opened
-            assert mocked_load_file.called is False, 'load_file should not have been called'
+
+        # WHEN the argument is processed
+        self.main_window.open_cmd_line_files(service)
+
+        # THEN the file should not be opened
+        assert mocked_load_file.called is False, 'load_file should not have been called'
 
     def test_main_window_title(self):
         """

=== modified file 'tests/functional/openlp_plugins/songs/test_songselect.py'
--- tests/functional/openlp_plugins/songs/test_songselect.py	2017-12-29 10:19:33 +0000
+++ tests/functional/openlp_plugins/songs/test_songselect.py	2018-01-07 18:07:40 +0000
@@ -765,9 +765,9 @@
         assert ssform.search_combobox.isEnabled() is True
 
     @patch('openlp.plugins.songs.forms.songselectform.Settings')
-    @patch('openlp.plugins.songs.forms.songselectform.QtCore.QThread')
+    @patch('openlp.plugins.songs.forms.songselectform.run_thread')
     @patch('openlp.plugins.songs.forms.songselectform.SearchWorker')
-    def test_on_search_button_clicked(self, MockedSearchWorker, MockedQtThread, MockedSettings):
+    def test_on_search_button_clicked(self, MockedSearchWorker, mocked_run_thread, MockedSettings):
         """
         Test that search fields are disabled when search button is clicked.
         """

=== modified file 'tests/interfaces/openlp_core/ui/test_mainwindow.py'
--- tests/interfaces/openlp_core/ui/test_mainwindow.py	2017-12-29 09:15:48 +0000
+++ tests/interfaces/openlp_core/ui/test_mainwindow.py	2018-01-07 18:07:40 +0000
@@ -44,21 +44,21 @@
         self.app.set_normal_cursor = MagicMock()
         self.app.args = []
         Registry().register('application', self.app)
-        Registry().set_flag('no_web_server', False)
+        Registry().set_flag('no_web_server', True)
         # Mock classes and methods used by mainwindow.
-        with patch('openlp.core.ui.mainwindow.SettingsForm') as mocked_settings_form, \
-                patch('openlp.core.ui.mainwindow.ImageManager') as mocked_image_manager, \
-                patch('openlp.core.ui.mainwindow.LiveController') as mocked_live_controller, \
-                patch('openlp.core.ui.mainwindow.PreviewController') as mocked_preview_controller, \
-                patch('openlp.core.ui.mainwindow.OpenLPDockWidget') as mocked_dock_widget, \
-                patch('openlp.core.ui.mainwindow.QtWidgets.QToolBox') as mocked_q_tool_box_class, \
-                patch('openlp.core.ui.mainwindow.QtWidgets.QMainWindow.addDockWidget') as mocked_add_dock_method, \
-                patch('openlp.core.ui.mainwindow.ServiceManager') as mocked_service_manager, \
-                patch('openlp.core.ui.mainwindow.ThemeManager') as mocked_theme_manager, \
-                patch('openlp.core.ui.mainwindow.ProjectorManager') as mocked_projector_manager, \
-                patch('openlp.core.ui.mainwindow.Renderer') as mocked_renderer, \
-                patch('openlp.core.ui.mainwindow.websockets.WebSocketServer') as mocked_websocketserver, \
-                patch('openlp.core.ui.mainwindow.server.HttpServer') as mocked_httpserver:
+        with patch('openlp.core.ui.mainwindow.SettingsForm'), \
+                patch('openlp.core.ui.mainwindow.ImageManager'), \
+                patch('openlp.core.ui.mainwindow.LiveController'), \
+                patch('openlp.core.ui.mainwindow.PreviewController'), \
+                patch('openlp.core.ui.mainwindow.OpenLPDockWidget'), \
+                patch('openlp.core.ui.mainwindow.QtWidgets.QToolBox'), \
+                patch('openlp.core.ui.mainwindow.QtWidgets.QMainWindow.addDockWidget'), \
+                patch('openlp.core.ui.mainwindow.ServiceManager'), \
+                patch('openlp.core.ui.mainwindow.ThemeManager'), \
+                patch('openlp.core.ui.mainwindow.ProjectorManager'), \
+                patch('openlp.core.ui.mainwindow.Renderer'), \
+                patch('openlp.core.ui.mainwindow.websockets.WebSocketServer'), \
+                patch('openlp.core.ui.mainwindow.server.HttpServer'):
             self.main_window = MainWindow()
 
     def tearDown(self):