Review: Needs Fixing
I think there's one instance you need to change. Please see my comments below
for more guidance.
Diff comments:
>
> === modified file 'tests/functional/openlp_plugins/bibles/test_bibleimport.py'
> --- tests/functional/openlp_plugins/bibles/test_bibleimport.py
> 2017-10-07 07:05:07 +0000
> +++ tests/functional/openlp_plugins/bibles/test_bibleimport.py
> 2017-10-11 06:59:39 +0000
> @@ -48,7 +48,7 @@
> b'
> <data><unsupported>Test<x>data</x><y>to</y>discard</unsupported></data>\n'
> b'</root>'
> )
> - self.open_patcher = patch('builtins.open')
> + self.open_patcher = patch.object(Path, 'open')
I don't see the import for this, which means it will fail. Also, you are not
actually using the Path object as-is in your tests, so this is mocked out in
the wrong place. You should be mocking it out where you import the object, not
where the object is declared.
self.open_patcher =
patch('openlp.plugins.bibles.lib.bibleimporter.Path.open')
> self.addCleanup(self.open_patcher.stop)
> self.mocked_open = self.open_patcher.start()
> self.critical_error_message_box_patcher = \
>
> === modified file
> 'tests/functional/openlp_plugins/bibles/test_wordprojectimport.py'
> --- tests/functional/openlp_plugins/bibles/test_wordprojectimport.py
> 2017-04-24 05:17:55 +0000
> +++ tests/functional/openlp_plugins/bibles/test_wordprojectimport.py
> 2017-10-11 06:59:39 +0000
> @@ -48,19 +49,17 @@
> self.addCleanup(self.manager_patcher.stop)
> self.manager_patcher.start()
>
> - @patch('openlp.plugins.bibles.lib.importers.wordproject.os')
> - @patch('openlp.plugins.bibles.lib.importers.wordproject.copen')
> - def test_process_books(self, mocked_open, mocked_os):
> + @patch.object(Path, 'read_text')
Presuming that the "read_text" method is attached to either the base_path or
the file_path objects, then this is correct. If it's not, you should be
patching it out where you import/use it.
> + def test_process_books(self, mocked_read_text):
> """
> Test the process_books() method
> """
> # GIVEN: A WordProject importer and a bunch of mocked things
> - importer = WordProjectBible(MagicMock(), path='.', name='.',
> filename='kj.zip')
> - importer.base_dir = ''
> + importer = WordProjectBible(MagicMock(), path='.', name='.',
> file_path=Path('kj.zip'))
> + importer.base_path = Path()
> importer.stop_import_flag = False
> importer.language_id = 'en'
> - mocked_open.return_value.__enter__.return_value.read.return_value =
> INDEX_PAGE
> - mocked_os.path.join.side_effect = lambda *x: ''.join(x)
> + mocked_read_text.return_value = INDEX_PAGE
>
> # WHEN: process_books() is called
> with patch.object(importer, 'find_and_create_book') as
> mocked_find_and_create_book, \
> @@ -69,26 +68,22 @@
> importer.process_books()
>
> # THEN: The right methods should have been called
> - mocked_os.path.join.assert_called_once_with('', 'index.htm')
> - mocked_open.assert_called_once_with('index.htm', encoding='utf-8',
> errors='ignore')
> + mocked_read_text.assert_called_once_with(encoding='utf-8',
> errors='ignore')
> assert mocked_find_and_create_book.call_count == 66, 'There should
> be 66 books'
> assert mocked_process_chapters.call_count == 66, 'There should be 66
> books'
> assert mocked_session.commit.call_count == 66, 'There should be 66
> books'
>
> - @patch('openlp.plugins.bibles.lib.importers.wordproject.os')
> - @patch('openlp.plugins.bibles.lib.importers.wordproject.copen')
> - def test_process_chapters(self, mocked_open, mocked_os):
> + @patch.object(Path, 'read_text')
Same as my previous comment.
> + def test_process_chapters(self, mocked_read_text):
> """
> Test the process_chapters() method
> """
> # GIVEN: A WordProject importer and a bunch of mocked things
> - importer = WordProjectBible(MagicMock(), path='.', name='.',
> filename='kj.zip')
> - importer.base_dir = ''
> + importer = WordProjectBible(MagicMock(), path='.', name='.',
> file_path=Path('kj.zip'))
> + importer.base_path = Path()
> importer.stop_import_flag = False
> importer.language_id = 'en'
> - mocked_open.return_value.__enter__.return_value.read.return_value =
> CHAPTER_PAGE
> - mocked_os.path.join.side_effect = lambda *x: ''.join(x)
> - mocked_os.path.normpath.side_effect = lambda x: x
> + mocked_read_text.return_value = CHAPTER_PAGE
> mocked_db_book = MagicMock()
> mocked_db_book.name = 'Genesis'
> book_id = 1
> @@ -102,24 +97,21 @@
> # THEN: The right methods should have been called
> expected_set_current_chapter_calls = [call('Genesis', ch) for ch in
> range(1, 51)]
> expected_process_verses_calls = [call(mocked_db_book, 1, ch) for ch
> in range(1, 51)]
> - mocked_os.path.join.assert_called_once_with('', '01/1.htm')
> - mocked_open.assert_called_once_with('01/1.htm', encoding='utf-8',
> errors='ignore')
> + mocked_read_text.assert_called_once_with(encoding='utf-8',
> errors='ignore')
> assert mocked_set_current_chapter.call_args_list ==
> expected_set_current_chapter_calls
> assert mocked_process_verses.call_args_list ==
> expected_process_verses_calls
>
> - @patch('openlp.plugins.bibles.lib.importers.wordproject.os')
> - @patch('openlp.plugins.bibles.lib.importers.wordproject.copen')
> - def test_process_verses(self, mocked_open, mocked_os):
> + @patch.object(Path, 'read_text')
Same as my previous comment.
> + def test_process_verses(self, mocked_read_text):
> """
> Test the process_verses() method
> """
> # GIVEN: A WordProject importer and a bunch of mocked things
> - importer = WordProjectBible(MagicMock(), path='.', name='.',
> filename='kj.zip')
> - importer.base_dir = ''
> + importer = WordProjectBible(MagicMock(), path='.', name='.',
> file_path=Path('kj.zip'))
> + importer.base_path = Path()
> importer.stop_import_flag = False
> importer.language_id = 'en'
> - mocked_open.return_value.__enter__.return_value.read.return_value =
> CHAPTER_PAGE
> - mocked_os.path.join.side_effect = lambda *x: '/'.join(x)
> + mocked_read_text.return_value = CHAPTER_PAGE
> mocked_db_book = MagicMock()
> mocked_db_book.name = 'Genesis'
> book_number = 1
--
https://code.launchpad.net/~phill-ridout/openlp/pathlib9/+merge/332092
Your team OpenLP Core is subscribed to branch lp:openlp.
_______________________________________________
Mailing list: https://launchpad.net/~openlp-core
Post to : openlp-core@lists.launchpad.net
Unsubscribe : https://launchpad.net/~openlp-core
More help : https://help.launchpad.net/ListHelp
