Migrate plugins from Settings() Alert and Bibles

Signed-off-by: Tim <tim.bentley@gmail.com>

openlp/plugins/alerts/lib/alertstab.py

@@ -22,7 +22,6 @@
 from PyQt5 import QtGui, QtWidgets
 
 from openlp.core.common.i18n import UiStrings, translate
-from openlp.core.common.settings import Settings
 from openlp.core.lib.settingstab import SettingsTab
 from openlp.core.lib.ui import create_valign_selection_widgets
 from openlp.core.widgets.buttons import ColorButton
@@ -188,17 +187,16 @@ class AlertsTab(SettingsTab):
         """
         Load the settings into the UI.
         """
-        settings = Settings()
+        self.settings.beginGroup(self.settings_section)
-        settings.beginGroup(self.settings_section)
+        self.timeout = self.settings.value('timeout')
-        self.timeout = settings.value('timeout')
+        self.font_color = self.settings.value('font color')
-        self.font_color = settings.value('font color')
+        self.font_size = self.settings.value('font size')
-        self.font_size = settings.value('font size')
+        self.background_color = self.settings.value('background color')
-        self.background_color = settings.value('background color')
+        self.font_face = self.settings.value('font face')
-        self.font_face = settings.value('font face')
+        self.location = self.settings.value('location')
-        self.location = settings.value('location')
+        self.repeat = self.settings.value('repeat')
-        self.repeat = settings.value('repeat')
+        self.scroll = self.settings.value('scroll')
-        self.scroll = settings.value('scroll')
+        self.settings.endGroup()
-        settings.endGroup()
         self.font_size_spin_box.setValue(self.font_size)
         self.timeout_spin_box.setValue(self.timeout)
         self.font_color_button.color = self.font_color
@@ -217,22 +215,21 @@ class AlertsTab(SettingsTab):
         """
         Save the changes on exit of the Settings dialog.
         """
-        settings = Settings()
+        self.settings.beginGroup(self.settings_section)
-        settings.beginGroup(self.settings_section)
         # Check value has changed as no event handles this field
-        if settings.value('location') != self.vertical_combo_box.currentIndex():
+        if self.settings.value('location') != self.vertical_combo_box.currentIndex():
             self.changed = True
-        settings.setValue('background color', self.background_color)
+        self.settings.setValue('background color', self.background_color)
-        settings.setValue('font color', self.font_color)
+        self.settings.setValue('font color', self.font_color)
-        settings.setValue('font size', self.font_size)
+        self.settings.setValue('font size', self.font_size)
         self.font_face = self.font_combo_box.currentFont().family()
-        settings.setValue('font face', self.font_face)
+        self.settings.setValue('font face', self.font_face)
-        settings.setValue('timeout', self.timeout)
+        self.settings.setValue('timeout', self.timeout)
         self.location = self.vertical_combo_box.currentIndex()
-        settings.setValue('location', self.location)
+        self.settings.setValue('location', self.location)
-        settings.setValue('repeat', self.repeat)
+        self.settings.setValue('repeat', self.repeat)
-        settings.setValue('scroll', self.scroll_check_box.isChecked())
+        self.settings.setValue('scroll', self.scroll_check_box.isChecked())
-        settings.endGroup()
+        self.settings.endGroup()
         if self.changed:
             self.settings_form.register_post_process('update_display_css')
         self.changed = False

openlp/plugins/bibles/lib/__init__.py

@@ -27,7 +27,7 @@ import re
 
 from openlp.core.common import Singleton
 from openlp.core.common.i18n import translate
-from openlp.core.common.settings import Settings
+from openlp.core.common.registry import Registry
 
 log = logging.getLogger(__name__)
 
@@ -156,14 +156,12 @@ def update_reference_separators():
                                              'Genesis Chapter 1 Verses 1 To 2 And Verses 4 To 5')]),
         '|'.join([translate('BiblesPlugin', 'end', 'ending identifier e.g. Genesis 1 verse 1 - end = '
                                                    'Genesis Chapter 1 Verses 1 To The Last Verse')])]
-    settings = Settings()
+    settings = Registry().get('settings')
-    settings.beginGroup('bibles')
     custom_separators = [
         settings.value('verse separator'),
         settings.value('range separator'),
         settings.value('list separator'),
         settings.value('end separator')]
-    settings.endGroup()
     for index, role in enumerate(['v', 'r', 'l', 'e']):
         if custom_separators[index].strip('|') == '':
             source_string = default_separators[index].strip('|')

openlp/plugins/bibles/lib/biblestab.py

@@ -26,7 +26,6 @@ from PyQt5 import QtCore, QtGui, QtWidgets
 from openlp.core.common.enum import DisplayStyle, LanguageSelection, LayoutStyle
 from openlp.core.common.i18n import UiStrings, translate
 from openlp.core.common.registry import Registry
-from openlp.core.common.settings import Settings
 from openlp.core.lib.settingstab import SettingsTab
 from openlp.core.lib.ui import find_and_set_in_combo_box
 from openlp.plugins.bibles.lib import get_reference_separator, update_reference_separators
@@ -354,21 +353,20 @@ class BiblesTab(SettingsTab):
         self.bible_search_while_typing = (check_state == QtCore.Qt.Checked)
 
     def load(self):
