Updated various thread usages

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

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.
         """

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
 

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

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

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'

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

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

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

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