Merge lp:~raoul-snyman/openlp/bug-1742910 into lp:openlp

[Raoul Snyman]

Fix bug #1742910 by moving the threads to the application object instead of the main window object.

Add this to your merge proposal:
--------------------------------------------------------------------------------
lp:~raoul-snyman/openlp/bug-1742910 (revision 2810)
https://ci.openlp.io/job/Branch-01-Pull/2418/ [SUCCESS]
https://ci.openlp.io/job/Branch-02a-Linux-Tests/2319/ [SUCCESS]
https://ci.openlp.io/job/Branch-02b-macOS-Tests/114/ [SUCCESS]
https://ci.openlp.io/job/Branch-03a-Build-Source/36/ [SUCCESS]
https://ci.openlp.io/job/Branch-03b-Build-macOS/35/ [SUCCESS]
https://ci.openlp.io/job/Branch-04a-Code-Analysis/1498/ [SUCCESS]
https://ci.openlp.io/job/Branch-04b-Test-Coverage/1311/ [SUCCESS]
https://ci.openlp.io/job/Branch-05-AppVeyor-Tests/262/ [FAILURE]
Stopping after failure

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

[lp:~raoul-snyman/openlp/bug-1742910]

2812. By Raoul Snyman on 2018-01-13

Go back to the way I was doing it originally. Much more succint.

Unmerged revisions

=== modified file 'openlp/core/app.py'
--- openlp/core/app.py	2018-01-07 04:40:40 +0000
+++ openlp/core/app.py	2018-01-13 06:00:15 +0000
@@ -63,8 +63,8 @@
     The core application class. This class inherits from Qt's QApplication
     class in order to provide the core of the application.
     """
-
     args = []
+    worker_threads = {}
 
     def exec(self):
         """

=== modified file 'openlp/core/common/path.py'
--- openlp/core/common/path.py	2017-12-29 09:15:48 +0000
+++ openlp/core/common/path.py	2018-01-13 06:00:15 +0000
@@ -26,9 +26,9 @@
 from openlp.core.common import is_win
 
 if is_win():
-    from pathlib import WindowsPath as PathVariant
+    from pathlib import WindowsPath as PathVariant          # pragma: nocover
 else:
-    from pathlib import PosixPath as PathVariant
+    from pathlib import PosixPath as PathVariant            # pragma: nocover
 
 log = logging.getLogger(__name__)
 

=== modified file 'openlp/core/threading.py'
--- openlp/core/threading.py	2018-01-07 17:50:29 +0000
+++ openlp/core/threading.py	2018-01-13 06:00:15 +0000
@@ -50,12 +50,12 @@
     """
     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:
+    application = Registry().get('application')
+    if thread_name in application.worker_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()
-    main_window.threads[thread_name] = {
+    application.worker_threads[thread_name] = {
         'thread': thread,
         'worker': worker
     }
@@ -78,7 +78,10 @@
     :param str thread_name: The name of the thread
     :returns: The worker for this thread name
     """
-    return Registry().get('main_window').threads.get(thread_name)
+    thread_info = Registry().get('application').worker_threads.get(thread_name)
+    if not thread_info:
+        raise KeyError('No thread named "{}" exists'.format(thread_name))
+    return thread_info.get('worker')
 
 
 def is_thread_finished(thread_name):
@@ -88,8 +91,8 @@
     :param str thread_name: The name of the thread
     :returns: True if the thread is finished, False if it is still running
     """
-    main_window = Registry().get('main_window')
-    return thread_name not in main_window.threads or main_window.threads[thread_name]['thread'].isFinished()
+    app = Registry().get('application')
+    return thread_name not in app.worker_threads or app.worker_threads[thread_name]['thread'].isFinished()
 
 
 def make_remove_thread(thread_name):
@@ -99,13 +102,14 @@
     :param str thread_name: The name of the thread which should be removed from the thread registry.
     :returns: A function which will remove the thread from the thread registry.
     """
