From 1dda8f339f7a130dcc7fa9a179ef71d7fb3a06f7 Mon Sep 17 00:00:00 2001
From: Raoul Snyman <raoul@snyman.info>
Date: Sat, 16 Dec 2017 23:50:23 -0700
Subject: [PATCH 01/22] Refactor threading to a centralised location which can
 keep track of all the threads

---
 openlp/core/threading.py                     | 48 ++++++++++++++------
 openlp/core/ui/mainwindow.py                 | 47 ++++++++++---------
 openlp/plugins/songs/forms/songselectform.py | 37 ++++-----------
 3 files changed, 69 insertions(+), 63 deletions(-)

diff --git a/openlp/core/threading.py b/openlp/core/threading.py
index 36dfd481f..9e4913082 100644
--- a/openlp/core/threading.py
+++ b/openlp/core/threading.py
@@ -24,26 +24,28 @@ The :mod:`openlp.core.threading` module contains some common threading code
 """
 from PyQt5 import QtCore
 
+from openlp.core.common.registry import Registry
 
-def run_thread(parent, worker, prefix='', auto_start=True):
+
+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 +53,25 @@ def run_thread(parent, worker, prefix='', auto_start=True):
     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 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 function: 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
diff --git a/openlp/core/ui/mainwindow.py b/openlp/core/ui/mainwindow.py
index 26a8b921a..6972b409f 100644
--- a/openlp/core/ui/mainwindow.py
+++ b/openlp/core/ui/mainwindow.py
@@ -482,8 +482,7 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties):
         """
         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).
@@ -1005,25 +1004,31 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties):
         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
+        # 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():
+            self.application.processEvents()
+            thread = self.threads[thread_name]['thread']
+            try:
+                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()
         # If we just did a settings import, close without saving changes.
         if self.settings_imported:
             self.clean_up(False)
diff --git a/openlp/plugins/songs/forms/songselectform.py b/openlp/plugins/songs/forms/songselectform.py
index 5d175bb5e..d8fe6da25 100644
--- a/openlp/plugins/songs/forms/songselectform.py
+++ b/openlp/plugins/songs/forms/songselectform.py
@@ -27,10 +27,10 @@ from time import sleep
 
 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 run_thread
 from openlp.plugins.songs.forms.songselectdialog import Ui_SongSelectDialog
 from openlp.plugins.songs.lib.songselect import SongSelectImport
 
@@ -74,7 +74,7 @@ class SearchWorker(QtCore.QObject):
         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 +90,6 @@ class SongSelectForm(QtWidgets.QDialog, Ui_SongSelectDialog):
         """
         Initialise the SongSelectForm
         """
-        self.thread = None
-        self.worker = None
         self.song_count = 0
         self.song = None
         self.set_progress_visible(False)
@@ -311,17 +309,11 @@ class SongSelectForm(QtWidgets.QDialog, Ui_SongSelectDialog):
         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 +400,3 @@ class SongSelectForm(QtWidgets.QDialog, Ui_SongSelectDialog):
         """
         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

From 786462148f520b2a1820c7494dcef3b2ad61cd30 Mon Sep 17 00:00:00 2001
From: Raoul Snyman <raoul@snyman.info>
Date: Sat, 16 Dec 2017 23:54:21 -0700
Subject: [PATCH 02/22] Fix an old run_thread() call

---
 openlp/core/version.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/openlp/core/version.py b/openlp/core/version.py
index 6ba1b9ecc..3727dcca1 100644
--- a/openlp/core/version.py
+++ b/openlp/core/version.py
@@ -127,7 +127,7 @@ def check_for_update(parent):
     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():

From d75b3f3ef1a3ceab55ccd3afa6ce4c25302b86a6 Mon Sep 17 00:00:00 2001
From: Raoul Snyman <raoul@snyman.info>
Date: Sun, 17 Dec 2017 00:25:29 -0700
Subject: [PATCH 03/22] Add some tests for threading, and fix a problem in a
 test related to the threading change

---
 tests/functional/openlp_plugins/songs/test_songselect.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/functional/openlp_plugins/songs/test_songselect.py b/tests/functional/openlp_plugins/songs/test_songselect.py
index fa2b2931d..ad711a84b 100644
--- a/tests/functional/openlp_plugins/songs/test_songselect.py
+++ b/tests/functional/openlp_plugins/songs/test_songselect.py
@@ -773,9 +773,9 @@ class TestSongSelectForm(TestCase, TestMixin):
         self.assertTrue(ssform.search_combobox.isEnabled())
 
     @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.
         """

From 9bbd3fa72daeab977b2c6bceda73f5d228870ee0 Mon Sep 17 00:00:00 2001
From: Raoul Snyman <raoul@snyman.info>
Date: Sun, 17 Dec 2017 00:45:40 -0700
Subject: [PATCH 04/22] Add threading tests

---
 .../functional/openlp_core/test_threading.py  | 89 +++++++++++++++++++
 1 file changed, 89 insertions(+)
 create mode 100644 tests/functional/openlp_core/test_threading.py

diff --git a/tests/functional/openlp_core/test_threading.py b/tests/functional/openlp_core/test_threading.py
new file mode 100644
index 000000000..e7b628b8e
--- /dev/null
+++ b/tests/functional/openlp_core/test_threading.py
@@ -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-2017 OpenLP Developers                                   #
+# --------------------------------------------------------------------------- #
+# This program is free software; you can redistribute it and/or modify it     #
+# under the terms of the GNU General Public License as published by the Free  #
+# Software Foundation; version 2 of the License.                              #
+#                                                                             #
+# This program is distributed in the hope that it will be useful, but WITHOUT #
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or       #
+# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for    #
+# more details.                                                               #
+#                                                                             #
+# You should have received a copy of the GNU General Public License along     #
+# with this program; if not, write to the Free Software Foundation, Inc., 59  #
+# Temple Place, Suite 330, Boston, MA 02111-1307 USA                          #
+###############################################################################
+"""
+Package to test the openlp.core.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()

From 2fa88b17dbaa045325d0c66ba413760c977d59f4 Mon Sep 17 00:00:00 2001
From: Raoul Snyman <raoul@snyman.info>
Date: Wed, 20 Dec 2017 07:17:07 -0700
Subject: [PATCH 05/22] Refactor threads to use new openlp.core.threading
 module

---
 openlp/core/api/http/server.py                | 26 ++++--------
 openlp/core/api/websockets.py                 | 21 +++++-----
 openlp/core/lib/imagemanager.py               | 21 ++++++----
 openlp/core/threading.py                      | 34 ++++++++++++++++
 openlp/core/ui/firsttimeform.py               | 40 +++++++++----------
 openlp/core/ui/mainwindow.py                  |  4 +-
 openlp/core/ui/media/systemplayer.py          | 28 ++++++-------
 openlp/core/version.py                        | 11 +++--
 openlp/plugins/songs/forms/songselectform.py  |  5 +--
 .../openlp_core/ui/media/test_systemplayer.py | 33 ++++-----------
 10 files changed, 117 insertions(+), 106 deletions(-)

diff --git a/openlp/core/api/http/server.py b/openlp/core/api/http/server.py
index c80275801..12e5eed7f 100644
--- a/openlp/core/api/http/server.py
+++ b/openlp/core/api/http/server.py
@@ -39,28 +39,21 @@ from openlp.core.api.http import application
 from openlp.core.api.poll import Poller
 from openlp.core.common.applocation import AppLocation
 from openlp.core.common.i18n import UiStrings
+from openlp.core.common.i18n import translate
 from openlp.core.common.mixins import LogMixin, RegistryProperties
 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.common.i18n import translate
+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.
         """
