Fixed a couple tests

openlp/core/api/endpoint/controller.py

@@ -49,7 +49,7 @@ def controller_text(request):
 
     :param request: the http request - not used
     """
-    log.debug("controller_text ")
+    log.debug('controller_text')
     live_controller = Registry().get('live_controller')
     current_item = live_controller.service_item
     data = []
@@ -58,13 +58,14 @@ def controller_text(request):
             item = {}
             # Handle text (songs, custom, bibles)
             if current_item.is_text():
-                if frame['verseTag']:
+                if frame['verse']:
-                    item['tag'] = str(frame['verseTag'])
+                    item['tag'] = str(frame['verse'])
                 else:
                     item['tag'] = str(index + 1)
-                item['chords_text'] = str(frame['chords_text'])
+                # TODO: Figure out rendering chords
-                item['text'] = str(frame['text'])
+                item['chords_text'] = str(frame.get('chords_text', ''))
-                item['html'] = str(frame['html'])
+                item['text'] = frame['text']
+                item['html'] = current_item.get_rendered_frame(index)
             # Handle images, unless a custom thumbnail is given or if thumbnails is disabled
             elif current_item.is_image() and not frame.get('image', '') and Settings().value('api/thumbnails'):
                 item['tag'] = str(index + 1)

openlp/core/display/render.py

@@ -25,6 +25,7 @@ The :mod:`~openlp.display.render` module contains functions for rendering.
 import html
 import logging
 import math
+import os
 import re
 import time
 
@@ -33,15 +34,15 @@ from PyQt5 import QtWidgets, QtGui
 from openlp.core.lib.formattingtags import FormattingTags
 from openlp.core.common.registry import Registry, RegistryBase
 from openlp.core.common.mixins import LogMixin, RegistryProperties
-from openlp.core.display.window import DisplayWindow
 from openlp.core.display.screens import ScreenList
+from openlp.core.display.window import DisplayWindow
 from openlp.core.lib import ItemCapabilities
 
 
 log = logging.getLogger(__name__)
 
 SLIM_CHARS = 'fiíIÍjlĺľrtť.,;/ ()|"\'!:\\'
-CHORD_LINE_MATCH = re.compile(r'\[(.*?)\]([\u0080-\uFFFF,\w]*)'
+CHORD_LINE_MATCH = re.compile(r'\[(.*?)\]([\u0080-\uFFFF,\w]*)'     # noqa
                               '([\u0080-\uFFFF,\w,\s,\.,\,,\!,\?,\;,\:,\|,\",\',\-,\_]*)(\Z)?')
 CHORD_TEMPLATE = '<span class="chordline">{chord}</span>'
 FIRST_CHORD_TEMPLATE = '<span class="chordline firstchordline">{chord}</span>'
@@ -438,11 +439,11 @@ class Renderer(RegistryBase, LogMixin, RegistryProperties, DisplayWindow):
                 self.setGeometry(screen.geometry.x(), screen.geometry.y(),
                                  screen.geometry.width(), screen.geometry.height())
                 break
-        # If the display is not show'ed and hidden like this webegine will not render 
+        # If the display is not show'ed and hidden like this webegine will not render
         self.show()
         self.hide()
         self.theme_height = 0
-    
+
     def calculate_line_count(self):
         """
         Calculate the number of lines that fits on one slide
@@ -524,11 +525,11 @@ class Renderer(RegistryBase, LogMixin, RegistryProperties, DisplayWindow):
                     # If there are (at least) two occurrences of [---] we use the first two slides (and neglect the last
                     # for now).
                     if len(slides) == 3:
-                        html_text = expand_tags('\n'.join(slides[:2]))
+                        html_text = render_tags('\n'.join(slides[:2]))
                     # We check both slides to determine if the optional split is needed (there is only one optional
                     # split).
                     else:
-                        html_text = expand_tags('\n'.join(slides))
+                        html_text = render_tags('\n'.join(slides))
                     html_text = html_text.replace('\n', '<br>')
                     if self._text_fits_on_slide(html_text):
                         # The first two optional slides fit (as a whole) on one slide. Replace the first occurrence