-    def remove_thread():
+
+    def remove_thread():                                                                        # pragma: nocover
         """
         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]
+        application = Registry().get('application')
+        if thread_name in application.worker_threads:
+            del application.worker_threads[thread_name]
     return remove_thread

=== modified file 'openlp/core/ui/mainwindow.py'
--- openlp/core/ui/mainwindow.py	2018-01-07 17:50:29 +0000
+++ openlp/core/ui/mainwindow.py	2018-01-13 06:00:15 +0000
@@ -477,7 +477,6 @@
         """
         super(MainWindow, self).__init__()
         Registry().register('main_window', self)
-        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).
@@ -557,11 +556,11 @@
         wait_dialog.setAutoClose(False)
         wait_dialog.setCancelButton(None)
         wait_dialog.show()
-        for thread_name in self.threads.keys():
+        for thread_name in self.application.worker_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']
+            thread = self.application.worker_threads[thread_name]['thread']
+            worker = self.application.worker_threads[thread_name]['worker']
             try:
                 if worker and hasattr(worker, 'stop'):
                     # If the worker has a stop method, run it

=== modified file 'tests/functional/openlp_core/api/http/test_error.py'
--- tests/functional/openlp_core/api/http/test_error.py	2017-12-29 09:15:48 +0000
+++ tests/functional/openlp_core/api/http/test_error.py	2018-01-13 06:00:15 +0000
@@ -22,38 +22,40 @@
 """
 Functional tests to test the API Error Class.
 """
-
-from unittest import TestCase
-
-from openlp.core.api.http.errors import NotFound, ServerError
-
-
-class TestApiError(TestCase):
-    """
-    A test suite to test out the Error in the API code
-    """
-    def test_not_found(self):
-        """
-        Test the Not Found error displays the correct information
-        """
-        # GIVEN:
-        # WHEN: I raise an exception
-        with self.assertRaises(Exception) as context:
-            raise NotFound()
-
-        # THEN: we get an error and a status
-        assert 'Not Found' == context.exception.message, 'A Not Found exception should be thrown'
-        assert 404 == context.exception.status, 'A 404 status should be thrown'
-
-    def test_server_error(self):
-        """
-        Test the server error displays the correct information
-        """
-        # GIVEN:
-        # WHEN: I raise an exception
-        with self.assertRaises(Exception) as context:
-            raise ServerError()
-
-        # THEN: we get an error and a status
-        assert'Server Error' == context.exception.message, 'A Not Found exception should be thrown'
-        assert 500 == context.exception.status, 'A 500 status should be thrown'
+from openlp.core.api.http.errors import HttpError, NotFound, ServerError
+
+
+def test_http_error():
+    """
+    Test the HTTPError class
+    """
+    # GIVEN: An HTTPError class
+    # WHEN: An instance is created
+    error = HttpError(400, 'Access Denied')
+
+    # THEN: The to_response() method should return the correct information
+    assert error.to_response() == ('Access Denied', 400), 'to_response() should have returned the correct info'
+
+
+def test_not_found():
+    """
+    Test the Not Found error displays the correct information
+    """
+    # GIVEN: A NotFound class
+    # WHEN: An instance is created
+    error = NotFound()
+
+    # THEN: The to_response() method should return the correct information
+    assert error.to_response() == ('Not Found', 404), 'to_response() should have returned the correct info'
+
+
+def test_server_error():
+    """
+    Test the server error displays the correct information
+    """
+    # GIVEN: A ServerError class
+    # WHEN: An instance of the class is created
+    error = ServerError()
+
+    # THEN: The to_response() method should return the correct information
+    assert error.to_response() == ('Server Error', 500), 'to_response() should have returned the correct info'

=== modified file 'tests/functional/openlp_core/api/test_deploy.py'
--- tests/functional/openlp_core/api/test_deploy.py	2017-12-29 09:15:48 +0000
+++ tests/functional/openlp_core/api/test_deploy.py	2018-01-13 06:00:15 +0000
@@ -21,11 +21,12 @@
 ###############################################################################
 from tempfile import mkdtemp
 from unittest import TestCase
-
-from openlp.core.api.deploy import deploy_zipfile
-from openlp.core.common.path import Path, copyfile
-
-TEST_PATH = (Path(__file__).parent / '..' / '..' / '..' / 'resources').resolve()
+from unittest.mock import MagicMock, patch
+
+from openlp.core.api.deploy import deploy_zipfile, download_sha256, download_and_check
+from openlp.core.common.path import Path
+
+CONFIG_FILE = '2c266badff1e3d140664c50fd1460a2b332b24d5ad8c267fa62e506b5eb6d894  deploy/site.zip\n2017_06_27'
 
 
 class TestRemoteDeploy(TestCase):
