From 80cdc35433d76348a6fa9c2c7a25ff3788fa170f Mon Sep 17 00:00:00 2001
From: Philip Ridout <phill.ridout@gmail.com>
Date: Sun, 4 Sep 2016 17:15:57 +0100
Subject: [PATCH] Move already refactored imorters over to using the log
 methods in OpenLPMixin

---
 openlp/core/common/openlpmixin.py             |   6 +
 openlp/plugins/bibles/lib/bibleimport.py      |  15 +--
 .../plugins/bibles/lib/importers/csvbible.py  |   7 +-
 .../plugins/bibles/lib/importers/opensong.py  |  57 ++++-----
 openlp/plugins/bibles/lib/importers/osis.py   |   5 +-
 openlp/plugins/bibles/lib/manager.py          |   6 +-
 .../openlp_plugins/bibles/test_bibleimport.py |  85 ++++++--------
 .../openlp_plugins/bibles/test_csvimport.py   |  12 +-
 .../bibles/test_opensongimport.py             | 111 ++++++++++--------
 .../openlp_plugins/bibles/test_osisimport.py  |   9 +-
 10 files changed, 147 insertions(+), 166 deletions(-)

diff --git a/openlp/core/common/openlpmixin.py b/openlp/core/common/openlpmixin.py
index 94505b86b..2f1d5bbba 100644
--- a/openlp/core/common/openlpmixin.py
+++ b/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
diff --git a/openlp/plugins/bibles/lib/bibleimport.py b/openlp/plugins/bibles/lib/bibleimport.py
index 34a1f04b7..eaef01aed 100644
--- a/openlp/plugins/bibles/lib/bibleimport.py
+++ b/openlp/plugins/bibles/lib/bibleimport.py
@@ -20,18 +20,14 @@
 # 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, trace_error_handler, translate
+from openlp.core.common import OpenLPMixin, languages, translate
 from openlp.core.lib import ValidationError
 from openlp.core.lib.ui import critical_error_message_box
 from openlp.plugins.bibles.lib.db import BibleDB, BiblesResourcesDB
 
-log = logging.getLogger(__name__)
-
 
 class BibleImport(OpenLPMixin, BibleDB):
     """
@@ -78,8 +74,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
@@ -97,7 +93,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))
@@ -135,8 +131,7 @@ class BibleImport(OpenLPMixin, BibleDB):
                     etree.strip_tags(tree, tags)
                 return tree.getroot()
         except OSError as e:
-            log.exception('Opening {file_name} failed.'.format(file_name=e.filename))
-            trace_error_handler(log)
+            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}'
diff --git a/openlp/plugins/bibles/lib/importers/csvbible.py b/openlp/plugins/bibles/lib/importers/csvbible.py
index d0beedf58..d9edd6cc7 100644
--- a/openlp/plugins/bibles/lib/importers/csvbible.py
+++ b/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']
 
diff --git a/openlp/plugins/bibles/lib/importers/opensong.py b/openlp/plugins/bibles/lib/importers/opensong.py
index 5219a4d12..d75f37e03 100644
--- a/openlp/plugins/bibles/lib/importers/opensong.py
+++ b/openlp/plugins/bibles/lib/importers/opensong.py
@@ -20,18 +20,9 @@
 # Temple Place, Suite 330, Boston, MA 02111-1307 USA                          #
 ###############################################################################
 
-import logging
-from lxml import etree
-
-from openlp.core.common import trace_error_handler, translate
-from openlp.core.lib.exceptions import ValidationError
-from openlp.core.lib.ui import critical_error_message_box
 from openlp.plugins.bibles.lib.bibleimport import BibleImport
 
 
-log = logging.getLogger(__name__)
-
-
 def get_text(element):
     """
     Recursively get all text in an objectify element and its child elements.
@@ -62,32 +53,32 @@ def parse_chapter_number(number, previous_number):
     return previous_number + 1
 
 
-def parse_verse_number(number, previous_number):
-    """
-    Parse the verse number retrieved from the xml
-
-    :param number: The raw data from the xml
-    :param previous_number: The previous verse number
-    :return: Number of current verse. (Int)
-    """
-    if not number:
-        return previous_number + 1
-    try:
-        return int(number)
-    except ValueError:
-        verse_parts = number.split('-')
-        if len(verse_parts) > 1:
-            number = int(verse_parts[0])
-            return number
-    except TypeError:
-        log.warning('Illegal verse number: {verse_no}'.format(verse_no=str(number)))
-    return previous_number + 1
-
-
 class OpenSongBible(BibleImport):
     """
     OpenSong Bible format importer class. This class is used to import Bibles from OpenSong's XML format.
     """
