Add time out to the loops and a delay in the second

Also add RegistryProperties, don't know how it was working without that

openlp/core/display/render.py

@@ -33,7 +33,7 @@ from PyQt5 import QtWidgets, QtGui
 
 from openlp.core.common import ThemeLevel
 from openlp.core.common.i18n import translate
-from openlp.core.common.mixins import LogMixin, RegistryProperties
+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.display.screens import ScreenList
@@ -797,7 +797,7 @@ class ThemePreviewRenderer(LogMixin, DisplayWindow):
             return pixmap
 
 
-class Renderer(RegistryBase, RegistryProperties, ThemePreviewRenderer):
+class Renderer(RegistryBase, ThemePreviewRenderer):
     """
     A virtual display used for rendering thumbnails and other offscreen tasks
     """

openlp/core/display/window.py

@@ -25,6 +25,7 @@ import json
 import logging
 import os
 import copy
+import time
 
 from PyQt5 import QtCore, QtWebChannel, QtWidgets
 
@@ -35,6 +36,7 @@ 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.mixins import RegistryProperties
 
 log = logging.getLogger(__name__)
 
@@ -100,7 +102,7 @@ class MediaWatcher(QtCore.QObject):
         self.muted.emit(is_muted)
 
 
-class DisplayWindow(QtWidgets.QWidget):
+class DisplayWindow(QtWidgets.QWidget, RegistryProperties):
     """
     This is a window to show the output
     """
@@ -142,6 +144,8 @@ class DisplayWindow(QtWidgets.QWidget):
         self.is_display = False
         self.scale = 1
         self.hide_mode = None
+        self.__script_done = True
+        self.__script_result = None
         if screen and screen.is_display:
             Registry().register_function('live_display_hide', self.hide_display)
             Registry().register_function('live_display_show', self.show_display)
@@ -218,6 +222,14 @@ class DisplayWindow(QtWidgets.QWidget):
         :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()
         if not is_sync:
             self.webview.page().runJavaScript(script)
         else:
@@ -232,9 +244,14 @@ class DisplayWindow(QtWidgets.QWidget):
                 self.__script_result = result
 
             self.webview.page().runJavaScript(script, handle_result)
+            end_time = time.time() + 10
             while not self.__script_done:
-                # TODO: Figure out how to break out of a potentially infinite loop
+                if time.time() > end_time:
-                QtWidgets.QApplication.instance().processEvents()
+                    self.__script_done = True
+                    log.error('Timed out waiting for javascript script to finish')
+                    break
+                time.sleep(0.001)
+                self.application.process_events()
             return self.__script_result
 
     def go_to_slide(self, verse):

tests/functional/openlp_core/display/test_window.py

@@ -22,6 +22,7 @@
 Package to test the openlp.core.display.window package.
 """
 import sys
+import time
 
 from unittest import TestCase
 from unittest.mock import MagicMock, patch
@@ -104,3 +105,42 @@ class TestDisplayWindow(TestCase, TestMixin):
 
         # THEN: javascript should not be run
         display_window.run_javascript.assert_called_once_with('Display.setScale(50.0);')
+
+    @patch.object(time, 'time')
+    def test_run_javascript_no_sync_no_wait(self, MockSettings, mocked_webengine, mocked_addWidget, mock_time):
+        """
+        test a script is run on the webview
+        """
+        # GIVEN: A (fake) webengine page
+        display_window = DisplayWindow()
+        webengine_page = MagicMock()
+        display_window.webview.page = MagicMock(return_value=webengine_page)
+
+        # WHEN: javascript is requested to run
+        display_window.run_javascript('javascript to execute')
+
+        # THEN: javascript should be run with no delay
+        webengine_page.runJavaScript.assert_called_once_with('javascript to execute')
+        mock_time.sleep.assert_not_called()
+
+    @patch.object(time, 'time')
+    def test_run_javascript_sync_no_wait(self, MockSettings, mocked_webengine, mocked_addWidget, mock_time):
+        """
+        test a synced script is run on the webview and immediately returns a result
+        """
+        # GIVEN: A (fake) webengine page with a js callback fn
+        def save_callback(script, callback):
+            callback(1234)
+        display_window = DisplayWindow()
+        display_window.webview = MagicMock()
+        webengine_page = MagicMock()
+        webengine_page.runJavaScript.side_effect = save_callback
+        display_window.webview.page.return_value = webengine_page
+
+        # WHEN: javascript is requested to run
+        result = display_window.run_javascript('javascript to execute', True)
+
+        # THEN: javascript should be run with no delay and return with the correct result
+        assert result == 1234
+        webengine_page.runJavaScript.assert_called_once()
+        mock_time.sleep.assert_not_called()

tests/functional/openlp_core/lib/test_theme.py

@@ -25,78 +25,160 @@ from pathlib import Path
 from unittest import TestCase
 from unittest.mock import MagicMock, patch
 
-from openlp.core.lib.theme import BackgroundType, Theme
+from openlp.core.lib.theme import BackgroundType, BackgroundGradientType, TransitionType, TransitionSpeed, Theme
 
 
-class TestBackgroundType(TestCase):
+class ThemeEnumerationTypes(TestCase):
     """
