openlp-core team mailing list archive
-
openlp-core team
-
Mailing list archive
-
Message #30347
[Merge] lp:~phill-ridout/openlp/yet-more-refactors into lp:openlp
Phill has proposed merging lp:~phill-ridout/openlp/yet-more-refactors into lp:openlp.
Commit message:
OpenSongBible refactors
Requested reviews:
OpenLP Core (openlp-core)
For more details, see:
https://code.launchpad.net/~phill-ridout/openlp/yet-more-refactors/+merge/303490
More refactors, focusing mainly on OpenSong Bible
lp:~phill-ridout/openlp/yet-more-refactors (revision 2712)
[SUCCESS] https://ci.openlp.io/job/Branch-01-Pull/1758/
[SUCCESS] https://ci.openlp.io/job/Branch-02-Functional-Tests/1669/
[SUCCESS] https://ci.openlp.io/job/Branch-03-Interface-Tests/1607/
[SUCCESS] https://ci.openlp.io/job/Branch-04a-Windows_Functional_Tests/1363/
[SUCCESS] https://ci.openlp.io/job/Branch-04b-Windows_Interface_Tests/953/
[SUCCESS] https://ci.openlp.io/job/Branch-05a-Code_Analysis/1021/
[SUCCESS] https://ci.openlp.io/job/Branch-05b-Test_Coverage/889/
[SUCCESS] https://ci.openlp.io/job/Branch-05c-Code_Analysis2/54/
--
Your team OpenLP Core is requested to review the proposed merge of lp:~phill-ridout/openlp/yet-more-refactors into lp:openlp.
=== modified file 'openlp/plugins/bibles/bibleplugin.py'
--- openlp/plugins/bibles/bibleplugin.py 2016-08-08 18:11:32 +0000
+++ openlp/plugins/bibles/bibleplugin.py 2016-08-21 07:47:26 +0000
@@ -140,10 +140,10 @@
def uses_theme(self, theme):
"""
- Called to find out if the bible plugin is currently using a theme. Returns ``1`` if the theme is being used,
- otherwise returns ``0``.
+ Called to find out if the bible plugin is currently using a theme.
:param theme: The theme
+ :return: 1 if the theme is being used, otherwise returns 0
"""
if str(self.settings_tab.bible_theme) == theme:
return 1
@@ -151,11 +151,11 @@
def rename_theme(self, old_theme, new_theme):
"""
- Rename the theme the bible plugin is using making the plugin use the
- new name.
+ Rename the theme the bible plugin is using, making the plugin use the new name.
:param old_theme: The name of the theme the plugin should stop using. Unused for this particular plugin.
:param new_theme: The new name the plugin should now use.
+ :return: None
"""
self.settings_tab.bible_theme = new_theme
self.settings_tab.save()
=== modified file 'openlp/plugins/bibles/lib/__init__.py'
--- openlp/plugins/bibles/lib/__init__.py 2016-05-05 15:41:48 +0000
+++ openlp/plugins/bibles/lib/__init__.py 2016-08-21 07:47:26 +0000
@@ -173,7 +173,7 @@
def update_reference_separators():
"""
- Updates separators and matches for parsing and formating scripture references.
+ Updates separators and matches for parsing and formatting scripture references.
"""
default_separators = [
'|'.join([
@@ -215,7 +215,7 @@
# escape reserved characters
for character in '\\.^$*+?{}[]()':
source_string = source_string.replace(character, '\\' + character)
- # add various unicode alternatives
+ # add various Unicode alternatives
source_string = source_string.replace('-', '(?:[-\u00AD\u2010\u2011\u2012\u2014\u2014\u2212\uFE63\uFF0D])')
source_string = source_string.replace(',', '(?:[,\u201A])')
REFERENCE_SEPARATORS['sep_{role}'.format(role=role)] = '\s*(?:{source})\s*'.format(source=source_string)
=== modified file 'openlp/plugins/bibles/lib/bibleimport.py'
--- openlp/plugins/bibles/lib/bibleimport.py 2016-08-09 19:32:29 +0000
+++ openlp/plugins/bibles/lib/bibleimport.py 2016-08-21 07:47:26 +0000
@@ -23,9 +23,11 @@
import logging
from lxml import etree, objectify
+from zipfile import is_zipfile
from openlp.core.common import OpenLPMixin, languages
-from openlp.core.lib import ValidationError
+from openlp.core.lib import ValidationError, translate
+from openlp.core.lib.ui import critical_error_message_box
from openlp.plugins.bibles.lib.db import BibleDB, BiblesResourcesDB
log = logging.getLogger(__name__)
@@ -35,11 +37,25 @@
"""
Helper class to import bibles from a third party source into OpenLP
"""
- # TODO: Test
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self.filename = kwargs['filename'] if 'filename' in kwargs else None
+ @staticmethod
+ def is_compressed(file):
+ """
+ Check if the supplied file is compressed
+
+ :param file: A path to the file to check
+ """
+ if is_zipfile(file):
+ critical_error_message_box(
+ message=translate('BiblesPlugin.BibleImport',
+ 'The file "{file}" you supplied is compressed. You must decompress it before import.'
+ ).format(file=file))
+ return True
+ return False
+
def get_language_id(self, file_language=None, bible_name=None):
"""
Get the language_id for the language of the bible. Fallback to user input if we cannot do this pragmatically.
=== modified file 'openlp/plugins/bibles/lib/importers/csvbible.py'
--- openlp/plugins/bibles/lib/importers/csvbible.py 2016-08-09 20:45:25 +0000
+++ openlp/plugins/bibles/lib/importers/csvbible.py 2016-08-21 07:47:26 +0000
@@ -142,7 +142,7 @@
book_ptr = None
for verse in verses:
if self.stop_import_flag:
- return None
+ break
verse_book = self.get_book_name(verse.book_id_name, books)
if book_ptr != verse_book:
book = self.get_book(verse_book)
=== modified file 'openlp/plugins/bibles/lib/importers/opensong.py'
--- openlp/plugins/bibles/lib/importers/opensong.py 2016-08-09 20:45:25 +0000
+++ openlp/plugins/bibles/lib/importers/opensong.py 2016-08-21 07:47:26 +0000
@@ -21,12 +21,12 @@
###############################################################################
import logging
-from lxml import etree, objectify
+from lxml import etree
from openlp.core.common import translate, trace_error_handler
+from openlp.core.lib.exceptions import ValidationError
from openlp.core.lib.ui import critical_error_message_box
from openlp.plugins.bibles.lib.bibleimport import BibleImport
-from openlp.plugins.bibles.lib.db import BibleDB, BiblesResourcesDB
log = logging.getLogger(__name__)
@@ -36,93 +36,150 @@
"""
OpenSong Bible format importer class. This class is used to import Bibles from OpenSong's XML format.
"""
- def get_text(self, element):
+ @staticmethod
+ def get_text(element):
"""
Recursively get all text in an objectify element and its child elements.
:param element: An objectify element to get the text from
+ :return: The text content of the element (str)
"""
verse_text = ''
if element.text:
verse_text = element.text
for sub_element in element.iterchildren():
- verse_text += self.get_text(sub_element)
+ verse_text += OpenSongBible.get_text(sub_element)
if element.tail:
verse_text += element.tail
return verse_text
+ @staticmethod
+ def parse_chapter_number(number, previous_number):
+ """
+ Parse the chapter number
+
+ :param number: The raw data from the xml
+ :param previous_number: The previous chapter number
+ :return: Number of current chapter. (Int)
+ """
+ if number:
+ return int(number.split()[-1])
+ return previous_number + 1
+
+ @staticmethod
+ def parse_verse_number(number, previous_number):
+ """
+ Parse the verse number retrieved from the xml
+
+ :param number: The raw data from the xml
+ :param previous_number: The previous verse number
+ :return: Number of current verse. (Int)
+ """
+ if not number:
+ return previous_number + 1
+ try:
+ return int(number)
+ except ValueError:
+ verse_parts = number.split('-')
+ if len(verse_parts) > 1:
+ number = int(verse_parts[0])
+ return number
+ except TypeError:
+ log.warning('Illegal verse number: {verse_no}'.format(verse_no=str(number)))
+ return previous_number + 1
+
+ @staticmethod
+ def validate_file(filename):
+ """
+ Validate the supplied file
+
+ :param filename: The supplied file
+ :return: True if valid. ValidationError is raised otherwise.
+ """
+ if BibleImport.is_compressed(filename):
+ raise ValidationError(msg='Compressed file')
+ bible = BibleImport.parse_xml(filename, use_objectify=True)
+ root_tag = bible.tag.lower()
+ if root_tag != 'bible':
+ if root_tag == 'xmlbible':
+ # Zefania bibles have a root tag of XMLBIBLE". Sometimes these bibles are referred to as 'OpenSong'
+ critical_error_message_box(
+ message=translate('BiblesPlugin.OpenSongImport',
+ 'Incorrect Bible file type supplied. This looks like a Zefania XML bible, '
+ 'please use the Zefania import option.'))
+ raise ValidationError(msg='Invalid xml.')
+ return True
+
+ def process_books(self, books):
+ """
+ Extract and create the books from the objectified xml
+
+ :param books: Objectified xml
+ :return: None
+ """
+ for book in books:
+ if self.stop_import_flag:
+ break
+ db_book = self.find_and_create_book(str(book.attrib['n']), len(books), self.language_id)
+ self.process_chapters(db_book, book.c)
+ self.session.commit()
+
+ def process_chapters(self, book, chapters):
+ """
+ Extract and create the chapters from the objectified xml for the book `book`
+
+ :param book: A database Book object to add the chapters to
+ :param chapters: Objectified xml containing chapters
+ :return: None
+ """
+ chapter_number = 0
+ for chapter in chapters:
+ if self.stop_import_flag:
+ break
+ chapter_number = self.parse_chapter_number(chapter.attrib['n'], chapter_number)
+ self.process_verses(book, chapter_number, chapter.v)
+ self.wizard.increment_progress_bar(translate('BiblesPlugin.Opensong',
+ 'Importing {name} {chapter}...'
+ ).format(name=book.name, chapter=chapter_number))
+
+ def process_verses(self, book, chapter_number, verses):
+ """
+ Extract and create the verses from the objectified xml
+
+ :param book: A database Book object
+ :param chapter_number: The chapter number to add the verses to (int)
+ :param verses: Objectified xml containing verses
+ :return: None
+ """
+ verse_number = 0
+ for verse in verses:
+ if self.stop_import_flag:
+ break
+ verse_number = self.parse_verse_number(verse.attrib['n'], verse_number)
+ self.create_verse(book.id, chapter_number, verse_number, self.get_text(verse))
+
def do_import(self, bible_name=None):
"""
- Loads a Bible from file.
+ Loads an Open Song Bible from a file.
+
+ :param bible_name: The name of the bible being imported
+ :return: True if import completed, False if import was unsuccessful
"""
log.debug('Starting OpenSong import from "{name}"'.format(name=self.filename))
- success = True
try:
+ self.validate_file(self.filename)
bible = self.parse_xml(self.filename, use_objectify=True)
# Check that we're not trying to import a Zefania XML bible, it is sometimes refered to as 'OpenSong'
- if bible.tag.upper() == 'XMLBIBLE':
- critical_error_message_box(
- message=translate('BiblesPlugin.OpenSongImport',
- 'Incorrect Bible file type supplied. This looks like a Zefania XML bible, '
- 'please use the Zefania import option.'))
- return False
# No language info in the opensong format, so ask the user
- language_id = self.get_language_id(bible_name=self.filename)
- if not language_id:
+ self.language_id = self.get_language_id(bible_name=self.filename)
+ if not self.language_id:
return False
- for book in bible.b:
- if self.stop_import_flag:
- break
- book_ref_id = self.get_book_ref_id_by_name(str(book.attrib['n']), len(bible.b), language_id)
- if not book_ref_id:
- log.error('Importing books from "{name}" failed'.format(name=self.filename))
- return False
- book_details = BiblesResourcesDB.get_book_by_id(book_ref_id)
- db_book = self.create_book(book.attrib['n'], book_ref_id, book_details['testament_id'])
- chapter_number = 0
- for chapter in book.c:
- if self.stop_import_flag:
- break
- number = chapter.attrib['n']
- if number:
- chapter_number = int(number.split()[-1])
- else:
- chapter_number += 1
- verse_number = 0
- for verse in chapter.v:
- if self.stop_import_flag:
- break
- number = verse.attrib['n']
- if number:
- try:
- number = int(number)
- except ValueError:
- verse_parts = number.split('-')
- if len(verse_parts) > 1:
- number = int(verse_parts[0])
- except TypeError:
- log.warning('Illegal verse number: {verse:d}'.format(verse=verse.attrib['n']))
- verse_number = number
- else:
- verse_number += 1
- self.create_verse(db_book.id, chapter_number, verse_number, self.get_text(verse))
- self.wizard.increment_progress_bar(translate('BiblesPlugin.Opensong',
- 'Importing {name} {chapter}...'
- ).format(name=db_book.name, chapter=chapter_number))
- self.session.commit()
+ self.process_books(bible.b)
self.application.process_events()
- except etree.XMLSyntaxError as inst:
+ except (AttributeError, ValidationError, etree.XMLSyntaxError):
+ log.exception('Loading Bible from OpenSong file failed')
trace_error_handler(log)
- critical_error_message_box(
- message=translate('BiblesPlugin.OpenSongImport',
- 'Incorrect Bible file type supplied. OpenSong Bibles may be '
- 'compressed. You must decompress them before import.'))
- log.exception(inst)
- success = False
- except (IOError, AttributeError):
- log.exception('Loading Bible from OpenSong file failed')
- success = False
+ return False
if self.stop_import_flag:
return False
- else:
- return success
+ return True
=== modified file 'openlp/plugins/bibles/lib/importers/osis.py'
--- openlp/plugins/bibles/lib/importers/osis.py 2016-08-10 19:08:09 +0000
+++ openlp/plugins/bibles/lib/importers/osis.py 2016-08-21 07:47:26 +0000
@@ -98,7 +98,7 @@
language_id = self.get_language_id(language[0] if language else None, bible_name=self.filename)
if not language_id:
return False
- num_books = int(osis_bible_tree.xpath("count(//ns:div[@type='book'])", namespaces=NS))
+ no_of_books = int(osis_bible_tree.xpath("count(//ns:div[@type='book'])", namespaces=NS))
# Precompile a few xpath-querys
verse_in_chapter = etree.XPath('count(//ns:chapter[1]/ns:verse)', namespaces=NS)
text_in_verse = etree.XPath('count(//ns:verse[1]/text())', namespaces=NS)
@@ -108,13 +108,8 @@
if self.stop_import_flag:
break
# Remove div-tags in the book
- etree.strip_tags(book, ('{http://www.bibletechnologies.net/2003/OSIS/namespace}div'))
- book_ref_id = self.get_book_ref_id_by_name(book.get('osisID'), num_books, language_id)
- if not book_ref_id:
- log.error('Importing books from "{name}" failed'.format(name=self.filename))
- return False
- book_details = BiblesResourcesDB.get_book_by_id(book_ref_id)
- db_book = self.create_book(book_details['name'], book_ref_id, book_details['testament_id'])
+ etree.strip_tags(book, '{http://www.bibletechnologies.net/2003/OSIS/namespace}div')
+ db_book = self.find_and_create_book(book.get('osisID'), no_of_books, language_id)
# Find out if chapter-tags contains the verses, or if it is used as milestone/anchor
if int(verse_in_chapter(book)) > 0:
# The chapter tags contains the verses
=== modified file 'openlp/plugins/bibles/lib/importers/zefania.py'
--- openlp/plugins/bibles/lib/importers/zefania.py 2016-08-09 20:45:25 +0000
+++ openlp/plugins/bibles/lib/importers/zefania.py 2016-08-21 07:47:26 +0000
@@ -54,7 +54,7 @@
language_id = self.get_language_id(language[0] if language else None, bible_name=self.filename)
if not language_id:
return False
- num_books = int(xmlbible.xpath('count(//BIBLEBOOK)'))
+ no_of_books = int(xmlbible.xpath('count(//BIBLEBOOK)'))
self.wizard.progress_bar.setMaximum(int(xmlbible.xpath('count(//CHAPTER)')))
for BIBLEBOOK in xmlbible:
if self.stop_import_flag:
@@ -64,7 +64,7 @@
if not bname and not bnumber:
continue
if bname:
- book_ref_id = self.get_book_ref_id_by_name(bname, num_books, language_id)
+ book_ref_id = self.get_book_ref_id_by_name(bname, no_of_books, language_id)
else:
log.debug('Could not find a name, will use number, basically a guess.')
book_ref_id = int(bnumber)
=== modified file 'tests/functional/openlp_core_ui/test_exceptionform.py'
--- tests/functional/openlp_core_ui/test_exceptionform.py 2016-06-25 14:41:06 +0000
+++ tests/functional/openlp_core_ui/test_exceptionform.py 2016-08-21 07:47:26 +0000
@@ -24,18 +24,13 @@
"""
import os
-import socket
import tempfile
-import urllib
from unittest import TestCase
from unittest.mock import mock_open
-from PyQt5.QtCore import QUrlQuery
-
from openlp.core.common import Registry
-from openlp.core.ui.firsttimeform import FirstTimeForm
-from tests.functional import MagicMock, patch
+from tests.functional import patch
from tests.helpers.testmixin import TestMixin
from openlp.core.ui import exceptionform
=== modified file 'tests/functional/openlp_plugins/bibles/test_bibleimport.py'
--- tests/functional/openlp_plugins/bibles/test_bibleimport.py 2016-08-07 11:20:53 +0000
+++ tests/functional/openlp_plugins/bibles/test_bibleimport.py 2016-08-21 07:47:26 +0000
@@ -29,8 +29,10 @@
from unittest import TestCase
from openlp.core.common.languages import Language
+from openlp.core.lib.exceptions import ValidationError
from openlp.plugins.bibles.lib.bibleimport import BibleImport
-from tests.functional import MagicMock, patch
+from openlp.plugins.bibles.lib.db import BibleDB
+from tests.functional import ANY, MagicMock, patch
class TestBibleImport(TestCase):
@@ -39,23 +41,48 @@
"""
def setUp(self):
- test_file = BytesIO(b'<?xml version="1.0" encoding="UTF-8" ?>\n'
- b'<root>\n'
- b' <data><div>Test<p>data</p><a>to</a>keep</div></data>\n'
- b' <data><unsupported>Test<x>data</x><y>to</y>discard</unsupported></data>\n'
- b'</root>')
+ test_file = BytesIO(
+ b'<?xml version="1.0" encoding="UTF-8" ?>\n'
+ b'<root>\n'
+ b' <data><div>Test<p>data</p><a>to</a>keep</div></data>\n'
+ b' <data><unsupported>Test<x>data</x><y>to</y>discard</unsupported></data>\n'
+ b'</root>'
+ )
self.file_patcher = patch('builtins.open', return_value=test_file)
+ self.addCleanup(self.file_patcher.stop)
+ self.file_patcher.start()
self.log_patcher = patch('openlp.plugins.bibles.lib.bibleimport.log')
+ self.addCleanup(self.log_patcher.stop)
+ self.mock_log = self.log_patcher.start()
self.setup_patcher = patch('openlp.plugins.bibles.lib.db.BibleDB._setup')
-
- self.addCleanup(self.file_patcher.stop)
- self.addCleanup(self.log_patcher.stop)
self.addCleanup(self.setup_patcher.stop)
-
- self.file_patcher.start()
- self.mock_log = self.log_patcher.start()
self.setup_patcher.start()
+ def init_kwargs_none_test(self):
+ """
+ Test the initialisation of the BibleImport Class when no key word arguments are supplied
+ """
+ # GIVEN: A patched BibleDB._setup, BibleImport class and mocked parent
+ # WHEN: Creating an instance of BibleImport with no key word arguments
+ instance = BibleImport(MagicMock())
+
+ # THEN: The filename attribute should be None
+ self.assertIsNone(instance.filename)
+ self.assertIsInstance(instance, BibleDB)
+
+ def init_kwargs_set_test(self):
+ """
+ Test the initialisation of the BibleImport Class when supplied with select keyword arguments
+ """
+ # GIVEN: A patched BibleDB._setup, BibleImport class and mocked parent
+ # WHEN: Creating an instance of BibleImport with selected key word arguments
+ kwargs = {'filename': 'bible.xml'}
+ instance = BibleImport(MagicMock(), **kwargs)
+
+ # THEN: The filename keyword should be set to bible.xml
+ self.assertEqual(instance.filename, 'bible.xml')
+ self.assertIsInstance(instance, BibleDB)
+
def get_language_id_language_found_test(self):
"""
Test get_language_id() when called with a name found in the languages list
@@ -81,8 +108,7 @@
Test get_language_id() when called with a name not found in the languages list
"""
# GIVEN: A mocked languages.get_language which returns language and an instance of BibleImport
- with patch('openlp.core.common.languages.get_language', return_value=None) \
- as mocked_languages_get_language, \
+ with patch('openlp.core.common.languages.get_language', return_value=None) as mocked_languages_get_language, \
patch('openlp.plugins.bibles.lib.db.BibleDB.get_language', return_value=20) as mocked_db_get_language:
instance = BibleImport(MagicMock())
instance.save_meta = MagicMock()
=== renamed file 'tests/functional/openlp_plugins/bibles/test_http.py' => 'tests/functional/openlp_plugins/bibles/test_bibleserver.py'
=== modified file 'tests/functional/openlp_plugins/bibles/test_csvimport.py'
--- tests/functional/openlp_plugins/bibles/test_csvimport.py 2016-08-09 20:56:04 +0000
+++ tests/functional/openlp_plugins/bibles/test_csvimport.py 2016-08-21 07:47:26 +0000
@@ -46,10 +46,10 @@
def setUp(self):
self.manager_patcher = patch('openlp.plugins.bibles.lib.db.Manager')
+ self.addCleanup(self.manager_patcher.stop)
+ self.manager_patcher.start()
self.registry_patcher = patch('openlp.plugins.bibles.lib.db.Registry')
- self.addCleanup(self.manager_patcher.stop)
self.addCleanup(self.registry_patcher.stop)
- self.manager_patcher.start()
self.registry_patcher.start()
def test_create_importer(self):
@@ -240,7 +240,7 @@
importer.wizard = MagicMock()
# WHEN: Calling process_verses
- result = importer.process_verses([], [])
+ result = importer.process_verses(['Dummy Verse'], [])
# THEN: get_book_name should not be called and the return value should be None
self.assertFalse(importer.get_book_name.called)
=== modified file 'tests/functional/openlp_plugins/bibles/test_opensongimport.py'
--- tests/functional/openlp_plugins/bibles/test_opensongimport.py 2016-08-09 20:45:25 +0000
+++ tests/functional/openlp_plugins/bibles/test_opensongimport.py 2016-08-21 07:47:26 +0000
@@ -23,32 +23,36 @@
This module contains tests for the OpenSong Bible importer.
"""
+import json
import os
-import json
from unittest import TestCase
-from tests.functional import MagicMock, patch
+from lxml import etree, objectify
+
+from tests.functional import MagicMock, patch, call
+from tests.helpers.testmixin import TestMixin
+from openlp.core.common import Registry
+from openlp.core.lib.exceptions import ValidationError
from openlp.plugins.bibles.lib.importers.opensong import OpenSongBible
-from openlp.plugins.bibles.lib.db import BibleDB
+from openlp.plugins.bibles.lib.bibleimport import BibleImport
TEST_PATH = os.path.abspath(os.path.join(os.path.dirname(__file__),
'..', '..', '..', 'resources', 'bibles'))
-class TestOpenSongImport(TestCase):
+class TestOpenSongImport(TestCase, TestMixin):
"""
Test the functions in the :mod:`opensongimport` module.
"""
def setUp(self):
- self.registry_patcher = patch('openlp.plugins.bibles.lib.db.Registry')
- self.registry_patcher.start()
self.manager_patcher = patch('openlp.plugins.bibles.lib.db.Manager')
+ self.addCleanup(self.manager_patcher.stop)
self.manager_patcher.start()
-
- def tearDown(self):
- self.registry_patcher.stop()
- self.manager_patcher.stop()
+ self.setup_application()
+ self.app.process_events = MagicMock()
+ Registry.create()
+ Registry().register('application', self.app)
def test_create_importer(self):
"""
@@ -61,7 +65,445 @@
importer = OpenSongBible(mocked_manager, path='.', name='.', filename='')
# THEN: The importer should be an instance of BibleDB
- self.assertIsInstance(importer, BibleDB)
+ self.assertIsInstance(importer, BibleImport)
+
+ def get_text_no_text_test(self):
+ """
+ Test that get_text handles elements containing text in a combination of text and tail attributes
+ """
+ # GIVEN: Some test data which contains an empty element and an instance of OpenSongBible
+ test_data = objectify.fromstring('<element></element>')
+
+ # WHEN: Calling get_text
+ result = OpenSongBible.get_text(test_data)
+
+ # THEN: A blank string should be returned
+ self.assertEqual(result, '')
+
+ def get_text_text_test(self):
+ """
+ Test that get_text handles elements containing text in a combination of text and tail attributes
+ """
+ # GIVEN: Some test data which contains all possible permutation of text and tail text possible and an instance
+ # of OpenSongBible
+ test_data = objectify.fromstring('<element>Element text '
+ '<sub_text_tail>sub_text_tail text </sub_text_tail>sub_text_tail tail '
+ '<sub_text>sub_text text </sub_text>'
+ '<sub_tail></sub_tail>sub_tail tail</element>')
+
+ # WHEN: Calling get_text
+ result = OpenSongBible.get_text(test_data)
+
+ # THEN: The text returned should be as expected
+ self.assertEqual(result, 'Element text sub_text_tail text sub_text_tail tail sub_text text sub_tail tail')
+
+ def parse_chapter_number_test(self):
+ """
+ Test parse_chapter_number when supplied with chapter number and an instance of OpenSongBible
+ """
+ # GIVEN: The number 10 represented as a string
+ # WHEN: Calling parse_chapter_nnumber
+ result = OpenSongBible.parse_chapter_number('10', 0)
+
+ # THEN: The 10 should be returned as an Int
+ self.assertEqual(result, 10)
+
+ def parse_chapter_number_empty_attribute_test(self):
+ """
+ Testparse_chapter_number when the chapter number is an empty string. (Bug #1074727)
+ """
+ # GIVEN: An empty string, and the previous chapter number set as 12 and an instance of OpenSongBible
+ # WHEN: Calling parse_chapter_number
+ result = OpenSongBible.parse_chapter_number('', 12)
+
+ # THEN: parse_chapter_number should increment the previous verse number
+ self.assertEqual(result, 13)
+
+ def parse_verse_number_valid_verse_no_test(self):
+ """
+ Test parse_verse_number when supplied with a valid verse number
+ """
+ # GIVEN: The number 15 represented as a string and an instance of OpenSongBible
+ # WHEN: Calling parse_verse_number
+ result = OpenSongBible.parse_verse_number('15', 0)
+
+ # THEN: parse_verse_number should return the verse number
+ self.assertEqual(result, 15)
+
+ def parse_verse_number_verse_range_test(self):
+ """
+ Test parse_verse_number when supplied with a verse range
+ """
+ # GIVEN: The range 24-26 represented as a string
+ # WHEN: Calling parse_verse_number
+ result = OpenSongBible.parse_verse_number('24-26', 0)
+
+ # THEN: parse_verse_number should return the first verse number in the range
+ self.assertEqual(result, 24)
+
+ def parse_verse_number_invalid_verse_no_test(self):
+ """
+ Test parse_verse_number when supplied with a invalid verse number
+ """
+ # GIVEN: An non numeric string represented as a string
+ # WHEN: Calling parse_verse_number
+ result = OpenSongBible.parse_verse_number('invalid', 41)
+
+ # THEN: parse_verse_number should increment the previous verse number
+ self.assertEqual(result, 42)
+
+ def parse_verse_number_empty_attribute_test(self):
+ """
+ Test parse_verse_number when the verse number is an empty string. (Bug #1074727)
+ """
+ # GIVEN: An empty string, and the previous verse number set as 14
+ # WHEN: Calling parse_verse_number
+ result = OpenSongBible.parse_verse_number('', 14)
+
+ # THEN: parse_verse_number should increment the previous verse number
+ self.assertEqual(result, 15)
+
+ @patch('openlp.plugins.bibles.lib.importers.opensong.log')
+ def parse_verse_number_invalid_type_test(self, mocked_log):
+ """
+ Test parse_verse_number when the verse number is an invalid type)
+ """
+ # GIVEN: A mocked out log, a Tuple, and the previous verse number set as 12
+ # WHEN: Calling parse_verse_number
+ result = OpenSongBible.parse_verse_number((1, 2, 3), 12)
+
+ # THEN: parse_verse_number should log the verse number it was called with increment the previous verse number
+ mocked_log.warning.assert_called_once_with('Illegal verse number: (1, 2, 3)')
+ self.assertEqual(result, 13)
+
+ @patch('openlp.plugins.bibles.lib.bibleimport.BibleImport.find_and_create_book')
+ def process_books_stop_import_test(self, mocked_find_and_create_book):
+ """
+ Test process_books when stop_import is set to True
+ """
+ # GIVEN: An isntance of OpenSongBible
+ importer = OpenSongBible(MagicMock(), path='.', name='.', filename='')
+
+ # WHEN: stop_import_flag is set to True
+ importer.stop_import_flag = True
+ importer.process_books(['Book'])
+
+ # THEN: find_and_create_book should not have been called
+ self.assertFalse(mocked_find_and_create_book.called)
+
+ @patch('openlp.plugins.bibles.lib.bibleimport.BibleImport.find_and_create_book',
+ **{'side_effect': ['db_book1', 'db_book2']})
+ def process_books_completes_test(self, mocked_find_and_create_book):
+ """
+ Test process_books when it processes all books
+ """
+ # GIVEN: An instance of OpenSongBible Importer and two mocked books
+ importer = OpenSongBible(MagicMock(), path='.', name='.', filename='')
+
+ book1 = MagicMock()
+ book1.attrib = {'n': 'Name1'}
+ book1.c = 'Chapter1'
+ book2 = MagicMock()
+ book2.attrib = {'n': 'Name2'}
+ book2.c = 'Chapter2'
+ importer.language_id = 10
+ importer.process_chapters = MagicMock()
+ importer.session = MagicMock()
+ importer.stop_import_flag = False
+
+ # WHEN: Calling process_books with the two books
+ importer.process_books([book1, book2])
+
+ # THEN: find_and_create_book and process_books should be called with the details from the mocked books
+ self.assertEqual(mocked_find_and_create_book.call_args_list, [call('Name1', 2, 10), call('Name2', 2, 10)])
+ self.assertEqual(importer.process_chapters.call_args_list,
+ [call('db_book1', 'Chapter1'), call('db_book2', 'Chapter2')])
+ self.assertEqual(importer.session.commit.call_count, 2)
+
+ def process_chapters_stop_import_test(self):
+ """
+ Test process_chapters when stop_import is set to True
+ """
+ # GIVEN: An isntance of OpenSongBible
+ importer = OpenSongBible(MagicMock(), path='.', name='.', filename='')
+ importer.parse_chapter_number = MagicMock()
+
+ # WHEN: stop_import_flag is set to True
+ importer.stop_import_flag = True
+ importer.process_chapters('Book', ['Chapter1'])
+
+ # THEN: importer.parse_chapter_number not have been called
+ self.assertFalse(importer.parse_chapter_number.called)
+
+ @patch('openlp.plugins.bibles.lib.importers.opensong.translate', **{'side_effect': lambda x, y: y})
+ def process_chapters_completes_test(self, mocked_translate):
+ """
+ Test process_chapters when it completes
+ """
+ # GIVEN: An instance of OpenSongBible
+ importer = OpenSongBible(MagicMock(), path='.', name='.', filename='')
+ importer.parse_chapter_number = MagicMock()
+ importer.parse_chapter_number.side_effect = [1, 2]
+ importer.wizard = MagicMock()
+
+ # WHEN: called with some valid data
+ book = MagicMock()
+ book.name = "Book"
+ chapter1 = MagicMock()
+ chapter1.attrib = {'n': '1'}
+ chapter1.c = 'Chapter1'
+ chapter1.v = ['Chapter1 Verses']
+ chapter2 = MagicMock()
+ chapter2.attrib = {'n': '2'}
+ chapter2.c = 'Chapter2'
+ chapter2.v = ['Chapter2 Verses']
+
+ importer.process_verses = MagicMock()
+ importer.stop_import_flag = False
+ importer.process_chapters(book, [chapter1, chapter2])
+
+ # THEN: parse_chapter_number, process_verses and increment_process_bar should have been called
+ self.assertEqual(importer.parse_chapter_number.call_args_list, [call('1', 0), call('2', 1)])
+ self.assertEqual(
+ importer.process_verses.call_args_list,
+ [call(book, 1, ['Chapter1 Verses']), call(book, 2, ['Chapter2 Verses'])])
+ self.assertEqual(importer.wizard.increment_progress_bar.call_args_list,
+ [call('Importing Book 1...'), call('Importing Book 2...')])
+
+ def process_verses_stop_import_test(self):
+ """
+ Test process_verses when stop_import is set to True
+ """
+ # GIVEN: An isntance of OpenSongBible
+ importer = OpenSongBible(MagicMock(), path='.', name='.', filename='')
+ importer.parse_verse_number = MagicMock()
+
+ # WHEN: stop_import_flag is set to True
+ importer.stop_import_flag = True
+ importer.process_verses('Book', 1, 'Verses')
+
+ # THEN: importer.parse_verse_number not have been called
+ self.assertFalse(importer.parse_verse_number.called)
+
+ def process_verses_completes_test(self):
+ """
+ Test process_verses when it completes
+ """
+ # GIVEN: An instance of OpenSongBible
+ importer = OpenSongBible(MagicMock(), path='.', name='.', filename='')
+ importer.get_text = MagicMock()
+ importer.get_text.side_effect = ['Verse1 Text', 'Verse2 Text']
+ importer.parse_verse_number = MagicMock()
+ importer.parse_verse_number.side_effect = [1, 2]
+ importer.wizard = MagicMock()
+
+ # WHEN: called with some valid data
+ book = MagicMock()
+ book.id = 1
+ verse1 = MagicMock()
+ verse1.attrib = {'n': '1'}
+ verse1.c = 'Chapter1'
+ verse1.v = ['Chapter1 Verses']
+ verse2 = MagicMock()
+ verse2.attrib = {'n': '2'}
+ verse2.c = 'Chapter2'
+ verse2.v = ['Chapter2 Verses']
+
+ importer.create_verse = MagicMock()
+ importer.stop_import_flag = False
+ importer.process_verses(book, 1, [verse1, verse2])
+
+ # THEN: parse_chapter_number, process_verses and increment_process_bar should have been called
+ self.assertEqual(importer.parse_verse_number.call_args_list, [call('1', 0), call('2', 1)])
+ self.assertEqual(importer.get_text.call_args_list, [call(verse1), call(verse2)])
+ self.assertEqual(
+ importer.create_verse.call_args_list,
+ [call(1, 1, 1, 'Verse1 Text'), call(1, 1, 2, 'Verse2 Text')])
+
+ @patch('openlp.plugins.bibles.lib.importers.opensong.BibleImport.is_compressed')
+ def validate_file_compressed_test(self, mocked_is_compressed):
+ """
+ Test that validate_file raises a ValidationError when supplied with a compressed file
+ """
+ # GIVEN: A mocked is_compressed method which returns True
+ mocked_is_compressed.return_value = True
+
+ # WHEN: Calling validate_file
+ # THEN: ValidationError should be raised
+ with self.assertRaises(ValidationError) as context:
+ OpenSongBible.validate_file('file.name')
+ self.assertEqual(context.exception.msg, 'Compressed file')
+
+ @patch('openlp.plugins.bibles.lib.importers.opensong.BibleImport.parse_xml')
+ @patch('openlp.plugins.bibles.lib.importers.opensong.BibleImport.is_compressed', **{'return_value': False})
+ def validate_file_bible_test(self, mocked_is_compressed, mocked_parse_xml):
+ """
+ Test that validate_file returns True with valid XML
+ """
+ # GIVEN: Some test data with an OpenSong Bible "bible" root tag
+ mocked_parse_xml.return_value = objectify.fromstring('<bible></bible>')
+
+ # WHEN: Calling validate_file
+ result = OpenSongBible.validate_file('file.name')
+
+ # THEN: A True should be returned
+ self.assertTrue(result)
+
+ @patch('openlp.plugins.bibles.lib.importers.opensong.critical_error_message_box')
+ @patch('openlp.plugins.bibles.lib.importers.opensong.BibleImport.parse_xml')
+ @patch('openlp.plugins.bibles.lib.importers.opensong.BibleImport.is_compressed', **{'return_value': False})
+ def validate_file_zefania_root_test(self, mocked_is_compressed, mocked_parse_xml, mocked_message_box):
+ """
+ Test that validate_file raises a ValidationError with a Zefinia root tag
+ """
+ # GIVEN: Some test data with a Zefinia "XMLBIBLE" root tag
+ mocked_parse_xml.return_value = objectify.fromstring('<XMLBIBLE></XMLBIBLE>')
+
+ # WHEN: Calling validate_file
+ # THEN: critical_error_message_box should be called and an ValidationError should be raised
+ with self.assertRaises(ValidationError) as context:
+ OpenSongBible.validate_file('file.name')
+ self.assertEqual(context.exception.msg, 'Invalid xml.')
+ mocked_message_box.assert_called_once_with(
+ message='Incorrect Bible file type supplied. This looks like a Zefania XML bible, please use the '
+ 'Zefania import option.')
+
+ @patch('openlp.plugins.bibles.lib.importers.opensong.critical_error_message_box')
+ @patch('openlp.plugins.bibles.lib.importers.opensong.BibleImport.parse_xml')
+ @patch('openlp.plugins.bibles.lib.importers.opensong.BibleImport.is_compressed', **{'return_value': False})
+ def validate_file_invalid_root_test(self, mocked_is_compressed, mocked_parse_xml, mocked_message_box):
+ """
+ Test that validate_file raises a ValidationError with an invalid root tag
+ """
+ # GIVEN: Some test data with an invalid root tag and an instance of OpenSongBible
+ mocked_parse_xml.return_value = objectify.fromstring('<song></song>')
+
+ # WHEN: Calling validate_file
+ # THEN: ValidationError should be raised, and the critical error message box should not have been called
+ with self.assertRaises(ValidationError) as context:
+ OpenSongBible.validate_file('file.name')
+ self.assertEqual(context.exception.msg, 'Invalid xml.')
+ self.assertFalse(mocked_message_box.called)
+
+ @patch('openlp.plugins.bibles.lib.importers.opensong.log')
+ @patch('openlp.plugins.bibles.lib.importers.opensong.trace_error_handler')
+ def do_import_attribute_error_test(self, mocked_trace_error_handler, mocked_log):
+ """
+ Test do_import when an AttributeError exception is raised
+ """
+ # GIVEN: An instance of OpenSongBible and a mocked validate_file which raises an AttributeError
+ importer = OpenSongBible(MagicMock(), path='.', name='.', filename='')
+ importer.validate_file = MagicMock(**{'side_effect': AttributeError()})
+ importer.parse_xml = MagicMock()
+
+ # WHEN: Calling do_import
+ result = importer.do_import()
+
+ # THEN: do_import should return False after logging the exception
+ mocked_log.exception.assert_called_once_with('Loading Bible from OpenSong file failed')
+ mocked_trace_error_handler.assert_called_once_with(mocked_log)
+ self.assertFalse(result)
+ self.assertFalse(importer.parse_xml.called)
+
+ @patch('openlp.plugins.bibles.lib.importers.opensong.log')
+ @patch('openlp.plugins.bibles.lib.importers.opensong.trace_error_handler')
+ def do_import_validation_error_test(self, mocked_trace_error_handler, mocked_log):
+ """
+ Test do_import when an ValidationError exception is raised
+ """
+ # GIVEN: An instance of OpenSongBible and a mocked validate_file which raises an ValidationError
+ importer = OpenSongBible(MagicMock(), path='.', name='.', filename='')
+ importer.validate_file = MagicMock(**{'side_effect': ValidationError()})
+ importer.parse_xml = MagicMock()
+
+ # WHEN: Calling do_import
+ result = importer.do_import()
+
+ # THEN: do_import should return False after logging the exception. parse_xml should not be called.
+ mocked_log.exception.assert_called_once_with('Loading Bible from OpenSong file failed')
+ mocked_trace_error_handler.assert_called_once_with(mocked_log)
+ self.assertFalse(result)
+ self.assertFalse(importer.parse_xml.called)
+
+ @patch('openlp.plugins.bibles.lib.importers.opensong.log')
+ @patch('openlp.plugins.bibles.lib.importers.opensong.trace_error_handler')
+ def do_import_xml_syntax_error_test(self, mocked_trace_error_handler, mocked_log):
+ """
+ Test do_import when an etree.XMLSyntaxError exception is raised
+ """
+ # GIVEN: An instance of OpenSongBible and a mocked validate_file which raises an etree.XMLSyntaxError
+ importer = OpenSongBible(MagicMock(), path='.', name='.', filename='')
+ importer.validate_file = MagicMock(**{'side_effect': etree.XMLSyntaxError(None, None, None, None)})
+ importer.parse_xml = MagicMock()
+
+ # WHEN: Calling do_import
+ result = importer.do_import()
+
+ # THEN: do_import should return False after logging the exception. parse_xml should not be called.
+ mocked_log.exception.assert_called_once_with('Loading Bible from OpenSong file failed')
+ mocked_trace_error_handler.assert_called_once_with(mocked_log)
+ self.assertFalse(result)
+ self.assertFalse(importer.parse_xml.called)
+
+ @patch('openlp.plugins.bibles.lib.importers.opensong.log')
+ def do_import_no_language_test(self, mocked_log):
+ """
+ Test do_import when the user cancels the language selection dialog
+ """
+ # GIVEN: An instance of OpenSongBible and a mocked get_language which returns False
+ importer = OpenSongBible(MagicMock(), path='.', name='.', filename='')
+ importer.validate_file = MagicMock()
+ importer.parse_xml = MagicMock()
+ importer.get_language_id = MagicMock(**{'return_value': False})
+ importer.process_books = MagicMock()
+
+ # WHEN: Calling do_import
+ result = importer.do_import()
+
+ # THEN: do_import should return False and process_books should have not been called
+ self.assertFalse(result)
+ self.assertFalse(importer.process_books.called)
+
+ @patch('openlp.plugins.bibles.lib.importers.opensong.log')
+ def do_import_stop_import_test(self, mocked_log):
+ """
+ Test do_import when the stop_import_flag is set to True
+ """
+ # GIVEN: An instance of OpenSongBible and stop_import_flag set to True
+ importer = OpenSongBible(MagicMock(), path='.', name='.', filename='')
+ importer.validate_file = MagicMock()
+ importer.parse_xml = MagicMock()
+ importer.get_language_id = MagicMock(**{'return_value': 10})
+ importer.process_books = MagicMock()
+ importer.stop_import_flag = True
+
+ # WHEN: Calling do_import
+ result = importer.do_import()
+
+ # THEN: do_import should return False and process_books should have not been called
+ self.assertFalse(result)
+ self.assertTrue(importer.application.process_events.called)
+
+ self.assertTrue(importer.application.process_events.called)
+
+ @patch('openlp.plugins.bibles.lib.importers.opensong.log')
+ def do_import_completes_test(self, mocked_log):
+ """
+ Test do_import when it completes successfully
+ """
+ # GIVEN: An instance of OpenSongBible and stop_import_flag set to True
+ importer = OpenSongBible(MagicMock(), path='.', name='.', filename='')
+ importer.validate_file = MagicMock()
+ importer.parse_xml = MagicMock()
+ importer.get_language_id = MagicMock(**{'return_value': 10})
+ importer.process_books = MagicMock()
+ importer.stop_import_flag = False
+
+ # WHEN: Calling do_import
+ result = importer.do_import()
+
+ # THEN: do_import should return True
+ self.assertTrue(result)
def test_file_import(self):
"""
@@ -92,22 +534,3 @@
self.assertTrue(importer.create_verse.called)
for verse_tag, verse_text in test_data['verses']:
importer.create_verse.assert_any_call(importer.create_book().id, 1, int(verse_tag), verse_text)
-
- def test_zefania_import_error(self):
- """
- Test that we give an error message if trying to import a zefania bible
- """
- # GIVEN: A mocked out "manager" and mocked out critical_error_message_box and an import
- with patch('openlp.plugins.bibles.lib.importers.opensong.critical_error_message_box') as \
- mocked_critical_error_message_box:
- mocked_manager = MagicMock()
- importer = OpenSongBible(mocked_manager, path='.', name='.', filename='')
-
- # WHEN: An trying to import a zefania bible
- importer.filename = os.path.join(TEST_PATH, 'zefania-dk1933.xml')
- importer.do_import()
-
- # THEN: The importer should have "shown" an error message
- mocked_critical_error_message_box.assert_called_with(message='Incorrect Bible file type supplied. '
- 'This looks like a Zefania XML bible, '
- 'please use the Zefania import option.')
=== modified file 'tests/functional/openlp_plugins/bibles/test_zefaniaimport.py'
--- tests/functional/openlp_plugins/bibles/test_zefaniaimport.py 2016-08-09 20:45:25 +0000
+++ tests/functional/openlp_plugins/bibles/test_zefaniaimport.py 2016-08-21 07:47:26 +0000
@@ -42,14 +42,12 @@
def setUp(self):
self.registry_patcher = patch('openlp.plugins.bibles.lib.db.Registry')
+ self.addCleanup(self.registry_patcher.stop)
self.registry_patcher.start()
self.manager_patcher = patch('openlp.plugins.bibles.lib.db.Manager')
+ self.addCleanup(self.manager_patcher.stop)
self.manager_patcher.start()
- def tearDown(self):
- self.registry_patcher.stop()
- self.manager_patcher.stop()
-
def test_create_importer(self):
"""
Test creating an instance of the Zefania file importer
Follow ups