@@ -617,7 +618,7 @@ class Renderer(RegistryBase, LogMixin, RegistryProperties, DisplayWindow):
         previous_raw = ''
         for line in lines:
             line = line.strip()
-            html_line = expand_tags(line)
+            html_line = render_tags(line)
             # Text too long so go to next page.
             if not self._text_fits_on_slide(previous_html + html_line):
                 # Check if there was a verse before the current one and append it, when it fits on the page.
@@ -634,7 +635,7 @@ class Renderer(RegistryBase, LogMixin, RegistryProperties, DisplayWindow):
                             continue
                 # Figure out how many words of the line will fit on screen as the line will not fit as a whole.
                 raw_words = words_split(line)
-                html_words = list(map(expand_tags, raw_words))
+                html_words = list(map(render_tags, raw_words))
                 previous_html, previous_raw = \
                     self._binary_chop(formatted, previous_html, previous_raw, html_words, raw_words, ' ', line_end)
             else:

openlp/core/lib/serviceitem.py

@@ -165,24 +165,23 @@ class ServiceItem(RegistryProperties):
         previous_pages = {}
         for index, raw_slide in enumerate(self.slides):
             verse_tag = raw_slide['verse']
-            if verse_tag in previous_pages and previous_pages[verse_tag][0] == raw_slide:
+            if verse_tag in previous_pages and previous_pages[verse_tag][1] == raw_slide:
-                pages = previous_pages[verse_tag][1]
+                page = previous_pages[verse_tag][1]
             else:
-                pages = self.renderer.format_slide(raw_slide['text'], self)
+                page = render_tags(raw_slide['text'], self)
-                previous_pages[verse_tag] = (raw_slide, pages)
+                previous_pages[verse_tag] = (raw_slide, page)
-            for page in pages:
+            rendered_slide = {
-                rendered_slide = {
+                'title': raw_slide['title'],
-                    'title': raw_slide['title'],
+                'text': page,
-                    'text': render_tags(page),
+                'verse': index,
-                    'verse': index,
+            }
-                }
+            self._rendered_slides.append(rendered_slide)
-                self._rendered_slides.append(rendered_slide)
+            display_slide = {
-                display_slide = {
+                'title': raw_slide['title'],
-                    'title': raw_slide['title'],
+                'text': remove_tags(page),
-                    'text': remove_tags(page),
+                'verse': verse_tag,
-                    'verse': verse_tag,
+            }
-                }
+            self._display_slides.append(display_slide)
-                self._display_slides.append(display_slide)
 
     @property
     def rendered_slides(self):
@@ -362,6 +361,7 @@ class ServiceItem(RegistryProperties):
         if self.service_item_type == ServiceItemType.Text:
             for slide in service_item['serviceitem']['data']:
                 self.add_from_text(slide['raw_slide'], slide['verseTag'])
+            self._create_slides()
         elif self.service_item_type == ServiceItemType.Image:
             settings_section = service_item['serviceitem']['header']['name']
             background = QtGui.QColor(Settings().value(settings_section + '/background color'))
@@ -485,7 +485,7 @@ class ServiceItem(RegistryProperties):
         Returns the frames for the ServiceItem
         """
         if self.service_item_type == ServiceItemType.Text:
-            return self._display_slides
+            return self.display_slides
         else:
             return self.slides
 
@@ -496,7 +496,8 @@ class ServiceItem(RegistryProperties):
         :param row: The service item slide to be returned
         """
         if self.service_item_type == ServiceItemType.Text:
-            return self._display_frames[row]['html'].split('\n')[0]
+            # return self.display_frames[row]['html'].split('\n')[0]
+            return self.rendered_slides[row]['text']
         elif self.service_item_type == ServiceItemType.Image:
             return self.slides[row]['path']
         else:

tests/functional/openlp_core/api/endpoint/test_controller.py

@@ -19,20 +19,17 @@
 # with this program; if not, write to the Free Software Foundation, Inc., 59  #
 # Temple Place, Suite 330, Boston, MA 02111-1307 USA                          #
 ###############################################################################
