From 43fef6cc2bfafb7ce53c34b90bd03d2c5b30b5ba Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Fri, 2 Aug 2013 22:51:39 +0100 Subject: [PATCH 01/23] fixed 1184869 by checking that the anchor tag actually has some text --- openlp/plugins/bibles/lib/http.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/plugins/bibles/lib/http.py b/openlp/plugins/bibles/lib/http.py index de48b2617..cb410dbaa 100644 --- a/openlp/plugins/bibles/lib/http.py +++ b/openlp/plugins/bibles/lib/http.py @@ -378,7 +378,7 @@ class BSExtract(object): send_error_message(u'parse') return None content = content.find_all(u'li') - return [book.contents[0].contents[0] for book in content] + return [book.contents[0].contents[0] for book in content if len(book.contents[0].contents)] def _get_application(self): """ From 364fde73c54926ef67c02ee813c5c08ac854d402 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Tue, 13 Aug 2013 19:02:04 +0100 Subject: [PATCH 02/23] Fix #120515 by reimplementing QFileDialog.getOpenFileNames --- openlp/core/lib/__init__.py | 1 + openlp/core/lib/filedialog.py | 66 +++++++++++++++++ openlp/core/lib/mediamanageritem.py | 4 +- openlp/core/lib/uistrings.py | 2 + openlp/core/ui/thememanager.py | 7 +- openlp/plugins/songs/forms/editsongform.py | 6 +- openlp/plugins/songs/forms/songimportform.py | 4 +- .../openlp_core_lib/test_file_dialog.py | 71 +++++++++++++++++++ 8 files changed, 151 insertions(+), 10 deletions(-) create mode 100644 openlp/core/lib/filedialog.py create mode 100644 tests/functional/openlp_core_lib/test_file_dialog.py diff --git a/openlp/core/lib/__init__.py b/openlp/core/lib/__init__.py index d6c338271..61891c1ef 100644 --- a/openlp/core/lib/__init__.py +++ b/openlp/core/lib/__init__.py @@ -377,6 +377,7 @@ def create_separated_list(stringlist): from registry import Registry from uistrings import UiStrings +from filedialog import FileDialog from screen import ScreenList from settings import Settings from listwidgetwithdnd import ListWidgetWithDnD diff --git a/openlp/core/lib/filedialog.py b/openlp/core/lib/filedialog.py new file mode 100644 index 000000000..b2159b511 --- /dev/null +++ b/openlp/core/lib/filedialog.py @@ -0,0 +1,66 @@ +# -*- coding: utf-8 -*- +# vim: autoindent shiftwidth=4 expandtab textwidth=80 tabstop=4 softtabstop=4 + +############################################################################### +# OpenLP - Open Source Lyrics Projection # +# --------------------------------------------------------------------------- # +# Copyright (c) 2008-2013 Raoul Snyman # +# Portions copyright (c) 2008-2013 Tim Bentley, Gerald Britton, Jonathan # +# Corwin, Samuel Findlay, Michael Gorven, Scott Guerrieri, Matthias Hub, # +# Meinert Jordan, Armin Köhler, Erik Lundin, Edwin Lunando, Brian T. Meyer. # +# Joshua Miller, Stevan Pettit, Andreas Preikschat, Mattias Põldaru, # +# Christian Richter, Philip Ridout, Simon Scudder, Jeffrey Smith, # +# Maikel Stuivenberg, Martin Thompson, Jon Tibble, Dave Warnock, # +# Frode Woldsund, Martin Zibricky # +# --------------------------------------------------------------------------- # +# 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 # +############################################################################### + +""" +Provide a work around for a bug in QFileDialog (#1209515) +""" +import logging +import os +import urllib + +from PyQt4 import QtGui + +from openlp.core.lib import UiStrings + +log = logging.getLogger(__name__) + +class FileDialog(QtGui.QFileDialog): + """ + Inherit form QFileDialog + """ + @staticmethod + def getOpenFileNames(parent, *args, **kwargs): + """ + Reimplement getOpenFileNames to fix the way it returns some file names that url encoded when selecting multiple + files + """ + files = QtGui.QFileDialog.getOpenFileNames(parent, *args, **kwargs) + file_list = [] + for file in files: + file = unicode(file) + if not os.path.exists(file): + log.info(u'File %s not found. Attempting to unquote.') + file = urllib.unquote(unicode(file)) + if not os.path.exists(file): + log.error(u'File %s not found.' % file) + QtGui.QMessageBox.information(parent, UiStrings().FNFT, UiStrings().FNF % file) + continue + log.info(u'File %s found.') + file_list.append(file) + return file_list \ No newline at end of file diff --git a/openlp/core/lib/mediamanageritem.py b/openlp/core/lib/mediamanageritem.py index 973d457bb..1aa622b1e 100644 --- a/openlp/core/lib/mediamanageritem.py +++ b/openlp/core/lib/mediamanageritem.py @@ -35,7 +35,7 @@ import re from PyQt4 import QtCore, QtGui -from openlp.core.lib import OpenLPToolbar, ServiceItem, StringContent, ListWidgetWithDnD, \ +from openlp.core.lib import FileDialog, OpenLPToolbar, ServiceItem, StringContent, ListWidgetWithDnD, \ ServiceItemContext, Settings, Registry, UiStrings, translate from openlp.core.lib.searchedit import SearchEdit from openlp.core.lib.ui import create_widget_action, critical_error_message_box @@ -305,7 +305,7 @@ class MediaManagerItem(QtGui.QWidget): """ Add a file to the list widget to make it available for showing """ - files = QtGui.QFileDialog.getOpenFileNames(self, self.on_new_prompt, + files = FileDialog().getOpenFileNames(self, self.on_new_prompt, Settings().value(self.settings_section + u'/last directory'), self.on_new_file_masks) log.info(u'New files(s) %s', files) if files: diff --git a/openlp/core/lib/uistrings.py b/openlp/core/lib/uistrings.py index 6d4b4d250..26ad63b55 100644 --- a/openlp/core/lib/uistrings.py +++ b/openlp/core/lib/uistrings.py @@ -83,6 +83,8 @@ class UiStrings(object): self.Error = translate('OpenLP.Ui', 'Error') self.Export = translate('OpenLP.Ui', 'Export') self.File = translate('OpenLP.Ui', 'File') + self.FNFT = unicode(translate('OpenLP.Ui', 'File Not Found')) + self.FNF = unicode(translate('OpenLP.Ui', 'File %s not found.\nPlease try selecting it individually.')) self.FontSizePtUnit = translate('OpenLP.Ui', 'pt', 'Abbreviated font pointsize unit') self.Help = translate('OpenLP.Ui', 'Help') self.Hours = translate('OpenLP.Ui', 'h', 'The abbreviated unit for hours') diff --git a/openlp/core/ui/thememanager.py b/openlp/core/ui/thememanager.py index 58f29cab6..a1f761d8f 100644 --- a/openlp/core/ui/thememanager.py +++ b/openlp/core/ui/thememanager.py @@ -38,8 +38,9 @@ import re from xml.etree.ElementTree import ElementTree, XML from PyQt4 import QtCore, QtGui -from openlp.core.lib import ImageSource, OpenLPToolbar, Registry, Settings, UiStrings, get_text_file_string, \ - build_icon, translate, check_item_selected, check_directory_exists, create_thumb, validate_thumb +from openlp.core.lib import FileDialog, ImageSource, OpenLPToolbar, Registry, Settings, UiStrings, \ + get_text_file_string, build_icon, translate, check_item_selected, check_directory_exists, create_thumb, \ + validate_thumb from openlp.core.lib.theme import ThemeXML, BackgroundType, VerticalType, BackgroundGradientType from openlp.core.lib.ui import critical_error_message_box, create_widget_action from openlp.core.theme import Theme @@ -373,7 +374,7 @@ class ThemeManager(QtGui.QWidget): Opens a file dialog to select the theme file(s) to import before attempting to extract OpenLP themes from those files. This process will load both OpenLP version 1 and version 2 themes. """ - files = QtGui.QFileDialog.getOpenFileNames(self, + files = FileDialog().getOpenFileNames(self, translate('OpenLP.ThemeManager', 'Select Theme Import File'), Settings().value(self.settings_section + u'/last directory import'), translate('OpenLP.ThemeManager', 'OpenLP Themes (*.theme *.otz)')) diff --git a/openlp/plugins/songs/forms/editsongform.py b/openlp/plugins/songs/forms/editsongform.py index 24d0d3024..f73a4468c 100644 --- a/openlp/plugins/songs/forms/editsongform.py +++ b/openlp/plugins/songs/forms/editsongform.py @@ -38,8 +38,8 @@ import shutil from PyQt4 import QtCore, QtGui -from openlp.core.lib import Registry, PluginStatus, MediaType, UiStrings, translate, create_separated_list, \ - check_directory_exists +from openlp.core.lib import FileDialog, Registry, PluginStatus, MediaType, UiStrings, translate, \ + create_separated_list, check_directory_exists from openlp.core.lib.ui import set_case_insensitive_completer, critical_error_message_box, find_and_set_in_combo_box from openlp.core.utils import AppLocation from openlp.plugins.songs.lib import VerseType, clean_song @@ -753,7 +753,7 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog): Loads file(s) from the filesystem. """ filters = u'%s (*)' % UiStrings().AllFiles - filenames = QtGui.QFileDialog.getOpenFileNames(self, + filenames = FileDialog().getOpenFileNames(self, translate('SongsPlugin.EditSongForm', 'Open File(s)'), u'', filters) for filename in filenames: item = QtGui.QListWidgetItem(os.path.split(unicode(filename))[1]) diff --git a/openlp/plugins/songs/forms/songimportform.py b/openlp/plugins/songs/forms/songimportform.py index c1aca92ad..bd9acc943 100644 --- a/openlp/plugins/songs/forms/songimportform.py +++ b/openlp/plugins/songs/forms/songimportform.py @@ -35,7 +35,7 @@ import os from PyQt4 import QtCore, QtGui -from openlp.core.lib import Registry, Settings, UiStrings, translate +from openlp.core.lib import FileDialog, Registry, Settings, UiStrings, translate from openlp.core.lib.ui import critical_error_message_box from openlp.core.ui.wizard import OpenLPWizard, WizardStrings from openlp.plugins.songs.lib.importer import SongFormat, SongFormatSelect @@ -244,7 +244,7 @@ class SongImportForm(OpenLPWizard): if filters: filters += u';;' filters += u'%s (*)' % UiStrings().AllFiles - filenames = QtGui.QFileDialog.getOpenFileNames(self, title, + filenames = FileDialog().getOpenFileNames(self, title, Settings().value(self.plugin.settings_section + u'/last directory import'), filters) if filenames: listbox.addItems(filenames) diff --git a/tests/functional/openlp_core_lib/test_file_dialog.py b/tests/functional/openlp_core_lib/test_file_dialog.py new file mode 100644 index 000000000..dc704eaef --- /dev/null +++ b/tests/functional/openlp_core_lib/test_file_dialog.py @@ -0,0 +1,71 @@ +""" +Package to test the openlp.core.lib.formattingtags package. +""" +import copy +from unittest import TestCase +from mock import MagicMock, call, patch + +from openlp.core.lib.filedialog import FileDialog +from openlp.core.lib.uistrings import UiStrings + +class TestFileDialog(TestCase): + """ + Test the functions in the :mod:`filedialog` module. + """ + def setUp(self): + self.os_patcher = patch(u'openlp.core.lib.filedialog.os') + self.qt_gui_patcher = patch(u'openlp.core.lib.filedialog.QtGui') + self.ui_strings_patcher = patch(u'openlp.core.lib.filedialog.UiStrings') + self.mocked_os = self.os_patcher.start() + self.mocked_qt_gui = self.qt_gui_patcher.start() + self.mocked_ui_strings = self.ui_strings_patcher.start() + self.mocked_parent = MagicMock() + + def tearDown(self): + self.os_patcher.stop() + self.qt_gui_patcher.stop() + self.ui_strings_patcher.stop() + + def get_open_file_names_canceled_test(self): + """ + Test that FileDialog.getOpenFileNames() returns and empty QStringList when QFileDialog is canceled + (returns an empty QStringList) + """ + self.mocked_os.reset() + + # GIVEN: An empty QStringList as a return value from QFileDialog.getOpenFileNames + self.mocked_qt_gui.QFileDialog.getOpenFileNames.return_value = [] + + # WHEN: FileDialog.getOpenFileNames is called + result = FileDialog.getOpenFileNames(self.mocked_parent) + + # THEN: The returned value should be an empty QStiingList and os.path.exists should not have been called + assert not self.mocked_os.path.exists.called + self.assertEqual(result, [], + u'FileDialog.getOpenFileNames should return and empty list when QFileDialog.getOpenFileNames is canceled') + + def returned_file_list_test(self): + """ + Test that FileDialog.getOpenFileNames handles a list of files properly when QFileList.getOpenFileNames + returns a good file name, a urlencoded file name and a non-existing file + """ + self.mocked_os.rest() + self.mocked_qt_gui.reset() + + # GIVEN: A List of known values as a return value from QFileDialog.getOpenFileNamesand a list of valid + # file names. + self.mocked_qt_gui.QFileDialog.getOpenFileNames.return_value = [ + u'/Valid File', u'/url%20encoded%20file%20%231', u'/non-existing'] + self.mocked_os.path.exists.side_effect = lambda file_name: file_name in [ + u'/Valid File', u'/url encoded file #1'] + + # WHEN: FileDialog.getOpenFileNames is called + result = FileDialog.getOpenFileNames(self.mocked_parent) + + # THEN: os.path.exists should have been called with known args. QmessageBox.information should have been + # called. The returned result should corrilate with the input. + self.mocked_os.path.exists.assert_has_calls([call(u'/Valid File'), call(u'/url%20encoded%20file%20%231'), + call(u'/url encoded file #1'), call(u'/non-existing'), call(u'/non-existing')]) + self.mocked_qt_gui.QmessageBox.information.called_with(self.mocked_parent, UiStrings().FNFT, + UiStrings().FNF % u'/non-existing') + self.assertEqual(result, [u'/Valid File', u'/url encoded file #1'], u'The returned file list is incorrect') \ No newline at end of file From 2ceb165901ff9c7e6cc50eced7fa76f56ff8f215 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Thu, 22 Aug 2013 05:33:19 +0000 Subject: [PATCH 03/23] change string names --- openlp/core/lib/filedialog.py | 7 ++++--- openlp/core/lib/uistrings.py | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/openlp/core/lib/filedialog.py b/openlp/core/lib/filedialog.py index b2159b511..ea2eba045 100644 --- a/openlp/core/lib/filedialog.py +++ b/openlp/core/lib/filedialog.py @@ -28,7 +28,7 @@ ############################################################################### """ -Provide a work around for a bug in QFileDialog (#1209515) +Provide a work around for a bug in QFileDialog """ import logging import os @@ -42,7 +42,7 @@ log = logging.getLogger(__name__) class FileDialog(QtGui.QFileDialog): """ - Inherit form QFileDialog + Subclass QFileDialog to work round a bug """ @staticmethod def getOpenFileNames(parent, *args, **kwargs): @@ -59,7 +59,8 @@ class FileDialog(QtGui.QFileDialog): file = urllib.unquote(unicode(file)) if not os.path.exists(file): log.error(u'File %s not found.' % file) - QtGui.QMessageBox.information(parent, UiStrings().FNFT, UiStrings().FNF % file) + QtGui.QMessageBox.information(parent, UiStrings().FileNotFound, + UiStrings().FileNotFoundMessage % file) continue log.info(u'File %s found.') file_list.append(file) diff --git a/openlp/core/lib/uistrings.py b/openlp/core/lib/uistrings.py index 26ad63b55..faab77a49 100644 --- a/openlp/core/lib/uistrings.py +++ b/openlp/core/lib/uistrings.py @@ -83,8 +83,8 @@ class UiStrings(object): self.Error = translate('OpenLP.Ui', 'Error') self.Export = translate('OpenLP.Ui', 'Export') self.File = translate('OpenLP.Ui', 'File') - self.FNFT = unicode(translate('OpenLP.Ui', 'File Not Found')) - self.FNF = unicode(translate('OpenLP.Ui', 'File %s not found.\nPlease try selecting it individually.')) + self.FileNotFound = unicode(translate('OpenLP.Ui', 'File Not Found')) + self.FileNotFoundMessage = unicode(translate('OpenLP.Ui', 'File %s not found.\nPlease try selecting it individually.')) self.FontSizePtUnit = translate('OpenLP.Ui', 'pt', 'Abbreviated font pointsize unit') self.Help = translate('OpenLP.Ui', 'Help') self.Hours = translate('OpenLP.Ui', 'h', 'The abbreviated unit for hours') From 49c9663dcfb0b49c744da3141916ca6568fbd4b0 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Thu, 22 Aug 2013 20:20:02 +0000 Subject: [PATCH 04/23] renamed var, removed some uneeded ()'s --- openlp/core/lib/mediamanageritem.py | 2 +- openlp/core/ui/thememanager.py | 2 +- openlp/plugins/songs/forms/editsongform.py | 2 +- openlp/plugins/songs/forms/songimportform.py | 2 +- tests/functional/openlp_core_lib/test_file_dialog.py | 4 ++-- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/openlp/core/lib/mediamanageritem.py b/openlp/core/lib/mediamanageritem.py index 1aa622b1e..018c6801f 100644 --- a/openlp/core/lib/mediamanageritem.py +++ b/openlp/core/lib/mediamanageritem.py @@ -305,7 +305,7 @@ class MediaManagerItem(QtGui.QWidget): """ Add a file to the list widget to make it available for showing """ - files = FileDialog().getOpenFileNames(self, self.on_new_prompt, + files = FileDialog.getOpenFileNames(self, self.on_new_prompt, Settings().value(self.settings_section + u'/last directory'), self.on_new_file_masks) log.info(u'New files(s) %s', files) if files: diff --git a/openlp/core/ui/thememanager.py b/openlp/core/ui/thememanager.py index a1f761d8f..1f64acbf7 100644 --- a/openlp/core/ui/thememanager.py +++ b/openlp/core/ui/thememanager.py @@ -374,7 +374,7 @@ class ThemeManager(QtGui.QWidget): Opens a file dialog to select the theme file(s) to import before attempting to extract OpenLP themes from those files. This process will load both OpenLP version 1 and version 2 themes. """ - files = FileDialog().getOpenFileNames(self, + files = FileDialog.getOpenFileNames(self, translate('OpenLP.ThemeManager', 'Select Theme Import File'), Settings().value(self.settings_section + u'/last directory import'), translate('OpenLP.ThemeManager', 'OpenLP Themes (*.theme *.otz)')) diff --git a/openlp/plugins/songs/forms/editsongform.py b/openlp/plugins/songs/forms/editsongform.py index f73a4468c..153a96e14 100644 --- a/openlp/plugins/songs/forms/editsongform.py +++ b/openlp/plugins/songs/forms/editsongform.py @@ -753,7 +753,7 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog): Loads file(s) from the filesystem. """ filters = u'%s (*)' % UiStrings().AllFiles - filenames = FileDialog().getOpenFileNames(self, + filenames = FileDialog.getOpenFileNames(self, translate('SongsPlugin.EditSongForm', 'Open File(s)'), u'', filters) for filename in filenames: item = QtGui.QListWidgetItem(os.path.split(unicode(filename))[1]) diff --git a/openlp/plugins/songs/forms/songimportform.py b/openlp/plugins/songs/forms/songimportform.py index bd9acc943..d0378cd45 100644 --- a/openlp/plugins/songs/forms/songimportform.py +++ b/openlp/plugins/songs/forms/songimportform.py @@ -244,7 +244,7 @@ class SongImportForm(OpenLPWizard): if filters: filters += u';;' filters += u'%s (*)' % UiStrings().AllFiles - filenames = FileDialog().getOpenFileNames(self, title, + filenames = FileDialog.getOpenFileNames(self, title, Settings().value(self.plugin.settings_section + u'/last directory import'), filters) if filenames: listbox.addItems(filenames) diff --git a/tests/functional/openlp_core_lib/test_file_dialog.py b/tests/functional/openlp_core_lib/test_file_dialog.py index dc704eaef..b895cc67d 100644 --- a/tests/functional/openlp_core_lib/test_file_dialog.py +++ b/tests/functional/openlp_core_lib/test_file_dialog.py @@ -66,6 +66,6 @@ class TestFileDialog(TestCase): # called. The returned result should corrilate with the input. self.mocked_os.path.exists.assert_has_calls([call(u'/Valid File'), call(u'/url%20encoded%20file%20%231'), call(u'/url encoded file #1'), call(u'/non-existing'), call(u'/non-existing')]) - self.mocked_qt_gui.QmessageBox.information.called_with(self.mocked_parent, UiStrings().FNFT, - UiStrings().FNF % u'/non-existing') + self.mocked_qt_gui.QmessageBox.information.called_with(self.mocked_parent, UiStrings().FileNotFound, + UiStrings().FileNotFoundMessage % u'/non-existing') self.assertEqual(result, [u'/Valid File', u'/url encoded file #1'], u'The returned file list is incorrect') \ No newline at end of file From 9f5bd87a5052d5d2de29ae0dbe81c69dcc62ac6d Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Sun, 15 Sep 2013 20:27:40 +0100 Subject: [PATCH 05/23] Py3 Fixes --- openlp/core/lib/__init__.py | 38 +++++++++++++++++------------------ openlp/core/lib/filedialog.py | 11 +++++----- openlp/core/lib/uistrings.py | 4 ++-- 3 files changed, 26 insertions(+), 27 deletions(-) diff --git a/openlp/core/lib/__init__.py b/openlp/core/lib/__init__.py index b327b4a1c..b4f6b4223 100644 --- a/openlp/core/lib/__init__.py +++ b/openlp/core/lib/__init__.py @@ -374,22 +374,22 @@ def create_separated_list(stringlist): return translate('OpenLP.core.lib', '%s, %s', u'Locale list separator: start') % (stringlist[0], merged) -from registry import Registry -from uistrings import UiStrings -from filedialog import FileDialog -from screen import ScreenList -from settings import Settings -from listwidgetwithdnd import ListWidgetWithDnD -from treewidgetwithdnd import TreeWidgetWithDnD -from formattingtags import FormattingTags -from spelltextedit import SpellTextEdit -from plugin import PluginStatus, StringContent, Plugin -from pluginmanager import PluginManager -from settingstab import SettingsTab -from serviceitem import ServiceItem, ServiceItemType, ItemCapabilities -from htmlbuilder import build_html, build_lyrics_format_css, build_lyrics_outline_css -from toolbar import OpenLPToolbar -from dockwidget import OpenLPDockWidget -from imagemanager import ImageManager -from renderer import Renderer -from mediamanageritem import MediaManagerItem \ No newline at end of file +from .registry import Registry +from .uistrings import UiStrings +from .filedialog import FileDialog +from .screen import ScreenList +from .settings import Settings +from .listwidgetwithdnd import ListWidgetWithDnD +from .treewidgetwithdnd import TreeWidgetWithDnD +from .formattingtags import FormattingTags +from .spelltextedit import SpellTextEdit +from .plugin import PluginStatus, StringContent, Plugin +from .pluginmanager import PluginManager +from .settingstab import SettingsTab +from .serviceitem import ServiceItem, ServiceItemType, ItemCapabilities +from .htmlbuilder import build_html, build_lyrics_format_css, build_lyrics_outline_css +from .toolbar import OpenLPToolbar +from .dockwidget import OpenLPDockWidget +from .imagemanager import ImageManager +from .renderer import Renderer +from .mediamanageritem import MediaManagerItem \ No newline at end of file diff --git a/openlp/core/lib/filedialog.py b/openlp/core/lib/filedialog.py index ea2eba045..03a8b8c47 100644 --- a/openlp/core/lib/filedialog.py +++ b/openlp/core/lib/filedialog.py @@ -32,7 +32,7 @@ Provide a work around for a bug in QFileDialog Date: Sun, 15 Sep 2013 20:42:29 +0100 Subject: [PATCH 06/23] more Py3 fixes --- openlp/core/lib/__init__.py | 6 ++--- .../openlp_core_lib/test_file_dialog.py | 26 +++++++++---------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/openlp/core/lib/__init__.py b/openlp/core/lib/__init__.py index b4f6b4223..3700e2162 100644 --- a/openlp/core/lib/__init__.py +++ b/openlp/core/lib/__init__.py @@ -370,8 +370,8 @@ def create_separated_list(stringlist): 'Locale list separator: end') % (stringlist[-2], stringlist[-1]) for index in reversed(list(range(1, len(stringlist) - 2))): merged = translate('OpenLP.core.lib', '%s, %s', - u'Locale list separator: middle') % (stringlist[index], merged) - return translate('OpenLP.core.lib', '%s, %s', u'Locale list separator: start') % (stringlist[0], merged) + 'Locale list separator: middle') % (stringlist[index], merged) + return translate('OpenLP.core.lib', '%s, %s', 'Locale list separator: start') % (stringlist[0], merged) from .registry import Registry @@ -392,4 +392,4 @@ from .toolbar import OpenLPToolbar from .dockwidget import OpenLPDockWidget from .imagemanager import ImageManager from .renderer import Renderer -from .mediamanageritem import MediaManagerItem \ No newline at end of file +from .mediamanageritem import MediaManagerItem diff --git a/tests/functional/openlp_core_lib/test_file_dialog.py b/tests/functional/openlp_core_lib/test_file_dialog.py index b895cc67d..0a5736ff0 100644 --- a/tests/functional/openlp_core_lib/test_file_dialog.py +++ b/tests/functional/openlp_core_lib/test_file_dialog.py @@ -1,5 +1,5 @@ """ -Package to test the openlp.core.lib.formattingtags package. +Package to test the openlp.core.lib.filedialog package. """ import copy from unittest import TestCase @@ -13,9 +13,9 @@ class TestFileDialog(TestCase): Test the functions in the :mod:`filedialog` module. """ def setUp(self): - self.os_patcher = patch(u'openlp.core.lib.filedialog.os') - self.qt_gui_patcher = patch(u'openlp.core.lib.filedialog.QtGui') - self.ui_strings_patcher = patch(u'openlp.core.lib.filedialog.UiStrings') + self.os_patcher = patch('openlp.core.lib.filedialog.os') + self.qt_gui_patcher = patch('openlp.core.lib.filedialog.QtGui') + self.ui_strings_patcher = patch('openlp.core.lib.filedialog.UiStrings') self.mocked_os = self.os_patcher.start() self.mocked_qt_gui = self.qt_gui_patcher.start() self.mocked_ui_strings = self.ui_strings_patcher.start() @@ -39,10 +39,10 @@ class TestFileDialog(TestCase): # WHEN: FileDialog.getOpenFileNames is called result = FileDialog.getOpenFileNames(self.mocked_parent) - # THEN: The returned value should be an empty QStiingList and os.path.exists should not have been called + # THEN: The returned value should be an empty QStringList and os.path.exists should not have been called assert not self.mocked_os.path.exists.called self.assertEqual(result, [], - u'FileDialog.getOpenFileNames should return and empty list when QFileDialog.getOpenFileNames is canceled') + 'FileDialog.getOpenFileNames should return and empty list when QFileDialog.getOpenFileNames is canceled') def returned_file_list_test(self): """ @@ -52,20 +52,20 @@ class TestFileDialog(TestCase): self.mocked_os.rest() self.mocked_qt_gui.reset() - # GIVEN: A List of known values as a return value from QFileDialog.getOpenFileNamesand a list of valid + # GIVEN: A List of known values as a return value from QFileDialog.getOpenFileNames and a list of valid # file names. self.mocked_qt_gui.QFileDialog.getOpenFileNames.return_value = [ - u'/Valid File', u'/url%20encoded%20file%20%231', u'/non-existing'] + '/Valid File', '/url%20encoded%20file%20%231', '/non-existing'] self.mocked_os.path.exists.side_effect = lambda file_name: file_name in [ - u'/Valid File', u'/url encoded file #1'] + '/Valid File', '/url encoded file #1'] # WHEN: FileDialog.getOpenFileNames is called result = FileDialog.getOpenFileNames(self.mocked_parent) # THEN: os.path.exists should have been called with known args. QmessageBox.information should have been # called. The returned result should corrilate with the input. - self.mocked_os.path.exists.assert_has_calls([call(u'/Valid File'), call(u'/url%20encoded%20file%20%231'), - call(u'/url encoded file #1'), call(u'/non-existing'), call(u'/non-existing')]) + self.mocked_os.path.exists.assert_has_calls([call('/Valid File'), call('/url%20encoded%20file%20%231'), + call('/url encoded file #1'), call('/non-existing'), call('/non-existing')]) self.mocked_qt_gui.QmessageBox.information.called_with(self.mocked_parent, UiStrings().FileNotFound, - UiStrings().FileNotFoundMessage % u'/non-existing') - self.assertEqual(result, [u'/Valid File', u'/url encoded file #1'], u'The returned file list is incorrect') \ No newline at end of file + UiStrings().FileNotFoundMessage % '/non-existing') + self.assertEqual(result, ['/Valid File', '/url encoded file #1'], 'The returned file list is incorrect') \ No newline at end of file From e8302e9e95ab2112dba7e8b04c96ec026a4663c8 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Tue, 24 Sep 2013 21:40:51 +0100 Subject: [PATCH 07/23] Added test for bug 1184869 --- .../openlp_plugins/bibles/test_http.py | 180 ++++++++++++++++++ 1 file changed, 180 insertions(+) create mode 100644 tests/functional/openlp_plugins/bibles/test_http.py diff --git a/tests/functional/openlp_plugins/bibles/test_http.py b/tests/functional/openlp_plugins/bibles/test_http.py new file mode 100644 index 000000000..e98c38c97 --- /dev/null +++ b/tests/functional/openlp_plugins/bibles/test_http.py @@ -0,0 +1,180 @@ +# -*- coding: utf-8 -*- +# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4 + +############################################################################### +# OpenLP - Open Source Lyrics Projection # +# --------------------------------------------------------------------------- # +# Copyright (c) 2008-2013 Raoul Snyman # +# Portions copyright (c) 2008-2013 Tim Bentley, Gerald Britton, Jonathan # +# Corwin, Samuel Findlay, Michael Gorven, Scott Guerrieri, Matthias Hub, # +# Meinert Jordan, Armin Köhler, Erik Lundin, Edwin Lunando, Brian T. Meyer. # +# Joshua Miller, Stevan Pettit, Andreas Preikschat, Mattias Põldaru, # +# Christian Richter, Philip Ridout, Simon Scudder, Jeffrey Smith, # +# Maikel Stuivenberg, Martin Thompson, Jon Tibble, Dave Warnock, # +# Frode Woldsund, Martin Zibricky, Patrick Zimmermann # +# --------------------------------------------------------------------------- # +# 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 # +############################################################################### +""" +This module contains tests for the http module of the Bibles plugin. +""" +from unittest import TestCase +from mock import MagicMock, patch +from bs4 import BeautifulSoup + +from openlp.plugins.bibles.lib.http import BSExtract + +#TODO: Items left to test +# BGExtract +# __init__ +# _remove_elements +# _extract_verse +# _clean_soup +# _extract_verses +# _extract_verses_old +# get_bible_chapter +# get_books_from_http +# _get_application +# CWExtract +# __init__ +# get_bible_chapter +# get_books_from_http +# _get_application +# HTTPBible +# __init__ +# do_import +# get_verses +# get_chapter +# get_books +# get_chapter_count +# get_verse_count +# _get_application +# get_soup_for_bible_ref +# send_error_message + +class TestBSExtract(TestCase): + """ + Test the BSExtractClass + """ + #TODO: Items left to test + # BSExtract + # __init__ + # get_bible_chapter + # get_books_from_http + # _get_application + def setUp(self): + self.get_soup_for_bible_ref_patcher = patch('openlp.plugins.bibles.lib.http.get_soup_for_bible_ref') + self.log_patcher = patch('openlp.plugins.bibles.lib.http.log') + self.send_error_message_patcher = patch('openlp.plugins.bibles.lib.http.send_error_message') + self.socket_patcher = patch('openlp.plugins.bibles.lib.http.socket') + self.urllib_patcher = patch('openlp.plugins.bibles.lib.http.urllib') + + self.mock_get_soup_for_bible_ref = self.get_soup_for_bible_ref_patcher.start() + self.mock_log = self.log_patcher.start() + self.mock_send_error_message = self.send_error_message_patcher.start() + self.mock_socket = self.socket_patcher.start() + self.mock_soup = MagicMock() + self.mock_urllib = self.urllib_patcher.start() + + def tearDown(self): + self.get_soup_for_bible_ref_patcher.stop() + self.log_patcher.stop() + self.send_error_message_patcher.stop() + self.socket_patcher.stop() + self.urllib_patcher.stop() + + def get_books_from_http_no_soup_test(self): + """ + Test the get_books_from_http method when get_soup_for_bible_ref returns a falsey value + """ + # GIVEN: An instance of BSExtract, and reset log, urllib & get_soup_for_bible_ref mocks + instance = BSExtract() + self.mock_log.debug.reset_mock() + self.mock_urllib.reset_mock() + self.mock_get_soup_for_bible_ref.reset_mock() + + # WHEN: get_books_from_http is called with 'NIV' and get_soup_for_bible_ref returns a None value + self.mock_urllib.parse.quote.return_value = 'NIV' + self.mock_get_soup_for_bible_ref.return_value = None + result = instance.get_books_from_http('NIV') + + # THEN: The rest mocks should be called with known values and get_books_from_http should return None + self.mock_log.debug.assert_called_once_with('BSExtract.get_books_from_http("%s")', 'NIV') + self.mock_urllib.parse.quote.assert_called_once_with(b'NIV') + self.mock_get_soup_for_bible_ref.assert_called_once_with( + 'http://m.bibleserver.com/overlay/selectBook?translation=NIV') + self.assertIsNone(result, + 'BSExtract.get_books_from_http should return None when get_soup_for_bible_ref returns a false value') + + def get_books_from_http_no_content_test(self): + """ + Test the get_books_from_http method when the specified element cannot be found in the tag object returned from + get_soup_for_bible_ref + """ + # GIVEN: An instance of BSExtract, and reset log, urllib, get_soup_for_bible_ref & soup mocks + instance = BSExtract() + self.mock_log.reset_mock() + self.mock_urllib.reset_mock() + self.mock_get_soup_for_bible_ref.reset_mock() + self.mock_soup.reset_mock() + + # WHEN: get_books_from_http is called with 'NIV', get_soup_for_bible_ref returns a mocked_soup object and + # mocked_soup.find returns None + self.mock_urllib.parse.quote.return_value = 'NIV' + self.mock_soup.find.return_value = None + self.mock_get_soup_for_bible_ref.return_value = self.mock_soup + result = instance.get_books_from_http('NIV') + + # THEN: The rest mocks should be called with known values and get_books_from_http should return None + self.mock_log.debug.assert_called_once_with('BSExtract.get_books_from_http("%s")', 'NIV') + self.mock_urllib.parse.quote.assert_called_once_with(b'NIV') + self.mock_get_soup_for_bible_ref.assert_called_once_with( + 'http://m.bibleserver.com/overlay/selectBook?translation=NIV') + self.mock_soup.find.assert_called_once_with('ul') + self.mock_log.error.assert_called_once_with('No books found in the Bibleserver response.') + self.mock_send_error_message.assert_called_once_with('parse') + self.assertIsNone(result, + 'BSExtract.get_books_from_http should return None when get_soup_for_bible_ref returns a false value') + + def get_books_from_http_content_test(self): + """ + Test the get_books_from_http method with sample HTML + Also a regression test for bug #1184869. (The anchor tag in the second list item is empty) + """ + # GIVEN: An instance of BSExtract, and reset log, urllib & get_soup_for_bible_ref mocks and sample HTML data + self.test_html = '' + self.test_soup = BeautifulSoup(self.test_html) + instance = BSExtract() + self.mock_log.reset_mock() + self.mock_urllib.reset_mock() + self.mock_get_soup_for_bible_ref.reset_mock() + self.mock_send_error_message.reset_mock() + + # WHEN: get_books_from_http is called with 'NIV' and get_soup_for_bible_ref returns tag object based on the + # supplied test data. + self.mock_urllib.parse.quote.return_value = 'NIV' + self.mock_get_soup_for_bible_ref.return_value = self.test_soup + result = instance.get_books_from_http('NIV') + + # THEN: The rest mocks should be called with known values and get_books_from_http should return the two books + # in the test data + self.mock_log.debug.assert_called_once_with('BSExtract.get_books_from_http("%s")', 'NIV') + self.mock_urllib.parse.quote.assert_called_once_with(b'NIV') + self.mock_get_soup_for_bible_ref.assert_called_once_with( + 'http://m.bibleserver.com/overlay/selectBook?translation=NIV') + self.assertFalse(self.mock_log.error.called, 'log.error should not have been called') + self.assertFalse(self.mock_send_error_message.called, 'send_error_message should not have been called') + self.assertEquals(result, ['Genesis', 'Leviticus']) From 43f99b81ebf41b2f19ef82c2fcb7b65ad7013e8e Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Tue, 22 Oct 2013 17:37:56 +0200 Subject: [PATCH 08/23] This variable has been renamed. --- openlp/plugins/presentations/lib/messagelistener.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openlp/plugins/presentations/lib/messagelistener.py b/openlp/plugins/presentations/lib/messagelistener.py index 6ab723c9e..259e65281 100644 --- a/openlp/plugins/presentations/lib/messagelistener.py +++ b/openlp/plugins/presentations/lib/messagelistener.py @@ -281,7 +281,7 @@ class Controller(object): class MessageListener(object): """ - This is the Presentation listener who acts on events from the slide controller and passes the messages on the the + This is the Presentation listener who acts on events from the slide controller and passes the messages on the correct presentation handlers """ log.info('Message Listener loaded') @@ -316,7 +316,7 @@ class MessageListener(object): hide_mode = message[2] file = item.get_frame_path() self.handler = item.processor - if self.handler == self.media_item.Automatic: + if self.handler == self.media_item.automatic: self.handler = self.media_item.findControllerByType(file) if not self.handler: return From b1873016c9a4b7e32239dde155a0cc53450528fb Mon Sep 17 00:00:00 2001 From: Tim Bentley Date: Wed, 23 Oct 2013 20:25:46 +0100 Subject: [PATCH 09/23] Move and tests --- openlp/core/ui/thememanager.py | 26 ++--- openlp/core/ui/thememanagerhelper.py | 26 ++++- .../functional/openlp_core_common/__init__.py | 29 +++++- .../openlp_core_common/test_applocation.py | 1 - .../functional/openlp_core_lib/test_theme.py | 10 +- .../openlp_core_ui/test_thememanagerhelper.py | 99 +++++++++++++++++++ 6 files changed, 167 insertions(+), 24 deletions(-) create mode 100644 tests/interfaces/openlp_core_ui/test_thememanagerhelper.py diff --git a/openlp/core/ui/thememanager.py b/openlp/core/ui/thememanager.py index 8e1838d5d..7e55a8b2c 100644 --- a/openlp/core/ui/thememanager.py +++ b/openlp/core/ui/thememanager.py @@ -33,7 +33,6 @@ import os import zipfile import shutil import logging -import re from xml.etree.ElementTree import ElementTree, XML from PyQt4 import QtCore, QtGui @@ -59,7 +58,7 @@ class ThemeManager(QtGui.QWidget, ThemeManagerHelper): """ super(ThemeManager, self).__init__(parent) Registry().register('theme_manager', self) - Registry().register_function('bootstrap_initialise', self.load_first_time_themes) + Registry().register_function('bootstrap_initialise', self.initialise) Registry().register_function('bootstrap_post_set_up', self._push_themes) self.settings_section = 'themes' self.theme_form = ThemeForm(self) @@ -135,15 +134,7 @@ class ThemeManager(QtGui.QWidget, ThemeManagerHelper): Registry().register_function('theme_update_global', self.change_global_from_tab) # Variables self.theme_list = [] - self.path = AppLocation.get_section_data_path(self.settings_section) - check_directory_exists(self.path) - self.thumb_path = os.path.join(self.path, 'thumbnails') - check_directory_exists(self.thumb_path) - self.theme_form.path = self.path self.old_background_image = None - self.bad_v1_name_chars = re.compile(r'[%+\[\]]') - # Last little bits of setting up - self.global_theme = Settings().value(self.settings_section + '/global theme') def check_list_state(self, item): """ @@ -392,6 +383,7 @@ class ThemeManager(QtGui.QWidget, ThemeManagerHelper): """ Imports any themes on start up and makes sure there is at least one theme """ + log.debug('load_first_time_themes called') self.application.set_busy_cursor() files = AppLocation.get_files(self.settings_section, '.otz') for theme_file in files: @@ -410,8 +402,8 @@ class ThemeManager(QtGui.QWidget, ThemeManagerHelper): def load_themes(self): """ - Loads the theme lists and triggers updates across the whole system - using direct calls or core functions and events for the plugins. + Loads the theme lists and triggers updates across the whole system using direct calls or core functions and + events for the plugins. The plugins will call back in to get the real list if they want it. """ log.debug('Load themes from dir') @@ -636,18 +628,18 @@ class ThemeManager(QtGui.QWidget, ThemeManagerHelper): self.main_window.finished_progress_bar() self.load_themes() - def generate_image(self, theme_data, forcePage=False): + def generate_image(self, theme_data, force_page=False): """ Call the renderer to build a Sample Image ``theme_data`` The theme to generated a preview for. - ``forcePage`` + ``force_page`` Flag to tell message lines per page need to be generated. """ log.debug('generate_image \n%s ', theme_data) - return self.renderer.generate_preview(theme_data, forcePage) + return self.renderer.generate_preview(theme_data, force_page) def get_preview_image(self, theme): """ @@ -672,7 +664,7 @@ class ThemeManager(QtGui.QWidget, ThemeManagerHelper): theme.extend_image_filename(path) return theme - def _validate_theme_action(self, select_text, confirm_title, confirm_text, testPlugin=True, confirm=True): + def _validate_theme_action(self, select_text, confirm_title, confirm_text, test_plugin=True, confirm=True): """ Check to see if theme has been selected and the destructive action is allowed. @@ -694,7 +686,7 @@ class ThemeManager(QtGui.QWidget, ThemeManagerHelper): message=translate('OpenLP.ThemeManager', 'You are unable to delete the default theme.')) return False # check for use in the system else where. - if testPlugin: + if test_plugin: for plugin in self.plugin_manager.plugins: if plugin.uses_theme(theme): critical_error_message_box(translate('OpenLP.ThemeManager', 'Validation Error'), diff --git a/openlp/core/ui/thememanagerhelper.py b/openlp/core/ui/thememanagerhelper.py index 8fa02bde5..bc04b3bd9 100644 --- a/openlp/core/ui/thememanagerhelper.py +++ b/openlp/core/ui/thememanagerhelper.py @@ -29,10 +29,34 @@ """ The Theme Controller helps manages adding, deleteing and modifying of themes. """ +import logging +import os + +from openlp.core.common import AppLocation, Settings, check_directory_exists + +log = logging.getLogger(__name__) class ThemeManagerHelper(object): """ Manages the non ui theme functions. """ - pass \ No newline at end of file + def initialise(self): + """ + Setup the manager + """ + log.debug('initialise called') + self.global_theme = Settings().value(self.settings_section + '/global theme') + self.build_theme_path() + self.load_first_time_themes() + + def build_theme_path(self): + """ + Set up the theme path variables + """ + log.debug('build theme path called') + self.path = AppLocation.get_section_data_path(self.settings_section) + check_directory_exists(self.path) + self.thumb_path = os.path.join(self.path, 'thumbnails') + check_directory_exists(self.thumb_path) + self.theme_form.path = self.path \ No newline at end of file diff --git a/tests/functional/openlp_core_common/__init__.py b/tests/functional/openlp_core_common/__init__.py index f87606f07..64d028205 100644 --- a/tests/functional/openlp_core_common/__init__.py +++ b/tests/functional/openlp_core_common/__init__.py @@ -1 +1,28 @@ -__author__ = 'tim' +# -*- coding: utf-8 -*- +# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4 + +############################################################################### +# OpenLP - Open Source Lyrics Projection # +# --------------------------------------------------------------------------- # +# Copyright (c) 2008-2013 Raoul Snyman # +# Portions copyright (c) 2008-2013 Tim Bentley, Gerald Britton, Jonathan # +# Corwin, Samuel Findlay, Michael Gorven, Scott Guerrieri, Matthias Hub, # +# Meinert Jordan, Armin Köhler, Erik Lundin, Edwin Lunando, Brian T. Meyer. # +# Joshua Miller, Stevan Pettit, Andreas Preikschat, Mattias Põldaru, # +# Christian Richter, Philip Ridout, Simon Scudder, Jeffrey Smith, # +# Maikel Stuivenberg, Martin Thompson, Jon Tibble, Dave Warnock, # +# Frode Woldsund, Martin Zibricky, Patrick Zimmermann # +# --------------------------------------------------------------------------- # +# 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 # +############################################################################### diff --git a/tests/functional/openlp_core_common/test_applocation.py b/tests/functional/openlp_core_common/test_applocation.py index 0dcb2e6b1..0683bb050 100644 --- a/tests/functional/openlp_core_common/test_applocation.py +++ b/tests/functional/openlp_core_common/test_applocation.py @@ -59,7 +59,6 @@ class TestAppLocation(TestCase): # WHEN: we call AppLocation.get_data_path() data_path = AppLocation.get_data_path() - print(data_path) # THEN: check that all the correct methods were called, and the result is correct mocked_settings.contains.assert_called_with('advanced/data path') diff --git a/tests/functional/openlp_core_lib/test_theme.py b/tests/functional/openlp_core_lib/test_theme.py index 1ece64b34..cf8d1f661 100644 --- a/tests/functional/openlp_core_lib/test_theme.py +++ b/tests/functional/openlp_core_lib/test_theme.py @@ -62,9 +62,11 @@ class TestTheme(TestCase): # THEN: We should get some default behaviours self.assertTrue(default_theme.background_border_color == '#000000', 'The theme should have a black border') - self.assertTrue(default_theme.background_type == 'solid', 'There theme should have a solid backgrounds') + self.assertTrue(default_theme.background_type == 'solid', 'The theme should have a solid backgrounds') self.assertTrue(default_theme.display_vertical_align == 0, - 'There theme should have display_vertical_align of 0') + 'The theme should have a display_vertical_align of 0') self.assertTrue(default_theme.font_footer_name == "Arial", - 'There theme should has font_footer_name of Arial') - self.assertTrue(default_theme.font_main_bold is False, 'There theme should has font_main_bold of false') \ No newline at end of file + 'The theme should have a font_footer_name of Arial') + self.assertTrue(default_theme.font_main_bold is False, 'The theme should have a font_main_bold of false') + self.assertTrue(len(default_theme.__dict__) == 47, 'The theme should have 47 variables') + diff --git a/tests/interfaces/openlp_core_ui/test_thememanagerhelper.py b/tests/interfaces/openlp_core_ui/test_thememanagerhelper.py new file mode 100644 index 000000000..eb964a045 --- /dev/null +++ b/tests/interfaces/openlp_core_ui/test_thememanagerhelper.py @@ -0,0 +1,99 @@ +# -*- coding: utf-8 -*- +# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4 + +############################################################################### +# OpenLP - Open Source Lyrics Projection # +# --------------------------------------------------------------------------- # +# Copyright (c) 2008-2013 Raoul Snyman # +# Portions copyright (c) 2008-2013 Tim Bentley, Gerald Britton, Jonathan # +# Corwin, Samuel Findlay, Michael Gorven, Scott Guerrieri, Matthias Hub, # +# Meinert Jordan, Armin Köhler, Erik Lundin, Edwin Lunando, Brian T. Meyer. # +# Joshua Miller, Stevan Pettit, Andreas Preikschat, Mattias Põldaru, # +# Christian Richter, Philip Ridout, Simon Scudder, Jeffrey Smith, # +# Maikel Stuivenberg, Martin Thompson, Jon Tibble, Dave Warnock, # +# Frode Woldsund, Martin Zibricky, Patrick Zimmermann # +# --------------------------------------------------------------------------- # +# 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 # +############################################################################### +""" +Interface tests to test the thememanagerhelper class and related methods. +""" +import os +from unittest import TestCase +from tempfile import mkstemp + +from openlp.core.common import AppLocation, get_frozen_path +from openlp.core.common import Settings +from openlp.core.ui import ThemeManagerHelper +from tests.functional import patch, MagicMock + + +class TestThemeManagerHelper(TestCase): + """ + Test the functions in the ThemeManagerHelp[er module + """ + def setUp(self): + """ + Create the UI + """ + fd, self.ini_file = mkstemp('.ini') + Settings().set_filename(self.ini_file) + self.helper = ThemeManagerHelper() + self.helper.settings_section = "themes" + + def tearDown(self): + """ + Delete all the C++ objects at the end so that we don't have a segfault + """ + os.unlink(self.ini_file) + os.unlink(Settings().fileName()) + + def test_initialise(self): + """ + Test the thememanagerhelper initialise - basic test + """ + # GIVEN: A new a call to initialise + Settings().setValue('themes/global theme', 'my_theme') + self.helper.build_theme_path = MagicMock() + self.helper.load_first_time_themes = MagicMock() + + # WHEN: the initialistion is run + self.helper.initialise() + + # THEN: + self.assertEqual(1, self.helper.build_theme_path.call_count, + 'The function build_theme_path should have been called') + self.assertEqual(1, self.helper.load_first_time_themes.call_count, + 'The function load_first_time_themes should have been called') + self.assertEqual(self.helper.global_theme , 'my_theme', + 'The global theme should have been set to my_theme') + + def test_build_theme_path(self): + """ + Test the thememanagerhelper build_theme_path - basic test + """ + # GIVEN: A new a call to initialise + with patch('openlp.core.common.applocation.check_directory_exists') as mocked_check_directory_exists: + # GIVEN: A mocked out Settings class and a mocked out AppLocation.get_directory() + mocked_check_directory_exists.return_value = True + Settings().setValue('themes/global theme', 'my_theme') + + self.helper.theme_form = MagicMock() + #self.helper.load_first_time_themes = MagicMock() + + # WHEN: the build_theme_path is run + self.helper.build_theme_path() + + # THEN: + A=1 \ No newline at end of file From abdc3937ef60298d73a55dc389d856ef728a2b79 Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Sun, 27 Oct 2013 13:39:13 +0100 Subject: [PATCH 10/23] Small test for PresentationController Constructor --- .../lib/presentationcontroller.py | 7 ++- .../test_presentationcontroller.py | 48 +++++++++++++++++++ 2 files changed, 51 insertions(+), 4 deletions(-) create mode 100644 tests/functional/openlp_plugins/presentations/test_presentationcontroller.py diff --git a/openlp/plugins/presentations/lib/presentationcontroller.py b/openlp/plugins/presentations/lib/presentationcontroller.py index 0a70b174a..e137dc39c 100644 --- a/openlp/plugins/presentations/lib/presentationcontroller.py +++ b/openlp/plugins/presentations/lib/presentationcontroller.py @@ -292,8 +292,8 @@ class PresentationDocument(object): class PresentationController(object): """ - This class is used to control interactions with presentation applications by creating a runtime environment. This is - a base class for presentation controllers to inherit from. + This class is used to control interactions with presentation applications by creating a runtime environment. + This is a base class for presentation controllers to inherit from. To create a new controller, take a copy of this file and name it so it ends with ``controller.py``, i.e. ``foobarcontroller.py``. Make sure it inherits @@ -341,8 +341,7 @@ class PresentationController(object): """ log.info('PresentationController loaded') - def __init__(self, plugin=None, name='PresentationController', - document_class=PresentationDocument): + def __init__(self, plugin=None, name='PresentationController', document_class=PresentationDocument): """ This is the constructor for the presentationcontroller object. This provides an easy way for descendent plugins to populate common data. This method *must* be overridden, like so:: diff --git a/tests/functional/openlp_plugins/presentations/test_presentationcontroller.py b/tests/functional/openlp_plugins/presentations/test_presentationcontroller.py new file mode 100644 index 000000000..97d34f4f1 --- /dev/null +++ b/tests/functional/openlp_plugins/presentations/test_presentationcontroller.py @@ -0,0 +1,48 @@ +# -*- coding: utf-8 -*- +# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4 + +############################################################################### +# OpenLP - Open Source Lyrics Projection # +# --------------------------------------------------------------------------- # +# Copyright (c) 2008-2013 Raoul Snyman # +# Portions copyright (c) 2008-2013 Tim Bentley, Gerald Britton, Jonathan # +# Corwin, Samuel Findlay, Michael Gorven, Scott Guerrieri, Matthias Hub, # +# Meinert Jordan, Armin Köhler, Erik Lundin, Edwin Lunando, Brian T. Meyer. # +# Joshua Miller, Stevan Pettit, Andreas Preikschat, Mattias Põldaru, # +# Christian Richter, Philip Ridout, Simon Scudder, Jeffrey Smith, # +# Maikel Stuivenberg, Martin Thompson, Jon Tibble, Dave Warnock, # +# Frode Woldsund, Martin Zibricky, Patrick Zimmermann # +# --------------------------------------------------------------------------- # +# 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 # +############################################################################### +""" +This module contains tests for the Presentation Controller. +""" +from unittest import TestCase + +from openlp.plugins.presentations.lib.presentationcontroller import PresentationController +from tests.functional import MagicMock + +class TestPresentationController(TestCase): + """ + Test the mediaitem methods. + """ + def setUp(self): + """ + Set up the components need for all tests. + """ + self.pres_controller = PresentationController(plugin=MagicMock()) + + def constructor_test(self): + assert(self.pres_controller.name == "PresentationController") From 0ed0b2148e1bf84d99b03824cf5f150694150fb8 Mon Sep 17 00:00:00 2001 From: Samuel Mehrbrodt Date: Sun, 27 Oct 2013 21:33:58 +0100 Subject: [PATCH 11/23] Small changes to the test --- .../test_presentationcontroller.py | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/tests/functional/openlp_plugins/presentations/test_presentationcontroller.py b/tests/functional/openlp_plugins/presentations/test_presentationcontroller.py index 97d34f4f1..61d62b473 100644 --- a/tests/functional/openlp_plugins/presentations/test_presentationcontroller.py +++ b/tests/functional/openlp_plugins/presentations/test_presentationcontroller.py @@ -36,13 +36,18 @@ from tests.functional import MagicMock class TestPresentationController(TestCase): """ - Test the mediaitem methods. + Test the PresentationController. """ - def setUp(self): - """ - Set up the components need for all tests. - """ - self.pres_controller = PresentationController(plugin=MagicMock()) - def constructor_test(self): - assert(self.pres_controller.name == "PresentationController") + """ + Test the Constructor + """ + # GIVEN: No presentation controller + controller = None + + # WHEN: The presentation controller object is created + controller = PresentationController(plugin=MagicMock()) + + # THEN: The name of the presentation controller should be correct + self.assertEqual('PresentationController', controller.name, + 'The name of the presentation controller should be correct') From 20a1a2ad70d0be6f1341c3464596a847e4d41b7b Mon Sep 17 00:00:00 2001 From: Felipe Polo-Wood Date: Fri, 8 Nov 2013 13:26:27 -0500 Subject: [PATCH 12/23] Fixed router so response and headers are sent correctly Fixed tests so they run on Windows Added unit test for new method get_content_type --- openlp/plugins/remotes/lib/httprouter.py | 50 +++++++++++-------- .../openlp_plugins/remotes/test_remotetab.py | 3 +- .../openlp_plugins/remotes/test_router.py | 20 +++++++- 3 files changed, 48 insertions(+), 25 deletions(-) diff --git a/openlp/plugins/remotes/lib/httprouter.py b/openlp/plugins/remotes/lib/httprouter.py index ce13ed812..eb2b68e6b 100644 --- a/openlp/plugins/remotes/lib/httprouter.py +++ b/openlp/plugins/remotes/lib/httprouter.py @@ -346,30 +346,14 @@ class HttpRouter(object): path = os.path.normpath(os.path.join(self.html_dir, file_name)) if not path.startswith(self.html_dir): return self.do_not_found() - ext = os.path.splitext(file_name)[1] - html = None - if ext == '.html': - self.send_header('Content-type', 'text/html') - variables = self.template_vars - html = Template(filename=path, input_encoding='utf-8', output_encoding='utf-8').render(**variables) - elif ext == '.css': - self.send_header('Content-type', 'text/css') - elif ext == '.js': - self.send_header('Content-type', 'application/javascript') - elif ext == '.jpg': - self.send_header('Content-type', 'image/jpeg') - elif ext == '.gif': - self.send_header('Content-type', 'image/gif') - elif ext == '.ico': - self.send_header('Content-type', 'image/x-icon') - elif ext == '.png': - self.send_header('Content-type', 'image/png') - else: - self.send_header('Content-type', 'text/plain') + content = None + ext, content_type = self.get_content_type(path) file_handle = None try: - if html: - content = html + if ext == '.html': + variables = self.template_vars + content = Template(filename=path, input_encoding='utf-8', + output_encoding='utf-8').render(**variables) else: file_handle = open(path, 'rb') log.debug('Opened %s' % path) @@ -380,8 +364,30 @@ class HttpRouter(object): finally: if file_handle: file_handle.close() + self.send_response(200) + self.send_header('Content-type', content_type) + self.end_headers() return content + def get_content_type(self, file_name): + """ + Examines the extension of the file and determines + what header to send back + Returns the extension found + """ + content_type = 'text/plain' + file_types = {'.html': 'text/html', + '.css': 'text/css', + '.js': 'application/javascript', + '.jpg': 'image/jpeg', + '.gif': 'image/gif', + '.ico': 'image/x-icon', + '.png': 'image/png' + } + ext = os.path.splitext(file_name)[1] + content_type = file_types.get(ext, 'text/plain') + return ext, content_type + def poll(self): """ Poll OpenLP to determine the current slide number and item name. diff --git a/tests/functional/openlp_plugins/remotes/test_remotetab.py b/tests/functional/openlp_plugins/remotes/test_remotetab.py index 067c5cff1..52aeeee99 100644 --- a/tests/functional/openlp_plugins/remotes/test_remotetab.py +++ b/tests/functional/openlp_plugins/remotes/test_remotetab.py @@ -62,7 +62,7 @@ class TestRemoteTab(TestCase): """ Create the UI """ - fd, self.ini_file = mkstemp('.ini') + self.fd, self.ini_file = mkstemp('.ini') Settings().set_filename(self.ini_file) self.application = QtGui.QApplication.instance() Settings().extend_default_settings(__default_settings__) @@ -76,6 +76,7 @@ class TestRemoteTab(TestCase): del self.application del self.parent del self.form + os.close(self.fd) os.unlink(self.ini_file) def get_ip_address_default_test(self): diff --git a/tests/functional/openlp_plugins/remotes/test_router.py b/tests/functional/openlp_plugins/remotes/test_router.py index a9ba16bf8..6762f7643 100644 --- a/tests/functional/openlp_plugins/remotes/test_router.py +++ b/tests/functional/openlp_plugins/remotes/test_router.py @@ -59,7 +59,7 @@ class TestRouter(TestCase): """ Create the UI """ - fd, self.ini_file = mkstemp('.ini') + self.fd, self.ini_file = mkstemp('.ini') Settings().set_filename(self.ini_file) self.application = QtGui.QApplication.instance() Settings().extend_default_settings(__default_settings__) @@ -70,6 +70,7 @@ class TestRouter(TestCase): Delete all the C++ objects at the end so that we don't have a segfault """ del self.application + os.close(self.fd) os.unlink(self.ini_file) def password_encrypter_test(self): @@ -109,4 +110,19 @@ class TestRouter(TestCase): assert function['function'] == mocked_function, \ 'The mocked function should match defined value.' assert function['secure'] == False, \ - 'The mocked function should not require any security.' \ No newline at end of file + 'The mocked function should not require any security.' + + def get_content_type_test(self): + """ + Test the get_content_type logic + """ + headers = [ ['test.html', 'text/html'], ['test.css', 'text/css'], + ['test.js', 'application/javascript'], ['test.jpg', 'image/jpeg'], + ['test.gif', 'image/gif'], ['test.ico', 'image/x-icon'], + ['test.png', 'image/png'], ['test.whatever', 'text/plain'], + ['test', 'text/plain'], ['', 'text/plain'], + ['/test/test.html', 'text/html'], + ['c:\\test\\test.html', 'text/html']] + for header in headers: + ext, content_type = self.router.get_content_type(header[0]) + self.assertEqual(content_type, header[1], 'Mismatch of content type') From 45bcbe92728fd5d822c3d86d9c3612827e3de4fe Mon Sep 17 00:00:00 2001 From: Felipe Polo-Wood Date: Sun, 10 Nov 2013 19:13:03 -0500 Subject: [PATCH 13/23] Moved the file_types to module level so they get processed once --- openlp/plugins/remotes/lib/httprouter.py | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/openlp/plugins/remotes/lib/httprouter.py b/openlp/plugins/remotes/lib/httprouter.py index eb2b68e6b..b3652d4df 100644 --- a/openlp/plugins/remotes/lib/httprouter.py +++ b/openlp/plugins/remotes/lib/httprouter.py @@ -128,6 +128,15 @@ from openlp.core.common import AppLocation, Settings, translate from openlp.core.lib import Registry, PluginStatus, StringContent, image_to_byte log = logging.getLogger(__name__) +file_types = { + '.html': 'text/html', + '.css': 'text/css', + '.js': 'application/javascript', + '.jpg': 'image/jpeg', + '.gif': 'image/gif', + '.ico': 'image/x-icon', + '.png': 'image/png' +} class HttpRouter(object): @@ -372,18 +381,10 @@ class HttpRouter(object): def get_content_type(self, file_name): """ Examines the extension of the file and determines - what header to send back - Returns the extension found + what the content_type should be, defaults to text/plain + Returns the extension and the content_type """ content_type = 'text/plain' - file_types = {'.html': 'text/html', - '.css': 'text/css', - '.js': 'application/javascript', - '.jpg': 'image/jpeg', - '.gif': 'image/gif', - '.ico': 'image/x-icon', - '.png': 'image/png' - } ext = os.path.splitext(file_name)[1] content_type = file_types.get(ext, 'text/plain') return ext, content_type From 649494589a0cf38694203b0f321ad08afb27cc7a Mon Sep 17 00:00:00 2001 From: Felipe Polo-Wood Date: Sun, 10 Nov 2013 19:55:06 -0500 Subject: [PATCH 14/23] Added unit tests for serve_file --- .../openlp_plugins/remotes/test_router.py | 47 ++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/tests/functional/openlp_plugins/remotes/test_router.py b/tests/functional/openlp_plugins/remotes/test_router.py index 6762f7643..5475405a0 100644 --- a/tests/functional/openlp_plugins/remotes/test_router.py +++ b/tests/functional/openlp_plugins/remotes/test_router.py @@ -37,7 +37,7 @@ from PyQt4 import QtGui from openlp.core.common import Settings from openlp.plugins.remotes.lib.httpserver import HttpRouter -from tests.functional import MagicMock +from mock import MagicMock, patch, mock_open __default_settings__ = { 'remotes/twelve hour': True, @@ -116,6 +116,7 @@ class TestRouter(TestCase): """ Test the get_content_type logic """ + # GIVEN: a set of files and their corresponding types headers = [ ['test.html', 'text/html'], ['test.css', 'text/css'], ['test.js', 'application/javascript'], ['test.jpg', 'image/jpeg'], ['test.gif', 'image/gif'], ['test.ico', 'image/x-icon'], @@ -123,6 +124,50 @@ class TestRouter(TestCase): ['test', 'text/plain'], ['', 'text/plain'], ['/test/test.html', 'text/html'], ['c:\\test\\test.html', 'text/html']] + # WHEN: calling each file type for header in headers: ext, content_type = self.router.get_content_type(header[0]) + # THEN: all types should match self.assertEqual(content_type, header[1], 'Mismatch of content type') + + def serve_file_without_params_test(self): + """ + Test the serve_file method without params + """ + # GIVEN: mocked environment + self.router.send_response = MagicMock() + self.router.send_header = MagicMock() + self.router.end_headers = MagicMock() + self.router.wfile = MagicMock() + self.router.html_dir = os.path.normpath('test/dir') + self.router.template_vars = MagicMock() + # WHEN: call serve_file with no file_name + self.router.serve_file() + # THEN: it should return a 404 + self.router.send_response.assert_called_once_with(404) + self.router.send_header.assert_called_once_with('Content-type','text/html') + self.assertEqual(self.router.end_headers.call_count, 1, + 'end_headers called once') + + def serve_file_with_valid_params_test(self): + """ + Test the serve_file method with an existing file + """ + # GIVEN: mocked environment + self.router.send_response = MagicMock() + self.router.send_header = MagicMock() + self.router.end_headers = MagicMock() + self.router.wfile = MagicMock() + self.router.html_dir = os.path.normpath('test/dir') + self.router.template_vars = MagicMock() + with patch('openlp.core.lib.os.path.exists') as mocked_exists, \ + patch('builtins.open', mock_open(read_data='123')): + mocked_exists.return_value = True + # WHEN: call serve_file with an existing html file + self.router.serve_file(os.path.normpath('test/dir/test.html')) + # THEN: it should return a 200 and the file + self.router.send_response.assert_called_once_with(200) + self.router.send_header.assert_called_once_with( + 'Content-type','text/html') + self.assertEqual(self.router.end_headers.call_count, 1, + 'end_headers called once') From e56414e22c3c82855e4eec426afb29acd6b535cb Mon Sep 17 00:00:00 2001 From: Felipe Polo-Wood Date: Thu, 14 Nov 2013 15:33:46 -0500 Subject: [PATCH 15/23] Fixed OS specific string Coding standard fixes --- openlp/plugins/remotes/lib/httprouter.py | 7 +++---- tests/functional/openlp_plugins/remotes/test_router.py | 7 ++++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/openlp/plugins/remotes/lib/httprouter.py b/openlp/plugins/remotes/lib/httprouter.py index b3652d4df..deba92c10 100644 --- a/openlp/plugins/remotes/lib/httprouter.py +++ b/openlp/plugins/remotes/lib/httprouter.py @@ -128,7 +128,7 @@ from openlp.core.common import AppLocation, Settings, translate from openlp.core.lib import Registry, PluginStatus, StringContent, image_to_byte log = logging.getLogger(__name__) -file_types = { +FILE_TYPES = { '.html': 'text/html', '.css': 'text/css', '.js': 'application/javascript', @@ -361,8 +361,7 @@ class HttpRouter(object): try: if ext == '.html': variables = self.template_vars - content = Template(filename=path, input_encoding='utf-8', - output_encoding='utf-8').render(**variables) + content = Template(filename=path, input_encoding='utf-8', output_encoding='utf-8').render(**variables) else: file_handle = open(path, 'rb') log.debug('Opened %s' % path) @@ -386,7 +385,7 @@ class HttpRouter(object): """ content_type = 'text/plain' ext = os.path.splitext(file_name)[1] - content_type = file_types.get(ext, 'text/plain') + content_type = FILE_TYPES.get(ext, 'text/plain') return ext, content_type def poll(self): diff --git a/tests/functional/openlp_plugins/remotes/test_router.py b/tests/functional/openlp_plugins/remotes/test_router.py index 5475405a0..4d7da2b91 100644 --- a/tests/functional/openlp_plugins/remotes/test_router.py +++ b/tests/functional/openlp_plugins/remotes/test_router.py @@ -37,7 +37,8 @@ from PyQt4 import QtGui from openlp.core.common import Settings from openlp.plugins.remotes.lib.httpserver import HttpRouter -from mock import MagicMock, patch, mock_open +from tests.functional import MagicMock, patch +from mock import mock_open __default_settings__ = { 'remotes/twelve hour': True, @@ -50,6 +51,7 @@ __default_settings__ = { 'remotes/ip address': '0.0.0.0' } +TEST_PATH = os.path.abspath(os.path.dirname(__file__)) class TestRouter(TestCase): """ @@ -122,8 +124,7 @@ class TestRouter(TestCase): ['test.gif', 'image/gif'], ['test.ico', 'image/x-icon'], ['test.png', 'image/png'], ['test.whatever', 'text/plain'], ['test', 'text/plain'], ['', 'text/plain'], - ['/test/test.html', 'text/html'], - ['c:\\test\\test.html', 'text/html']] + [os.path.join(TEST_PATH,'test.html'), 'text/html']] # WHEN: calling each file type for header in headers: ext, content_type = self.router.get_content_type(header[0]) From 81c2f1a4b3bd069ce69d0a1d1e98492259585807 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Sat, 16 Nov 2013 21:04:16 +0000 Subject: [PATCH 16/23] updated tests --- openlp/core/lib/filedialog.py | 2 +- tests/functional/openlp_core_lib/test_file_dialog.py | 12 +++++++----- tests/functional/openlp_core_lib/test_htmlbuilder.py | 2 +- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/openlp/core/lib/filedialog.py b/openlp/core/lib/filedialog.py index 03a8b8c47..bac1b5ce2 100644 --- a/openlp/core/lib/filedialog.py +++ b/openlp/core/lib/filedialog.py @@ -36,7 +36,7 @@ from urllib import parse from PyQt4 import QtGui -from openlp.core.lib import UiStrings +from openlp.core.common import UiStrings log = logging.getLogger(__name__) diff --git a/tests/functional/openlp_core_lib/test_file_dialog.py b/tests/functional/openlp_core_lib/test_file_dialog.py index 0a5736ff0..f42a865d7 100644 --- a/tests/functional/openlp_core_lib/test_file_dialog.py +++ b/tests/functional/openlp_core_lib/test_file_dialog.py @@ -1,12 +1,11 @@ """ Package to test the openlp.core.lib.filedialog package. """ -import copy from unittest import TestCase -from mock import MagicMock, call, patch +from openlp.core.common import UiStrings from openlp.core.lib.filedialog import FileDialog -from openlp.core.lib.uistrings import UiStrings +from tests.functional import MagicMock, patch class TestFileDialog(TestCase): """ @@ -64,8 +63,11 @@ class TestFileDialog(TestCase): # THEN: os.path.exists should have been called with known args. QmessageBox.information should have been # called. The returned result should corrilate with the input. - self.mocked_os.path.exists.assert_has_calls([call('/Valid File'), call('/url%20encoded%20file%20%231'), - call('/url encoded file #1'), call('/non-existing'), call('/non-existing')]) + self.mocked_os.path.exists.assert_callde_with('/Valid File') + self.mocked_os.path.exists.assert_callde_with('/url%20encoded%20file%20%231') + self.mocked_os.path.exists.assert_callde_with('/url encoded file #1') + self.mocked_os.path.exists.assert_callde_with('/non-existing') + self.mocked_os.path.exists.assert_callde_with('/non-existing') self.mocked_qt_gui.QmessageBox.information.called_with(self.mocked_parent, UiStrings().FileNotFound, UiStrings().FileNotFoundMessage % '/non-existing') self.assertEqual(result, ['/Valid File', '/url encoded file #1'], 'The returned file list is incorrect') \ No newline at end of file diff --git a/tests/functional/openlp_core_lib/test_htmlbuilder.py b/tests/functional/openlp_core_lib/test_htmlbuilder.py index 0ffab5458..fafa277d7 100644 --- a/tests/functional/openlp_core_lib/test_htmlbuilder.py +++ b/tests/functional/openlp_core_lib/test_htmlbuilder.py @@ -3,13 +3,13 @@ Package to test the openlp.core.lib.htmlbuilder module. """ from unittest import TestCase -from mock import MagicMock, patch from PyQt4 import QtCore from openlp.core.lib.htmlbuilder import build_html, build_background_css, build_lyrics_css, build_lyrics_outline_css, \ build_lyrics_format_css, build_footer_css from openlp.core.lib.theme import HorizontalType, VerticalType +from tests.functional import MagicMock, patch HTML = """ From f32409b5a528061adc9b1f7f1aba599ec012f04e Mon Sep 17 00:00:00 2001 From: Tim Bentley Date: Fri, 6 Dec 2013 19:00:37 +0000 Subject: [PATCH 17/23] format changes --- openlp/plugins/presentations/lib/messagelistener.py | 2 +- tests/interfaces/openlp_core_ui/test_thememanagerhelper.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/openlp/plugins/presentations/lib/messagelistener.py b/openlp/plugins/presentations/lib/messagelistener.py index 6ab723c9e..43f93cf2c 100644 --- a/openlp/plugins/presentations/lib/messagelistener.py +++ b/openlp/plugins/presentations/lib/messagelistener.py @@ -316,7 +316,7 @@ class MessageListener(object): hide_mode = message[2] file = item.get_frame_path() self.handler = item.processor - if self.handler == self.media_item.Automatic: + if self.handler == self.media_item.automatic: self.handler = self.media_item.findControllerByType(file) if not self.handler: return diff --git a/tests/interfaces/openlp_core_ui/test_thememanagerhelper.py b/tests/interfaces/openlp_core_ui/test_thememanagerhelper.py index eb964a045..3209fa61a 100644 --- a/tests/interfaces/openlp_core_ui/test_thememanagerhelper.py +++ b/tests/interfaces/openlp_core_ui/test_thememanagerhelper.py @@ -73,11 +73,11 @@ class TestThemeManagerHelper(TestCase): # THEN: self.assertEqual(1, self.helper.build_theme_path.call_count, - 'The function build_theme_path should have been called') + 'The function build_theme_path should have been called') self.assertEqual(1, self.helper.load_first_time_themes.call_count, - 'The function load_first_time_themes should have been called') + 'The function load_first_time_themes should have been called only once') self.assertEqual(self.helper.global_theme , 'my_theme', - 'The global theme should have been set to my_theme') + 'The global theme should have been set to my_theme') def test_build_theme_path(self): """ From 9fd590bb7041b181351960af653983f422c3ee34 Mon Sep 17 00:00:00 2001 From: Tim Bentley Date: Fri, 6 Dec 2013 19:07:02 +0000 Subject: [PATCH 18/23] clean up tests --- tests/interfaces/openlp_core_ui/test_thememanagerhelper.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/interfaces/openlp_core_ui/test_thememanagerhelper.py b/tests/interfaces/openlp_core_ui/test_thememanagerhelper.py index 3209fa61a..aa70edd51 100644 --- a/tests/interfaces/openlp_core_ui/test_thememanagerhelper.py +++ b/tests/interfaces/openlp_core_ui/test_thememanagerhelper.py @@ -33,7 +33,6 @@ import os from unittest import TestCase from tempfile import mkstemp -from openlp.core.common import AppLocation, get_frozen_path from openlp.core.common import Settings from openlp.core.ui import ThemeManagerHelper from tests.functional import patch, MagicMock @@ -76,7 +75,7 @@ class TestThemeManagerHelper(TestCase): 'The function build_theme_path should have been called') self.assertEqual(1, self.helper.load_first_time_themes.call_count, 'The function load_first_time_themes should have been called only once') - self.assertEqual(self.helper.global_theme , 'my_theme', + self.assertEqual(self.helper.global_theme, 'my_theme', 'The global theme should have been set to my_theme') def test_build_theme_path(self): @@ -96,4 +95,5 @@ class TestThemeManagerHelper(TestCase): self.helper.build_theme_path() # THEN: - A=1 \ No newline at end of file + self.assertEqual(self.helper.path, self.helper.theme_form.path, + 'The theme path and the main path should be the same value') \ No newline at end of file From 440cefa5f677d81b083e5c06b1134a254c54d4e5 Mon Sep 17 00:00:00 2001 From: Tim Bentley Date: Fri, 6 Dec 2013 19:09:29 +0000 Subject: [PATCH 19/23] formatting --- tests/functional/openlp_core_lib/test_theme.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/functional/openlp_core_lib/test_theme.py b/tests/functional/openlp_core_lib/test_theme.py index cf8d1f661..abedc230b 100644 --- a/tests/functional/openlp_core_lib/test_theme.py +++ b/tests/functional/openlp_core_lib/test_theme.py @@ -64,9 +64,9 @@ class TestTheme(TestCase): self.assertTrue(default_theme.background_border_color == '#000000', 'The theme should have a black border') self.assertTrue(default_theme.background_type == 'solid', 'The theme should have a solid backgrounds') self.assertTrue(default_theme.display_vertical_align == 0, - 'The theme should have a display_vertical_align of 0') + 'The theme should have a display_vertical_align of 0') self.assertTrue(default_theme.font_footer_name == "Arial", - 'The theme should have a font_footer_name of Arial') + 'The theme should have a font_footer_name of Arial') self.assertTrue(default_theme.font_main_bold is False, 'The theme should have a font_main_bold of false') self.assertTrue(len(default_theme.__dict__) == 47, 'The theme should have 47 variables') From 2a262e4ef68e3bda5bff2ed716008b41057bf016 Mon Sep 17 00:00:00 2001 From: Jonathan Springer Date: Sun, 8 Dec 2013 21:05:52 -0500 Subject: [PATCH 20/23] pass all keyboard events to shortcut dialog --- openlp/core/ui/shortcutlistform.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/core/ui/shortcutlistform.py b/openlp/core/ui/shortcutlistform.py index efe876e3e..fb459824d 100644 --- a/openlp/core/ui/shortcutlistform.py +++ b/openlp/core/ui/shortcutlistform.py @@ -74,7 +74,7 @@ class ShortcutListForm(QtGui.QDialog, Ui_ShortcutListDialog): if event.key() == QtCore.Qt.Key_Space: self.keyReleaseEvent(event) elif self.primaryPushButton.isChecked() or self.alternatePushButton.isChecked(): - event.ignore() + self.keyReleaseEvent(event) elif event.key() == QtCore.Qt.Key_Escape: event.accept() self.close() From 06d08f61a6ad61a80d7ccb3338782f52e593c9b5 Mon Sep 17 00:00:00 2001 From: Raoul Snyman Date: Wed, 18 Dec 2013 23:44:00 +0200 Subject: [PATCH 21/23] Fix bug #1211049: Add a list of valid user agents and a method to return an appropriate user agent for the current platform Fixes: https://launchpad.net/bugs/1211049 --- openlp/core/utils/__init__.py | 36 ++- .../openlp_core_utils/test_utils.py | 242 +++++++++++++++++- 2 files changed, 275 insertions(+), 3 deletions(-) diff --git a/openlp/core/utils/__init__.py b/openlp/core/utils/__init__.py index 6ceb592f4..7a11fe3b2 100644 --- a/openlp/core/utils/__init__.py +++ b/openlp/core/utils/__init__.py @@ -40,6 +40,7 @@ import sys import urllib.request import urllib.error import urllib.parse +from random import randint from PyQt4 import QtGui, QtCore @@ -61,10 +62,29 @@ APPLICATION_VERSION = {} IMAGES_FILTER = None ICU_COLLATOR = None UNO_CONNECTION_TYPE = 'pipe' -#UNO_CONNECTION_TYPE = u'socket' CONTROL_CHARS = re.compile(r'[\x00-\x1F\x7F-\x9F]', re.UNICODE) INVALID_FILE_CHARS = re.compile(r'[\\/:\*\?"<>\|\+\[\]%]', re.UNICODE) DIGITS_OR_NONDIGITS = re.compile(r'\d+|\D+', re.UNICODE) +USER_AGENTS = { + 'win32': [ + 'Mozilla/5.0 (Windows NT 5.1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/30.0.1599.101 Safari/537.36', + 'Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/30.0.1599.101 Safari/537.36', + 'Mozilla/5.0 (Windows NT 6.2; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/28.0.1500.71 Safari/537.36' + ], + 'darwin': [ + 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_8_3) AppleWebKit/537.31 (KHTML, like Gecko) Chrome/26.0.1410.43 Safari/537.31', + 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_7_3) AppleWebKit/536.11 (KHTML, like Gecko) Chrome/20.0.1132.57 Safari/536.11', + 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_6_8) AppleWebKit/536.11 (KHTML, like Gecko) Chrome/20.0.1132.47 Safari/536.11', + ], + 'linux2': [ + 'Mozilla/5.0 (X11; Linux i686) AppleWebKit/537.22 (KHTML, like Gecko) Ubuntu Chromium/25.0.1364.160 Chrome/25.0.1364.160 Safari/537.22', + 'Mozilla/5.0 (X11; CrOS armv7l 2913.260.0) AppleWebKit/537.11 (KHTML, like Gecko) Chrome/23.0.1271.99 Safari/537.11', + 'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.27 (KHTML, like Gecko) Chrome/26.0.1389.0 Safari/537.27' + ], + 'default': [ + 'Mozilla/5.0 (X11; NetBSD amd64; rv:18.0) Gecko/20130120 Firefox/18.0' + ] +} class VersionThread(QtCore.QThread): @@ -298,6 +318,17 @@ def delete_file(file_path_name): return False +def _get_user_agent(): + """ + Return a user agent customised for the platform the user is on. + """ + browser_list = USER_AGENTS.get(sys.platform, None) + if not browser_list: + browser_list = USER_AGENTS['default'] + random_index = randint(0, len(browser_list) - 1) + return browser_list[random_index] + + def get_web_page(url, header=None, update_openlp=False): """ Attempts to download the webpage at url and returns that page or None. @@ -318,6 +349,9 @@ def get_web_page(url, header=None, update_openlp=False): if not url: return None req = urllib.request.Request(url) + user_agent = _get_user_agent() + log.debug('Using user agent: %s', user_agent) + req.add_header('User-Agent', user_agent) if header: req.add_header(header[0], header[1]) page = None diff --git a/tests/functional/openlp_core_utils/test_utils.py b/tests/functional/openlp_core_utils/test_utils.py index dfc31b245..d5760ccb0 100644 --- a/tests/functional/openlp_core_utils/test_utils.py +++ b/tests/functional/openlp_core_utils/test_utils.py @@ -32,14 +32,74 @@ Functional tests to test the AppLocation class and related methods. from unittest import TestCase from openlp.core.utils import clean_filename, get_filesystem_encoding, get_locale_key, \ - get_natural_key, split_filename -from tests.functional import patch + get_natural_key, split_filename, _get_user_agent, get_web_page, get_uno_instance, add_actions +from tests.functional import MagicMock, patch class TestUtils(TestCase): """ A test suite to test out various methods around the AppLocation class. """ + def add_actions_empty_list_test(self): + """ + Test that no actions are added when the list is empty + """ + # GIVEN: a mocked action list, and an empty list + mocked_target = MagicMock() + empty_list = [] + + # WHEN: The empty list is added to the mocked target + add_actions(mocked_target, empty_list) + + # THEN: The add method on the mocked target is never called + self.assertEqual(0, mocked_target.addSeparator.call_count, 'addSeparator method should not have been called') + self.assertEqual(0, mocked_target.addAction.call_count, 'addAction method should not have been called') + + def add_actions_none_action_test(self): + """ + Test that a separator is added when a None action is in the list + """ + # GIVEN: a mocked action list, and a list with None in it + mocked_target = MagicMock() + separator_list = [None] + + # WHEN: The list is added to the mocked target + add_actions(mocked_target, separator_list) + + # THEN: The addSeparator method is called, but the addAction method is never called + mocked_target.addSeparator.assert_called_with() + self.assertEqual(0, mocked_target.addAction.call_count, 'addAction method should not have been called') + + def add_actions_add_action_test(self): + """ + Test that an action is added when a valid action is in the list + """ + # GIVEN: a mocked action list, and a list with an action in it + mocked_target = MagicMock() + action_list = ['action'] + + # WHEN: The list is added to the mocked target + add_actions(mocked_target, action_list) + + # THEN: The addSeparator method is not called, and the addAction method is called + self.assertEqual(0, mocked_target.addSeparator.call_count, 'addSeparator method should not have been called') + mocked_target.addAction.assert_called_with('action') + + def add_actions_action_and_none_test(self): + """ + Test that an action and a separator are added when a valid action and None are in the list + """ + # GIVEN: a mocked action list, and a list with an action and None in it + mocked_target = MagicMock() + action_list = ['action', None] + + # WHEN: The list is added to the mocked target + add_actions(mocked_target, action_list) + + # THEN: The addSeparator method is called, and the addAction method is called + mocked_target.addSeparator.assert_called_with() + mocked_target.addAction.assert_called_with('action') + def get_filesystem_encoding_sys_function_not_called_test(self): """ Test the get_filesystem_encoding() function does not call the sys.getdefaultencoding() function @@ -153,3 +213,181 @@ class TestUtils(TestCase): # THEN: We get a properly sorted list self.assertEqual(['1st item', 'item 3b', 'item 10a'], sorted_list, 'Numbers should be sorted naturally') + + def get_uno_instance_pipe_test(self): + """ + Test that when the UNO connection type is "pipe" the resolver is given the "pipe" URI + """ + # GIVEN: A mock resolver object and UNO_CONNECTION_TYPE is "pipe" + mock_resolver = MagicMock() + + # WHEN: get_uno_instance() is called + get_uno_instance(mock_resolver) + + # THEN: the resolve method is called with the correct argument + mock_resolver.resolve.assert_called_with('uno:pipe,name=openlp_pipe;urp;StarOffice.ComponentContext') + + def get_user_agent_linux_test(self): + """ + Test that getting a user agent on Linux returns a user agent suitable for Linux + """ + with patch('openlp.core.utils.sys') as mocked_sys: + + # GIVEN: The system is Linux + mocked_sys.platform = 'linux2' + + # WHEN: We call _get_user_agent() + user_agent = _get_user_agent() + + # THEN: The user agent is a Linux (or ChromeOS) user agent + result = 'Linux' in user_agent or 'CrOS' in user_agent + self.assertTrue(result, u'The user agent should be a valid Linux user agent') + + def get_user_agent_windows_test(self): + """ + Test that getting a user agent on Windows returns a user agent suitable for Windows + """ + with patch('openlp.core.utils.sys') as mocked_sys: + + # GIVEN: The system is Linux + mocked_sys.platform = 'win32' + + # WHEN: We call _get_user_agent() + user_agent = _get_user_agent() + + # THEN: The user agent is a Linux (or ChromeOS) user agent + self.assertIn('Windows', user_agent, u'The user agent should be a valid Windows user agent') + + def get_user_agent_macos_test(self): + """ + Test that getting a user agent on OS X returns a user agent suitable for OS X + """ + with patch('openlp.core.utils.sys') as mocked_sys: + + # GIVEN: The system is Linux + mocked_sys.platform = 'darwin' + + # WHEN: We call _get_user_agent() + user_agent = _get_user_agent() + + # THEN: The user agent is a Linux (or ChromeOS) user agent + self.assertIn('Mac OS X', user_agent, u'The user agent should be a valid OS X user agent') + + def get_user_agent_default_test(self): + """ + Test that getting a user agent on a non-Linux/Windows/OS X platform returns the default user agent + """ + with patch('openlp.core.utils.sys') as mocked_sys: + + # GIVEN: The system is Linux + mocked_sys.platform = 'freebsd' + + # WHEN: We call _get_user_agent() + user_agent = _get_user_agent() + + # THEN: The user agent is a Linux (or ChromeOS) user agent + self.assertIn('NetBSD', user_agent, u'The user agent should be the default user agent') + + def get_web_page_no_url_test(self): + """ + Test that sending a URL of None to the get_web_page method returns None + """ + # GIVEN: A None url + test_url = None + + # WHEN: We try to get the test URL + result = get_web_page(test_url) + + # THEN: None should be returned + self.assertIsNone(result, 'The return value of get_web_page should be None') + + def get_web_page_test(self): + """ + Test that the get_web_page method works correctly + """ + with patch('openlp.core.utils.urllib.request.Request') as MockRequest, \ + patch('openlp.core.utils.urllib.request.urlopen') as mock_urlopen, \ + patch('openlp.core.utils._get_user_agent') as mock_get_user_agent, \ + patch('openlp.core.utils.Registry') as MockRegistry: + # GIVEN: Mocked out objects and a fake URL + mocked_request_object = MagicMock() + MockRequest.return_value = mocked_request_object + mocked_page_object = MagicMock() + mock_urlopen.return_value = mocked_page_object + mock_get_user_agent.return_value = 'user_agent' + fake_url = 'this://is.a.fake/url' + + # WHEN: The get_web_page() method is called + returned_page = get_web_page(fake_url) + + # THEN: The correct methods are called with the correct arguments and a web page is returned + MockRequest.assert_called_with(fake_url) + mocked_request_object.add_header.assert_called_with('User-Agent', 'user_agent') + self.assertEqual(1, mocked_request_object.add_header.call_count, + 'There should only be 1 call to add_header') + mock_urlopen.assert_called_with(mocked_request_object) + mocked_page_object.geturl.assert_called_with() + self.assertEqual(0, MockRegistry.call_count, 'The Registry() object should have never been called') + self.assertEqual(mocked_page_object, returned_page, 'The returned page should be the mock object') + + def get_web_page_with_header_test(self): + """ + Test that adding a header to the call to get_web_page() adds the header to the request + """ + with patch('openlp.core.utils.urllib.request.Request') as MockRequest, \ + patch('openlp.core.utils.urllib.request.urlopen') as mock_urlopen, \ + patch('openlp.core.utils._get_user_agent') as mock_get_user_agent: + # GIVEN: Mocked out objects, a fake URL and a fake header + mocked_request_object = MagicMock() + MockRequest.return_value = mocked_request_object + mocked_page_object = MagicMock() + mock_urlopen.return_value = mocked_page_object + mock_get_user_agent.return_value = 'user_agent' + fake_url = 'this://is.a.fake/url' + fake_header = ('Fake-Header', 'fake value') + + # WHEN: The get_web_page() method is called + returned_page = get_web_page(fake_url, header=fake_header) + + # THEN: The correct methods are called with the correct arguments and a web page is returned + MockRequest.assert_called_with(fake_url) + mocked_request_object.add_header.assert_called_with('Fake-Header', 'fake value') + self.assertEqual(2, mocked_request_object.add_header.call_count, + 'There should only be 2 calls to add_header') + mock_urlopen.assert_called_with(mocked_request_object) + mocked_page_object.geturl.assert_called_with() + self.assertEqual(mocked_page_object, returned_page, 'The returned page should be the mock object') + + def get_web_page_update_openlp_test(self): + """ + Test that passing "update_openlp" as true to get_web_page calls Registry().get('app').process_events() + """ + with patch('openlp.core.utils.urllib.request.Request') as MockRequest, \ + patch('openlp.core.utils.urllib.request.urlopen') as mock_urlopen, \ + patch('openlp.core.utils._get_user_agent') as mock_get_user_agent, \ + patch('openlp.core.utils.Registry') as MockRegistry: + # GIVEN: Mocked out objects, a fake URL + mocked_request_object = MagicMock() + MockRequest.return_value = mocked_request_object + mocked_page_object = MagicMock() + mock_urlopen.return_value = mocked_page_object + mock_get_user_agent.return_value = 'user_agent' + mocked_registry_object = MagicMock() + mocked_application_object = MagicMock() + mocked_registry_object.get.return_value = mocked_application_object + MockRegistry.return_value = mocked_registry_object + fake_url = 'this://is.a.fake/url' + + # WHEN: The get_web_page() method is called + returned_page = get_web_page(fake_url, update_openlp=True) + + # THEN: The correct methods are called with the correct arguments and a web page is returned + MockRequest.assert_called_with(fake_url) + mocked_request_object.add_header.assert_called_with('User-Agent', 'user_agent') + self.assertEqual(1, mocked_request_object.add_header.call_count, + 'There should only be 1 call to add_header') + mock_urlopen.assert_called_with(mocked_request_object) + mocked_page_object.geturl.assert_called_with() + mocked_registry_object.get.assert_called_with('application') + mocked_application_object.process_events.assert_called_with() + self.assertEqual(mocked_page_object, returned_page, 'The returned page should be the mock object') From 553c93e790bb9e2542eed83d31c877105022407e Mon Sep 17 00:00:00 2001 From: Raoul Snyman Date: Wed, 18 Dec 2013 23:54:56 +0200 Subject: [PATCH 22/23] Tidy up some tests --- tests/functional/__init__.py | 4 +-- .../openlp_plugins/remotes/test_router.py | 31 ++++++++++--------- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/tests/functional/__init__.py b/tests/functional/__init__.py index 3f471cf61..f3b85b9b3 100644 --- a/tests/functional/__init__.py +++ b/tests/functional/__init__.py @@ -11,9 +11,9 @@ import sys from PyQt4 import QtGui if sys.version_info[1] >= 3: - from unittest.mock import patch, MagicMock + from unittest.mock import MagicMock, patch, mock_open else: - from mock import patch, MagicMock + from mock import MagicMock, patch, mock_open # Only one QApplication can be created. Use QtGui.QApplication.instance() when you need to "create" a QApplication. application = QtGui.QApplication([]) diff --git a/tests/functional/openlp_plugins/remotes/test_router.py b/tests/functional/openlp_plugins/remotes/test_router.py index 4d7da2b91..f0af48afd 100644 --- a/tests/functional/openlp_plugins/remotes/test_router.py +++ b/tests/functional/openlp_plugins/remotes/test_router.py @@ -37,8 +37,7 @@ from PyQt4 import QtGui from openlp.core.common import Settings from openlp.plugins.remotes.lib.httpserver import HttpRouter -from tests.functional import MagicMock, patch -from mock import mock_open +from tests.functional import MagicMock, patch, mock_open __default_settings__ = { 'remotes/twelve hour': True, @@ -53,6 +52,7 @@ __default_settings__ = { TEST_PATH = os.path.abspath(os.path.dirname(__file__)) + class TestRouter(TestCase): """ Test the functions in the :mod:`lib` module. @@ -91,7 +91,7 @@ class TestRouter(TestCase): # THEN: the function should return the correct password self.assertEqual(router.auth, test_value, - 'The result for make_sha_hash should return the correct encrypted password') + 'The result for make_sha_hash should return the correct encrypted password') def process_http_request_test(self): """ @@ -109,10 +109,8 @@ class TestRouter(TestCase): function, args = router.process_http_request('/stage/api/poll', None) # THEN: the function should have been called only once - assert function['function'] == mocked_function, \ - 'The mocked function should match defined value.' - assert function['secure'] == False, \ - 'The mocked function should not require any security.' + self.assertEqual(mocked_function, function['function'], 'The mocked function should match defined value.') + self.assertFalse(function['secure'], 'The mocked function should not require any security.') def get_content_type_test(self): """ @@ -124,10 +122,12 @@ class TestRouter(TestCase): ['test.gif', 'image/gif'], ['test.ico', 'image/x-icon'], ['test.png', 'image/png'], ['test.whatever', 'text/plain'], ['test', 'text/plain'], ['', 'text/plain'], - [os.path.join(TEST_PATH,'test.html'), 'text/html']] + [os.path.join(TEST_PATH, 'test.html'), 'text/html']] + # WHEN: calling each file type for header in headers: ext, content_type = self.router.get_content_type(header[0]) + # THEN: all types should match self.assertEqual(content_type, header[1], 'Mismatch of content type') @@ -142,13 +142,14 @@ class TestRouter(TestCase): self.router.wfile = MagicMock() self.router.html_dir = os.path.normpath('test/dir') self.router.template_vars = MagicMock() + # WHEN: call serve_file with no file_name self.router.serve_file() + # THEN: it should return a 404 self.router.send_response.assert_called_once_with(404) self.router.send_header.assert_called_once_with('Content-type','text/html') - self.assertEqual(self.router.end_headers.call_count, 1, - 'end_headers called once') + self.assertEqual(self.router.end_headers.call_count, 1, 'end_headers called once') def serve_file_with_valid_params_test(self): """ @@ -162,13 +163,13 @@ class TestRouter(TestCase): self.router.html_dir = os.path.normpath('test/dir') self.router.template_vars = MagicMock() with patch('openlp.core.lib.os.path.exists') as mocked_exists, \ - patch('builtins.open', mock_open(read_data='123')): + patch('builtins.open', mock_open(read_data='123')): mocked_exists.return_value = True + # WHEN: call serve_file with an existing html file self.router.serve_file(os.path.normpath('test/dir/test.html')) + # THEN: it should return a 200 and the file self.router.send_response.assert_called_once_with(200) - self.router.send_header.assert_called_once_with( - 'Content-type','text/html') - self.assertEqual(self.router.end_headers.call_count, 1, - 'end_headers called once') + self.router.send_header.assert_called_once_with('Content-type', 'text/html') + self.assertEqual(self.router.end_headers.call_count, 1, 'end_headers called once') From 3204a4db728c95e1823f148080a4957538263f9e Mon Sep 17 00:00:00 2001 From: Raoul Snyman Date: Fri, 20 Dec 2013 21:25:42 +0200 Subject: [PATCH 23/23] - Specify OpenLP as the user agent when querying SF.net - Fix another test - Update tests for get_web_page --- openlp/core/ui/firsttimeform.py | 10 +++--- openlp/core/utils/__init__.py | 6 ++-- .../openlp_core_utils/test_utils.py | 32 ++++++++++++++++++- .../openlp_plugins/remotes/test_router.py | 2 +- 4 files changed, 41 insertions(+), 9 deletions(-) diff --git a/openlp/core/ui/firsttimeform.py b/openlp/core/ui/firsttimeform.py index e48395000..cd8c26f7b 100644 --- a/openlp/core/ui/firsttimeform.py +++ b/openlp/core/ui/firsttimeform.py @@ -37,7 +37,7 @@ import urllib.request import urllib.parse import urllib.error from tempfile import gettempdir -from configparser import SafeConfigParser +from configparser import ConfigParser from PyQt4 import QtCore, QtGui @@ -68,7 +68,7 @@ class ThemeScreenshotThread(QtCore.QThread): filename = config.get('theme_%s' % theme, 'filename') screenshot = config.get('theme_%s' % theme, 'screenshot') urllib.request.urlretrieve('%s%s' % (self.parent().web, screenshot), - os.path.join(gettempdir(), 'openlp', screenshot)) + os.path.join(gettempdir(), 'openlp', screenshot)) item = QtGui.QListWidgetItem(title, self.parent().themes_list_widget) item.setData(QtCore.Qt.UserRole, filename) item.setCheckState(QtCore.Qt.Unchecked) @@ -90,14 +90,16 @@ class FirstTimeForm(QtGui.QWizard, Ui_FirstTimeWizard): self.screens = screens # check to see if we have web access self.web = 'http://openlp.org/files/frw/' - self.config = SafeConfigParser() - self.web_access = get_web_page('%s%s' % (self.web, 'download.cfg')) + self.config = ConfigParser() + user_agent = 'OpenLP/' + Registry().get('application').applicationVersion() + self.web_access = get_web_page('%s%s' % (self.web, 'download.cfg'), header=('User-Agent', user_agent)) if self.web_access: files = self.web_access.read() self.config.read_string(files.decode()) self.update_screen_list_combo() self.was_download_cancelled = False self.theme_screenshot_thread = None + self.has_run_wizard = False self.downloading = translate('OpenLP.FirstTimeWizard', 'Downloading %s...') self.cancel_button.clicked.connect(self.on_cancel_button_clicked) self.no_internet_finish_button.clicked.connect(self.on_no_internet_finish_button_clicked) diff --git a/openlp/core/utils/__init__.py b/openlp/core/utils/__init__.py index 7a11fe3b2..f19d88ba9 100644 --- a/openlp/core/utils/__init__.py +++ b/openlp/core/utils/__init__.py @@ -349,9 +349,9 @@ def get_web_page(url, header=None, update_openlp=False): if not url: return None req = urllib.request.Request(url) - user_agent = _get_user_agent() - log.debug('Using user agent: %s', user_agent) - req.add_header('User-Agent', user_agent) + if not header or header[0].lower() != 'user-agent': + user_agent = _get_user_agent() + req.add_header('User-Agent', user_agent) if header: req.add_header(header[0], header[1]) page = None diff --git a/tests/functional/openlp_core_utils/test_utils.py b/tests/functional/openlp_core_utils/test_utils.py index d5760ccb0..d9cdd9582 100644 --- a/tests/functional/openlp_core_utils/test_utils.py +++ b/tests/functional/openlp_core_utils/test_utils.py @@ -325,6 +325,7 @@ class TestUtils(TestCase): mocked_request_object.add_header.assert_called_with('User-Agent', 'user_agent') self.assertEqual(1, mocked_request_object.add_header.call_count, 'There should only be 1 call to add_header') + mock_get_user_agent.assert_called_with() mock_urlopen.assert_called_with(mocked_request_object) mocked_page_object.geturl.assert_called_with() self.assertEqual(0, MockRegistry.call_count, 'The Registry() object should have never been called') @@ -351,9 +352,38 @@ class TestUtils(TestCase): # THEN: The correct methods are called with the correct arguments and a web page is returned MockRequest.assert_called_with(fake_url) - mocked_request_object.add_header.assert_called_with('Fake-Header', 'fake value') + mocked_request_object.add_header.assert_called_with(fake_header[0], fake_header[1]) self.assertEqual(2, mocked_request_object.add_header.call_count, 'There should only be 2 calls to add_header') + mock_get_user_agent.assert_called_with() + mock_urlopen.assert_called_with(mocked_request_object) + mocked_page_object.geturl.assert_called_with() + self.assertEqual(mocked_page_object, returned_page, 'The returned page should be the mock object') + + def get_web_page_with_user_agent_in_headers_test(self): + """ + Test that adding a user agent in the header when calling get_web_page() adds that user agent to the request + """ + with patch('openlp.core.utils.urllib.request.Request') as MockRequest, \ + patch('openlp.core.utils.urllib.request.urlopen') as mock_urlopen, \ + patch('openlp.core.utils._get_user_agent') as mock_get_user_agent: + # GIVEN: Mocked out objects, a fake URL and a fake header + mocked_request_object = MagicMock() + MockRequest.return_value = mocked_request_object + mocked_page_object = MagicMock() + mock_urlopen.return_value = mocked_page_object + fake_url = 'this://is.a.fake/url' + user_agent_header = ('User-Agent', 'OpenLP/2.1.0') + + # WHEN: The get_web_page() method is called + returned_page = get_web_page(fake_url, header=user_agent_header) + + # THEN: The correct methods are called with the correct arguments and a web page is returned + MockRequest.assert_called_with(fake_url) + mocked_request_object.add_header.assert_called_with(user_agent_header[0], user_agent_header[1]) + self.assertEqual(1, mocked_request_object.add_header.call_count, + 'There should only be 1 call to add_header') + self.assertEqual(0, mock_get_user_agent.call_count, '_get_user_agent should not have been called') mock_urlopen.assert_called_with(mocked_request_object) mocked_page_object.geturl.assert_called_with() self.assertEqual(mocked_page_object, returned_page, 'The returned page should be the mock object') diff --git a/tests/functional/openlp_plugins/remotes/test_router.py b/tests/functional/openlp_plugins/remotes/test_router.py index f0af48afd..b65499c64 100644 --- a/tests/functional/openlp_plugins/remotes/test_router.py +++ b/tests/functional/openlp_plugins/remotes/test_router.py @@ -168,7 +168,7 @@ class TestRouter(TestCase): # WHEN: call serve_file with an existing html file self.router.serve_file(os.path.normpath('test/dir/test.html')) - + # THEN: it should return a 200 and the file self.router.send_response.assert_called_once_with(200) self.router.send_header.assert_called_once_with('Content-type', 'text/html')