Merge branch 'settings_1a' into 'master'

Migrate Settings - Part 1

See merge request openlp/openlp!84

openlp/core/app.py

@@ -400,6 +400,9 @@ def main():
     Registry.create()
     Registry().register('application', application)
     Registry().set_flag('no_web_server', args.no_web_server)
+    # Upgrade settings.
+    settings = Settings()
+    Registry().register('settings', settings)
     application.setApplicationVersion(get_version()['version'])
     # Check if an instance of OpenLP is already running. Quit if there is a running instance and the user only wants one
     server = Server()
@@ -414,8 +417,6 @@ def main():
     if application.is_data_path_missing():
         server.close_server()
         sys.exit()
-    # Upgrade settings.
-    settings = Settings()
     if settings.can_upgrade():
         now = datetime.now()
         # Only back up if OpenLP has previously run.
@@ -435,7 +436,7 @@ def main():
                     translate('OpenLP', 'Settings back up failed.\n\nContinuing to upgrade.'))
         settings.upgrade_settings()
     # First time checks in settings
-    if not Settings().value('core/has run wizard'):
+    if not settings.value('core/has run wizard'):
         if not FirstTimeLanguageForm().exec():
             # if cancel then stop processing
             server.close_server()

openlp/core/common/mixins.py

@@ -125,6 +125,7 @@ class RegistryProperties(object):
     _settings_form = None
     _alerts_manager = None
     _projector_manager = None
+    _settings = None
 
     @property
     def application(self):
@@ -246,3 +247,12 @@ class RegistryProperties(object):
         if not hasattr(self, '_projector_manager') or not self._projector_manager:
             self._projector_manager = Registry().get('projector_manager')
         return self._projector_manager
+
+    @property
+    def settings(self):
+        """
+        Adds the projector manager to the class dynamically
+        """
+        if not hasattr(self, '_settings') or not self._settings:
+            self._settings = Registry().get('settings')
+        return self._settings

openlp/core/ui/media/mediacontroller.py

@@ -37,7 +37,6 @@ from openlp.core.api.http import register_endpoint
 from openlp.core.common.i18n import translate
 from openlp.core.common.mixins import LogMixin, RegistryProperties
 from openlp.core.common.registry import Registry, RegistryBase
-from openlp.core.common.settings import Settings
 from openlp.core.lib.serviceitem import ItemCapabilities
 from openlp.core.lib.ui import critical_error_message_box
 from openlp.core.ui import DisplayControllerType
@@ -64,7 +63,6 @@ class MediaController(RegistryBase, LogMixin, RegistryProperties):
         self.live_timer.setInterval(TICK_TIME)
         self.preview_timer = QtCore.QTimer()
         self.preview_timer.setInterval(TICK_TIME)
-        self.settings = Settings()
         # Signals
         self.live_timer.timeout.connect(self.media_state_live)
         self.preview_timer.timeout.connect(self.media_state_preview)

openlp/plugins/bibles/lib/mediaitem.py

@@ -131,7 +131,7 @@ class BibleMediaItem(MediaManagerItem):
         self.bibles_go_live.connect(self.go_live_remote)
         self.bibles_add_to_service.connect(self.add_to_service_remote)
         # Place to store the search results for both bibles.
-        self.settings = self.plugin.settings_tab
+        self.settings_tab = self.plugin.settings_tab
         self.quick_preview_allowed = True
         self.has_search = True
         self.search_results = []