-        settings = Settings()
+        self.settings.beginGroup(self.settings_section)
-        settings.beginGroup(self.settings_section)
+        self.is_verse_number_visible = self.settings.value('is verse number visible')
-        self.is_verse_number_visible = settings.value('is verse number visible')
+        self.show_new_chapters = self.settings.value('display new chapter')
-        self.show_new_chapters = settings.value('display new chapter')
+        self.display_style = self.settings.value('display brackets')
-        self.display_style = settings.value('display brackets')
+        self.layout_style = self.settings.value('verse layout style')
-        self.layout_style = settings.value('verse layout style')
+        self.bible_theme = self.settings.value('bible theme')
-        self.bible_theme = settings.value('bible theme')
+        self.second_bibles = self.settings.value('second bibles')
-        self.second_bibles = settings.value('second bibles')
         self.is_verse_number_visible_check_box.setChecked(self.is_verse_number_visible)
         self.check_is_verse_number_visible()
         self.new_chapters_check_box.setChecked(self.show_new_chapters)
         self.display_style_combo_box.setCurrentIndex(self.display_style)
         self.layout_style_combo_box.setCurrentIndex(self.layout_style)
         self.bible_second_check_box.setChecked(self.second_bibles)
-        verse_separator = settings.value('verse separator')
+        verse_separator = self.settings.value('verse separator')
         if (verse_separator.strip('|') == '') or (verse_separator == get_reference_separator('sep_v_default')):
             self.verse_separator_line_edit.setText(get_reference_separator('sep_v_default'))
             self.verse_separator_line_edit.setPalette(self.get_grey_text_palette(True))
@@ -377,7 +375,7 @@ class BiblesTab(SettingsTab):
             self.verse_separator_line_edit.setText(verse_separator)
             self.verse_separator_line_edit.setPalette(self.get_grey_text_palette(False))
             self.verse_separator_check_box.setChecked(True)
-        range_separator = settings.value('range separator')
+        range_separator = self.settings.value('range separator')
         if (range_separator.strip('|') == '') or (range_separator == get_reference_separator('sep_r_default')):
             self.range_separator_line_edit.setText(get_reference_separator('sep_r_default'))
             self.range_separator_line_edit.setPalette(self.get_grey_text_palette(True))
@@ -386,7 +384,7 @@ class BiblesTab(SettingsTab):
             self.range_separator_line_edit.setText(range_separator)
             self.range_separator_line_edit.setPalette(self.get_grey_text_palette(False))
             self.range_separator_check_box.setChecked(True)
-        list_separator = settings.value('list separator')
+        list_separator = self.settings.value('list separator')
         if (list_separator.strip('|') == '') or (list_separator == get_reference_separator('sep_l_default')):
             self.list_separator_line_edit.setText(get_reference_separator('sep_l_default'))
             self.list_separator_line_edit.setPalette(self.get_grey_text_palette(True))
@@ -395,7 +393,7 @@ class BiblesTab(SettingsTab):
             self.list_separator_line_edit.setText(list_separator)
             self.list_separator_line_edit.setPalette(self.get_grey_text_palette(False))
             self.list_separator_check_box.setChecked(True)
-        end_separator = settings.value('end separator')
+        end_separator = self.settings.value('end separator')
         if (end_separator.strip('|') == '') or (end_separator == get_reference_separator('sep_e_default')):
             self.end_separator_line_edit.setText(get_reference_separator('sep_e_default'))
             self.end_separator_line_edit.setPalette(self.get_grey_text_palette(True))
@@ -404,49 +402,48 @@ class BiblesTab(SettingsTab):
             self.end_separator_line_edit.setText(end_separator)
             self.end_separator_line_edit.setPalette(self.get_grey_text_palette(False))
             self.end_separator_check_box.setChecked(True)
-        self.language_selection = settings.value('book name language')
+        self.language_selection = self.settings.value('book name language')
         self.language_selection_combo_box.setCurrentIndex(self.language_selection)
-        self.reset_to_combined_quick_search = settings.value('reset to combined quick search')
+        self.reset_to_combined_quick_search = self.settings.value('reset to combined quick search')
         self.reset_to_combined_quick_search_check_box.setChecked(self.reset_to_combined_quick_search)
-        self.hide_combined_quick_error = settings.value('hide combined quick error')
+        self.hide_combined_quick_error = self.settings.value('hide combined quick error')
         self.hide_combined_quick_error_check_box.setChecked(self.hide_combined_quick_error)
-        self.bible_search_while_typing = settings.value('is search while typing enabled')
+        self.bible_search_while_typing = self.settings.value('is search while typing enabled')
         self.bible_search_while_typing_check_box.setChecked(self.bible_search_while_typing)
-        settings.endGroup()
+        self.settings.endGroup()
 
     def save(self):
-        settings = Settings()
+        self.self.settings.beginGroup(self.settings_section)
-        settings.beginGroup(self.settings_section)
+        self.settings.setValue('is verse number visible', self.is_verse_number_visible)
-        settings.setValue('is verse number visible', self.is_verse_number_visible)
+        self.settings.setValue('display new chapter', self.show_new_chapters)
-        settings.setValue('display new chapter', self.show_new_chapters)
+        self.settings.setValue('display brackets', self.display_style)
-        settings.setValue('display brackets', self.display_style)
+        self.settings.setValue('verse layout style', self.layout_style)
-        settings.setValue('verse layout style', self.layout_style)
+        self.settings.setValue('second bibles', self.second_bibles)
-        settings.setValue('second bibles', self.second_bibles)
+        self.settings.setValue('bible theme', self.bible_theme)
-        settings.setValue('bible theme', self.bible_theme)
         if self.verse_separator_check_box.isChecked():
-            settings.setValue('verse separator', self.verse_separator_line_edit.text())
+            self.settings.setValue('verse separator', self.verse_separator_line_edit.text())
         else:
-            settings.remove('verse separator')
+            self.settings.remove('verse separator')
         if self.range_separator_check_box.isChecked():
-            settings.setValue('range separator', self.range_separator_line_edit.text())
+            self.settings.setValue('range separator', self.range_separator_line_edit.text())
         else:
-            settings.remove('range separator')
+            self.settings.remove('range separator')
         if self.list_separator_check_box.isChecked():
-            settings.setValue('list separator', self.list_separator_line_edit.text())
+            self.settings.setValue('list separator', self.list_separator_line_edit.text())
         else:
-            settings.remove('list separator')
+            self.settings.remove('list separator')
         if self.end_separator_check_box.isChecked():
-            settings.setValue('end separator', self.end_separator_line_edit.text())
+            self.settings.setValue('end separator', self.end_separator_line_edit.text())
         else:
-            settings.remove('end separator')
+            self.settings.remove('end separator')
         update_reference_separators()
-        if self.language_selection != settings.value('book name language'):
+        if self.language_selection != self.settings.value('book name language'):
-            settings.setValue('book name language', self.language_selection)
+            self.settings.setValue('book name language', self.language_selection)
             self.settings_form.register_post_process('bibles_load_list')
-        settings.setValue('reset to combined quick search', self.reset_to_combined_quick_search)
+        self.settings.setValue('reset to combined quick search', self.reset_to_combined_quick_search)
-        settings.setValue('hide combined quick error', self.hide_combined_quick_error)
+        self.settings.setValue('hide combined quick error', self.hide_combined_quick_error)
-        settings.setValue('is search while typing enabled', self.bible_search_while_typing)
+        self.settings.setValue('is search while typing enabled', self.bible_search_while_typing)
-        settings.endGroup()
+        self.settings.endGroup()
         if self.tab_visited:
             self.settings_form.register_post_process('bibles_config_updated')
         self.tab_visited = False

openlp/plugins/bibles/lib/manager.py

@@ -26,7 +26,7 @@ from openlp.core.common.enum import LanguageSelection
 from openlp.core.common.applocation import AppLocation
 from openlp.core.common.i18n import UiStrings, translate
 from openlp.core.common.mixins import LogMixin, RegistryProperties
-from openlp.core.common.settings import Settings
+from openlp.core.common.registry import Registry
 from openlp.plugins.bibles.lib import parse_reference
 from openlp.plugins.bibles.lib.db import BibleDB, BibleMeta
 
@@ -296,7 +296,7 @@ class BibleManager(LogMixin, RegistryProperties):
         if not language_selection or language_selection.value == "None" or language_selection.value == "-1":
             # If None is returned, it's not the singleton object but a
             # BibleMeta object with the value "None"
-            language_selection = Settings().value(self.settings_section + '/book name language')
+            language_selection = Registry().get('settings').value(self.settings_section + '/book name language')
         else:
             language_selection = language_selection.value
         try:

openlp/plugins/bibles/lib/upgrade.py

@@ -28,7 +28,8 @@ from sqlalchemy import Table
 from sqlalchemy.sql.expression import delete, select
 
 from openlp.core.common.i18n import translate
-from openlp.core.common.settings import ProxyMode, Settings
+from openlp.core.common.registry import Registry
+from openlp.core.common.settings import ProxyMode
 from openlp.core.lib.db import get_upgrade_op
 
 
@@ -51,7 +52,7 @@ def upgrade_2(session, metadata):
     Remove the individual proxy settings, after the implementation of central proxy settings.
     Added in 2.5 (3.0 development)
     """
-    settings = Settings()
+    settings = Registry().get('settings')
     op = get_upgrade_op(session)
     metadata_table = Table('metadata', metadata, autoload=True)
     proxy, = session.execute(select([metadata_table.c.value], metadata_table.c.key == 'proxy_server')).first() or ('', )

tests/functional/openlp_plugins/bibles/test_lib.py

@@ -21,228 +21,234 @@
 """
 This module contains tests for the lib submodule of the Bibles plugin.
 """
-from unittest import TestCase
+import pytest
 from unittest.mock import MagicMock, patch
 
+from openlp.core.common.registry import Registry
 from openlp.plugins.bibles import lib
 from openlp.plugins.bibles.lib import SearchResults, get_reference_match
-from tests.helpers.testmixin import TestMixin
 
 
-class TestLib(TestCase, TestMixin):
+@pytest.yield_fixture
+def mocked_bible_test(registry):
+    """Local test setup"""
+    ret_value = MagicMock(**{'value.return_value': ''})
+    Registry().register('settings', ret_value)
+
+
+@patch('openlp.plugins.bibles.lib.update_reference_separators')
+def test_get_reference_separator(mocked_update_reference_separators):
     """
-    Test the functions in the :mod:`lib` module.
+    Test the get_reference_separator method
     """
-    @patch('openlp.plugins.bibles.lib.update_reference_separators')
+    # GIVEN: A list of expected separators and the lib module's constant is empty
-    def test_get_reference_separator(self, mocked_update_reference_separators):
+    lib.REFERENCE_SEPARATORS = None
+    separators = {'sep_r': '\\s*(?:e)\\s*', 'sep_e_default': 'end', 'sep_v_display': 'w', 'sep_l_display': 'r',
+                  'sep_v_default': ':|v|V|verse|verses', 'sep_l': '\\s*(?:r)\\s*', 'sep_l_default': ',|and',
+                  'sep_e': '\\s*(?:t)\\s*', 'sep_v': '\\s*(?:w)\\s*', 'sep_r_display': 'e', 'sep_r_default': '-|to'}
+
+    def _update_side_effect():
         """