@@ -71,9 +64,7 @@ class HttpWorker(QtCore.QObject):
             serve(application, host=address, port=port)
         except OSError:
             log.exception('An error occurred when serving the application.')
-
-    def stop(self):
-        pass
+        self.quit.emit()
 
 
 class HttpServer(RegistryBase, RegistryProperties, LogMixin):
@@ -86,11 +77,8 @@ class HttpServer(RegistryBase, RegistryProperties, LogMixin):
         """
         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()
+            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')
diff --git a/openlp/core/api/websockets.py b/openlp/core/api/websockets.py
index 3417eb74d..dc7cf8f82 100644
--- a/openlp/core/api/websockets.py
+++ b/openlp/core/api/websockets.py
@@ -34,11 +34,12 @@ from PyQt5 import QtCore
 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):
+class WebSocketWorker(ThreadWorker):
     """
     A special Qt thread class to allow the WebSockets server to run at the same time as the UI.
     """
@@ -49,15 +50,20 @@ class WebSocketWorker(QtCore.QObject):
         :param server: The http server class.
         """
         self.ws_server = server
-        super(WebSocketWorker, self).__init__()
+        super().__init__()
 
-    def run(self):
+    def start(self):
         """
-        Run the thread.
+        Run the worker.
         """
         self.ws_server.start_server()
+        self.quit.emit()
 
+    @QtCore.pyqtSlot()
     def stop(self):
+        """
+        Stop the websocket server
+        """
         self.ws_server.stop = True
 
 
@@ -72,11 +78,8 @@ class WebSocketServer(RegistryProperties, LogMixin):
         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()
+            worker = WebSocketWorker(self)
+            run_thread(worker, 'websocket_server')
 
     def start_server(self):
         """
diff --git a/openlp/core/lib/imagemanager.py b/openlp/core/lib/imagemanager.py
index 81a1b659d..0762bcd64 100644
--- a/openlp/core/lib/imagemanager.py
+++ b/openlp/core/lib/imagemanager.py
@@ -35,13 +35,14 @@ from openlp.core.common.registry import Registry
 from openlp.core.common.settings import Settings
 from openlp.core.display.screens import ScreenList
 from openlp.core.lib import 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,15 @@ class ImageThread(QtCore.QThread):
         ``manager``
             The image manager.
         """
-        super(ImageThread, self).__init__(None)
+        super().__init__()
         self.image_manager = manager
 
-    def run(self):
+    def start(self):
         """
         Run the thread.
         """
         self.image_manager.process()
+        self.quit.emit()
 
 
 class Priority(object):
@@ -179,7 +181,6 @@ class ImageManager(QtCore.QObject):
         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 +231,13 @@ class ImageManager(QtCore.QObject):
         """
         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):
         """
diff --git a/openlp/core/threading.py b/openlp/core/threading.py
index 9e4913082..96c80cbfd 100644
--- a/openlp/core/threading.py
+++ b/openlp/core/threading.py
@@ -27,6 +27,19 @@ from PyQt5 import QtCore
 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.
@@ -58,6 +71,27 @@ def run_thread(worker, thread_name, can_start=True):
         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 ThreadWorker: 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 bool: 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.
diff --git a/openlp/core/ui/firsttimeform.py b/openlp/core/ui/firsttimeform.py
index 37fac2dd4..6c739c0b4 100644
--- a/openlp/core/ui/firsttimeform.py
+++ b/openlp/core/ui/firsttimeform.py
@@ -36,20 +36,21 @@ from PyQt5 import QtCore, QtWidgets
 
 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.i18n import translate
-from openlp.core.common.path import Path, create_paths
 from openlp.core.common.mixins import RegistryProperties
+from openlp.core.common.path import Path, create_paths
 from openlp.core.common.registry import Registry
 from openlp.core.common.settings import Settings
 from openlp.core.lib import PluginStatus, build_icon
 from openlp.core.lib.ui import critical_error_message_box
-from openlp.core.common.httputils import get_web_page, get_url_file_size, url_get_file, CONNECTION_TIMEOUT
-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
     """
@@ -67,11 +68,11 @@ class ThemeScreenshotWorker(QtCore.QObject):
         self.sha256 = sha256
         self.screenshot = screenshot
         socket.setdefaulttimeout(CONNECTION_TIMEOUT)
-        super(ThemeScreenshotWorker, self).__init__()
+        super().__init__()
 
-    def run(self):
+    def start(self):
         """
-        Overridden method to run the thread.
+        Run the worker
         """
         if self.was_download_cancelled:
             return
@@ -80,9 +81,10 @@ class ThemeScreenshotWorker(QtCore.QObject):
                                        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:
+        except:                                                                 # noqa
             log.exception('Unable to download screenshot')
         finally:
+            self.quit.emit()
             self.finished.emit()
 
     @QtCore.pyqtSlot(bool)
@@ -145,12 +147,13 @@ class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties):
             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 +174,6 @@ class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties):
         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 +258,10 @@ class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties):
                 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,9 +351,11 @@ class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties):
         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]):
diff --git a/openlp/core/ui/mainwindow.py b/openlp/core/ui/mainwindow.py
index de633b5b3..d905aec25 100644
--- a/openlp/core/ui/mainwindow.py
+++ b/openlp/core/ui/mainwindow.py
@@ -1071,8 +1071,8 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties):
         :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)
+        # 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())
diff --git a/openlp/core/ui/media/systemplayer.py b/openlp/core/ui/media/systemplayer.py
index a6423db02..59245aa89 100644
--- a/openlp/core/ui/media/systemplayer.py
+++ b/openlp/core/ui/media/systemplayer.py
@@ -31,6 +31,7 @@ from PyQt5 import QtCore, QtMultimedia, QtMultimediaWidgets
 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__)
@@ -294,39 +295,38 @@ class SystemPlayer(MediaPlayer):
         :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.result = None
+        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()
diff --git a/openlp/core/version.py b/openlp/core/version.py
index 3727dcca1..e4d269f81 100644
--- a/openlp/core/version.py
+++ b/openlp/core/version.py
@@ -35,7 +35,7 @@ from PyQt5 import QtCore
 
 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_TIMEOUT = 30
 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):
         """