+
+    def parse_verse_number(self, number, previous_number):
+        """
+        Parse the verse number retrieved from the xml
+
+        :param number: The raw data from the xml
+        :param previous_number: The previous verse number
+        :return: Number of current verse. (Int)
+        """
+        if not number:
+            return previous_number + 1
+        try:
+            return int(number)
+        except ValueError:
+            verse_parts = number.split('-')
+            if len(verse_parts) > 1:
+                number = int(verse_parts[0])
+                return number
+        except TypeError:
+            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
@@ -131,7 +122,7 @@ class OpenSongBible(BibleImport):
         for verse in verses:
             if self.stop_import_flag:
                 break
-            verse_number = parse_verse_number(verse.attrib['n'], verse_number)
+            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):
@@ -141,7 +132,7 @@ class OpenSongBible(BibleImport):
         :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))
+        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:
diff --git a/openlp/plugins/bibles/lib/importers/osis.py b/openlp/plugins/bibles/lib/importers/osis.py
index 549854804..3c799daa3 100644
--- a/openlp/plugins/bibles/lib/importers/osis.py
+++ b/openlp/plugins/bibles/lib/importers/osis.py
@@ -20,13 +20,10 @@
 # Temple Place, Suite 330, Boston, MA 02111-1307 USA                          #
 ###############################################################################
 
-import logging
 from lxml import etree
 
 from openlp.plugins.bibles.lib.bibleimport import BibleImport
 
-log = logging.getLogger(__name__)
-
 NS = {'ns': 'http://www.bibletechnologies.net/2003/OSIS/namespace'}
 # Tags we don't use and can remove the content
 REMOVABLE_ELEMENTS = (
@@ -162,7 +159,7 @@ class OSISBible(BibleImport):
         """
         Loads a Bible from file.
         """
-        log.debug('Starting OSIS import from "{name}"'.format(name=self.filename))
+        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:
diff --git a/openlp/plugins/bibles/lib/manager.py b/openlp/plugins/bibles/lib/manager.py
index d2286bed2..dbd9ae0dd 100644
--- a/openlp/plugins/bibles/lib/manager.py
+++ b/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.
     """
diff --git a/tests/functional/openlp_plugins/bibles/test_bibleimport.py b/tests/functional/openlp_plugins/bibles/test_bibleimport.py
index d293150b5..50baa655b 100644
--- a/tests/functional/openlp_plugins/bibles/test_bibleimport.py
+++ b/tests/functional/openlp_plugins/bibles/test_bibleimport.py
@@ -32,7 +32,7 @@ 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 ANY, MagicMock, patch
+from tests.functional import MagicMock, patch
 
 
 class TestBibleImport(TestCase):
@@ -51,9 +51,6 @@ class TestBibleImport(TestCase):
         self.open_patcher = patch('builtins.open')
         self.addCleanup(self.open_patcher.stop)
         self.mocked_open = self.open_patcher.start()
-        self.log_patcher = patch('openlp.plugins.bibles.lib.bibleimport.log')
-        self.addCleanup(self.log_patcher.stop)
-        self.mocked_log = self.log_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)
@@ -61,9 +58,6 @@ class TestBibleImport(TestCase):
         self.setup_patcher = patch('openlp.plugins.bibles.lib.db.BibleDB._setup')
         self.addCleanup(self.setup_patcher.stop)
         self.setup_patcher.start()
-        self.trace_error_handler_patcher = patch('openlp.plugins.bibles.lib.bibleimport.trace_error_handler')
-        self.addCleanup(self.trace_error_handler_patcher.stop)
-        self.mocked_trace_error_handler = self.trace_error_handler_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)
@@ -101,7 +95,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()
 
@@ -120,7 +114,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=None) as mocked_languages_get_language, \
-                patch('openlp.plugins.bibles.lib.db.BibleDB.get_language', return_value=20) as mocked_db_get_language:
+                patch.object(BibleImport, 'get_language', return_value=20) as mocked_db_get_language:
             instance = BibleImport(MagicMock())
             instance.save_meta = MagicMock()
 
@@ -140,8 +134,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.mocked_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()
 
@@ -151,7 +145,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.mocked_log.error.called)
+            self.assertFalse(mocked_log_error.error.called)
             instance.save_meta.assert_called_once_with('language_id', 40)
             self.assertEqual(result, 40)
 
@@ -162,8 +156,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.mocked_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()
 
@@ -173,7 +167,7 @@ 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.mocked_log.error.assert_called_once_with(
+            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)
@@ -281,7 +275,6 @@ class TestBibleImport(TestCase):
 
         # THEN: parse_xml should have caught the error, informed the user and returned None
         self.mocked_log.exception.assert_called_once_with('Opening file.tst failed.')
-        self.mocked_trace_error_handler.assert_called_once_with(self.mocked_log)
         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')
@@ -291,45 +284,45 @@ class TestBibleImport(TestCase):
         """
         Test that parse_xml handles a FileNotFoundError exception correctly
         """
-        # 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='')
+        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')
+            # WHEN: Calling parse_xml
+            result = importer.parse_xml('file.tst')
 
-        # THEN: parse_xml should have caught the error, informed the user and returned None
-        self.mocked_log.exception.assert_called_once_with('Opening file.tst failed.')
-        self.mocked_trace_error_handler.assert_called_once_with(self.mocked_log)
-        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)
+            # 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
         """
-        # 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='')
+        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')
+            # WHEN: Calling parse_xml
+            result = importer.parse_xml('file.tst')
 
-        # THEN: parse_xml should have caught the error, informed the user and returned None
-        self.mocked_log.exception.assert_called_once_with('Opening file.tst failed.')
-        self.mocked_trace_error_handler.assert_called_once_with(self.mocked_log)
-        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)
+            # 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 validate_xml_file_compressed_file_test(self):
         """
diff --git a/tests/functional/openlp_plugins/bibles/test_csvimport.py b/tests/functional/openlp_plugins/bibles/test_csvimport.py
index e72c85de6..a4a19279e 100644
--- a/tests/functional/openlp_plugins/bibles/test_csvimport.py
+++ b/tests/functional/openlp_plugins/bibles/test_csvimport.py
@@ -281,16 +281,14 @@ 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')
             self.assertFalse(result)
 
@@ -300,8 +298,7 @@ class TestCSVImport(TestCase):
         """
         # 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']])
