From 879a51a1fe12d7c5e0f70f5e7e800f4360afc311 Mon Sep 17 00:00:00 2001 From: Raoul Snyman Date: Sat, 13 Aug 2016 16:29:12 +0200 Subject: [PATCH 1/3] Fix the tests for SongSelect and write tests for the SystemPlayer --- .bzrignore | 1 + openlp/core/ui/media/systemplayer.py | 22 +- openlp/plugins/songs/lib/__init__.py | 17 +- openlp/plugins/songs/lib/songselect.py | 115 ++-- .../openlp_core_ui/test_slidecontroller.py | 4 +- .../openlp_core_ui_media/test_systemplayer.py | 528 ++++++++++++++++++ .../openlp_plugins/songs/test_songselect.py | 72 ++- 7 files changed, 668 insertions(+), 91 deletions(-) create mode 100644 tests/functional/openlp_core_ui_media/test_systemplayer.py diff --git a/.bzrignore b/.bzrignore index 97af7bea6..2c7f64680 100644 --- a/.bzrignore +++ b/.bzrignore @@ -46,3 +46,4 @@ cover coverage tags output +htmlcov diff --git a/openlp/core/ui/media/systemplayer.py b/openlp/core/ui/media/systemplayer.py index ad1907044..a36ce445b 100644 --- a/openlp/core/ui/media/systemplayer.py +++ b/openlp/core/ui/media/systemplayer.py @@ -83,17 +83,17 @@ class SystemPlayer(MediaPlayer): elif mime_type.startswith('video/'): self._add_to_list(self.video_extensions_list, mime_type) - def _add_to_list(self, mime_type_list, mimetype): + def _add_to_list(self, mime_type_list, mime_type): """ Add mimetypes to the provided list """ # Add all extensions which mimetypes provides us for supported types. - extensions = mimetypes.guess_all_extensions(str(mimetype)) + extensions = mimetypes.guess_all_extensions(mime_type) for extension in extensions: ext = '*%s' % extension if ext not in mime_type_list: mime_type_list.append(ext) - log.info('MediaPlugin: %s extensions: %s' % (mimetype, ' '.join(extensions))) + log.info('MediaPlugin: %s extensions: %s', mime_type, ' '.join(extensions)) def setup(self, display): """ @@ -284,25 +284,25 @@ class SystemPlayer(MediaPlayer): :return: True if file can be played otherwise False """ thread = QtCore.QThread() - check_media_player = CheckMedia(path) - check_media_player.setVolume(0) - check_media_player.moveToThread(thread) - check_media_player.finished.connect(thread.quit) - thread.started.connect(check_media_player.play) + check_media_worker = CheckMediaWorker(path) + check_media_worker.setVolume(0) + check_media_worker.moveToThread(thread) + check_media_worker.finished.connect(thread.quit) + thread.started.connect(check_media_worker.play) thread.start() while thread.isRunning(): self.application.processEvents() - return check_media_player.result + return check_media_worker.result -class CheckMedia(QtMultimedia.QMediaPlayer): +class CheckMediaWorker(QtMultimedia.QMediaPlayer): """ Class used to check if a media file is playable """ finished = QtCore.pyqtSignal() def __init__(self, path): - super(CheckMedia, self).__init__(None, QtMultimedia.QMediaPlayer.VideoSurface) + super(CheckMediaWorker, self).__init__(None, QtMultimedia.QMediaPlayer.VideoSurface) self.result = None self.error.connect(functools.partial(self.signals, 'error')) diff --git a/openlp/plugins/songs/lib/__init__.py b/openlp/plugins/songs/lib/__init__.py index 4d8586204..662172374 100644 --- a/openlp/plugins/songs/lib/__init__.py +++ b/openlp/plugins/songs/lib/__init__.py @@ -31,9 +31,8 @@ from PyQt5 import QtWidgets from openlp.core.common import AppLocation, CONTROL_CHARS from openlp.core.lib import translate -from openlp.plugins.songs.lib.db import MediaFile, Song -from .db import Author -from .ui import SongStrings +from openlp.plugins.songs.lib.db import Author, MediaFile, Song, Topic +from openlp.plugins.songs.lib.ui import SongStrings log = logging.getLogger(__name__) @@ -315,8 +314,8 @@ def retrieve_windows_encoding(recommendation=None): ] recommended_index = -1 if recommendation: - for index in range(len(encodings)): - if recommendation == encodings[index][0]: + for index, encoding in enumerate(encodings): + if recommendation == encoding[0]: recommended_index = index break if recommended_index > -1: @@ -442,7 +441,7 @@ def strip_rtf(text, default_encoding=None): # Encoded buffer. ebytes = bytearray() for match in PATTERN.finditer(text): - iinu, word, arg, hex, char, brace, tchar = match.groups() + iinu, word, arg, hex_, char, brace, tchar = match.groups() # \x (non-alpha character) if char: if char in '\\{}': @@ -450,7 +449,7 @@ def strip_rtf(text, default_encoding=None): else: word = char # Flush encoded buffer to output buffer - if ebytes and not hex and not tchar: + if ebytes and not hex_ and not tchar: failed = False while True: try: @@ -507,11 +506,11 @@ def strip_rtf(text, default_encoding=None): elif iinu: ignorable = True # \'xx - elif hex: + elif hex_: if curskip > 0: curskip -= 1 elif not ignorable: - ebytes.append(int(hex, 16)) + ebytes.append(int(hex_, 16)) elif tchar: if curskip > 0: curskip -= 1 diff --git a/openlp/plugins/songs/lib/songselect.py b/openlp/plugins/songs/lib/songselect.py index 60f4383d2..84d6d7b90 100644 --- a/openlp/plugins/songs/lib/songselect.py +++ b/openlp/plugins/songs/lib/songselect.py @@ -23,7 +23,8 @@ The :mod:`~openlp.plugins.songs.lib.songselect` module contains the SongSelect importer itself. """ import logging -import sys +import random +import re from http.cookiejar import CookieJar from urllib.parse import urlencode from urllib.request import HTTPCookieProcessor, URLError, build_opener @@ -32,14 +33,20 @@ from html import unescape from bs4 import BeautifulSoup, NavigableString -from openlp.plugins.songs.lib import Song, VerseType, clean_song, Author +from openlp.plugins.songs.lib import Song, Author, Topic, VerseType, clean_song from openlp.plugins.songs.lib.openlyricsxml import SongXML -USER_AGENT = 'Mozilla/5.0 (Linux; U; Android 4.0.3; en-us; GT-I9000 ' \ - 'Build/IML74K) AppleWebKit/534.30 (KHTML, like Gecko) Version/4.0 ' \ - 'Mobile Safari/534.30' -BASE_URL = 'https://mobile.songselect.com' -LOGIN_URL = BASE_URL + '/account/login' +USER_AGENTS = [ + 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/52.0.2743.116 Safari/537.36' + 'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/52.0.2743.82 Safari/537.36', + 'Mozilla/5.0 (X11; Linux x86_64; rv:47.0) Gecko/20100101 Firefox/47.0', + 'Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:46.0) Gecko/20100101 Firefox/46.0', + 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:47.0) Gecko/20100101 Firefox/47.0' +] +BASE_URL = 'https://songselect.ccli.com' +LOGIN_PAGE = 'https://profile.ccli.com/account/signin?appContext=SongSelect&returnUrl='\ + 'https%3a%2f%2fsongselect.ccli.com%2f' +LOGIN_URL = 'https://profile.ccli.com/' LOGOUT_URL = BASE_URL + '/account/logout' SEARCH_URL = BASE_URL + '/search/results' @@ -60,7 +67,7 @@ class SongSelectImport(object): self.db_manager = db_manager self.html_parser = HTMLParser() self.opener = build_opener(HTTPCookieProcessor(CookieJar())) - self.opener.addheaders = [('User-Agent', USER_AGENT)] + self.opener.addheaders = [('User-Agent', random.choice(USER_AGENTS))] self.run_search = True def login(self, username, password, callback=None): @@ -76,27 +83,27 @@ class SongSelectImport(object): if callback: callback() try: - login_page = BeautifulSoup(self.opener.open(LOGIN_URL).read(), 'lxml') - except (TypeError, URLError) as e: - log.exception('Could not login to SongSelect, {error}'.format(error=e)) + login_page = BeautifulSoup(self.opener.open(LOGIN_PAGE).read(), 'lxml') + except (TypeError, URLError) as error: + log.exception('Could not login to SongSelect, {error}'.format(error=error)) return False if callback: callback() token_input = login_page.find('input', attrs={'name': '__RequestVerificationToken'}) data = urlencode({ '__RequestVerificationToken': token_input['value'], - 'UserName': username, - 'Password': password, + 'emailAddress': username, + 'password': password, 'RememberMe': 'false' }) try: posted_page = BeautifulSoup(self.opener.open(LOGIN_URL, data.encode('utf-8')).read(), 'lxml') - except (TypeError, URLError) as e: - log.exception('Could not login to SongSelect, {error}'.format(error=e)) + except (TypeError, URLError) as error: + log.exception('Could not login to SongSelect, {error}'.format(error=error)) return False if callback: callback() - return not posted_page.find('input', attrs={'name': '__RequestVerificationToken'}) + return posted_page.find('input', id='SearchText') is not None def logout(self): """ @@ -104,8 +111,8 @@ class SongSelectImport(object): """ try: self.opener.open(LOGOUT_URL) - except (TypeError, URLError) as e: - log.exception('Could not log of SongSelect, {error}'.format(error=e)) + except (TypeError, URLError) as error: + log.exception('Could not log of SongSelect, {error}'.format(error=error)) def search(self, search_text, max_results, callback=None): """ @@ -117,7 +124,15 @@ class SongSelectImport(object): :return: List of songs """ self.run_search = True - params = {'allowredirect': 'false', 'SearchTerm': search_text} + params = { + 'SongContent': '', + 'PrimaryLanguage': '', + 'Keys': '', + 'Themes': '', + 'List': '', + 'Sort': '', + 'SearchText': search_text + } current_page = 1 songs = [] while self.run_search: @@ -125,17 +140,17 @@ class SongSelectImport(object): params['page'] = current_page try: results_page = BeautifulSoup(self.opener.open(SEARCH_URL + '?' + urlencode(params)).read(), 'lxml') - search_results = results_page.find_all('li', 'result pane') - except (TypeError, URLError) as e: - log.exception('Could not search SongSelect, {error}'.format(error=e)) + search_results = results_page.find_all('div', 'song-result') + except (TypeError, URLError) as error: + log.exception('Could not search SongSelect, {error}'.format(error=error)) search_results = None if not search_results: break for result in search_results: song = { - 'title': unescape(result.find('h3').string), - 'authors': [unescape(author.string) for author in result.find_all('li')], - 'link': BASE_URL + result.find('a')['href'] + 'title': unescape(result.find('p', 'song-result-title').find('a').string).strip(), + 'authors': unescape(result.find('p', 'song-result-subtitle').string).strip().split(', '), + 'link': BASE_URL + result.find('p', 'song-result-title').find('a')['href'] } if callback: callback(song) @@ -157,33 +172,42 @@ class SongSelectImport(object): callback() try: song_page = BeautifulSoup(self.opener.open(song['link']).read(), 'lxml') - except (TypeError, URLError) as e: - log.exception('Could not get song from SongSelect, {error}'.format(error=e)) + except (TypeError, URLError) as error: + log.exception('Could not get song from SongSelect, {error}'.format(error=error)) return None if callback: callback() try: - lyrics_page = BeautifulSoup(self.opener.open(song['link'] + '/lyrics').read(), 'lxml') + lyrics_page = BeautifulSoup(self.opener.open(song['link'] + '/viewlyrics').read(), 'lxml') except (TypeError, URLError): log.exception('Could not get lyrics from SongSelect') return None if callback: callback() - song['copyright'] = '/'.join([li.string for li in song_page.find('ul', 'copyright').find_all('li')]) - song['copyright'] = unescape(song['copyright']) - song['ccli_number'] = song_page.find('ul', 'info').find('li').string.split(':')[1].strip() + copyright_elements = [] + theme_elements = [] + copyrights_regex = re.compile(r'\bCopyrights\b') + themes_regex = re.compile(r'\bThemes\b') + for ul in song_page.find_all('ul', 'song-meta-list'): + if ul.find('li', string=copyrights_regex): + copyright_elements.extend(ul.find_all('li')[1:]) + if ul.find('li', string=themes_regex): + theme_elements.extend(ul.find_all('li')[1:]) + song['copyright'] = '/'.join([unescape(li.string).strip() for li in copyright_elements]) + song['topics'] = [unescape(li.string).strip() for li in theme_elements] + song['ccli_number'] = song_page.find('div', 'song-content-data').find('ul').find('li').find('strong').string.strip() song['verses'] = [] - verses = lyrics_page.find('section', 'lyrics').find_all('p') - verse_labels = lyrics_page.find('section', 'lyrics').find_all('h3') - for counter in range(len(verses)): - verse = {'label': verse_labels[counter].string, 'lyrics': ''} - for v in verses[counter].contents: + verses = lyrics_page.find('div', 'song-viewer lyrics').find_all('p') + verse_labels = lyrics_page.find('div', 'song-viewer lyrics').find_all('h3') + for verse, label in zip(verses, verse_labels): + song_verse = {'label': unescape(label.string).strip(), 'lyrics': ''} + for v in verse.contents: if isinstance(v, NavigableString): - verse['lyrics'] = verse['lyrics'] + v.string + song_verse['lyrics'] += unescape(v.string).strip() else: - verse['lyrics'] += '\n' - verse['lyrics'] = verse['lyrics'].strip(' \n\r\t') - song['verses'].append(unescape(verse)) + song_verse['lyrics'] += '\n' + song_verse['lyrics'] = song_verse['lyrics'].strip(' \n\r\t') + song['verses'].append(song_verse) for counter, author in enumerate(song['authors']): song['authors'][counter] = unescape(author) return song @@ -199,7 +223,11 @@ class SongSelectImport(object): song_xml = SongXML() verse_order = [] for verse in song['verses']: - verse_type, verse_number = verse['label'].split(' ')[:2] + if ' ' in verse['label']: + verse_type, verse_number = verse['label'].split(' ', 1) + else: + verse_type = verse['label'] + verse_number = 1 verse_type = VerseType.from_loose_input(verse_type) verse_number = int(verse_number) song_xml.add_verse_to_lyrics(VerseType.tags[verse_type], verse_number, verse['lyrics']) @@ -220,6 +248,11 @@ class SongSelectImport(object): last_name = name_parts[1] author = Author.populate(first_name=first_name, last_name=last_name, display_name=author_name) db_song.add_author(author) + for topic_name in song.get('topics', []): + topic = self.db_manager.get_object_filtered(Topic, Topic.name == topic_name) + if not topic: + topic = Topic.populate(name=topic_name) + db_song.topics.append(topic) self.db_manager.save_object(db_song) return db_song diff --git a/tests/functional/openlp_core_ui/test_slidecontroller.py b/tests/functional/openlp_core_ui/test_slidecontroller.py index 01f895daa..ef1ce5793 100644 --- a/tests/functional/openlp_core_ui/test_slidecontroller.py +++ b/tests/functional/openlp_core_ui/test_slidecontroller.py @@ -243,7 +243,7 @@ class TestSlideController(TestCase): mocked_service_item = MagicMock() mocked_service_item.from_service = False mocked_preview_widget.current_slide_number.return_value = 1 - mocked_preview_widget.slide_count.return_value = 2 + mocked_preview_widget.slide_count = MagicMock(return_value=2) mocked_live_controller.preview_widget = MagicMock() Registry.create() Registry().register('live_controller', mocked_live_controller) @@ -273,7 +273,7 @@ class TestSlideController(TestCase): mocked_service_item.from_service = True mocked_service_item.unique_identifier = 42 mocked_preview_widget.current_slide_number.return_value = 1 - mocked_preview_widget.slide_count.return_value = 2 + mocked_preview_widget.slide_count = MagicMock(return_value=2) mocked_live_controller.preview_widget = MagicMock() Registry.create() Registry().register('live_controller', mocked_live_controller) diff --git a/tests/functional/openlp_core_ui_media/test_systemplayer.py b/tests/functional/openlp_core_ui_media/test_systemplayer.py new file mode 100644 index 000000000..357e91e44 --- /dev/null +++ b/tests/functional/openlp_core_ui_media/test_systemplayer.py @@ -0,0 +1,528 @@ +# -*- 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 # +############################################################################### +""" +Package to test the openlp.core.ui.media.systemplayer package. +""" +from unittest import TestCase + +from PyQt5 import QtCore, QtMultimedia + +from openlp.core.common import Registry +from openlp.core.ui.media import MediaState +from openlp.core.ui.media.systemplayer import SystemPlayer, CheckMediaWorker, ADDITIONAL_EXT + +from tests.functional import MagicMock, call, patch + +class TestSystemPlayer(TestCase): + """ + Test the system media player + """ + @patch('openlp.core.ui.media.systemplayer.mimetypes') + @patch('openlp.core.ui.media.systemplayer.QtMultimedia.QMediaPlayer') + def test_constructor(self, MockQMediaPlayer, mocked_mimetypes): + """ + Test the SystemPlayer constructor + """ + # GIVEN: The SystemPlayer class and a mockedQMediaPlayer + mocked_media_player = MagicMock() + mocked_media_player.supportedMimeTypes.return_value = [ + 'application/postscript', + 'audio/aiff', + 'audio/x-aiff', + 'text/html', + 'video/animaflex', + 'video/x-ms-asf' + ] + mocked_mimetypes.guess_all_extensions.side_effect = [ + ['.aiff'], + ['.aiff'], + ['.afl'], + ['.asf'] + ] + MockQMediaPlayer.return_value = mocked_media_player + + # WHEN: An object is created from it + player = SystemPlayer(self) + + # THEN: The correct initial values should be set up + self.assertEqual('system', player.name) + self.assertEqual('System', player.original_name) + self.assertEqual('&System', player.display_name) + self.assertEqual(self, player.parent) + self.assertEqual(ADDITIONAL_EXT, player.additional_extensions) + MockQMediaPlayer.assert_called_once_with(None, QtMultimedia.QMediaPlayer.VideoSurface) + mocked_mimetypes.init.assert_called_once_with() + mocked_media_player.service.assert_called_once_with() + mocked_media_player.supportedMimeTypes.assert_called_once_with() + self.assertEqual(['*.aiff'], player.audio_extensions_list) + self.assertEqual(['*.afl', '*.asf'], player.video_extensions_list) + + @patch('openlp.core.ui.media.systemplayer.QtMultimediaWidgets.QVideoWidget') + @patch('openlp.core.ui.media.systemplayer.QtMultimedia.QMediaPlayer') + def test_setup(self, MockQMediaPlayer, MockQVideoWidget): + """ + Test the setup() method of SystemPlayer + """ + # GIVEN: A SystemPlayer instance and a mock display + player = SystemPlayer(self) + mocked_display = MagicMock() + mocked_display.size.return_value = [1, 2, 3, 4] + mocked_video_widget = MagicMock() + mocked_media_player = MagicMock() + MockQVideoWidget.return_value = mocked_video_widget + MockQMediaPlayer.return_value = mocked_media_player + + # WHEN: setup() is run + player.setup(mocked_display) + + # THEN: The player should have a display widget + MockQVideoWidget.assert_called_once_with(mocked_display) + self.assertEqual(mocked_video_widget, mocked_display.video_widget) + mocked_display.size.assert_called_once_with() + mocked_video_widget.resize.assert_called_once_with([1, 2, 3, 4]) + MockQMediaPlayer.assert_called_with(mocked_display) + self.assertEqual(mocked_media_player, mocked_display.media_player) + mocked_media_player.setVideoOutput.assert_called_once_with(mocked_video_widget) + mocked_video_widget.raise_.assert_called_once_with() + mocked_video_widget.hide.assert_called_once_with() + self.assertTrue(player.has_own_widget) + + def test_check_available(self): + """ + Test the check_available() method on SystemPlayer + """ + # GIVEN: A SystemPlayer instance + player = SystemPlayer(self) + + # WHEN: check_available is run + result = player.check_available() + + # THEN: it should be available + self.assertTrue(result) + + def test_load_valid_media(self): + """ + Test the load() method of SystemPlayer with a valid media file + """ + # GIVEN: A SystemPlayer instance and a mocked display + player = SystemPlayer(self) + mocked_display = MagicMock() + mocked_display.controller.media_info.volume = 1 + mocked_display.controller.media_info.file_info.absoluteFilePath.return_value = '/path/to/file' + + # WHEN: The load() method is run + with patch.object(player, 'check_media') as mocked_check_media, \ + patch.object(player, 'volume') as mocked_volume: + mocked_check_media.return_value = True + result = player.load(mocked_display) + + # THEN: the file is sent to the video widget + mocked_display.controller.media_info.file_info.absoluteFilePath.assert_called_once_with() + mocked_check_media.assert_called_once_with('/path/to/file') + mocked_display.media_player.setMedia.assert_called_once_with( + QtMultimedia.QMediaContent(QtCore.QUrl.fromLocalFile('/path/to/file'))) + mocked_volume.assert_called_once_with(mocked_display, 1) + self.assertTrue(result) + + def test_load_invalid_media(self): + """ + Test the load() method of SystemPlayer with an invalid media file + """ + # GIVEN: A SystemPlayer instance and a mocked display + player = SystemPlayer(self) + mocked_display = MagicMock() + mocked_display.controller.media_info.volume = 1 + mocked_display.controller.media_info.file_info.absoluteFilePath.return_value = '/path/to/file' + + # WHEN: The load() method is run + with patch.object(player, 'check_media') as mocked_check_media, \ + patch.object(player, 'volume') as mocked_volume: + mocked_check_media.return_value = False + result = player.load(mocked_display) + + # THEN: stuff + mocked_display.controller.media_info.file_info.absoluteFilePath.assert_called_once_with() + mocked_check_media.assert_called_once_with('/path/to/file') + self.assertFalse(result) + + def test_resize(self): + """ + Test the resize() method of the SystemPlayer + """ + # GIVEN: A SystemPlayer instance and a mocked display + player = SystemPlayer(self) + mocked_display = MagicMock() + mocked_display.size.return_value = [1, 2, 3, 4] + + # WHEN: The resize() method is called + player.resize(mocked_display) + + # THEN: The player is resized + mocked_display.size.assert_called_once_with() + mocked_display.video_widget.resize.assert_called_once_with([1, 2, 3, 4]) + + @patch('openlp.core.ui.media.systemplayer.functools') + def test_play_is_live(self, mocked_functools): + """ + Test the play() method of the SystemPlayer on the live display + """ + # GIVEN: A SystemPlayer instance and a mocked display + mocked_functools.partial.return_value = 'function' + player = SystemPlayer(self) + mocked_display = MagicMock() + mocked_display.controller.is_live = True + mocked_display.controller.media_info.start_time = 1 + mocked_display.controller.media_info.volume = 1 + + # WHEN: play() is called + 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: + mocked_get_live_state.return_value = QtMultimedia.QMediaPlayer.PlayingState + result = player.play(mocked_display) + + # THEN: the media file is played + mocked_get_live_state.assert_called_once_with() + 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_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() + self.assertTrue(result) + + @patch('openlp.core.ui.media.systemplayer.functools') + def test_play_is_preview(self, mocked_functools): + """ + Test the play() method of the SystemPlayer on the preview display + """ + # GIVEN: A SystemPlayer instance and a mocked display + mocked_functools.partial.return_value = 'function' + player = SystemPlayer(self) + mocked_display = MagicMock() + mocked_display.controller.is_live = False + mocked_display.controller.media_info.start_time = 1 + mocked_display.controller.media_info.volume = 1 + + # WHEN: play() is called + with patch.object(player, 'get_preview_state') as mocked_get_preview_state, \ + patch.object(player, 'seek') as mocked_seek, \ + patch.object(player, 'volume') as mocked_volume, \ + patch.object(player, 'set_state') as mocked_set_state: + mocked_get_preview_state.return_value = QtMultimedia.QMediaPlayer.PlayingState + result = player.play(mocked_display) + + # THEN: the media file is played + mocked_get_preview_state.assert_called_once_with() + 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_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() + self.assertTrue(result) + + def test_pause_is_live(self): + """ + Test the pause() method of the SystemPlayer on the live display + """ + # GIVEN: A SystemPlayer instance + player = SystemPlayer(self) + mocked_display = MagicMock() + mocked_display.controller.is_live = True + + # WHEN: The pause method is called + with patch.object(player, 'get_live_state') as mocked_get_live_state, \ + patch.object(player, 'set_state') as mocked_set_state: + mocked_get_live_state.return_value = QtMultimedia.QMediaPlayer.PausedState + player.pause(mocked_display) + + # THEN: The video is paused + mocked_display.media_player.pause.assert_called_once_with() + mocked_get_live_state.assert_called_once_with() + mocked_set_state.assert_called_once_with(MediaState.Paused, mocked_display) + + def test_pause_is_preview(self): + """ + Test the pause() method of the SystemPlayer on the preview display + """ + # GIVEN: A SystemPlayer instance + player = SystemPlayer(self) + mocked_display = MagicMock() + mocked_display.controller.is_live = False + + # WHEN: The pause method is called + with patch.object(player, 'get_preview_state') as mocked_get_preview_state, \ + patch.object(player, 'set_state') as mocked_set_state: + mocked_get_preview_state.return_value = QtMultimedia.QMediaPlayer.PausedState + player.pause(mocked_display) + + # THEN: The video is paused + mocked_display.media_player.pause.assert_called_once_with() + mocked_get_preview_state.assert_called_once_with() + mocked_set_state.assert_called_once_with(MediaState.Paused, mocked_display) + + def test_stop(self): + """ + Test the stop() method of the SystemPlayer + """ + # GIVEN: A SystemPlayer instance + player = SystemPlayer(self) + mocked_display = MagicMock() + + # WHEN: The stop method is called + with patch.object(player, 'set_visible') as mocked_set_visible, \ + patch.object(player, 'set_state') as mocked_set_state: + player.stop(mocked_display) + + # THEN: The video is stopped + mocked_display.media_player.stop.assert_called_once_with() + mocked_set_visible.assert_called_once_with(mocked_display, False) + mocked_set_state.assert_called_once_with(MediaState.Stopped, mocked_display) + + def test_volume(self): + """ + Test the volume() method of the SystemPlayer + """ + # GIVEN: A SystemPlayer instance + player = SystemPlayer(self) + mocked_display = MagicMock() + mocked_display.has_audio = True + + # WHEN: The stop method is called + player.volume(mocked_display, 2) + + # THEN: The video is stopped + mocked_display.media_player.setVolume.assert_called_once_with(2) + + def test_seek(self): + """ + Test the seek() method of the SystemPlayer + """ + # GIVEN: A SystemPlayer instance + player = SystemPlayer(self) + mocked_display = MagicMock() + + # WHEN: The stop method is called + player.seek(mocked_display, 2) + + # THEN: The video is stopped + mocked_display.media_player.setPosition.assert_called_once_with(2) + + def test_reset(self): + """ + Test the reset() method of the SystemPlayer + """ + # GIVEN: A SystemPlayer instance + player = SystemPlayer(self) + mocked_display = MagicMock() + + # WHEN: reset() is called + with patch.object(player, 'set_state') as mocked_set_state, \ + patch.object(player, 'set_visible') as mocked_set_visible: + player.reset(mocked_display) + + # THEN: The media player is reset + mocked_display.media_player.stop() + mocked_display.media_player.setMedia.assert_called_once_with(QtMultimedia.QMediaContent()) + mocked_set_visible.assert_called_once_with(mocked_display, False) + mocked_display.video_widget.setVisible.assert_called_once_with(False) + mocked_set_state.assert_called_once_with(MediaState.Off, mocked_display) + + def test_set_visible(self): + """ + Test the set_visible() method on the SystemPlayer + """ + # GIVEN: A SystemPlayer instance and a mocked display + player = SystemPlayer(self) + player.has_own_widget = True + mocked_display = MagicMock() + + # WHEN: set_visible() is called + player.set_visible(mocked_display, True) + + # THEN: The widget should be visible + mocked_display.video_widget.setVisible.assert_called_once_with(True) + + def test_set_duration(self): + """ + Test the set_duration() method of the SystemPlayer + """ + # GIVEN: a mocked controller + mocked_controller = MagicMock() + mocked_controller.media_info.length = 5 + + # WHEN: The set_duration() is called. NB: the 10 here is ignored by the code + SystemPlayer.set_duration(mocked_controller, 10) + + # THEN: The maximum length of the slider should be set + mocked_controller.seek_slider.setMaximum.assert_called_once_with(5) + + def test_update_ui(self): + """ + Test the update_ui() method on the SystemPlayer + """ + # GIVEN: A SystemPlayer instance + player = SystemPlayer(self) + player.state = MediaState.Playing + mocked_display = MagicMock() + mocked_display.media_player.state.return_value = QtMultimedia.QMediaPlayer.PausedState + mocked_display.controller.media_info.end_time = 1 + mocked_display.media_player.position.return_value = 2 + mocked_display.controller.seek_slider.isSliderDown.return_value = False + + # WHEN: update_ui() is called + with patch.object(player, 'stop') as mocked_stop, \ + patch.object(player, 'set_visible') as mocked_set_visible: + player.update_ui(mocked_display) + + # THEN: The UI is updated + expected_stop_calls = [call(mocked_display), call(mocked_display)] + expected_position_calls = [call(), call()] + expected_block_signals_calls = [call(True), call(False)] + mocked_display.media_player.state.assert_called_once_with() + self.assertEqual(2, mocked_stop.call_count) + self.assertEqual(expected_stop_calls, mocked_stop.call_args_list) + self.assertEqual(2, mocked_display.media_player.position.call_count) + self.assertEqual(expected_position_calls, mocked_display.media_player.position.call_args_list) + mocked_set_visible.assert_called_once_with(mocked_display, False) + mocked_display.controller.seek_slider.isSliderDown.assert_called_once_with() + self.assertEqual(expected_block_signals_calls, + mocked_display.controller.seek_slider.blockSignals.call_args_list) + mocked_display.controller.seek_slider.setSliderPosition.assert_called_once_with(2) + + def test_get_media_display_css(self): + """ + Test the get_media_display_css() method of the SystemPlayer + """ + # GIVEN: A SystemPlayer instance + player = SystemPlayer(self) + + # WHEN: get_media_display_css() is called + result = player.get_media_display_css() + + # THEN: The css should be empty + self.assertEqual('', result) + + def test_get_info(self): + """ + Test the get_info() method of the SystemPlayer + """ + # GIVEN: A SystemPlayer instance + player = SystemPlayer(self) + + # WHEN: get_info() is called + result = player.get_info() + + # THEN: The info should be correct + expected_info = 'This media player uses your operating system to provide media capabilities.
' \ + 'Audio
[]
Video
[]
' + self.assertEqual(expected_info, result) + + @patch('openlp.core.ui.media.systemplayer.CheckMediaWorker') + @patch('openlp.core.ui.media.systemplayer.QtCore.QThread') + def test_check_media(self, MockQThread, MockCheckMediaWorker): + """ + Test the check_media() method of the SystemPlayer + """ + # GIVEN: A SystemPlayer instance and a mocked thread + valid_file = '/path/to/video.ogv' + mocked_application = MagicMock() + Registry().create() + Registry().register('application', mocked_application) + player = SystemPlayer(self) + mocked_thread = MagicMock() + mocked_thread.isRunning.side_effect = [True, False] + mocked_thread.quit = 'quit' # actually supposed to be a slot, but it's all mocked out anyway + MockQThread.return_value = mocked_thread + mocked_check_media_worker = MagicMock() + mocked_check_media_worker.play = 'play' + mocked_check_media_worker.result = True + MockCheckMediaWorker.return_value = mocked_check_media_worker + + # WHEN: check_media() is called with a valid media file + result = player.check_media(valid_file) + + # THEN: It should return True + MockQThread.assert_called_once_with() + MockCheckMediaWorker.assert_called_once_with(valid_file) + mocked_check_media_worker.setVolume.assert_called_once_with(0) + mocked_check_media_worker.moveToThread.assert_called_once_with(mocked_thread) + mocked_check_media_worker.finished.connect.assert_called_once_with('quit') + mocked_thread.started.connect.assert_called_once_with('play') + mocked_thread.start.assert_called_once_with() + self.assertEqual(2, mocked_thread.isRunning.call_count) + mocked_application.processEvents.assert_called_once_with() + self.assertTrue(result) + + +class TestCheckMediaWorker(TestCase): + """ + Test the CheckMediaWorker class + """ + def test_constructor(self): + """ + Test the constructor of the CheckMediaWorker class + """ + # GIVEN: A file path + path = 'file.ogv' + + # WHEN: The CheckMediaWorker object is instantiated + worker = CheckMediaWorker(path) + + # THEN: The correct values should be set up + self.assertIsNotNone(worker) + + def test_signals_media(self): + """ + Test the signals() signal of the CheckMediaWorker class with a "media" origin + """ + # GIVEN: A CheckMediaWorker instance + worker = CheckMediaWorker('file.ogv') + + # WHEN: signals() is called with media and BufferedMedia + with patch.object(worker, 'stop') as mocked_stop, \ + patch.object(worker, 'finished') as mocked_finished: + worker.signals('media', worker.BufferedMedia) + + # THEN: The worker should exit and the result should be True + mocked_stop.assert_called_once_with() + mocked_finished.emit.assert_called_once_with() + self.assertTrue(worker.result) + + def test_signals_error(self): + """ + Test the signals() signal of the CheckMediaWorker class with a "error" origin + """ + # GIVEN: A CheckMediaWorker instance + worker = CheckMediaWorker('file.ogv') + + # WHEN: signals() is called with error and BufferedMedia + with patch.object(worker, 'stop') as mocked_stop, \ + patch.object(worker, 'finished') as mocked_finished: + worker.signals('error', None) + + # THEN: The worker should exit and the result should be True + mocked_stop.assert_called_once_with() + mocked_finished.emit.assert_called_once_with() + self.assertFalse(worker.result) diff --git a/tests/functional/openlp_plugins/songs/test_songselect.py b/tests/functional/openlp_plugins/songs/test_songselect.py index 1c61ddc43..362163f58 100644 --- a/tests/functional/openlp_plugins/songs/test_songselect.py +++ b/tests/functional/openlp_plugins/songs/test_songselect.py @@ -28,14 +28,13 @@ from urllib.error import URLError from PyQt5 import QtWidgets -from tests.helpers.songfileimport import SongImportTestHelper from openlp.core import Registry from openlp.plugins.songs.forms.songselectform import SongSelectForm, SearchWorker from openlp.plugins.songs.lib import Song from openlp.plugins.songs.lib.songselect import SongSelectImport, LOGOUT_URL, BASE_URL -from openlp.plugins.songs.lib.importers.cclifile import CCLIFileImport from tests.functional import MagicMock, patch, call +from tests.helpers.songfileimport import SongImportTestHelper from tests.helpers.testmixin import TestMixin TEST_PATH = os.path.abspath( @@ -71,7 +70,7 @@ class TestSongSelectImport(TestCase, TestMixin): mocked_opener = MagicMock() mocked_build_opener.return_value = mocked_opener mocked_login_page = MagicMock() - mocked_login_page.find.return_value = {'value': 'blah'} + mocked_login_page.find.side_effect = [{'value': 'blah'}, None] MockedBeautifulSoup.return_value = mocked_login_page mock_callback = MagicMock() importer = SongSelectImport(None) @@ -112,7 +111,7 @@ class TestSongSelectImport(TestCase, TestMixin): mocked_opener = MagicMock() mocked_build_opener.return_value = mocked_opener mocked_login_page = MagicMock() - mocked_login_page.find.side_effect = [{'value': 'blah'}, None] + mocked_login_page.find.side_effect = [{'value': 'blah'}, MagicMock()] MockedBeautifulSoup.return_value = mocked_login_page mock_callback = MagicMock() importer = SongSelectImport(None) @@ -165,7 +164,7 @@ class TestSongSelectImport(TestCase, TestMixin): 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(1, mocked_results_page.find_all.call_count, 'find_all should have been called once') - mocked_results_page.find_all.assert_called_with('li', 'result pane') + 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') @@ -177,12 +176,18 @@ class TestSongSelectImport(TestCase, TestMixin): # GIVEN: A bunch of mocked out stuff and an importer object # first search result mocked_result1 = MagicMock() - mocked_result1.find.side_effect = [MagicMock(string='Title 1'), {'href': '/url1'}] - mocked_result1.find_all.return_value = [MagicMock(string='Author 1-1'), MagicMock(string='Author 1-2')] + mocked_result1.find.side_effect = [ + MagicMock(find=MagicMock(return_value=MagicMock(string='Title 1'))), + MagicMock(string='James, John'), + MagicMock(find=MagicMock(return_value={'href': '/url1'})) + ] # second search result mocked_result2 = MagicMock() - mocked_result2.find.side_effect = [MagicMock(string='Title 2'), {'href': '/url2'}] - mocked_result2.find_all.return_value = [MagicMock(string='Author 2-1'), MagicMock(string='Author 2-2')] + mocked_result2.find.side_effect = [ + MagicMock(find=MagicMock(return_value=MagicMock(string='Title 2'))), + MagicMock(string='Philip'), + MagicMock(find=MagicMock(return_value={'href': '/url2'})) + ] # rest of the stuff mocked_opener = MagicMock() mocked_build_opener.return_value = mocked_opener @@ -199,10 +204,10 @@ class TestSongSelectImport(TestCase, TestMixin): self.assertEqual(2, mock_callback.call_count, 'callback should have been called twice') self.assertEqual(2, mocked_opener.open.call_count, 'open should have been called twice') self.assertEqual(2, mocked_results_page.find_all.call_count, 'find_all should have been called twice') - mocked_results_page.find_all.assert_called_with('li', 'result pane') + mocked_results_page.find_all.assert_called_with('div', 'song-result') expected_list = [ - {'title': 'Title 1', 'authors': ['Author 1-1', 'Author 1-2'], 'link': BASE_URL + '/url1'}, - {'title': 'Title 2', 'authors': ['Author 2-1', 'Author 2-2'], 'link': BASE_URL + '/url2'} + {'title': 'Title 1', 'authors': ['James', 'John'], 'link': BASE_URL + '/url1'}, + {'title': 'Title 2', 'authors': ['Philip'], 'link': BASE_URL + '/url2'} ] self.assertListEqual(expected_list, results, 'The search method should have returned two songs') @@ -215,16 +220,25 @@ class TestSongSelectImport(TestCase, TestMixin): # GIVEN: A bunch of mocked out stuff and an importer object # first search result mocked_result1 = MagicMock() - mocked_result1.find.side_effect = [MagicMock(string='Title 1'), {'href': '/url1'}] - mocked_result1.find_all.return_value = [MagicMock(string='Author 1-1'), MagicMock(string='Author 1-2')] + mocked_result1.find.side_effect = [ + MagicMock(find=MagicMock(return_value=MagicMock(string='Title 1'))), + MagicMock(string='James, John'), + MagicMock(find=MagicMock(return_value={'href': '/url1'})) + ] # second search result mocked_result2 = MagicMock() - mocked_result2.find.side_effect = [MagicMock(string='Title 2'), {'href': '/url2'}] - mocked_result2.find_all.return_value = [MagicMock(string='Author 2-1'), MagicMock(string='Author 2-2')] + mocked_result2.find.side_effect = [ + MagicMock(find=MagicMock(return_value=MagicMock(string='Title 2'))), + MagicMock(string='Philip'), + MagicMock(find=MagicMock(return_value={'href': '/url2'})) + ] # third search result mocked_result3 = MagicMock() - mocked_result3.find.side_effect = [MagicMock(string='Title 3'), {'href': '/url3'}] - mocked_result3.find_all.return_value = [MagicMock(string='Author 3-1'), MagicMock(string='Author 3-2')] + mocked_result3.find.side_effect = [ + MagicMock(find=MagicMock(return_value=MagicMock(string='Title 3'))), + MagicMock(string='Luke, Matthew'), + MagicMock(find=MagicMock(return_value={'href': '/url3'})) + ] # rest of the stuff mocked_opener = MagicMock() mocked_build_opener.return_value = mocked_opener @@ -241,9 +255,9 @@ class TestSongSelectImport(TestCase, TestMixin): self.assertEqual(2, mock_callback.call_count, 'callback should have been called twice') self.assertEqual(2, mocked_opener.open.call_count, 'open should have been called twice') self.assertEqual(2, mocked_results_page.find_all.call_count, 'find_all should have been called twice') - mocked_results_page.find_all.assert_called_with('li', 'result pane') - expected_list = [{'title': 'Title 1', 'authors': ['Author 1-1', 'Author 1-2'], 'link': BASE_URL + '/url1'}, - {'title': 'Title 2', 'authors': ['Author 2-1', 'Author 2-2'], 'link': BASE_URL + '/url2'}] + mocked_results_page.find_all.assert_called_with('div', 'song-result') + expected_list = [{'title': 'Title 1', 'authors': ['James', 'John'], 'link': BASE_URL + '/url1'}, + {'title': 'Title 2', 'authors': ['Philip'], 'link': BASE_URL + '/url2'}] self.assertListEqual(expected_list, results, 'The search method should have returned two songs') @patch('openlp.plugins.songs.lib.songselect.build_opener') @@ -337,7 +351,7 @@ class TestSongSelectImport(TestCase, TestMixin): self.assertIsNotNone(result, 'The get_song() method should have returned a song dictionary') self.assertEqual(2, mocked_lyrics_page.find.call_count, 'The find() method should have been called twice') self.assertEqual(2, mocked_find_all.call_count, 'The find_all() method should have been called twice') - self.assertEqual([call('section', 'lyrics'), call('section', 'lyrics')], + self.assertEqual([call('div', 'song-viewer lyrics'), call('div', 'song-viewer lyrics')], mocked_lyrics_page.find.call_args_list, 'The find() method should have been called with the right arguments') self.assertEqual([call('p'), call('h3')], mocked_find_all.call_args_list, @@ -348,8 +362,9 @@ class TestSongSelectImport(TestCase, TestMixin): self.assertEqual(3, len(result['verses']), 'Three verses should have been returned') @patch('openlp.plugins.songs.lib.songselect.clean_song') + @patch('openlp.plugins.songs.lib.songselect.Topic') @patch('openlp.plugins.songs.lib.songselect.Author') - def test_save_song_new_author(self, MockedAuthor, mocked_clean_song): + def test_save_song_new_author(self, MockedAuthor, MockedTopic, mocked_clean_song): """ Test that saving a song with a new author performs the correct actions """ @@ -366,6 +381,7 @@ class TestSongSelectImport(TestCase, TestMixin): 'ccli_number': '123456' } MockedAuthor.display_name.__eq__.return_value = False + MockedTopic.name.__eq__.return_value = False mocked_db_manager = MagicMock() mocked_db_manager.get_object_filtered.return_value = None importer = SongSelectImport(mocked_db_manager) @@ -634,7 +650,7 @@ class TestSongSelectForm(TestCase, TestMixin): # WHEN: _update_login_progress() is called with patch.object(ssform, 'login_progress_bar') as mocked_login_progress_bar: mocked_login_progress_bar.value.return_value = 3 - ssform._update_login_progress() + ssform._update_login_progress() # pylint: disable=protected-access # THEN: The login progress bar should be updated mocked_login_progress_bar.setValue.assert_called_with(4) @@ -649,7 +665,7 @@ class TestSongSelectForm(TestCase, TestMixin): # WHEN: _update_song_progress() is called with patch.object(ssform, 'song_progress_bar') as mocked_song_progress_bar: mocked_song_progress_bar.value.return_value = 2 - ssform._update_song_progress() + ssform._update_song_progress() # pylint: disable=protected-access # THEN: The song progress bar should be updated mocked_song_progress_bar.setValue.assert_called_with(3) @@ -806,7 +822,7 @@ class TestSearchWorker(TestCase, TestMixin): worker.start() # THEN: The "finished" and "quit" signals should be emitted - importer.search.assert_called_with(search_text, 1000, worker._found_song_callback) + importer.search.assert_called_with(search_text, 1000, worker._found_song_callback) # pylint: disable=protected-access mocked_finished.emit.assert_called_with() mocked_quit.emit.assert_called_with() @@ -828,7 +844,7 @@ class TestSearchWorker(TestCase, TestMixin): worker.start() # THEN: The "finished" and "quit" signals should be emitted - importer.search.assert_called_with(search_text, 1000, worker._found_song_callback) + importer.search.assert_called_with(search_text, 1000, worker._found_song_callback) # pylint: disable=protected-access mocked_show_info.emit.assert_called_with('More than 1000 results', 'Your search has returned more than 1000 ' 'results, it has been stopped. Please ' 'refine your search to fetch better ' @@ -848,7 +864,7 @@ class TestSearchWorker(TestCase, TestMixin): # WHEN: The start() method is called with patch.object(worker, 'found_song') as mocked_found_song: - worker._found_song_callback(song) + worker._found_song_callback(song) # pylint: disable=protected-access # THEN: The "found_song" signal should have been emitted mocked_found_song.emit.assert_called_with(song) From 2bd58e1c294621588bf34c6c25297d603780ec42 Mon Sep 17 00:00:00 2001 From: Raoul Snyman Date: Sat, 13 Aug 2016 16:41:24 +0200 Subject: [PATCH 2/3] Fix some pep8 issues --- openlp/plugins/songs/lib/songselect.py | 6 ++++-- .../functional/openlp_core_ui_media/test_systemplayer.py | 1 + tests/functional/openlp_plugins/songs/test_songselect.py | 9 +++++---- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/openlp/plugins/songs/lib/songselect.py b/openlp/plugins/songs/lib/songselect.py index 84d6d7b90..ccec2c2d8 100644 --- a/openlp/plugins/songs/lib/songselect.py +++ b/openlp/plugins/songs/lib/songselect.py @@ -37,7 +37,8 @@ from openlp.plugins.songs.lib import Song, Author, Topic, VerseType, clean_song from openlp.plugins.songs.lib.openlyricsxml import SongXML USER_AGENTS = [ - 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/52.0.2743.116 Safari/537.36' + 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) ' \ + 'Chrome/52.0.2743.116 Safari/537.36', 'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/52.0.2743.82 Safari/537.36', 'Mozilla/5.0 (X11; Linux x86_64; rv:47.0) Gecko/20100101 Firefox/47.0', 'Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:46.0) Gecko/20100101 Firefox/46.0', @@ -195,7 +196,8 @@ class SongSelectImport(object): theme_elements.extend(ul.find_all('li')[1:]) song['copyright'] = '/'.join([unescape(li.string).strip() for li in copyright_elements]) song['topics'] = [unescape(li.string).strip() for li in theme_elements] - song['ccli_number'] = song_page.find('div', 'song-content-data').find('ul').find('li').find('strong').string.strip() + song['ccli_number'] = song_page.find('div', 'song-content-data').find('ul').find('li')\ + .find('strong').string.strip() song['verses'] = [] verses = lyrics_page.find('div', 'song-viewer lyrics').find_all('p') verse_labels = lyrics_page.find('div', 'song-viewer lyrics').find_all('h3') diff --git a/tests/functional/openlp_core_ui_media/test_systemplayer.py b/tests/functional/openlp_core_ui_media/test_systemplayer.py index 357e91e44..d1873d2df 100644 --- a/tests/functional/openlp_core_ui_media/test_systemplayer.py +++ b/tests/functional/openlp_core_ui_media/test_systemplayer.py @@ -32,6 +32,7 @@ from openlp.core.ui.media.systemplayer import SystemPlayer, CheckMediaWorker, AD from tests.functional import MagicMock, call, patch + class TestSystemPlayer(TestCase): """ Test the system media player diff --git a/tests/functional/openlp_plugins/songs/test_songselect.py b/tests/functional/openlp_plugins/songs/test_songselect.py index 362163f58..70c0d40fc 100644 --- a/tests/functional/openlp_plugins/songs/test_songselect.py +++ b/tests/functional/openlp_plugins/songs/test_songselect.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- # vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4 +# pylint: disable=protected-access ############################################################################### # OpenLP - Open Source Lyrics Projection # @@ -650,7 +651,7 @@ class TestSongSelectForm(TestCase, TestMixin): # WHEN: _update_login_progress() is called with patch.object(ssform, 'login_progress_bar') as mocked_login_progress_bar: mocked_login_progress_bar.value.return_value = 3 - ssform._update_login_progress() # pylint: disable=protected-access + ssform._update_login_progress() # THEN: The login progress bar should be updated mocked_login_progress_bar.setValue.assert_called_with(4) @@ -665,7 +666,7 @@ class TestSongSelectForm(TestCase, TestMixin): # WHEN: _update_song_progress() is called with patch.object(ssform, 'song_progress_bar') as mocked_song_progress_bar: mocked_song_progress_bar.value.return_value = 2 - ssform._update_song_progress() # pylint: disable=protected-access + ssform._update_song_progress() # THEN: The song progress bar should be updated mocked_song_progress_bar.setValue.assert_called_with(3) @@ -822,7 +823,7 @@ class TestSearchWorker(TestCase, TestMixin): worker.start() # THEN: The "finished" and "quit" signals should be emitted - importer.search.assert_called_with(search_text, 1000, worker._found_song_callback) # pylint: disable=protected-access + importer.search.assert_called_with(search_text, 1000, worker._found_song_callback) mocked_finished.emit.assert_called_with() mocked_quit.emit.assert_called_with() @@ -844,7 +845,7 @@ class TestSearchWorker(TestCase, TestMixin): worker.start() # THEN: The "finished" and "quit" signals should be emitted - importer.search.assert_called_with(search_text, 1000, worker._found_song_callback) # pylint: disable=protected-access + importer.search.assert_called_with(search_text, 1000, worker._found_song_callback) mocked_show_info.emit.assert_called_with('More than 1000 results', 'Your search has returned more than 1000 ' 'results, it has been stopped. Please ' 'refine your search to fetch better ' From fbd9b330a3feb23c5a304f54e2e8f8dc4a71d2af Mon Sep 17 00:00:00 2001 From: Raoul Snyman Date: Sat, 13 Aug 2016 16:50:15 +0200 Subject: [PATCH 3/3] Fix another pep8 issues --- openlp/plugins/songs/lib/songselect.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/plugins/songs/lib/songselect.py b/openlp/plugins/songs/lib/songselect.py index ccec2c2d8..e24a5a0c9 100644 --- a/openlp/plugins/songs/lib/songselect.py +++ b/openlp/plugins/songs/lib/songselect.py @@ -37,7 +37,7 @@ from openlp.plugins.songs.lib import Song, Author, Topic, VerseType, clean_song from openlp.plugins.songs.lib.openlyricsxml import SongXML USER_AGENTS = [ - 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) ' \ + 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) ' 'Chrome/52.0.2743.116 Safari/537.36', 'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/52.0.2743.82 Safari/537.36', 'Mozilla/5.0 (X11; Linux x86_64; rv:47.0) Gecko/20100101 Firefox/47.0',