@@ -112,18 +111,18 @@ def update_check_date():
     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)
diff --git a/openlp/plugins/songs/forms/songselectform.py b/openlp/plugins/songs/forms/songselectform.py
index d8fe6da25..cb7bf5c06 100644
--- a/openlp/plugins/songs/forms/songselectform.py
+++ b/openlp/plugins/songs/forms/songselectform.py
@@ -30,21 +30,20 @@ from PyQt5 import QtCore, QtWidgets
 from openlp.core.common.i18n import translate
 from openlp.core.common.mixins import RegistryProperties
 from openlp.core.common.settings import Settings
-from openlp.core.threading import run_thread
+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__()
diff --git a/tests/functional/openlp_core/ui/media/test_systemplayer.py b/tests/functional/openlp_core/ui/media/test_systemplayer.py
index 2fe2e0619..33e8d808c 100644
--- a/tests/functional/openlp_core/ui/media/test_systemplayer.py
+++ b/tests/functional/openlp_core/ui/media/test_systemplayer.py
@@ -462,8 +462,8 @@ class TestSystemPlayer(TestCase):
         self.assertEqual(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):
+    @patch('openlp.core.ui.media.systemplayer.run_thread')
+    def test_check_media(self, mocked_run_thread, MockCheckMediaWorker):
         """
         Test the check_media() method of the SystemPlayer
         """
@@ -473,29 +473,12 @@ class TestSystemPlayer(TestCase):
         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)
+        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()
-        self.assertEqual(2, mocked_thread.isRunning.call_count)
-        mocked_application.processEvents.assert_called_once_with()
-        self.assertTrue(result)
+        assert False, 'Fix this test'
 
 
 class TestCheckMediaWorker(TestCase):
@@ -524,12 +507,12 @@ class TestCheckMediaWorker(TestCase):
 
         # WHEN: signals() is called with media and BufferedMedia
         with patch.object(worker, 'stop') as mocked_stop, \
-                patch.object(worker, 'finished') as mocked_finished:
+                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_finished.emit.assert_called_once_with()
+        mocked_quit.emit.assert_called_once_with()
         self.assertTrue(worker.result)
 
     def test_signals_error(self):
@@ -541,10 +524,10 @@ class TestCheckMediaWorker(TestCase):
 
         # WHEN: signals() is called with error and BufferedMedia
         with patch.object(worker, 'stop') as mocked_stop, \
-                patch.object(worker, 'finished') as mocked_finished:
+                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_finished.emit.assert_called_once_with()
+        mocked_quit.emit.assert_called_once_with()
         self.assertFalse(worker.result)

From 2eb89c83617854178742ab767eae7cd061acccaa Mon Sep 17 00:00:00 2001
From: Raoul Snyman <raoul@snyman.info>
Date: Sat, 23 Dec 2017 14:53:13 -0700
Subject: [PATCH 06/22] Some enhancements to projectors

---
 openlp/core/projectors/manager.py | 3 +--
 openlp/core/projectors/pjlink.py  | 4 ++--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/openlp/core/projectors/manager.py b/openlp/core/projectors/manager.py
index b352858b1..ef3d2bda4 100644
--- a/openlp/core/projectors/manager.py
+++ b/openlp/core/projectors/manager.py
@@ -292,8 +292,7 @@ class ProjectorManager(QtWidgets.QWidget, RegistryBase, UiProjectorManager, LogM
         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):
diff --git a/openlp/core/projectors/pjlink.py b/openlp/core/projectors/pjlink.py
index 12ae7e1d0..3adb90a26 100644
--- a/openlp/core/projectors/pjlink.py
+++ b/openlp/core/projectors/pjlink.py
@@ -91,11 +91,11 @@ class PJLinkUDP(QtNetwork.QUdpSocket):
         '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
 
 

From f1575dd50bb675eb2ded7526ed8a80fc6250d253 Mon Sep 17 00:00:00 2001
From: Raoul Snyman <raoul@snyman.info>
Date: Wed, 3 Jan 2018 23:01:35 -0700
Subject: [PATCH 07/22] Updated various thread usages

---
 openlp/core/api/deploy.py                     |   4 +-
 openlp/core/api/http/server.py                |   2 +-
 openlp/core/common/httputils.py               |  15 +-
 openlp/core/lib/imagemanager.py               |  14 +-
 openlp/core/ui/firsttimeform.py               |  24 +--
 .../openlp_core/api/http/test_http.py         |  16 +-
 .../openlp_core/api/test_websockets.py        |  16 +-
 .../openlp_core/common/test_httputils.py      |   4 +-
 .../openlp_core/lib/test_image_manager.py     | 195 +++++++++++++-----
 .../openlp_core/ui/test_firsttimeform.py      |  28 +--
 10 files changed, 208 insertions(+), 110 deletions(-)

diff --git a/openlp/core/api/deploy.py b/openlp/core/api/deploy.py
index a42f83f0c..83eef9ea4 100644
--- a/openlp/core/api/deploy.py
+++ b/openlp/core/api/deploy.py
@@ -26,7 +26,7 @@ from zipfile import ZipFile
 
 from openlp.core.common.applocation import AppLocation
 from openlp.core.common.registry import Registry
-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
 
 
 def deploy_zipfile(app_root_path, zip_name):
@@ -65,7 +65,7 @@ def download_and_check(callback=None):
     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',
+    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')
diff --git a/openlp/core/api/http/server.py b/openlp/core/api/http/server.py
index 12e5eed7f..cb50d5078 100644
--- a/openlp/core/api/http/server.py
+++ b/openlp/core/api/http/server.py
@@ -155,7 +155,7 @@ class DownloadProgressDialog(QtWidgets.QProgressDialog):
         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.
         """
diff --git a/openlp/core/common/httputils.py b/openlp/core/common/httputils.py
index 21b778b80..d07185638 100644
--- a/openlp/core/common/httputils.py
+++ b/openlp/core/common/httputils.py
@@ -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 @@ def get_web_page(url, headers=None, update_openlp=False, proxies=None):
             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 @@ def get_url_file_size(url):
                 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 @@ def url_get_file(callback, url, file_path, sha256=None):
                     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 @@ def url_get_file(callback, url, file_path, sha256=None):
                 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
 
diff --git a/openlp/core/lib/imagemanager.py b/openlp/core/lib/imagemanager.py
index 0762bcd64..96b315717 100644
--- a/openlp/core/lib/imagemanager.py
+++ b/openlp/core/lib/imagemanager.py
@@ -57,11 +57,17 @@ class ImageWorker(ThreadWorker):
 
     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):
     """
