Merge branch 'master' of gitlab.com:openlp/openlp

openlp/core/display/render.py

@@ -486,6 +486,24 @@ class ThemePreviewRenderer(LogMixin, DisplayWindow):
             footer_html = 'Dummy footer text'
         return footer_html
 
+    def wait_till_loaded(self):
+        """
+        Wait until web engine page loaded
+        :return boolean: True on success, False on timeout
+        """
+        # Timeout in 10 seconds
+        end_time = time.time() + 10
+        app = Registry().get('application')
+        success = True
+        while not self._is_initialised:
+            if time.time() > end_time:
+                log.error('Timed out waiting for web engine page to load')
+                success = False
+                break
+            time.sleep(0.1)
+            app.process_events()
+        return success
+
     def _wait_and_process(self, delay):
         """
         Wait while allowing things to process
@@ -510,7 +528,6 @@ class ThemePreviewRenderer(LogMixin, DisplayWindow):
         self.force_page = force_page
         if not self.force_page:
             self.set_theme(theme_data, is_sync=True)
-            self.theme_height = theme_data.font_main_height
             slides = self.format_slide(VERSE, None)
             verses = dict()
             verses['title'] = TITLE
@@ -525,26 +542,6 @@ class ThemePreviewRenderer(LogMixin, DisplayWindow):
         self.force_page = False
         return None
 
-    def get_theme(self, item):
-        """
-        :param item: The :class:`~openlp.core.lib.serviceitem.ServiceItem` item object
-        :return string: The name of the theme to be used
-
-        """
-        # Just assume we use the global theme.
-        theme_name = Registry().get('theme_manager').global_theme
-        # The theme level is either set to Service or Item. Use the service theme if one is set. We also have to use the
-        # service theme, even when the theme level is set to Item, because the item does not necessarily have to have a
-        # theme.
-        if self.theme_level != ThemeLevel.Global:
-            # When the theme level is at Service and we actually have a service theme then use it.
-            if self.theme_level == ThemeLevel.Service:
-                theme_name = Registry().get('service_manager').service_theme
-        # If we have Item level and have an item theme then use it.
-        if self.theme_level == ThemeLevel.Song and item.theme:
-            theme_name = item.theme
-        return theme_name
-
     def format_slide(self, text, item):
         """
         Calculate how much text can fit on a slide.
@@ -557,11 +554,8 @@ class ThemePreviewRenderer(LogMixin, DisplayWindow):
             QtWidgets.QApplication.instance().processEvents()
         self.log_debug('format slide')
         if item:
-            theme_name = self.get_theme(item)
-            theme_data = Registry().get('theme_manager').get_theme_data(theme_name)
-            self.theme_height = theme_data.font_main_height
             # Set theme for preview
-            self.set_theme(theme_data)
+            self.set_theme(item.get_theme_data(self.theme_level))
         # Add line endings after each line of text used for bibles.
         line_end = '<br>'
         if item and item.is_capable(ItemCapabilities.NoLineBreaks):
@@ -821,7 +815,6 @@ class Renderer(RegistryBase, RegistryProperties, ThemePreviewRenderer):
         # If the display is not show'ed and hidden like this webegine will not render
         self.show()
         self.hide()
-        self.theme_height = 0
         self.theme_level = ThemeLevel.Global
 
     def set_theme_level(self, theme_level):

openlp/core/display/window.py

@@ -402,7 +402,9 @@ class DisplayWindow(QtWidgets.QWidget):
         Set the HTML scale
         """
         self.scale = scale
-        self.run_javascript('Display.setScale({scale});'.format(scale=scale * 100))
+        # Only scale if initialised (scale run again once initialised)
+        if self._is_initialised:
+            self.run_javascript('Display.setScale({scale});'.format(scale=scale * 100))
 
     def alert(self, text, settings):
         """

openlp/core/lib/serviceitem.py

@@ -33,11 +33,12 @@ from pathlib import Path
 from PyQt5 import QtGui
 
 from openlp.core.state import State
-from openlp.core.common import md5_hash
+from openlp.core.common import ThemeLevel, md5_hash
 from openlp.core.common.applocation import AppLocation
 from openlp.core.common.i18n import translate
 from openlp.core.common.mixins import RegistryProperties
 from openlp.core.common.settings import Settings
+from openlp.core.common.registry import Registry
 from openlp.core.display.render import remove_tags, render_tags, render_chords_for_printing
 from openlp.core.lib import ItemCapabilities
 from openlp.core.ui.icons import UiIcons
@@ -91,7 +92,6 @@ class ServiceItem(RegistryProperties):
         self.capabilities = []
         self.is_valid = True
         self.icon = None
-        self.theme_data = None
         self.main = None
         self.footer = None
         self.bg_image_bytes = None
