head

openlp/core/common/openlpmixin.py

@@ -71,6 +71,12 @@ class OpenLPMixin(object):
         """
         self.logger.info(message)
 
+    def log_warning(self, message):
+        """
+        Common log warning handler
+        """
+        self.logger.warning(message)
+
     def log_error(self, message):
         """
         Common log error handler which prints the calling path

openlp/core/common/settings.py

@@ -214,7 +214,10 @@ class Settings(QtCore.QSettings):
         ('media/players', 'media/players_temp', [(media_players_conv, None)]),  # Convert phonon to system
         ('media/players_temp', 'media/players', []),  # Move temp setting from above to correct setting
         ('advanced/default color', 'core/logo background color', []),  # Default image renamed + moved to general > 2.4.
-        ('advanced/default image', '/core/logo file', [])  # Default image renamed + moved to general after 2.4.
+        ('advanced/default image', 'core/logo file', []),  # Default image renamed + moved to general after 2.4.
+        ('shortcuts/escapeItem', 'shortcuts/desktopScreenEnable', []),  # Escape item was removed in 2.6.
+        ('shortcuts/offlineHelpItem', 'shortcuts/HelpItem', []),  # Online and Offline help were combined in 2.6.
+        ('shortcuts/onlineHelpItem', 'shortcuts/HelpItem', [])  # Online and Offline help were combined in 2.6.
     ]
 
     @staticmethod
@@ -261,10 +264,10 @@ class Settings(QtCore.QSettings):
             'shortcuts/blankScreen': [QtGui.QKeySequence(QtCore.Qt.Key_Period)],
             'shortcuts/collapse': [QtGui.QKeySequence(QtCore.Qt.Key_Minus)],
             'shortcuts/desktopScreen': [QtGui.QKeySequence(QtCore.Qt.Key_D)],
+            'shortcuts/desktopScreenEnable': [QtGui.QKeySequence(QtCore.Qt.Key_Escape)],
             'shortcuts/delete': [QtGui.QKeySequence(QtGui.QKeySequence.Delete)],
             'shortcuts/down': [QtGui.QKeySequence(QtCore.Qt.Key_Down)],
             'shortcuts/editSong': [],
-            'shortcuts/escapeItem': [QtGui.QKeySequence(QtCore.Qt.Key_Escape)],
             'shortcuts/expand': [QtGui.QKeySequence(QtCore.Qt.Key_Plus)],
             'shortcuts/exportThemeItem': [],
             'shortcuts/fileNewItem': [QtGui.QKeySequence(QtGui.QKeySequence.New)],
@@ -273,6 +276,7 @@ class Settings(QtCore.QSettings):
             'shortcuts/fileSaveItem': [QtGui.QKeySequence(QtGui.QKeySequence.Save)],
             'shortcuts/fileOpenItem': [QtGui.QKeySequence(QtGui.QKeySequence.Open)],
             'shortcuts/goLive': [],
+            'shortcuts/HelpItem': [QtGui.QKeySequence(QtGui.QKeySequence.HelpContents)],
             'shortcuts/importThemeItem': [],
             'shortcuts/importBibleItem': [],
             'shortcuts/listViewBiblesDeleteItem': [QtGui.QKeySequence(QtGui.QKeySequence.Delete)],
@@ -333,8 +337,6 @@ class Settings(QtCore.QSettings):
                                            QtGui.QKeySequence(QtCore.Qt.Key_PageDown)],
             'shortcuts/nextService': [QtGui.QKeySequence(QtCore.Qt.Key_Right)],
             'shortcuts/newService': [],
-            'shortcuts/offlineHelpItem': [QtGui.QKeySequence(QtGui.QKeySequence.HelpContents)],
-            'shortcuts/onlineHelpItem': [QtGui.QKeySequence(QtGui.QKeySequence.HelpContents)],
             'shortcuts/openService': [],
             'shortcuts/saveService': [],
             'shortcuts/previousItem_live': [QtGui.QKeySequence(QtCore.Qt.Key_Up),

openlp/core/ui/exceptiondialog.py

@@ -97,9 +97,9 @@ class Ui_ExceptionDialog(object):
             translate('OpenLP.ExceptionDialog', '<strong>Please describe what you were trying to do.</strong> '
                                                 '&nbsp;If possible, write in English.'))
         exception_part1 = (translate('OpenLP.ExceptionDialog',
-                                     '<strong>Oops, OpenLP hit a problem and couldn\'t recover!</strong> <br><br>'
-                                     '<strong>You can help </strong> the OpenLP developers to <strong>fix this</strong>'
-                                     ' by<br> sending them a <strong>bug report</strong> to {email}{newlines}'
+                                     '<strong>Oops, OpenLP hit a problem and couldn\'t recover!<br><br>'
+                                     'You can help </strong> the OpenLP developers to <strong>fix this</strong>'
+                                     ' by<br> sending them a <strong>bug report to {email}</strong>{newlines}'
                                      ).format(email='<a href = "mailto:bugs@openlp.org" > bugs@openlp.org</a>',
                                               newlines='<br><br>'))
         self.message_label.setText(
@@ -107,7 +107,7 @@ class Ui_ExceptionDialog(object):
                       '<strong>No email app? </strong> You can <strong>save</strong> this '
                       'information to a <strong>file</strong> and<br>'
                       'send it from your <strong>mail on browser</strong> via an <strong>attachment.</strong><br><br>'
-                      '<strong>Thank you<strong> for being part of making OpenLP better!<br>'
+                      '<strong>Thank you</strong> for being part of making OpenLP better!<br>'
                       ).format(first_part=exception_part1))
         self.send_report_button.setText(translate('OpenLP.ExceptionDialog', 'Send E-Mail'))
         self.save_report_button.setText(translate('OpenLP.ExceptionDialog', 'Save to File'))

openlp/core/ui/exceptionform.py

@@ -208,7 +208,7 @@ class ExceptionForm(QtWidgets.QDialog, Ui_ExceptionDialog, RegistryProperties):
             self.__button_state(False)
             self.description_word_count.setText(
                 translate('OpenLP.ExceptionDialog', '<strong>Please enter a more detailed description of the situation'
-                          ))
+                                                    '</strong>'))
 
     def on_attach_file_button_clicked(self):
         """

openlp/core/ui/mainwindow.py

@@ -309,21 +309,13 @@ class Ui_MainWindow(object):
         self.about_item.setMenuRole(QtWidgets.QAction.AboutRole)
         if is_win():
             self.local_help_file = os.path.join(AppLocation.get_directory(AppLocation.AppDir), 'OpenLP.chm')
-            self.offline_help_item = create_action(main_window, 'offlineHelpItem',
-                                                   icon=':/system/system_help_contents.png',
-                                                   can_shortcuts=True,
-                                                   category=UiStrings().Help, triggers=self.on_offline_help_clicked)
         elif is_macosx():
             self.local_help_file = os.path.join(AppLocation.get_directory(AppLocation.AppDir),
                                                 '..', 'Resources', 'OpenLP.help')
-            self.offline_help_item = create_action(main_window, 'offlineHelpItem',
-                                                   icon=':/system/system_help_contents.png',
-                                                   can_shortcuts=True,
-                                                   category=UiStrings().Help, triggers=self.on_offline_help_clicked)
-        self.on_line_help_item = create_action(main_window, 'onlineHelpItem',
-                                               icon=':/system/system_online_help.png',
-                                               can_shortcuts=True,
-                                               category=UiStrings().Help, triggers=self.on_online_help_clicked)
+        self.on_help_item = create_action(main_window, 'HelpItem',
+                                          icon=':/system/system_help_contents.png',
+                                          can_shortcuts=True,
+                                          category=UiStrings().Help, triggers=self.on_help_clicked)
         self.web_site_item = create_action(main_window, 'webSiteItem', can_shortcuts=True, category=UiStrings().Help)
         # Shortcuts not connected to buttons or menu entries.
         self.search_shortcut_action = create_action(main_window,
@@ -362,11 +354,7 @@ class Ui_MainWindow(object):
         add_actions(self.tools_menu, (self.tools_open_data_folder, None))
         add_actions(self.tools_menu, (self.tools_first_time_wizard, None))
         add_actions(self.tools_menu, [self.update_theme_images])
-        if (is_win() or is_macosx()) and (hasattr(sys, 'frozen') and sys.frozen == 1):
-            add_actions(self.help_menu, (self.offline_help_item, self.on_line_help_item, None, self.web_site_item,
-                        self.about_item))
-        else:
-            add_actions(self.help_menu, (self.on_line_help_item, None, self.web_site_item, self.about_item))
+        add_actions(self.help_menu, (self.on_help_item, None, self.web_site_item, self.about_item))
         add_actions(self.menu_bar, (self.file_menu.menuAction(), self.view_menu.menuAction(),
                     self.tools_menu.menuAction(), self.settings_menu.menuAction(), self.help_menu.menuAction()))
         add_actions(self, [self.search_shortcut_action])
@@ -462,9 +450,7 @@ class Ui_MainWindow(object):
                                                                                    'from here.'))
         self.about_item.setText(translate('OpenLP.MainWindow', '&About'))
         self.about_item.setStatusTip(translate('OpenLP.MainWindow', 'More information about OpenLP.'))
-        if is_win() or is_macosx():
-            self.offline_help_item.setText(translate('OpenLP.MainWindow', '&User Guide'))
-        self.on_line_help_item.setText(translate('OpenLP.MainWindow', '&Online Help'))
+        self.on_help_item.setText(translate('OpenLP.MainWindow', '&User Manual'))
         self.search_shortcut_action.setText(UiStrings().Search)
         self.search_shortcut_action.setToolTip(
             translate('OpenLP.MainWindow', 'Jump to the search box of the current active plugin.'))
@@ -778,18 +764,16 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties):
         import webbrowser
         webbrowser.open_new('http://openlp.org/')
 
-    def on_offline_help_clicked(self):
+    def on_help_clicked(self):
         """
-        Load the local OpenLP help file
+        If is_macosx or is_win, open the local OpenLP help file.
+        Use the Online manual in other cases. (Linux)
         """
-        QtGui.QDesktopServices.openUrl(QtCore.QUrl("file:///" + self.local_help_file))
-
-    def on_online_help_clicked(self):
-        """
-        Load the online OpenLP manual
-        """
-        import webbrowser
-        webbrowser.open_new('http://manual.openlp.org/')
+        if is_macosx() or is_win():
+            QtGui.QDesktopServices.openUrl(QtCore.QUrl("file:///" + self.local_help_file))
+        else:
+            import webbrowser
+            webbrowser.open_new('http://manual.openlp.org/')
 
     def on_about_item_clicked(self):
         """

openlp/core/ui/media/systemplayer.py

@@ -95,6 +95,16 @@ class SystemPlayer(MediaPlayer):
                 mime_type_list.append(ext)
         log.info('MediaPlugin: %s extensions: %s', mime_type, ' '.join(extensions))
 
+    def disconnect_slots(self, signal):
+        """
+        Safely disconnect the slots from `signal`
+        """
+        try:
+            signal.disconnect()
+        except TypeError:
+            # If disconnect() is called on a signal without slots, it throws a TypeError
+            pass
+
     def setup(self, display):
         """
         Set up the player widgets
@@ -160,6 +170,7 @@ class SystemPlayer(MediaPlayer):
         if start_time > 0:
             self.seek(display, controller.media_info.start_time * 1000)
         self.volume(display, controller.media_info.volume)
+        self.disconnect_slots(display.media_player.durationChanged)
         display.media_player.durationChanged.connect(functools.partial(self.set_duration, controller))
         self.set_state(MediaState.Playing, display)
         display.video_widget.raise_()

openlp/core/ui/shortcutlistform.py

@@ -426,11 +426,11 @@ class ShortcutListForm(QtWidgets.QDialog, Ui_ShortcutListDialog, RegistryPropert
                     is_valid = False
         if not is_valid:
             text = translate('OpenLP.ShortcutListDialog',
-                             'The shortcut "{key}" is already assigned to another action, please'
-                             ' use a different shortcut.'
+                             'The shortcut "{key}" is already assigned to another action,\n'
+                             'please use a different shortcut.'
                              ).format(key=self.get_shortcut_string(key_sequence))
             self.main_window.warning_message(translate('OpenLP.ShortcutListDialog', 'Duplicate Shortcut'),
-                                             text, for_display=True)
+                                             text)
             self.dialog_was_shown = True
         return is_valid
 

openlp/core/ui/slidecontroller.py

@@ -234,25 +234,32 @@ class SlideController(DisplayController, RegistryProperties):
             self.hide_menu.setPopupMode(QtWidgets.QToolButton.MenuButtonPopup)
             self.hide_menu.setMenu(QtWidgets.QMenu(translate('OpenLP.SlideController', 'Hide'), self.toolbar))
             self.toolbar.add_toolbar_widget(self.hide_menu)
-            self.blank_screen = create_action(self, 'blankScreen',
-                                              text=translate('OpenLP.SlideController', 'Blank Screen'),
-                                              icon=':/slides/slide_blank.png',
-                                              checked=False, can_shortcuts=True, category=self.category,
-                                              triggers=self.on_blank_display)
-            self.theme_screen = create_action(self, 'themeScreen',
-                                              text=translate('OpenLP.SlideController', 'Blank to Theme'),
-                                              icon=':/slides/slide_theme.png',
-                                              checked=False, can_shortcuts=True, category=self.category,
-                                              triggers=self.on_theme_display)
+            # The order of the blank to modes in Shortcuts list comes from here.
+            self.desktop_screen_enable = create_action(self, 'desktopScreenEnable',
+                                                       text=translate('OpenLP.SlideController', 'Show Desktop'),
+                                                       icon=':/slides/slide_desktop.png', can_shortcuts=True,
+                                                       context=QtCore.Qt.WidgetWithChildrenShortcut,
+                                                       category=self.category, triggers=self.on_hide_display_enable)
             self.desktop_screen = create_action(self, 'desktopScreen',
-                                                text=translate('OpenLP.SlideController', 'Show Desktop'),
+                                                text=translate('OpenLP.SlideController', 'Toggle Desktop'),
                                                 icon=':/slides/slide_desktop.png',
                                                 checked=False, can_shortcuts=True, category=self.category,
                                                 triggers=self.on_hide_display)
+            self.theme_screen = create_action(self, 'themeScreen',
+                                              text=translate('OpenLP.SlideController', 'Toggle Blank to Theme'),
+                                              icon=':/slides/slide_theme.png',
+                                              checked=False, can_shortcuts=True, category=self.category,
+                                              triggers=self.on_theme_display)
+            self.blank_screen = create_action(self, 'blankScreen',
+                                              text=translate('OpenLP.SlideController', 'Toggle Blank Screen'),
+                                              icon=':/slides/slide_blank.png',
+                                              checked=False, can_shortcuts=True, category=self.category,
+                                              triggers=self.on_blank_display)
             self.hide_menu.setDefaultAction(self.blank_screen)
             self.hide_menu.menu().addAction(self.blank_screen)
             self.hide_menu.menu().addAction(self.theme_screen)
             self.hide_menu.menu().addAction(self.desktop_screen)
+            self.hide_menu.menu().addAction(self.desktop_screen_enable)
             # Wide menu of display control buttons.
             self.blank_screen_button = QtWidgets.QToolButton(self.toolbar)
             self.blank_screen_button.setObjectName('blank_screen_button')
@@ -512,23 +519,6 @@ class SlideController(DisplayController, RegistryProperties):
                                           can_shortcuts=True, context=QtCore.Qt.WidgetWithChildrenShortcut,
                                           category=self.category,
                                           triggers=self.service_next)
-        self.escape_item = create_action(parent, 'escapeItem',
-                                         text=translate('OpenLP.SlideController', 'Escape Item'),
-                                         can_shortcuts=True, context=QtCore.Qt.WidgetWithChildrenShortcut,
-                                         category=self.category,
-                                         triggers=self.live_escape)
-
-    def live_escape(self, field=None):
-        """
-        If you press ESC on the live screen it should close the display temporarily.
-        """
-        self.display.setVisible(False)
-        self.media_controller.media_stop(self)
-        # Stop looping if active
-        if self.play_slides_loop.isChecked():
-            self.on_play_slides_loop(False)
-        elif self.play_slides_once.isChecked():
-            self.on_play_slides_once(False)
 
     def toggle_display(self, action):
         """
@@ -622,7 +612,7 @@ class SlideController(DisplayController, RegistryProperties):
         widget.addActions([
             self.previous_item, self.next_item,
             self.previous_service, self.next_service,
-            self.escape_item,
+            self.desktop_screen_enable,
             self.desktop_screen,
             self.theme_screen,
             self.blank_screen])
@@ -965,7 +955,7 @@ class SlideController(DisplayController, RegistryProperties):
             else:
                 Registry().execute('live_display_show')
         else:
-            self.live_escape()
+            self.on_hide_display_enable()
 
     def on_slide_blank(self):
         """
@@ -1025,6 +1015,7 @@ class SlideController(DisplayController, RegistryProperties):
     def on_hide_display(self, checked=None):
         """
         Handle the Hide screen button
+        This toggles the desktop screen.
 
         :param checked: the new state of the of the widget
         """
@@ -1043,6 +1034,20 @@ class SlideController(DisplayController, RegistryProperties):
         self.update_preview()
         self.on_toggle_loop()
 
+    def on_hide_display_enable(self, checked=None):
+        """
+        Handle the on_hide_display_enable
+        This only enables the desktop screen.
+
+        :param checked: the new state of the of the widget
+        """
+        self.blank_screen.setChecked(False)
+        self.theme_screen.setChecked(False)
+        Registry().execute('live_display_hide', HideMode.Screen)
+        self.desktop_screen.setChecked(True)
+        self.update_preview()
+        self.on_toggle_loop()
+
     def blank_plugin(self):
         """
         Blank/Hide the display screen within a plugin if required.

openlp/plugins/bibles/bibleplugin.py

@@ -140,10 +140,10 @@ class BiblePlugin(Plugin):
 
     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 @@ class BiblePlugin(Plugin):
 
     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()

openlp/plugins/bibles/forms/bibleimportform.py

@@ -25,6 +25,7 @@ The bible import functions for OpenLP
 import logging
 import os
 import urllib.error
+from lxml import etree
 
 from PyQt5 import QtWidgets
 try:
@@ -33,14 +34,15 @@ try:
 except:
     PYSWORD_AVAILABLE = False
 
-from openlp.core.common import AppLocation, Settings, UiStrings, translate, clean_filename
+from openlp.core.common import AppLocation, Settings, UiStrings, trace_error_handler, translate
+from openlp.core.common.languagemanager import get_locale_key
 from openlp.core.lib.db import delete_database
+from openlp.core.lib.exceptions import ValidationError
 from openlp.core.lib.ui import critical_error_message_box
 from openlp.core.ui.lib.wizard import OpenLPWizard, WizardStrings
-from openlp.core.common.languagemanager import get_locale_key
-from openlp.plugins.bibles.lib.manager import BibleFormat
 from openlp.plugins.bibles.lib.db import clean_filename
 from openlp.plugins.bibles.lib.importers.http import CWExtract, BGExtract, BSExtract
+from openlp.plugins.bibles.lib.manager import BibleFormat
 
 log = logging.getLogger(__name__)
 
@@ -809,16 +811,22 @@ class BibleImportForm(OpenLPWizard):
                                                      sword_path=self.field('sword_zip_path'),
                                                      sword_key=self.sword_zipbible_combo_box.itemData(
                                                          self.sword_zipbible_combo_box.currentIndex()))