@@ -557,8 +557,8 @@ class BibleMediaItem(MediaManagerItem):
         :return: None
         """
         # TODO: Change layout_style to a property
-        self.settings.layout_style = index
+        self.settings_tab.layout_style = index
-        self.settings.layout_style_combo_box.setCurrentIndex(index)
+        self.settings_tab.layout_style_combo_box.setCurrentIndex(index)
         Settings().setValue('{section}/verse layout style'.format(section=self.settings_section), index)
 
     def on_version_combo_box_index_changed(self):
@@ -945,12 +945,12 @@ class BibleMediaItem(MediaManagerItem):
                 raw_slides.append(bible_text.rstrip())
                 bible_text = ''
             # If we are 'Verse Per Slide' then create a new slide.
-            elif self.settings.layout_style == LayoutStyle.VersePerSlide:
+            elif self.settings_tab.layout_style == LayoutStyle.VersePerSlide:
                 bible_text = '{first_version}{data[text]}'.format(first_version=verse_text, data=data)
                 raw_slides.append(bible_text.rstrip())
                 bible_text = ''
             # If we are 'Verse Per Line' then force a new line.
-            elif self.settings.layout_style == LayoutStyle.VersePerLine:
+            elif self.settings_tab.layout_style == LayoutStyle.VersePerLine:
                 bible_text = '{bible} {verse}{data[text]}\n'.format(bible=bible_text, verse=verse_text, data=data)
             # We have to be 'Continuous'.
             else:
@@ -966,7 +966,7 @@ class BibleMediaItem(MediaManagerItem):
         if bible_text:
             raw_slides.append(bible_text.lstrip())
         # Service Item: Capabilities
-        if self.settings.layout_style == LayoutStyle.Continuous and not data['second_bible']:
+        if self.settings_tab.layout_style == LayoutStyle.Continuous and not data['second_bible']:
             # Split the line but do not replace line breaks in renderer.
             service_item.add_capability(ItemCapabilities.NoLineBreaks)
         service_item.add_capability(ItemCapabilities.CanPreview)
@@ -976,8 +976,8 @@ class BibleMediaItem(MediaManagerItem):
         # Service Item: Title
         service_item.title = '{verse} {version}'.format(verse=verses.format_verses(), version=verses.format_versions())
         # Service Item: Theme
-        if self.settings.bible_theme:
+        if self.settings_tab.bible_theme:
-            service_item.theme = self.settings.bible_theme
+            service_item.theme = self.settings_tab.bible_theme
         for slide in raw_slides:
             service_item.add_from_text(slide)
         return True
@@ -994,10 +994,10 @@ class BibleMediaItem(MediaManagerItem):
         :param verse: The verse number (int).
         :return: An empty or formatted string
         """
-        if not self.settings.is_verse_number_visible:
+        if not self.settings_tab.is_verse_number_visible:
             return ''
         verse_separator = get_reference_separators()['verse']
-        if not self.settings.show_new_chapters or old_chapter != chapter:
+        if not self.settings_tab.show_new_chapters or old_chapter != chapter:
             verse_text = '{chapter}{sep}{verse}'.format(chapter=chapter, sep=verse_separator, verse=verse)
         else:
             verse_text = verse
@@ -1006,7 +1006,7 @@ class BibleMediaItem(MediaManagerItem):
             DisplayStyle.Round: ('(', ')'),
             DisplayStyle.Curly: ('{', '}'),
             DisplayStyle.Square: ('[', ']')
-        }[self.settings.display_style]
+        }[self.settings_tab.display_style]
         return '{{su}}{bracket[0]}{verse_text}{bracket[1]}{{/su}} '.format(verse_text=verse_text, bracket=bracket)
 
     def search(self, string, show_error=True):

tests/conftest.py

@@ -0,0 +1,62 @@
+# -*- 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 <https://www.gnu.org/licenses/>. #
+##########################################################################
+"""
+All the tests
+"""
+import os
+from tempfile import mkstemp
+
+import pytest
+from PyQt5 import QtCore, QtWidgets
+
+from openlp.core.common.registry import Registry
+from openlp.core.common.settings import Settings
+
+
+@pytest.yield_fixture
+def qapp():
+    """An instance of QApplication"""
+    app = QtWidgets.QApplication([])
+    yield app
+    del app
+
+
+@pytest.yield_fixture
+def settings(qapp):
+    """A Settings() instance"""
+    fd, ini_file = mkstemp('.ini')
+    Settings.set_filename(ini_file)
+    Registry.create()
+    Settings().setDefaultFormat(QtCore.QSettings.IniFormat)
+    # Needed on windows to make sure a Settings object is available during the tests
+    sets = Settings()
+    sets.setValue('themes/global theme', 'my_theme')
+    Registry().register('settings', set)
+    yield sets
+    del sets
+    os.close(fd)
+    os.unlink(Settings().fileName())
+
+
+@pytest.fixture
+def registry():
+    """An instance of the Registry"""
+    Registry.create()

tests/functional/openlp_core/test_app.py

@@ -19,6 +19,7 @@
 # along with this program.  If not, see <https://www.gnu.org/licenses/>. #
 ##########################################################################
 import sys
+
 from unittest import TestCase, skip
 from unittest.mock import MagicMock, patch
 
@@ -157,6 +158,8 @@ def test_parse_options_file_and_debug():
     assert args.rargs == ['dummy_temp'], 'The service file should not be blank'
 
 