@@ -115,6 +115,29 @@ class ServiceItem(RegistryProperties):
         self._new_item()
         self.metadata = []
 
+    def get_theme_data(self, theme_level=None):
+        """
+        Get the theme appropriate for this item
+
+        :param theme_level: The theme_level to use,
+                            the value in Settings is used when this value is missinig
+        """
+        if theme_level is None:
+            theme_level = Settings().value('themes/theme level')
+        theme_manager = Registry().get('theme_manager')
+        # Just assume we use the global theme.
+        theme = theme_manager.global_theme
+        if theme_level != ThemeLevel.Global:
+            service_theme = Settings().value('servicemanager/service theme')
+            # Service or Song level, so assume service theme (if it exists and item in service)
+            # but use song theme if level is song (and it exists)
+            if service_theme and self.from_service:
+                theme = service_theme
+            if theme_level == ThemeLevel.Song and self.theme:
+                theme = self.theme
+        theme = theme_manager.get_theme_data(theme)
+        return theme
+
     def _new_item(self):
         """
         Method to set the internal id of the item. This is used to compare service items to see if they are the same.

openlp/core/ui/firsttimeform.py

@@ -246,7 +246,7 @@ class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties):
             self.alert_check_box.setChecked(self.plugin_manager.get_plugin_by_name('alerts').is_active())
             # Add any existing themes to list.
             self.theme_combo_box.insertSeparator(0)
-            self.theme_combo_box.addItems(sorted(self.theme_manager.get_themes()))
+            self.theme_combo_box.addItems(sorted(self.theme_manager.get_theme_names()))
             default_theme = Settings().value('themes/global theme')
             # Pre-select the current default theme.
             index = self.theme_combo_box.findText(default_theme)
@@ -351,7 +351,7 @@ class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties):
         """
         existing_themes = []
         if self.theme_manager:
-            existing_themes = self.theme_manager.get_themes()
+            existing_themes = self.theme_manager.get_theme_names()
         for list_index in range(self.themes_list_widget.count()):
             item = self.themes_list_widget.item(list_index)
             if item.text() not in existing_themes:

openlp/core/ui/media/__init__.py

@@ -108,7 +108,7 @@ def format_milliseconds(milliseconds):
     """
     Format milliseconds into a human readable time string.
     :param milliseconds: Milliseconds to format
-    :return: Time string in format: hh.mm.ss,ttt
+    :return: Time string in format: hh:mm:ss,ttt
     """
     milliseconds = int(milliseconds)
     seconds, millis = divmod(milliseconds, 1000)

openlp/core/ui/slidecontroller.py

@@ -858,9 +858,7 @@ class SlideController(QtWidgets.QWidget, LogMixin, RegistryProperties):
                 [self.service_item, self.is_live, self.hide_mode(), slide_no])
         else:
             # Get theme
-            # TODO this is wrong!!!
+            theme_data = service_item.get_theme_data()
-            theme_name = service_item.theme if service_item.theme else Registry().get('theme_manager').global_theme
-            theme_data = Registry().get('theme_manager').get_theme_data(theme_name)
             # Set theme for preview
             self.preview_display.set_theme(theme_data)
             # Set theme for displays

openlp/core/ui/thememanager.py

@@ -141,9 +141,23 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R
         super(ThemeManager, self).__init__(parent)
         self.settings_section = 'themes'
         # Variables
-        self.theme_list = []
+        self._theme_list = {}
         self.old_background_image_path = None
 
+    def get_global_theme(self):
+        return self.get_theme_data(self.global_theme)
+
+    def get_theme_data(self, theme_name):
+        """
+        Gets a theme given a name, returns the default theme if missing
+        """
+        theme = Theme()
+        if theme_name in self._theme_list:
+            theme = self._theme_list[theme_name]
+        else:
+            self.log_warning('Missing requested theme data for "{}", using default theme'.format(theme_name))
+        return theme
+
     def bootstrap_initialise(self):
         """
         process the bootstrap initialise setup request
@@ -375,7 +389,7 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R
 
         :param theme: The theme to delete.
         """
-        self.theme_list.remove(theme)
+        self._theme_list.pop(theme)
         thumb = '{name}.png'.format(name=theme)
         delete_file(self.theme_path / thumb)
         delete_file(self.thumb_path / thumb)