@@ -132,7 +138,7 @@ class Image(object):
 
 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`
@@ -310,9 +316,7 @@ class ImageManager(QtCore.QObject):
                 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):
         """
diff --git a/openlp/core/ui/firsttimeform.py b/openlp/core/ui/firsttimeform.py
index 6c739c0b4..940b1ccec 100644
--- a/openlp/core/ui/firsttimeform.py
+++ b/openlp/core/ui/firsttimeform.py
@@ -36,7 +36,7 @@ from PyQt5 import QtCore, QtWidgets
 
 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, CONNECTION_TIMEOUT
 from openlp.core.common.i18n import translate
 from openlp.core.common.mixins import RegistryProperties
 from openlp.core.common.path import Path, create_paths
@@ -55,37 +55,35 @@ 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().__init__()
 
     def start(self):
         """
         Run the worker
         """
-        if self.was_download_cancelled:
+        if self.was_cancelled:
             return
         try:
-            urllib.request.urlretrieve('{host}{name}'.format(host=self.themes_url, name=self.screenshot),
+            is_success = download_file(self, '{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)
+            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.quit.emit()
-            self.finished.emit()
 
     @QtCore.pyqtSlot(bool)
     def set_download_canceled(self, toggle):
@@ -358,7 +356,7 @@ class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties):
                     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,7 +560,7 @@ class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties):
                 self._increment_progress_bar(self.downloading.format(name=filename), 0)
                 self.previous_size = 0
                 destination = Path(songs_destination, str(filename))
-                if not url_get_file(self, '{path}{name}'.format(path=self.songs_url, name=filename),
+                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
@@ -573,7 +571,7 @@ class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties):
                 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),
+                if not download_file(self, '{path}{name}'.format(path=self.bibles_url, name=bible),
                                     Path(bibles_destination, bible),
                                     sha256):
                     missed_files.append('Bible: {name}'.format(name=bible))
@@ -585,7 +583,7 @@ class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties):
                 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),
+                if not download_file(self, '{path}{name}'.format(path=self.themes_url, name=theme),
                                     Path(themes_destination, theme),
                                     sha256):
                     missed_files.append('Theme: {name}'.format(name=theme))
diff --git a/tests/functional/openlp_core/api/http/test_http.py b/tests/functional/openlp_core/api/http/test_http.py
index 7dd8418ae..b8b30cc25 100644
--- a/tests/functional/openlp_core/api/http/test_http.py
+++ b/tests/functional/openlp_core/api/http/test_http.py
@@ -42,8 +42,8 @@ class TestHttpServer(TestCase):
         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
         """
@@ -53,12 +53,12 @@ class TestHttpServer(TestCase):
         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'
+        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.QtCore.QThread')
-    def test_server_start_not_required(self, mock_qthread, mock_thread):
+    @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
         """
@@ -68,5 +68,5 @@ class TestHttpServer(TestCase):
         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'
diff --git a/tests/functional/openlp_core/api/test_websockets.py b/tests/functional/openlp_core/api/test_websockets.py
index 7da90123d..4b1917c36 100644
--- a/tests/functional/openlp_core/api/test_websockets.py
+++ b/tests/functional/openlp_core/api/test_websockets.py
@@ -63,8 +63,8 @@ class TestWSServer(TestCase, TestMixin):
         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
         """
@@ -74,12 +74,12 @@ class TestWSServer(TestCase, TestMixin):
         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
         """
@@ -89,8 +89,8 @@ class TestWSServer(TestCase, TestMixin):
         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):
         """
diff --git a/tests/functional/openlp_core/common/test_httputils.py b/tests/functional/openlp_core/common/test_httputils.py
index b7c08993f..7cc75c0e4 100644
--- a/tests/functional/openlp_core/common/test_httputils.py
+++ b/tests/functional/openlp_core/common/test_httputils.py
@@ -27,7 +27,7 @@ import tempfile
 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
@@ -236,7 +236,7 @@ class TestHttpUtils(TestCase, TestMixin):
         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
diff --git a/tests/functional/openlp_core/lib/test_image_manager.py b/tests/functional/openlp_core/lib/test_image_manager.py
index 3c47e81a1..2a0e61c9a 100644
--- a/tests/functional/openlp_core/lib/test_image_manager.py
+++ b/tests/functional/openlp_core/lib/test_image_manager.py
@@ -25,20 +25,113 @@ Package to test the openlp.core.ui package.
 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
 
 TEST_PATH = os.path.abspath(os.path.join(os.path.dirname(__file__), '..', '..', '..', 'resources'))
 
 
+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 @@ class TestImageManager(TestCase, TestMixin):
         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 @@ class TestImageManager(TestCase, TestMixin):
             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 @@ class TestImageManager(TestCase, TestMixin):
             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'
+        # 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)
+        # 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'"
+        # 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.
+        # 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):
         """
diff --git a/tests/functional/openlp_core/ui/test_firsttimeform.py b/tests/functional/openlp_core/ui/test_firsttimeform.py
index 2629eaa37..f99fff2f0 100644
--- a/tests/functional/openlp_core/ui/test_firsttimeform.py
+++ b/tests/functional/openlp_core/ui/test_firsttimeform.py
@@ -94,7 +94,6 @@ class TestFirstTimeForm(TestCase, TestMixin):
         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):
@@ -157,32 +156,33 @@ class TestFirstTimeForm(TestCase, TestMixin):
             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
+        mocked_worker = MagicMock()
+        mocked_get_thread_worker.return_value = mocked_worker
+        mocked_is_thread_finished.side_effect = [False, True]
         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:
+        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):
         """

From 64b26774a13a5a8c283d29adefd08cc95bc1f8e8 Mon Sep 17 00:00:00 2001
From: Raoul Snyman <raoul@snyman.info>
Date: Wed, 3 Jan 2018 23:37:59 -0700
Subject: [PATCH 08/22] For some reason, the thumbnail downloader was still
 using a string path

---
 openlp/core/ui/firsttimeform.py | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/openlp/core/ui/firsttimeform.py b/openlp/core/ui/firsttimeform.py
index 7dae8a938..631ae4dfc 100644
--- a/openlp/core/ui/firsttimeform.py
+++ b/openlp/core/ui/firsttimeform.py
@@ -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 PyQt5 import QtCore, QtWidgets
 
 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, download_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
@@ -75,8 +73,9 @@ class ThemeScreenshotWorker(ThreadWorker):
         if self.was_cancelled:
             return
         try:
+            download_path = Path(gettempdir()) / 'openlp' / self.screenshot
             is_success = download_file(self, '{host}{name}'.format(host=self.themes_url, name=self.screenshot),
-                                       os.path.join(gettempdir(), 'openlp', 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)
@@ -561,7 +560,7 @@ class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties):
                 self.previous_size = 0
                 destination = songs_destination_path / str(filename)
                 if not download_file(self, '{path}{name}'.format(path=self.songs_url, name=filename),
-                                    destination, sha256):
+                                     destination, sha256):
                     missed_files.append('Song: {name}'.format(name=filename))
         # Download Bibles
         bibles_iterator = QtWidgets.QTreeWidgetItemIterator(self.bibles_tree_widget)
@@ -572,7 +571,7 @@ class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties):
                 self._increment_progress_bar(self.downloading.format(name=bible), 0)
                 self.previous_size = 0
                 if not download_file(self, '{path}{name}'.format(path=self.bibles_url, name=bible),
-                                    bibles_destination_path / bible, sha256):
+                                     bibles_destination_path / bible, sha256):
                     missed_files.append('Bible: {name}'.format(name=bible))
             bibles_iterator += 1
         # Download themes
@@ -583,7 +582,7 @@ class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties):
                 self._increment_progress_bar(self.downloading.format(name=theme), 0)
                 self.previous_size = 0
                 if not download_file(self, '{path}{name}'.format(path=self.themes_url, name=theme),
-                                    themes_destination_path / theme, sha256):
+                                     themes_destination_path / theme, sha256):
                     missed_files.append('Theme: {name}'.format(name=theme))
         if missed_files:
             file_list = ''

From 544c396bbc8cb5e9d9dbe84c6f3de4355b637150 Mon Sep 17 00:00:00 2001
From: Raoul Snyman <raoul@snyman.info>
Date: Thu, 4 Jan 2018 00:00:55 -0700
Subject: [PATCH 09/22] Fix some linting issues

---
 openlp/core/api/deploy.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/openlp/core/api/deploy.py b/openlp/core/api/deploy.py
index 61893f768..e336019d3 100644
--- a/openlp/core/api/deploy.py
+++ b/openlp/core/api/deploy.py
@@ -66,6 +66,6 @@ def download_and_check(callback=None):
     file_size = get_url_file_size('https://get.openlp.org/webclient/site.zip')
     callback.setRange(0, file_size)
     if download_file(callback, 'https://get.openlp.org/webclient/site.zip',
-                    AppLocation.get_section_data_path('remotes') / 'site.zip',
-                    sha256=sha256):
+                     AppLocation.get_section_data_path('remotes') / 'site.zip',
+                     sha256=sha256):
         deploy_zipfile(AppLocation.get_section_data_path('remotes'), 'site.zip')

From db7ff02abb11daf9a24e549d38ea3bf0fc35dac0 Mon Sep 17 00:00:00 2001
From: Raoul Snyman <raoul@snyman.info>
Date: Thu, 4 Jan 2018 10:17:20 -0700
Subject: [PATCH 10/22] Move the thread code to after the event has been
 accepted

---
 openlp/core/ui/mainwindow.py | 58 ++++++++++++++++++++----------------
 1 file changed, 32 insertions(+), 26 deletions(-)

diff --git a/openlp/core/ui/mainwindow.py b/openlp/core/ui/mainwindow.py
index a16bfaa19..d73b7d41b 100644
--- a/openlp/core/ui/mainwindow.py
+++ b/openlp/core/ui/mainwindow.py
@@ -24,7 +24,6 @@ This is the main window, where all the action happens.
 """
 import logging
 import sys
-import time
 from datetime import datetime
 from distutils import dir_util
 from distutils.errors import DistutilsFileError
@@ -548,6 +547,36 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties):
         # 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():
+            self.application.processEvents()
+            thread = self.threads[thread_name]['thread']
+            try:
+                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
@@ -999,31 +1028,6 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties):
         if not self.application.is_event_loop_active:
             event.ignore()
             return
-        # 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():
-            self.application.processEvents()
-            thread = self.threads[thread_name]['thread']
-            try:
-                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()
         # If we just did a settings import, close without saving changes.
         if self.settings_imported:
             self.clean_up(False)
@@ -1060,6 +1064,8 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties):
             else:
                 self.clean_up()
                 event.accept()
+        if event.isAccepted():
+            self._wait_for_threads()
 
     def clean_up(self, save_settings=True):
         """

From 0601cf15435a2ec625efd390761b8d2f389b0849 Mon Sep 17 00:00:00 2001
From: Raoul Snyman <raoul@snyman.info>
Date: Thu, 4 Jan 2018 14:03:15 -0700
Subject: [PATCH 11/22] Move cleanup to after thread wait; Figured out why the
 webserver was exiting early

---
 openlp/core/api/http/server.py | 12 ++++++++++--
 openlp/core/api/websockets.py  | 13 ++++++++++---
 openlp/core/ui/mainwindow.py   | 19 +++++++++----------
 3 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/openlp/core/api/http/server.py b/openlp/core/api/http/server.py
index 6c63e6936..f2ae08d71 100644
--- a/openlp/core/api/http/server.py
+++ b/openlp/core/api/http/server.py
@@ -27,7 +27,7 @@ import logging
 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
@@ -61,11 +61,19 @@ class HttpWorker(ThreadWorker):
         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):
+        """
+        A method to stop the worker
+        """
+        if hasattr(self, 'server'):
+            self.server.close()
+
 
 class HttpServer(RegistryBase, RegistryProperties, LogMixin):
     """
diff --git a/openlp/core/api/websockets.py b/openlp/core/api/websockets.py
index bf303249f..bfa0d73b6 100644
--- a/openlp/core/api/websockets.py
+++ b/openlp/core/api/websockets.py
@@ -29,7 +29,6 @@ import logging
 import time
 
 import websockets
-from PyQt5 import QtCore
 
 from openlp.core.common.mixins import LogMixin, RegistryProperties
 from openlp.core.common.registry import Registry
@@ -59,12 +58,11 @@ class WebSocketWorker(ThreadWorker):
         self.ws_server.start_server()
         self.quit.emit()
 
-    @QtCore.pyqtSlot()
     def stop(self):
         """
         Stop the websocket server
         """
-        self.ws_server.stop = True
+        self.ws_server.stop_server()
 
 
 class WebSocketServer(RegistryProperties, LogMixin):
@@ -97,6 +95,15 @@ class WebSocketServer(RegistryProperties, LogMixin):
         else:
             log.debug('Failed to start ws server on port {port}'.format(port=port))
 
+    def stop_server(self):
+        """
+        Stop the websocket server
+        """
+        if hasattr(self.ws_server, 'ws_server'):
+            self.ws_server.ws_server.close()
+        elif hasattr(self.ws_server, 'server'):
+            self.ws_server.server.close()
+
     def start_websocket_instance(self, address, port):
         """
         Start the server
diff --git a/openlp/core/ui/mainwindow.py b/openlp/core/ui/mainwindow.py
index d73b7d41b..773ecd775 100644
--- a/openlp/core/ui/mainwindow.py
+++ b/openlp/core/ui/mainwindow.py
@@ -499,8 +499,8 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties):
         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)
@@ -560,7 +560,11 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties):
         for thread_name in self.threads.keys():
             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
@@ -1028,20 +1032,14 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties):
         if not self.application.is_event_loop_active:
             event.ignore()
             return
-        # 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()
@@ -1057,15 +1055,16 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties):
                 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):
         """

From 2d009f1884b91b4c63398a00f0c485367b420fec Mon Sep 17 00:00:00 2001
From: Raoul Snyman <raoul@snyman.info>
Date: Thu, 4 Jan 2018 14:13:24 -0700
Subject: [PATCH 12/22] Change a log.error to a log.exception

---
 openlp/core/api/websockets.py | 2 +-
 openlp/core/ui/mainwindow.py  | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/openlp/core/api/websockets.py b/openlp/core/api/websockets.py
index bfa0d73b6..95c50a0b7 100644
--- a/openlp/core/api/websockets.py
+++ b/openlp/core/api/websockets.py
@@ -118,7 +118,7 @@ class WebSocketServer(RegistryProperties, LogMixin):
                 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))
+                log.exception('Failed to start ws server {why}'.format(why=e))
                 loop += 1
                 time.sleep(0.1)
 
diff --git a/openlp/core/ui/mainwindow.py b/openlp/core/ui/mainwindow.py
index 773ecd775..0841bfa1d 100644
--- a/openlp/core/ui/mainwindow.py
+++ b/openlp/core/ui/mainwindow.py
@@ -558,6 +558,7 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties):
         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']

From 3e9073275affa8dcf93b381939f585bda701a5b0 Mon Sep 17 00:00:00 2001
From: Raoul Snyman <raoul@snyman.info>
Date: Thu, 4 Jan 2018 22:32:12 -0700
Subject: [PATCH 13/22] Fix an issue with versions of websockets > 4.0

---
 openlp/core/api/websockets.py | 147 ++++++++++++++--------------------
 1 file changed, 59 insertions(+), 88 deletions(-)

diff --git a/openlp/core/api/websockets.py b/openlp/core/api/websockets.py
index 95c50a0b7..785f3c8ff 100644
--- a/openlp/core/api/websockets.py
+++ b/openlp/core/api/websockets.py
@@ -28,7 +28,7 @@ import json
 import logging
 import time
 
-import websockets
+from websockets import serve
 
 from openlp.core.common.mixins import LogMixin, RegistryProperties
 from openlp.core.common.registry import Registry
@@ -38,31 +38,76 @@ from openlp.core.threading import ThreadWorker, run_thread
 log = logging.getLogger(__name__)
 
 
-class WebSocketWorker(ThreadWorker):
+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('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().__init__()
-
     def start(self):
         """
         Run the worker.
         """
-        self.ws_server.start_server()
+        address = Settings().value('api/ip address')
+        port = Settings().value('api/websocket port')
+        # Start the event loop
+        event_loop = asyncio.new_event_loop()
+        asyncio.set_event_loop(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 as e:
+                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
+            event_loop.run_until_complete(self.server)
+            event_loop.run_forever()
         self.quit.emit()
 
     def stop(self):
         """
         Stop the websocket server
         """
-        self.ws_server.stop_server()
+        if hasattr(self.server, 'ws_server'):
+            self.server.ws_server.close()
+        elif hasattr(self.server, 'server'):
+            self.server.server.close()
 
 
 class WebSocketServer(RegistryProperties, LogMixin):
@@ -75,79 +120,5 @@ class WebSocketServer(RegistryProperties, LogMixin):
         """
         super(WebSocketServer, self).__init__()
         if Registry().get_flag('no_web_server'):
-            self.settings_section = 'api'
-            worker = WebSocketWorker(self)
+            worker = WebSocketWorker()
             run_thread(worker, 'websocket_server')
-
-    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 stop_server(self):
-        """
-        Stop the websocket server
-        """
-        if hasattr(self.ws_server, 'ws_server'):
-            self.ws_server.ws_server.close()
-        elif hasattr(self.ws_server, 'server'):
-            self.ws_server.server.close()
-
-    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.exception('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)

From f693d3aa3782d3b284179a4cbbef2fa52538052c Mon Sep 17 00:00:00 2001
From: Raoul Snyman <raoul@snyman.info>
Date: Fri, 5 Jan 2018 21:41:49 -0700
Subject: [PATCH 14/22] Call close() to stop everything properly

---
 openlp/core/api/websockets.py | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/openlp/core/api/websockets.py b/openlp/core/api/websockets.py
index 785f3c8ff..ea03c0681 100644
--- a/openlp/core/api/websockets.py
+++ b/openlp/core/api/websockets.py
@@ -79,8 +79,8 @@ class WebSocketWorker(ThreadWorker, RegistryProperties, LogMixin):
         address = Settings().value('api/ip address')
         port = Settings().value('api/websocket port')
         # Start the event loop
-        event_loop = asyncio.new_event_loop()
-        asyncio.set_event_loop(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
@@ -96,8 +96,8 @@ class WebSocketWorker(ThreadWorker, RegistryProperties, LogMixin):
                 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
-            event_loop.run_until_complete(self.server)
-            event_loop.run_forever()
+            self.event_loop.run_until_complete(self.server)
+            self.event_loop.run_forever()
         self.quit.emit()
 
     def stop(self):
@@ -108,6 +108,8 @@ class WebSocketWorker(ThreadWorker, RegistryProperties, LogMixin):
             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):

From 5612be1b5ae21e65b2c9f2b75d22f70c583a3e1c Mon Sep 17 00:00:00 2001
From: Raoul Snyman <raoul@snyman.info>
Date: Sat, 6 Jan 2018 00:02:45 -0700
Subject: [PATCH 15/22] Stop the HTTP server properly too

---
 openlp/core/api/http/server.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/openlp/core/api/http/server.py b/openlp/core/api/http/server.py
index f2ae08d71..9d00bbbbb 100644
--- a/openlp/core/api/http/server.py
+++ b/openlp/core/api/http/server.py
@@ -72,7 +72,10 @@ class HttpWorker(ThreadWorker):
         A method to stop the worker
         """
         if hasattr(self, 'server'):
-            self.server.close()
+            # 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):

From 7b28262987cba49f8df85985e9e1db2e7a39670d Mon Sep 17 00:00:00 2001
From: Raoul Snyman <raoul@snyman.info>
Date: Sat, 6 Jan 2018 21:36:17 -0700
Subject: [PATCH 16/22] Fix a mismatch of arguments in a call to
 reload_bibles()

---
 openlp/core/ui/mainwindow.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/openlp/core/ui/mainwindow.py b/openlp/core/ui/mainwindow.py
index 0841bfa1d..79d354a90 100644
--- a/openlp/core/ui/mainwindow.py
+++ b/openlp/core/ui/mainwindow.py
@@ -728,7 +728,7 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties):
         # 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):

From 954d1618bcc59a8029cf2bd1b230f47a82bbe94a Mon Sep 17 00:00:00 2001
From: Raoul Snyman <raoul@snyman.info>
Date: Sat, 6 Jan 2018 21:36:45 -0700
Subject: [PATCH 17/22] Fix the direction of the '--no-web-server' command line
 option

---
 openlp/core/api/http/server.py | 2 +-
 openlp/core/api/websockets.py  | 2 +-
 openlp/core/app.py             | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/openlp/core/api/http/server.py b/openlp/core/api/http/server.py
index 9d00bbbbb..914ce2278 100644
--- a/openlp/core/api/http/server.py
+++ b/openlp/core/api/http/server.py
@@ -87,7 +87,7 @@ class HttpServer(RegistryBase, RegistryProperties, LogMixin):
         Initialise the http server, and start the http server
         """
         super(HttpServer, self).__init__(parent)
-        if Registry().get_flag('no_web_server'):
+        if not Registry().get_flag('no_web_server'):
             worker = HttpWorker()
             run_thread(worker, 'http_server')
             Registry().register_function('download_website', self.first_time)
diff --git a/openlp/core/api/websockets.py b/openlp/core/api/websockets.py
index ea03c0681..d75905000 100644
--- a/openlp/core/api/websockets.py
+++ b/openlp/core/api/websockets.py
@@ -121,6 +121,6 @@ class WebSocketServer(RegistryProperties, LogMixin):
         Initialise and start the WebSockets server
         """
         super(WebSocketServer, self).__init__()
-        if Registry().get_flag('no_web_server'):
+        if not Registry().get_flag('no_web_server'):
             worker = WebSocketWorker()
             run_thread(worker, 'websocket_server')
diff --git a/openlp/core/app.py b/openlp/core/app.py
index 94ecdb450..9a1f42d9d 100644
--- a/openlp/core/app.py
+++ b/openlp/core/app.py
@@ -305,7 +305,7 @@ def parse_options(args=None):
     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.
@@ -358,7 +358,7 @@ def main(args=None):
     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

From 738e8e0283aaecdd9d0a27785e7ec9d0a5d4e2a4 Mon Sep 17 00:00:00 2001
From: Raoul Snyman <raoul@snyman.info>
Date: Sat, 6 Jan 2018 21:40:40 -0700
Subject: [PATCH 18/22] Remove the Qt style option - it never worked anyway

---
 openlp/core/app.py | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/openlp/core/app.py b/openlp/core/app.py
index 9a1f42d9d..b6b3a73f9 100644
--- a/openlp/core/app.py
+++ b/openlp/core/app.py
@@ -304,7 +304,6 @@ def parse_options(args=None):
                              '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_true',
                         help='Turn off the Web and Socket Server ')
     parser.add_argument('rargs', nargs='?', default=[])
@@ -343,8 +342,6 @@ def main(args=None):
         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

From 35fd553024add532c12ce3cef666f51e8c0d27a3 Mon Sep 17 00:00:00 2001
From: Raoul Snyman <raoul@snyman.info>
Date: Sat, 6 Jan 2018 21:45:13 -0700
Subject: [PATCH 19/22] If XDG is available, log files belong in the real cache
 dir

---
 openlp/core/common/applocation.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/openlp/core/common/applocation.py b/openlp/core/common/applocation.py
index a219e2906..14e6e4577 100644
--- a/openlp/core/common/applocation.py
+++ b/openlp/core/common/applocation.py
@@ -157,7 +157,7 @@ def _get_os_dir_path(dir_type):
                 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')

From 4fae7f829d33406cf89e098b8ee23d8ee6be26ee Mon Sep 17 00:00:00 2001
From: Raoul Snyman <raoul@snyman.info>
Date: Sat, 6 Jan 2018 22:24:55 -0700
Subject: [PATCH 20/22] Fix the tests I broke

---
 .../openlp_core/api/http/test_http.py         |  4 +--
 .../openlp_core/api/test_websockets.py        |  4 +--
 tests/functional/openlp_core/test_app.py      | 30 +++++++++++--------
 .../openlp_core/ui/test_mainwindow.py         | 17 ++++++-----
 .../openlp_core/ui/test_mainwindow.py         | 28 ++++++++---------
 5 files changed, 45 insertions(+), 38 deletions(-)

diff --git a/tests/functional/openlp_core/api/http/test_http.py b/tests/functional/openlp_core/api/http/test_http.py
index 4feb95526..257e0c6d1 100644
--- a/tests/functional/openlp_core/api/http/test_http.py
+++ b/tests/functional/openlp_core/api/http/test_http.py
@@ -49,7 +49,7 @@ class TestHttpServer(TestCase):
         """
         # GIVEN: A new httpserver
         # WHEN: I start the server
-        Registry().set_flag('no_web_server', True)
+        Registry().set_flag('no_web_server', False)
         HttpServer()
 
         # THEN: the api environment should have been created
@@ -64,7 +64,7 @@ class TestHttpServer(TestCase):
         """
         # GIVEN: A new httpserver
         # WHEN: I start the server
-        Registry().set_flag('no_web_server', False)
+        Registry().set_flag('no_web_server', True)
         HttpServer()
 
         # THEN: the api environment should have been created
diff --git a/tests/functional/openlp_core/api/test_websockets.py b/tests/functional/openlp_core/api/test_websockets.py
index 9e4ec381b..3d9bcf682 100644
--- a/tests/functional/openlp_core/api/test_websockets.py
+++ b/tests/functional/openlp_core/api/test_websockets.py
@@ -70,7 +70,7 @@ class TestWSServer(TestCase, TestMixin):
         """
         # 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
@@ -85,7 +85,7 @@ class TestWSServer(TestCase, TestMixin):
         """
         # 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
diff --git a/tests/functional/openlp_core/test_app.py b/tests/functional/openlp_core/test_app.py
index efce25a62..cd365c443 100644
--- a/tests/functional/openlp_core/test_app.py
+++ b/tests/functional/openlp_core/test_app.py
@@ -36,14 +36,15 @@ def test_parse_options_basic():
     """
     # 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 @@ def test_parse_options_debug():
     """
     # 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 @@ def test_parse_options_debug_and_portable():
     """
     # 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 @@ def test_parse_options_all_no_file():
     """
     # 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 @@ def test_parse_options_file():
     """
     # 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 @@ def test_parse_options_file_and_debug():
     """
     # 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'
 
 
diff --git a/tests/functional/openlp_core/ui/test_mainwindow.py b/tests/functional/openlp_core/ui/test_mainwindow.py
index 81b29a939..fb00b3522 100644
--- a/tests/functional/openlp_core/ui/test_mainwindow.py
+++ b/tests/functional/openlp_core/ui/test_mainwindow.py
@@ -60,9 +60,10 @@ class TestMainWindow(TestCase, TestMixin):
         # 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 @@ class TestMainWindow(TestCase, TestMixin):
         """
         Delete all the C++ objects and stop all the patchers
         """
-        self.add_toolbar_action_patcher.stop()
         del self.main_window
+        self.add_toolbar_action_patcher.stop()
 
     def test_cmd_line_file(self):
         """
@@ -92,20 +93,20 @@ class TestMainWindow(TestCase, TestMixin):
         # 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("")
+        # 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'
+        # 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):
         """
diff --git a/tests/interfaces/openlp_core/ui/test_mainwindow.py b/tests/interfaces/openlp_core/ui/test_mainwindow.py
index e48be9101..529408d2c 100644
--- a/tests/interfaces/openlp_core/ui/test_mainwindow.py
+++ b/tests/interfaces/openlp_core/ui/test_mainwindow.py
@@ -44,21 +44,21 @@ class TestMainWindow(TestCase, TestMixin):
         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):

From 7e99381dba15ce2663db8fe61a8a1376c156b681 Mon Sep 17 00:00:00 2001
From: Raoul Snyman <raoul@snyman.info>
Date: Sun, 7 Jan 2018 10:50:29 -0700
Subject: [PATCH 21/22] Fix some issues highlighted by Tim and Phill, and added
 a file that was erroneously removed

---
 openlp/core/api/websockets.py                 |  10 +-
 openlp/core/threading.py                      |   6 +-
 openlp/core/ui/mainwindow.py                  |   3 -
 .../functional/openlp_core/test_threading.py  |   2 +-
 .../openlp_core/ui/media/test_systemplayer.py | 549 ++++++++++++++++++
 5 files changed, 558 insertions(+), 12 deletions(-)
 create mode 100644 tests/functional/openlp_core/ui/media/test_systemplayer.py

diff --git a/openlp/core/api/websockets.py b/openlp/core/api/websockets.py
index d75905000..5528b4250 100644
--- a/openlp/core/api/websockets.py
+++ b/openlp/core/api/websockets.py
@@ -40,13 +40,13 @@ log = logging.getLogger(__name__)
 
 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
+    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
-    :return:
     """
     log.debug('WebSocket handler registered with client')
     previous_poll = None
@@ -88,7 +88,7 @@ class WebSocketWorker(ThreadWorker, RegistryProperties, LogMixin):
             try:
                 self.server = serve(handle_websocket, address, port)
                 log.debug('WebSocket server started on {addr}:{port}'.format(addr=address, port=port))
-            except Exception as e:
+            except Exception:
                 log.exception('Failed to start WebSocket server')
                 loop += 1
                 time.sleep(0.1)
diff --git a/openlp/core/threading.py b/openlp/core/threading.py
index b978b2c22..73e11cdf5 100644
--- a/openlp/core/threading.py
+++ b/openlp/core/threading.py
@@ -76,7 +76,7 @@ def get_thread_worker(thread_name):
     Get the worker by the thread name
 
     :param str thread_name: The name of the thread
-    :returns ThreadWorker: The worker for this thread name
+    :returns: The worker for this thread name
     """
     return Registry().get('main_window').threads.get(thread_name)
 
@@ -86,7 +86,7 @@ def is_thread_finished(thread_name):
     Check if a thread is finished running.
 
     :param str thread_name: The name of the thread
-    :returns bool: True if the thread is finished, False if it is still running
+    :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()
@@ -97,7 +97,7 @@ 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 function: A function which will remove the thread from the thread registry.
+    :returns: A function which will remove the thread from the thread registry.
     """
     def remove_thread():
         """
diff --git a/openlp/core/ui/mainwindow.py b/openlp/core/ui/mainwindow.py
index 79d354a90..54d91d36e 100644
--- a/openlp/core/ui/mainwindow.py
+++ b/openlp/core/ui/mainwindow.py
@@ -1073,9 +1073,6 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties):
 
         :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())
diff --git a/tests/functional/openlp_core/test_threading.py b/tests/functional/openlp_core/test_threading.py
index e7b628b8e..382774e51 100644
--- a/tests/functional/openlp_core/test_threading.py
+++ b/tests/functional/openlp_core/test_threading.py
@@ -4,7 +4,7 @@
 ###############################################################################
 # OpenLP - Open Source Lyrics Projection                                      #
 # --------------------------------------------------------------------------- #
-# Copyright (c) 2008-2017 OpenLP Developers                                   #
+# 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  #
diff --git a/tests/functional/openlp_core/ui/media/test_systemplayer.py b/tests/functional/openlp_core/ui/media/test_systemplayer.py
new file mode 100644
index 000000000..99c009e29
--- /dev/null
+++ b/tests/functional/openlp_core/ui/media/test_systemplayer.py
@@ -0,0 +1,549 @@
+# -*- 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

From c5b7b46a55bedf8fc5e058b90e8392d6227f1f75 Mon Sep 17 00:00:00 2001
From: Raoul Snyman <raoul@snyman.info>
Date: Sun, 7 Jan 2018 11:07:22 -0700
Subject: [PATCH 22/22] Fix the tests I now added back in

---
 .../openlp_core/ui/media/test_systemplayer.py | 30 ++++++++-----------
 1 file changed, 12 insertions(+), 18 deletions(-)

diff --git a/tests/functional/openlp_core/ui/media/test_systemplayer.py b/tests/functional/openlp_core/ui/media/test_systemplayer.py
index 99c009e29..fc03e2541 100644
--- a/tests/functional/openlp_core/ui/media/test_systemplayer.py
+++ b/tests/functional/openlp_core/ui/media/test_systemplayer.py
@@ -171,7 +171,7 @@ class TestSystemPlayer(TestCase):
 
         # WHEN: The load() method is run
         with patch.object(player, 'check_media') as mocked_check_media, \
-                patch.object(player, 'volume') as mocked_volume:
+                patch.object(player, 'volume'):
             mocked_check_media.return_value = False
             result = player.load(mocked_display)
 
@@ -461,8 +461,9 @@ class TestSystemPlayer(TestCase):
         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):
+    @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
         """
@@ -472,12 +473,8 @@ class TestSystemPlayer(TestCase):
         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_is_thread_finished.side_effect = [False, True]
         mocked_check_media_worker = MagicMock()
-        mocked_check_media_worker.play = 'play'
         mocked_check_media_worker.result = True
         MockCheckMediaWorker.return_value = mocked_check_media_worker
 
@@ -485,14 +482,11 @@ class TestSystemPlayer(TestCase):
         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_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
 
@@ -523,12 +517,12 @@ class TestCheckMediaWorker(TestCase):
 
         # WHEN: signals() is called with media and BufferedMedia
         with patch.object(worker, 'stop') as mocked_stop, \
-                patch.object(worker, 'finished') as mocked_finished:
+                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_finished.emit.assert_called_once_with()
+        mocked_quit.emit.assert_called_once_with()
         assert worker.result is True
 
     def test_signals_error(self):
@@ -540,10 +534,10 @@ class TestCheckMediaWorker(TestCase):
 
         # WHEN: signals() is called with error and BufferedMedia
         with patch.object(worker, 'stop') as mocked_stop, \
-                patch.object(worker, 'finished') as mocked_finished:
+                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_finished.emit.assert_called_once_with()
+        mocked_quit.emit.assert_called_once_with()
         assert worker.result is False