-        if importer.do_import(license_version):
-            self.manager.save_meta_data(license_version, license_version, license_copyright, license_permissions)
-            self.manager.reload_bibles()
-            if bible_type == BibleFormat.WebDownload:
-                self.progress_label.setText(
-                    translate('BiblesPlugin.ImportWizardForm', 'Registered Bible. Please note, that verses will be '
-                              'downloaded on demand and thus an internet connection is required.'))
-            else:
-                self.progress_label.setText(WizardStrings.FinishedImport)
-        else:
-            self.progress_label.setText(translate('BiblesPlugin.ImportWizardForm', 'Your Bible import failed.'))
-            del self.manager.db_cache[importer.name]
-            delete_database(self.plugin.settings_section, importer.file)
+
+        try:
+            if importer.do_import(license_version) and not importer.stop_import_flag:
+                self.manager.save_meta_data(license_version, license_version, license_copyright, license_permissions)
+                self.manager.reload_bibles()
+                if bible_type == BibleFormat.WebDownload:
+                    self.progress_label.setText(
+                        translate('BiblesPlugin.ImportWizardForm', 'Registered Bible. Please note, that verses will be '
+                                  'downloaded on demand and thus an internet connection is required.'))
+                else:
+                    self.progress_label.setText(WizardStrings.FinishedImport)
+                return
+        except (AttributeError, ValidationError, etree.XMLSyntaxError):
+            log.exception('Importing bible failed')
+            trace_error_handler(log)
+
+        self.progress_label.setText(translate('BiblesPlugin.ImportWizardForm', 'Your Bible import failed.'))
+        del self.manager.db_cache[importer.name]
+        delete_database(self.plugin.settings_section, importer.file)

openlp/plugins/bibles/lib/__init__.py

@@ -173,7 +173,7 @@ class BibleStrings(object):
 
 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 @@ def update_reference_separators():
         # 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)

openlp/plugins/bibles/lib/bibleimport.py

@@ -20,25 +20,84 @@
 # Temple Place, Suite 330, Boston, MA 02111-1307 USA                          #
 ###############################################################################
 
-import logging
-
 from lxml import etree, objectify
+from zipfile import is_zipfile
 
-from openlp.core.common import OpenLPMixin, languages
+from openlp.core.common import OpenLPMixin, Registry, RegistryProperties, languages, translate
 from openlp.core.lib import ValidationError
-from openlp.plugins.bibles.lib.db import BibleDB, BiblesResourcesDB
-
-log = logging.getLogger(__name__)
+from openlp.core.lib.ui import critical_error_message_box
+from openlp.plugins.bibles.lib.db import AlternativeBookNamesDB, BibleDB, BiblesResourcesDB
 
 
-class BibleImport(OpenLPMixin, BibleDB):
+class BibleImport(OpenLPMixin, RegistryProperties, BibleDB):
     """
     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
+        self.wizard = None
+        self.stop_import_flag = False
+        Registry().register_function('openlp_stop_wizard', self.stop_import)
+
+    @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_book_ref_id_by_name(self, book, maxbooks=66, language_id=None):
+        """
+        Find the book id from the name or abbreviation of the book. If it doesn't currently exist, ask the user.
+
+        :param book: The name or abbreviation of the book
+        :param maxbooks: The number of books in the bible
+        :param language_id: The language_id of the bible
+        :return: The id of the bible, or None
+        """
+        self.log_debug('BibleDB.get_book_ref_id_by_name:("{book}", "{lang}")'.format(book=book, lang=language_id))
+        book_temp = BiblesResourcesDB.get_book(book, True)
+        if book_temp:
+            return book_temp['id']
+        book_id = BiblesResourcesDB.get_alternative_book_name(book)
+        if book_id:
+            return book_id
+        book_id = AlternativeBookNamesDB.get_book_reference_id(book)
+        if book_id:
+            return book_id
+        from openlp.plugins.bibles.forms import BookNameForm
+        book_name = BookNameForm(self.wizard)
+        if book_name.exec(book, self.get_books(), maxbooks) and book_name.book_id:
+            AlternativeBookNamesDB.create_alternative_book_name(book, book_name.book_id, language_id)
+            return book_name.book_id
+
+    def get_language(self, bible_name=None):
+        """
+        If no language is given it calls a dialog window where the user could  select the bible language.
+        Return the language id of a bible.
+
+        :param bible_name: The language the bible is.
+        """
+        self.log_debug('BibleImpoer.get_language()')
+        from openlp.plugins.bibles.forms import LanguageForm
+        language_id = None
+        language_form = LanguageForm(self.wizard)
+        if language_form.exec(bible_name):
+            combo_box = language_form.language_combo_box
+            language_id = combo_box.itemData(combo_box.currentIndex())
+        if not language_id:
+            return None
+        self.save_meta('language_id', language_id)
+        return language_id
 
     def get_language_id(self, file_language=None, bible_name=None):
         """
@@ -58,8 +117,8 @@ class BibleImport(OpenLPMixin, BibleDB):
             language_id = self.get_language(bible_name)
         if not language_id:
             # User cancelled get_language dialog
-            log.error('Language detection failed when importing from "{name}". User aborted language selection.'
-                      .format(name=bible_name))
+            self.log_error('Language detection failed when importing from "{name}". User aborted language selection.'
+                           .format(name=bible_name))
             return None
         self.save_meta('language_id', language_id)
         return language_id
@@ -77,7 +136,7 @@ class BibleImport(OpenLPMixin, BibleDB):
         if name:
             book_ref_id = self.get_book_ref_id_by_name(name, no_of_books, language_id)
         else:
-            log.debug('No book name supplied. Falling back to guess_id')
+            self.log_debug('No book name supplied. Falling back to guess_id')
             book_ref_id = guess_id
         if not book_ref_id:
             raise ValidationError(msg='Could not resolve book_ref_id in "{}"'.format(self.filename))
@@ -87,8 +146,7 @@ class BibleImport(OpenLPMixin, BibleDB):
                                       'importing {file}'.format(book_ref=book_ref_id, file=self.filename))
         return self.create_book(name, book_ref_id, book_details['testament_id'])
 
-    @staticmethod
-    def parse_xml(filename, use_objectify=False, elements=None, tags=None):
+    def parse_xml(self, filename, use_objectify=False, elements=None, tags=None):
         """
         Parse and clean the supplied file by removing any elements or tags we don't use.
         :param filename: The filename of the xml file to parse. Str
@@ -97,17 +155,80 @@ class BibleImport(OpenLPMixin, BibleDB):
         :param tags: A tuple of element names (Str) to remove, preserving their content.
         :return: The root element of the xml document
         """
-        with open(filename, 'rb') as import_file:
-            # NOTE: We don't need to do any of the normal encoding detection here, because lxml does it's own encoding
-            # detection, and the two mechanisms together interfere with each other.
-            if not use_objectify:
-                tree = etree.parse(import_file, parser=etree.XMLParser(recover=True))
-            else:
-                tree = objectify.parse(import_file, parser=objectify.makeparser(recover=True))
-            if elements:
-                # Strip tags we don't use - remove content
-                etree.strip_elements(tree, elements, with_tail=False)
-            if tags:
-                # Strip tags we don't use - keep content
-                etree.strip_tags(tree, tags)
-            return tree.getroot()
+        try:
+            with open(filename, 'rb') as import_file:
+                # NOTE: We don't need to do any of the normal encoding detection here, because lxml does it's own
+                # encoding detection, and the two mechanisms together interfere with each other.
+                if not use_objectify:
+                    tree = etree.parse(import_file, parser=etree.XMLParser(recover=True))
+                else:
+                    tree = objectify.parse(import_file, parser=objectify.makeparser(recover=True))
+                if elements or tags:
+                    self.wizard.increment_progress_bar(
+                        translate('BiblesPlugin.OsisImport', 'Removing unused tags (this may take a few minutes)...'))
+                if elements:
+                    # Strip tags we don't use - remove content
+                    etree.strip_elements(tree, elements, with_tail=False)
+                if tags:
+                    # Strip tags we don't use - keep content
+                    etree.strip_tags(tree, tags)
+                return tree.getroot()
+        except OSError as e:
+            self.log_exception('Opening {file_name} failed.'.format(file_name=e.filename))
+            critical_error_message_box(
+                title='An Error Occured When Opening A File',
+                message='The following error occurred when trying to open\n{file_name}:\n\n{error}'
+                .format(file_name=e.filename, error=e.strerror))
+        return None
+
+    def register(self, wizard):
+        """
+        This method basically just initialises the database. It is called from the Bible Manager when a Bible is
+        imported. Descendant classes may want to override this method to supply their own custom
+        initialisation as well.
+
+        :param wizard: The actual Qt wizard form.
+        """
+        self.wizard = wizard
+        return self.name
+
+    def set_current_chapter(self, book_name, chapter_name):
+        self.wizard.increment_progress_bar(translate('BiblesPlugin.OsisImport', 'Importing {book} {chapter}...')
+                                           .format(book=book_name, chapter=chapter_name))
+
+    def stop_import(self):
+        """
+        Stops the import of the Bible.
+        """
+        self.log_debug('Stopping import')
+        self.stop_import_flag = True
+
+    def validate_xml_file(self, filename, tag):
+        """
+        Validate the supplied file
+
+        :param filename: The supplied file
+        :param tag: The expected root tag type
+        :return: True if valid. ValidationError is raised otherwise.
+        """
+        if BibleImport.is_compressed(filename):
+            raise ValidationError(msg='Compressed file')
+        bible = self.parse_xml(filename, use_objectify=True)
+        if bible is None:
+            raise ValidationError(msg='Error when opening file')
+        root_tag = bible.tag.lower()
+        bible_type = translate('BiblesPlugin.BibleImport', 'unknown type of',
+                               'This looks like an unknown type of XML bible.')
+        if root_tag == tag:
+            return True
+        elif root_tag == 'bible':
+            bible_type = "OpenSong"
+        elif root_tag == '{http://www.bibletechnologies.net/2003/osis/namespace}osis':
+            bible_type = 'OSIS'
+        elif root_tag == 'xmlbible':
+            bible_type = 'Zefania'
+        critical_error_message_box(
+            message=translate('BiblesPlugin.BibleImport',
+                              'Incorrect Bible file type supplied. This looks like an {bible_type} XML bible.'
+                              .format(bible_type=bible_type)))
+        raise ValidationError(msg='Invalid xml.')

openlp/plugins/bibles/lib/db.py

@@ -33,7 +33,7 @@ from sqlalchemy.exc import OperationalError
 from sqlalchemy.orm import class_mapper, mapper, relation
 from sqlalchemy.orm.exc import UnmappedClassError
 
-from openlp.core.common import Registry, RegistryProperties, AppLocation, translate, clean_filename
+from openlp.core.common import AppLocation, translate, clean_filename
 from openlp.core.lib.db import BaseModel, init_db, Manager
 from openlp.core.lib.ui import critical_error_message_box
 from openlp.plugins.bibles.lib import upgrade
@@ -106,7 +106,7 @@ def init_schema(url):
     return session
 
 
-class BibleDB(Manager, RegistryProperties):
+class BibleDB(Manager):
     """
     This class represents a database-bound Bible. It is used as a base class for all the custom importers, so that
     the can implement their own import methods, but benefit from the database methods in here via inheritance,
@@ -140,7 +140,6 @@ class BibleDB(Manager, RegistryProperties):
             raise KeyError('Missing keyword argument "path".')
         if 'name' not in kwargs and 'file' not in kwargs:
             raise KeyError('Missing keyword argument "name" or "file".')
-        self.stop_import_flag = False
         if 'name' in kwargs:
             self.name = kwargs['name']
             if not isinstance(self.name, str):
@@ -153,15 +152,6 @@ class BibleDB(Manager, RegistryProperties):
                 self.get_name()
         if 'path' in kwargs:
             self.path = kwargs['path']
-        self.wizard = None
-        Registry().register_function('openlp_stop_wizard', self.stop_import)
-
-    def stop_import(self):
-        """
-        Stops the import of the Bible.
-        """
-        log.debug('Stopping import')
-        self.stop_import_flag = True
 
     def get_name(self):
         """
@@ -171,17 +161,6 @@ class BibleDB(Manager, RegistryProperties):
         self.name = version_name.value if version_name else None
         return self.name
 
-    def register(self, wizard):
-        """
-        This method basically just initialises the database. It is called from the Bible Manager when a Bible is
-        imported. Descendant classes may want to override this method to supply their own custom
-        initialisation as well.
-
-        :param wizard: The actual Qt wizard form.
-        """
-        self.wizard = wizard
-        return self.name
-
     def create_book(self, name, bk_ref_id, testament=1):
         """
         Add a book to the database.
@@ -306,26 +285,6 @@ class BibleDB(Manager, RegistryProperties):
         log.debug('BibleDB.get_book_by_book_ref_id("{ref}")'.format(ref=ref_id))
         return self.get_object_filtered(Book, Book.book_reference_id.like(ref_id))
 
-    def get_book_ref_id_by_name(self, book, maxbooks, language_id=None):
-        log.debug('BibleDB.get_book_ref_id_by_name:("{book}", "{lang}")'.format(book=book, lang=language_id))
-        book_id = None
-        if BiblesResourcesDB.get_book(book, True):
-            book_temp = BiblesResourcesDB.get_book(book, True)
-            book_id = book_temp['id']
-        elif BiblesResourcesDB.get_alternative_book_name(book):
-            book_id = BiblesResourcesDB.get_alternative_book_name(book)
-        elif AlternativeBookNamesDB.get_book_reference_id(book):
-            book_id = AlternativeBookNamesDB.get_book_reference_id(book)
-        else:
-            from openlp.plugins.bibles.forms import BookNameForm
-            book_name = BookNameForm(self.wizard)
-            if book_name.exec(book, self.get_books(), maxbooks):
-                book_id = book_name.book_id
-            if book_id:
-                AlternativeBookNamesDB.create_alternative_book_name(
-                    book, book_id, language_id)
-        return book_id
-
     def get_book_ref_id_by_localised_name(self, book, language_selection):
         """
         Return the id of a named book.
@@ -462,25 +421,6 @@ class BibleDB(Manager, RegistryProperties):
             return 0
         return count
 
-    def get_language(self, bible_name=None):
-        """
-        If no language is given it calls a dialog window where the user could  select the bible language.
-        Return the language id of a bible.
-
-        :param bible_name: The language the bible is.
-        """
-        log.debug('BibleDB.get_language()')
-        from openlp.plugins.bibles.forms import LanguageForm
-        language_id = None
-        language_form = LanguageForm(self.wizard)
-        if language_form.exec(bible_name):
-            combo_box = language_form.language_combo_box
-            language_id = combo_box.itemData(combo_box.currentIndex())
-        if not language_id:
-            return None
-        self.save_meta('language_id', language_id)
-        return language_id
-
     def dump_bible(self):
         """
         Utility debugging method to dump the contents of a bible.

openlp/plugins/bibles/lib/importers/csvbible.py

@@ -50,7 +50,6 @@ There are two acceptable formats of the verses file.  They are:
 All CSV files are expected to use a comma (',') as the delimiter and double quotes ('"') as the quote symbol.
 """
 import csv
-import logging
 from collections import namedtuple
 
 from openlp.core.common import get_file_encoding, translate
@@ -58,8 +57,6 @@ from openlp.core.lib.exceptions import ValidationError
 from openlp.plugins.bibles.lib.bibleimport import BibleImport
 
 
-log = logging.getLogger(__name__)
-
 Book = namedtuple('Book', 'id, testament_id, name, abbreviation')
 Verse = namedtuple('Verse', 'book_id_name, chapter_number, number, text')
 
@@ -68,15 +65,13 @@ class CSVBible(BibleImport):
     """
     This class provides a specialisation for importing of CSV Bibles.
     """
-    log.info('CSVBible loaded')
-
     def __init__(self, *args, **kwargs):
         """
         Loads a Bible from a set of CSV files. This class assumes the files contain all the information and a clean
         bible is being loaded.
         """
-        log.info(self.__class__.__name__)
         super().__init__(*args, **kwargs)
+        self.log_info(self.__class__.__name__)
         self.books_file = kwargs['booksfile']
         self.verses_file = kwargs['versefile']
 
@@ -123,12 +118,11 @@ class CSVBible(BibleImport):
         number_of_books = len(books)
         for book in books:
             if self.stop_import_flag:
-                return None
+                break
             self.wizard.increment_progress_bar(
                 translate('BiblesPlugin.CSVBible', 'Importing books... {book}').format(book=book.name))
             self.find_and_create_book(book.name, number_of_books, self.language_id)
             book_list.update({int(book.id): book.name})
-        self.application.process_events()
         return book_list
 
     def process_verses(self, verses, books):
@@ -142,7 +136,7 @@ class CSVBible(BibleImport):
         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)
@@ -151,9 +145,7 @@ class CSVBible(BibleImport):
                     translate('BiblesPlugin.CSVBible', 'Importing verses from {book}...',
                               'Importing verses from <book name>...').format(book=book.name))
                 self.session.commit()
-            self.create_verse(book.id, verse.chapter_number, verse.number, verse.text)
-        self.wizard.increment_progress_bar(translate('BiblesPlugin.CSVBible', 'Importing verses... done.'))
-        self.application.process_events()
+            self.create_verse(book.id, int(verse.chapter_number), int(verse.number), verse.text)
         self.session.commit()
 
     def do_import(self, bible_name=None):
@@ -163,24 +155,16 @@ class CSVBible(BibleImport):
         :param bible_name: Optional name of the bible being imported. Str or None
         :return: True if the import was successful, False if it failed or was cancelled
         """
-        try:
-            self.language_id = self.get_language(bible_name)
-            if not self.language_id:
-                raise ValidationError(msg='Invalid language selected')
-            books = self.parse_csv_file(self.books_file, Book)
-            self.wizard.progress_bar.setValue(0)
-            self.wizard.progress_bar.setMinimum(0)
-            self.wizard.progress_bar.setMaximum(len(books))
-            book_list = self.process_books(books)
-            if self.stop_import_flag:
-                return False
-            verses = self.parse_csv_file(self.verses_file, Verse)
-            self.wizard.progress_bar.setValue(0)
-            self.wizard.progress_bar.setMaximum(len(books) + 1)
-            self.process_verses(verses, book_list)
-            if self.stop_import_flag:
-                return False
-        except ValidationError:
-            log.exception('Could not import CSV bible')
+        self.language_id = self.get_language(bible_name)
+        if not self.language_id:
             return False
+        books = self.parse_csv_file(self.books_file, Book)
+        self.wizard.progress_bar.setValue(0)
+        self.wizard.progress_bar.setMinimum(0)
+        self.wizard.progress_bar.setMaximum(len(books))
+        book_list = self.process_books(books)
+        verses = self.parse_csv_file(self.verses_file, Verse)
+        self.wizard.progress_bar.setValue(0)
+        self.wizard.progress_bar.setMaximum(len(books) + 1)
+        self.process_verses(verses, book_list)
         return True

openlp/plugins/bibles/lib/importers/http.py

@@ -493,7 +493,7 @@ class CWExtract(RegistryProperties):
         for verse in verses_div:
             self.application.process_events()
             verse_number = int(verse.find('strong').contents[0])
-            verse_span = verse.find('span')
+            verse_span = verse.find('span', class_='verse-%d' % verse_number)
             tags_to_remove = verse_span.find_all(['a', 'sup'])
             for tag in tags_to_remove:
                 tag.decompose()

openlp/plugins/bibles/lib/importers/opensong.py

@@ -20,109 +20,126 @@
 # Temple Place, Suite 330, Boston, MA 02111-1307 USA                          #
 ###############################################################################
 
-import logging
-from lxml import etree, objectify
-
-from openlp.core.common import translate, trace_error_handler
-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__)
+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 += get_text(sub_element)
+    if element.tail:
+        verse_text += element.tail
+    return verse_text
+
+
+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
 
 
 class OpenSongBible(BibleImport):
     """
     OpenSong Bible format importer class. This class is used to import Bibles from OpenSong's XML format.
     """
-    def get_text(self, element):
-        """
-        Recursively get all text in an objectify element and its child elements.
 
