From 68f37e635ab0422c8898ffdb76c37656d58465f6 Mon Sep 17 00:00:00 2001 From: Raoul Snyman Date: Wed, 18 Dec 2019 07:51:04 -0700 Subject: [PATCH] Fix exception thrown due to a form not existing yet - Fix an issue where an exception was thrown because the theme progress form didn't exist yet - Refactor a few things - Fix other tests - Add a test for wait_for --- openlp/core/common/utils.py | 49 ++++++++++++++ openlp/core/display/html/display.js | 2 + openlp/core/display/render.py | 38 ++--------- openlp/core/display/window.py | 67 ++++++++++++------- openlp/core/ui/thememanager.py | 13 +++- openlp/core/ui/themeprogressform.py | 8 ++- .../openlp_core/ui/test_thememanager.py | 22 ++++-- tests/js/fake_webchannel.js | 7 +- tests/openlp_core/common/test_utils.py | 44 ++++++++++++ 9 files changed, 182 insertions(+), 68 deletions(-) create mode 100644 openlp/core/common/utils.py create mode 100644 tests/openlp_core/common/test_utils.py diff --git a/openlp/core/common/utils.py b/openlp/core/common/utils.py new file mode 100644 index 000000000..5d0c0a0bc --- /dev/null +++ b/openlp/core/common/utils.py @@ -0,0 +1,49 @@ +# -*- coding: utf-8 -*- + +########################################################################## +# OpenLP - Open Source Lyrics Projection # +# ---------------------------------------------------------------------- # +# Copyright (c) 2008-2019 OpenLP Developers # +# ---------------------------------------------------------------------- # +# This program is free software: you can redistribute it and/or modify # +# it under the terms of the GNU General Public License as published by # +# the Free Software Foundation, either version 3 of the License, or # +# (at your option) any later version. # +# # +# This program is distributed in the hope that it will be useful, # +# but WITHOUT ANY WARRANTY; without even the implied warranty of # +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the # +# GNU General Public License for more details. # +# # +# You should have received a copy of the GNU General Public License # +# along with this program. If not, see . # +########################################################################## +""" +The :mod:`~openlp.core.display.window` module contains the display window +""" +import logging +import time + +from openlp.core.common.registry import Registry + +log = logging.getLogger(__name__) + + +def wait_for(check_func, error_message='Timed out waiting for completion', timeout=10): + """ + Wait until web engine page loaded + :return boolean: True on success, False on timeout + """ + # Timeout in 10 seconds + end_time = time.time() + timeout + app = Registry().get('application') + success = True + while not check_func(): + now = time.time() + if now > end_time: + log.error(error_message) + success = False + break + time.sleep(0.001) + app.process_events() + return success diff --git a/openlp/core/display/html/display.js b/openlp/core/display/html/display.js index 44ca1374b..86e0a288a 100644 --- a/openlp/core/display/html/display.js +++ b/openlp/core/display/html/display.js @@ -383,6 +383,7 @@ var Display = { init: function (doTransitions=false) { Display._doTransitions = doTransitions; Reveal.initialize(Display._revealConfig); + displayWatcher.setInitialised(true); }, /** * Reinitialise Reveal @@ -1134,4 +1135,5 @@ var Display = { }; new QWebChannel(qt.webChannelTransport, function (channel) { window.mediaWatcher = channel.objects.mediaWatcher; + window.displayWatcher = channel.objects.displayWatcher; }); diff --git a/openlp/core/display/render.py b/openlp/core/display/render.py index 423ee9cb8..32ef0b467 100644 --- a/openlp/core/display/render.py +++ b/openlp/core/display/render.py @@ -36,6 +36,7 @@ from openlp.core.common.i18n import translate from openlp.core.common.mixins import LogMixin from openlp.core.common.registry import Registry, RegistryBase from openlp.core.common.settings import Settings +from openlp.core.common.utils import wait_for from openlp.core.display.screens import ScreenList from openlp.core.display.window import DisplayWindow from openlp.core.lib import ItemCapabilities @@ -486,35 +487,6 @@ 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 - - :param delay: The amount of time in seconds to delay, can be a float - """ - end_time = time.time() + delay - app = Registry().get('application') - while time.time() < end_time: - app.process_events() - def generate_preview(self, theme_data, force_page=False, generate_screenshot=True): """ Generate a preview of a theme. @@ -535,7 +507,8 @@ class ThemePreviewRenderer(LogMixin, DisplayWindow): verses['verse'] = 'V1' verses['footer'] = self.generate_footer() self.load_verses([verses], is_sync=True) - self._wait_and_process(1) + # Wait for a second + wait_for(lambda: False, timeout=1) self.force_page = False if generate_screenshot: return self.grab() @@ -550,8 +523,7 @@ class ThemePreviewRenderer(LogMixin, DisplayWindow): :param item: The :class:`~openlp.core.lib.serviceitem.ServiceItem` item object. """ - while not self._is_initialised: - QtWidgets.QApplication.instance().processEvents() + wait_for(lambda: self._is_initialised) self.log_debug('format slide') if item: # Set theme for preview @@ -771,8 +743,10 @@ class ThemePreviewRenderer(LogMixin, DisplayWindow): :param text: The text to check. It may contain HTML tags. """ self.clear_slides() + self.log_debug('_text_fits_on_slide: 1\n{text}'.format(text=text)) self.run_javascript('Display.addTextSlide("v1", "{text}", "Dummy Footer");' .format(text=text.replace('"', '\\"')), is_sync=True) + self.log_debug('_text_fits_on_slide: 2') does_text_fits = self.run_javascript('Display.doesContentFit();', is_sync=True) return does_text_fits diff --git a/openlp/core/display/window.py b/openlp/core/display/window.py index 1fec3921d..9ac6f6ea1 100644 --- a/openlp/core/display/window.py +++ b/openlp/core/display/window.py @@ -25,18 +25,18 @@ import json import logging import os import copy -import time from PyQt5 import QtCore, QtWebChannel, QtWidgets -from openlp.core.common.i18n import translate -from openlp.core.common.path import path_to_str -from openlp.core.common.settings import Settings -from openlp.core.common.registry import Registry from openlp.core.common.applocation import AppLocation -from openlp.core.ui import HideMode -from openlp.core.display.screens import ScreenList +from openlp.core.common.i18n import translate from openlp.core.common.mixins import RegistryProperties +from openlp.core.common.path import path_to_str +from openlp.core.common.registry import Registry +from openlp.core.common.settings import Settings +from openlp.core.common.utils import wait_for +from openlp.core.display.screens import ScreenList +from openlp.core.ui import HideMode log = logging.getLogger(__name__) @@ -102,6 +102,21 @@ class MediaWatcher(QtCore.QObject): self.muted.emit(is_muted) +class DisplayWatcher(QtCore.QObject): + """ + This facilitates communication from the Display object in the browser back to the Python + """ + initialised = QtCore.pyqtSignal(bool) + + @QtCore.pyqtSlot(bool) + def setInitialised(self, is_initialised): + """ + This method is called from the JS in the browser to set the _is_initialised attribute + """ + log.info('Display is initialised: {init}'.format(init=is_initialised)) + self.initialised.emit(is_initialised) + + class DisplayWindow(QtWidgets.QWidget, RegistryProperties): """ This is a window to show the output @@ -137,10 +152,13 @@ class DisplayWindow(QtWidgets.QWidget, RegistryProperties): self.checkerboard_path = display_base_path / 'checkerboard.png' self.openlp_splash_screen_path = display_base_path / 'openlp-splash-screen.png' self.set_url(QtCore.QUrl.fromLocalFile(path_to_str(self.display_path))) - self.media_watcher = MediaWatcher(self) self.channel = QtWebChannel.QWebChannel(self) + self.media_watcher = MediaWatcher(self) self.channel.registerObject('mediaWatcher', self.media_watcher) + self.display_watcher = DisplayWatcher(self) + self.channel.registerObject('displayWatcher', self.display_watcher) self.webview.page().setWebChannel(self.channel) + self.display_watcher.initialised.connect(self.on_initialised) self.is_display = False self.scale = 1 self.hide_mode = None @@ -155,6 +173,16 @@ class DisplayWindow(QtWidgets.QWidget, RegistryProperties): if len(ScreenList()) > 1 or Settings().value('core/display on monitor'): self.show() + @property + def is_initialised(self): + return self._is_initialised + + def on_initialised(self, is_initialised): + """ + Update the initialised status + """ + self._is_initialised = is_initialised + def update_from_screen(self, screen): """ Update the number and the geometry from the screen. @@ -208,7 +236,7 @@ class DisplayWindow(QtWidgets.QWidget, RegistryProperties): """ js_is_display = str(self.is_display).lower() self.run_javascript('Display.init({do_transitions});'.format(do_transitions=js_is_display)) - self._is_initialised = True + wait_for(lambda: self._is_initialised) if self.scale != 1: self.set_scale(self.scale) if self._can_show_startup_screen: @@ -222,14 +250,8 @@ class DisplayWindow(QtWidgets.QWidget, RegistryProperties): :param is_sync: Run the script synchronously. Defaults to False """ log.debug(script) - # Wait for other scripts to finish - end_time = time.time() + 10 - while not self.__script_done: - if time.time() > end_time: - log.error('Timed out waiting for preivous javascript script to finish') - break - time.sleep(0.1) - self.application.process_events() + # Wait for previous scripts to finish + wait_for(lambda: self.__script_done) if not is_sync: self.webview.page().runJavaScript(script) else: @@ -244,14 +266,9 @@ class DisplayWindow(QtWidgets.QWidget, RegistryProperties): self.__script_result = result self.webview.page().runJavaScript(script, handle_result) - end_time = time.time() + 10 - while not self.__script_done: - if time.time() > end_time: - self.__script_done = True - log.error('Timed out waiting for javascript script to finish') - break - time.sleep(0.001) - self.application.process_events() + # Wait for script to finish + if not wait_for(lambda: self.__script_done): + self.__script_done = True return self.__script_result def go_to_slide(self, verse): diff --git a/openlp/core/ui/thememanager.py b/openlp/core/ui/thememanager.py index 22a158fb4..0d8bcae56 100644 --- a/openlp/core/ui/thememanager.py +++ b/openlp/core/ui/thememanager.py @@ -37,6 +37,7 @@ from openlp.core.common.mixins import LogMixin, RegistryProperties from openlp.core.common.path import create_paths from openlp.core.common.registry import Registry, RegistryBase from openlp.core.common.settings import Settings +from openlp.core.common.utils import wait_for from openlp.core.lib import build_icon, check_item_selected, create_thumb, get_text_file_string, validate_thumb from openlp.core.lib.exceptions import ValidationError from openlp.core.lib.theme import Theme @@ -165,7 +166,6 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R self.setup_ui(self) self.global_theme = Settings().value(self.settings_section + '/global theme') self.build_theme_path() - self.upgrade_themes() # TODO: Can be removed when upgrade path from OpenLP 2.4 no longer needed def bootstrap_post_set_up(self): """ @@ -175,8 +175,14 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R self.theme_form = ThemeForm(self) self.theme_form.path = self.theme_path self.file_rename_form = FileRenameForm() - Registry().register_function('theme_update_global', self.change_global_from_tab) + + def bootstrap_completion(self): + """ + process the bootstrap completion request + """ + self.upgrade_themes() # TODO: Can be removed when upgrade path from OpenLP 2.4 no longer needed self.load_themes() + Registry().register_function('theme_update_global', self.change_global_from_tab) def upgrade_themes(self): """ @@ -184,6 +190,8 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R :rtype: None """ + # Wait for 2 seconds to allow some other things to start processing first + wait_for(lambda: False, timeout=1) xml_file_paths = AppLocation.get_section_data_path('themes').glob('*/*.xml') for xml_file_path in xml_file_paths: theme_data = get_text_file_string(xml_file_path) @@ -722,7 +730,6 @@ 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/openlp/core/ui/themeprogressform.py b/openlp/core/ui/themeprogressform.py index c636dd834..634eb9cb6 100644 --- a/openlp/core/ui/themeprogressform.py +++ b/openlp/core/ui/themeprogressform.py @@ -23,12 +23,13 @@ The theme regeneration progress dialog """ from PyQt5 import QtWidgets +from openlp.core.common.mixins import RegistryProperties, LogMixin +from openlp.core.common.utils import wait_for from openlp.core.display.screens import ScreenList from openlp.core.ui.themeprogressdialog import UiThemeProgressDialog -from openlp.core.common.mixins import RegistryProperties -class ThemeProgressForm(QtWidgets.QDialog, UiThemeProgressDialog, RegistryProperties): +class ThemeProgressForm(QtWidgets.QDialog, UiThemeProgressDialog, RegistryProperties, LogMixin): """ The theme regeneration progress dialog """ @@ -38,9 +39,9 @@ class ThemeProgressForm(QtWidgets.QDialog, UiThemeProgressDialog, RegistryProper self._theme_list = [] def show(self): + self.progress_bar.setValue(0) self.progress_bar.setMinimum(0) self.progress_bar.setMaximum(0) - self.progress_bar.setValue(0) try: screens = ScreenList() self.ratio = screens.current.display_geometry.width() / screens.current.display_geometry.height() @@ -52,6 +53,7 @@ class ThemeProgressForm(QtWidgets.QDialog, UiThemeProgressDialog, RegistryProper def get_preview(self, theme_name, theme_data): self.label.setText(theme_name) self.progress_bar.setValue(self.progress_bar.value() + 1) + wait_for(lambda: self.theme_display.is_initialised) self.theme_display.set_scale(float(self.theme_display.width()) / self.renderer.width()) return self.theme_display.generate_preview(theme_data, generate_screenshot=True) diff --git a/tests/interfaces/openlp_core/ui/test_thememanager.py b/tests/interfaces/openlp_core/ui/test_thememanager.py index df47d1d81..789be5146 100644 --- a/tests/interfaces/openlp_core/ui/test_thememanager.py +++ b/tests/interfaces/openlp_core/ui/test_thememanager.py @@ -58,7 +58,6 @@ class TestThemeManager(TestCase, TestMixin): # GIVEN: A new a call to initialise self.theme_manager.setup_ui = MagicMock() self.theme_manager.build_theme_path = MagicMock() - self.theme_manager.upgrade_themes = MagicMock() Settings().setValue('themes/global theme', 'my_theme') # WHEN: the initialisation is run @@ -68,7 +67,6 @@ class TestThemeManager(TestCase, TestMixin): self.theme_manager.setup_ui.assert_called_once_with(self.theme_manager) assert self.theme_manager.global_theme == 'my_theme' self.theme_manager.build_theme_path.assert_called_once_with() - self.theme_manager.upgrade_themes.assert_called_once_with() @patch('openlp.core.ui.thememanager.create_paths') @patch('openlp.core.ui.thememanager.AppLocation.get_section_data_path') @@ -110,7 +108,6 @@ class TestThemeManager(TestCase, TestMixin): Test the functions of bootstrap_post_setup are called. """ # GIVEN: - self.theme_manager.load_themes = MagicMock() self.theme_manager.theme_path = MagicMock() # WHEN: @@ -118,4 +115,21 @@ class TestThemeManager(TestCase, TestMixin): self.theme_manager.bootstrap_post_set_up() # THEN: - assert 1 == self.theme_manager.load_themes.call_count, "load_themes should have been called once" + assert self.theme_manager.progress_form is not None + assert self.theme_manager.theme_form is not None + assert self.theme_manager.file_rename_form is not None + + def test_bootstrap_completion(self): + """ + Test the functions of bootstrap_post_setup are called. + """ + # GIVEN: + self.theme_manager.load_themes = MagicMock() + self.theme_manager.upgrade_themes = MagicMock() + + # WHEN: + self.theme_manager.bootstrap_completion() + + # THEN: + self.theme_manager.upgrade_themes.assert_called_once() + self.theme_manager.load_themes.assert_called_once() diff --git a/tests/js/fake_webchannel.js b/tests/js/fake_webchannel.js index 08fa871a9..7d5b0fe5a 100644 --- a/tests/js/fake_webchannel.js +++ b/tests/js/fake_webchannel.js @@ -1,5 +1,10 @@ // This is a mock QWebChannel var qt = {webChannelTransport: 1}; +var displayWatcher = { + setInitialised: function (is_initialised) { + // do nothing + } +} var QWebChannel = function (transport, callback) { - callback({objects: {mediaWatcher: {}}}); + callback({objects: {mediaWatcher: {}, displayWatcher: displayWatcher}}); }; diff --git a/tests/openlp_core/common/test_utils.py b/tests/openlp_core/common/test_utils.py new file mode 100644 index 000000000..4d18969f7 --- /dev/null +++ b/tests/openlp_core/common/test_utils.py @@ -0,0 +1,44 @@ +# -*- coding: utf-8 -*- + +########################################################################## +# OpenLP - Open Source Lyrics Projection # +# ---------------------------------------------------------------------- # +# Copyright (c) 2008-2019 OpenLP Developers # +# ---------------------------------------------------------------------- # +# This program is free software: you can redistribute it and/or modify # +# it under the terms of the GNU General Public License as published by # +# the Free Software Foundation, either version 3 of the License, or # +# (at your option) any later version. # +# # +# This program is distributed in the hope that it will be useful, # +# but WITHOUT ANY WARRANTY; without even the implied warranty of # +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the # +# GNU General Public License for more details. # +# # +# You should have received a copy of the GNU General Public License # +# along with this program. If not, see . # +########################################################################## +""" +Interface tests to test the themeManager class and related methods. +""" +from unittest.mock import MagicMock + +from openlp.core.common.registry import Registry +from openlp.core.common.utils import wait_for + + +def test_wait_for(registry): + """ + Test the wait_for function + """ + # GIVEN: Mocked app and Registry + mock_app = MagicMock() + Registry().register('application', mock_app) + mock_func = MagicMock() + mock_func.side_effect = [False, True] + + # WHEN: wait_for is run + wait_for(mock_func) + + # THEN: the functions got called + assert mock_func.call_count == 2