@@ -45,17 +46,95 @@
         """
         self.app_root_path.rmtree()
 
-    def test_deploy_zipfile(self):
+    @patch('openlp.core.api.deploy.ZipFile')
+    def test_deploy_zipfile(self, MockZipFile):
         """
         Remote Deploy tests - test the dummy zip file is processed correctly
         """
         # GIVEN: A new downloaded zip file
-        zip_path = TEST_PATH / 'remotes' / 'site.zip'
-        app_root_path = self.app_root_path / 'site.zip'
-        copyfile(zip_path, app_root_path)
-
-        # WHEN: I process the zipfile
-        deploy_zipfile(self.app_root_path, 'site.zip')
-
-        # THEN: test if www directory has been created
-        assert (self.app_root_path / 'www').is_dir(), 'We should have a www directory'
+        mocked_zipfile = MagicMock()
+        MockZipFile.return_value = mocked_zipfile
+        root_path = Path('/tmp/remotes')
+
+        # WHEN: deploy_zipfile() is called
+        deploy_zipfile(root_path, 'site.zip')
+
+        # THEN: the zip file should have been extracted to the right location
+        MockZipFile.assert_called_once_with('/tmp/remotes/site.zip')
+        mocked_zipfile.extractall.assert_called_once_with('/tmp/remotes')
+
+    @patch('openlp.core.api.deploy.Registry')
+    @patch('openlp.core.api.deploy.get_web_page')
+    def test_download_sha256_connection_error(self, mocked_get_web_page, MockRegistry):
+        """
+        Test that if a ConnectionError occurs while downloading a sha256 False is returned
+        """
+        # GIVEN: A bunch of mocks
+        MockRegistry.return_value.get.return_value.applicationVersion.return_value = '1.0'
+        mocked_get_web_page.side_effect = ConnectionError()
+
+        # WHEN: download_sha256() is called
+        result = download_sha256()
+
+        # THEN: The result should be False
+        assert result is False, 'download_sha256() should return False when encountering ConnectionError'
+
+    @patch('openlp.core.api.deploy.Registry')
+    @patch('openlp.core.api.deploy.get_web_page')
+    def test_download_sha256_no_config(self, mocked_get_web_page, MockRegistry):
+        """
+        Test that if there's no config when downloading a sha256 None is returned
+        """
+        # GIVEN: A bunch of mocks
+        MockRegistry.return_value.get.return_value.applicationVersion.return_value = '1.0'
+        mocked_get_web_page.return_value = None
+
+        # WHEN: download_sha256() is called
+        result = download_sha256()
+
+        # THEN: The result should be Nonw
+        assert result is None, 'download_sha256() should return None when there is a problem downloading the page'
+
+    @patch('openlp.core.api.deploy.Registry')
+    @patch('openlp.core.api.deploy.get_web_page')
+    def test_download_sha256(self, mocked_get_web_page, MockRegistry):
+        """
+        Test that the sha256 and the version are returned
+        """
+        # GIVEN: A bunch of mocks
+        MockRegistry.return_value.get.return_value.applicationVersion.return_value = '1.0'
+        mocked_get_web_page.return_value = CONFIG_FILE
+
+        # WHEN: download_sha256() is called
+        result = download_sha256()
+
+        # THEN: The result should be Nonw
+        assert result == ('2c266badff1e3d140664c50fd1460a2b332b24d5ad8c267fa62e506b5eb6d894', '2017_06_27'), \
+            'download_sha256() should return a tuple of sha256 and version'
+
+    @patch('openlp.core.api.deploy.Registry')
+    @patch('openlp.core.api.deploy.download_sha256')
+    @patch('openlp.core.api.deploy.get_url_file_size')
+    @patch('openlp.core.api.deploy.download_file')
+    @patch('openlp.core.api.deploy.AppLocation.get_section_data_path')
+    @patch('openlp.core.api.deploy.deploy_zipfile')
+    def test_download_and_check(self, mocked_deploy_zipfile, mocked_get_data_path, mocked_download_file,
+                                mocked_get_url_file_size, mocked_download_sha256, MockRegistry):
+        # GIVEN: A bunch of mocks
+        mocked_get_data_path.return_value = Path('/tmp/remotes')
+        mocked_download_file.return_value = True
+        mocked_get_url_file_size.return_value = 5
+        mocked_download_sha256.return_value = ('asdfgh', '0.1')
+        MockRegistry.return_value.get.return_value.applicationVersion.return_value = '1.0'
+        mocked_callback = MagicMock()
+
+        # WHEN: download_and_check() is called
+        download_and_check(mocked_callback)
+
+        # THEN: The correct things should have been done
+        mocked_download_sha256.assert_called_once_with()
+        mocked_get_url_file_size.assert_called_once_with('https://get.openlp.org/webclient/site.zip')
+        mocked_callback.setRange.assert_called_once_with(0, 5)
+        mocked_download_file.assert_called_once_with(mocked_callback, 'https://get.openlp.org/webclient/site.zip',
+                                                     Path('/tmp/remotes/site.zip'), sha256='asdfgh')
+        mocked_deploy_zipfile.assert_called_once_with(Path('/tmp/remotes'), 'site.zip')

=== modified file 'tests/functional/openlp_core/common/test_path.py'
--- tests/functional/openlp_core/common/test_path.py	2017-12-29 09:15:48 +0000
+++ tests/functional/openlp_core/common/test_path.py	2018-01-13 06:00:15 +0000
@@ -27,7 +27,7 @@
 from unittest.mock import ANY, MagicMock, patch
 
 from openlp.core.common.path import Path, copy, copyfile, copytree, create_paths, path_to_str, replace_params, \
-    str_to_path, which
+    str_to_path, which, files_to_paths
 
 
 class TestShutil(TestCase):
@@ -401,3 +401,16 @@
         except:
             # THEN: `create_paths` raises an exception
             pass
+
+    def test_files_to_paths(self):
+        """
+        Test the files_to_paths() method
+        """
+        # GIVEN: A list of string filenames
+        test_files = ['/tmp/openlp/file1.txt', '/tmp/openlp/file2.txt']
+
+        # WHEN: files_to_paths is called
+        result = files_to_paths(test_files)
+
+        # THEN: The result should be a list of Paths
+        assert result == [Path('/tmp/openlp/file1.txt'), Path('/tmp/openlp/file2.txt')]

=== removed file 'tests/functional/openlp_core/lib/test_path.py'
--- tests/functional/openlp_core/lib/test_path.py	2017-12-23 09:09:45 +0000
+++ tests/functional/openlp_core/lib/test_path.py	1970-01-01 00:00:00 +0000
@@ -1,87 +0,0 @@
-# -*- coding: utf-8 -*-
-# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4
-
-###############################################################################
-# OpenLP - Open Source Lyrics Projection                                      #
-# --------------------------------------------------------------------------- #
-# Copyright (c) 2008-2017 OpenLP Developers                                   #
-# --------------------------------------------------------------------------- #
-# This program is free software; you can redistribute it and/or modify it     #
-# under the terms of the GNU General Public License as published by the Free  #
-# Software Foundation; version 2 of the License.                              #
-#                                                                             #
-# This program is distributed in the hope that it will be useful, but WITHOUT #
-# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or       #
-# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for    #
-# more details.                                                               #
-#                                                                             #
-# You should have received a copy of the GNU General Public License along     #
-# with this program; if not, write to the Free Software Foundation, Inc., 59  #
-# Temple Place, Suite 330, Boston, MA 02111-1307 USA                          #
-###############################################################################
-"""
-Package to test the openlp.core.lib.path package.
-"""
-import os
-from unittest import TestCase
-
-from openlp.core.common.path import Path, path_to_str, str_to_path
-
-
-class TestPath(TestCase):
-    """
-    Tests for the :mod:`openlp.core.lib.path` module
-    """
-
-    def test_path_to_str_type_error(self):
-        """
-        Test that `path_to_str` raises a type error when called with an invalid type
-        """
-        # GIVEN: The `path_to_str` function
-        # WHEN: Calling `path_to_str` with an invalid Type
-        # THEN: A TypeError should have been raised
-        with self.assertRaises(TypeError):
-            path_to_str(str())
-
-    def test_path_to_str_none(self):
-        """
-        Test that `path_to_str` correctly converts the path parameter when passed with None
-        """
-        # GIVEN: The `path_to_str` function
-        # WHEN: Calling the `path_to_str` function with None
-        result = path_to_str(None)
-
-        # THEN: `path_to_str` should return an empty string
-        assert result == ''
-
-    def test_path_to_str_path_object(self):
-        """
-        Test that `path_to_str` correctly converts the path parameter when passed a Path object
-        """
-        # GIVEN: The `path_to_str` function
-        # WHEN: Calling the `path_to_str` function with a Path object
-        result = path_to_str(Path('test/path'))
-
-        # THEN: `path_to_str` should return a string representation of the Path object
-        assert result == os.path.join('test', 'path')
-
-    def test_str_to_path_type_error(self):
-        """
-        Test that `str_to_path` raises a type error when called with an invalid type
-        """
-        # GIVEN: The `str_to_path` function
-        # WHEN: Calling `str_to_path` with an invalid Type
-        # THEN: A TypeError should have been raised
-        with self.assertRaises(TypeError):
-            str_to_path(Path())
-
-    def test_str_to_path_empty_str(self):
-        """
-        Test that `str_to_path` correctly converts the string parameter when passed with and empty string
-        """
-        # GIVEN: The `str_to_path` function
-        # WHEN: Calling the `str_to_path` function with None
-        result = str_to_path('')
-
-        # THEN: `path_to_str` should return None
-        assert result is None

=== modified file 'tests/functional/openlp_core/lib/test_ui.py'
--- tests/functional/openlp_core/lib/test_ui.py	2017-12-17 20:19:19 +0000
+++ tests/functional/openlp_core/lib/test_ui.py	2018-01-13 06:00:15 +0000
@@ -23,14 +23,14 @@
 Package to test the openlp.core.lib.ui package.
 """
 from unittest import TestCase