-        :param element: An objectify element to get the text from
+    def parse_verse_number(self, number, previous_number):
         """
-        verse_text = ''
-        if element.text:
-            verse_text = element.text
-        for sub_element in element.iterchildren():
-            verse_text += self.get_text(sub_element)
-        if element.tail:
-            verse_text += element.tail
-        return verse_text
+        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:
+            self.log_warning('Illegal verse number: {verse_no}'.format(verse_no=str(number)))
+        return previous_number + 1
+
+    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 = parse_chapter_number(chapter.attrib['n'], chapter_number)
+            self.set_current_chapter(book.name, chapter_number)
+            self.process_verses(book, chapter_number, chapter.v)
+
+    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, 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:
-            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:
-                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.application.process_events()
-        except etree.XMLSyntaxError as inst:
-            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
-        if self.stop_import_flag:
+        self.log_debug('Starting OpenSong import from "{name}"'.format(name=self.filename))
+        self.validate_xml_file(self.filename, 'bible')
+        bible = self.parse_xml(self.filename, use_objectify=True)
+        if bible is None:
             return False
-        else:
-            return success
+        # No language info in the opensong format, so ask the user
+        self.language_id = self.get_language_id(bible_name=self.filename)
+        if not self.language_id:
+            return False
+        self.process_books(bible.b)
+        return True

openlp/plugins/bibles/lib/importers/osis.py

@@ -20,15 +20,9 @@
 # Temple Place, Suite 330, Boston, MA 02111-1307 USA                          #
 ###############################################################################
 
-import logging
 from lxml import etree
 
-from openlp.core.common import translate, trace_error_handler
-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 BiblesResourcesDB
-
-log = logging.getLogger(__name__)
 
 NS = {'ns': 'http://www.bibletechnologies.net/2003/OSIS/namespace'}
 # Tags we don't use and can remove the content
@@ -74,104 +68,106 @@ REMOVABLE_TAGS = (
     '{http://www.bibletechnologies.net/2003/OSIS/namespace}caption'
 )
 
-
-def replacement(match):
-    return match.group(2).upper()
+# Precompile a few xpath-querys
+verse_in_chapter = etree.XPath('//ns:chapter[1]/ns:verse', namespaces=NS)
+text_in_verse = etree.XPath('//ns:verse[1]/text()', namespaces=NS)
 
 
 class OSISBible(BibleImport):
     """
     `OSIS <http://www.bibletechnologies.net/>`_ Bible format importer class.
     """
+    def process_books(self, bible_data):
+        """
+        Extract and create the bible books from the parsed xml
+
+        :param bible_data: parsed xml
+        :return: None
+        """
+        # Find books in the bible
+        bible_books = bible_data.xpath("//ns:div[@type='book']", namespaces=NS)
+        no_of_books = len(bible_books)
+        for book in bible_books:
+            if self.stop_import_flag:
+                break
+            # Remove div-tags in the book
+            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, self.language_id)
+            self.process_chapters(db_book, book)
+            self.session.commit()
+
+    def process_chapters(self, book, chapters):
+        """
+        Extract the chapters, and do some initial processing of the verses
+
+        :param book: An OpenLP bible database book object
+        :param chapters: parsed chapters
+        :return: None
+        """
+        # Find out if chapter-tags contains the verses, or if it is used as milestone/anchor
+        if verse_in_chapter(chapters):
+            # The chapter tags contains the verses
+            for chapter in chapters:
+                chapter_number = int(chapter.get("osisID").split('.')[1])
+                self.set_current_chapter(book.name, chapter_number)
+                # Find out if verse-tags contains the text, or if it is used as milestone/anchor
+                if not text_in_verse(chapter):
+                    # verse-tags are used as milestone
+                    for verse in chapter:
+                        # If this tag marks the start of a verse, the verse text is between this tag and
+                        # the next tag, which the "tail" attribute gives us.
+                        self.process_verse(book, chapter_number, verse, use_milestones=True)
+                else:
+                    # Verse-tags contains the text
+                    for verse in chapter:
+                        self.process_verse(book, chapter_number, verse)
+        else:
+            # The chapter tags is used as milestones. For now we assume verses is also milestones
+            chapter_number = 0
+            for element in chapters:
+                if element.tag == '{http://www.bibletechnologies.net/2003/OSIS/namespace}chapter' \
+                        and element.get('sID'):
+                    chapter_number = int(element.get("osisID").split('.')[1])
+                    self.set_current_chapter(book.name, chapter_number)
+                elif element.tag == '{http://www.bibletechnologies.net/2003/OSIS/namespace}verse':
+                    # If this tag marks the start of a verse, the verse text is between this tag and
+                    # the next tag, which the "tail" attribute gives us.
+                    self.process_verse(book, chapter_number, element, use_milestones=True)
+
+    def process_verse(self, book, chapter_number, element, use_milestones=False):
+        """
+        Process a verse element
+        :param book: A database Book object
+        :param chapter_number: The chapter number to add the verses to (int)
+        :param element: The verse element to process. (etree element type)
+        :param use_milestones: set to True to process a 'milestone' verse. Defaults to False
+        :return: None
+        """
+        osis_id = element.get("osisID")
+        if not osis_id:
+            return None
+        verse_number = int(osis_id.split('.')[2])
+        verse_text = ''
+        if use_milestones and element.get('sID'):
+            verse_text = element.tail
+        elif not use_milestones:
+            verse_text = element.text
+        if verse_text:
+            self.create_verse(book.id, chapter_number, verse_number, verse_text.strip())
+
     def do_import(self, bible_name=None):
         """
         Loads a Bible from file.
         """
-        log.debug('Starting OSIS import from "{name}"'.format(name=self.filename))
-        success = True
-        try:
-            self.wizard.increment_progress_bar(translate('BiblesPlugin.OsisImport',
-                                                         'Removing unused tags (this may take a few minutes)...'))
-            osis_bible_tree = self.parse_xml(self.filename, elements=REMOVABLE_ELEMENTS, tags=REMOVABLE_TAGS)
-            # Find bible language]
-            language = osis_bible_tree.xpath("//ns:osisText/@xml:lang", namespaces=NS)
-            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))
-            # 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)
-            # Find books in the bible
-            bible_books = osis_bible_tree.xpath("//ns:div[@type='book']", namespaces=NS)
-            for book in bible_books:
-                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'])
-                # 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
-                    for chapter in book:
-                        chapter_number = chapter.get("osisID").split('.')[1]
-                        # Find out if verse-tags contains the text, or if it is used as milestone/anchor
-                        if int(text_in_verse(chapter)) == 0:
-                            # verse-tags are used as milestone
-                            for verse in chapter:
-                                # If this tag marks the start of a verse, the verse text is between this tag and
-                                # the next tag, which the "tail" attribute gives us.
-                                if verse.get('sID'):
-                                    verse_number = verse.get("osisID").split('.')[2]
-                                    verse_text = verse.tail
-                                    if verse_text:
-                                        self.create_verse(db_book.id, chapter_number, verse_number, verse_text.strip())
-                        else:
-                            # Verse-tags contains the text
-                            for verse in chapter:
-                                verse_number = verse.get("osisID").split('.')[2]
-                                if verse.text:
-                                    self.create_verse(db_book.id, chapter_number, verse_number, verse.text.strip())
-                        self.wizard.increment_progress_bar(
-                            translate('BiblesPlugin.OsisImport', 'Importing %(bookname)s %(chapter)s...') %
-                            {'bookname': db_book.name, 'chapter': chapter_number})
-                else:
-                    # The chapter tags is used as milestones. For now we assume verses is also milestones
-                    chapter_number = 0
-                    for element in book:
-                        if element.tag == '{http://www.bibletechnologies.net/2003/OSIS/namespace}chapter' \
-                                and element.get('sID'):
-                            chapter_number = element.get("osisID").split('.')[1]
-                            self.wizard.increment_progress_bar(
-                                translate('BiblesPlugin.OsisImport', 'Importing %(bookname)s %(chapter)s...') %
-                                {'bookname': db_book.name, 'chapter': chapter_number})
-                        elif element.tag == '{http://www.bibletechnologies.net/2003/OSIS/namespace}verse' \
-                                and element.get('sID'):
-                            # If this tag marks the start of a verse, the verse text is between this tag and
-                            # the next tag, which the "tail" attribute gives us.
-                            verse_number = element.get("osisID").split('.')[2]
-                            verse_text = element.tail
-                            if verse_text:
-                                self.create_verse(db_book.id, chapter_number, verse_number, verse_text.strip())
-                self.session.commit()
-            self.application.process_events()
-        except (ValueError, IOError):
-            log.exception('Loading bible from OSIS file failed')
-            trace_error_handler(log)
-            success = False
-        except etree.XMLSyntaxError as e:
-            log.exception('Loading bible from OSIS file failed')
-            trace_error_handler(log)
-            success = False
-            critical_error_message_box(message=translate('BiblesPlugin.OsisImport',
-                                                         'The file is not a valid OSIS-XML file:'
-                                                         '\n{text}').format(text=e.msg))
-        if self.stop_import_flag:
+        self.log_debug('Starting OSIS import from "{name}"'.format(name=self.filename))
+        self.validate_xml_file(self.filename, '{http://www.bibletechnologies.net/2003/osis/namespace}osis')
+        bible = self.parse_xml(self.filename, elements=REMOVABLE_ELEMENTS, tags=REMOVABLE_TAGS)
+        if bible is None:
             return False
-        else:
-            return success
+        # Find bible language
+        language = bible.xpath("//ns:osisText/@xml:lang", namespaces=NS)
+        self.language_id = self.get_language_id(language[0] if language else None, bible_name=self.filename)
+        if not self.language_id:
+            return False
+        self.process_books(bible)
+        return True

openlp/plugins/bibles/lib/importers/zefania.py

@@ -54,7 +54,7 @@ class ZefaniaBible(BibleImport):
             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 @@ class ZefaniaBible(BibleImport):
                 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)
@@ -79,7 +79,8 @@ class ZefaniaBible(BibleImport):
                     chapter_number = CHAPTER.get("cnumber")
                     for VERS in CHAPTER:
                         verse_number = VERS.get("vnumber")
-                        self.create_verse(db_book.id, chapter_number, verse_number, VERS.text.replace('<BR/>', '\n'))
+                        self.create_verse(
+                            db_book.id, int(chapter_number), int(verse_number), VERS.text.replace('<BR/>', '\n'))
                     self.wizard.increment_progress_bar(
                         translate('BiblesPlugin.Zefnia',
                                   'Importing {book} {chapter}...').format(book=db_book.name,

openlp/plugins/bibles/lib/manager.py

@@ -23,8 +23,8 @@
 import logging
 import os
 
-from openlp.core.common import RegistryProperties, AppLocation, Settings, translate, delete_file, UiStrings
-from openlp.plugins.bibles.lib import parse_reference, LanguageSelection
+from openlp.core.common import AppLocation, OpenLPMixin, RegistryProperties, Settings, translate, delete_file, UiStrings
+from openlp.plugins.bibles.lib import LanguageSelection, parse_reference
 from openlp.plugins.bibles.lib.db import BibleDB, BibleMeta
 from .importers.csvbible import CSVBible
 from .importers.http import HTTPBible
@@ -88,7 +88,7 @@ class BibleFormat(object):
         ]
 
 
-class BibleManager(RegistryProperties):
+class BibleManager(OpenLPMixin, RegistryProperties):
     """
     The Bible manager which holds and manages all the Bibles.
     """
@@ -367,7 +367,6 @@ class BibleManager(RegistryProperties):
             second_web_bible = self.db_cache[second_bible].get_object(BibleMeta, 'download_source')
         if web_bible or second_web_bible:
             # If either Bible is Web, cursor is reset to normal and search ends w/o any message.
-            self.check_search_result()
             self.application.set_normal_cursor()
             return None
         # Fetch the results from db. If no results are found, return None, no message is given for this.

openlp/plugins/bibles/lib/mediaitem.py

@@ -254,8 +254,8 @@ class BibleMediaItem(MediaManagerItem):
         self.quickStyleComboBox.activated.connect(self.on_quick_style_combo_box_changed)
         self.advancedStyleComboBox.activated.connect(self.on_advanced_style_combo_box_changed)
         # Buttons
-        self.advancedClearButton.clicked.connect(self.on_clear_button)
-        self.quickClearButton.clicked.connect(self.on_clear_button)
+        self.advancedClearButton.clicked.connect(self.on_advanced_clear_button_clicked)
+        self.quickClearButton.clicked.connect(self.on_clear_button_clicked)
         self.advancedSearchButton.clicked.connect(self.on_advanced_search_button)
         self.quickSearchButton.clicked.connect(self.on_quick_search_button)
         # Other stuff
@@ -548,19 +548,31 @@ class BibleMediaItem(MediaManagerItem):
             self.advancedTab.setVisible(True)
             self.advanced_book_combo_box.setFocus()
 
-    def on_clear_button(self):
+    def on_clear_button_clicked(self):
         # Clear the list, then set the "No search Results" message, then clear the text field and give it focus.
         self.list_view.clear()
         self.check_search_result()
         self.quick_search_edit.clear()
         self.quick_search_edit.setFocus()
 
+    def on_advanced_clear_button_clicked(self):
+        # The same as the on_clear_button_clicked, but gives focus to Book name field in "Select" (advanced).
+        self.list_view.clear()
+        self.check_search_result()
+        self.advanced_book_combo_box.setFocus()
+
     def on_lock_button_toggled(self, checked):
-        self.quick_search_edit.setFocus()
+        """
+        Toggle the lock button, if Search tab is used, set focus to search field.
+        :param checked: The state of the toggle button. bool
+        :return: None
+        """
         if checked:
             self.sender().setIcon(self.lock_icon)
         else:
             self.sender().setIcon(self.unlock_icon)
+        if self.quickTab.isVisible():
+            self.quick_search_edit.setFocus()
 
     def on_quick_style_combo_box_changed(self):
         self.settings.layout_style = self.quickStyleComboBox.currentIndex()

openlp/plugins/bibles/lib/upgrade.py

@@ -24,8 +24,6 @@ The :mod:`upgrade` module provides a way for the database and schema that is the
 """
 import logging
 
-from sqlalchemy import delete, func, insert, select
-
 log = logging.getLogger(__name__)
 __version__ = 1
 
@@ -35,166 +33,6 @@ def upgrade_1(session, metadata):
     """
     Version 1 upgrade.
 
-    This upgrade renames a number of keys to a single naming convention.
+    This upgrade renamed a number of keys to a single naming convention.
     """
-    metadata_table = metadata.tables['metadata']
-    # Copy "Version" to "name" ("version" used by upgrade system)
-    try:
-        session.execute(insert(metadata_table).values(
-            key='name',
-            value=select(
-                [metadata_table.c.value],
-                metadata_table.c.key == 'Version'
-            ).as_scalar()
-        ))
-        session.execute(delete(metadata_table).where(metadata_table.c.key == 'Version'))
-    except:
-        log.exception('Exception when upgrading Version')
-    # Copy "Copyright" to "copyright"
-    try:
-        session.execute(insert(metadata_table).values(
-            key='copyright',
-            value=select(
-                [metadata_table.c.value],
-                metadata_table.c.key == 'Copyright'
-            ).as_scalar()
-        ))
-        session.execute(delete(metadata_table).where(metadata_table.c.key == 'Copyright'))
-    except:
-        log.exception('Exception when upgrading Copyright')
-    # Copy "Permissions" to "permissions"
-    try:
-        session.execute(insert(metadata_table).values(
-            key='permissions',
-            value=select(
-                [metadata_table.c.value],
-                metadata_table.c.key == 'Permissions'
-            ).as_scalar()
-        ))
-        session.execute(delete(metadata_table).where(metadata_table.c.key == 'Permissions'))
-    except:
-        log.exception('Exception when upgrading Permissions')
-    # Copy "Bookname language" to "book_name_language"
-    try:
-        value_count = session.execute(
-            select(
-                [func.count(metadata_table.c.value)],
-                metadata_table.c.key == 'Bookname language'
-            )
-        ).scalar()
-        if value_count > 0:
-            session.execute(insert(metadata_table).values(
-                key='book_name_language',
-                value=select(
-                    [metadata_table.c.value],
-                    metadata_table.c.key == 'Bookname language'
-                ).as_scalar()
-            ))
-            session.execute(delete(metadata_table).where(metadata_table.c.key == 'Bookname language'))
-    except:
-        log.exception('Exception when upgrading Bookname language')
-    # Copy "download source" to "download_source"
-    try:
-        value_count = session.execute(
-            select(
-                [func.count(metadata_table.c.value)],
-                metadata_table.c.key == 'download source'
-            )
-        ).scalar()
-        log.debug('download source: {count}'.format(count=value_count))
-        if value_count > 0:
-            session.execute(insert(metadata_table).values(
-                key='download_source',
-                value=select(
-                    [metadata_table.c.value],
-                    metadata_table.c.key == 'download source'
-                ).as_scalar()
-            ))
-            session.execute(delete(metadata_table).where(metadata_table.c.key == 'download source'))
-    except:
-        log.exception('Exception when upgrading download source')
-    # Copy "download name" to "download_name"
-    try:
-        value_count = session.execute(
-            select(
-                [func.count(metadata_table.c.value)],
-                metadata_table.c.key == 'download name'
-            )
-        ).scalar()
-        log.debug('download name: {count}'.format(count=value_count))
-        if value_count > 0:
-            session.execute(insert(metadata_table).values(
-                key='download_name',
-                value=select(
-                    [metadata_table.c.value],
-                    metadata_table.c.key == 'download name'
-                ).as_scalar()
-            ))
-            session.execute(delete(metadata_table).where(metadata_table.c.key == 'download name'))
-    except:
-        log.exception('Exception when upgrading download name')
-    # Copy "proxy server" to "proxy_server"
-    try:
-        value_count = session.execute(
-            select(
-                [func.count(metadata_table.c.value)],
-                metadata_table.c.key == 'proxy server'
-            )
-        ).scalar()
-        log.debug('proxy server: {count}'.format(count=value_count))
-        if value_count > 0:
-            session.execute(insert(metadata_table).values(
-                key='proxy_server',
-                value=select(
-                    [metadata_table.c.value],
-                    metadata_table.c.key == 'proxy server'
-                ).as_scalar()
-            ))
-            session.execute(delete(metadata_table).where(metadata_table.c.key == 'proxy server'))
-    except:
-        log.exception('Exception when upgrading proxy server')
-    # Copy "proxy username" to "proxy_username"
-    try:
-        value_count = session.execute(
-            select(
-                [func.count(metadata_table.c.value)],
-                metadata_table.c.key == 'proxy username'
-            )
-        ).scalar()
-        log.debug('proxy username: {count}'.format(count=value_count))
-        if value_count > 0:
-            session.execute(insert(metadata_table).values(
-                key='proxy_username',
-                value=select(
-                    [metadata_table.c.value],
-                    metadata_table.c.key == 'proxy username'
-                ).as_scalar()
-            ))
-            session.execute(delete(metadata_table).where(metadata_table.c.key == 'proxy username'))
-    except:
-        log.exception('Exception when upgrading proxy username')
-    # Copy "proxy password" to "proxy_password"
-    try:
-        value_count = session.execute(
-            select(
-                [func.count(metadata_table.c.value)],
-                metadata_table.c.key == 'proxy password'
-            )
-        ).scalar()
-        log.debug('proxy password: {count}'.format(count=value_count))
-        if value_count > 0:
-            session.execute(insert(metadata_table).values(
-                key='proxy_password',
-                value=select(
-                    [metadata_table.c.value],
-                    metadata_table.c.key == 'proxy password'
-                ).as_scalar()
-            ))
-            session.execute(delete(metadata_table).where(metadata_table.c.key == 'proxy password'))
-    except:
-        log.exception('Exception when upgrading proxy password')
-    try:
-        session.execute(delete(metadata_table).where(metadata_table.c.key == 'dbversion'))
-    except:
-        log.exception('Exception when deleting dbversion')
-    session.commit()
+    log.info('No upgrades to perform')

openlp/plugins/songs/lib/importers/mediashout.py

@@ -101,7 +101,7 @@ class MediaShoutImport(SongImport):
             self.song_book_name = song.SongID
         for verse in verses:
             tag = VERSE_TAGS[verse.Type] + str(verse.Number) if verse.Type < len(VERSE_TAGS) else 'O'
