From 86f85081b653f6048f18208bd8f33a16ea637e95 Mon Sep 17 00:00:00 2001 From: Tim Bentley Date: Tue, 21 Jan 2020 16:59:46 +0000 Subject: [PATCH] Migrate plugins from Settings() Alert and Bibles Signed-off-by: Tim --- openlp/plugins/alerts/lib/alertstab.py | 45 +- openlp/plugins/bibles/lib/__init__.py | 6 +- openlp/plugins/bibles/lib/biblestab.py | 77 ++-- openlp/plugins/bibles/lib/manager.py | 4 +- openlp/plugins/bibles/lib/upgrade.py | 5 +- .../openlp_plugins/bibles/test_lib.py | 384 +++++++++--------- .../openlp_plugins/bibles/test_manager.py | 3 - .../openlp_plugins/bibles/test_upgrade.py | 25 +- 8 files changed, 274 insertions(+), 275 deletions(-) diff --git a/openlp/plugins/alerts/lib/alertstab.py b/openlp/plugins/alerts/lib/alertstab.py index 988237bd7..1f8379931 100644 --- a/openlp/plugins/alerts/lib/alertstab.py +++ b/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() - settings.beginGroup(self.settings_section) - self.timeout = settings.value('timeout') - self.font_color = settings.value('font color') - self.font_size = settings.value('font size') - self.background_color = settings.value('background color') - self.font_face = settings.value('font face') - self.location = settings.value('location') - self.repeat = settings.value('repeat') - self.scroll = settings.value('scroll') - settings.endGroup() + self.settings.beginGroup(self.settings_section) + self.timeout = self.settings.value('timeout') + self.font_color = self.settings.value('font color') + self.font_size = self.settings.value('font size') + self.background_color = self.settings.value('background color') + self.font_face = self.settings.value('font face') + self.location = self.settings.value('location') + self.repeat = self.settings.value('repeat') + self.scroll = self.settings.value('scroll') + self.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() - settings.beginGroup(self.settings_section) + self.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) - settings.setValue('font color', self.font_color) - settings.setValue('font size', self.font_size) + self.settings.setValue('background color', self.background_color) + self.settings.setValue('font color', self.font_color) + self.settings.setValue('font size', self.font_size) self.font_face = self.font_combo_box.currentFont().family() - settings.setValue('font face', self.font_face) - settings.setValue('timeout', self.timeout) + self.settings.setValue('font face', self.font_face) + self.settings.setValue('timeout', self.timeout) self.location = self.vertical_combo_box.currentIndex() - settings.setValue('location', self.location) - settings.setValue('repeat', self.repeat) - settings.setValue('scroll', self.scroll_check_box.isChecked()) - settings.endGroup() + self.settings.setValue('location', self.location) + self.settings.setValue('repeat', self.repeat) + self.settings.setValue('scroll', self.scroll_check_box.isChecked()) + self.settings.endGroup() if self.changed: self.settings_form.register_post_process('update_display_css') self.changed = False diff --git a/openlp/plugins/bibles/lib/__init__.py b/openlp/plugins/bibles/lib/__init__.py index 1c3c4c709..929bfec8d 100644 --- a/openlp/plugins/bibles/lib/__init__.py +++ b/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.beginGroup('bibles') + settings = Registry().get('settings') 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('|') diff --git a/openlp/plugins/bibles/lib/biblestab.py b/openlp/plugins/bibles/lib/biblestab.py index 044d4778c..a3de729a7 100644 --- a/openlp/plugins/bibles/lib/biblestab.py +++ b/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() - settings.beginGroup(self.settings_section) - self.is_verse_number_visible = settings.value('is verse number visible') - self.show_new_chapters = settings.value('display new chapter') - self.display_style = settings.value('display brackets') - self.layout_style = settings.value('verse layout style') - self.bible_theme = settings.value('bible theme') - self.second_bibles = settings.value('second bibles') + self.settings.beginGroup(self.settings_section) + self.is_verse_number_visible = self.settings.value('is verse number visible') + self.show_new_chapters = self.settings.value('display new chapter') + self.display_style = self.settings.value('display brackets') + self.layout_style = self.settings.value('verse layout style') + self.bible_theme = self.settings.value('bible theme') + self.second_bibles = self.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() - settings.beginGroup(self.settings_section) - settings.setValue('is verse number visible', self.is_verse_number_visible) - settings.setValue('display new chapter', self.show_new_chapters) - settings.setValue('display brackets', self.display_style) - settings.setValue('verse layout style', self.layout_style) - settings.setValue('second bibles', self.second_bibles) - settings.setValue('bible theme', self.bible_theme) + self.self.settings.beginGroup(self.settings_section) + self.settings.setValue('is verse number visible', self.is_verse_number_visible) + self.settings.setValue('display new chapter', self.show_new_chapters) + self.settings.setValue('display brackets', self.display_style) + self.settings.setValue('verse layout style', self.layout_style) + self.settings.setValue('second bibles', self.second_bibles) + self.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'): - settings.setValue('book name language', self.language_selection) + if self.language_selection != self.settings.value('book name language'): + 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) - settings.setValue('hide combined quick error', self.hide_combined_quick_error) - settings.setValue('is search while typing enabled', self.bible_search_while_typing) - settings.endGroup() + self.settings.setValue('reset to combined quick search', self.reset_to_combined_quick_search) + self.settings.setValue('hide combined quick error', self.hide_combined_quick_error) + self.settings.setValue('is search while typing enabled', self.bible_search_while_typing) + self.settings.endGroup() if self.tab_visited: self.settings_form.register_post_process('bibles_config_updated') self.tab_visited = False diff --git a/openlp/plugins/bibles/lib/manager.py b/openlp/plugins/bibles/lib/manager.py index b054b63ca..0536b2023 100644 --- a/openlp/plugins/bibles/lib/manager.py +++ b/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: diff --git a/openlp/plugins/bibles/lib/upgrade.py b/openlp/plugins/bibles/lib/upgrade.py index 178195646..550c54209 100644 --- a/openlp/plugins/bibles/lib/upgrade.py +++ b/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 ('', ) diff --git a/tests/functional/openlp_plugins/bibles/test_lib.py b/tests/functional/openlp_plugins/bibles/test_lib.py index 08b0cd6b0..b9eae6e67 100644 --- a/tests/functional/openlp_plugins/bibles/test_lib.py +++ b/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') - def test_get_reference_separator(self, mocked_update_reference_separators): + # GIVEN: A list of expected separators and the lib module's constant is empty + 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 = 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'} + lib.REFERENCE_SEPARATORS = separators - def _update_side_effect(): - """ - Update the references after mocking out the method - """ - lib.REFERENCE_SEPARATORS = separators + mocked_update_reference_separators.side_effect = _update_side_effect - 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 - for key, value in separators.items(): - lib.get_reference_separator(key) + # THEN: get_reference_separator should return the correct separator + assert separators[key] == value + 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): - """ - Test that the 'full' regex parses bible verse references correctly. - """ - # 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 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(): + """ + Test the creation and construction of the SearchResults class + """ + # GIVEN: A book, chapter and a verse list + book = 'Genesis' + chapter = 1 + 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.' + } - full_reference_match = get_reference_match('full') - for reference_text, book_result, ranges_result 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 [''] + # WHEN: We create the search results object + search_results = SearchResults(book, chapter, verse_list) - 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) + # THEN: It should have a book, a chapter and a verse list + assert search_results is not None, 'The search_results object should not be None' + 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' - # 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 - # expected result - assert match is not None, '{text} should provide a match'.format(text=reference_text) - assert book_result == match.group('book'), \ - '{text} does not provide the expected result for the book group.'\ - .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) +def test_search_results_has_verse_list(): + """ + Test that a SearchResults object with a valid verse list returns True when checking ``has_verse_list()`` + """ + # GIVEN: A valid SearchResults object with a proper verse list + search_results = SearchResults('Genesis', 1, {1: 'In the beginning God created the heavens and the earth.'}) - def test_reference_matched_range(self): - """ - 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 [''] + # WHEN: We check that the SearchResults object has a verse list + has_verse_list = search_results.has_verse_list() - 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) + # THEN: It should be True + assert has_verse_list is True, 'The SearchResults object should have a verse list' - # 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 - # 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_search_results_has_no_verse_list(): + """ + Test that a SearchResults object with an empty verse list returns False when checking ``has_verse_list()`` + """ + # GIVEN: A valid SearchResults object with an empty verse list + search_results = SearchResults('Genesis', 1, {}) - def test_reference_matched_range_separator(self): - # 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 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 [''] + # WHEN: We check that the SearchResults object has a verse list + has_verse_list = search_results.has_verse_list() - 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) + # THEN: It should be False + assert has_verse_list is False, 'The SearchResults object should have a verse list' - # 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 +def test_reference_matched_full(mocked_bible_test): + """ + 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): - """ - Test the creation and construction of the SearchResults class - """ - # GIVEN: A book, chapter and a verse list - book = 'Genesis' - chapter = 1 - 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.' - } + full_reference_match = get_reference_match('full') + for reference_text, book_result, ranges_result 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 [''] - # WHEN: We create the search results object - search_results = SearchResults(book, chapter, verse_list) + 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) - # THEN: It should have a book, a chapter and a verse list - assert search_results is not None, 'The search_results object should not be None' - 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' + # WHEN: Attempting to parse the input string + match = full_reference_match.match(reference_text) - def test_search_results_has_verse_list(self): - """ - Test that a SearchResults object with a valid verse list returns True when checking ``has_verse_list()`` - """ - # GIVEN: A valid SearchResults object with a proper verse list - search_results = SearchResults('Genesis', 1, {1: 'In the beginning God created the heavens and the earth.'}) + # THEN: A match should be returned, and the book and reference should match the + # expected result + assert match is not None, '{text} should provide a match'.format(text=reference_text) + assert book_result == match.group('book'), \ + '{text} does not provide the expected result for the book group.'\ + .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 - assert has_verse_list is True, 'The SearchResults object should have a verse list' +def test_reference_matched_range(mocked_bible_test): + """ + 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): - """ - Test that a SearchResults object with an empty verse list returns False when checking ``has_verse_list()`` - """ - # GIVEN: A valid SearchResults object with an empty verse list - search_results = SearchResults('Genesis', 1, {}) + 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: We check that the SearchResults object has a verse list - has_verse_list = search_results.has_verse_list() + # WHEN: Attempting to parse the input string + match = full_reference_match.match(reference_text) - # THEN: It should be False - assert has_verse_list is False, 'The SearchResults object should have a verse list' + # THEN: A match should be returned, and the to/from chapter/verses should match as + # 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 diff --git a/tests/functional/openlp_plugins/bibles/test_manager.py b/tests/functional/openlp_plugins/bibles/test_manager.py index bca149999..725c822d2 100644 --- a/tests/functional/openlp_plugins/bibles/test_manager.py +++ b/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): """ diff --git a/tests/functional/openlp_plugins/bibles/test_upgrade.py b/tests/functional/openlp_plugins/bibles/test_upgrade.py index 1215c18f4..93df98586 100644 --- a/tests/functional/openlp_plugins/bibles/test_upgrade.py +++ b/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.mocked_settings = patched_settings.start() - self.addCleanup(patched_settings.stop) - self.mocked_settings_instance = MagicMock() - self.mocked_settings.return_value = self.mocked_settings_instance + self.build_settings() + Registry().create() + Registry().register('service_list', MagicMock()) 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)]