-        Test the get_reference_separator method
+        Update the references after mocking out the method
         """
-        # GIVEN: A list of expected separators and the lib module's constant is empty
+        lib.REFERENCE_SEPARATORS = separators
-        lib.REFERENCE_SEPARATORS = None
-        separators = {'sep_r': '\\s*(?:e)\\s*', 'sep_e_default': 'end', 'sep_v_display': 'w', 'sep_l_display': 'r',
-                      'sep_v_default': ':|v|V|verse|verses', 'sep_l': '\\s*(?:r)\\s*', 'sep_l_default': ',|and',
-                      'sep_e': '\\s*(?:t)\\s*', 'sep_v': '\\s*(?:w)\\s*', 'sep_r_display': 'e', 'sep_r_default': '-|to'}
 
-        def _update_side_effect():
+    mocked_update_reference_separators.side_effect = _update_side_effect
-            """
-            Update the references after mocking out the method
-            """
-            lib.REFERENCE_SEPARATORS = separators
 
-        mocked_update_reference_separators.side_effect = _update_side_effect
+    # WHEN: Calling get_reference_separator
+    for key, value in separators.items():
+        lib.get_reference_separator(key)
 
-        # WHEN: Calling get_reference_separator
+        # THEN: get_reference_separator should return the correct separator
-        for key, value in separators.items():
+        assert separators[key] == value
-            lib.get_reference_separator(key)
+    mocked_update_reference_separators.assert_called_once_with()
 
-            # THEN: get_reference_separator should return the correct separator
-            assert separators[key] == value
-        mocked_update_reference_separators.assert_called_once_with()
 
-    def test_reference_matched_full(self):
+def test_search_results_creation():
-        """
+    """
-        Test that the 'full' regex parses bible verse references correctly.
+    Test the creation and construction of the SearchResults class
-        """
+    """
-        # GIVEN: Some test data which contains different references to parse, with the expected results.
+    # GIVEN: A book, chapter and a verse list
-        with patch('openlp.plugins.bibles.lib.Settings', return_value=MagicMock(**{'value.return_value': ''})):
+    book = 'Genesis'
-            # The following test data tests with about 240 variants when using the default 'separators'
+    chapter = 1
-            # The amount is exactly 222 without '1. John 23' and'1. John. 23'
+    verse_list = {
-            test_data = [
+        1: 'In the beginning God created the heavens and the earth.',
-                # Input reference, book name, chapter + verse reference
+        2: 'The earth was without form and void, and darkness was over the face of the deep. And the Spirit of '
-                ('Psalm 23', 'Psalm', '23'),
+           'God was hovering over the face of the waters.'
-                ('Psalm. 23', 'Psalm', '23'),
+    }
-                ('Psalm 23{to}24', 'Psalm', '23-24'),
-                ('Psalm 23{verse}1{to}2', 'Psalm', '23:1-2'),
-                ('Psalm 23{verse}1{to}{end}', 'Psalm', '23:1-end'),
-                ('Psalm 23{verse}1{to}2{_and}5{to}6', 'Psalm', '23:1-2,5-6'),
-                ('Psalm 23{verse}1{to}2{_and}5{to}{end}', 'Psalm', '23:1-2,5-end'),
-                ('Psalm 23{verse}1{to}2{_and}24{verse}1{to}3', 'Psalm', '23:1-2,24:1-3'),
-                ('Psalm 23{verse}1{to}{end}{_and}24{verse}1{to}{end}', 'Psalm', '23:1-end,24:1-end'),
-                ('Psalm 23{verse}1{to}24{verse}1', 'Psalm', '23:1-24:1'),
-                ('Psalm 23{_and}24', 'Psalm', '23,24'),
-                ('1 John 23', '1 John', '23'),
-                ('1 John. 23', '1 John', '23'),
-                ('1. John 23', '1. John', '23'),
-                ('1. John. 23', '1. John', '23'),
-                ('1 John 23{to}24', '1 John', '23-24'),
-                ('1 John 23{verse}1{to}2', '1 John', '23:1-2'),
-                ('1 John 23{verse}1{to}{end}', '1 John', '23:1-end'),
-                ('1 John 23{verse}1{to}2{_and}5{to}6', '1 John', '23:1-2,5-6'),
-                ('1 John 23{verse}1{to}2{_and}5{to}{end}', '1 John', '23:1-2,5-end'),
-                ('1 John 23{verse}1{to}2{_and}24{verse}1{to}3', '1 John', '23:1-2,24:1-3'),
-                ('1 John 23{verse}1{to}{end}{_and}24{verse}1{to}{end}', '1 John', '23:1-end,24:1-end'),
-                ('1 John 23{verse}1{to}24{verse}1', '1 John', '23:1-24:1'),
-                ('1 John 23{_and}24', '1 John', '23,24')]
 
-            full_reference_match = get_reference_match('full')
+    # WHEN: We create the search results object
-            for reference_text, book_result, ranges_result in test_data:
+    search_results = SearchResults(book, chapter, verse_list)
-                to_separators = ['-', ' - ', 'to', ' to '] if '{to}' in reference_text else ['']
-                verse_separators = [':', ' : ', 'v', ' v ', 'V', ' V ', 'verse', ' verse ', 'verses', ' verses '] \
-                    if '{verse}' in reference_text else ['']
-                and_separators = [',', ' , ', 'and', ' and '] if '{_and}' in reference_text else ['']
-                end_separators = ['end', ' end '] if '{end}' in reference_text else ['']
 
-                for to in to_separators:
+    # THEN: It should have a book, a chapter and a verse list
-                    for verse in verse_separators:
+    assert search_results is not None, 'The search_results object should not be None'
-                        for _and in and_separators:
+    assert search_results.book == book, 'The book should be "Genesis"'
-                            for end in end_separators:
+    assert search_results.chapter == chapter, 'The chapter should be 1'
-                                reference_text = reference_text.format(to=to, verse=verse, _and=_and, end=end)
+    assert search_results.verse_list == verse_list, 'The verse lists should be identical'
 
-                                # WHEN: Attempting to parse the input string
-                                match = full_reference_match.match(reference_text)
 
-                                # THEN: A match should be returned, and the book and reference should match the
+def test_search_results_has_verse_list():
-                                #       expected result
+    """
-                                assert match is not None, '{text} should provide a match'.format(text=reference_text)
+    Test that a SearchResults object with a valid verse list returns True when checking ``has_verse_list()``
-                                assert book_result == match.group('book'), \
+    """
-                                    '{text} does not provide the expected result for the book group.'\
+    # GIVEN: A valid SearchResults object with a proper verse list
-                                    .format(text=reference_text)
+    search_results = SearchResults('Genesis', 1, {1: 'In the beginning God created the heavens and the earth.'})
-                                assert ranges_result == match.group('ranges'), \
-                                    '{text} does not provide the expected result for the ranges group.' \
-                                    .format(text=reference_text)
 
-    def test_reference_matched_range(self):
+    # WHEN: We check that the SearchResults object has a verse list
-        """
+    has_verse_list = search_results.has_verse_list()
-        Test that the 'range' regex parses bible verse references correctly.
-        Note: This test takes in to account that the regex does not work quite as expected!
-        see https://gitlab.com/openlp/openlp/issues/240
-        """
-        # GIVEN: Some test data which contains different references to parse, with the expected results.
-        with patch('openlp.plugins.bibles.lib.Settings', return_value=MagicMock(**{'value.return_value': ''})):
-            # The following test data tests with 45 variants when using the default 'separators'
-            test_data = [
-                ('23', None, '23', None, None, None),
-                ('23{to}24', None, '23', '-24', None, '24'),
-                ('23{verse}1{to}2', '23', '1', '-2', None, '2'),
-                ('23{verse}1{to}{end}', '23', '1', '-end', None, None),
-                ('23{verse}1{to}24{verse}1', '23', '1', '-24:1', '24', '1')]
-            full_reference_match = get_reference_match('range')
-            for reference_text, from_chapter, from_verse, range_to, to_chapter, to_verse in test_data:
-                to_separators = ['-', ' - ', 'to', ' to '] if '{to}' in reference_text else ['']
-                verse_separators = [':', ' : ', 'v', ' v ', 'V', ' V ', 'verse', ' verse ', 'verses', ' verses '] \
-                    if '{verse}' in reference_text else ['']
-                and_separators = [',', ' , ', 'and', ' and '] if '{_and}' in reference_text else ['']
-                end_separators = ['end', ' end '] if '{end}' in reference_text else ['']
 
-                for to in to_separators:
+    # THEN: It should be True
-                    for verse in verse_separators:
+    assert has_verse_list is True, 'The SearchResults object should have a verse list'
-                        for _and in and_separators:
-                            for end in end_separators:
-                                reference_text = reference_text.format(to=to, verse=verse, _and=_and, end=end)
 
-                                # WHEN: Attempting to parse the input string
-                                match = full_reference_match.match(reference_text)
 
-                                # THEN: A match should be returned, and the to/from chapter/verses should match as
+def test_search_results_has_no_verse_list():
-                                #       expected
+    """
-                                assert match is not None, '{text} should provide a match'.format(text=reference_text)
+    Test that a SearchResults object with an empty verse list returns False when checking ``has_verse_list()``
-                                assert match.group('from_chapter') == from_chapter
+    """
-                                assert match.group('from_verse') == from_verse
+    # GIVEN: A valid SearchResults object with an empty verse list
-                                assert match.group('range_to') == range_to
+    search_results = SearchResults('Genesis', 1, {})
-                                assert match.group('to_chapter') == to_chapter
-                                assert match.group('to_verse') == to_verse
 
-    def test_reference_matched_range_separator(self):
+    # WHEN: We check that the SearchResults object has a verse list
-        # GIVEN: Some test data which contains different references to parse, with the expected results.
+    has_verse_list = search_results.has_verse_list()
-        with patch('openlp.plugins.bibles.lib.Settings', return_value=MagicMock(**{'value.return_value': ''})):
-            # The following test data tests with 111 variants when using the default 'separators'
-            # The regex for handling ranges is a bit screwy, see https://gitlab.com/openlp/openlp/issues/240
-            test_data = [
-                ('23', ['23']),
-                ('23{to}24', ['23-24']),
-                ('23{verse}1{to}2', ['23:1-2']),
-                ('23{verse}1{to}{end}', ['23:1-end']),
-                ('23{verse}1{to}2{_and}5{to}6', ['23:1-2', '5-6']),
-                ('23{verse}1{to}2{_and}5{to}{end}', ['23:1-2', '5-end']),
-                ('23{verse}1{to}2{_and}24{verse}1{to}3', ['23:1-2', '24:1-3']),
-                ('23{verse}1{to}{end}{_and}24{verse}1{to}{end}', ['23:1-end', '24:1-end']),
-                ('23{verse}1{to}24{verse}1', ['23:1-24:1']),
-                ('23,24', ['23', '24'])]
-            full_reference_match = get_reference_match('range_separator')
-            for reference_text, ranges in test_data:
-                to_separators = ['-', ' - ', 'to', ' to '] if '{to}' in reference_text else ['']
-                verse_separators = [':', ' : ', 'v', ' v ', 'V', ' V ', 'verse', ' verse ', 'verses', ' verses '] \
-                    if '{verse}' in reference_text else ['']
-                and_separators = [',', ' , ', 'and', ' and '] if '{_and}' in reference_text else ['']
-                end_separators = ['end', ' end '] if '{end}' in reference_text else ['']
 
-                for to in to_separators:
+    # THEN: It should be False
-                    for verse in verse_separators:
+    assert has_verse_list is False, 'The SearchResults object should have a verse list'
-                        for _and in and_separators:
-                            for end in end_separators:
-                                reference_text = reference_text.format(to=to, verse=verse, _and=_and, end=end)
 
-                                # WHEN: Attempting to parse the input string
-                                references = full_reference_match.split(reference_text)
 
-                                # THEN: The list of references should be as the expected results
+def test_reference_matched_full(mocked_bible_test):
-                                assert references == ranges
+    """
+    Test that the 'full' regex parses bible verse references correctly.
+    """
+    # GIVEN: Some test data which contains different references to parse, with the expected results.
+    # The following test data tests with about 240 variants when using the default 'separators'
+    # The amount is exactly 222 without '1. John 23' and'1. John. 23'
+    test_data = [
+        # Input reference, book name, chapter + verse reference
+        ('Psalm 23', 'Psalm', '23'),
+        ('Psalm. 23', 'Psalm', '23'),
+        ('Psalm 23{to}24', 'Psalm', '23-24'),
+        ('Psalm 23{verse}1{to}2', 'Psalm', '23:1-2'),
+        ('Psalm 23{verse}1{to}{end}', 'Psalm', '23:1-end'),
+        ('Psalm 23{verse}1{to}2{_and}5{to}6', 'Psalm', '23:1-2,5-6'),
+        ('Psalm 23{verse}1{to}2{_and}5{to}{end}', 'Psalm', '23:1-2,5-end'),
+        ('Psalm 23{verse}1{to}2{_and}24{verse}1{to}3', 'Psalm', '23:1-2,24:1-3'),
+        ('Psalm 23{verse}1{to}{end}{_and}24{verse}1{to}{end}', 'Psalm', '23:1-end,24:1-end'),
+        ('Psalm 23{verse}1{to}24{verse}1', 'Psalm', '23:1-24:1'),
+        ('Psalm 23{_and}24', 'Psalm', '23,24'),
+        ('1 John 23', '1 John', '23'),
+        ('1 John. 23', '1 John', '23'),
+        ('1. John 23', '1. John', '23'),
+        ('1. John. 23', '1. John', '23'),
+        ('1 John 23{to}24', '1 John', '23-24'),
+        ('1 John 23{verse}1{to}2', '1 John', '23:1-2'),
+        ('1 John 23{verse}1{to}{end}', '1 John', '23:1-end'),
+        ('1 John 23{verse}1{to}2{_and}5{to}6', '1 John', '23:1-2,5-6'),
+        ('1 John 23{verse}1{to}2{_and}5{to}{end}', '1 John', '23:1-2,5-end'),
+        ('1 John 23{verse}1{to}2{_and}24{verse}1{to}3', '1 John', '23:1-2,24:1-3'),
+        ('1 John 23{verse}1{to}{end}{_and}24{verse}1{to}{end}', '1 John', '23:1-end,24:1-end'),
+        ('1 John 23{verse}1{to}24{verse}1', '1 John', '23:1-24:1'),
+        ('1 John 23{_and}24', '1 John', '23,24')]
 
-    def test_search_results_creation(self):
+    full_reference_match = get_reference_match('full')
-        """
+    for reference_text, book_result, ranges_result in test_data:
-        Test the creation and construction of the SearchResults class
+        to_separators = ['-', ' - ', 'to', ' to '] if '{to}' in reference_text else ['']
-        """
+        verse_separators = [':', ' : ', 'v', ' v ', 'V', ' V ', 'verse', ' verse ', 'verses', ' verses '] \
-        # GIVEN: A book, chapter and a verse list
+            if '{verse}' in reference_text else ['']
-        book = 'Genesis'
+        and_separators = [',', ' , ', 'and', ' and '] if '{_and}' in reference_text else ['']
-        chapter = 1
+        end_separators = ['end', ' end '] if '{end}' in reference_text else ['']
-        verse_list = {
-            1: 'In the beginning God created the heavens and the earth.',
-            2: 'The earth was without form and void, and darkness was over the face of the deep. And the Spirit of '
-               'God was hovering over the face of the waters.'
-        }
 
-        # WHEN: We create the search results object
+        for to in to_separators:
-        search_results = SearchResults(book, chapter, verse_list)
+            for verse in verse_separators:
+                for _and in and_separators:
+                    for end in end_separators:
+                        reference_text = reference_text.format(to=to, verse=verse, _and=_and, end=end)
 
-        # THEN: It should have a book, a chapter and a verse list
+                        # WHEN: Attempting to parse the input string
-        assert search_results is not None, 'The search_results object should not be None'
+                        match = full_reference_match.match(reference_text)
-        assert search_results.book == book, 'The book should be "Genesis"'
-        assert search_results.chapter == chapter, 'The chapter should be 1'
-        assert search_results.verse_list == verse_list, 'The verse lists should be identical'
 
-    def test_search_results_has_verse_list(self):
+                        # THEN: A match should be returned, and the book and reference should match the
-        """
+                        #       expected result
-        Test that a SearchResults object with a valid verse list returns True when checking ``has_verse_list()``
+                        assert match is not None, '{text} should provide a match'.format(text=reference_text)
-        """
+                        assert book_result == match.group('book'), \
-        # GIVEN: A valid SearchResults object with a proper verse list
+                            '{text} does not provide the expected result for the book group.'\
-        search_results = SearchResults('Genesis', 1, {1: 'In the beginning God created the heavens and the earth.'})
+                            .format(text=reference_text)
+                        assert ranges_result == match.group('ranges'), \
+                            '{text} does not provide the expected result for the ranges group.' \
+                            .format(text=reference_text)
 
-        # WHEN: We check that the SearchResults object has a verse list
-        has_verse_list = search_results.has_verse_list()
 
-        # THEN: It should be True
+def test_reference_matched_range(mocked_bible_test):
-        assert has_verse_list is True, 'The SearchResults object should have a verse list'
+    """
+    Test that the 'range' regex parses bible verse references correctly.
+    Note: This test takes in to account that the regex does not work quite as expected!
+    see https://gitlab.com/openlp/openlp/issues/240
+    """
+    # GIVEN: Some test data which contains different references to parse, with the expected results.
+    # The following test data tests with 45 variants when using the default 'separators'
+    test_data = [
+        ('23', None, '23', None, None, None),
+        ('23{to}24', None, '23', '-24', None, '24'),
+        ('23{verse}1{to}2', '23', '1', '-2', None, '2'),
+        ('23{verse}1{to}{end}', '23', '1', '-end', None, None),
+        ('23{verse}1{to}24{verse}1', '23', '1', '-24:1', '24', '1')]
+    full_reference_match = get_reference_match('range')
+    for reference_text, from_chapter, from_verse, range_to, to_chapter, to_verse in test_data:
+        to_separators = ['-', ' - ', 'to', ' to '] if '{to}' in reference_text else ['']
+        verse_separators = [':', ' : ', 'v', ' v ', 'V', ' V ', 'verse', ' verse ', 'verses', ' verses '] \
+            if '{verse}' in reference_text else ['']
+        and_separators = [',', ' , ', 'and', ' and '] if '{_and}' in reference_text else ['']
+        end_separators = ['end', ' end '] if '{end}' in reference_text else ['']
 
-    def test_search_results_has_no_verse_list(self):
+        for to in to_separators:
-        """
+            for verse in verse_separators:
-        Test that a SearchResults object with an empty verse list returns False when checking ``has_verse_list()``
+                for _and in and_separators:
-        """
+                    for end in end_separators:
-        # GIVEN: A valid SearchResults object with an empty verse list
+                        reference_text = reference_text.format(to=to, verse=verse, _and=_and, end=end)
-        search_results = SearchResults('Genesis', 1, {})
 
-        # WHEN: We check that the SearchResults object has a verse list
+                        # WHEN: Attempting to parse the input string
-        has_verse_list = search_results.has_verse_list()
+                        match = full_reference_match.match(reference_text)
 
-        # THEN: It should be False
+                        # THEN: A match should be returned, and the to/from chapter/verses should match as
-        assert has_verse_list is False, 'The SearchResults object should have a verse list'
+                        #       expected
+                        assert match is not None, '{text} should provide a match'.format(text=reference_text)
+                        assert match.group('from_chapter') == from_chapter
+                        assert match.group('from_verse') == from_verse
+                        assert match.group('range_to') == range_to
+                        assert match.group('to_chapter') == to_chapter
+                        assert match.group('to_verse') == to_verse
+
+
+def test_reference_matched_range_separator(mocked_bible_test):
+    # GIVEN: Some test data which contains different references to parse, with the expected results.
+    # The following test data tests with 111 variants when using the default 'separators'
+    # The regex for handling ranges is a bit screwy, see https://gitlab.com/openlp/openlp/issues/240
+    test_data = [
+        ('23', ['23']),
+        ('23{to}24', ['23-24']),
+        ('23{verse}1{to}2', ['23:1-2']),
+        ('23{verse}1{to}{end}', ['23:1-end']),
+        ('23{verse}1{to}2{_and}5{to}6', ['23:1-2', '5-6']),
+        ('23{verse}1{to}2{_and}5{to}{end}', ['23:1-2', '5-end']),
+        ('23{verse}1{to}2{_and}24{verse}1{to}3', ['23:1-2', '24:1-3']),
+        ('23{verse}1{to}{end}{_and}24{verse}1{to}{end}', ['23:1-end', '24:1-end']),
+        ('23{verse}1{to}24{verse}1', ['23:1-24:1']),
+        ('23,24', ['23', '24'])]
+    full_reference_match = get_reference_match('range_separator')
+    for reference_text, ranges in test_data:
+        to_separators = ['-', ' - ', 'to', ' to '] if '{to}' in reference_text else ['']
+        verse_separators = [':', ' : ', 'v', ' v ', 'V', ' V ', 'verse', ' verse ', 'verses', ' verses '] \
+            if '{verse}' in reference_text else ['']
+        and_separators = [',', ' , ', 'and', ' and '] if '{_and}' in reference_text else ['']
+        end_separators = ['end', ' end '] if '{end}' in reference_text else ['']
+
+        for to in to_separators:
+            for verse in verse_separators:
+                for _and in and_separators:
+                    for end in end_separators:
+                        reference_text = reference_text.format(to=to, verse=verse, _and=_and, end=end)
+
+                        # WHEN: Attempting to parse the input string
+                        references = full_reference_match.split(reference_text)
+
+                        # THEN: The list of references should be as the expected results
+                        assert references == ranges

tests/functional/openlp_plugins/bibles/test_manager.py

@@ -40,9 +40,6 @@ class TestManager(TestCase):
         log_patcher = patch('openlp.plugins.bibles.lib.manager.log')
         self.addCleanup(log_patcher.stop)
         self.mocked_log = log_patcher.start()
-        settings_patcher = patch('openlp.plugins.bibles.lib.manager.Settings')
-        self.addCleanup(settings_patcher.stop)
-        settings_patcher.start()
 
     def test_delete_bible(self):
         """