-    Test the BackgroundType enum methods.
+    Test the theme enum methods.
     """
-    def test_solid_to_string(self):
+    def test_background_type_to_string(self):
         """
         Test the to_string method of :class:`BackgroundType`
         """
-        # GIVEN: A BackgroundType member
+        # GIVEN: The BackgroundType members
-        background_type = BackgroundType.Solid
+        background_type_solid = BackgroundType.Solid
+        background_type_gradient = BackgroundType.Gradient
+        background_type_image = BackgroundType.Image
+        background_type_transparent = BackgroundType.Transparent
+        background_type_video = BackgroundType.Video
+        background_type_stream = BackgroundType.Stream
 
         # WHEN: Calling BackgroundType.to_string
-        # THEN: The string equivalent should have been returned
+        # THEN: The string equivalents should be returned
-        assert BackgroundType.to_string(background_type) == 'solid'
+        assert BackgroundType.to_string(background_type_solid) == 'solid'
+        assert BackgroundType.to_string(background_type_gradient) == 'gradient'
+        assert BackgroundType.to_string(background_type_image) == 'image'
+        assert BackgroundType.to_string(background_type_transparent) == 'transparent'
+        assert BackgroundType.to_string(background_type_video) == 'video'
+        assert BackgroundType.to_string(background_type_stream) == 'stream'
 
-    def test_gradient_to_string(self):
+    def test_background_type_from_string(self):
         """
-        Test the to_string method of :class:`BackgroundType`
+        Test the from_string method of :class:`BackgroundType`
         """
-        # GIVEN: A BackgroundType member
+        # GIVEN: The BackgroundType strings
-        background_type = BackgroundType.Gradient
+        background_type_solid = 'solid'
+        background_type_gradient = 'gradient'
+        background_type_image = 'image'
+        background_type_transparent = 'transparent'
+        background_type_video = 'video'
+        background_type_stream = 'stream'
 
-        # WHEN: Calling BackgroundType.to_string
+        # WHEN: Calling BackgroundType.from_string
-        # THEN: The string equivalent should have been returned
+        # THEN: The enum equivalents should be returned
-        assert BackgroundType.to_string(background_type) == 'gradient'
+        assert BackgroundType.from_string(background_type_solid) == BackgroundType.Solid
+        assert BackgroundType.from_string(background_type_gradient) == BackgroundType.Gradient
+        assert BackgroundType.from_string(background_type_image) == BackgroundType.Image
+        assert BackgroundType.from_string(background_type_transparent) == BackgroundType.Transparent
+        assert BackgroundType.from_string(background_type_video) == BackgroundType.Video
+        assert BackgroundType.from_string(background_type_stream) == BackgroundType.Stream
 
-    def test_image_to_string(self):
+    def test_background_gradient_type_to_string(self):
         """
-        Test the to_string method of :class:`BackgroundType`
+        Test the to_string method of :class:`BackgroundGradientType`
         """
-        # GIVEN: A BackgroundType member
+        # GIVEN: The BackgroundGradientType member
-        background_type = BackgroundType.Image
+        background_gradient_horizontal = BackgroundGradientType.Horizontal
+        background_gradient_vertical = BackgroundGradientType.Vertical
+        background_gradient_circular = BackgroundGradientType.Circular
+        background_gradient_left_top = BackgroundGradientType.LeftTop
+        background_gradient_left_bottom = BackgroundGradientType.LeftBottom
 
-        # WHEN: Calling BackgroundType.to_string
+        # WHEN: Calling BackgroundGradientType.to_string
-        # THEN: The string equivalent should have been returned
+        # THEN: The string equivalents should be returned
-        assert BackgroundType.to_string(background_type) == 'image'
+        assert BackgroundGradientType.to_string(background_gradient_horizontal) == 'horizontal'
+        assert BackgroundGradientType.to_string(background_gradient_vertical) == 'vertical'
+        assert BackgroundGradientType.to_string(background_gradient_circular) == 'circular'
+        assert BackgroundGradientType.to_string(background_gradient_left_top) == 'leftTop'
+        assert BackgroundGradientType.to_string(background_gradient_left_bottom) == 'leftBottom'
 
-    def test_transparent_to_string(self):
+    def test_background_gradient_type_from_string(self):
         """