-import sys
+# import sys
 from unittest import TestCase
-# You should have received a copy of the GNU General Public License along     #
+from unittest.mock import MagicMock, patch
-# with this program; if not, write to the Free Software Foundation, Inc., 59  #
-from unittest.mock import MagicMock
 
 from PyQt5 import QtCore
 
 # Mock QtWebEngineWidgets
-sys.modules['PyQt5.QtWebEngineWidgets'] = MagicMock()
+# sys.modules['PyQt5.QtWebEngineWidgets'] = MagicMock()
 
 from openlp.core.api.endpoint.controller import controller_direction, controller_text
 from openlp.core.common.registry import Registry
-from openlp.core.display.render import Renderer
 from openlp.core.display.screens import ScreenList
 from openlp.core.lib.serviceitem import ServiceItem
 from tests.utils import convert_file_service_item
@@ -64,9 +61,11 @@ class TestController(TestCase):
         self.desktop.primaryScreen.return_value = SCREEN['primary']
         self.desktop.screenCount.return_value = SCREEN['number']
         self.desktop.screenGeometry.return_value = SCREEN['size']
-        self.screens = ScreenList.create(self.desktop)
+        with patch('openlp.core.display.screens.QtWidgets.QApplication.screens') as mocked_screens:
-        renderer = Renderer()
+            mocked_screens.return_value = [
-        renderer.empty_height = 1000
+                MagicMock(**{'geometry.return_value': SCREEN['size']})
+            ]
+            self.screens = ScreenList.create(self.desktop)
         Registry().register('live_controller', self.mocked_live_controller)
 
     def test_controller_text_empty(self):
@@ -74,13 +73,17 @@ class TestController(TestCase):
         Remote API Tests : test the controller text method can be called with empty service item
         """
         # GIVEN: A mocked service with a dummy service item
-        self.mocked_live_controller.service_item = MagicMock()
+        mocked_service_item = MagicMock()
+        mocked_service_item.get_frames.return_value = []
+        mocked_service_item.unique_identifier = 'mock-service-item'
+        self.mocked_live_controller.service_item = mocked_service_item
+
         # WHEN: I trigger the method
-        ret = controller_text("SomeText")
+        ret = controller_text(MagicMock())
+
         # THEN: I get a basic set of results
-        results = ret['results']
+        assert ret['results']['item'] == 'mock-service-item'
-        assert isinstance(results['item'], MagicMock)
+        assert len(ret['results']['slides']) == 0
-        assert len(results['slides']) == 0
 
     def test_controller_text(self):
         """
@@ -90,9 +93,10 @@ class TestController(TestCase):
         line = convert_file_service_item(TEST_PATH, 'serviceitem_custom_1.osj')
         self.mocked_live_controller.service_item = ServiceItem(None)
         self.mocked_live_controller.service_item.set_from_service(line)
-        self.mocked_live_controller.service_item.render(True)
+
         # WHEN: I trigger the method
-        ret = controller_text("SomeText")
+        ret = controller_text(MagicMock())
+
         # THEN: I get a basic set of results
         results = ret['results']
         assert isinstance(ret, dict)
@@ -103,19 +107,27 @@ class TestController(TestCase):
         Text the live next method is triggered
         """
         # GIVEN: A mocked service with a dummy service item
+        mocked_emit = MagicMock()
+        self.mocked_live_controller.slidecontroller_live_next.emit = mocked_emit
         self.mocked_live_controller.service_item = MagicMock()
+
         # WHEN: I trigger the method
         controller_direction(None, 'live', 'next')
+
         # THEN: The correct method is called
-        self.mocked_live_controller.slidecontroller_live_next.emit.assert_called_once_with()
+        mocked_emit.assert_called_once_with()
 
     def test_controller_direction_previous(self):
         """
         Text the live next method is triggered
         """
         # GIVEN: A mocked service with a dummy service item
+        mocked_emit = MagicMock()
+        self.mocked_live_controller.slidecontroller_live_previous.emit = mocked_emit
         self.mocked_live_controller.service_item = MagicMock()
+
         # WHEN: I trigger the method
         controller_direction(None, 'live', 'previous')
+
         # THEN: The correct method is called
-        self.mocked_live_controller.slidecontroller_live_previous.emit.assert_called_once_with()
+        mocked_emit.assert_called_once_with()

tests/functional/openlp_core/common/test_actions.py

@@ -23,138 +23,133 @@
 Package to test the openlp.core.common.actions package.
 """
 from unittest import TestCase