-from unittest.mock import MagicMock, patch
+from unittest.mock import MagicMock, patch, call
 
 from PyQt5 import QtCore, QtGui, QtWidgets
 
 from openlp.core.common.i18n import UiStrings, translate
 from openlp.core.lib.ui import add_welcome_page, create_button_box, create_horizontal_adjusting_combo_box, \
     create_button, create_action, create_valign_selection_widgets, find_and_set_in_combo_box, create_widget_action, \
-    set_case_insensitive_completer
+    set_case_insensitive_completer, critical_error_message_box
 
 
 class TestUi(TestCase):
@@ -80,6 +80,34 @@
         assert 1 == len(btnbox.buttons())
         assert QtWidgets.QDialogButtonBox.HelpRole, btnbox.buttonRole(btnbox.buttons()[0])
 
+    @patch('openlp.core.lib.ui.Registry')
+    def test_critical_error_message_box(self, MockRegistry):
+        """
+        Test the critical_error_message_box() function
+        """
+        # GIVEN: A mocked Registry
+        # WHEN: critical_error_message_box() is called
+        critical_error_message_box('Error', 'This is an error')
+
+        # THEN: The error_message() method on the main window should be called
+        MockRegistry.return_value.get.return_value.error_message.assert_called_once_with('Error', 'This is an error')
+
+    @patch('openlp.core.lib.ui.QtWidgets.QMessageBox.critical')
+    def test_critical_error_question(self, mocked_critical):
+        """
+        Test the critical_error_message_box() function
+        """
+        # GIVEN: A mocked critical() method and some other mocks
+        mocked_parent = MagicMock()
+
+        # WHEN: critical_error_message_box() is called
+        critical_error_message_box(None, 'This is a question', mocked_parent, True)
+
+        # THEN: The error_message() method on the main window should be called
+        mocked_critical.assert_called_once_with(mocked_parent, 'Error', 'This is a question',
+                                                QtWidgets.QMessageBox.StandardButtons(QtWidgets.QMessageBox.Yes |
+                                                                                      QtWidgets.QMessageBox.No))
+
     def test_create_horizontal_adjusting_combo_box(self):
         """
         Test creating a horizontal adjusting combo box
@@ -92,65 +120,64 @@
 
         # THEN: We should get a ComboBox
         assert isinstance(combo, QtWidgets.QComboBox)
-        assert 'combo1' == combo.objectName()
+        assert combo.objectName() == 'combo1'
         assert QtWidgets.QComboBox.AdjustToMinimumContentsLength == combo.sizeAdjustPolicy()
 
-    def test_create_button(self):
+    @patch('openlp.core.lib.ui.log')
+    def test_create_button(self, mocked_log):
         """
         Test creating a button
         """
         # GIVEN: A dialog
         dialog = QtWidgets.QDialog()
 
-        # WHEN: We create the button
-        btn = create_button(dialog, 'my_btn')
-
-        # THEN: We should get a button with a name
-        assert isinstance(btn, QtWidgets.QPushButton)
-        assert 'my_btn' == btn.objectName()
-        assert btn.isEnabled() is True
-
         # WHEN: We create a button with some attributes
-        btn = create_button(dialog, 'my_btn', text='Hello', tooltip='How are you?', enabled=False)
+        btn = create_button(dialog, 'my_btn', text='Hello', tooltip='How are you?', enabled=False, role='test', test=1)
 
         # THEN: We should get a button with those attributes
         assert isinstance(btn, QtWidgets.QPushButton)
-        assert 'Hello' == btn.text()
-        assert 'How are you?' == btn.toolTip()
+        assert btn.objectName() == 'my_btn'
+        assert btn.text() == 'Hello'
+        assert btn.toolTip() == 'How are you?'
         assert btn.isEnabled() is False
+        assert mocked_log.warning.call_args_list == [call('The role "test" is not defined in create_push_button().'),
+                                                     call('Parameter test was not consumed in create_button().')]
+
+    def test_create_tool_button(self):
+        """
+        Test creating a toolbutton
+        """
+        # GIVEN: A dialog
+        dialog = QtWidgets.QDialog()
 
         # WHEN: We create a toolbutton
         btn = create_button(dialog, 'my_btn', btn_class='toolbutton')
 
         # THEN: We should get a toolbutton
         assert isinstance(btn, QtWidgets.QToolButton)
-        assert 'my_btn' == btn.objectName()
+        assert btn.objectName() == 'my_btn'
         assert btn.isEnabled() is True
 
-    def test_create_action(self):
+    @patch('openlp.core.lib.ui.log')
+    def test_create_action(self, mocked_log):
         """
         Test creating an action
         """
         # GIVEN: A dialog
         dialog = QtWidgets.QDialog()
 
-        # WHEN: We create an action
-        action = create_action(dialog, 'my_action')
-
-        # THEN: We should get a QAction
-        assert isinstance(action, QtWidgets.QAction)
-        assert 'my_action' == action.objectName()
-
         # WHEN: We create an action with some properties
         action = create_action(dialog, 'my_action', text='my text', icon=':/wizards/wizard_firsttime.bmp',
-                               tooltip='my tooltip', statustip='my statustip')
+                               tooltip='my tooltip', statustip='my statustip', test=1)
 
         # THEN: These properties should be set
         assert isinstance(action, QtWidgets.QAction)
-        assert 'my text' == action.text()
+        assert action.objectName() == 'my_action'
+        assert action.text() == 'my text'
         assert isinstance(action.icon(), QtGui.QIcon)
-        assert 'my tooltip' == action.toolTip()
-        assert 'my statustip' == action.statusTip()
+        assert action.toolTip() == 'my tooltip'
+        assert action.statusTip() == 'my statustip'
+        mocked_log.warning.assert_called_once_with('Parameter test was not consumed in create_action().')
 
     def test_create_action_on_mac_osx(self):
         """