-        Test the to_string method of :class:`BackgroundType`
+        Test the from_string method of :class:`BackgroundGradientType`
         """
-        # GIVEN: A BackgroundType member
+        # GIVEN: The BackgroundGradientType strings
-        background_type = BackgroundType.Transparent
+        background_gradient_horizontal = 'horizontal'
+        background_gradient_vertical = 'vertical'
+        background_gradient_circular = 'circular'
+        background_gradient_left_top = 'leftTop'
+        background_gradient_left_bottom = 'leftBottom'
 
-        # WHEN: Calling BackgroundType.to_string
+        # WHEN: Calling BackgroundGradientType.from_string
-        # THEN: The string equivalent should have been returned
+        # THEN: The enum equivalents should be returned
-        assert BackgroundType.to_string(background_type) == 'transparent'
+        assert BackgroundGradientType.from_string(background_gradient_horizontal) == BackgroundGradientType.Horizontal
+        assert BackgroundGradientType.from_string(background_gradient_vertical) == BackgroundGradientType.Vertical
+        assert BackgroundGradientType.from_string(background_gradient_circular) == BackgroundGradientType.Circular
+        assert BackgroundGradientType.from_string(background_gradient_left_top) == BackgroundGradientType.LeftTop
+        assert BackgroundGradientType.from_string(background_gradient_left_bottom) == BackgroundGradientType.LeftBottom
 
-    def test_video_to_string(self):
+    def test_transition_type_to_string(self):
         """
-        Test the to_string method of :class:`BackgroundType`
+        Test the to_string method of :class:`TransitionType`
         """
-        # GIVEN: A BackgroundType member
+        # GIVEN: The TransitionType member
-        background_type = BackgroundType.Video
+        transition_type_fade = TransitionType.Fade
+        transition_type_slide = TransitionType.Slide
+        transition_type_convex = TransitionType.Convex
+        transition_type_concave = TransitionType.Concave
+        transition_type_zoom = TransitionType.Zoom
 
-        # WHEN: Calling BackgroundType.to_string
+        # WHEN: Calling TransitionType.to_string
-        # THEN: The string equivalent should have been returned
+        # THEN: The string equivalents should be returned
-        assert BackgroundType.to_string(background_type) == 'video'
+        assert TransitionType.to_string(transition_type_fade) == 'fade'
+        assert TransitionType.to_string(transition_type_slide) == 'slide'
+        assert TransitionType.to_string(transition_type_convex) == 'convex'
+        assert TransitionType.to_string(transition_type_concave) == 'concave'
+        assert TransitionType.to_string(transition_type_zoom) == 'zoom'
 
-    def test_stream_to_string(self):
+    def test_transition_type_from_string(self):
         """
-        Test the to_string method of :class:`BackgroundType`
+        Test the from_string method of :class:`TransitionType`
         """
-        # GIVEN: A BackgroundType member
+        # GIVEN: The TransitionType strings
-        background_type = BackgroundType.Stream
+        transition_type_fade = 'fade'
+        transition_type_slide = 'slide'
+        transition_type_convex = 'convex'
+        transition_type_concave = 'concave'
+        transition_type_zoom = 'zoom'
 
-        # WHEN: Calling BackgroundType.to_string
+        # WHEN: Calling TransitionType.from_string
-        # THEN: The string equivalent should have been returned
+        # THEN: The enum equivalents should be returned
-        assert BackgroundType.to_string(background_type) == 'stream'
+        assert TransitionType.from_string(transition_type_fade) == TransitionType.Fade
+        assert TransitionType.from_string(transition_type_slide) == TransitionType.Slide
+        assert TransitionType.from_string(transition_type_convex) == TransitionType.Convex
+        assert TransitionType.from_string(transition_type_concave) == TransitionType.Concave
+        assert TransitionType.from_string(transition_type_zoom) == TransitionType.Zoom
+
+    def test_transition_speed_to_string(self):
+        """
+        Test the to_string method of :class:`TransitionSpeed`
+        """
+        # GIVEN: The TransitionSpeed member
+        transition_speed_normal = TransitionSpeed.Normal
+        transition_speed_fast = TransitionSpeed.Fast
+        transition_speed_slow = TransitionSpeed.Slow
+
+        # WHEN: Calling TransitionSpeed.to_string
+        # THEN: The string equivalents should be returned
+        assert TransitionSpeed.to_string(transition_speed_normal) == 'normal'
+        assert TransitionSpeed.to_string(transition_speed_fast) == 'fast'
+        assert TransitionSpeed.to_string(transition_speed_slow) == 'slow'
+
+    def test_transition_speed_from_string(self):
+        """
+        Test the from_string method of :class:`TransitionSpeed`
+        """
+        # GIVEN: The TransitionSpeed strings
+        transition_speed_normal = 'normal'
+        transition_speed_fast = 'fast'
+        transition_speed_slow = 'slow'
+
+        # WHEN: Calling TransitionSpeed.from_string
+        # THEN: The enum equivalents should be returned
+        assert TransitionSpeed.from_string(transition_speed_normal) == TransitionSpeed.Normal
+        assert TransitionSpeed.from_string(transition_speed_fast) == TransitionSpeed.Fast
+        assert TransitionSpeed.from_string(transition_speed_slow) == TransitionSpeed.Slow
 
 
 class TestTheme(TestCase):