@@ -314,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'])
diff --git a/tests/functional/openlp_plugins/bibles/test_opensongimport.py b/tests/functional/openlp_plugins/bibles/test_opensongimport.py
index c902440b0..68ebdd37c 100644
--- a/tests/functional/openlp_plugins/bibles/test_opensongimport.py
+++ b/tests/functional/openlp_plugins/bibles/test_opensongimport.py
@@ -27,14 +27,12 @@ import json
 import os
 from unittest import TestCase
 
-from lxml import etree, objectify
+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.core.lib.exceptions import ValidationError
-from openlp.plugins.bibles.lib.importers.opensong import OpenSongBible, get_text, parse_chapter_number,\
-    parse_verse_number
+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__),
@@ -127,9 +125,11 @@ class TestOpenSongImport(TestCase, TestMixin):
         """
         Test parse_verse_number when supplied with a valid verse number
         """
-        # GIVEN: The number 15 represented as a string and an instance of OpenSongBible
+        # 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 = parse_verse_number('15', 0)
+        result = importer.parse_verse_number('15', 0)
 
         # THEN: parse_verse_number should return the verse number
         self.assertEqual(result, 15)
@@ -138,9 +138,11 @@ class TestOpenSongImport(TestCase, TestMixin):
         """
         Test parse_verse_number when supplied with a verse range
         """
-        # GIVEN: The range 24-26 represented as a string
+        # 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 = parse_verse_number('24-26', 0)
+        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)
@@ -149,9 +151,11 @@ class TestOpenSongImport(TestCase, TestMixin):
         """
         Test parse_verse_number when supplied with a invalid verse number
         """
-        # GIVEN: An non numeric string represented as a string
+        # 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 = parse_verse_number('invalid', 41)
+        result = importer.parse_verse_number('invalid', 41)
 
         # THEN: parse_verse_number should increment the previous verse number
         self.assertEqual(result, 42)