-            self.add_verse(verse.Text, tag)
+            self.add_verse(self.tidy_text(verse.Text), tag)
         for order in verse_order:
             if order.Type < len(VERSE_TAGS):
                 self.verse_order_list.append(VERSE_TAGS[order.Type] + str(order.Number))

openlp/plugins/songs/lib/importers/songimport.py

@@ -140,10 +140,13 @@ class SongImport(QtCore.QObject):
         text = text.replace('\u2026', '...')
         text = text.replace('\u2013', '-')
         text = text.replace('\u2014', '-')
+        # Replace vertical tab with 2 linebreaks
+        text = text.replace('\v', '\n\n')
+        # Replace form feed (page break) with 2 linebreaks
+        text = text.replace('\f', '\n\n')
         # Remove surplus blank lines, spaces, trailing/leading spaces
-        text = re.sub(r'[ \t\v]+', ' ', text)
+        text = re.sub(r'[ \t]+', ' ', text)
         text = re.sub(r' ?(\r\n?|\n) ?', '\n', text)
-        text = re.sub(r' ?(\n{5}|\f)+ ?', '\f', text)
         return text
 
     def process_song_text(self, text):

resources/images/openlp-2.qrc

@@ -130,7 +130,6 @@
     <file>clear_shortcut.png</file>
     <file>system_about.png</file>
     <file>system_help_contents.png</file>
-    <file>system_online_help.png</file>
     <file>system_mediamanager.png</file>
     <file>system_volunteer.png</file>
     <file>system_servicemanager.png</file>

tests/functional/openlp_core_ui/test_exceptionform.py

@@ -24,18 +24,13 @@ Package to test the openlp.core.ui.exeptionform package.
 """
 
 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

tests/functional/openlp_core_ui/test_slidecontroller.py

@@ -208,29 +208,6 @@ class TestSlideController(TestCase):
         mocked_on_theme_display.assert_called_once_with(False)
         mocked_on_hide_display.assert_called_once_with(False)
 
-    def test_live_escape(self):
-        """
-        Test that when the live_escape() method is called, the display is set to invisible and any media is stopped
-        """
-        # GIVEN: A new SlideController instance and mocked out display and media_controller
-        mocked_display = MagicMock()
-        mocked_media_controller = MagicMock()
-        Registry.create()
-        Registry().register('media_controller', mocked_media_controller)
-        slide_controller = SlideController(None)
-        slide_controller.display = mocked_display
-        play_slides = MagicMock()
-        play_slides.isChecked.return_value = False
-        slide_controller.play_slides_loop = play_slides
-        slide_controller.play_slides_once = play_slides
-
-        # WHEN: live_escape() is called
-        slide_controller.live_escape()
-
-        # THEN: the display should be set to invisible and the media controller stopped
-        mocked_display.setVisible.assert_called_once_with(False)
-        mocked_media_controller.media_stop.assert_called_once_with(slide_controller)
-
     def test_on_go_live_live_controller(self):
         """
         Test that when the on_go_live() method is called the message is sent to the live controller and focus is
@@ -697,7 +674,7 @@ class TestSlideController(TestCase):
         slide_controller.next_item = MagicMock()
         slide_controller.previous_service = MagicMock()
         slide_controller.next_service = MagicMock()
-        slide_controller.escape_item = MagicMock()
+        slide_controller.desktop_screen_enable = MagicMock()
         slide_controller.desktop_screen = MagicMock()
         slide_controller.blank_screen = MagicMock()
         slide_controller.theme_screen = MagicMock()
@@ -709,7 +686,7 @@ class TestSlideController(TestCase):
         mocked_widget.addActions.assert_called_with([
             slide_controller.previous_item, slide_controller.next_item,
             slide_controller.previous_service, slide_controller.next_service,
-            slide_controller.escape_item, slide_controller.desktop_screen,
+            slide_controller.desktop_screen_enable, slide_controller.desktop_screen,
             slide_controller.theme_screen, slide_controller.blank_screen
         ])
 

tests/functional/openlp_core_ui_media/test_systemplayer.py

@@ -107,6 +107,22 @@ class TestSystemPlayer(TestCase):
         mocked_video_widget.hide.assert_called_once_with()
         self.assertTrue(player.has_own_widget)
 
+    def test_disconnect_slots(self):
+        """
+        Test that we the disconnect slots method catches the TypeError
+        """
+        # GIVEN: A SystemPlayer class and a signal that throws a TypeError
+        player = SystemPlayer(self)
+        mocked_signal = MagicMock()
+        mocked_signal.disconnect.side_effect = \
+            TypeError('disconnect() failed between \'durationChanged\' and all its connections')
+
+        # WHEN: disconnect_slots() is called
+        player.disconnect_slots(mocked_signal)
+
+        # THEN: disconnect should have been called and the exception should have been ignored
+        mocked_signal.disconnect.assert_called_once_with()
+
     def test_check_available(self):
         """
         Test the check_available() method on SystemPlayer
@@ -198,7 +214,8 @@ class TestSystemPlayer(TestCase):
         with patch.object(player, 'get_live_state') as mocked_get_live_state, \
                 patch.object(player, 'seek') as mocked_seek, \
                 patch.object(player, 'volume') as mocked_volume, \
-                patch.object(player, 'set_state') as mocked_set_state:
+                patch.object(player, 'set_state') as mocked_set_state, \
+                patch.object(player, 'disconnect_slots') as mocked_disconnect_slots:
             mocked_get_live_state.return_value = QtMultimedia.QMediaPlayer.PlayingState
             result = player.play(mocked_display)
 
@@ -207,6 +224,7 @@ class TestSystemPlayer(TestCase):
         mocked_display.media_player.play.assert_called_once_with()
         mocked_seek.assert_called_once_with(mocked_display, 1000)
         mocked_volume.assert_called_once_with(mocked_display, 1)
+        mocked_disconnect_slots.assert_called_once_with(mocked_display.media_player.durationChanged)
         mocked_display.media_player.durationChanged.connect.assert_called_once_with('function')
         mocked_set_state.assert_called_once_with(MediaState.Playing, mocked_display)
         mocked_display.video_widget.raise_.assert_called_once_with()

tests/functional/openlp_plugins/bibles/test_bibleimport.py

@@ -27,9 +27,12 @@ from io import BytesIO
 from lxml import etree, objectify
 
 from unittest import TestCase
+from PyQt5.QtWidgets import QDialog
 
 from openlp.core.common.languages import Language
+from openlp.core.lib.exceptions import ValidationError
 from openlp.plugins.bibles.lib.bibleimport import BibleImport
+from openlp.plugins.bibles.lib.db import BibleDB
 from tests.functional import MagicMock, patch
 
 
@@ -39,22 +42,103 @@ class TestBibleImport(TestCase):
     """
 
     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>')
-        self.file_patcher = patch('builtins.open', return_value=test_file)
-        self.log_patcher = patch('openlp.plugins.bibles.lib.bibleimport.log')
+        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>'
+        )
+        self.open_patcher = patch('builtins.open')
+        self.addCleanup(self.open_patcher.stop)
+        self.mocked_open = self.open_patcher.start()
+        self.critical_error_message_box_patcher = \
+            patch('openlp.plugins.bibles.lib.bibleimport.critical_error_message_box')
+        self.addCleanup(self.critical_error_message_box_patcher.stop)
+        self.mocked_critical_error_message_box = self.critical_error_message_box_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()
+        self.translate_patcher = patch('openlp.plugins.bibles.lib.bibleimport.translate',
+                                       side_effect=lambda module, string_to_translate, *args: string_to_translate)
+        self.addCleanup(self.translate_patcher.stop)
+        self.mocked_translate = self.translate_patcher.start()
+        self.registry_patcher = patch('openlp.plugins.bibles.lib.bibleimport.Registry')
+        self.addCleanup(self.registry_patcher.stop)
+        self.registry_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_canceled_test(self):
+        """
+        Test the BibleImport.get_language method when the user rejects the dialog box
+        """
+        # GIVEN: A mocked LanguageForm with an exec method which returns QtDialog.Rejected and an instance of BibleDB
+        with patch.object(BibleDB, '_setup'), patch('openlp.plugins.bibles.forms.LanguageForm') as mocked_language_form:
+
+            # The integer value of QtDialog.Rejected is 0. Using the enumeration causes a seg fault for some reason
+            mocked_language_form_instance = MagicMock(**{'exec.return_value': 0})
+            mocked_language_form.return_value = mocked_language_form_instance
+            instance = BibleImport(MagicMock())
+            mocked_wizard = MagicMock()
+            instance.wizard = mocked_wizard
+
+            # WHEN: Calling get_language()
+            result = instance.get_language()
+
+            # THEN: get_language() should return False
+            mocked_language_form.assert_called_once_with(mocked_wizard)
+            mocked_language_form_instance.exec.assert_called_once_with(None)
+            self.assertFalse(result, 'get_language() should return False if the user rejects the dialog box')
+
+    def get_language_accepted_test(self):
+        """
+        Test the BibleImport.get_language method when the user accepts the dialog box
+        """
+        # GIVEN: A mocked LanguageForm with an exec method which returns QtDialog.Accepted an instance of BibleDB and
+        #       a combobox with the selected item data as 10
+        with patch.object(BibleDB, 'save_meta'), patch.object(BibleDB, '_setup'), \
+                patch('openlp.plugins.bibles.forms.LanguageForm') as mocked_language_form:
+
+            # The integer value of QtDialog.Accepted is 1. Using the enumeration causes a seg fault for some reason
+            mocked_language_form_instance = MagicMock(**{'exec.return_value': 1,
+                                                         'language_combo_box.itemData.return_value': 10})
+            mocked_language_form.return_value = mocked_language_form_instance
+            instance = BibleImport(MagicMock())
+            mocked_wizard = MagicMock()
+            instance.wizard = mocked_wizard
+
+            # WHEN: Calling get_language()
+            result = instance.get_language('Bible Name')
+
+            # THEN: get_language() should return the id of the selected language in the combo box
+            mocked_language_form.assert_called_once_with(mocked_wizard)
+            mocked_language_form_instance.exec.assert_called_once_with('Bible Name')
+            self.assertEqual(result, 10, 'get_language() should return the id of the language the user has chosen when '
+                                         'they accept the dialog box')
 
     def get_language_id_language_found_test(self):
         """
@@ -63,7 +147,7 @@ class TestBibleImport(TestCase):
         # GIVEN: A mocked languages.get_language which returns language and an instance of BibleImport
         with patch('openlp.core.common.languages.get_language', return_value=Language(30, 'English', 'en')) \
                 as mocked_languages_get_language, \
-                patch('openlp.plugins.bibles.lib.db.BibleDB.get_language') as mocked_db_get_language:
+                patch.object(BibleImport, 'get_language') as mocked_db_get_language:
             instance = BibleImport(MagicMock())
             instance.save_meta = MagicMock()
 
@@ -81,9 +165,8 @@ class TestBibleImport(TestCase):
         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, \
-                patch('openlp.plugins.bibles.lib.db.BibleDB.get_language', return_value=20) as mocked_db_get_language:
+        with patch('openlp.core.common.languages.get_language', return_value=None) as mocked_languages_get_language, \
+                patch.object(BibleImport, 'get_language', return_value=20) as mocked_db_get_language:
             instance = BibleImport(MagicMock())
             instance.save_meta = MagicMock()
 
@@ -103,8 +186,8 @@ class TestBibleImport(TestCase):
         # GIVEN: A mocked languages.get_language which returns None a mocked BibleDB.get_language which returns a
         #       language id.
         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=40) as mocked_db_get_language:
-            self.mock_log.error.reset_mock()
+                patch.object(BibleImport, 'get_language', return_value=40) as mocked_db_get_language, \
+                patch.object(BibleImport, 'log_error') as mocked_log_error:
             instance = BibleImport(MagicMock())
             instance.save_meta = MagicMock()
 
@@ -114,7 +197,7 @@ class TestBibleImport(TestCase):
             # THEN: The id of the language returned from BibleDB.get_language should be returned
             mocked_languages_get_language.assert_called_once_with('English')
             mocked_db_get_language.assert_called_once_with('KJV')
-            self.assertFalse(self.mock_log.error.called)
+            self.assertFalse(mocked_log_error.error.called)
             instance.save_meta.assert_called_once_with('language_id', 40)
             self.assertEqual(result, 40)
 
@@ -125,8 +208,8 @@ class TestBibleImport(TestCase):
         # GIVEN: A mocked languages.get_language which returns None a mocked BibleDB.get_language which returns a
         #       language id.
         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=None) as mocked_db_get_language:
-            self.mock_log.error.reset_mock()
+                patch.object(BibleImport, 'get_language', return_value=None) as mocked_db_get_language, \
+                patch.object(BibleImport, 'log_error') as mocked_log_error:
             instance = BibleImport(MagicMock())
             instance.save_meta = MagicMock()
 
@@ -136,18 +219,148 @@ class TestBibleImport(TestCase):
             # THEN: None should be returned and an error should be logged
             mocked_languages_get_language.assert_called_once_with('Qwerty')
             mocked_db_get_language.assert_called_once_with('KJV')
-            self.mock_log.error.assert_called_once_with('Language detection failed when importing from "KJV". '
-                                                        'User aborted language selection.')
+            mocked_log_error.assert_called_once_with(
+                'Language detection failed when importing from "KJV". User aborted language selection.')
             self.assertFalse(instance.save_meta.called)
             self.assertIsNone(result)
 
+    def get_book_ref_id_by_name_get_book_test(self):
+        """
+        Test get_book_ref_id_by_name when the book is found as a book in BiblesResourcesDB
+        """
+        # GIVEN: An instance of BibleImport and a mocked BiblesResourcesDB which returns a book id when get_book is
+        #        called
+        with patch.object(BibleImport, 'log_debug'), \
+                patch('openlp.plugins.bibles.lib.bibleimport.BiblesResourcesDB',
+                      **{'get_book.return_value': {'id': 20}}):
+            instance = BibleImport(MagicMock())
+
+            # WHEN: Calling get_book_ref_id_by_name
+            result = instance.get_book_ref_id_by_name('Gen', 66, 4)
+
+            # THEN: The bible id should be returned
+            self.assertEqual(result, 20)
+
+    def get_book_ref_id_by_name_get_alternative_book_name_test(self):
+        """
+        Test get_book_ref_id_by_name when the book is found as an alternative book in BiblesResourcesDB
+        """
+        # GIVEN: An instance of BibleImport and a mocked BiblesResourcesDB which returns a book id when
+        #        get_alternative_book_name is called
+        with patch.object(BibleImport, 'log_debug'), \
+                patch('openlp.plugins.bibles.lib.bibleimport.BiblesResourcesDB',
+                      **{'get_book.return_value': None, 'get_alternative_book_name.return_value': 30}):
+            instance = BibleImport(MagicMock())
+
+            # WHEN: Calling get_book_ref_id_by_name
+            result = instance.get_book_ref_id_by_name('Gen', 66, 4)
+
+            # THEN: The bible id should be returned
+            self.assertEqual(result, 30)
+
+    def get_book_ref_id_by_name_get_book_reference_id_test(self):
+        """
+        Test get_book_ref_id_by_name when the book is found as a book in AlternativeBookNamesDB
+        """
+        # GIVEN: An instance of BibleImport and a mocked AlternativeBookNamesDB which returns a book id when
+        #        get_book_reference_id is called
+        with patch.object(BibleImport, 'log_debug'), \
+                patch('openlp.plugins.bibles.lib.bibleimport.BiblesResourcesDB',
+                      **{'get_book.return_value': None, 'get_alternative_book_name.return_value': None}), \
+                patch('openlp.plugins.bibles.lib.bibleimport.AlternativeBookNamesDB',
+                      **{'get_book_reference_id.return_value': 40}):
+            instance = BibleImport(MagicMock())
+
+            # WHEN: Calling get_book_ref_id_by_name
+            result = instance.get_book_ref_id_by_name('Gen', 66, 4)
+
+            # THEN: The bible id should be returned
+            self.assertEqual(result, 40)
+
+    def get_book_ref_id_by_name_book_name_form_rejected_test(self):
+        """
+        Test get_book_ref_id_by_name when the user rejects the BookNameForm
+        """
+        # GIVEN: An instance of BibleImport and a mocked BookNameForm which simulates a user rejecting the dialog
+        with patch.object(BibleImport, 'log_debug'), patch.object(BibleImport, 'get_books'), \
+                patch('openlp.plugins.bibles.lib.bibleimport.BiblesResourcesDB',
+                      **{'get_book.return_value': None, 'get_alternative_book_name.return_value': None}), \
+                patch('openlp.plugins.bibles.lib.bibleimport.AlternativeBookNamesDB',
+                      **{'get_book_reference_id.return_value': None}), \
+                patch('openlp.plugins.bibles.forms.BookNameForm',
+                      return_value=MagicMock(**{'exec.return_value': QDialog.Rejected})):
+            instance = BibleImport(MagicMock())
+
+            # WHEN: Calling get_book_ref_id_by_name
+            result = instance.get_book_ref_id_by_name('Gen', 66, 4)
+
+            # THEN: None should be returned
+            self.assertIsNone(result)
+
+    def get_book_ref_id_by_name_book_name_form_accepted_test(self):
+        """
+        Test get_book_ref_id_by_name when the user accepts the BookNameForm
+        """
+        # GIVEN: An instance of BibleImport and a mocked BookNameForm which simulates a user accepting the dialog
+        with patch.object(BibleImport, 'log_debug'), patch.object(BibleImport, 'get_books'), \
+                patch('openlp.plugins.bibles.lib.bibleimport.BiblesResourcesDB',
+                      **{'get_book.return_value': None, 'get_alternative_book_name.return_value': None}), \
+                patch('openlp.plugins.bibles.lib.bibleimport.AlternativeBookNamesDB',
+                      **{'get_book_reference_id.return_value': None}) as mocked_alternative_book_names_db, \
+                patch('openlp.plugins.bibles.forms.BookNameForm',
+                      return_value=MagicMock(**{'exec.return_value': QDialog.Accepted, 'book_id': 50})):
+            instance = BibleImport(MagicMock())
+
+            # WHEN: Calling get_book_ref_id_by_name
+            result = instance.get_book_ref_id_by_name('Gen', 66, 4)
+
+            # THEN: An alternative book name should be created and a bible id should be returned
+            mocked_alternative_book_names_db.create_alternative_book_name.assert_called_once_with('Gen', 50, 4)
+            self.assertEqual(result, 50)
+
+    def is_compressed_compressed_test(self):
+        """
+        Test is_compressed when the 'file' being tested is compressed
+        """
+        # GIVEN: An instance of BibleImport and a mocked is_zipfile which returns True
+        with patch('openlp.plugins.bibles.lib.bibleimport.is_zipfile', return_value=True):
+            instance = BibleImport(MagicMock())
+
+            # WHEN: Calling is_compressed
+            result = instance.is_compressed('file.ext')
+
+            # THEN: Then critical_error_message_box should be called informing the user that the file is compressed and
+            #       True should be returned
+            self.mocked_critical_error_message_box.assert_called_once_with(
+                message='The file "file.ext" you supplied is compressed. You must decompress it before import.')
+            self.assertTrue(result)
+
+    def is_compressed_not_compressed_test(self):
+        """
+        Test is_compressed when the 'file' being tested is not compressed
+        """
+        # GIVEN: An instance of BibleImport and a mocked is_zipfile which returns False
+        with patch('openlp.plugins.bibles.lib.bibleimport.is_zipfile', return_value=False):
+            instance = BibleImport(MagicMock())
+
+            # WHEN: Calling is_compressed
+            result = instance.is_compressed('file.ext')
+
+            # THEN: False should be returned and critical_error_message_box should not have been called
+            self.assertFalse(result)
+            self.assertFalse(self.mocked_critical_error_message_box.called)
+
     def parse_xml_etree_test(self):
         """
         Test BibleImport.parse_xml() when called with the use_objectify default value
         """
-        # GIVEN: A sample "file" to parse
+        # GIVEN: A sample "file" to parse and an instance of BibleImport
+        self.mocked_open.return_value = self.test_file
+        instance = BibleImport(MagicMock())
+        instance.wizard = MagicMock()
+
         # WHEN: Calling parse_xml
-        result = BibleImport.parse_xml('file.tst')
+        result = instance.parse_xml('file.tst')
 
         # THEN: The result returned should contain the correct data, and should be an instance of eetree_Element
         self.assertEqual(etree.tostring(result),
@@ -159,9 +372,13 @@ class TestBibleImport(TestCase):
         """
         Test BibleImport.parse_xml() when called with use_objectify set to True
         """
-        # GIVEN: A sample "file" to parse
+        # GIVEN: A sample "file" to parse and an instance of BibleImport
+        self.mocked_open.return_value = self.test_file
+        instance = BibleImport(MagicMock())
+        instance.wizard = MagicMock()
+
         # WHEN: Calling parse_xml
-        result = BibleImport.parse_xml('file.tst', use_objectify=True)
+        result = instance.parse_xml('file.tst', use_objectify=True)
 
         # THEN: The result returned should contain the correct data, and should be an instance of ObjectifiedElement
         self.assertEqual(etree.tostring(result),
@@ -173,11 +390,14 @@ class TestBibleImport(TestCase):
         """
         Test BibleImport.parse_xml() when given a tuple of elements to remove
         """
-        # GIVEN: A tuple of elements to remove
+        # GIVEN: A tuple of elements to remove and an instance of BibleImport
+        self.mocked_open.return_value = self.test_file
         elements = ('unsupported', 'x', 'y')
+        instance = BibleImport(MagicMock())
+        instance.wizard = MagicMock()
 
         # WHEN: Calling parse_xml, with a test file
-        result = BibleImport.parse_xml('file.tst', elements=elements)
+        result = instance.parse_xml('file.tst', elements=elements)
 
         # THEN: The result returned should contain the correct data
         self.assertEqual(etree.tostring(result),
@@ -187,11 +407,14 @@ class TestBibleImport(TestCase):
         """
         Test BibleImport.parse_xml() when given a tuple of tags to remove
         """
-        # GIVEN: A tuple of tags to remove
+        # GIVEN: A tuple of tags to remove and an instance of BibleImport
+        self.mocked_open.return_value = self.test_file
         tags = ('div', 'p', 'a')
+        instance = BibleImport(MagicMock())
+        instance.wizard = MagicMock()
 
         # WHEN: Calling parse_xml, with a test file
-        result = BibleImport.parse_xml('file.tst', tags=tags)
+        result = instance.parse_xml('file.tst', tags=tags)
 
         # THEN: The result returned should contain the correct data
         self.assertEqual(etree.tostring(result), b'<root>\n    <data>Testdatatokeep</data>\n    <data><unsupported>Test'
@@ -201,12 +424,192 @@ class TestBibleImport(TestCase):
         """
         Test BibleImport.parse_xml() when given a tuple of elements and of tags to remove
         """
-        # GIVEN: A tuple of elements and of tags to remove
+        # GIVEN: A tuple of elements and of tags to remove and an instacne of BibleImport
+        self.mocked_open.return_value = self.test_file
         elements = ('unsupported', 'x', 'y')
         tags = ('div', 'p', 'a')
+        instance = BibleImport(MagicMock())
+        instance.wizard = MagicMock()
 
         # WHEN: Calling parse_xml, with a test file
-        result = BibleImport.parse_xml('file.tst', elements=elements, tags=tags)
+        result = instance.parse_xml('file.tst', elements=elements, tags=tags)
 
         # THEN: The result returned should contain the correct data
         self.assertEqual(etree.tostring(result), b'<root>\n    <data>Testdatatokeep</data>\n    <data/>\n</root>')
+
+    def parse_xml_file_file_not_found_exception_test(self):
+        """
+        Test that parse_xml handles a FileNotFoundError exception correctly
+        """
+        with patch.object(BibleImport, 'log_exception') as mocked_log_exception:
+            # GIVEN: A mocked open which raises a FileNotFoundError and an instance of BibleImporter
+            exception = FileNotFoundError()
+            exception.filename = 'file.tst'
+            exception.strerror = 'No such file or directory'
+            self.mocked_open.side_effect = exception
+            importer = BibleImport(MagicMock(), path='.', name='.', filename='')
+
+            # WHEN: Calling parse_xml
+            result = importer.parse_xml('file.tst')
+
+            # THEN: parse_xml should have caught the error, informed the user and returned None
+            mocked_log_exception.assert_called_once_with('Opening file.tst failed.')
+            self.mocked_critical_error_message_box.assert_called_once_with(
+                title='An Error Occured When Opening A File',
+                message='The following error occurred when trying to open\nfile.tst:\n\nNo such file or directory')
+            self.assertIsNone(result)
+
+    def parse_xml_file_permission_error_exception_test(self):
+        """
+        Test that parse_xml handles a PermissionError exception correctly
+        """
+        with patch.object(BibleImport, 'log_exception') as mocked_log_exception:
+            # GIVEN: A mocked open which raises a PermissionError and an instance of BibleImporter
+            exception = PermissionError()
+            exception.filename = 'file.tst'
+            exception.strerror = 'Permission denied'
+            self.mocked_open.side_effect = exception
+            importer = BibleImport(MagicMock(), path='.', name='.', filename='')
+
+            # WHEN: Calling parse_xml
+            result = importer.parse_xml('file.tst')
+
+            # THEN: parse_xml should have caught the error, informed the user and returned None
+            mocked_log_exception.assert_called_once_with('Opening file.tst failed.')
+            self.mocked_critical_error_message_box.assert_called_once_with(
+                title='An Error Occured When Opening A File',
+                message='The following error occurred when trying to open\nfile.tst:\n\nPermission denied')
+            self.assertIsNone(result)
+
+    def set_current_chapter_test(self):
+        """
+        Test set_current_chapter
+        """
+        # GIVEN: An instance of BibleImport and a mocked wizard
+        importer = BibleImport(MagicMock(), path='.', name='.', filename='')
+        importer.wizard = MagicMock()
+
+        # WHEN: Calling set_current_chapter
+        importer.set_current_chapter('Book_Name', 'Chapter')
+
+        # THEN: Increment_progress_bar should have been called with a text string
+        importer.wizard.increment_progress_bar.assert_called_once_with('Importing Book_Name Chapter...')
+
+    def validate_xml_file_compressed_file_test(self):
+        """
+        Test that validate_xml_file raises a ValidationError when is_compressed returns True
+        """
+        # GIVEN: A mocked parse_xml which returns None
+        with patch.object(BibleImport, 'is_compressed', return_value=True):
+            importer = BibleImport(MagicMock(), path='.', name='.', filename='')
+
+            # WHEN: Calling is_compressed
+            # THEN: ValidationError should be raised, with the message 'Compressed file'
+            with self.assertRaises(ValidationError) as context:
+                importer.validate_xml_file('file.name', 'xbible')
+            self.assertEqual(context.exception.msg, 'Compressed file')
+
+    def validate_xml_file_parse_xml_fails_test(self):
+        """
+        Test that validate_xml_file raises a ValidationError when parse_xml returns None
+        """
+        # GIVEN: A mocked parse_xml which returns None
+        with patch.object(BibleImport, 'parse_xml', return_value=None), \
+                patch.object(BibleImport, 'is_compressed', return_value=False):
+            importer = BibleImport(MagicMock(), path='.', name='.', filename='')
+
+            # WHEN: Calling validate_xml_file
+            # THEN: ValidationError should be raised, with the message 'Error when opening file'
+            #       the user that an OpenSong bible was found
+            with self.assertRaises(ValidationError) as context:
+                importer.validate_xml_file('file.name', 'xbible')
+            self.assertEqual(context.exception.msg, 'Error when opening file')
+
+    def validate_xml_file_success_test(self):
+        """
+        Test that validate_xml_file returns True with valid XML
+        """
+        # GIVEN: Some test data with an OpenSong Bible "bible" root tag
+        with patch.object(BibleImport, 'parse_xml', return_value=objectify.fromstring('<bible></bible>')), \
+                patch.object(BibleImport, 'is_compressed', return_value=False):
+            importer = BibleImport(MagicMock(), path='.', name='.', filename='')
+
+            # WHEN: Calling validate_xml_file
+            result = importer.validate_xml_file('file.name', 'bible')
+
+            # THEN: True should be returned
+            self.assertTrue(result)
+
+    def validate_xml_file_opensong_root_test(self):
+        """
+        Test that validate_xml_file raises a ValidationError with an OpenSong root tag
+        """
+        # GIVEN: Some test data with an Zefania root tag and an instance of BibleImport
+        with patch.object(BibleImport, 'parse_xml', return_value=objectify.fromstring('<bible></bible>')), \
+                patch.object(BibleImport, 'is_compressed', return_value=False):
+            importer = BibleImport(MagicMock(), path='.', name='.', filename='')
+
+            # WHEN: Calling validate_xml_file
+            # THEN: ValidationError should be raised, and the critical error message box should was called informing
+            #       the user that an OpenSong bible was found
+            with self.assertRaises(ValidationError) as context:
+                importer.validate_xml_file('file.name', 'xbible')
+            self.assertEqual(context.exception.msg, 'Invalid xml.')
+            self.mocked_critical_error_message_box.assert_called_once_with(
+                message='Incorrect Bible file type supplied. This looks like an OpenSong XML bible.')
+
+    def validate_xml_file_osis_root_test(self):
+        """
+        Test that validate_xml_file raises a ValidationError with an OSIS root tag
+        """
+        # GIVEN: Some test data with an Zefania root tag and an instance of BibleImport
+        with patch.object(BibleImport, 'parse_xml', return_value=objectify.fromstring(
+                '<osis xmlns=\'http://www.bibletechnologies.net/2003/OSIS/namespace\'></osis>')), \
+                patch.object(BibleImport, 'is_compressed', return_value=False):
+            importer = BibleImport(MagicMock(), path='.', name='.', filename='')
+
+            # WHEN: Calling validate_xml_file
+            # THEN: ValidationError should be raised, and the critical error message box should was called informing
+            #       the user that an OSIS bible was found
+            with self.assertRaises(ValidationError) as context:
+                importer.validate_xml_file('file.name', 'xbible')
+            self.assertEqual(context.exception.msg, 'Invalid xml.')
+            self.mocked_critical_error_message_box.assert_called_once_with(
+                message='Incorrect Bible file type supplied. This looks like an OSIS XML bible.')
+
+    def validate_xml_file_zefania_root_test(self):
+        """
+        Test that validate_xml_file raises a ValidationError with an Zefania root tag
+        """
+        # GIVEN: Some test data with an Zefania root tag and an instance of BibleImport
+        with patch.object(BibleImport, 'parse_xml', return_value=objectify.fromstring('<xmlbible></xmlbible>')), \
+                patch.object(BibleImport, 'is_compressed', return_value=False):
+            importer = BibleImport(MagicMock(), path='.', name='.', filename='')
+
+            # WHEN: Calling validate_xml_file
+            # THEN: ValidationError should be raised, and the critical error message box should was called informing
+            #       the user that an Zefania bible was found
+            with self.assertRaises(ValidationError) as context:
+                importer.validate_xml_file('file.name', 'xbible')
+            self.assertEqual(context.exception.msg, 'Invalid xml.')
+            self.mocked_critical_error_message_box.assert_called_once_with(
+                message='Incorrect Bible file type supplied. This looks like an Zefania XML bible.')
+
+    def validate_xml_file_unknown_root_test(self):
+        """
+        Test that validate_xml_file raises a ValidationError with an unknown root tag
+        """
+        # GIVEN: Some test data with an unknown root tag and an instance of BibleImport
+        with patch.object(
+                BibleImport, 'parse_xml', return_value=objectify.fromstring('<unknownbible></unknownbible>')), \
+                patch.object(BibleImport, 'is_compressed', return_value=False):
+            importer = BibleImport(MagicMock(), path='.', name='.', filename='')
+
+            # WHEN: Calling validate_xml_file
+            # THEN: ValidationError should be raised, and the critical error message box should was called informing
+            #       the user that a unknown xml bible was found
+            with self.assertRaises(ValidationError) as context:
+                importer.validate_xml_file('file.name', 'xbible')
+            self.assertEqual(context.exception.msg, 'Invalid xml.')
+            self.mocked_critical_error_message_box.assert_called_once_with(
+                message='Incorrect Bible file type supplied. This looks like an unknown type of XML bible.')

tests/functional/openlp_plugins/bibles/test_csvimport.py

@@ -46,10 +46,10 @@ class TestCSVImport(TestCase):
 
     def setUp(self):
         self.manager_patcher = patch('openlp.plugins.bibles.lib.db.Manager')
-        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 = patch('openlp.plugins.bibles.lib.bibleimport.Registry')
+        self.addCleanup(self.registry_patcher.stop)
         self.registry_patcher.start()
 
     def test_create_importer(self):
@@ -194,9 +194,9 @@ class TestCSVImport(TestCase):
             # WHEN: Calling process_books
             result = importer.process_books(['Book 1'])
 
-            # THEN: increment_progress_bar should not be called and the return value should be None
+            # THEN: increment_progress_bar should not be called and the return value should be an empty dictionary
             self.assertFalse(importer.wizard.increment_progress_bar.called)
-            self.assertIsNone(result)
+            self.assertEqual(result, {})
 
     def process_books_test(self):
         """
