From 13eb461f5b9a167bc118b27ecbf0555ec07e1c2a Mon Sep 17 00:00:00 2001 From: Raoul Snyman Date: Fri, 30 Sep 2016 12:01:10 +0200 Subject: [PATCH] Fix bug #1629079 and add some more tests --- openlp/plugins/songs/lib/songselect.py | 4 +- .../openlp_plugins/songs/test_songselect.py | 69 +++++++++++++++++++ 2 files changed, 71 insertions(+), 2 deletions(-) diff --git a/openlp/plugins/songs/lib/songselect.py b/openlp/plugins/songs/lib/songselect.py index eb9bdede4..9c4aab21a 100644 --- a/openlp/plugins/songs/lib/songselect.py +++ b/openlp/plugins/songs/lib/songselect.py @@ -114,7 +114,7 @@ class SongSelectImport(object): try: self.opener.open(LOGOUT_URL) except (TypeError, URLError) as error: - log.exception('Could not log of SongSelect, %s', error) + log.exception('Could not log out of SongSelect, %s', error) def search(self, search_text, max_results, callback=None): """ @@ -255,7 +255,7 @@ class SongSelectImport(object): topic = self.db_manager.get_object_filtered(Topic, Topic.name == topic_name) if not topic: topic = Topic.populate(name=topic_name) - db_song.add_topic(topic) + db_song.topics.append(topic) self.db_manager.save_object(db_song) return db_song diff --git a/tests/functional/openlp_plugins/songs/test_songselect.py b/tests/functional/openlp_plugins/songs/test_songselect.py index f0fcbdf51..149ec5d65 100644 --- a/tests/functional/openlp_plugins/songs/test_songselect.py +++ b/tests/functional/openlp_plugins/songs/test_songselect.py @@ -61,6 +61,30 @@ class TestSongSelectImport(TestCase, TestMixin): self.assertIsNotNone(importer.opener, 'There should be a valid opener object') self.assertEqual(1, mocked_build_opener.call_count, 'The build_opener method should have been called once') + @patch('openlp.plugins.songs.lib.songselect.build_opener') + @patch('openlp.plugins.songs.lib.songselect.BeautifulSoup') + def login_fails_type_error_test(self, MockedBeautifulSoup, mocked_build_opener): + """ + Test that when logging in to SongSelect fails due to a TypeError, the login method returns False + """ + # GIVEN: A bunch of mocked out stuff and an importer object + mocked_opener = MagicMock() + mocked_build_opener.return_value = mocked_opener + mocked_login_page = MagicMock() + mocked_login_page.find.side_effect = [{'value': 'blah'}, None] + MockedBeautifulSoup.side_effect = [mocked_login_page, TypeError('wrong type')] + mock_callback = MagicMock() + importer = SongSelectImport(None) + + # WHEN: The login method is called after being rigged to fail + result = importer.login('username', 'password', mock_callback) + + # THEN: callback was called 3 times, open was called twice, find was called twice, and False was returned + self.assertEqual(2, mock_callback.call_count, 'callback should have been called 3 times') + self.assertEqual(1, mocked_login_page.find.call_count, 'find should have been called twice') + self.assertEqual(2, mocked_opener.open.call_count, 'opener should have been called twice') + self.assertFalse(result, 'The login method should have returned False') + @patch('openlp.plugins.songs.lib.songselect.build_opener') @patch('openlp.plugins.songs.lib.songselect.BeautifulSoup') def login_fails_test(self, MockedBeautifulSoup, mocked_build_opener): @@ -143,6 +167,27 @@ class TestSongSelectImport(TestCase, TestMixin): self.assertEqual(1, mocked_opener.open.call_count, 'opener should have been called once') mocked_opener.open.assert_called_with(LOGOUT_URL) + @patch('openlp.plugins.songs.lib.songselect.build_opener') + @patch('openlp.plugins.songs.lib.songselect.log') + def logout_fails_test(self, mocked_log, mocked_build_opener): + """ + Test that when the logout method is called, it logs the user out of SongSelect + """ + # GIVEN: A bunch of mocked out stuff and an importer object + type_error = TypeError('wrong type') + mocked_opener = MagicMock() + mocked_opener.open.side_effect = type_error + mocked_build_opener.return_value = mocked_opener + importer = SongSelectImport(None) + + # WHEN: The login method is called after being rigged to fail + importer.logout() + + # THEN: The opener is called once with the logout url + self.assertEqual(1, mocked_opener.open.call_count, 'opener should have been called once') + mocked_opener.open.assert_called_with(LOGOUT_URL) + mocked_log.exception.assert_called_once_with('Could not log out of SongSelect, %s', type_error) + @patch('openlp.plugins.songs.lib.songselect.build_opener') @patch('openlp.plugins.songs.lib.songselect.BeautifulSoup') def search_returns_no_results_test(self, MockedBeautifulSoup, mocked_build_opener): @@ -168,6 +213,30 @@ class TestSongSelectImport(TestCase, TestMixin): mocked_results_page.find_all.assert_called_with('div', 'song-result') self.assertEqual([], results, 'The search method should have returned an empty list') + @patch('openlp.plugins.songs.lib.songselect.build_opener') + @patch('openlp.plugins.songs.lib.songselect.BeautifulSoup') + def search_returns_no_results_after_exception_test(self, MockedBeautifulSoup, mocked_build_opener): + """ + Test that when the search finds no results, it simply returns an empty list + """ + # GIVEN: A bunch of mocked out stuff and an importer object + mocked_opener = MagicMock() + mocked_build_opener.return_value = mocked_opener + mocked_results_page = MagicMock() + mocked_results_page.find_all.return_value = [] + MockedBeautifulSoup.side_effect = TypeError('wrong type') + mock_callback = MagicMock() + importer = SongSelectImport(None) + + # WHEN: The login method is called after being rigged to fail + results = importer.search('text', 1000, mock_callback) + + # THEN: callback was never called, open was called once, find_all was called once, an empty list returned + self.assertEqual(0, mock_callback.call_count, 'callback should not have been called') + self.assertEqual(1, mocked_opener.open.call_count, 'open should have been called once') + self.assertEqual(0, mocked_results_page.find_all.call_count, 'find_all should not have been called') + self.assertEqual([], results, 'The search method should have returned an empty list') + @patch('openlp.plugins.songs.lib.songselect.build_opener') @patch('openlp.plugins.songs.lib.songselect.BeautifulSoup') def search_returns_two_results_test(self, MockedBeautifulSoup, mocked_build_opener):