From 5935bbe85011dee9fa6399d38a277e97d70fe0b3 Mon Sep 17 00:00:00 2001 From: Ee Savior <2930438-eSavior@users.noreply.gitlab.com> Date: Mon, 2 Dec 2019 18:02:13 +0000 Subject: [PATCH 1/4] core-ui-media-__init__: Create test and fix docstring error for format_milliseconds(...). Achieve 100% coverage for __init__.py module --- openlp/core/ui/media/__init__.py | 2 +- .../openlp_core/ui/media/test_media.py | 16 ++++++++++++++++ .../songs/forms/test_songmaintenanceform.py | 1 - 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/openlp/core/ui/media/__init__.py b/openlp/core/ui/media/__init__.py index 52662c7f6..1524b161a 100644 --- a/openlp/core/ui/media/__init__.py +++ b/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) diff --git a/tests/functional/openlp_core/ui/media/test_media.py b/tests/functional/openlp_core/ui/media/test_media.py index 733969c28..1b70c3a29 100644 --- a/tests/functional/openlp_core/ui/media/test_media.py +++ b/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" diff --git a/tests/interfaces/openlp_plugins/songs/forms/test_songmaintenanceform.py b/tests/interfaces/openlp_plugins/songs/forms/test_songmaintenanceform.py index 4fb36618a..6647c07cb 100644 --- a/tests/interfaces/openlp_plugins/songs/forms/test_songmaintenanceform.py +++ b/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) From f8ae9cf7199fbe21bad3c95839179fb212430fae Mon Sep 17 00:00:00 2001 From: Daniel Date: Mon, 2 Dec 2019 18:07:41 +0000 Subject: [PATCH 2/4] Change theme_list in theme manager to a dictionary containing the theme objects This prevents the need to read the theme file every time it's data is needed. --- openlp/core/display/render.py | 27 +--- openlp/core/lib/serviceitem.py | 27 +++- openlp/core/ui/firsttimeform.py | 4 +- openlp/core/ui/slidecontroller.py | 4 +- openlp/core/ui/thememanager.py | 44 ++++-- .../openlp_core/display/test_render.py | 112 +--------------- .../openlp_core/lib/test_serviceitem.py | 126 +++++++++++++++++- .../openlp_core/ui/test_firsttimeform.py | 4 +- .../openlp_core/ui/test_thememanager.py | 13 +- 9 files changed, 194 insertions(+), 167 deletions(-) diff --git a/openlp/core/display/render.py b/openlp/core/display/render.py index 22222de5b..5552a620c 100644 --- a/openlp/core/display/render.py +++ b/openlp/core/display/render.py @@ -510,7 +510,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 +524,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 +536,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 = '
' if item and item.is_capable(ItemCapabilities.NoLineBreaks): @@ -821,7 +797,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): diff --git a/openlp/core/lib/serviceitem.py b/openlp/core/lib/serviceitem.py index 9ff4176e0..a9bc61511 100644 --- a/openlp/core/lib/serviceitem.py +++ b/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. diff --git a/openlp/core/ui/firsttimeform.py b/openlp/core/ui/firsttimeform.py index a81e8dd7c..9106e555a 100644 --- a/openlp/core/ui/firsttimeform.py +++ b/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: diff --git a/openlp/core/ui/slidecontroller.py b/openlp/core/ui/slidecontroller.py index e0ea58010..f9a88ec71 100644 --- a/openlp/core/ui/slidecontroller.py +++ b/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_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) + theme_data = service_item.get_theme_data() # Set theme for preview self.preview_display.set_theme(theme_data) # Set theme for displays diff --git a/openlp/core/ui/thememanager.py b/openlp/core/ui/thememanager.py index e2d3e2c68..6710ff507 100644 --- a/openlp/core/ui/thememanager.py +++ b/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,17 @@ 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 - self.progress_form.theme_list = theme_list + theme_name_list = theme_name_list or self.get_theme_names() + self.progress_form.theme_list = theme_name_list self.progress_form.show() - for theme_name in theme_list: - 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() diff --git a/tests/functional/openlp_core/display/test_render.py b/tests/functional/openlp_core/display/test_render.py index 2f44127c6..7efc750de 100644 --- a/tests/functional/openlp_core/display/test_render.py +++ b/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' diff --git a/tests/functional/openlp_core/lib/test_serviceitem.py b/tests/functional/openlp_core/lib/test_serviceitem.py index 10cc46058..6e1b7d846 100644 --- a/tests/functional/openlp_core/lib/test_serviceitem.py +++ b/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 diff --git a/tests/functional/openlp_core/ui/test_firsttimeform.py b/tests/functional/openlp_core/ui/test_firsttimeform.py index 4bca63533..32c9abc0a 100644 --- a/tests/functional/openlp_core/ui/test_firsttimeform.py +++ b/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'), diff --git a/tests/functional/openlp_core/ui/test_thememanager.py b/tests/functional/openlp_core/ui/test_thememanager.py index b7aae7c2c..028e3c7e8 100644 --- a/tests/functional/openlp_core/ui/test_thememanager.py +++ b/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', 'theme_data'), - call('Test', 'theme_data')] + assert theme_manager.progress_form.get_preview.call_args_list == [call('Default', get_theme_data('Default')), + call('Test', get_theme_data('Test'))] 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() From 7c4d9a5ffde2cd870869f5aeb8fb84b3cff77a51 Mon Sep 17 00:00:00 2001 From: Daniel Date: Tue, 3 Dec 2019 17:30:35 +0000 Subject: [PATCH 3/4] Time out after 10 seconds in wait till loaded fn also fixes comment, update_preview_images() does not handle the timeout case but in reality it should never happen. (just in the case it does, it won't stall the whole application) --- openlp/core/display/render.py | 18 +++++++++++ openlp/core/display/window.py | 4 ++- openlp/core/ui/thememanager.py | 1 + .../openlp_core/display/test_window.py | 30 +++++++++++++++++++ 4 files changed, 52 insertions(+), 1 deletion(-) diff --git a/openlp/core/display/render.py b/openlp/core/display/render.py index 5552a620c..f71aa2693 100644 --- a/openlp/core/display/render.py +++ b/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 diff --git a/openlp/core/display/window.py b/openlp/core/display/window.py index 52c1a8fc3..ac15c1ade 100644 --- a/openlp/core/display/window.py +++ b/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): """ diff --git a/openlp/core/ui/thememanager.py b/openlp/core/ui/thememanager.py index 6710ff507..22a158fb4 100644 --- a/openlp/core/ui/thememanager.py +++ b/openlp/core/ui/thememanager.py @@ -722,6 +722,7 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R theme_name_list = theme_name_list or self.get_theme_names() self.progress_form.theme_list = theme_name_list self.progress_form.show() + self.progress_form.theme_display.wait_till_loaded() 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) diff --git a/tests/functional/openlp_core/display/test_window.py b/tests/functional/openlp_core/display/test_window.py index df1b8aded..180195c2c 100644 --- a/tests/functional/openlp_core/display/test_window.py +++ b/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);') From c3e6d3f7767f45e036bf8e5904dcaef07d680f17 Mon Sep 17 00:00:00 2001 From: Ee Savior <2930438-eSavior@users.noreply.gitlab.com> Date: Tue, 3 Dec 2019 19:54:57 +0000 Subject: [PATCH 4/4] Test improvements for bible ref parsing --- .../bibles/test_lib_parse_reference.py | 97 +++++++++++++++---- 1 file changed, 79 insertions(+), 18 deletions(-) diff --git a/tests/interfaces/openlp_plugins/bibles/test_lib_parse_reference.py b/tests/interfaces/openlp_plugins/bibles/test_lib_parse_reference.py index 62d8dcbeb..cc3935f36 100644 --- a/tests/interfaces/openlp_plugins/bibles/test_lib_parse_reference.py +++ b/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 - assert [(54, 1, 1, -1)] == results, "The bible verses should matches the expected results" + # THEN a one tuple verse array should be returned + 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 - assert [(54, 1, 1, 2)] == results, "The bible verses should matches the expected results" + # THEN a one tuple verse array should be returned + 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 - assert [] == results, "The bible Search should return an empty list" + # THEN an empty verse array should be returned + 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 - assert [(54, 1, 3, -1)] == results, "The bible verses should matches the expected results" + # THEN a one tuple verse array should be returned + 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 - assert [(54, 1, 3, -1)] == results, "The bible verses should matches the expected results" + # THEN a one tuple verse array should be returned + 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"