tests/functional/openlp_plugins/bibles/test_upgrade.py

@@ -29,6 +29,7 @@ from unittest.mock import MagicMock, call, patch
 
 from sqlalchemy import create_engine
 
+from openlp.core.common.registry import Registry
 from openlp.core.common.settings import ProxyMode
 from openlp.core.lib.db import upgrade_db
 from openlp.plugins.bibles.lib import upgrade
@@ -51,11 +52,9 @@ class TestUpgrade(TestCase, TestMixin):
         shutil.copyfile(db_path, db_tmp_path)
         self.db_url = 'sqlite:///' + str(db_tmp_path)
 
-        patched_settings = patch('openlp.plugins.bibles.lib.upgrade.Settings')
+        self.build_settings()
-        self.mocked_settings = patched_settings.start()
+        Registry().create()
-        self.addCleanup(patched_settings.stop)
+        Registry().register('service_list', MagicMock())
-        self.mocked_settings_instance = MagicMock()
-        self.mocked_settings.return_value = self.mocked_settings_instance
 
         patched_message_box = patch('openlp.plugins.bibles.lib.upgrade.QtWidgets.QMessageBox')
         self.mocked_message_box = patched_message_box.start()
@@ -67,6 +66,7 @@ class TestUpgrade(TestCase, TestMixin):
         """
         # Ignore errors since windows can have problems with locked files
         shutil.rmtree(self.tmp_path, ignore_errors=True)
+        self.destroy_settings()
 
     def test_upgrade_2_none_selected(self):
         """