@@ -207,7 +207,6 @@ class TestCSVImport(TestCase):
         with patch('openlp.plugins.bibles.lib.db.BibleDB._setup'),\
                 patch('openlp.plugins.bibles.lib.importers.csvbible.translate'):
             importer = CSVBible(mocked_manager, path='.', name='.', booksfile='books.csv', versefile='verse.csv')
-            type(importer).application = PropertyMock()
             importer.find_and_create_book = MagicMock()
             importer.language_id = 10
             importer.stop_import_flag = False
@@ -222,7 +221,6 @@ class TestCSVImport(TestCase):
             # 		The returned data should be a dictionary with both song's id and names.
             self.assertEqual(importer.find_and_create_book.mock_calls,
                              [call('1. Mosebog', 2, 10), call('2. Mosebog', 2, 10)])
-            importer.application.process_events.assert_called_once_with()
             self.assertDictEqual(result, {1: '1. Mosebog', 2: '2. Mosebog'})
 
     def process_verses_stopped_import_test(self):
@@ -233,19 +231,16 @@ class TestCSVImport(TestCase):
         mocked_manager = MagicMock()
         with patch('openlp.plugins.bibles.lib.db.BibleDB._setup'):
             importer = CSVBible(mocked_manager, path='.', name='.', booksfile='books.csv', versefile='verse.csv')
-            type(importer).application = PropertyMock()
             importer.get_book_name = MagicMock()
             importer.session = MagicMock()
             importer.stop_import_flag = True
             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)
-            importer.wizard.increment_progress_bar.assert_called_once_with('Importing verses... done.')
-            importer.application.process_events.assert_called_once_with()
             self.assertIsNone(result)
 
     def process_verses_successful_test(self):
@@ -257,7 +252,6 @@ class TestCSVImport(TestCase):
         with patch('openlp.plugins.bibles.lib.db.BibleDB._setup'),\
                 patch('openlp.plugins.bibles.lib.importers.csvbible.translate'):
             importer = CSVBible(mocked_manager, path='.', name='.', booksfile='books.csv', versefile='verse.csv')
-            type(importer).application = PropertyMock()
             importer.create_verse = MagicMock()
             importer.get_book = MagicMock(return_value=Book('1', '1', '1. Mosebog', '1Mos'))
             importer.get_book_name = MagicMock(return_value='1. Mosebog')
@@ -280,7 +274,6 @@ class TestCSVImport(TestCase):
                              [call('1', 1, 1, 'I Begyndelsen skabte Gud Himmelen og Jorden.'),
                               call('1', 1, 2, 'Og Jorden var øde og tom, og der var Mørke over Verdensdybet. '
                                               'Men Guds Ånd svævede over Vandene.')])
-            importer.application.process_events.assert_called_once_with()
 
     def do_import_invalid_language_id_test(self):
         """
@@ -288,82 +281,24 @@ class TestCSVImport(TestCase):
         """
         # GIVEN: An instance of CSVBible and a mocked get_language which simulates the user cancelling the language box
         mocked_manager = MagicMock()
-        with patch('openlp.plugins.bibles.lib.db.BibleDB._setup'),\
-                patch('openlp.plugins.bibles.lib.importers.csvbible.log') as mocked_log:
+        with patch('openlp.plugins.bibles.lib.db.BibleDB._setup'):
             importer = CSVBible(mocked_manager, path='.', name='.', booksfile='books.csv', versefile='verse.csv')
             importer.get_language = MagicMock(return_value=None)
 
             # WHEN: Calling do_import
             result = importer.do_import('Bible Name')
 
-            # THEN: The log.exception method should have been called to show that it reached the except clause.
-            # False should be returned.
+            # THEN: The False should be returned.
             importer.get_language.assert_called_once_with('Bible Name')
-            mocked_log.exception.assert_called_once_with('Could not import CSV bible')
             self.assertFalse(result)
 
-    def do_import_stop_import_test(self):
-        """
-        Test do_import when the import is stopped
-        """
-        # GIVEN: An instance of CSVBible with stop_import set to True
-        mocked_manager = MagicMock()
-        with patch('openlp.plugins.bibles.lib.db.BibleDB._setup'),\
-                patch('openlp.plugins.bibles.lib.importers.csvbible.log') as mocked_log:
-            importer = CSVBible(mocked_manager, path='.', name='.', booksfile='books.csv', versefile='verse.csv')
-            importer.get_language = MagicMock(return_value=10)
-            importer.parse_csv_file = MagicMock(return_value=['Book 1', 'Book 2', 'Book 3'])
-            importer.process_books = MagicMock()
-            importer.stop_import_flag = True
-            importer.wizard = MagicMock()
-
-            # WHEN: Calling do_import
-            result = importer.do_import('Bible Name')
-
-            # THEN: log.exception should not be called, parse_csv_file should only be called once,
-            # and False should be returned.
-            self.assertFalse(mocked_log.exception.called)
-            importer.parse_csv_file.assert_called_once_with('books.csv', Book)
-            importer.process_books.assert_called_once_with(['Book 1', 'Book 2', 'Book 3'])
-            self.assertFalse(result)
-
-    def do_import_stop_import_2_test(self):
-        """
-        Test do_import when the import is stopped
-        """
-        # GIVEN: An instance of CSVBible with stop_import which is True the second time of calling
-        mocked_manager = MagicMock()
-        with patch('openlp.plugins.bibles.lib.db.BibleDB._setup'),\
-                patch('openlp.plugins.bibles.lib.importers.csvbible.log') as mocked_log:
-            CSVBible.stop_import_flag = PropertyMock(side_effect=[False, True])
-            importer = CSVBible(mocked_manager, path='.', name='.', booksfile='books.csv', versefile='verses.csv')
-            importer.get_language = MagicMock(return_value=10)
-            importer.parse_csv_file = MagicMock(side_effect=[['Book 1'], ['Verse 1']])
-            importer.process_books = MagicMock(return_value=['Book 1'])
-            importer.process_verses = MagicMock(return_value=['Verse 1'])
-            importer.wizard = MagicMock()
-
-            # WHEN: Calling do_import
-            result = importer.do_import('Bible Name')
-
-            # THEN: log.exception should not be called, parse_csv_file should be called twice,
-            # and False should be returned.
-            self.assertFalse(mocked_log.exception.called)
-            self.assertEqual(importer.parse_csv_file.mock_calls, [call('books.csv', Book), call('verses.csv', Verse)])
-            importer.process_verses.assert_called_once_with(['Verse 1'], ['Book 1'])
-            self.assertFalse(result)
-
-            # Cleanup
-            del CSVBible.stop_import_flag
-
     def do_import_success_test(self):
         """
         Test do_import when the import succeeds
         """
         # GIVEN: An instance of CSVBible
         mocked_manager = MagicMock()