@@ -491,9 +505,9 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R
         events for the plugins.
         The plugins will call back in to get the real list if they want it.
         """
-        self.theme_list = []
+        self._theme_list.clear()
         self.theme_list_widget.clear()
-        files = AppLocation.get_files(self.settings_section, '.png')
+        files = AppLocation.get_files(self.settings_section, '/*.json')
         # Sort the themes by its name considering language specific
         files.sort(key=lambda file_name: get_locale_key(str(file_name)))
         # now process the file list of png files
@@ -515,22 +529,22 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R
                 item_name.setIcon(icon)
                 item_name.setData(QtCore.Qt.UserRole, text_name)
                 self.theme_list_widget.addItem(item_name)
-                self.theme_list.append(text_name)
+                self._theme_list[text_name] = self._get_theme_data(text_name)
         self._push_themes()
 
     def _push_themes(self):
         """
         Notify listeners that the theme list has been updated
         """
-        Registry().execute('theme_update_list', self.get_themes())
+        Registry().execute('theme_update_list', self.get_theme_names())
 
-    def get_themes(self):
+    def get_theme_names(self):
         """
         Return the list of loaded themes
         """
-        return self.theme_list
+        return [theme_name for theme_name in self._theme_list]
 
-    def get_theme_data(self, theme_name):
+    def _get_theme_data(self, theme_name):
         """
         Returns a theme object from a JSON file
 
@@ -699,15 +713,18 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R
         thumb_path = self.thumb_path / '{name}.png'.format(name=theme_name)
         create_thumb(sample_path_name, thumb_path, False)
 
-    def update_preview_images(self, theme_list=None):
+    def update_preview_images(self, theme_name_list=None):
         """
         Called to update the themes' preview images.
+
+        :param theme_name_list: A list of theme names in the theme data folder
         """
-        theme_list = theme_list or self.theme_list
+        theme_name_list = theme_name_list or self.get_theme_names()
-        self.progress_form.theme_list = theme_list
+        self.progress_form.theme_list = theme_name_list
         self.progress_form.show()
-        for theme_name in theme_list:
+        self.progress_form.theme_display.wait_till_loaded()
-            theme_data = self.get_theme_data(theme_name)
+        for theme_name in theme_name_list:
+            theme_data = self._get_theme_data(theme_name)
             preview_pixmap = self.progress_form.get_preview(theme_name, theme_data)
             self.save_preview(theme_name, preview_pixmap)
         self.progress_form.close()

tests/functional/openlp_core/display/test_render.py

@@ -21,20 +21,10 @@
 """
 Test the :mod:`~openlp.core.display.render` package.
 """
-import sys
+from unittest.mock import patch
 
-from unittest import TestCase
-from unittest.mock import MagicMock, patch
-
-from openlp.core.common import ThemeLevel
-from tests.helpers.testmixin import TestMixin
-
-from PyQt5 import QtWidgets
-sys.modules['PyQt5.QtWebEngineWidgets'] = MagicMock()
-
-from openlp.core.common.registry import Registry
 from openlp.core.display.render import compare_chord_lyric_width, find_formatting_tags, remove_tags, render_chords, \
-    render_chords_for_printing, render_tags, ThemePreviewRenderer
+    render_chords_for_printing, render_tags
 from openlp.core.lib.formattingtags import FormattingTags
 
 
@@ -217,101 +207,3 @@ def test_find_formatting_tags():
 
     # THEN: The list of active tags should contain only 'st'
     assert active_tags == ['st'], 'The list of active tags should contain only "st"'