=== modified file 'tests/functional/openlp_core/test_threading.py'
--- tests/functional/openlp_core/test_threading.py	2018-01-07 17:50:29 +0000
+++ tests/functional/openlp_core/test_threading.py	2018-01-13 06:00:15 +0000
@@ -22,9 +22,10 @@
 """
 Package to test the openlp.core.threading package.
 """
+from inspect import isfunction
 from unittest.mock import MagicMock, call, patch
 
-from openlp.core.version import run_thread
+from openlp.core.threading import ThreadWorker, run_thread, get_thread_worker, is_thread_finished, make_remove_thread
 
 
 def test_run_thread_no_name():
@@ -47,9 +48,9 @@
     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
+    mocked_application = MagicMock()
+    mocked_application.worker_threads = {'test_thread': MagicMock()}
+    MockRegistry.return_value.get.return_value = mocked_application
 
     # WHEN: run_thread() is called
     try:
@@ -66,18 +67,19 @@
     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
+    mocked_application = MagicMock()
+    mocked_application.worker_threads = {}
+    MockRegistry.return_value.get.return_value = mocked_application
 
     # 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']
+    assert len(mocked_application.worker_threads.keys()) == 1, 'There should be 1 item in the list of threads'
+    assert list(mocked_application.worker_threads.keys()) == ['test_thread'], \
+        'The test_thread item should be in the list'
+    mocked_worker = mocked_application.worker_threads['test_thread']['worker']
+    mocked_thread = mocked_application.worker_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)]
@@ -87,3 +89,103 @@
         '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()