@@ -99,12 +99,15 @@ class TestProxyMetaUpgrade(TestCase, TestMixin):
         shutil.copyfile(db_path, db_tmp_path)
         self.db_url = 'sqlite:///' + str(db_tmp_path)
 
-        patched_settings = patch('openlp.plugins.bibles.lib.upgrade.Settings')
-        self.mocked_settings = patched_settings.start()
-        self.addCleanup(patched_settings.stop)
         self.mocked_settings_instance = MagicMock()
+        self.mocked_settings = MagicMock()
         self.mocked_settings.return_value = self.mocked_settings_instance
 
+        self.build_settings()
+        Registry().create()
+        Registry().register('service_list', MagicMock())
+        Registry().register('settings', self.mocked_settings)
+
         patched_message_box = patch('openlp.plugins.bibles.lib.upgrade.QtWidgets.QMessageBox')
         mocked_message_box = patched_message_box.start()
         self.addCleanup(patched_message_box.stop)
@@ -162,7 +165,7 @@ class TestProxyMetaUpgrade(TestCase, TestMixin):
         assert len(conn.execute('SELECT * FROM metadata WHERE key = "proxy_password"').fetchall()) == 0
         assert conn.execute('SELECT * FROM metadata WHERE key = "version"').first().value == '2'
 