-        with patch('openlp.plugins.bibles.lib.db.BibleDB._setup'),\
-                patch('openlp.plugins.bibles.lib.importers.csvbible.log') as mocked_log:
+        with patch('openlp.plugins.bibles.lib.db.BibleDB._setup'):
             importer = CSVBible(mocked_manager, path='.', name='.', booksfile='books.csv', versefile='verses.csv')
             importer.get_language = MagicMock(return_value=10)
             importer.parse_csv_file = MagicMock(side_effect=[['Book 1'], ['Verse 1']])
@@ -376,9 +311,8 @@ class TestCSVImport(TestCase):
             # WHEN: Calling do_import
             result = importer.do_import('Bible Name')
 
-            # THEN: log.exception should not be called, parse_csv_file should be called twice,
+            # THEN: parse_csv_file should be called twice,
             # and True should be returned.
-            self.assertFalse(mocked_log.exception.called)
             self.assertEqual(importer.parse_csv_file.mock_calls, [call('books.csv', Book), call('verses.csv', Verse)])
             importer.process_books.assert_called_once_with(['Book 1'])
             importer.process_verses.assert_called_once_with(['Verse 1'], ['Book 1'])
@@ -413,6 +347,6 @@ class TestCSVImport(TestCase):
             # THEN: The create_verse() method should have been called with each verse in the file.
             self.assertTrue(importer.create_verse.called)
             for verse_tag, verse_text in test_data['verses']:
-                importer.create_verse.assert_any_call(importer.get_book().id, '1', verse_tag, verse_text)
+                importer.create_verse.assert_any_call(importer.get_book().id, 1, verse_tag, verse_text)
             importer.create_book.assert_any_call('1. Mosebog', importer.get_book_ref_id_by_name(), 1)
             importer.create_book.assert_any_call('1. Krønikebog', importer.get_book_ref_id_by_name(), 1)

tests/functional/openlp_plugins/bibles/test_db.py

@@ -25,63 +25,9 @@ This module contains tests for the db submodule of the Bibles plugin.
 
 from unittest import TestCase
 
-from openlp.plugins.bibles.lib.db import BibleDB
-from tests.functional import MagicMock, patch
-
 
 class TestBibleDB(TestCase):
     """
     Test the functions in the BibleDB class.
     """
-
-    def test_get_language_canceled(self):
-        """
-        Test the BibleDB.get_language method when the user rejects the dialog box
-        """
-        # GIVEN: A mocked LanguageForm with an exec method which returns QtDialog.Rejected and an instance of BibleDB
-        with patch('openlp.plugins.bibles.lib.db.BibleDB._setup'),\
-                patch('openlp.plugins.bibles.forms.LanguageForm') as mocked_language_form:
-
-            # The integer value of QtDialog.Rejected is 0. Using the enumeration causes a seg fault for some reason
-            mocked_language_form_instance = MagicMock(**{'exec.return_value': 0})
-            mocked_language_form.return_value = mocked_language_form_instance
-            mocked_parent = MagicMock()
-            instance = BibleDB(mocked_parent)
-            mocked_wizard = MagicMock()
-            instance.wizard = mocked_wizard
-
-            # WHEN: Calling get_language()
-            result = instance.get_language()
-
-            # THEN: get_language() should return False
-            mocked_language_form.assert_called_once_with(mocked_wizard)
-            mocked_language_form_instance.exec.assert_called_once_with(None)
-            self.assertFalse(result, 'get_language() should return False if the user rejects the dialog box')
-
-    def test_get_language_accepted(self):
-        """
-        Test the BibleDB.get_language method when the user accepts the dialog box
-        """
-        # GIVEN: A mocked LanguageForm with an exec method which returns QtDialog.Accepted an instance of BibleDB and
-        #       a combobox with the selected item data as 10
-        with patch('openlp.plugins.bibles.lib.db.BibleDB._setup'), \
-                patch('openlp.plugins.bibles.lib.db.BibleDB.save_meta'), \
-                patch('openlp.plugins.bibles.forms.LanguageForm') as mocked_language_form:
-
-            # The integer value of QtDialog.Accepted is 1. Using the enumeration causes a seg fault for some reason
-            mocked_language_form_instance = MagicMock(**{'exec.return_value': 1,
-                                                         'language_combo_box.itemData.return_value': 10})
-            mocked_language_form.return_value = mocked_language_form_instance
-            mocked_parent = MagicMock()
-            instance = BibleDB(mocked_parent)
-            mocked_wizard = MagicMock()
-            instance.wizard = mocked_wizard
-
-            # WHEN: Calling get_language()
-            result = instance.get_language('Bible Name')
-
-            # THEN: get_language() should return the id of the selected language in the combo box
-            mocked_language_form.assert_called_once_with(mocked_wizard)
-            mocked_language_form_instance.exec.assert_called_once_with('Bible Name')
-            self.assertEqual(result, 10, 'get_language() should return the id of the language the user has chosen when '
-                                         'they accept the dialog box')
+    pass

tests/functional/openlp_plugins/bibles/test_mediaitem.py

@@ -114,7 +114,28 @@ class TestMediaItem(TestCase, TestMixin):
                 self.assertEqual(self.media_item.search_results, {})
                 self.assertEqual(self.media_item.second_search_results, {})
 
-    def on_quick_search_button_general_test(self):
+    def test_required_icons(self):
+        """
+        Test that all the required icons are set properly.
+        """
+        # GIVEN: Mocked icons that need to be called.
+        self.media_item.has_import_icon = MagicMock()
+        self.media_item.has_new_icon = MagicMock()
+        self.media_item.has_edit_icon = MagicMock()
+        self.media_item.has_delete_icon = MagicMock()
+        self.media_item.add_to_service_item = MagicMock()
+
+        # WHEN: self.media_item.required_icons is called
+        self.media_item.required_icons()
+
+        # THEN: On windows it should return True, on other platforms False
+        self.assertTrue(self.media_item.has_import_icon, 'Check that the icon is as True.')
+        self.assertFalse(self.media_item.has_new_icon, 'Check that the icon is called as False.')
+        self.assertTrue(self.media_item.has_edit_icon, 'Check that the icon is called as True.')
+        self.assertTrue(self.media_item.has_delete_icon, 'Check that the icon is called as True.')
+        self.assertFalse(self.media_item.add_to_service_item, 'Check that the icon is called as False')
+
+    def test_on_quick_search_button_general(self):
         """
         Test that general things, which should be called on all Quick searches are called.
         """
@@ -150,3 +171,60 @@ class TestMediaItem(TestCase, TestMixin):
         self.assertEqual(2, self.media_item.quickSearchButton.setEnabled.call_count, 'Disable and Enable the button')
         self.assertEqual(1, self.media_item.check_search_result.call_count, 'Check results Should had been called once')
         self.assertEqual(1, self.app.set_normal_cursor.call_count, 'Normal cursor should had been called once')
+
+    def test_on_clear_button_clicked(self):
+        """
+        Test that the on_clear_button_clicked works properly. (Used by Bible search tab)
+        """
+        # GIVEN: Mocked list_view, check_search_results & quick_search_edit.
+        self.media_item.list_view = MagicMock()
+        self.media_item.check_search_result = MagicMock()
+        self.media_item.quick_search_edit = MagicMock()
+
+        # WHEN: on_clear_button_clicked is called
+        self.media_item.on_clear_button_clicked()
+
+        # THEN: Search result should be reset and search field should receive focus.
+        self.media_item.list_view.clear.assert_called_once_with(),
+        self.media_item.check_search_result.assert_called_once_with(),
+        self.media_item.quick_search_edit.clear.assert_called_once_with(),
+        self.media_item.quick_search_edit.setFocus.assert_called_once_with()
+
+    def test_on_lock_button_toggled_search_tab_lock_icon(self):
+        """
+        Test that "on_lock_button_toggled" gives focus to the right field and toggles the lock properly.
+        """
+        # GIVEN: Mocked sender & Search edit, quickTab returning value = True on isVisible.
+        self.media_item.sender = MagicMock()
+        self.media_item.quick_search_edit = MagicMock()
+        self.media_item.quickTab = MagicMock(**{'isVisible.return_value': True})
+
+        self.media_item.lock_icon = 'lock icon'
+        sender_instance_mock = MagicMock()
+        self.media_item.sender = MagicMock(return_value=sender_instance_mock)
+
+        # WHEN: on_lock_button_toggled is called and checked returns = True.
+        self.media_item.on_lock_button_toggled(True)
+
+        # THEN: on_quick_search_edit should receive focus and Lock icon should be set.
+        self.media_item.quick_search_edit.setFocus.assert_called_once_with()
+        sender_instance_mock.setIcon.assert_called_once_with('lock icon')
+
+    def test_on_lock_button_toggled_unlock_icon(self):
+        """
+         Test that lock button unlocks properly and lock toggles properly.
+        """
+        # GIVEN: Mocked sender & Search edit, quickTab returning value = False on isVisible.
+        self.media_item.sender = MagicMock()
+        self.media_item.quick_search_edit = MagicMock()
+        self.media_item.quickTab = MagicMock()
+        self.media_item.quickTab.isVisible = MagicMock()
+        self.media_item.unlock_icon = 'unlock icon'
+        sender_instance_mock = MagicMock()
+        self.media_item.sender = MagicMock(return_value=sender_instance_mock)
+
+        # WHEN: on_lock_button_toggled is called and checked returns = False.
+        self.media_item.on_lock_button_toggled(False)
+
+        # THEN: Unlock icon should be set.
+        sender_instance_mock.setIcon.assert_called_once_with('unlock icon')

tests/functional/openlp_plugins/bibles/test_opensongimport.py