+# Problem seems to be with the what the OpenLP object is defined.
+# Running each test on its own is fine but as a block you get seg faults in strange places.
 @skip('Figure out why this is causing a segfault')
 class TestOpenLP(TestCase):
     """

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

@@ -21,155 +21,62 @@
 """
 Package to test the openlp.core.ui package.
 """
-from unittest import TestCase, skip
-from unittest.mock import patch
-
-from PyQt5 import QtCore
-
 from openlp.core.ui.media import parse_optical_path
-from tests.helpers.testmixin import TestMixin
 
 
-class TestMedia(TestCase, TestMixin):
+def test_parse_optical_path_linux():
+    """
+    Test that test_parse_optical_path() parses a optical path with linux device path correctly
+    """
 
-    @skip
+    # GIVEN: An optical formatted path
-    def test_get_media_players_no_config(self):
+    org_title_track = 1
-        """
+    org_audio_track = 2
-        Test that when there's no config, get_media_players() returns an empty list of players (not a string)
+    org_subtitle_track = -1
-        """
+    org_start = 1234
-        def value_results(key):
+    org_end = 4321
-            if key == 'media/players':
+    org_name = 'test name'
-                return ''
+    org_device_path = '/dev/dvd'
-            else:
+    path = 'optical:%d:%d:%d:%d:%d:%s:%s' % (org_title_track, org_audio_track, org_subtitle_track,
-                return False
+                                             org_start, org_end, org_name, org_device_path)
 
-        # GIVEN: A mocked out Settings() object
+    # WHEN: parsing the path
-        with patch('openlp.core.ui.media.Settings.value') as mocked_value:
+    (device_path, title_track, audio_track, subtitle_track, start, end, name) = parse_optical_path(path)
-            mocked_value.side_effect = value_results
 
-            # WHEN: get_media_players() is called
+    # THEN: The return values should match the original values
-            used_players, overridden_player = 'vlc'
+    assert org_title_track == int(title_track), 'Returned title_track should match the original'
+    assert org_audio_track == audio_track, 'Returned audio_track should match the original'
+    assert org_subtitle_track == int(subtitle_track), 'Returned subtitle_track should match the original'
+    assert org_start == start, 'Returned start should match the original'
+    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'
 
-            # THEN: the used_players should be an empty list, and the overridden player should be an empty string
-            assert [] == used_players, 'Used players should be an empty list'
-            assert '' == overridden_player, 'Overridden player should be an empty string'
 
-    @skip
+def test_parse_optical_path_win():
-    def test_get_media_players_no_players(self):
+    """
-        """
+    Test that test_parse_optical_path() parses a optical path with windows device path correctly
-        Test that when there's no players but overridden player is set, get_media_players() returns 'auto'
+    """
-        """
-        def value_results(key):
-            if key == 'media/override player':
-                return QtCore.Qt.Checked
-            else:
-                return ''
 
-        # GIVEN: A mocked out Settings() object
+    # GIVEN: An optical formatted path
-        with patch('openlp.core.ui.media.Settings.value') as mocked_value:
+    org_title_track = 1
-            mocked_value.side_effect = value_results
+    org_audio_track = 2
+    org_subtitle_track = -1
+    org_start = 1234
+    org_end = 4321
+    org_name = 'test name'
+    org_device_path = 'D:'
+    path = 'optical:%d:%d:%d:%d:%d:%s:%s' % (org_title_track, org_audio_track, org_subtitle_track,
+                                             org_start, org_end, org_name, org_device_path)
 
-            # WHEN: get_media_players() is called
+    # WHEN: parsing the path
-            used_players, overridden_player = 'vlc'
+    (device_path, title_track, audio_track, subtitle_track, start, end, name) = parse_optical_path(path)
 
-            # THEN: the used_players should be an empty list, and the overridden player should be an empty string
+    # THEN: The return values should match the original values
-            assert [] == used_players, 'Used players should be an empty list'
+    assert org_title_track == int(title_track), 'Returned title_track should match the original'
-            assert 'auto' == overridden_player, 'Overridden player should be "auto"'
+    assert org_audio_track == audio_track, 'Returned audio_track should match the original'
-
+    assert org_subtitle_track == int(subtitle_track), 'Returned subtitle_track should match the original'
-    @skip
+    assert org_start == start, 'Returned start should match the original'
-    def test_get_media_players_with_valid_list(self):
+    assert org_end == end, 'Returned end should match the original'
-        """
+    assert org_name == name, 'Returned end should match the original'
-        Test that when get_media_players() is called the string list is interpreted correctly
+    assert org_device_path == device_path, 'Returned device_path should match the original'
-        """
-        def value_results(key):
-            if key == 'media/players':
-                return '[vlc]'
-            else:
-                return False
-
-        # GIVEN: A mocked out Settings() object
-        with patch('openlp.core.ui.media.Settings.value') as mocked_value:
-            mocked_value.side_effect = value_results
-
-            # WHEN: get_media_players() is called
-            used_players = 'vlc'
-
-            # THEN: the used_players should be an empty list, and the overridden player should be an empty string
-            assert ['vlc', 'webkit', 'system'] == used_players, 'Used players should be correct'
-
-    @skip
-    def test_get_media_players_with_overridden_player(self):
-        """
-        Test that when get_media_players() is called the overridden player is correctly set
-        """
-        def value_results(key):
-            if key == 'media/players':
-                return '[vlc]'
-            else:
-                return QtCore.Qt.Checked
-
-        # GIVEN: A mocked out Settings() object
-        with patch('openlp.core.ui.media.Settings.value') as mocked_value:
-            mocked_value.side_effect = value_results
-
-            # WHEN: get_media_players() is called
-            used_players = 'vlc'
-
-            # THEN: the used_players should be an empty list, and the overridden player should be an empty string
-            assert ['vlc'] == used_players, 'Used players should be correct'
-
-    def test_parse_optical_path_linux(self):
-        """
-        Test that test_parse_optical_path() parses a optical path with linux device path correctly
-        """
-
-        # GIVEN: An optical formatted path
-        org_title_track = 1
-        org_audio_track = 2
-        org_subtitle_track = -1
-        org_start = 1234
-        org_end = 4321
-        org_name = 'test name'
-        org_device_path = '/dev/dvd'
-        path = 'optical:%d:%d:%d:%d:%d:%s:%s' % (org_title_track, org_audio_track, org_subtitle_track,
-                                                 org_start, org_end, org_name, org_device_path)
-
-        # WHEN: parsing the path
-        (device_path, title_track, audio_track, subtitle_track, start, end, name) = parse_optical_path(path)
-
-        # THEN: The return values should match the original values
-        assert org_title_track == int(title_track), 'Returned title_track should match the original'
-        assert org_audio_track == audio_track, 'Returned audio_track should match the original'
-        assert org_subtitle_track == int(subtitle_track), 'Returned subtitle_track should match the original'
-        assert org_start == start, 'Returned start should match the original'
-        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_parse_optical_path_win(self):
-        """
-        Test that test_parse_optical_path() parses a optical path with windows device path correctly
-        """
-
-        # GIVEN: An optical formatted path
-        org_title_track = 1
-        org_audio_track = 2
-        org_subtitle_track = -1
-        org_start = 1234
-        org_end = 4321
-        org_name = 'test name'
-        org_device_path = 'D:'
-        path = 'optical:%d:%d:%d:%d:%d:%s:%s' % (org_title_track, org_audio_track, org_subtitle_track,
-                                                 org_start, org_end, org_name, org_device_path)
-
-        # WHEN: parsing the path
-        (device_path, title_track, audio_track, subtitle_track, start, end, name) = parse_optical_path(path)
-
-        # THEN: The return values should match the original values
-        assert org_title_track == int(title_track), 'Returned title_track should match the original'
-        assert org_audio_track == audio_track, 'Returned audio_track should match the original'
-        assert org_subtitle_track == int(subtitle_track), 'Returned subtitle_track should match the original'
-        assert org_start == start, 'Returned start should match the original'
-        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'

tests/functional/openlp_plugins/bibles/test_mediaitem.py

@@ -738,14 +738,14 @@ class TestMediaItem(TestCase, TestMixin):
         Test on_style_combo_box_index_changed
         """
         # GIVEN: An instance of :class:`MediaManagerItem` a mocked media_item.settings
-        self.media_item.settings = MagicMock()
+        self.media_item.settings_tab = MagicMock()
 
         # WHEN: Calling on_style_combo_box_index_changed
         self.media_item.on_style_combo_box_index_changed(2)
 
         # THEN: The layout_style setting should have been set
-        assert self.media_item.settings.layout_style == 2
+        assert self.media_item.settings_tab.layout_style == 2
-        self.media_item.settings.layout_style_combo_box.setCurrentIndex.assert_called_once_with(2)
+        self.media_item.settings_tab.layout_style_combo_box.setCurrentIndex.assert_called_once_with(2)
         self.mocked_settings_instance.setValue.assert_called_once_with('bibles/verse layout style', 2)
 
     def test_on_version_combo_box_index_changed_no_bible(self):