+
+
+def test_thread_worker():
+    """
+    Test that creating a thread worker object and calling start throws and NotImplementedError
+    """
+    # GIVEN: A ThreadWorker class
+    worker = ThreadWorker()
+
+    try:
+        # WHEN: calling start()
+        worker.start()
+        assert False, 'A NotImplementedError should have been thrown'
+    except NotImplementedError:
+        # A NotImplementedError should be thrown
+        pass
+    except Exception:
+        assert False, 'A NotImplementedError should have been thrown'
+
+
+@patch('openlp.core.threading.Registry')
+def test_get_thread_worker(MockRegistry):
+    """
+    Test that calling the get_thread_worker() function returns the correct worker
+    """
+    # GIVEN: A mocked thread worker
+    mocked_worker = MagicMock()
+    MockRegistry.return_value.get.return_value.worker_threads = {'test_thread': {'worker': mocked_worker}}
+
+    # WHEN: get_thread_worker() is called
+    worker = get_thread_worker('test_thread')
+
+    # THEN: The mocked worker is returned
+    assert worker is mocked_worker, 'The mocked worker should have been returned'
+
+
+@patch('openlp.core.threading.Registry')
+def test_get_thread_worker_mising(MockRegistry):
+    """
+    Test that calling the get_thread_worker() function raises a KeyError if it does not exist
+    """
+    # GIVEN: A mocked thread worker
+    MockRegistry.return_value.get.return_value.worker_threads = {}
+
+    try:
+        # WHEN: get_thread_worker() is called
+        get_thread_worker('test_thread')
+        assert False, 'A KeyError should have been raised'
+    except KeyError:
+        # THEN: The mocked worker is returned
+        pass
+    except Exception:
+        assert False, 'A KeyError should have been raised'
+
+
+@patch('openlp.core.threading.Registry')
+def test_is_thread_finished(MockRegistry):
+    """
+    Test the is_thread_finished() function
+    """
+    # GIVEN: A mock thread and worker
+    mocked_thread = MagicMock()
+    mocked_thread.isFinished.return_value = False
+    MockRegistry.return_value.get.return_value.worker_threads = {'test': {'thread': mocked_thread}}
+
+    # WHEN: is_thread_finished() is called
+    result = is_thread_finished('test')
+
+    # THEN: The result should be correct
+    assert result is False, 'is_thread_finished should have returned False'
+
+
+@patch('openlp.core.threading.Registry')
+def test_is_thread_finished_missing(MockRegistry):
+    """
+    Test that calling the is_thread_finished() function returns True if the thread doesn't exist
+    """
+    # GIVEN: A mocked thread worker
+    MockRegistry.return_value.get.return_value.worker_threads = {}
+
+    # WHEN: get_thread_worker() is called
+    result = is_thread_finished('test_thread')
+
+    # THEN: The result should be correct
+    assert result is True, 'is_thread_finished should return True when a thread is missing'
+
+
+def test_make_remove_thread():
+    """
+    Test the make_remove_thread() function
+    """
+    # GIVEN: A thread name
+    thread_name = 'test_thread'
+
+    # WHEN: make_remove_thread() is called
+    rm_func = make_remove_thread(thread_name)
+
+    # THEN: The result should be a function
+    assert isfunction(rm_func), 'make_remove_thread should return a function'
+    assert rm_func.__name__ == 'remove_thread'