-        assert self.mocked_settings_instance.setValue.call_args_list == [
+        assert self.mocked_settings.setValue.call_args_list == [
             call('advanced/proxy http', 'proxy_server'), call('advanced/proxy username', 'proxy_username'),
             call('advanced/proxy password', 'proxy_password'), call('advanced/proxy mode', ProxyMode.MANUAL_PROXY)]
 
@@ -185,7 +188,7 @@ class TestProxyMetaUpgrade(TestCase, TestMixin):
         assert len(conn.execute('SELECT * FROM metadata WHERE key = "proxy_password"').fetchall()) == 0
         assert conn.execute('SELECT * FROM metadata WHERE key = "version"').first().value == '2'
 
-        assert self.mocked_settings_instance.setValue.call_args_list == [
+        assert self.mocked_settings.setValue.call_args_list == [
             call('advanced/proxy https', 'proxy_server'), call('advanced/proxy username', 'proxy_username'),
             call('advanced/proxy password', 'proxy_password'), call('advanced/proxy mode', ProxyMode.MANUAL_PROXY)]
 
@@ -209,7 +212,7 @@ class TestProxyMetaUpgrade(TestCase, TestMixin):
         assert len(conn.execute('SELECT * FROM metadata WHERE key = "proxy_password"').fetchall()) == 0
         assert conn.execute('SELECT * FROM metadata WHERE key = "version"').first().value == '2'
 
-        assert self.mocked_settings_instance.setValue.call_args_list == [
+        assert self.mocked_settings.setValue.call_args_list == [
             call('advanced/proxy http', 'proxy_server'), call('advanced/proxy https', 'proxy_server'),
             call('advanced/proxy username', 'proxy_username'), call('advanced/proxy password', 'proxy_password'),
             call('advanced/proxy mode', ProxyMode.MANUAL_PROXY)]