-
-
-class TestThemePreviewRenderer(TestMixin, TestCase):
-
-    def setUp(self):
-        """
-        Set up the components need for all tests.
-        """
-        # Create the Registry
-        self.application = QtWidgets.QApplication.instance()
-        Registry.create()
-        self.application.setOrganizationName('OpenLP-tests')
-        self.application.setOrganizationDomain('openlp.org')
-
-    def test_get_theme_global(self):
-        """
-        Test the return of the global theme if set to Global level
-        """
-        # GIVEN: A set up with a Global Theme and settings at Global
-        mocked_theme_manager = MagicMock()
-        mocked_theme_manager.global_theme = 'my_global_theme'
-        Registry().register('theme_manager', mocked_theme_manager)
-        with patch('openlp.core.display.webengine.WebEngineView'), \
-                patch('PyQt5.QtWidgets.QVBoxLayout'):
-            tpr = ThemePreviewRenderer()
-            tpr.theme_level = ThemeLevel.Global
-        # WHEN: I Request the theme to Use
-            theme = tpr.get_theme(MagicMock())
-
-        # THEN: The list of active tags should contain only 'st'
-            assert theme == mocked_theme_manager.global_theme, 'The Theme returned is not that of the global theme'
-
-    def test_get_theme_service(self):
-        """
-        Test the return of the global theme if set to Global level
-        """
-        # GIVEN: A set up with a Global Theme and settings at Global
-        mocked_theme_manager = MagicMock()
-        mocked_theme_manager.global_theme = 'my_global_theme'
-        mocked_service_manager = MagicMock()
-        mocked_service_manager.service_theme = 'my_service_theme'
-        Registry().register('theme_manager', mocked_theme_manager)
-        Registry().register('service_manager', mocked_service_manager)
-        with patch('openlp.core.display.webengine.WebEngineView'), \
-                patch('PyQt5.QtWidgets.QVBoxLayout'):
-            tpr = ThemePreviewRenderer()
-            tpr.theme_level = ThemeLevel.Service
-        # WHEN: I Request the theme to Use
-            theme = tpr.get_theme(MagicMock())
-
-        # THEN: The list of active tags should contain only 'st'
-            assert theme == mocked_service_manager.service_theme, 'The Theme returned is not that of the Service theme'
-
-    def test_get_theme_item_level_none(self):
-        """
-        Test the return of the global theme if set to Global level
-        """
-        # GIVEN: A set up with a Global Theme and settings at Global
-        mocked_theme_manager = MagicMock()
-        mocked_theme_manager.global_theme = 'my_global_theme'
-        mocked_service_manager = MagicMock()
-        mocked_service_manager.service_theme = 'my_service_theme'
-        mocked_item = MagicMock()
-        mocked_item.theme = None
-        Registry().register('theme_manager', mocked_theme_manager)
-        Registry().register('service_manager', mocked_service_manager)
-        with patch('openlp.core.display.webengine.WebEngineView'), \
-                patch('PyQt5.QtWidgets.QVBoxLayout'):
-            tpr = ThemePreviewRenderer()
-            tpr.theme_level = ThemeLevel.Song
-        # WHEN: I Request the theme to Use
-            theme = tpr.get_theme(mocked_item)
-
-        # THEN: The list of active tags should contain only 'st'
-            assert theme == mocked_theme_manager.global_theme, 'The Theme returned is not that of the global theme'
-
-    def test_get_theme_item_level_set(self):
-        """
-        Test the return of the global theme if set to Global level
-        """
-        # GIVEN: A set up with a Global Theme and settings at Global
-        mocked_theme_manager = MagicMock()
-        mocked_theme_manager.global_theme = 'my_global_theme'
-        mocked_service_manager = MagicMock()
-        mocked_service_manager.service_theme = 'my_service_theme'
-        mocked_item = MagicMock()
-        mocked_item.theme = "my_item_theme"
-        Registry().register('theme_manager', mocked_theme_manager)
-        Registry().register('service_manager', mocked_service_manager)
-        with patch('openlp.core.display.webengine.WebEngineView'), \
-                patch('PyQt5.QtWidgets.QVBoxLayout'):
-            tpr = ThemePreviewRenderer()
-            tpr.theme_level = ThemeLevel.Song
-        # WHEN: I Request the theme to Use
-            theme = tpr.get_theme(mocked_item)
-
-        # THEN: The list of active tags should contain only 'st'
-            assert theme == mocked_item.theme, 'The Theme returned is not that of the item theme'

tests/functional/openlp_core/display/test_window.py

@@ -74,3 +74,33 @@ class TestDisplayWindow(TestCase, TestMixin):
         # THEN: The x11 override flag should not be set
         x11_bit = display_window.windowFlags() & QtCore.Qt.X11BypassWindowManagerHint
         assert x11_bit != QtCore.Qt.X11BypassWindowManagerHint
+
+    def test_set_scale_not_initialised(self, MockSettings, mocked_webengine, mocked_addWidget):
+        """
+        Test that the scale js is not run if the page is not initialised
+        """
+        # GIVEN: A display window not yet initialised
+        display_window = DisplayWindow()
+        display_window._is_initialised = False
+        display_window.run_javascript = MagicMock()
+
+        # WHEN: set scale is run
+        display_window.set_scale(0.5)
+
+        # THEN: javascript should not be run
+        display_window.run_javascript.assert_not_called()
+
+    def test_set_scale_initialised(self, MockSettings, mocked_webengine, mocked_addWidget):
+        """
+        Test that the scale js is not run if the page is not initialised
+        """
+        # GIVEN: A display window not yet initialised
+        display_window = DisplayWindow()
+        display_window._is_initialised = True
+        display_window.run_javascript = MagicMock()
+
+        # WHEN: set scale is run
+        display_window.set_scale(0.5)
+
+        # THEN: javascript should not be run
+        display_window.run_javascript.assert_called_once_with('Display.setScale(50.0);')

tests/functional/openlp_core/lib/test_serviceitem.py

@@ -24,10 +24,10 @@ Package to test the openlp.core.lib package.
 import os
 from pathlib import Path
 from unittest import TestCase
-from unittest.mock import MagicMock, patch
+from unittest.mock import Mock, MagicMock, patch
 
 from openlp.core.state import State