-from unittest.mock import MagicMock, call, patch
+from unittest.mock import MagicMock
 
 from PyQt5 import QtCore, QtGui, QtWidgets
 
-import openlp.core.common.actions
 from openlp.core.common.actions import ActionList, CategoryActionList
 from openlp.core.common.settings import Settings
 from tests.helpers.testmixin import TestMixin
 
 
-class TestCategoryActionList(TestCase):
+MOCK_ACTION1 = MagicMock(**{'text.return_value': 'first'})
-    def setUp(self):
+MOCK_ACTION2 = MagicMock(**{'text.return_value': 'second'})
-        """
-        Create an instance and a few example actions.
-        """
-        self.action1 = MagicMock()
-        self.action1.text.return_value = 'first'
-        self.action2 = MagicMock()
-        self.action2.text.return_value = 'second'
-        self.list = CategoryActionList()
 
-    def tearDown(self):
-        """
-        Clean up
-        """
-        del self.list
 
-    def test_contains(self):
+def test_action_list_contains():
-        """
+    """
-        Test the __contains__() method
+    Test the __contains__() method
-        """
+    """
-        # GIVEN: The list.
+    # GIVEN: The list and 2 actions
-        # WHEN: Add an action
+    category_list = CategoryActionList()
-        self.list.append(self.action1)
 
-        # THEN: The actions should (not) be in the list.
+    # WHEN: Add an action
-        assert self.action1 in self.list
+    category_list.append(MOCK_ACTION1)
-        assert self.action2 not in self.list
 
-    def test_len(self):
+    # THEN: The actions should (not) be in the list.
-        """
+    assert MOCK_ACTION1 in category_list
-        Test the __len__ method
+    assert MOCK_ACTION2 not in category_list
-        """
-        # GIVEN: The list.
-        # WHEN: Do nothing.
-        # THEN: Check the length.
-        assert len(self.list) == 0, "The length should be 0."
 
-        # GIVEN: The list.
-        # WHEN: Append an action.
-        self.list.append(self.action1)
 
-        # THEN: Check the length.
+def test_action_list_empty_len():
-        assert len(self.list) == 1, "The length should be 1."
+    """
+    Test the __len__ method when the list is empty
+    """
+    # GIVEN: The list without any actions
+    category_list = CategoryActionList()
 
-    def test_append(self):
+    # WHEN: Do nothing.
-        """
+    list_len = len(category_list)
-        Test the append() method
-        """
-        # GIVEN: The list.
-        # WHEN: Append an action.
-        self.list.append(self.action1)
-        self.list.append(self.action2)
 
-        # THEN: Check if the actions are in the list and check if they have the correct weights.
+    # THEN: Check the length.
-        assert self.action1 in self.list
+    assert list_len == 0, 'The length should be 0.'
-        assert self.action2 in self.list
-        assert self.list.actions[0] == (0, self.action1)
-        assert self.list.actions[1] == (1, self.action2)
 
-    def test_add(self):
-        """
-        Test the add() method
-        """
-        # GIVEN: The list and weights.
-        action1_weight = 42
-        action2_weight = 41
 
-        # WHEN: Add actions and their weights.
+def test_action_list_len():
-        self.list.add(self.action1, action1_weight)
+    """
-        self.list.add(self.action2, action2_weight)
+    Test the __len__ method when the list is not empty
+    """
+    # GIVEN: The list with 2 items in it
+    category_list = CategoryActionList()
+    category_list.append(MOCK_ACTION1)
+    category_list.append(MOCK_ACTION2)
 