@@ -23,32 +23,38 @@
 This module contains tests for the OpenSong Bible importer.
 """
 
-import os
 import json
+import os
 from unittest import TestCase
 
-from tests.functional import MagicMock, patch
-from openlp.plugins.bibles.lib.importers.opensong import OpenSongBible
-from openlp.plugins.bibles.lib.db import BibleDB
+from lxml import objectify
+
+from tests.functional import MagicMock, patch, call
+from tests.helpers.testmixin import TestMixin
+from openlp.core.common import Registry
+from openlp.plugins.bibles.lib.importers.opensong import OpenSongBible, get_text, parse_chapter_number
+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.find_and_create_book_patch = patch.object(BibleImport, 'find_and_create_book')
+        self.addCleanup(self.find_and_create_book_patch.stop)
+        self.mocked_find_and_create_book = self.find_and_create_book_patch.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 +67,332 @@ class TestOpenSongImport(TestCase):
         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 = 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 = 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 = 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 = 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: An instance of OpenSongBible, the number 15 represented as a string and an instance of OpenSongBible
+        importer = OpenSongBible(MagicMock(), path='.', name='.', filename='')
+
+        # WHEN: Calling parse_verse_number
+        result = importer.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: An instance of OpenSongBible, and the range 24-26 represented as a string
+        importer = OpenSongBible(MagicMock(), path='.', name='.', filename='')
+
+        # WHEN: Calling parse_verse_number
+        result = importer.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 instance of OpenSongBible, a non numeric string represented as a string
+        importer = OpenSongBible(MagicMock(), path='.', name='.', filename='')
+
+        # WHEN: Calling parse_verse_number
+        result = importer.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 instance of OpenSongBible, an empty string, and the previous verse number set as 14
+        importer = OpenSongBible(MagicMock(), path='.', name='.', filename='')
+        # WHEN: Calling parse_verse_number
+        result = importer.parse_verse_number('', 14)
+
+        # THEN: parse_verse_number should increment the previous verse number
+        self.assertEqual(result, 15)
+
+    def parse_verse_number_invalid_type_test(self):
+        """
+        Test parse_verse_number when the verse number is an invalid type)
+        """
+        with patch.object(OpenSongBible, 'log_warning')as mocked_log_warning:
+            # GIVEN: An instanceofOpenSongBible, a Tuple, and the previous verse number set as 12
+            importer = OpenSongBible(MagicMock(), path='.', name='.', filename='')
+
+            # WHEN: Calling parse_verse_number
+            result = importer.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)
+
+    def process_books_stop_import_test(self):
+        """
+        Test process_books when stop_import is set to True
+        """
+        # GIVEN: An instance 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(self.mocked_find_and_create_book.called)
+
+    def process_books_completes_test(self):
+        """
+        Test process_books when it processes all books
+        """
+        # GIVEN: An instance of OpenSongBible Importer and two mocked books
+        self.mocked_find_and_create_book.side_effect = ['db_book1', 'db_book2']
+        with patch.object(OpenSongBible, 'process_chapters') as mocked_process_chapters:
+            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.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(self.mocked_find_and_create_book.call_args_list,
+                             [call('Name1', 2, 10), call('Name2', 2, 10)])
+            self.assertEqual(mocked_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.parse_chapter_number', **{'side_effect': [1, 2]})
+    def process_chapters_completes_test(self, mocked_parse_chapter_number):
+        """
+        Test process_chapters when it completes
+        """
+        # GIVEN: An instance of OpenSongBible
+        importer = OpenSongBible(MagicMock(), path='.', name='.', filename='')
+        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(mocked_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
+        """
+        with patch('openlp.plugins.bibles.lib.importers.opensong.get_text',
+                   **{'side_effect': ['Verse1 Text', 'Verse2 Text']}) as mocked_get_text, \
+                patch.object(OpenSongBible, 'parse_verse_number',
+                             **{'side_effect': [1, 2]}) as mocked_parse_verse_number:
+            # GIVEN: An instance of OpenSongBible
+            importer = OpenSongBible(MagicMock(), path='.', name='.', filename='')
+            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(mocked_parse_verse_number.call_args_list, [call('1', 0), call('2', 1)])
+            self.assertEqual(mocked_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')])
+
+    def do_import_parse_xml_fails_test(self):
+        """
+        Test do_import when parse_xml fails (returns None)
+        """
+        # GIVEN: An instance of OpenSongBible and a mocked parse_xml which returns False
+        with patch.object(OpenSongBible, 'log_debug'), \
+                patch.object(OpenSongBible, 'validate_xml_file'), \
+                patch.object(OpenSongBible, 'parse_xml', return_value=None), \
+                patch.object(OpenSongBible, 'get_language_id') as mocked_language_id:
+            importer = OpenSongBible(MagicMock(), path='.', name='.', filename='')
+
+            # WHEN: Calling do_import
+            result = importer.do_import()
+
+            # THEN: do_import should return False and get_language_id should have not been called
+            self.assertFalse(result)
+            self.assertFalse(mocked_language_id.called)
+
+    def do_import_no_language_test(self):
+        """
+        Test do_import when the user cancels the language selection dialog
+        """
+        # GIVEN: An instance of OpenSongBible and a mocked get_language which returns False
+        with patch.object(OpenSongBible, 'log_debug'), \
+                patch.object(OpenSongBible, 'validate_xml_file'), \
+                patch.object(OpenSongBible, 'parse_xml'), \
+                patch.object(OpenSongBible, 'get_language_id', return_value=False), \
+                patch.object(OpenSongBible, 'process_books') as mocked_process_books:
+            importer = OpenSongBible(MagicMock(), path='.', name='.', filename='')
+
+            # 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(mocked_process_books.called)
+
+    def do_import_completes_test(self):
+        """
+        Test do_import when it completes successfully
+        """
+        # GIVEN: An instance of OpenSongBible
+        with patch.object(OpenSongBible, 'log_debug'), \
+                patch.object(OpenSongBible, 'validate_xml_file'), \
+                patch.object(OpenSongBible, 'parse_xml'), \
+                patch.object(OpenSongBible, 'get_language_id', return_value=10), \
+                patch.object(OpenSongBible, 'process_books'):
+            importer = OpenSongBible(MagicMock(), path='.', name='.', filename='')
+
+            # WHEN: Calling do_import
+            result = importer.do_import()
+
+            # THEN: do_import should return True
+            self.assertTrue(result)
+
+
+class TestOpenSongImportFileImports(TestCase, TestMixin):
+    """
+    Test the functions in the :mod:`opensongimport` module.
+    """
+    def setUp(self):
+        self.manager_patcher = patch('openlp.plugins.bibles.lib.db.Manager')
+        self.addCleanup(self.manager_patcher.stop)
+        self.manager_patcher.start()
 
     def test_file_import(self):
         """
@@ -92,22 +423,3 @@ class TestOpenSongImport(TestCase):
             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.')

tests/functional/openlp_plugins/bibles/test_osisimport.py

@@ -27,29 +27,35 @@ import os
 import json
 from unittest import TestCase
 
-from tests.functional import MagicMock, patch
-from openlp.plugins.bibles.lib.importers.osis import OSISBible
+from tests.functional import MagicMock, call, patch
+from openlp.plugins.bibles.lib.bibleimport import BibleImport
 from openlp.plugins.bibles.lib.db import BibleDB
+from openlp.plugins.bibles.lib.importers.osis import OSISBible
 
-TEST_PATH = os.path.abspath(os.path.join(os.path.dirname(__file__),
-                                         '..', '..', '..', 'resources', 'bibles'))
+TEST_PATH = os.path.abspath(os.path.join(os.path.dirname(__file__), '..', '..', '..', 'resources', 'bibles'))
 
 
 class TestOsisImport(TestCase):
     """
     Test the functions in the :mod:`osisimport` module.
     """
-
     def setUp(self):
-        self.registry_patcher = patch('openlp.plugins.bibles.lib.db.Registry')
+        self.etree_patcher = patch('openlp.plugins.bibles.lib.importers.osis.etree')
+        self.addCleanup(self.etree_patcher.stop)
+        self.mocked_etree = self.etree_patcher.start()
+        self.create_verse_patcher = patch('openlp.plugins.bibles.lib.db.BibleDB.create_verse')
+        self.addCleanup(self.create_verse_patcher.stop)
+        self.mocked_create_verse = self.create_verse_patcher.start()
+        self.find_and_create_book_patch = patch.object(BibleImport, 'find_and_create_book')
+        self.addCleanup(self.find_and_create_book_patch.stop)
+        self.mocked_find_and_create_book = self.find_and_create_book_patch.start()
+        self.registry_patcher = patch('openlp.plugins.bibles.lib.bibleimport.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 OSIS file importer
@@ -63,6 +69,353 @@ class TestOsisImport(TestCase):
         # THEN: The importer should be an instance of BibleDB
         self.assertIsInstance(importer, BibleDB)
 
+    def process_books_stop_import_test(self):
+        """
+        Test process_books when stop_import is set to True
+        """
+        # GIVEN: An instance of OSISBible adn some mocked data
+        importer = OSISBible(MagicMock(), path='.', name='.', filename='')
+        mocked_data = MagicMock(**{'xpath.return_value': ['Book']})
+
+        # WHEN: stop_import_flag is set to True and process_books is called
+        importer.stop_import_flag = True
+        importer.process_books(mocked_data)
+
+        # THEN: find_and_create_book should not have been called
+        self.assertFalse(self.mocked_find_and_create_book.called)
+
+    def process_books_completes_test(self):
+        """
+        Test process_books when it processes all books
+        """
+        # GIVEN: An instance of OSISBible Importer and two mocked books
+        self.mocked_find_and_create_book.side_effect = ['db_book1', 'db_book2']
+        with patch.object(OSISBible, 'process_chapters') as mocked_process_chapters:
+            importer = OSISBible(MagicMock(), path='.', name='.', filename='')
+
+            book1 = MagicMock()
+            book1.get.return_value = 'Name1'
+            book2 = MagicMock()
+            book2.get.return_value = 'Name2'
+            mocked_data = MagicMock(**{'xpath.return_value': [book1, book2]})
+            importer.language_id = 10
+            importer.session = MagicMock()
+            importer.stop_import_flag = False
+
+            # WHEN: Calling process_books with the two books
+            importer.process_books(mocked_data)
+
+            # THEN: find_and_create_book and process_books should be called with the details from the mocked books
+            self.assertEqual(self.mocked_find_and_create_book.call_args_list,
+                             [call('Name1', 2, 10), call('Name2', 2, 10)])
+            self.assertEqual(mocked_process_chapters.call_args_list,
+                             [call('db_book1', book1), call('db_book2', book2)])
+            self.assertEqual(importer.session.commit.call_count, 2)
+
+    def process_chapters_verse_in_chapter_verse_text_test(self):
+        """
+        Test process_chapters when supplied with an etree element with a verse element nested in it
+        """
+        with patch('openlp.plugins.bibles.lib.importers.osis.verse_in_chapter', return_value=True), \
+                patch('openlp.plugins.bibles.lib.importers.osis.text_in_verse', return_value=True), \
+                patch.object(OSISBible, 'set_current_chapter') as mocked_set_current_chapter, \
+                patch.object(OSISBible, 'process_verse') as mocked_process_verse:
+
+            # GIVEN: Some test data and an instance of OSISBible
+            test_book = MagicMock()
+            test_verse = MagicMock()
+            test_verse.tail = '\n    '  # Whitespace
+            test_verse.text = 'Verse Text'
+            test_chapter = MagicMock()
+            test_chapter.__iter__.return_value = [test_verse]
+            test_chapter.get.side_effect = lambda x: {'osisID': '1.2.4', 'sID': '999'}.get(x)
+            importer = OSISBible(MagicMock(), path='.', name='.', filename='')
+
+            # WHEN: Calling process_chapters
+            importer.process_chapters(test_book, [test_chapter])
+
+            # THEN: set_current_chapter and process_verse should have been called with the test data
+            mocked_set_current_chapter.assert_called_once_with(test_book.name, 2)
+            mocked_process_verse.assert_called_once_with(test_book, 2, test_verse)
+
+    def process_chapters_verse_in_chapter_verse_milestone_test(self):
+        """
+        Test process_chapters when supplied with an etree element with a verse element nested, when the verse system is
+        based on milestones
+        """
+        with patch('openlp.plugins.bibles.lib.importers.osis.verse_in_chapter', return_value=True), \
+                patch('openlp.plugins.bibles.lib.importers.osis.text_in_verse', return_value=False), \
+                patch.object(OSISBible, 'set_current_chapter') as mocked_set_current_chapter, \
+                patch.object(OSISBible, 'process_verse') as mocked_process_verse:
+
+            # GIVEN: Some test data and an instance of OSISBible
+            test_book = MagicMock()
+            test_verse = MagicMock()
+            test_verse.tail = '\n    '  # Whitespace
+            test_verse.text = 'Verse Text'
+            test_chapter = MagicMock()
+            test_chapter.__iter__.return_value = [test_verse]
+            test_chapter.get.side_effect = lambda x: {'osisID': '1.2.4', 'sID': '999'}.get(x)
+            importer = OSISBible(MagicMock(), path='.', name='.', filename='')
+
+            # WHEN: Calling process_chapters
+            importer.process_chapters(test_book, [test_chapter])
+
+            # THEN: set_current_chapter and process_verse should have been called with the test data
+            mocked_set_current_chapter.assert_called_once_with(test_book.name, 2)
+            mocked_process_verse.assert_called_once_with(test_book, 2, test_verse, use_milestones=True)
+
+    def process_chapters_milestones_chapter_no_sid_test(self):
+        """
+        Test process_chapters when supplied with an etree element with a chapter and verse element in the milestone
+        configuration, where the chapter is the "closing" milestone. (Missing the sID attribute)
+        """
+        with patch('openlp.plugins.bibles.lib.importers.osis.verse_in_chapter', return_value=False), \
+                patch.object(OSISBible, 'set_current_chapter') as mocked_set_current_chapter, \
+                patch.object(OSISBible, 'process_verse') as mocked_process_verse:
+
+            # GIVEN: Some test data and an instance of OSISBible
+            test_book = MagicMock()
+            test_chapter = MagicMock()
+            test_chapter.tag = '{http://www.bibletechnologies.net/2003/OSIS/namespace}chapter'
+            test_chapter.get.side_effect = lambda x: {'osisID': '1.2.4'}.get(x)
+
+            # WHEN: Calling process_chapters
+            importer = OSISBible(MagicMock(), path='.', name='.', filename='')
+            importer.process_chapters(test_book, [test_chapter])
+
+            # THEN: neither set_current_chapter or process_verse should have been called
+            self.assertFalse(mocked_set_current_chapter.called)
+            self.assertFalse(mocked_process_verse.called)
+
+    def process_chapters_milestones_chapter_sid_test(self):
+        """
+        Test process_chapters when supplied with an etree element with a chapter and verse element in the milestone
+        configuration, where the chapter is the "opening" milestone. (Has the sID attribute)
+        """
+        with patch('openlp.plugins.bibles.lib.importers.osis.verse_in_chapter', return_value=False), \
+                patch.object(OSISBible, 'set_current_chapter') as mocked_set_current_chapter, \
+                patch.object(OSISBible, 'process_verse') as mocked_process_verse:
+
+            # GIVEN: Some test data and an instance of OSISBible
+            test_book = MagicMock()
+            test_chapter = MagicMock()
+            test_chapter.tag = '{http://www.bibletechnologies.net/2003/OSIS/namespace}chapter'
+            test_chapter.get.side_effect = lambda x: {'osisID': '1.2.4', 'sID': '999'}.get(x)
+            importer = OSISBible(MagicMock(), path='.', name='.', filename='')
+
+            # WHEN: Calling process_chapters
+            importer.process_chapters(test_book, [test_chapter])
+
+            # THEN: set_current_chapter should have been called with the test data
+            mocked_set_current_chapter.assert_called_once_with(test_book.name, 2)
+            self.assertFalse(mocked_process_verse.called)
+
+    def process_chapters_milestones_verse_tag_test(self):
+        """
+        Test process_chapters when supplied with an etree element with a chapter and verse element in the milestone
+        configuration, where the verse is the "opening" milestone. (Has the sID attribute)
+        """
+        with patch('openlp.plugins.bibles.lib.importers.osis.verse_in_chapter', return_value=False), \
+                patch.object(OSISBible, 'set_current_chapter') as mocked_set_current_chapter, \
+                patch.object(OSISBible, 'process_verse') as mocked_process_verse:
+
+            # GIVEN: Some test data and an instance of OSISBible
+            test_book = MagicMock()
+            test_verse = MagicMock()
+            test_verse.get.side_effect = lambda x: {'osisID': '1.2.4', 'sID': '999'}.get(x)
+            test_verse.tag = '{http://www.bibletechnologies.net/2003/OSIS/namespace}verse'
+            test_verse.tail = '\n    '  # Whitespace
+            test_verse.text = 'Verse Text'
+
+            # WHEN: Calling process_chapters
+            importer = OSISBible(MagicMock(), path='.', name='.', filename='')
+            importer.process_chapters(test_book, [test_verse])
+
+            # THEN: process_verse should have been called with the test data
+            self.assertFalse(mocked_set_current_chapter.called)
+            mocked_process_verse.assert_called_once_with(test_book, 0, test_verse, use_milestones=True)
+
+    def process_verse_no_osis_id_test(self):
+        """
+        Test process_verse when the element supplied does not have and osisID attribute
+        """
+        # GIVEN: An instance of OSISBible, and some mocked test data
+        test_book = MagicMock()
+        test_verse = MagicMock()
+        test_verse.get.side_effect = lambda x: {}.get(x)
+        test_verse.tail = 'Verse Text'
+        test_verse.text = None
+        importer = OSISBible(MagicMock(), path='.', name='.', filename='')
+
+        # WHEN: Calling process_verse with the test data
+        importer.process_verse(test_book, 2, test_verse)
+
+        # THEN: create_verse should not have been called
+        self.assertFalse(self.mocked_create_verse.called)
+
+    def process_verse_use_milestones_no_s_id_test(self):
+        """
+        Test process_verse when called with use_milestones set to True, but the element supplied does not have and sID
+        attribute
+        """
+        # GIVEN: An instance of OSISBible, and some mocked test data
+        test_book = MagicMock()
+        test_verse = MagicMock()
+        test_verse.get.side_effect = lambda x: {}.get(x)
+        test_verse.tail = 'Verse Text'
+        test_verse.text = None
+        importer = OSISBible(MagicMock(), path='.', name='.', filename='')
+
+        # WHEN: Calling process_verse with the test data
+        importer.process_verse(test_book, 2, test_verse)
+
+        # THEN: create_verse should not have been called
+        self.assertFalse(self.mocked_create_verse.called)
+
+    def process_verse_use_milestones_no_tail_test(self):
+        """
+        Test process_verse when called with use_milestones set to True, but the element supplied does not have a 'tail'
+        """
+        # GIVEN: An instance of OSISBible, and some mocked test data
+        test_book = MagicMock()
+        test_verse = MagicMock()
+        test_verse.tail = None
+        test_verse.text = None
+        test_verse.get.side_effect = lambda x: {'osisID': '1.2.4', 'sID': '999'}.get(x)
+        importer = OSISBible(MagicMock(), path='.', name='.', filename='')
+
+        # WHEN: Calling process_verse with the test data
+        importer.process_verse(test_book, 2, test_verse, use_milestones=True)
+
+        # THEN: create_verse should not have been called
+        self.assertFalse(self.mocked_create_verse.called)
+
+    def process_verse_use_milestones_success_test(self):
+        """
+        Test process_verse when called with use_milestones set to True, and the verse element successfully imports
+        """
+        # GIVEN: An instance of OSISBible, and some mocked test data
+        test_book = MagicMock()
+        test_book.id = 1
+        test_verse = MagicMock()
+        test_verse.tail = 'Verse Text'
+        test_verse.text = None
+        test_verse.get.side_effect = lambda x: {'osisID': '1.2.4', 'sID': '999'}.get(x)
+        importer = OSISBible(MagicMock(), path='.', name='.', filename='')
+
+        # WHEN: Calling process_verse with the test data
+        importer.process_verse(test_book, 2, test_verse, use_milestones=True)
+
+        # THEN: create_verse should have been called with the test data
+        self.mocked_create_verse.assert_called_once_with(1, 2, 4, 'Verse Text')
+
+    def process_verse_no_text_test(self):
+        """
+        Test process_verse when called with an empty verse element
+        """
+        # GIVEN: An instance of OSISBible, and some mocked test data
+        test_book = MagicMock()
+        test_book.id = 1
+        test_verse = MagicMock()
+        test_verse.tail = '\n    '  # Whitespace
+        test_verse.text = None
+        test_verse.get.side_effect = lambda x: {'osisID': '1.2.4', 'sID': '999'}.get(x)
+        importer = OSISBible(MagicMock(), path='.', name='.', filename='')
+
+        # WHEN: Calling process_verse with the test data
+        importer.process_verse(test_book, 2, test_verse)
+
+        # THEN: create_verse should not have been called
+        self.assertFalse(self.mocked_create_verse.called)
+
+    def process_verse_success_test(self):
+        """
+        Test process_verse when called with an element with text set
+        """
+        # GIVEN: An instance of OSISBible, and some mocked test data
+        test_book = MagicMock()
+        test_book.id = 1
+        test_verse = MagicMock()
+        test_verse.tail = '\n    '  # Whitespace
+        test_verse.text = 'Verse Text'
+        test_verse.get.side_effect = lambda x: {'osisID': '1.2.4', 'sID': '999'}.get(x)
+        importer = OSISBible(MagicMock(), path='.', name='.', filename='')
+
+        # WHEN: Calling process_verse with the test data
+        importer.process_verse(test_book, 2, test_verse)
+
+        # THEN: create_verse should have been called with the test data
+        self.mocked_create_verse.assert_called_once_with(1, 2, 4, 'Verse Text')
+
+    def do_import_parse_xml_fails_test(self):
+        """
+        Test do_import when parse_xml fails (returns None)
+        """
+        # GIVEN: An instance of OpenSongBible and a mocked parse_xml which returns False
+        with patch.object(OSISBible, 'log_debug'), \
+                patch.object(OSISBible, 'validate_xml_file'), \
+                patch.object(OSISBible, 'parse_xml', return_value=None), \
+                patch.object(OSISBible, 'get_language_id') as mocked_language_id:
+            importer = OSISBible(MagicMock(), path='.', name='.', filename='')
+
+            # WHEN: Calling do_import
+            result = importer.do_import()
+
+            # THEN: do_import should return False and get_language_id should have not been called
+            self.assertFalse(result)
+            self.assertFalse(mocked_language_id.called)
+
+    def do_import_no_language_test(self):
+        """
+        Test do_import when the user cancels the language selection dialog
+        """
+        # GIVEN: An instance of OpenSongBible and a mocked get_language which returns False
+        with patch.object(OSISBible, 'log_debug'), \
+                patch.object(OSISBible, 'validate_xml_file'), \
+                patch.object(OSISBible, 'parse_xml'), \
+                patch.object(OSISBible, 'get_language_id', **{'return_value': False}), \
+                patch.object(OSISBible, 'process_books') as mocked_process_books:
+            importer = OSISBible(MagicMock(), path='.', name='.', filename='')
+
+            # 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(mocked_process_books.called)
+
+    def do_import_completes_test(self):
+        """
+        Test do_import when it completes successfully
+        """
+        # GIVEN: An instance of OpenSongBible
+        with patch.object(OSISBible, 'log_debug'), \
+                patch.object(OSISBible, 'validate_xml_file'), \
+                patch.object(OSISBible, 'parse_xml'), \
+                patch.object(OSISBible, 'get_language_id', **{'return_value': 10}), \
+                patch.object(OSISBible, 'process_books'):
+            importer = OSISBible(MagicMock(), path='.', name='.', filename='')
+
+            # WHEN: Calling do_import
+            result = importer.do_import()
+
+            # THEN: do_import should return True
+            self.assertTrue(result)
+
+
+class TestOsisImportFileImports(TestCase):
+    """
+    Test the functions in the :mod:`osisimport` module.
+    """
+    def setUp(self):
+        self.registry_patcher = patch('openlp.plugins.bibles.lib.bibleimport.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 test_file_import_nested_tags(self):
         """
         Test the actual import of OSIS Bible file, with nested chapter and verse tags
@@ -91,7 +444,7 @@ class TestOsisImport(TestCase):
             # THEN: The create_verse() method should have been called with each verse in the file.
             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', verse_tag, verse_text)
+                importer.create_verse.assert_any_call(importer.create_book().id, 1, verse_tag, verse_text)
 
     def test_file_import_mixed_tags(self):
         """
@@ -121,7 +474,7 @@ class TestOsisImport(TestCase):
             # THEN: The create_verse() method should have been called with each verse in the file.
             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', verse_tag, verse_text)
+                    importer.create_verse.assert_any_call(importer.create_book().id, 1, verse_tag, verse_text)
 
     def test_file_import_milestone_tags(self):
         """
@@ -151,7 +504,7 @@ class TestOsisImport(TestCase):
             # THEN: The create_verse() method should have been called with each verse in the file.
             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', verse_tag, verse_text)
+                importer.create_verse.assert_any_call(importer.create_book().id, 1, verse_tag, verse_text)
 
     def test_file_import_empty_verse_tags(self):
         """
@@ -181,4 +534,4 @@ class TestOsisImport(TestCase):
             # THEN: The create_verse() method should have been called with each verse in the file.
             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', verse_tag, verse_text)
+                importer.create_verse.assert_any_call(importer.create_book().id, 1, verse_tag, verse_text)

tests/functional/openlp_plugins/bibles/test_swordimport.py

@@ -46,7 +46,7 @@ class TestSwordImport(TestCase):
     """
 
     def setUp(self):
-        self.registry_patcher = patch('openlp.plugins.bibles.lib.db.Registry')
+        self.registry_patcher = patch('openlp.plugins.bibles.lib.bibleimport.Registry')
         self.registry_patcher.start()
         self.manager_patcher = patch('openlp.plugins.bibles.lib.db.Manager')
         self.manager_patcher.start()
@@ -70,8 +70,7 @@ class TestSwordImport(TestCase):
 
     @patch('openlp.plugins.bibles.lib.importers.sword.SwordBible.application')
     @patch('openlp.plugins.bibles.lib.importers.sword.modules')
-    @patch('openlp.core.common.languages')
-    def test_simple_import(self, mocked_languages, mocked_pysword_modules, mocked_application):
+    def test_simple_import(self, mocked_pysword_modules, mocked_application):
         """
         Test that a simple SWORD import works
         """
@@ -88,7 +87,7 @@ class TestSwordImport(TestCase):
         importer.create_verse = MagicMock()
         importer.create_book = MagicMock()
         importer.session = MagicMock()
-        mocked_languages.get_language.return_value = 'Danish'
+        importer.get_language = MagicMock(return_value='Danish')
         mocked_bible = MagicMock()
         mocked_genesis = MagicMock()
         mocked_genesis.name = 'Genesis'

tests/functional/openlp_plugins/bibles/test_zefaniaimport.py

@@ -41,15 +41,13 @@ class TestZefaniaImport(TestCase):
     """
 
     def setUp(self):
-        self.registry_patcher = patch('openlp.plugins.bibles.lib.db.Registry')
+        self.registry_patcher = patch('openlp.plugins.bibles.lib.bibleimport.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
@@ -90,7 +88,7 @@ class TestZefaniaImport(TestCase):
             # THEN: The create_verse() method should have been called with each verse in the file.
             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', verse_tag, verse_text)
+                importer.create_verse.assert_any_call(importer.create_book().id, 1, verse_tag, verse_text)
             importer.create_book.assert_any_call('Genesis', 1, 1)
 
     def test_file_import_no_book_name(self):
@@ -120,5 +118,5 @@ class TestZefaniaImport(TestCase):
             # THEN: The create_verse() method should have been called with each verse in the file.
             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', verse_tag, verse_text)
+                importer.create_verse.assert_any_call(importer.create_book().id, 1, verse_tag, verse_text)
             importer.create_book.assert_any_call('Exodus', 2, 1)

tests/functional/openlp_plugins/songs/test_mediashout.py

@@ -0,0 +1,228 @@
+# -*- coding: utf-8 -*-
+# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4
+
+###############################################################################
+# OpenLP - Open Source Lyrics Projection                                      #
+# --------------------------------------------------------------------------- #
+# Copyright (c) 2008-2016 OpenLP Developers                                   #
+# --------------------------------------------------------------------------- #
+# This program is free software; you can redistribute it and/or modify it     #
+# under the terms of the GNU General Public License as published by the Free  #
+# Software Foundation; version 2 of the License.                              #
+#                                                                             #
+# This program is distributed in the hope that it will be useful, but WITHOUT #
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or       #
+# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for    #
+# more details.                                                               #
+#                                                                             #
+# You should have received a copy of the GNU General Public License along     #
+# with this program; if not, write to the Free Software Foundation, Inc., 59  #
+# Temple Place, Suite 330, Boston, MA 02111-1307 USA                          #
+###############################################################################
+"""
+Test the MediaShout importer
+"""
+from unittest import TestCase, skipUnless
+from collections import namedtuple
+
+from openlp.core.common import Registry
+try:
+    from openlp.plugins.songs.lib.importers.mediashout import MediaShoutImport
+    CAN_RUN_TESTS = True
+except ImportError:
+    CAN_RUN_TESTS = False
+
+from tests.functional import MagicMock, patch, call
+
+
+@skipUnless(CAN_RUN_TESTS, 'Not Windows, skipping test')
+class TestMediaShoutImport(TestCase):
+    """
+    Test the MediaShout importer
+    """
+    def setUp(self):
+        """
+        Set the tests up
+        """
+        Registry().create()
+
+    def test_constructor(self):
+        """
+        Test the MediaShoutImport constructor
+        """
+        # GIVEN: A MediaShoutImport class
+        # WHEN: It is created
+        importer = MediaShoutImport(MagicMock(), filename='mediashout.db')
+
+        # THEN: It should not be None
+        self.assertIsNotNone(importer)
+
+    @patch('openlp.plugins.songs.lib.importers.mediashout.pyodbc')
+    def test_do_import_fails_to_connect(self, mocked_pyodbc):
+        """
+        Test that do_import exits early when unable to connect to the database
+        """
+        # GIVEN: A MediaShoutImport instance
+        importer = MediaShoutImport(MagicMock(), filename='mediashout.db')
+        mocked_pyodbc.connect.side_effect = Exception('Unable to connect')
+
+        # WHEN: do_import is called
+        with patch.object(importer, 'log_error') as mocked_log_error:
+            importer.do_import()
+
+        # THEN: The songs should have been imported
+        mocked_log_error.assert_called_once_with('mediashout.db', 'Unable to open the MediaShout database.')
+
+    @patch('openlp.plugins.songs.lib.importers.mediashout.pyodbc')
+    def test_do_import(self, mocked_pyodbc):
+        """
+        Test the MediaShoutImport do_import method
+        """
+        SongRecord = namedtuple('SongRecord', 'Record, Title, Author, Copyright, SongID, CCLI, Notes')
+        VerseRecord = namedtuple('VerseRecord', 'Type, Number, Text')
+        PlayOrderRecord = namedtuple('PlayOrderRecord', 'Type, Number, POrder')
+        ThemeRecord = namedtuple('ThemeRecord', 'Name')
+        GroupRecord = namedtuple('GroupRecord', 'Name')
+        song = SongRecord(1, 'Amazing Grace', 'William Wilberforce', 'Public Domain', 1, '654321', '')
+        verse = VerseRecord('Verse', 1, 'Amazing grace, how sweet the sound\nThat saved a wretch like me')
+        play_order = PlayOrderRecord('Verse', 1, 1)
+        theme = ThemeRecord('Grace')
+        group = GroupRecord('Hymns')
+
+        # GIVEN: A MediaShoutImport instance and a bunch of stuff mocked out
+        importer = MediaShoutImport(MagicMock(), filename='mediashout.db')
+        mocked_cursor = MagicMock()
+        mocked_cursor.fetchall.side_effect = [[song], [verse], [play_order], [theme], [group]]
+        mocked_cursor.tables.fetchone.return_value = True
+        mocked_connection = MagicMock()
+        mocked_connection.cursor.return_value = mocked_cursor
+        mocked_pyodbc.connect.return_value = mocked_connection
+
+        # WHEN: do_import is called
+        with patch.object(importer, 'import_wizard') as mocked_import_wizard, \
+                patch.object(importer, 'process_song') as mocked_process_song:
+            importer.do_import()
+
+        # THEN: The songs should have been imported
+        expected_execute_calls = [
+            call('SELECT Record, Title, Author, Copyright, SongID, CCLI, Notes FROM Songs ORDER BY Title'),
+            call('SELECT Type, Number, Text FROM Verses WHERE Record = ? ORDER BY Type, Number', 1.0),
+            call('SELECT Type, Number, POrder FROM PlayOrder WHERE Record = ? ORDER BY POrder', 1.0),
+            call('SELECT Name FROM Themes INNER JOIN SongThemes ON SongThemes.ThemeId = Themes.ThemeId '
+                 'WHERE SongThemes.Record = ?', 1.0),
+            call('SELECT Name FROM Groups INNER JOIN SongGroups ON SongGroups.GroupId = Groups.GroupId '
+                 'WHERE SongGroups.Record = ?', 1.0)
+        ]
+        self.assertEqual(expected_execute_calls, mocked_cursor.execute.call_args_list)
+        mocked_process_song.assert_called_once_with(song, [verse], [play_order], [theme, group])
+
+    @patch('openlp.plugins.songs.lib.importers.mediashout.pyodbc')
+    def test_do_import_breaks_on_stop(self, mocked_pyodbc):
+        """
+        Test the MediaShoutImport do_import stops when the user presses the cancel button
+        """
+        SongRecord = namedtuple('SongRecord', 'Record, Title, Author, Copyright, SongID, CCLI, Notes')
+        song = SongRecord(1, 'Amazing Grace', 'William Wilberforce', 'Public Domain', 1, '654321', '')
+
+        # GIVEN: A MediaShoutImport instance and a bunch of stuff mocked out
+        importer = MediaShoutImport(MagicMock(), filename='mediashout.db')
+        mocked_cursor = MagicMock()
+        mocked_cursor.fetchall.return_value = [song]
+        mocked_connection = MagicMock()
+        mocked_connection.cursor.return_value = mocked_cursor
+        mocked_pyodbc.connect.return_value = mocked_connection
+
+        # WHEN: do_import is called, but cancelled
+        with patch.object(importer, 'import_wizard') as mocked_import_wizard, \
+                patch.object(importer, 'process_song') as mocked_process_song:
+            importer.stop_import_flag = True
+            importer.do_import()
+
+        # THEN: The songs should have been imported
+        mocked_cursor.execute.assert_called_once_with(
+            'SELECT Record, Title, Author, Copyright, SongID, CCLI, Notes FROM Songs ORDER BY Title')
+        mocked_process_song.assert_not_called()
+
+    def test_process_song(self):
+        """
+        Test the process_song method of the MediaShoutImport
+        """
+        # GIVEN: An importer and a song
+        SongRecord = namedtuple('SongRecord', 'Record, Title, Author, Copyright, SongID, CCLI, Notes')
+        VerseRecord = namedtuple('VerseRecord', 'Type, Number, Text')
+        PlayOrderRecord = namedtuple('PlayOrderRecord', 'Type, Number, POrder')
+        ThemeRecord = namedtuple('ThemeRecord', 'Name')
+        GroupRecord = namedtuple('GroupRecord', 'Name')
+        song = SongRecord(1, 'Amazing Grace', 'William Wilberforce', 'Public Domain', 'Hymns', '654321',
+                          'Great old hymn')
+        verse = VerseRecord(0, 1, 'Amazing grace, how sweet the sound\nThat saved a wretch like me')
+        play_order = PlayOrderRecord(0, 1, 1)
+        theme = ThemeRecord('Grace')
+        group = GroupRecord('Hymns')
+        importer = MediaShoutImport(MagicMock(), filename='mediashout.db')
+
+        # WHEN: A song is processed
+        with patch.object(importer, 'set_defaults') as mocked_set_defaults, \
+                patch.object(importer, 'parse_author') as mocked_parse_author, \
+                patch.object(importer, 'add_copyright') as mocked_add_copyright, \
+                patch.object(importer, 'add_verse') as mocked_add_verse, \
+                patch.object(importer, 'finish') as mocked_finish:
+            importer.topics = []
+            importer.verse_order_list = []
+            importer.process_song(song, [verse], [play_order], [theme, group])
+
+        # THEN: It should be added to the database
+        mocked_set_defaults.assert_called_once_with()
+        self.assertEqual('Amazing Grace', importer.title)
+        mocked_parse_author.assert_called_once_with('William Wilberforce')
+        mocked_add_copyright.assert_called_once_with('Public Domain')
+        self.assertEqual('Great old hymn', importer.comments)
+        self.assertEqual(['Grace', 'Hymns'], importer.topics)
+        self.assertEqual('Hymns', importer.song_book_name)
+        self.assertEqual('', importer.song_number)
+        mocked_add_verse.assert_called_once_with(
+            'Amazing grace, how sweet the sound\nThat saved a wretch like me', 'V1')
+        self.assertEqual(['V1'], importer.verse_order_list)
+        mocked_finish.assert_called_once_with()
+
+    def test_process_song_with_song_number(self):
+        """
+        Test the process_song method with a song that has a song number
+        """
+        # GIVEN: An importer and a song
+        SongRecord = namedtuple('SongRecord', 'Record, Title, Author, Copyright, SongID, CCLI, Notes')
+        VerseRecord = namedtuple('VerseRecord', 'Type, Number, Text')
+        PlayOrderRecord = namedtuple('PlayOrderRecord', 'Type, Number, POrder')
+        ThemeRecord = namedtuple('ThemeRecord', 'Name')
+        GroupRecord = namedtuple('GroupRecord', 'Name')
+        song = SongRecord(1, 'Amazing Grace', 'William Wilberforce', 'Public Domain', 'Hymns-2', '654321',
+                          'Great old hymn')
+        verse = VerseRecord(0, 1, 'Amazing grace, how sweet the sound\nThat saved a wretch like me')
+        play_order = PlayOrderRecord(0, 1, 1)
+        theme = ThemeRecord('Grace')
+        group = GroupRecord('Hymns')
+        importer = MediaShoutImport(MagicMock(), filename='mediashout.db')
+
+        # WHEN: A song is processed
+        with patch.object(importer, 'set_defaults') as mocked_set_defaults, \
+                patch.object(importer, 'parse_author') as mocked_parse_author, \
+                patch.object(importer, 'add_copyright') as mocked_add_copyright, \
+                patch.object(importer, 'add_verse') as mocked_add_verse, \
+                patch.object(importer, 'finish') as mocked_finish:
+            importer.topics = []
+            importer.verse_order_list = []
+            importer.process_song(song, [verse], [play_order], [theme, group])
+
+        # THEN: It should be added to the database
+        mocked_set_defaults.assert_called_once_with()
+        self.assertEqual('Amazing Grace', importer.title)
+        mocked_parse_author.assert_called_once_with('William Wilberforce')
+        mocked_add_copyright.assert_called_once_with('Public Domain')
+        self.assertEqual('Great old hymn', importer.comments)
+        self.assertEqual(['Grace', 'Hymns'], importer.topics)
+        self.assertEqual('Hymns', importer.song_book_name)
+        self.assertEqual('2', importer.song_number)
+        mocked_add_verse.assert_called_once_with(
+            'Amazing grace, how sweet the sound\nThat saved a wretch like me', 'V1')
+        self.assertEqual(['V1'], importer.verse_order_list)
+        mocked_finish.assert_called_once_with()

tests/interfaces/openlp_plugins/bibles/test_lib_http.py

@@ -163,3 +163,19 @@ class TestBibleHTTP(TestCase):
         # THEN: The list should not be None, and some known bibles should be there
         self.assertIsNotNone(bibles)
         self.assertIn(('Giovanni Diodati 1649 (Italian)', 'gdb', 'it'), bibles)
+
+    def test_crosswalk_get_verse_text(self):
+        """
+        Test verse text from Crosswalk.com
+        """
+        # GIVEN: A new Crosswalk extraction class
+        handler = CWExtract()
+
+        # WHEN: downloading NIV Genesis from Crosswalk
+        niv_genesis_chapter_one = handler.get_bible_chapter('niv', 'Genesis', 1)
+
+        # THEN: The verse list should contain the verses
+        self.assertTrue(niv_genesis_chapter_one.has_verse_list())
+        self.assertEquals('In the beginning God created the heavens and the earth.',
+                          niv_genesis_chapter_one.verse_list[1],
+                          'The first chapter of genesis should have been fetched.')

tests/resources/bibles/dk1933.json

@@ -2,15 +2,15 @@
     "book": "Genesis",
     "chapter": 1,
     "verses": [
-        [ "1", "I Begyndelsen skabte Gud Himmelen og Jorden."],
-        [ "2", "Og Jorden var øde og tom, og der var Mørke over Verdensdybet. Men Guds Ånd svævede over Vandene." ],
-        [ "3", "Og Gud sagde: \"Der blive Lys!\" Og der blev Lys." ],
-        [ "4", "Og Gud så, at Lyset var godt, og Gud satte Skel mellem Lyset og Mørket," ],
-        [ "5", "og Gud kaldte Lyset Dag, og Mørket kaldte han Nat. Og det blev Aften, og det blev Morgen, første Dag." ],
-        [ "6", "Derpå sagde Gud: \"Der blive en Hvælving midt i Vandene til at skille Vandene ad!\"" ],
-        [ "7", "Og således skete det: Gud gjorde Hvælvingen og skilte Vandet under Hvælvingen fra Vandet over Hvælvingen;" ],
-        [ "8", "og Gud kaldte Hvælvingen Himmel. Og det blev Aften, og det blev Morgen, anden Dag." ],
-        [ "9", "Derpå sagde Gud: \"Vandet under Himmelen samle sig på eet Sted, så det faste Land kommer til Syne!\" Og således skete det;" ],
-        [ "10", "og Gud kaldte det faste Land Jord, og Stedet, hvor Vandet samlede sig, kaldte han Hav. Og Gud så, at det var godt." ]
+        [ 1, "I Begyndelsen skabte Gud Himmelen og Jorden."],
+        [ 2, "Og Jorden var øde og tom, og der var Mørke over Verdensdybet. Men Guds Ånd svævede over Vandene." ],
+        [ 3, "Og Gud sagde: \"Der blive Lys!\" Og der blev Lys." ],
+        [ 4, "Og Gud så, at Lyset var godt, og Gud satte Skel mellem Lyset og Mørket," ],
+        [ 5, "og Gud kaldte Lyset Dag, og Mørket kaldte han Nat. Og det blev Aften, og det blev Morgen, første Dag." ],
+        [ 6, "Derpå sagde Gud: \"Der blive en Hvælving midt i Vandene til at skille Vandene ad!\"" ],
+        [ 7, "Og således skete det: Gud gjorde Hvælvingen og skilte Vandet under Hvælvingen fra Vandet over Hvælvingen;" ],
+        [ 8, "og Gud kaldte Hvælvingen Himmel. Og det blev Aften, og det blev Morgen, anden Dag." ],
+        [ 9, "Derpå sagde Gud: \"Vandet under Himmelen samle sig på eet Sted, så det faste Land kommer til Syne!\" Og således skete det;" ],
+        [ 10, "og Gud kaldte det faste Land Jord, og Stedet, hvor Vandet samlede sig, kaldte han Hav. Og Gud så, at det var godt." ]
     ]
 }

tests/resources/bibles/kjv.json

@@ -2,15 +2,15 @@
     "book": "Genesis",
     "chapter": 1,
     "verses": [
-        [ "1", "In the beginning God created the heaven and the earth."],
-        [ "2", "And the earth was without form, and void; and darkness was upon the face of the deep. And the Spirit of God moved upon the face of the waters." ],
-        [ "3", "And God said, Let there be light: and there was light." ],
-        [ "4", "And God saw the light, that it was good: and God divided the light from the darkness." ],
-        [ "5", "And God called the light Day, and the darkness he called Night. And the evening and the morning were the first day." ],
-        [ "6", "And God said, Let there be a firmament in the midst of the waters, and let it divide the waters from the waters." ],
-        [ "7", "And God made the firmament, and divided the waters which were under the firmament from the waters which were above the firmament: and it was so." ],
-        [ "8", "And God called the firmament Heaven. And the evening and the morning were the second day." ],
-        [ "9", "And God said, Let the waters under the heaven be gathered together unto one place, and let the dry land appear: and it was so." ],
-        [ "10", "And God called the dry land Earth; and the gathering together of the waters called he Seas: and God saw that it was good." ]
+        [ 1, "In the beginning God created the heaven and the earth."],
+        [ 2, "And the earth was without form, and void; and darkness was upon the face of the deep. And the Spirit of God moved upon the face of the waters." ],
+        [ 3, "And God said, Let there be light: and there was light." ],
+        [ 4, "And God saw the light, that it was good: and God divided the light from the darkness." ],
+        [ 5, "And God called the light Day, and the darkness he called Night. And the evening and the morning were the first day." ],
+        [ 6, "And God said, Let there be a firmament in the midst of the waters, and let it divide the waters from the waters." ],
+        [ 7, "And God made the firmament, and divided the waters which were under the firmament from the waters which were above the firmament: and it was so." ],
+        [ 8, "And God called the firmament Heaven. And the evening and the morning were the second day." ],
+        [ 9, "And God said, Let the waters under the heaven be gathered together unto one place, and let the dry land appear: and it was so." ],
+        [ 10, "And God called the dry land Earth; and the gathering together of the waters called he Seas: and God saw that it was good." ]
     ]
 }

tests/resources/bibles/rst.json

@@ -2,15 +2,15 @@
     "book": "Exodus",
     "chapter": 1,
     "verses": [
-        [ "1", "Вот имена сынов Израилевых, которые вошли в Египет с Иаковом, вошли каждый с домом своим:" ],
-        [ "2", "Рувим, Симеон, Левий и Иуда," ],
-        [ "3", "Иссахар, Завулон и Вениамин," ],
-        [ "4", "Дан и Неффалим, Гад и Асир." ],
-        [ "5", "Всех же душ, происшедших от чресл Иакова, было семьдесят, а Иосиф был [уже] в Египте." ],
-        [ "6", "И умер Иосиф и все братья его и весь род их;" ],
-        [ "7", "а сыны Израилевы расплодились и размножились, и возросли и усилились чрезвычайно, и наполнилась ими земля та." ],
-        [ "8", "И восстал в Египте новый царь, который не знал Иосифа," ],
-        [ "9", "и сказал народу своему: вот, народ сынов Израилевых многочислен и сильнее нас;" ],
-        [ "10", "перехитрим же его, чтобы он не размножался; иначе, когда случится война, соединится и он с нашими неприятелями, и вооружится против нас, и выйдет из земли [нашей]." ]
+        [ 1, "Вот имена сынов Израилевых, которые вошли в Египет с Иаковом, вошли каждый с домом своим:" ],
+        [ 2, "Рувим, Симеон, Левий и Иуда," ],
+        [ 3, "Иссахар, Завулон и Вениамин," ],
+        [ 4, "Дан и Неффалим, Гад и Асир." ],
+        [ 5, "Всех же душ, происшедших от чресл Иакова, было семьдесят, а Иосиф был [уже] в Египте." ],
+        [ 6, "И умер Иосиф и все братья его и весь род их;" ],
+        [ 7, "а сыны Израилевы расплодились и размножились, и возросли и усилились чрезвычайно, и наполнилась ими земля та." ],
+        [ 8, "И восстал в Египте новый царь, который не знал Иосифа," ],
+        [ 9, "и сказал народу своему: вот, народ сынов Израилевых многочислен и сильнее нас;" ],
+        [ 10, "перехитрим же его, чтобы он не размножался; иначе, когда случится война, соединится и он с нашими неприятелями, и вооружится против нас, и выйдет из земли [нашей]." ]
     ]
 }

tests/resources/bibles/web.json

@@ -2,15 +2,15 @@
     "book": "Genesis",
     "chapter": "1",
     "verses": [
-        [ "1", "In the beginning God created the heavens and the earth."],
-        [ "2", "Now the earth was formless and empty. Darkness was on the surface of the deep. God’s Spirit was hovering over the surface of the waters." ],
-        [ "3", "God said, “Let there be light,” and there was light." ],
-        [ "4", "God saw the light, and saw that it was good. God divided the light from the darkness." ],
-        [ "5", "God called the light “day,” and the darkness he called “night.” There was evening and there was morning, one day." ],
-        [ "6", "God said, “Let there be an expanse in the middle of the waters, and let it divide the waters from the waters.”" ],
-        [ "7", "God made the expanse, and divided the waters which were under the expanse from the waters which were above the expanse; and it was so." ],
-        [ "8", "God called the expanse “sky.” There was evening and there was morning, a second day." ],
-        [ "9", "God said, “Let the waters under the sky be gathered together to one place, and let the dry land appear;” and it was so." ],
-        [ "10", "God called the dry land “earth,” and the gathering together of the waters he called “seas.” God saw that it was good." ]
+        [ 1, "In the beginning God created the heavens and the earth."],
+        [ 2, "Now the earth was formless and empty. Darkness was on the surface of the deep. God’s Spirit was hovering over the surface of the waters." ],
+        [ 3, "God said, “Let there be light,” and there was light." ],
+        [ 4, "God saw the light, and saw that it was good. God divided the light from the darkness." ],
+        [ 5, "God called the light “day,” and the darkness he called “night.” There was evening and there was morning, one day." ],
+        [ 6, "God said, “Let there be an expanse in the middle of the waters, and let it divide the waters from the waters.”" ],
+        [ 7, "God made the expanse, and divided the waters which were under the expanse from the waters which were above the expanse; and it was so." ],
+        [ 8, "God called the expanse “sky.” There was evening and there was morning, a second day." ],
+        [ 9, "God said, “Let the waters under the sky be gathered together to one place, and let the dry land appear;” and it was so." ],
+        [ 10, "God called the dry land “earth,” and the gathering together of the waters he called “seas.” God saw that it was good." ]
     ]
 }