-from openlp.core.common import md5_hash
+from openlp.core.common import ThemeLevel, md5_hash
 from openlp.core.common.registry import Registry
 from openlp.core.common.settings import Settings
 from openlp.core.lib.formattingtags import FormattingTags
@@ -342,3 +342,125 @@ class TestServiceItem(TestCase, TestMixin):
             '"’Twas grace that taught my hea" has been returned as the title'
         assert Path('/test/amazing_grace.mp3') == service_item.background_audio[0], \
             '"/test/amazing_grace.mp3" should be in the background_audio list'
+
+    def test_service_item_get_theme_data_global_level(self):
+        """
+        Test the service item - get theme data when set to global theme level
+        """
+        # GIVEN: A service item with a theme and theme level set to global
+        service_item = ServiceItem(None)
+        service_item.theme = 'song_theme'
+        mocked_theme_manager = MagicMock()
+        mocked_theme_manager.global_theme = 'global_theme'
+        mocked_theme_manager.get_theme_data = Mock(side_effect=lambda value: value)
+        Registry().register('theme_manager', mocked_theme_manager)
+        Settings().setValue('servicemanager/service theme', 'service_theme')
+        Settings().setValue('themes/theme level', ThemeLevel.Global)
+
+        # WHEN: Get theme data is run
+        theme = service_item.get_theme_data()
+
+        # THEN: theme should be the global theme
+        assert theme == mocked_theme_manager.global_theme
+
+    def test_service_item_get_theme_data_service_level_service_undefined(self):
+        """
+        Test the service item - get theme data when set to service theme level
+        """
+        # GIVEN: A service item with a theme and theme level set to service
+        service_item = ServiceItem(None)
+        service_item.theme = 'song_theme'
+        mocked_theme_manager = MagicMock()
+        mocked_theme_manager.global_theme = 'global_theme'
+        mocked_theme_manager.get_theme_data = Mock(side_effect=lambda value: value)
+        Registry().register('theme_manager', mocked_theme_manager)
+        Settings().setValue('servicemanager/service theme', 'service_theme')
+        Settings().setValue('themes/theme level', ThemeLevel.Service)
+
+        # WHEN: Get theme data is run
+        theme = service_item.get_theme_data()
+
+        # THEN: theme should be the global theme
+        assert theme == mocked_theme_manager.global_theme
+
+    def test_service_item_get_theme_data_service_level_service_defined(self):
+        """
+        Test the service item - get theme data when set to service theme level
+        """
+        # GIVEN: A service item with a theme and theme level set to service
+        service_item = ServiceItem(None)
+        service_item.theme = 'song_theme'
+        service_item.from_service = True
+        mocked_theme_manager = MagicMock()
+        mocked_theme_manager.global_theme = 'global_theme'
+        mocked_theme_manager.get_theme_data = Mock(side_effect=lambda value: value)
+        Registry().register('theme_manager', mocked_theme_manager)
+        Settings().setValue('servicemanager/service theme', 'service_theme')
+        Settings().setValue('themes/theme level', ThemeLevel.Service)
+
+        # WHEN: Get theme data is run
+        theme = service_item.get_theme_data()
+
+        # THEN: theme should be the service theme
+        assert theme == Settings().value('servicemanager/service theme')
+
+    def test_service_item_get_theme_data_song_level(self):
+        """
+        Test the service item - get theme data when set to song theme level
+        """
+        # GIVEN: A service item with a theme and theme level set to song
+        service_item = ServiceItem(None)
+        service_item.theme = 'song_theme'
+        mocked_theme_manager = MagicMock()
+        mocked_theme_manager.global_theme = 'global_theme'
+        mocked_theme_manager.get_theme_data = Mock(side_effect=lambda value: value)
+        Registry().register('theme_manager', mocked_theme_manager)
+        Settings().setValue('servicemanager/service theme', 'service_theme')
+        Settings().setValue('themes/theme level', ThemeLevel.Song)
+
+        # WHEN: Get theme data is run
+        theme = service_item.get_theme_data()
+
+        # THEN: theme should be the song theme
+        assert theme == service_item.theme
+
+    def test_service_item_get_theme_data_song_level_service_fallback(self):
+        """
+        Test the service item - get theme data when set to song theme level
+                                but the song theme doesn't exist
+        """
+        # GIVEN: A service item with a theme and theme level set to song
+        service_item = ServiceItem(None)
+        service_item.from_service = True
+        mocked_theme_manager = MagicMock()
+        mocked_theme_manager.global_theme = 'global_theme'
+        mocked_theme_manager.get_theme_data = Mock(side_effect=lambda value: value)
+        Registry().register('theme_manager', mocked_theme_manager)
+        Settings().setValue('servicemanager/service theme', 'service_theme')
+        Settings().setValue('themes/theme level', ThemeLevel.Song)
+
+        # WHEN: Get theme data is run
+        theme = service_item.get_theme_data()
+
+        # THEN: theme should be the serice theme
+        assert theme == Settings().value('servicemanager/service theme')
+
+    def test_service_item_get_theme_data_song_level_global_fallback(self):
+        """
+        Test the service item - get theme data when set to song theme level
+                                but the song and service theme don't exist
+        """
+        # GIVEN: A service item with a theme and theme level set to song
+        service_item = ServiceItem(None)
+        mocked_theme_manager = MagicMock()
+        mocked_theme_manager.global_theme = 'global_theme'
+        mocked_theme_manager.get_theme_data = Mock(side_effect=lambda value: value)
+        Registry().register('theme_manager', mocked_theme_manager)
+        Settings().setValue('servicemanager/service theme', 'service_theme')
+        Settings().setValue('themes/theme level', ThemeLevel.Song)
+
+        # WHEN: Get theme data is run
+        theme = service_item.get_theme_data()
+
+        # THEN: theme should be the global theme
+        assert theme == mocked_theme_manager.global_theme