@@ -160,25 +164,29 @@ class TestOpenSongImport(TestCase, TestMixin):
         """
         Test parse_verse_number when the verse number is an empty string. (Bug #1074727)
         """
-        # GIVEN: An empty string, and the previous verse number set as 14
+        # 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 = parse_verse_number('', 14)
+        result = importer.parse_verse_number('', 14)
 
         # THEN: parse_verse_number should increment the previous verse number
         self.assertEqual(result, 15)
 
-    @patch('openlp.plugins.bibles.lib.importers.opensong.log')
-    def parse_verse_number_invalid_type_test(self, mocked_log):
+    def parse_verse_number_invalid_type_test(self):
         """
         Test parse_verse_number when the verse number is an invalid type)
         """
-        # GIVEN: A mocked out log, a Tuple, and the previous verse number set as 12
-        # WHEN: Calling parse_verse_number
-        result = parse_verse_number((1, 2, 3), 12)
+        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='')
 
-        # 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)
+            # 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):
         """
@@ -238,9 +246,8 @@ class TestOpenSongImport(TestCase, TestMixin):
         # THEN: importer.parse_chapter_number not have been called
         self.assertFalse(importer.parse_chapter_number.called)
 
-    @patch('openlp.plugins.bibles.lib.importers.opensong.translate', **{'side_effect': lambda x, y: y})
     @patch('openlp.plugins.bibles.lib.importers.opensong.parse_chapter_number', **{'side_effect': [1, 2]})
-    def process_chapters_completes_test(self, mocked_parse_chapter_number, mocked_translate):
+    def process_chapters_completes_test(self, mocked_parse_chapter_number):
         """
         Test process_chapters when it completes
         """
@@ -287,45 +294,47 @@ class TestOpenSongImport(TestCase, TestMixin):
         # THEN: importer.parse_verse_number not have been called
         self.assertFalse(importer.parse_verse_number.called)
 
-    @patch('openlp.plugins.bibles.lib.importers.opensong.parse_verse_number', **{'side_effect': [1, 2]})
-    @patch('openlp.plugins.bibles.lib.importers.opensong.get_text', **{'side_effect': ['Verse1 Text', 'Verse2 Text']})
-    def process_verses_completes_test(self, mocked_get_text, mocked_parse_verse_number):
+    def process_verses_completes_test(self):
         """
         Test process_verses when it completes
         """
-        # GIVEN: An instance of OpenSongBible
-        importer = OpenSongBible(MagicMock(), path='.', name='.', filename='')
-        importer.wizard = MagicMock()
+        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']
+            # 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])
+            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')])
+            # 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('openlp.plugins.bibles.lib.importers.opensong.log'), \
+        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:
@@ -343,7 +352,7 @@ class TestOpenSongImport(TestCase, TestMixin):
         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('openlp.plugins.bibles.lib.importers.opensong.log'), \
+        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), \
@@ -362,7 +371,7 @@ class TestOpenSongImport(TestCase, TestMixin):
         Test do_import when it completes successfully
         """
         # GIVEN: An instance of OpenSongBible
-        with patch('openlp.plugins.bibles.lib.importers.opensong.log'), \
+        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), \
diff --git a/tests/functional/openlp_plugins/bibles/test_osisimport.py b/tests/functional/openlp_plugins/bibles/test_osisimport.py
index 6cf157711..dd0f661a0 100644
--- a/tests/functional/openlp_plugins/bibles/test_osisimport.py
+++ b/tests/functional/openlp_plugins/bibles/test_osisimport.py
@@ -32,8 +32,7 @@ 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):
@@ -354,7 +353,7 @@ class TestOsisImport(TestCase):
         Test do_import when parse_xml fails (returns None)
         """
         # GIVEN: An instance of OpenSongBible and a mocked parse_xml which returns False
-        with patch('openlp.plugins.bibles.lib.importers.opensong.log'), \
+        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:
@@ -372,7 +371,7 @@ class TestOsisImport(TestCase):
         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('openlp.plugins.bibles.lib.importers.opensong.log'), \
+        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}), \
@@ -391,7 +390,7 @@ class TestOsisImport(TestCase):
         Test do_import when it completes successfully
         """
         # GIVEN: An instance of OpenSongBible
-        with patch('openlp.plugins.bibles.lib.importers.opensong.log'), \
+        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}), \