-        # THEN: Check if they were added and have the specified weights.
+    # WHEN: The length of the list is calculated
-        assert self.action1 in self.list
+    list_len = len(category_list)
-        assert self.action2 in self.list
-        # Now check if action1 is second and action2 is first (due to their weights).
-        assert self.list.actions[0] == (41, self.action2)
-        assert self.list.actions[1] == (42, self.action1)
 
-    def test_iterator(self):
+    # THEN: It should have 2 items
-        """
+    assert list_len == 2, 'The list should have 2 items in it'
-        Test the __iter__ and __next__ methods
-        """
-        # GIVEN: The list including two actions
-        self.list.add(self.action1)
-        self.list.add(self.action2)
 
-        # WHEN: Iterating over the list
-        local_list = [a for a in self.list]
-        # THEN: Make sure they are returned in correct order
-        assert len(self.list) == 2
-        assert local_list[0] is self.action1
-        assert local_list[1] is self.action2
 
-    def test_remove(self):
+def test_action_list_append():
-        """
+    """
-        Test the remove() method
+    Test the append() method
-        """
+    """
-        # GIVEN: The list
+    # GIVEN: The list.
-        self.list.append(self.action1)
+    category_list = CategoryActionList()
 
-        # WHEN: Delete an item from the list.
+    # WHEN: Append an action.
-        self.list.remove(self.action1)
+    category_list.append(MOCK_ACTION1)
+    category_list.append(MOCK_ACTION2)
 
-        # THEN: Now the element should not be in the list anymore.
+    # THEN: Check if the actions are in the list and check if they have the correct weights.
-        assert self.action1 not in self.list
+    assert MOCK_ACTION1 in category_list
+    assert MOCK_ACTION2 in category_list
+    assert category_list.actions[0] == (0, MOCK_ACTION1)
+    assert category_list.actions[1] == (1, MOCK_ACTION2)
 
-    def test_remove_not_in_list(self):
-        """
-        Test the remove() method when action not in list
-        """
-        with patch.object(openlp.core.common.actions, 'log') as mock_log:
-            log_warn_calls = [call('Action "" does not exist.')]
 
-            # GIVEN: The list
+def test_action_list_add():
-            self.list.append(self.action1)
+    """
+    Test the add() method
+    """
+    # GIVEN: The list and weights.
+    action1_weight = 42
+    action2_weight = 41
+    category_list = CategoryActionList()
 
-            # WHEN: Delete an item not in the list.
+    # WHEN: Add actions and their weights.
-            self.list.remove('')
+    category_list.add(MOCK_ACTION1, action1_weight)
+    category_list.add(MOCK_ACTION2, action2_weight)
 
-            # THEN: Warning should be logged
+    # THEN: Check if they were added and have the specified weights.
-            mock_log.warning.assert_has_calls(log_warn_calls)
+    assert MOCK_ACTION1 in category_list
+    assert MOCK_ACTION2 in category_list
+    assert category_list.actions[0] == (41, MOCK_ACTION2)
+    assert category_list.actions[1] == (42, MOCK_ACTION1)
+
+
+def test_action_list_iterator():
+    """
+    Test the __iter__ and __next__ methods
+    """
+    # GIVEN: The list including two actions
+    category_list = CategoryActionList()
+    category_list.append(MOCK_ACTION1)
+    category_list.append(MOCK_ACTION2)
+
+    # WHEN: Iterating over the list
+    local_list = [a for a in category_list]
+
+    # THEN: Make sure they are returned in correct order
+    assert len(category_list) == 2
+    assert local_list[0] is MOCK_ACTION1
+    assert local_list[1] is MOCK_ACTION2
+
+
+def test_action_list_remove():
+    """
+    Test the remove() method
+    """
+    # GIVEN: The list
+    category_list = CategoryActionList()
+    category_list.append(MOCK_ACTION1)
+
+    # WHEN: Delete an item from the list.
+    category_list.remove(MOCK_ACTION1)
+
+    # THEN: Now the element should not be in the list anymore.
+    assert MOCK_ACTION1 not in category_list
 
 
 class TestActionList(TestCase, TestMixin):