tests/functional/openlp_core/ui/media/test_media.py

@@ -22,6 +22,7 @@
 Package to test the openlp.core.ui package.
 """
 from openlp.core.ui.media import parse_optical_path
+from openlp.core.ui.media import format_milliseconds
 
 
 def test_parse_optical_path_linux():
@@ -80,3 +81,18 @@ def test_parse_optical_path_win():
     assert org_end == end, 'Returned end should match the original'
     assert org_name == name, 'Returned end should match the original'
     assert org_device_path == device_path, 'Returned device_path should match the original'
+
+
+def test_format_milliseconds():
+    """
+    Test that format_milliseconds(...) makes an expected human readable time string
+    """
+
+    # GIVEN: 200 million milliseconds (Revelation 9:16)
+    num_soldiers_on_horses_as_milliseconds = 200 * 1000 * 1000
+
+    # WHEN: converting milliseconds to human readable string
+    num_soldiers_on_horses_as_formatted_time_string = format_milliseconds(num_soldiers_on_horses_as_milliseconds)
+
+    # THEN: The formatted time string should be 55 hours, 33 minutes, 20 seconds, and 000 milliseconds
+    assert num_soldiers_on_horses_as_formatted_time_string == "55:33:20,000"

tests/functional/openlp_core/ui/test_firsttimeform.py

@@ -197,7 +197,7 @@ class TestFirstTimeForm(TestCase, TestMixin):
                 patch('openlp.core.ui.firsttimeform.create_paths') as mocked_create_paths, \
                 patch.object(frw.application, 'set_normal_cursor'):
             mocked_plugin_manager = MagicMock()
-            mocked_theme_manager = MagicMock(**{'get_themes.return_value': ['b', 'a', 'c']})
+            mocked_theme_manager = MagicMock(**{'get_theme_names.return_value': ['b', 'a', 'c']})
             Registry().register('plugin_manager', mocked_plugin_manager)
             Registry().register('theme_manager', mocked_theme_manager)
 
@@ -212,7 +212,7 @@ class TestFirstTimeForm(TestCase, TestMixin):
             mocked_settings.value.assert_has_calls([call('core/has run wizard'), call('themes/global theme')])
             mocked_gettempdir.assert_called_once()
             mocked_create_paths.assert_called_once_with(Path('temp', 'openlp'))
-            mocked_theme_manager.get_themes.assert_called_once()
+            mocked_theme_manager.get_theme_names.assert_called_once()
             mocked_theme_combo_box.clear.assert_called_once()
             mocked_plugin_manager.get_plugin_by_name.assert_has_calls(
                 [call('songs'), call('bibles'), call('presentations'), call('images'), call('media'), call('custom'),

tests/functional/openlp_core/ui/test_thememanager.py

@@ -26,7 +26,7 @@ import shutil
 from pathlib import Path
 from tempfile import mkdtemp
 from unittest import TestCase
-from unittest.mock import ANY, MagicMock, patch, call
+from unittest.mock import ANY, Mock, MagicMock, patch, call
 
 from PyQt5 import QtWidgets
 
@@ -353,12 +353,14 @@ class TestThemeManager(TestCase):
         Test that the update_preview_images() method works correctly
         """
         # GIVEN: A ThemeManager
+        def get_theme_data(value):
+            return '{}_theme_data'.format(value)
         theme_manager = ThemeManager(None)
         theme_manager.save_preview = MagicMock()
-        theme_manager.get_theme_data = MagicMock(return_value='theme_data')
+        theme_manager._get_theme_data = Mock(side_effect=get_theme_data)
         theme_manager.progress_form = MagicMock(**{'get_preview.return_value': 'preview'})
         theme_manager.load_themes = MagicMock()
-        theme_list = ['Default', 'Test']
+        theme_list = {'Default': get_theme_data('Default'), 'Test': get_theme_data('Test')}
 
         # WHEN: ThemeManager.update_preview_images() is called
         theme_manager.update_preview_images(theme_list)
@@ -366,9 +368,8 @@ class TestThemeManager(TestCase):
         # THEN: Things should work right
         assert theme_manager.progress_form.theme_list == theme_list
         theme_manager.progress_form.show.assert_called_once_with()
-        assert theme_manager.get_theme_data.call_args_list == [call('Default'), call('Test')]
+        assert theme_manager.progress_form.get_preview.call_args_list == [call('Default', get_theme_data('Default')),
-        assert theme_manager.progress_form.get_preview.call_args_list == [call('Default', 'theme_data'),
+                                                                          call('Test', get_theme_data('Test'))]
-                                                                          call('Test', 'theme_data')]
         assert theme_manager.save_preview.call_args_list == [call('Default', 'preview'), call('Test', 'preview')]
         theme_manager.progress_form.close.assert_called_once_with()
         theme_manager.load_themes.assert_called_once_with()

tests/interfaces/openlp_plugins/bibles/test_lib_parse_reference.py

@@ -70,58 +70,58 @@ class TestBibleManager(TestCase, TestMixin):
         del self.manager
         self.destroy_settings()
 
-    def test_parse_reference_one(self):
+    def test_parse_reference_numbered_book_single_chapter_no_verse_reference(self):
         """
         Test the parse_reference method with 1 Timothy 1
         """
         # GIVEN given a bible in the bible manager
         # WHEN asking to parse the bible reference
         results = parse_reference('1 Timothy 1', self.manager.db_cache['tests'], MagicMock(), 54)
-        # THEN a verse array should be returned
+        # THEN a one tuple verse array should be returned
-        assert [(54, 1, 1, -1)] == results, "The bible verses should matches the expected results"
+        assert [(54, 1, 1, -1)] == results, "The bible verses should match the expected results"
 
-    def test_parse_reference_two(self):
+    def test_parse_reference_numbered_book_single_range_single_chapter_multiple_verses(self):
         """
         Test the parse_reference method with 1 Timothy 1:1-2
         """
         # GIVEN given a bible in the bible manager
         # WHEN asking to parse the bible reference
         results = parse_reference('1 Timothy 1:1-2', self.manager.db_cache['tests'], MagicMock(), 54)
-        # THEN a verse array should be returned
+        # THEN a one tuple verse array should be returned
-        assert [(54, 1, 1, 2)] == results, "The bible verses should matches the expected results"
+        assert [(54, 1, 1, 2)] == results, "The bible verses should match the expected results"
 
-    def test_parse_reference_three(self):
+    def test_parse_reference_numbered_book_single_range_multiple_chapters_specific_verses(self):
         """
         Test the parse_reference method with 1 Timothy 1:1-2
         """
         # GIVEN given a bible in the bible manager
         # WHEN asking to parse the bible reference
         results = parse_reference('1 Timothy 1:1-2:1', self.manager.db_cache['tests'], MagicMock(), 54)
-        # THEN a verse array should be returned
+        # THEN a two tuple verse array should be returned
         assert [(54, 1, 1, -1), (54, 2, 1, 1)] == results, \
             "The bible verses should match the expected results"
 
-    def test_parse_reference_four(self):
+    def test_parse_reference_invalid_book(self):
         """
-        Test the parse_reference method with non existence book
+        Test the parse_reference method with non existent book
         """
         # GIVEN given a bible in the bible manager
         # WHEN asking to parse the bible reference
         results = parse_reference('Raoul 1', self.manager.db_cache['tests'], MagicMock())
-        # THEN a verse array should be returned
+        # THEN an empty verse array should be returned
-        assert [] == results, "The bible Search should return an empty list"
+        assert [] == results, "The bible search should return an empty list"
 
-    def test_parse_reference_five(self):
+    def test_parse_reference_numbered_book_single_range_single_chapter_with_end_reference(self):
         """
         Test the parse_reference method with 1 Timothy 1:3-end
         """
         # GIVEN given a bible in the bible manager
         # WHEN asking to parse the bible reference
         results = parse_reference('1 Timothy 1:3-end', self.manager.db_cache['tests'], MagicMock(), 54)
-        # THEN a verse array should be returned
+        # THEN a one tuple verse array should be returned
-        assert [(54, 1, 3, -1)] == results, "The bible verses should matches the expected results"
+        assert [(54, 1, 3, -1)] == results, "The bible verses should match the expected results"
 
-    def test_parse_reference_six(self):
+    def test_parse_reference_numbered_book_single_range_single_chapter_with_end_reference_no_bible_ref_id(self):
         """
         Test the parse_reference method with 1 Timothy 1:3-end without a bible ref id to match
         how the GUI does the search.  This is logged in issue #282
@@ -129,5 +129,66 @@ class TestBibleManager(TestCase, TestMixin):
         # GIVEN given a bible in the bible manager
         # WHEN asking to parse the bible reference in Language 0 (english)
         results = parse_reference('1 Timothy 1:3-end', self.manager.db_cache['tests'], 0)
-        # THEN a verse array should be returned
+        # THEN a one tuple verse array should be returned
-        assert [(54, 1, 3, -1)] == results, "The bible verses should matches the expected results"
+        assert [(54, 1, 3, -1)] == results, "The bible verses should match the expected results"
+
+    def test_parse_reference_book_ref_id_invalid(self):
+        """
+        Test the parse_reference method with 1 Timothy 1:1 with an invalid bible ref id
+        """
+        # GIVEN given a bible in the bible manager
+        # WHEN asking to parse the bible reference with an invalid bible reference id
+        results = parse_reference('1 Timothy 1:1', self.manager.db_cache['tests'], MagicMock(), -666)
+        # THEN an empty verse array should be returned
+        assert [] == results, "The bible verse list should be empty"
+
+    def test_parse_reference_no_from_chapter_in_second_range(self):
+        """
+        Test the parse_reference method with 1 Timothy 1:1,3
+        """
+        # GIVEN given a bible in the bible manager
+        # WHEN asking to parse the bible reference that has no from_chapter in the second range
+        results = parse_reference('1 Timothy 1:1,3', self.manager.db_cache['tests'], MagicMock(), 54)
+        # THEN a two tuple verse array should be returned
+        assert [(54, 1, 1, 1), (54, 1, 3, 3)] == results, "The bible verses should match the expected results"
+
+    def test_parse_reference_to_chapter_less_than_from_chapter(self):
+        """
+        Test the parse_reference method with 1 Timothy 2:1-1:1
+        """
+        # GIVEN given a bible in the bible manager
+        # WHEN asking to parse the bible reference with a to_chapter less than the from_chapter
+        results = parse_reference('1 Timothy 2:1-1:1', self.manager.db_cache['tests'], MagicMock(), 54)
+        # THEN an empty verse array should be returned
+        assert [] == results, "The bible verse list should be empty"
+
+    def test_parse_reference_no_from_chapter_specified(self):
+        """
+        Test the parse_reference method with 1 Timothy :1-2
+        """
+        # GIVEN given a bible in the bible manager
+        # WHEN asking to parse the bible reference with no from_chapter specified
+        results = parse_reference('1 Timothy :1-2', self.manager.db_cache['tests'], MagicMock(), 54)
+        # THEN a two tuple verse array should be returned with the bible verse references treated as chapter references
+        assert [(54, 1, 1, -1), (54, 2, 1, -1)] == results, "The bible verses should match the expected results"
+
+    def test_parse_reference_three_chapters(self):
+        """
+        Test the parse_reference method with 1 Timothy 1-3
+        """
+        # GIVEN given a bible in the bible manager
+        # WHEN asking to parse the bible reference with three chapters
+        results = parse_reference('1 Timothy 1-3', self.manager.db_cache['tests'], MagicMock(), 54)
+        # THEN a three tuple verse array should be returned
+        assert [(54, 1, 1, -1), (54, 2, 1, -1), (54, 3, 1, -1)] == results, \
+            "The bible verses should match the expected results"
+
+    def test_parse_reference_non_regexp_matching_reference(self):
+        """
+        Test the parse_reference method with 1 Timothy
+        """
+        # GIVEN given a bible in the bible manager
+        # WHEN asking to parse the bible reference that fails the regexp matching
+        results = parse_reference('1 Timothy', self.manager.db_cache['tests'], MagicMock(), 54)
+        # THEN an empty verse array should be returned
+        assert [] == results, "The bible verse list should be empty"

tests/interfaces/openlp_plugins/songs/forms/test_songmaintenanceform.py

@@ -232,7 +232,6 @@ class TestSongMaintenanceForm(TestCase, TestMixin):
         expected_widget_item_calls = [call('John Wesley'), call('John Newton')]
         mocked_authors_list_widget.clear.assert_called_once_with()
         self.mocked_manager.get_all_objects.assert_called_once_with(MockedAuthor)
-        # former third argument for below not needed anymore: MockedQListWidgetItem.call_args_list
         self.assertCountEqual(MockedQListWidgetItem.call_args_list, expected_widget_item_calls)
         mocked_author_item1.setData.assert_called_once_with(QtCore.Qt.UserRole, 2)
         mocked_author_item2.setData.assert_called_once_with(QtCore.Qt.UserRole, 1)