From 6f34f8f2b86724485ff7f83dc12cc9a0642cb69a Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Thu, 7 Jun 2018 18:44:35 +0100 Subject: [PATCH 01/18] Start on implementing global OpenLP proxying --- openlp/core/common/httputils.py | 38 +++++- openlp/core/common/settings.py | 13 ++ openlp/core/ui/advancedtab.py | 55 ++++---- .../openlp_core/common/test_httputils.py | 123 +++++++++++++++++- 4 files changed, 198 insertions(+), 31 deletions(-) diff --git a/openlp/core/common/httputils.py b/openlp/core/common/httputils.py index e7d11fad5..40077f630 100644 --- a/openlp/core/common/httputils.py +++ b/openlp/core/common/httputils.py @@ -32,6 +32,7 @@ import requests from openlp.core.common import trace_error_handler from openlp.core.common.registry import Registry +from openlp.core.common.settings import ProxyMode, Settings log = logging.getLogger(__name__ + '.__init__') @@ -64,6 +65,39 @@ CONNECTION_TIMEOUT = 30 CONNECTION_RETRIES = 2 +def get_proxy_settings(mode=None): + """ + Create a dictionary containing the proxy settings. + + :param ProxyMode | None mode: Specify the source of the proxy settings + :return: A dict using the format expected by the requests library. + :rtype: dict | None + """ + settings = Settings() + if mode is None: + mode = settings.value('advanced/proxy mode') + if mode == ProxyMode.NO_PROXY: + return {'http': None, 'https': None} + elif mode == ProxyMode.SYSTEM_PROXY: + # The requests library defaults to using the proxy settings in the environment variables + return + elif mode == ProxyMode.MANUAL_PROXY: + http_addr = settings.value('advanced/proxy http') + https_addr = settings.value('advanced/proxy https') + username = settings.value('advanced/proxy username') + password = settings.value('advanced/proxy password') + basic_auth = '' + if username: + basic_auth = '{username}:{password}@'.format(username=username, password=password) + http_value = None + https_value = None + if http_addr is not None: + http_value = 'http://{basic_auth}{http_addr}'.format(basic_auth=basic_auth, http_addr=http_addr) + if https_addr is not None: + https_value = 'https://{basic_auth}{https_addr}'.format(basic_auth=basic_auth, https_addr=https_addr) + return {'http': http_value, 'https': https_value} + + def get_user_agent(): """ Return a user agent customised for the platform the user is on. @@ -75,7 +109,7 @@ def get_user_agent(): return browser_list[random_index] -def get_web_page(url, headers=None, update_openlp=False, proxies=None): +def get_web_page(url, headers=None, update_openlp=False, proxy_mode=None): """ Attempts to download the webpage at url and returns that page or None. @@ -90,6 +124,8 @@ def get_web_page(url, headers=None, update_openlp=False, proxies=None): headers = {} if 'user-agent' not in [key.lower() for key in headers.keys()]: headers['User-Agent'] = get_user_agent() + if proxy_mode is None: + proxies = get_proxy_settings(mode=proxy_mode) log.debug('Downloading URL = %s' % url) retries = 0 while retries < CONNECTION_RETRIES: diff --git a/openlp/core/common/settings.py b/openlp/core/common/settings.py index 91a587616..74d686b09 100644 --- a/openlp/core/common/settings.py +++ b/openlp/core/common/settings.py @@ -26,6 +26,7 @@ import datetime import json import logging import os +from enum import IntEnum from tempfile import gettempdir from PyQt5 import QtCore, QtGui @@ -38,6 +39,13 @@ log = logging.getLogger(__name__) __version__ = 2 + +class ProxyMode(IntEnum): + NO_PROXY = 1 + SYSTEM_PROXY = 2 + MANUAL_PROXY = 3 + + # Fix for bug #1014422. X11_BYPASS_DEFAULT = True if is_linux(): # pragma: no cover @@ -116,6 +124,11 @@ class Settings(QtCore.QSettings): 'advanced/print file meta data': False, 'advanced/print notes': False, 'advanced/print slide text': False, + 'advanced/proxy mode': ProxyMode.SYSTEM_PROXY, + 'advanced/proxy http': '', + 'advanced/proxy https': '', + 'advanced/proxy username': '', + 'advanced/proxy password': '', 'advanced/recent file count': 4, 'advanced/save current plugin': False, 'advanced/slide limits': SlideLimits.End, diff --git a/openlp/core/ui/advancedtab.py b/openlp/core/ui/advancedtab.py index ffec0e5b8..fa970f234 100644 --- a/openlp/core/ui/advancedtab.py +++ b/openlp/core/ui/advancedtab.py @@ -35,6 +35,7 @@ from openlp.core.lib import SettingsTab, build_icon from openlp.core.ui.style import HAS_DARK_STYLE from openlp.core.widgets.edits import PathEdit from openlp.core.widgets.enums import PathEditType +from openlp.core.widgets.widgets import ProxyWidget log = logging.getLogger(__name__) @@ -76,6 +77,9 @@ class AdvancedTab(SettingsTab): self.media_plugin_check_box = QtWidgets.QCheckBox(self.ui_group_box) self.media_plugin_check_box.setObjectName('media_plugin_check_box') self.ui_layout.addRow(self.media_plugin_check_box) + self.hide_mouse_check_box = QtWidgets.QCheckBox(self.ui_group_box) + self.hide_mouse_check_box.setObjectName('hide_mouse_check_box') + self.ui_layout.addWidget(self.hide_mouse_check_box) self.double_click_live_check_box = QtWidgets.QCheckBox(self.ui_group_box) self.double_click_live_check_box.setObjectName('double_click_live_check_box') self.ui_layout.addRow(self.double_click_live_check_box) @@ -116,6 +120,24 @@ class AdvancedTab(SettingsTab): self.use_dark_style_checkbox = QtWidgets.QCheckBox(self.ui_group_box) self.use_dark_style_checkbox.setObjectName('use_dark_style_checkbox') self.ui_layout.addRow(self.use_dark_style_checkbox) + # Service Item Slide Limits + self.slide_group_box = QtWidgets.QGroupBox(self.left_column) + self.slide_group_box.setObjectName('slide_group_box') + self.slide_layout = QtWidgets.QVBoxLayout(self.slide_group_box) + self.slide_layout.setObjectName('slide_layout') + self.slide_label = QtWidgets.QLabel(self.slide_group_box) + self.slide_label.setWordWrap(True) + self.slide_layout.addWidget(self.slide_label) + self.end_slide_radio_button = QtWidgets.QRadioButton(self.slide_group_box) + self.end_slide_radio_button.setObjectName('end_slide_radio_button') + self.slide_layout.addWidget(self.end_slide_radio_button) + self.wrap_slide_radio_button = QtWidgets.QRadioButton(self.slide_group_box) + self.wrap_slide_radio_button.setObjectName('wrap_slide_radio_button') + self.slide_layout.addWidget(self.wrap_slide_radio_button) + self.next_item_radio_button = QtWidgets.QRadioButton(self.slide_group_box) + self.next_item_radio_button.setObjectName('next_item_radio_button') + self.slide_layout.addWidget(self.next_item_radio_button) + self.left_layout.addWidget(self.slide_group_box) # Data Directory self.data_directory_group_box = QtWidgets.QGroupBox(self.left_column) self.data_directory_group_box.setObjectName('data_directory_group_box') @@ -142,33 +164,6 @@ class AdvancedTab(SettingsTab): self.data_directory_layout.addRow(self.data_directory_copy_check_layout) self.data_directory_layout.addRow(self.new_data_directory_has_files_label) self.left_layout.addWidget(self.data_directory_group_box) - # Hide mouse - self.hide_mouse_group_box = QtWidgets.QGroupBox(self.right_column) - self.hide_mouse_group_box.setObjectName('hide_mouse_group_box') - self.hide_mouse_layout = QtWidgets.QVBoxLayout(self.hide_mouse_group_box) - self.hide_mouse_layout.setObjectName('hide_mouse_layout') - self.hide_mouse_check_box = QtWidgets.QCheckBox(self.hide_mouse_group_box) - self.hide_mouse_check_box.setObjectName('hide_mouse_check_box') - self.hide_mouse_layout.addWidget(self.hide_mouse_check_box) - self.right_layout.addWidget(self.hide_mouse_group_box) - # Service Item Slide Limits - self.slide_group_box = QtWidgets.QGroupBox(self.right_column) - self.slide_group_box.setObjectName('slide_group_box') - self.slide_layout = QtWidgets.QVBoxLayout(self.slide_group_box) - self.slide_layout.setObjectName('slide_layout') - self.slide_label = QtWidgets.QLabel(self.slide_group_box) - self.slide_label.setWordWrap(True) - self.slide_layout.addWidget(self.slide_label) - self.end_slide_radio_button = QtWidgets.QRadioButton(self.slide_group_box) - self.end_slide_radio_button.setObjectName('end_slide_radio_button') - self.slide_layout.addWidget(self.end_slide_radio_button) - self.wrap_slide_radio_button = QtWidgets.QRadioButton(self.slide_group_box) - self.wrap_slide_radio_button.setObjectName('wrap_slide_radio_button') - self.slide_layout.addWidget(self.wrap_slide_radio_button) - self.next_item_radio_button = QtWidgets.QRadioButton(self.slide_group_box) - self.next_item_radio_button.setObjectName('next_item_radio_button') - self.slide_layout.addWidget(self.next_item_radio_button) - self.right_layout.addWidget(self.slide_group_box) # Display Workarounds self.display_workaround_group_box = QtWidgets.QGroupBox(self.right_column) self.display_workaround_group_box.setObjectName('display_workaround_group_box') @@ -223,6 +218,9 @@ class AdvancedTab(SettingsTab): self.service_name_example.setObjectName('service_name_example') self.service_name_layout.addRow(self.service_name_example_label, self.service_name_example) self.right_layout.addWidget(self.service_name_group_box) + # Proxies + self.proxy_widget = ProxyWidget(self.right_column) + self.right_layout.addWidget(self.proxy_widget) # After the last item on each side, add some spacing self.left_layout.addStretch() self.right_layout.addStretch() @@ -311,7 +309,6 @@ class AdvancedTab(SettingsTab): translate('OpenLP.AdvancedTab', 'Revert to the default service name "{name}".').format(name=UiStrings().DefaultServiceName)) self.service_name_example_label.setText(translate('OpenLP.AdvancedTab', 'Example:')) - self.hide_mouse_group_box.setTitle(translate('OpenLP.AdvancedTab', 'Mouse Cursor')) self.hide_mouse_check_box.setText(translate('OpenLP.AdvancedTab', 'Hide mouse cursor when over display window')) self.data_directory_new_label.setText(translate('OpenLP.AdvancedTab', 'Path:')) self.data_directory_cancel_button.setText(translate('OpenLP.AdvancedTab', 'Cancel')) @@ -334,6 +331,7 @@ class AdvancedTab(SettingsTab): self.wrap_slide_radio_button.setText(translate('OpenLP.GeneralTab', '&Wrap around')) self.next_item_radio_button.setText(translate('OpenLP.GeneralTab', '&Move to next/previous service item')) self.search_as_type_check_box.setText(translate('SongsPlugin.GeneralTab', 'Enable search as you type')) + self.proxy_widget.retranslate_ui() def load(self): """ @@ -436,6 +434,7 @@ class AdvancedTab(SettingsTab): if HAS_DARK_STYLE: settings.setValue('use_dark_style', self.use_dark_style_checkbox.isChecked()) settings.endGroup() + self.proxy_widget.save() def on_search_as_type_check_box_changed(self, check_state): self.is_search_as_you_type_enabled = (check_state == QtCore.Qt.Checked) diff --git a/tests/functional/openlp_core/common/test_httputils.py b/tests/functional/openlp_core/common/test_httputils.py index 7d3966279..1a979454c 100644 --- a/tests/functional/openlp_core/common/test_httputils.py +++ b/tests/functional/openlp_core/common/test_httputils.py @@ -27,13 +27,14 @@ import tempfile from unittest import TestCase from unittest.mock import MagicMock, patch -from openlp.core.common.httputils import get_user_agent, get_web_page, get_url_file_size, download_file +from openlp.core.common.httputils import ProxyMode, download_file, get_proxy_settings, get_url_file_size, \ + get_user_agent, get_web_page from openlp.core.common.path import Path +from openlp.core.common.settings import Settings from tests.helpers.testmixin import TestMixin class TestHttpUtils(TestCase, TestMixin): - """ A test suite to test out various http helper functions. """ @@ -240,3 +241,121 @@ class TestHttpUtils(TestCase, TestMixin): # THEN: socket.timeout should have been caught # NOTE: Test is if $tmpdir/tempfile is still there, then test fails since ftw deletes bad downloaded files assert os.path.exists(self.tempfile) is False, 'tempfile should have been deleted' + + +class TestGetProxySettings(TestCase, TestMixin): + def setUp(self): + self.build_settings() + self.addCleanup(self.destroy_settings) + + @patch('openlp.core.common.httputils.Settings') + def test_mode_arg_specified(self, MockSettings): + """ + Test that the argument is used rather than reading the 'advanced/proxy mode' setting + """ + # GIVEN: Mocked settings + mocked_settings = MagicMock() + MockSettings.return_value = mocked_settings + + # WHEN: Calling `get_proxy_settings` with the mode arg specified + get_proxy_settings(mode=ProxyMode.NO_PROXY) + + # THEN: The mode arg should have been used rather than looking it up in the settings + mocked_settings.value.assert_not_called() + + @patch('openlp.core.common.httputils.Settings') + def test_mode_incorrect_arg_specified(self, MockSettings): + """ + Test that the system settings are used when the mode arg specieied is invalid + """ + # GIVEN: Mocked settings + mocked_settings = MagicMock() + MockSettings.return_value = mocked_settings + + # WHEN: Calling `get_proxy_settings` with an invalid mode arg specified + result = get_proxy_settings(mode='qwerty') + + # THEN: An None should be returned + mocked_settings.value.assert_not_called() + assert result is None + + + def test_no_proxy_mode(self): + """ + Test that a dictionary with http and https values are set to None is returned, when `NO_PROXY` mode is specified + """ + # GIVEN: A `proxy mode` setting of NO_PROXY + Settings().setValue('advanced/proxy mode', ProxyMode.NO_PROXY) + + # WHEN: Calling `get_proxy_settings` + result = get_proxy_settings() + + # THEN: The returned value should be a dictionary with http and https values set to None + assert result == {'http': None, 'https': None} + + def test_system_proxy_mode(self): + """ + Test that None is returned, when `SYSTEM_PROXY` mode is specified + """ + # GIVEN: A `proxy mode` setting of SYSTEM_PROXY + Settings().setValue('advanced/proxy mode', ProxyMode.SYSTEM_PROXY) + + # WHEN: Calling `get_proxy_settings` + result = get_proxy_settings() + + # THEN: The returned value should be None + assert result is None + + def test_manual_proxy_mode_no_auth(self): + """ + Test that the correct proxy addresses are returned when basic authentication is not used + """ + # GIVEN: A `proxy mode` setting of MANUAL_PROXY with proxy servers, but no auth credentials are supplied + Settings().setValue('advanced/proxy mode', ProxyMode.MANUAL_PROXY) + Settings().setValue('advanced/proxy http', 'testhttp.server:port') + Settings().setValue('advanced/proxy https', 'testhttps.server:port') + Settings().setValue('advanced/proxy username', '') + Settings().setValue('advanced/proxy password', '') + + # WHEN: Calling `get_proxy_settings` + result = get_proxy_settings() + + # THEN: The returned value should be the proxy servers without authentication + assert result == {'http': 'http://testhttp.server:port', 'https': 'https://testhttps.server:port'} + + + def test_manual_proxy_mode_auth(self): + """ + Test that the correct proxy addresses are returned when basic authentication is used + """ + # GIVEN: A `proxy mode` setting of MANUAL_PROXY with proxy servers and auth credentials supplied + Settings().setValue('advanced/proxy mode', ProxyMode.MANUAL_PROXY) + Settings().setValue('advanced/proxy http', 'testhttp.server:port') + Settings().setValue('advanced/proxy https', 'testhttps.server:port') + Settings().setValue('advanced/proxy username', 'user') + Settings().setValue('advanced/proxy password', 'pass') + + # WHEN: Calling `get_proxy_settings` + result = get_proxy_settings() + + # THEN: The returned value should be the proxy servers with the authentication credentials + assert result == {'http': 'http://user:pass@testhttp.server:port', + 'https': 'https://user:pass@testhttps.server:port'} + + def test_manual_proxy_mode_no_servers(self): + """ + Test that the system proxies are overidden when the MANUAL_PROXY mode is specified, but no server addresses are + supplied + """ + # GIVEN: A `proxy mode` setting of MANUAL_PROXY with no servers specified + Settings().setValue('advanced/proxy mode', ProxyMode.MANUAL_PROXY) + Settings().setValue('advanced/proxy http', None) + Settings().setValue('advanced/proxy https', None) + Settings().setValue('advanced/proxy username', 'user') + Settings().setValue('advanced/proxy password', 'pass') + + # WHEN: Calling `get_proxy_settings` + result = get_proxy_settings() + + # THEN: The returned value should be the proxy servers set to None + assert result == {'http': None, 'https': None} From dce509e909ae74055e61bfab07a727107964609f Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Thu, 7 Jun 2018 18:45:24 +0100 Subject: [PATCH 02/18] Add some forgetten files --- openlp/core/widgets/widgets.py | 134 +++++++++++++++ .../openlp_core/widgets/test_widgets.py | 155 ++++++++++++++++++ 2 files changed, 289 insertions(+) create mode 100644 openlp/core/widgets/widgets.py create mode 100644 tests/interfaces/openlp_core/widgets/test_widgets.py diff --git a/openlp/core/widgets/widgets.py b/openlp/core/widgets/widgets.py new file mode 100644 index 000000000..bb65410bd --- /dev/null +++ b/openlp/core/widgets/widgets.py @@ -0,0 +1,134 @@ +# -*- coding: utf-8 -*- +# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4 + +############################################################################### +# OpenLP - Open Source Lyrics Projection # +# --------------------------------------------------------------------------- # +# Copyright (c) 2008-2018 OpenLP Developers # +# --------------------------------------------------------------------------- # +# This program is free software; you can redistribute it and/or modify it # +# under the terms of the GNU General Public License as published by the Free # +# Software Foundation; version 2 of the License. # +# # +# This program is distributed in the hope that it will be useful, but WITHOUT # +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or # +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for # +# more details. # +# # +# You should have received a copy of the GNU General Public License along # +# with this program; if not, write to the Free Software Foundation, Inc., 59 # +# Temple Place, Suite 330, Boston, MA 02111-1307 USA # +############################################################################### +""" +The :mod:`~openlp.core.widgets.widgets` module contains custom widgets used in OpenLP +""" +from PyQt5 import QtWidgets + +from openlp.core.common.i18n import translate +from openlp.core.common.settings import ProxyMode, Settings + + +class ProxyWidget(QtWidgets.QGroupBox): + """ + A proxy settings widget that implements loading and saving its settings. + """ + def __init__(self, parent=None): + """ + Initialise the widget. + + :param QtWidgets.QWidget | None parent: The widgets parent + """ + super().__init__(parent) + self._setup() + + def _setup(self): + """ + A setup method seperate from __init__ to allow easier testing + """ + self.setup_ui() + self.load() + + def setup_ui(self): + """ + Create the widget layout and sub widgets + """ + self.layout = QtWidgets.QFormLayout(self) + self.radio_group = QtWidgets.QButtonGroup(self) + self.no_proxy_radio = QtWidgets.QRadioButton('', self) + self.radio_group.addButton(self.no_proxy_radio, ProxyMode.NO_PROXY) + self.layout.setWidget(0, QtWidgets.QFormLayout.SpanningRole, self.no_proxy_radio) + self.use_sysem_proxy_radio = QtWidgets.QRadioButton('', self) + self.radio_group.addButton(self.use_sysem_proxy_radio, ProxyMode.SYSTEM_PROXY) + self.layout.setWidget(1, QtWidgets.QFormLayout.SpanningRole, self.use_sysem_proxy_radio) + self.manual_proxy_radio = QtWidgets.QRadioButton('', self) + self.radio_group.addButton(self.manual_proxy_radio, ProxyMode.MANUAL_PROXY) + self.layout.setWidget(2, QtWidgets.QFormLayout.SpanningRole, self.manual_proxy_radio) + self.http_edit = QtWidgets.QLineEdit(self) + self.layout.addRow('HTTP:', self.http_edit) + self.https_edit = QtWidgets.QLineEdit(self) + self.layout.addRow('HTTPS:', self.https_edit) + self.username_edit = QtWidgets.QLineEdit(self) + self.layout.addRow('Username:', self.username_edit) + self.password_edit = QtWidgets.QLineEdit(self) + self.password_edit.setEchoMode(QtWidgets.QLineEdit.Password) + self.layout.addRow('Password:', self.password_edit) + # Signal / Slots + self.radio_group.buttonToggled.connect(self.on_radio_group_button_toggled) + + # @QtCore.pyqtSlot(int, bool) For some reason PyQt doesn't think this signature exists. + # (It does according to the docs) + def on_radio_group_button_toggled(self, button, checked): + """ + Handles the toggled signal on the radio buttons. The signal is emitted twice if a radio butting being toggled on + causes another radio button in the group to be toggled off. + + En/Disables the `Manual Proxy` line edits depending on the currently selected radio button + + :param QtWidgets.QRadioButton button: The button that has toggled + :param bool checked: The buttons new state + """ + id = self.radio_group.id(button) # The work around (see above comment) + enable_manual_edits = id == ProxyMode.MANUAL_PROXY and checked + self.http_edit.setEnabled(enable_manual_edits) + self.https_edit.setEnabled(enable_manual_edits) + self.username_edit.setEnabled(enable_manual_edits) + self.password_edit.setEnabled(enable_manual_edits) + + def retranslate_ui(self): + """ + Translate the Ui + """ + self.setTitle(translate('OpenLP.ProxyWidget', 'Proxy Server Settings')) + self.no_proxy_radio.setText(translate('OpenLP.ProxyWidget', 'No prox&y')) + self.use_sysem_proxy_radio.setText(translate('OpenLP.ProxyWidget', '&Use system proxy')) + self.manual_proxy_radio.setText(translate('OpenLP.ProxyWidget', '&Manual proxy configuration')) + proxy_example = translate('OpenLP.ProxyWidget', 'e.g. proxy_server_address:port_no') + self.layout.labelForField(self.http_edit).setText('HTTP:') + self.http_edit.setPlaceholderText(proxy_example) + self.layout.labelForField(self.https_edit).setText('HTTPS:') + self.https_edit.setPlaceholderText(proxy_example) + self.layout.labelForField(self.username_edit).setText('Username:') + self.layout.labelForField(self.password_edit).setText('Password:') + + def load(self): + """ + Load the data from the settings to the widget. + """ + settings = Settings() + checked_radio = self.radio_group.button(settings.value('advanced/proxy mode')) + checked_radio.setChecked(True) + self.http_edit.setText(settings.value('advanced/proxy http')) + self.https_edit.setText(settings.value('advanced/proxy https')) + self.username_edit.setText(settings.value('advanced/proxy username')) + self.password_edit.setText(settings.value('advanced/proxy password')) + + def save(self): + """ + Save the widget data to the settings + """ + settings = Settings() # TODO: Migrate from old system + settings.setValue('advanced/proxy mode', self.radio_group.checkedId()) + settings.setValue('advanced/proxy http', self.http_edit.text()) + settings.setValue('advanced/proxy https', self.https_edit.text()) + settings.setValue('advanced/proxy username', self.username_edit.text()) + settings.setValue('advanced/proxy password', self.password_edit.text()) diff --git a/tests/interfaces/openlp_core/widgets/test_widgets.py b/tests/interfaces/openlp_core/widgets/test_widgets.py new file mode 100644 index 000000000..8036cee3f --- /dev/null +++ b/tests/interfaces/openlp_core/widgets/test_widgets.py @@ -0,0 +1,155 @@ +# -*- coding: utf-8 -*- +# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4 + +############################################################################### +# OpenLP - Open Source Lyrics Projection # +# --------------------------------------------------------------------------- # +# Copyright (c) 2008-2018 OpenLP Developers # +# --------------------------------------------------------------------------- # +# This program is free software; you can redistribute it and/or modify it # +# under the terms of the GNU General Public License as published by the Free # +# Software Foundation; version 2 of the License. # +# # +# This program is distributed in the hope that it will be useful, but WITHOUT # +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or # +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for # +# more details. # +# # +# You should have received a copy of the GNU General Public License along # +# with this program; if not, write to the Free Software Foundation, Inc., 59 # +# Temple Place, Suite 330, Boston, MA 02111-1307 USA # +############################################################################### +""" +Module to test the custom widgets. +""" +from unittest import TestCase +from unittest.mock import MagicMock, call, patch + +from openlp.core.common.registry import Registry +from openlp.core.common.settings import ProxyMode +from openlp.core.widgets.widgets import ProxyWidget +from tests.helpers.testmixin import TestMixin + + +class TestProxyWidget(TestCase, TestMixin): + """ + Test the EditCustomForm. + """ + def setUp(self): + """ + Create the UI + """ + Registry.create() + self.setup_application() + + def test_radio_button_exclusivity(self): + """ + Test that only one radio button can be checked at a time, and that the line edits are only enabled when the + `manual_proxy_radio` is checked + """ + # GIVEN: An instance of the `openlp.core.common.widgets.widgets.ProxyWidget` + proxy_widget = ProxyWidget() + + # WHEN: 'Checking' the `no_proxy_radio` button + proxy_widget.no_proxy_radio.setChecked(True) + + # THEN: The other radio buttons should not be checked and the line edits should not be enabled + assert proxy_widget.use_sysem_proxy_radio.isChecked() is False + assert proxy_widget.manual_proxy_radio.isChecked() is False + assert proxy_widget.http_edit.isEnabled() is False + assert proxy_widget.https_edit.isEnabled() is False + assert proxy_widget.username_edit.isEnabled() is False + assert proxy_widget.password_edit.isEnabled() is False + + # WHEN: 'Checking' the `use_sysem_proxy_radio` button + proxy_widget.use_sysem_proxy_radio.setChecked(True) + + # THEN: The other radio buttons should not be checked and the line edits should not be enabled + assert proxy_widget.no_proxy_radio.isChecked() is False + assert proxy_widget.manual_proxy_radio.isChecked() is False + assert proxy_widget.http_edit.isEnabled() is False + assert proxy_widget.https_edit.isEnabled() is False + assert proxy_widget.username_edit.isEnabled() is False + assert proxy_widget.password_edit.isEnabled() is False + + # WHEN: 'Checking' the `manual_proxy_radio` button + proxy_widget.manual_proxy_radio.setChecked(True) + + # THEN: The other radio buttons should not be checked and the line edits should be enabled + assert proxy_widget.no_proxy_radio.isChecked() is False + assert proxy_widget.use_sysem_proxy_radio.isChecked() is False + assert proxy_widget.http_edit.isEnabled() is True + assert proxy_widget.https_edit.isEnabled() is True + assert proxy_widget.username_edit.isEnabled() is True + assert proxy_widget.password_edit.isEnabled() is True + + def test_proxy_widget_load_default_settings(self): + """ + Test that the default settings are loaded from the config correctly + """ + # GIVEN: And instance of the widget with default settings + proxy_widget = ProxyWidget() + + # WHEN: Calling the `load` method + proxy_widget.load() + + # THEN: The widget should be in its default state + assert proxy_widget.use_sysem_proxy_radio.isChecked() is True + assert proxy_widget.http_edit.text() == '' + assert proxy_widget.https_edit.text() == '' + assert proxy_widget.username_edit.text() == '' + assert proxy_widget.password_edit.text() == '' + + @patch.object(ProxyWidget, 'load') + @patch('openlp.core.widgets.widgets.Settings') + def test_proxy_widget_save_no_proxy_settings(self, settings_patcher, proxy_widget_load_patcher): + """ + Test that the settings are saved correctly + """ + # GIVEN: A Mocked settings instance of the proxy widget with some known values set + settings_instance = MagicMock() + settings_patcher.return_value = settings_instance + proxy_widget = ProxyWidget() + proxy_widget.no_proxy_radio.setChecked(True) + proxy_widget.http_edit.setText('') + proxy_widget.https_edit.setText('') + proxy_widget.username_edit.setText('') + proxy_widget.password_edit.setText('') + + # WHEN: Calling save + proxy_widget.save() + + # THEN: The settings should be set as expected + settings_instance.setValue.assert_has_calls( + [call('advanced/proxy mode', ProxyMode.NO_PROXY), + call('advanced/proxy http', ''), + call('advanced/proxy https', ''), + call('advanced/proxy username', ''), + call('advanced/proxy password', '')]) + + @patch.object(ProxyWidget, 'load') + @patch('openlp.core.widgets.widgets.Settings') + def test_proxy_widget_save_manual_settings(self, settings_patcher, proxy_widget_load_patcher): + """ + Test that the settings are saved correctly + """ + # GIVEN: A Mocked and instance of the proxy widget with some known values set + settings_instance = MagicMock() + settings_patcher.return_value = settings_instance + proxy_widget = ProxyWidget() + proxy_widget.manual_proxy_radio.setChecked(True) + proxy_widget.http_edit.setText('http_proxy_server:port') + proxy_widget.https_edit.setText('https_proxy_server:port') + proxy_widget.username_edit.setText('username') + proxy_widget.password_edit.setText('password') + + # WHEN: Calling save + proxy_widget.save() + + # THEN: The settings should be set as expected + settings_instance.setValue.assert_has_calls( + [call('advanced/proxy mode', ProxyMode.MANUAL_PROXY), + call('advanced/proxy http', 'http_proxy_server:port'), + call('advanced/proxy https', 'https_proxy_server:port'), + call('advanced/proxy username', 'username'), + call('advanced/proxy password', 'password')]) From fac5d4b79892f87bef5e754fd476163c53ad686f Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Fri, 8 Jun 2018 07:12:23 +0100 Subject: [PATCH 03/18] Fixes --- openlp/core/common/httputils.py | 4 ++-- scripts/jenkins_script.py | 1 + tests/functional/openlp_core/common/test_httputils.py | 4 ++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/openlp/core/common/httputils.py b/openlp/core/common/httputils.py index 40077f630..69310c327 100644 --- a/openlp/core/common/httputils.py +++ b/openlp/core/common/httputils.py @@ -91,9 +91,9 @@ def get_proxy_settings(mode=None): basic_auth = '{username}:{password}@'.format(username=username, password=password) http_value = None https_value = None - if http_addr is not None: + if http_addr: http_value = 'http://{basic_auth}{http_addr}'.format(basic_auth=basic_auth, http_addr=http_addr) - if https_addr is not None: + if https_addr: https_value = 'https://{basic_auth}{https_addr}'.format(basic_auth=basic_auth, https_addr=https_addr) return {'http': http_value, 'https': https_value} diff --git a/scripts/jenkins_script.py b/scripts/jenkins_script.py index 5a1e67c45..9fa1bfdb3 100755 --- a/scripts/jenkins_script.py +++ b/scripts/jenkins_script.py @@ -199,6 +199,7 @@ def get_repo_name(): """ This returns the name of branch of the working directory. For example it returns *lp:~googol/openlp/render*. """ + return 'lp:~phill-ridout/openlp/proxies' # Run the bzr command. bzr = Popen(('bzr', 'info'), stdout=PIPE, stderr=PIPE) raw_output, error = bzr.communicate() diff --git a/tests/functional/openlp_core/common/test_httputils.py b/tests/functional/openlp_core/common/test_httputils.py index 1a979454c..a24f39967 100644 --- a/tests/functional/openlp_core/common/test_httputils.py +++ b/tests/functional/openlp_core/common/test_httputils.py @@ -349,8 +349,8 @@ class TestGetProxySettings(TestCase, TestMixin): """ # GIVEN: A `proxy mode` setting of MANUAL_PROXY with no servers specified Settings().setValue('advanced/proxy mode', ProxyMode.MANUAL_PROXY) - Settings().setValue('advanced/proxy http', None) - Settings().setValue('advanced/proxy https', None) + Settings().setValue('advanced/proxy http', '') + Settings().setValue('advanced/proxy https', '') Settings().setValue('advanced/proxy username', 'user') Settings().setValue('advanced/proxy password', 'pass') From 94758a9778720b9985d0f8625a56720a8057dfc8 Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Fri, 8 Jun 2018 07:21:23 +0100 Subject: [PATCH 04/18] Pep --- tests/functional/openlp_core/common/test_httputils.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/functional/openlp_core/common/test_httputils.py b/tests/functional/openlp_core/common/test_httputils.py index a24f39967..0552cce8f 100644 --- a/tests/functional/openlp_core/common/test_httputils.py +++ b/tests/functional/openlp_core/common/test_httputils.py @@ -279,7 +279,6 @@ class TestGetProxySettings(TestCase, TestMixin): mocked_settings.value.assert_not_called() assert result is None - def test_no_proxy_mode(self): """ Test that a dictionary with http and https values are set to None is returned, when `NO_PROXY` mode is specified @@ -323,7 +322,6 @@ class TestGetProxySettings(TestCase, TestMixin): # THEN: The returned value should be the proxy servers without authentication assert result == {'http': 'http://testhttp.server:port', 'https': 'https://testhttps.server:port'} - def test_manual_proxy_mode_auth(self): """ Test that the correct proxy addresses are returned when basic authentication is used From 03400afc7c30ca77efe9d5a9d22bde5d398cf42d Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Fri, 8 Jun 2018 07:21:41 +0100 Subject: [PATCH 05/18] Pep --- scripts/lp-merge.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/lp-merge.py b/scripts/lp-merge.py index d017821e4..ea30a06cf 100755 --- a/scripts/lp-merge.py +++ b/scripts/lp-merge.py @@ -104,7 +104,7 @@ def get_merge_info(url): # Find the p tag that contains the commit message #
...
...

commit_message = soup.find('div', id='commit-message').find('div', id='edit-commit_message')\ - .find('div', 'yui3-editable_text-text').p + .find('div', 'yui3-editable_text-text').p merge_info['commit_message'] = commit_message.string # Find all tr-tags with this class. Makes it possible to get bug numbers. # Date: Fri, 8 Jun 2018 21:55:20 +0100 Subject: [PATCH 06/18] fix --- openlp/core/common/httputils.py | 15 ++++++++------- scripts/jenkins_script.py | 1 - 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/openlp/core/common/httputils.py b/openlp/core/common/httputils.py index 69310c327..54173a8d2 100644 --- a/openlp/core/common/httputils.py +++ b/openlp/core/common/httputils.py @@ -109,14 +109,15 @@ def get_user_agent(): return browser_list[random_index] -def get_web_page(url, headers=None, update_openlp=False, proxy_mode=None): +def get_web_page(url, headers=None, update_openlp=False, proxy=None): """ Attempts to download the webpage at url and returns that page or None. :param url: The URL to be downloaded. - :param header: An optional HTTP header to pass in the request to the web server. - :param update_openlp: Tells OpenLP to update itself if the page is successfully downloaded. - Defaults to False. + :param dict | None headers: An optional HTTP header to pass in the request to the web server. + :param update_openlp: Tells OpenLP to update itself if the page is successfully downloaded. Defaults to False. + :param dict | ProxyMode | None proxy: ProxyMode enum or a dictionary containing the proxy servers, with their types + as the key e.g. {'http': 'http://proxyserver:port', 'https': 'https://proxyserver:port'} """ if not url: return None @@ -124,13 +125,13 @@ def get_web_page(url, headers=None, update_openlp=False, proxy_mode=None): headers = {} if 'user-agent' not in [key.lower() for key in headers.keys()]: headers['User-Agent'] = get_user_agent() - if proxy_mode is None: - proxies = get_proxy_settings(mode=proxy_mode) + if not isinstance(proxy, dict): + proxy = get_proxy_settings(mode=proxy) log.debug('Downloading URL = %s' % url) retries = 0 while retries < CONNECTION_RETRIES: try: - response = requests.get(url, headers=headers, proxies=proxies, timeout=float(CONNECTION_TIMEOUT)) + response = requests.get(url, headers=headers, proxies=proxy, timeout=float(CONNECTION_TIMEOUT)) log.debug('Downloaded page {url}'.format(url=response.url)) break except OSError: diff --git a/scripts/jenkins_script.py b/scripts/jenkins_script.py index 9fa1bfdb3..5a1e67c45 100755 --- a/scripts/jenkins_script.py +++ b/scripts/jenkins_script.py @@ -199,7 +199,6 @@ def get_repo_name(): """ This returns the name of branch of the working directory. For example it returns *lp:~googol/openlp/render*. """ - return 'lp:~phill-ridout/openlp/proxies' # Run the bzr command. bzr = Popen(('bzr', 'info'), stdout=PIPE, stderr=PIPE) raw_output, error = bzr.communicate() From 70c777b7e36a634074f73ab99750acb647591eab Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Sun, 10 Jun 2018 07:38:16 +0100 Subject: [PATCH 07/18] Add translate methods --- openlp/core/widgets/widgets.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/openlp/core/widgets/widgets.py b/openlp/core/widgets/widgets.py index bb65410bd..052057eff 100644 --- a/openlp/core/widgets/widgets.py +++ b/openlp/core/widgets/widgets.py @@ -75,8 +75,6 @@ class ProxyWidget(QtWidgets.QGroupBox): # Signal / Slots self.radio_group.buttonToggled.connect(self.on_radio_group_button_toggled) - # @QtCore.pyqtSlot(int, bool) For some reason PyQt doesn't think this signature exists. - # (It does according to the docs) def on_radio_group_button_toggled(self, button, checked): """ Handles the toggled signal on the radio buttons. The signal is emitted twice if a radio butting being toggled on @@ -103,12 +101,12 @@ class ProxyWidget(QtWidgets.QGroupBox): self.use_sysem_proxy_radio.setText(translate('OpenLP.ProxyWidget', '&Use system proxy')) self.manual_proxy_radio.setText(translate('OpenLP.ProxyWidget', '&Manual proxy configuration')) proxy_example = translate('OpenLP.ProxyWidget', 'e.g. proxy_server_address:port_no') - self.layout.labelForField(self.http_edit).setText('HTTP:') + self.layout.labelForField(self.http_edit).setText(translate('OpenLP.ProxyWidget', 'HTTP:')) self.http_edit.setPlaceholderText(proxy_example) - self.layout.labelForField(self.https_edit).setText('HTTPS:') + self.layout.labelForField(self.https_edit).setText(translate('OpenLP.ProxyWidget', 'HTTPS:')) self.https_edit.setPlaceholderText(proxy_example) - self.layout.labelForField(self.username_edit).setText('Username:') - self.layout.labelForField(self.password_edit).setText('Password:') + self.layout.labelForField(self.username_edit).setText(translate('OpenLP.ProxyWidget', 'Username:')) + self.layout.labelForField(self.password_edit).setText(translate('OpenLP.ProxyWidget', 'Password:')) def load(self): """ From 2aaa0bf2870e5edab9bc314c534fa406d21a2156 Mon Sep 17 00:00:00 2001 From: Phill Ridout Date: Sun, 10 Jun 2018 07:38:42 +0100 Subject: [PATCH 08/18] break out the code using multiple when/thens --- .../openlp_core/widgets/test_widgets.py | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/tests/interfaces/openlp_core/widgets/test_widgets.py b/tests/interfaces/openlp_core/widgets/test_widgets.py index 8036cee3f..5353596fd 100644 --- a/tests/interfaces/openlp_core/widgets/test_widgets.py +++ b/tests/interfaces/openlp_core/widgets/test_widgets.py @@ -42,13 +42,14 @@ class TestProxyWidget(TestCase, TestMixin): Registry.create() self.setup_application() - def test_radio_button_exclusivity(self): + def test_radio_button_exclusivity_no_proxy(self): """ Test that only one radio button can be checked at a time, and that the line edits are only enabled when the `manual_proxy_radio` is checked """ - # GIVEN: An instance of the `openlp.core.common.widgets.widgets.ProxyWidget` + # GIVEN: An instance of the `openlp.core.common.widgets.widgets.ProxyWidget` with a radio already checked proxy_widget = ProxyWidget() + proxy_widget.manual_proxy_radio.setChecked(True) # WHEN: 'Checking' the `no_proxy_radio` button proxy_widget.no_proxy_radio.setChecked(True) @@ -61,6 +62,15 @@ class TestProxyWidget(TestCase, TestMixin): assert proxy_widget.username_edit.isEnabled() is False assert proxy_widget.password_edit.isEnabled() is False + def test_radio_button_exclusivity_system_proxy(self): + """ + Test that only one radio button can be checked at a time, and that the line edits are only enabled when the + `manual_proxy_radio` is checked + """ + # GIVEN: An instance of the `openlp.core.common.widgets.widgets.ProxyWidget` with a radio already checked + proxy_widget = ProxyWidget() + proxy_widget.manual_proxy_radio.setChecked(True) + # WHEN: 'Checking' the `use_sysem_proxy_radio` button proxy_widget.use_sysem_proxy_radio.setChecked(True) @@ -72,6 +82,15 @@ class TestProxyWidget(TestCase, TestMixin): assert proxy_widget.username_edit.isEnabled() is False assert proxy_widget.password_edit.isEnabled() is False + def test_radio_button_exclusivity_manual_proxy(self): + """ + Test that only one radio button can be checked at a time, and that the line edits are only enabled when the + `manual_proxy_radio` is checked + """ + # GIVEN: An instance of the `openlp.core.common.widgets.widgets.ProxyWidget` with a radio already checked + proxy_widget = ProxyWidget() + proxy_widget.no_proxy_radio.setChecked(True) + # WHEN: 'Checking' the `manual_proxy_radio` button proxy_widget.manual_proxy_radio.setChecked(True) From 318e90f89318585c120fab1320045979ae457952 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Mon, 2 Jul 2018 22:38:47 +0200 Subject: [PATCH 09/18] Fixes for pycodestyle --- openlp/core/api/endpoint/pluginhelpers.py | 2 +- openlp/core/api/tab.py | 2 +- openlp/core/lib/__init__.py | 2 +- openlp/core/projectors/pjlink.py | 2 +- openlp/core/ui/media/__init__.py | 2 +- openlp/plugins/bibles/forms/editbibleform.py | 2 +- openlp/plugins/bibles/lib/__init__.py | 2 +- openlp/plugins/bibles/lib/mediaitem.py | 2 +- .../media/forms/mediaclipselectorform.py | 2 +- openlp/plugins/songs/forms/editsongform.py | 4 ++-- openlp/plugins/songs/lib/__init__.py | 4 ++-- .../plugins/songs/lib/importers/easyslides.py | 4 ++-- .../songs/lib/importers/foilpresenter.py | 24 +++++++++---------- openlp/plugins/songs/lib/importers/lyrix.py | 2 +- .../plugins/songs/lib/importers/opensong.py | 6 ++--- openlp/plugins/songs/lib/importers/opspro.py | 22 ++++++++--------- .../lib/importers/presentationmanager.py | 2 +- .../songs/lib/importers/songsoffellowship.py | 2 +- .../songs/lib/importers/worshipassistant.py | 2 +- scripts/lp-merge.py | 2 +- setup.cfg | 7 +++++- tests/functional/openlp_core/api/test_tab.py | 2 +- .../openlp_core/common/test_init.py | 2 +- .../openlp_core/ui/test_filerenamedialog.py | 2 +- 24 files changed, 55 insertions(+), 50 deletions(-) diff --git a/openlp/core/api/endpoint/pluginhelpers.py b/openlp/core/api/endpoint/pluginhelpers.py index a74fa524a..75dc8e8e6 100644 --- a/openlp/core/api/endpoint/pluginhelpers.py +++ b/openlp/core/api/endpoint/pluginhelpers.py @@ -115,7 +115,7 @@ def display_thumbnails(request, controller_name, log, dimensions, file_name, sli height = -1 image = None if dimensions: - match = re.search('(\d+)x(\d+)', dimensions) + match = re.search(r'(\d+)x(\d+)', dimensions) if match: # let's make sure that the dimensions are within reason width = sorted([10, int(match.group(1)), 1000])[1] diff --git a/openlp/core/api/tab.py b/openlp/core/api/tab.py index 840fb0d0b..722c7547a 100644 --- a/openlp/core/api/tab.py +++ b/openlp/core/api/tab.py @@ -55,7 +55,7 @@ class ApiTab(SettingsTab): self.address_label.setObjectName('address_label') self.address_edit = QtWidgets.QLineEdit(self.server_settings_group_box) self.address_edit.setSizePolicy(QtWidgets.QSizePolicy.Preferred, QtWidgets.QSizePolicy.Fixed) - self.address_edit.setValidator(QtGui.QRegExpValidator(QtCore.QRegExp('\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}'), + self.address_edit.setValidator(QtGui.QRegExpValidator(QtCore.QRegExp(r'\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}'), self)) self.address_edit.setObjectName('address_edit') self.server_settings_layout.addRow(self.address_label, self.address_edit) diff --git a/openlp/core/lib/__init__.py b/openlp/core/lib/__init__.py index bcc87c19e..c4aa92702 100644 --- a/openlp/core/lib/__init__.py +++ b/openlp/core/lib/__init__.py @@ -415,7 +415,7 @@ def expand_chords(text): chords_on_prev_line = True # Matches a chord, a tail, a remainder and a line end. See expand_and_align_chords_in_line() for more info. new_line += re.sub(r'\[(.*?)\]([\u0080-\uFFFF,\w]*)' - '([\u0080-\uFFFF,\w,\s,\.,\,,\!,\?,\;,\:,\|,\",\',\-,\_]*)(\Z)?', + r'([\u0080-\uFFFF,\w,\s,\.,\,,\!,\?,\;,\:,\|,\",\',\-,\_]*)(\Z)?', expand_and_align_chords_in_line, line) new_line += '' expanded_text_lines.append(new_line) diff --git a/openlp/core/projectors/pjlink.py b/openlp/core/projectors/pjlink.py index b2b23a188..3ea1bb414 100644 --- a/openlp/core/projectors/pjlink.py +++ b/openlp/core/projectors/pjlink.py @@ -338,7 +338,7 @@ class PJLinkCommands(object): # Due to stupid projectors not following standards (Optoma, BenQ comes to mind), # AND the different responses that can be received, the semi-permanent way to # fix the class reply is to just remove all non-digit characters. - chk = re.findall('\d', data) + chk = re.findall(r'\d', data) if len(chk) < 1: log.error('({ip}) No numbers found in class version reply "{data}" - ' 'defaulting to class "1"'.format(ip=self.entry.name, data=data)) diff --git a/openlp/core/ui/media/__init__.py b/openlp/core/ui/media/__init__.py index da80aee5f..976e0b744 100644 --- a/openlp/core/ui/media/__init__.py +++ b/openlp/core/ui/media/__init__.py @@ -80,7 +80,7 @@ def get_media_players(): """ log.debug('get_media_players') saved_players = Settings().value('media/players') - reg_ex = QtCore.QRegExp(".*\[(.*)\].*") + reg_ex = QtCore.QRegExp(r'.*\[(.*)\].*') if Settings().value('media/override player') == QtCore.Qt.Checked: if reg_ex.exactMatch(saved_players): overridden_player = '{text}'.format(text=reg_ex.cap(1)) diff --git a/openlp/plugins/bibles/forms/editbibleform.py b/openlp/plugins/bibles/forms/editbibleform.py index bc89827d2..086a65ceb 100644 --- a/openlp/plugins/bibles/forms/editbibleform.py +++ b/openlp/plugins/bibles/forms/editbibleform.py @@ -183,7 +183,7 @@ class EditBibleForm(QtWidgets.QDialog, Ui_EditBibleDialog, RegistryProperties): """ Validate a book. """ - book_regex = re.compile('[\d]*[^\d]+$') + book_regex = re.compile(r'[\d]*[^\d]+$') if not new_book_name: self.book_name_edit[abbreviation].setFocus() critical_error_message_box( diff --git a/openlp/plugins/bibles/lib/__init__.py b/openlp/plugins/bibles/lib/__init__.py index 73c1ca68d..da6913647 100644 --- a/openlp/plugins/bibles/lib/__init__.py +++ b/openlp/plugins/bibles/lib/__init__.py @@ -217,7 +217,7 @@ def update_reference_separators(): # add various Unicode alternatives source_string = source_string.replace('-', '(?:[-\u00AD\u2010\u2011\u2012\u2014\u2014\u2212\uFE63\uFF0D])') source_string = source_string.replace(',', '(?:[,\u201A])') - REFERENCE_SEPARATORS['sep_{role}'.format(role=role)] = '\s*(?:{source})\s*'.format(source=source_string) + REFERENCE_SEPARATORS['sep_{role}'.format(role=role)] = r'\s*(?:{source})\s*'.format(source=source_string) REFERENCE_SEPARATORS['sep_{role}_default'.format(role=role)] = default_separators[index] # verse range match: (:)?(-((:)?|end)?)? range_regex = '(?:(?P[0-9]+){sep_v})?' \ diff --git a/openlp/plugins/bibles/lib/mediaitem.py b/openlp/plugins/bibles/lib/mediaitem.py index c2cbec7ae..2f42a05ac 100755 --- a/openlp/plugins/bibles/lib/mediaitem.py +++ b/openlp/plugins/bibles/lib/mediaitem.py @@ -41,7 +41,7 @@ from openlp.plugins.bibles.lib import DisplayStyle, LayoutStyle, VerseReferenceL log = logging.getLogger(__name__) -VALID_TEXT_SEARCH = re.compile('\w\w\w') +VALID_TEXT_SEARCH = re.compile(r'\w\w\w') def get_reference_separators(): diff --git a/openlp/plugins/media/forms/mediaclipselectorform.py b/openlp/plugins/media/forms/mediaclipselectorform.py index 7c0db8db7..462aebd08 100644 --- a/openlp/plugins/media/forms/mediaclipselectorform.py +++ b/openlp/plugins/media/forms/mediaclipselectorform.py @@ -210,7 +210,7 @@ class MediaClipSelectorForm(QtWidgets.QDialog, Ui_MediaClipSelector, RegistryPro # detect if we're dealing with a DVD or CD, so we use different loading approaches depending on the OS. if is_win(): # If the given path is in the format "D:\" or "D:", prefix it with "/" to make VLC happy - pattern = re.compile('^\w:\\\\*$') + pattern = re.compile(r'^\w:\\\\*$') if pattern.match(path): path = '/' + path self.vlc_media = self.vlc_instance.media_new_location('dvd://' + path) diff --git a/openlp/plugins/songs/forms/editsongform.py b/openlp/plugins/songs/forms/editsongform.py index 36b144aef..53aa443bb 100644 --- a/openlp/plugins/songs/forms/editsongform.py +++ b/openlp/plugins/songs/forms/editsongform.py @@ -105,7 +105,7 @@ class EditSongForm(QtWidgets.QDialog, Ui_EditSongDialog, RegistryProperties): self.topics_list_view.setSortingEnabled(False) self.topics_list_view.setAlternatingRowColors(True) self.audio_list_widget.setAlternatingRowColors(True) - self.find_verse_split = re.compile('---\[\]---\n') + self.find_verse_split = re.compile(r'---\[\]---\n') self.whitespace = re.compile(r'\W+') self.find_tags = re.compile(r'\{/?\w+\}') @@ -316,7 +316,7 @@ class EditSongForm(QtWidgets.QDialog, Ui_EditSongDialog, RegistryProperties): multiple.append(verse_tag) self.song.lyrics = str(sxml.extract_xml(), 'utf-8') for verse in multiple: - self.song.verse_order = re.sub('([' + verse.upper() + verse.lower() + '])(\W|$)', + self.song.verse_order = re.sub(r'([' + verse.upper() + verse.lower() + r'])(\W|$)', r'\g<1>1\2', self.song.verse_order) except: log.exception('Problem processing song Lyrics \n{xml}'.format(xml=sxml.dump_xml())) diff --git a/openlp/plugins/songs/lib/__init__.py b/openlp/plugins/songs/lib/__init__.py index 2578d0f92..c86d822fd 100644 --- a/openlp/plugins/songs/lib/__init__.py +++ b/openlp/plugins/songs/lib/__init__.py @@ -557,7 +557,7 @@ def transpose_lyrics(lyrics, transpose_value): :return: The transposed lyrics """ # Split text by verse delimiter - both normal and optional - verse_list = re.split('(---\[.+?:.+?\]---|\[---\])', lyrics) + verse_list = re.split(r'(---\[.+?:.+?\]---|\[---\])', lyrics) transposed_lyrics = '' notation = Settings().value('songs/chord notation') for verse in verse_list: @@ -580,7 +580,7 @@ def transpose_verse(verse_text, transpose_value, notation): if '[' not in verse_text: return verse_text # Split the lyrics based on chord tags - lyric_list = re.split('(\[|\]|/)', verse_text) + lyric_list = re.split(r'(\[|\]|/)', verse_text) transposed_lyrics = '' in_tag = False for word in lyric_list: diff --git a/openlp/plugins/songs/lib/importers/easyslides.py b/openlp/plugins/songs/lib/importers/easyslides.py index 8f847c5f3..4a6fc5bf8 100644 --- a/openlp/plugins/songs/lib/importers/easyslides.py +++ b/openlp/plugins/songs/lib/importers/easyslides.py @@ -37,7 +37,7 @@ class EasySlidesImport(SongImport): Import songs exported from EasySlides The format example is here: - http://wiki.openlp.org/Development:EasySlides\_-_Song_Data_Format + http://wiki.openlp.org/Development:EasySlides_-_Song_Data_Format """ def __init__(self, manager, **kwargs): """ @@ -210,7 +210,7 @@ class EasySlidesImport(SongImport): vn = '1' # have we got any digits? # If so, versenumber is everything from the digits to the end - match = re.match('(.*)(\d+.*)', marker) + match = re.match(r'(.*)(\d+.*)', marker) if match: marker = match.group(1).strip() vn = match.group(2) diff --git a/openlp/plugins/songs/lib/importers/foilpresenter.py b/openlp/plugins/songs/lib/importers/foilpresenter.py index 82601f18b..5458b1fad 100644 --- a/openlp/plugins/songs/lib/importers/foilpresenter.py +++ b/openlp/plugins/songs/lib/importers/foilpresenter.py @@ -273,15 +273,15 @@ class FoilPresenter(object): elif copyright.find('C,)') != -1: temp = copyright.partition('C,)') copyright = temp[0] - copyright = re.compile('\\n').sub(' ', copyright) - copyright = re.compile('\(.*\)').sub('', copyright) + copyright = re.compile(r'\\n').sub(' ', copyright) + copyright = re.compile(r'\(.*\)').sub('', copyright) if copyright.find('Rechte') != -1: temp = copyright.partition('Rechte') copyright = temp[0] - markers = ['Text +u\.?n?d? +Melodie[\w\,\. ]*:', - 'Text +u\.?n?d? +Musik', 'T & M', 'Melodie und Satz', - 'Text[\w\,\. ]*:', 'Melodie', 'Musik', 'Satz', - 'Weise', '[dD]eutsch', '[dD]t[\.\:]', 'Englisch', + markers = [r'Text +u\.?n?d? +Melodie[\w\,\. ]*:', + r'Text +u\.?n?d? +Musik', 'T & M', 'Melodie und Satz', + r'Text[\w\,\. ]*:', 'Melodie', 'Musik', 'Satz', + 'Weise', '[dD]eutsch', r'[dD]t[\.\:]', 'Englisch', '[oO]riginal', 'Bearbeitung', '[R|r]efrain'] for marker in markers: copyright = re.compile(marker).sub('', copyright, re.U) @@ -301,17 +301,17 @@ class FoilPresenter(object): break author_temp = [] for author in strings: - temp = re.split(',(?=\D{2})|(?<=\D),|\/(?=\D{3,})|(?<=\D);', author) + temp = re.split(r',(?=\D{2})|(?<=\D),|\/(?=\D{3,})|(?<=\D);', author) for tempx in temp: author_temp.append(tempx) for author in author_temp: - regex = '^[\/,;\-\s\.]+|[\/,;\-\s\.]+$|\s*[0-9]{4}\s*[\-\/]?\s*([0-9]{4})?[\/,;\-\s\.]*$' + regex = r'^[\/,;\-\s\.]+|[\/,;\-\s\.]+$|\s*[0-9]{4}\s*[\-\/]?\s*([0-9]{4})?[\/,;\-\s\.]*$' author = re.compile(regex).sub('', author) - author = re.compile('[0-9]{1,2}\.\s?J(ahr)?h\.|um\s*$|vor\s*$').sub('', author) - author = re.compile('[N|n]ach.*$').sub('', author) + author = re.compile(r'[0-9]{1,2}\.\s?J(ahr)?h\.|um\s*$|vor\s*$').sub('', author) + author = re.compile(r'[N|n]ach.*$').sub('', author) author = author.strip() - if re.search('\w+\.?\s+\w{3,}\s+[a|u]nd\s|\w+\.?\s+\w{3,}\s+&\s', author, re.U): - temp = re.split('\s[a|u]nd\s|\s&\s', author) + if re.search(r'\w+\.?\s+\w{3,}\s+[a|u]nd\s|\w+\.?\s+\w{3,}\s+&\s', author, re.U): + temp = re.split(r'\s[a|u]nd\s|\s&\s', author) for tempx in temp: tempx = tempx.strip() authors.append(tempx) diff --git a/openlp/plugins/songs/lib/importers/lyrix.py b/openlp/plugins/songs/lib/importers/lyrix.py index 386fe43e2..e987b2502 100644 --- a/openlp/plugins/songs/lib/importers/lyrix.py +++ b/openlp/plugins/songs/lib/importers/lyrix.py @@ -80,7 +80,7 @@ class LyrixImport(SongImport): continue # Detect and get CCLI number if line.lower().startswith('ccli'): - ccli = re.findall('\d+', line)[0] + ccli = re.findall(r'\d+', line)[0] try: # If the CCLI was found, we are near the end # Find author diff --git a/openlp/plugins/songs/lib/importers/opensong.py b/openlp/plugins/songs/lib/importers/opensong.py index 25e52777a..470330b3b 100644 --- a/openlp/plugins/songs/lib/importers/opensong.py +++ b/openlp/plugins/songs/lib/importers/opensong.py @@ -156,7 +156,7 @@ class OpenSongImport(SongImport): ustring = str(root.__getattr__(attr)) if isinstance(fn_or_string, str): if attr in ['ccli']: - ustring = ''.join(re.findall('\d+', ustring)) + ustring = ''.join(re.findall(r'\d+', ustring)) if ustring: setattr(self, fn_or_string, int(ustring)) else: @@ -231,7 +231,7 @@ class OpenSongImport(SongImport): content = this_line[1:right_bracket].lower() # have we got any digits? If so, verse number is everything from the digits to the end (openlp does not # have concept of part verses, so just ignore any non integers on the end (including floats)) - match = re.match('(\D*)(\d+)', content) + match = re.match(r'(\D*)(\d+)', content) if match is not None: verse_tag = match.group(1) verse_num = match.group(2) @@ -303,7 +303,7 @@ class OpenSongImport(SongImport): # whitespace. order = order.lower().split() for verse_def in order: - match = re.match('(\D*)(\d+.*)', verse_def) + match = re.match(r'(\D*)(\d+.*)', verse_def) if match is not None: verse_tag = match.group(1) verse_num = match.group(2) diff --git a/openlp/plugins/songs/lib/importers/opspro.py b/openlp/plugins/songs/lib/importers/opspro.py index 5edcfdd39..c21bab2d2 100644 --- a/openlp/plugins/songs/lib/importers/opspro.py +++ b/openlp/plugins/songs/lib/importers/opspro.py @@ -122,7 +122,7 @@ class OPSProImport(SongImport): # Try to split lyrics based on various rules if lyrics: lyrics_text = lyrics.Lyrics - verses = re.split('\r\n\s*?\r\n', lyrics_text) + verses = re.split(r'\r\n\s*?\r\n', lyrics_text) verse_tag_defs = {} verse_tag_texts = {} for verse_text in verses: @@ -130,13 +130,13 @@ class OPSProImport(SongImport): continue verse_def = 'v' # Detect verse number - verse_number = re.match('^(\d+)\r\n', verse_text) + verse_number = re.match(r'^(\d+)\r\n', verse_text) if verse_number: - verse_text = re.sub('^\d+\r\n', '', verse_text) + verse_text = re.sub(r'^\d+\r\n', '', verse_text) verse_def = 'v' + verse_number.group(1) # Detect verse tags - elif re.match('^.+?\:\r\n', verse_text): - tag_match = re.match('^(.+?)\:\r\n(.*)', verse_text, flags=re.DOTALL) + elif re.match(r'^.+?\:\r\n', verse_text): + tag_match = re.match(r'^(.+?)\:\r\n(.*)', verse_text, flags=re.DOTALL) tag = tag_match.group(1).lower() tag = tag.split(' ')[0] verse_text = tag_match.group(2) @@ -147,25 +147,25 @@ class OPSProImport(SongImport): verse_tag_defs[tag] = verse_def verse_tag_texts[tag] = verse_text # Detect tag reference - elif re.match('^\(.*?\)$', verse_text): - tag_match = re.match('^\((.*?)\)$', verse_text) + elif re.match(r'^\(.*?\)$', verse_text): + tag_match = re.match(r'^\((.*?)\)$', verse_text) tag = tag_match.group(1).lower() if tag in verse_tag_defs: verse_text = verse_tag_texts[tag] verse_def = verse_tag_defs[tag] # Detect end tag - elif re.match('^\[slot\]\r\n', verse_text, re.IGNORECASE): + elif re.match(r'^\[slot\]\r\n', verse_text, re.IGNORECASE): verse_def = 'e' - verse_text = re.sub('^\[slot\]\r\n', '', verse_text, flags=re.IGNORECASE) + verse_text = re.sub(r'^\[slot\]\r\n', '', verse_text, flags=re.IGNORECASE) # Replace the join tag with line breaks verse_text = verse_text.replace('[join]', '') # Replace the split tag with line breaks and an optional split - verse_text = re.sub('\[splits?\]', '\r\n[---]', verse_text) + verse_text = re.sub(r'\[splits?\]', '\r\n[---]', verse_text) # Handle translations if lyrics.IsDualLanguage: verse_text = self.handle_translation(verse_text) # Remove comments - verse_text = re.sub('\(.*?\)\r\n', '', verse_text, flags=re.IGNORECASE) + verse_text = re.sub(r'\(.*?\)\r\n', '', verse_text, flags=re.IGNORECASE) self.add_verse(verse_text, verse_def) self.finish() diff --git a/openlp/plugins/songs/lib/importers/presentationmanager.py b/openlp/plugins/songs/lib/importers/presentationmanager.py index 9b16f24a6..49cd9e220 100644 --- a/openlp/plugins/songs/lib/importers/presentationmanager.py +++ b/openlp/plugins/songs/lib/importers/presentationmanager.py @@ -51,7 +51,7 @@ class PresentationManagerImport(SongImport): encoding = get_file_encoding(file_path)['encoding'] # Open file with detected encoding and remove encoding declaration text = file_path.read_text(encoding=encoding) - text = re.sub('.+\?>\n', '', text) + text = re.sub(r'.+\?>\n', '', text) try: tree = etree.fromstring(text, parser=etree.XMLParser(recover=True)) except ValueError: diff --git a/openlp/plugins/songs/lib/importers/songsoffellowship.py b/openlp/plugins/songs/lib/importers/songsoffellowship.py index d0c0a6720..a5d254252 100644 --- a/openlp/plugins/songs/lib/importers/songsoffellowship.py +++ b/openlp/plugins/songs/lib/importers/songsoffellowship.py @@ -333,7 +333,7 @@ class SongsOfFellowshipImport(OpenOfficeImport): There is a complicated word "One", which is sometimes lower and sometimes upper depending on context. Never mind, keep it lower. """ - text_arr = re.split('(\W+)', text) + text_arr = re.split(r'(\W+)', text) text_arr[0] = text_arr[0].capitalize() for i in range(1, len(text_arr)): # Do not translate these. Fixed strings in SOF song file diff --git a/openlp/plugins/songs/lib/importers/worshipassistant.py b/openlp/plugins/songs/lib/importers/worshipassistant.py index 02df16b4a..6f372876e 100644 --- a/openlp/plugins/songs/lib/importers/worshipassistant.py +++ b/openlp/plugins/songs/lib/importers/worshipassistant.py @@ -142,7 +142,7 @@ class WorshipAssistantImport(SongImport): # drop the square brackets right_bracket = line.find(']') content = line[1:right_bracket].lower() - match = re.match('(\D*)(\d+)', content) + match = re.match(r'(\D*)(\d+)', content) if match is not None: verse_tag = match.group(1) verse_num = match.group(2) diff --git a/scripts/lp-merge.py b/scripts/lp-merge.py index 9bb8ac17a..c10e9e0d2 100755 --- a/scripts/lp-merge.py +++ b/scripts/lp-merge.py @@ -124,7 +124,7 @@ def get_merge_info(url): script_tag = soup.find('script', attrs={"id": "codereview-script"}) content = script_tag.contents[0] start_pos = content.find('source_revid') + 16 - pattern = re.compile('.*\w-\d\d\d\d\d+') + pattern = re.compile(r'.*\w-\d\d\d\d\d+') match = pattern.match(content[start_pos:]) merge_info['author_email'] = match.group()[:-15] # Launchpad doesn't supply the author's true name, so we'll just grab whatever they use for display on LP diff --git a/setup.cfg b/setup.cfg index e2ef020e0..223e5c7c2 100644 --- a/setup.cfg +++ b/setup.cfg @@ -15,5 +15,10 @@ ignore = E402 [pycodestyle] exclude = resources.py,vlc.py max-line-length = 120 -ignore = E402 +# Ignoring: +# E402... +# E722 do not use bare 'except' +# W503 line break before binary operator +# W504 line break after binary operator +ignore = E402,E722,W503,W504 diff --git a/tests/functional/openlp_core/api/test_tab.py b/tests/functional/openlp_core/api/test_tab.py index cf8b4d0b0..e2c93f32e 100644 --- a/tests/functional/openlp_core/api/test_tab.py +++ b/tests/functional/openlp_core/api/test_tab.py @@ -87,7 +87,7 @@ class TestApiTab(TestCase, TestMixin): ip_address = self.form.get_ip_address(ZERO_URL) # THEN: the default ip address will be returned - assert re.match('\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}', ip_address), \ + assert re.match(r'\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}', ip_address), \ 'The return value should be a valid ip address' assert ip_address in ip4_list, 'The return address should be in the list of local IP addresses' diff --git a/tests/functional/openlp_core/common/test_init.py b/tests/functional/openlp_core/common/test_init.py index 83f0c1c7c..6256517d0 100644 --- a/tests/functional/openlp_core/common/test_init.py +++ b/tests/functional/openlp_core/common/test_init.py @@ -238,7 +238,7 @@ class TestInit(TestCase, TestMixin): Test the clean_filename() function """ # GIVEN: A invalid file name and the valid file name. - invalid_name = 'A_file_with_invalid_characters_[\\/:\*\?"<>\|\+\[\]%].py' + invalid_name = 'A_file_with_invalid_characters_[\\/:\*\?"<>\|\+\[\]%].py' # nopep8 wanted_name = 'A_file_with_invalid_characters______________________.py' # WHEN: Clean the name. diff --git a/tests/interfaces/openlp_core/ui/test_filerenamedialog.py b/tests/interfaces/openlp_core/ui/test_filerenamedialog.py index e268aef2f..ebff60310 100644 --- a/tests/interfaces/openlp_core/ui/test_filerenamedialog.py +++ b/tests/interfaces/openlp_core/ui/test_filerenamedialog.py @@ -99,7 +99,7 @@ class TestStartFileRenameForm(TestCase, TestMixin): # GIVEN: QLineEdit with a validator set with illegal file name characters. # WHEN: 'Typing' a string containing invalid file characters. - QtTest.QTest.keyClicks(self.form.file_name_edit, 'I/n\\v?a*l|i \F[i\l]e" :N+a%me') + QtTest.QTest.keyClicks(self.form.file_name_edit, r'I/n\\v?a*l|i \F[i\l]e" :N+a%me') # THEN: The text in the QLineEdit should be the same as the input string with the invalid characters filtered # out. From 56a811c094a2e4a6982f6f40a2808e9f88fc6426 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Wed, 4 Jul 2018 22:42:55 +0200 Subject: [PATCH 10/18] More pycodestyle fixes --- openlp/core/api/http/wsgiapp.py | 2 +- openlp/core/lib/htmlbuilder.py | 4 ++-- openlp/plugins/bibles/lib/__init__.py | 2 +- openlp/plugins/songs/lib/importers/dreambeam.py | 2 +- openlp/plugins/songs/lib/importers/powersong.py | 2 +- tests/functional/openlp_core/common/test_init.py | 5 +++-- tests/functional/openlp_core/lib/test_htmlbuilder.py | 2 +- 7 files changed, 10 insertions(+), 9 deletions(-) diff --git a/openlp/core/api/http/wsgiapp.py b/openlp/core/api/http/wsgiapp.py index 48364f340..aa90a28aa 100644 --- a/openlp/core/api/http/wsgiapp.py +++ b/openlp/core/api/http/wsgiapp.py @@ -39,7 +39,7 @@ log = logging.getLogger(__name__) def _route_to_regex(route): - """ + r""" Convert a route to a regular expression For example: diff --git a/openlp/core/lib/htmlbuilder.py b/openlp/core/lib/htmlbuilder.py index f6a0c6e61..5ef94288c 100644 --- a/openlp/core/lib/htmlbuilder.py +++ b/openlp/core/lib/htmlbuilder.py @@ -19,7 +19,7 @@ # with this program; if not, write to the Free Software Foundation, Inc., 59 # # Temple Place, Suite 330, Boston, MA 02111-1307 USA # ############################################################################### -""" +r""" This module is responsible for generating the HTML for :class:`~openlp.core.ui.maindisplay`. The ``build_html`` function is the function which has to be called from outside. The generated and returned HTML will look similar to this:: @@ -416,7 +416,7 @@ from openlp.core.lib.theme import BackgroundType, BackgroundGradientType, Vertic log = logging.getLogger(__name__) -HTML_SRC = Template(""" +HTML_SRC = Template(r""" diff --git a/openlp/plugins/bibles/lib/__init__.py b/openlp/plugins/bibles/lib/__init__.py index da6913647..b1a51bfd7 100644 --- a/openlp/plugins/bibles/lib/__init__.py +++ b/openlp/plugins/bibles/lib/__init__.py @@ -255,7 +255,7 @@ def get_reference_match(match_type): def parse_reference(reference, bible, language_selection, book_ref_id=False): - """ + r""" This is the next generation über-awesome function that takes a person's typed in string and converts it to a list of references to be queried from the Bible database files. diff --git a/openlp/plugins/songs/lib/importers/dreambeam.py b/openlp/plugins/songs/lib/importers/dreambeam.py index 532bd865d..71e233eed 100644 --- a/openlp/plugins/songs/lib/importers/dreambeam.py +++ b/openlp/plugins/songs/lib/importers/dreambeam.py @@ -73,7 +73,7 @@ class DreamBeamImport(SongImport): Valid extensions for a DreamBeam song file are: - * \*.xml + * .xml """ def do_import(self): diff --git a/openlp/plugins/songs/lib/importers/powersong.py b/openlp/plugins/songs/lib/importers/powersong.py index d73e3ed9c..c21586166 100644 --- a/openlp/plugins/songs/lib/importers/powersong.py +++ b/openlp/plugins/songs/lib/importers/powersong.py @@ -70,7 +70,7 @@ class PowerSongImport(SongImport): """ Checks if source is a PowerSong 1.0 folder: * is a directory - * contains at least one \*.song file + * contains at least one * .song file :param openlp.core.common.path.Path import_source: Should be a Path object that fulfills the above criteria :return: If the source is valid diff --git a/tests/functional/openlp_core/common/test_init.py b/tests/functional/openlp_core/common/test_init.py index 6256517d0..552bb9f29 100644 --- a/tests/functional/openlp_core/common/test_init.py +++ b/tests/functional/openlp_core/common/test_init.py @@ -238,13 +238,14 @@ class TestInit(TestCase, TestMixin): Test the clean_filename() function """ # GIVEN: A invalid file name and the valid file name. - invalid_name = 'A_file_with_invalid_characters_[\\/:\*\?"<>\|\+\[\]%].py' # nopep8 - wanted_name = 'A_file_with_invalid_characters______________________.py' + invalid_name = 'A_file_with_invalid_characters_[\\/:*?"<>|+[]%].py' + wanted_name = 'A_file_with_invalid_characters________________.py' # WHEN: Clean the name. result = clean_filename(invalid_name) # THEN: The file name should be cleaned. + print(result) assert wanted_name == result, 'The file name should not contain any special characters.' def test_delete_file_no_path(self): diff --git a/tests/functional/openlp_core/lib/test_htmlbuilder.py b/tests/functional/openlp_core/lib/test_htmlbuilder.py index 11bfde2b6..a1c749d50 100644 --- a/tests/functional/openlp_core/lib/test_htmlbuilder.py +++ b/tests/functional/openlp_core/lib/test_htmlbuilder.py @@ -12,7 +12,7 @@ from openlp.core.lib.htmlbuilder import build_html, build_background_css, build_ from openlp.core.lib.theme import HorizontalType, VerticalType from tests.helpers.testmixin import TestMixin -HTML = """ +HTML = r""" From 5bb449455d85adaa9b23942c8050c7175391ece4 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Wed, 4 Jul 2018 22:50:31 +0200 Subject: [PATCH 11/18] Disable broken pylint check on specific line --- openlp/core/ui/slidecontroller.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/core/ui/slidecontroller.py b/openlp/core/ui/slidecontroller.py index 581e01253..fa204f8e8 100644 --- a/openlp/core/ui/slidecontroller.py +++ b/openlp/core/ui/slidecontroller.py @@ -1126,7 +1126,7 @@ class SlideController(DisplayController, LogMixin, RegistryProperties): # done by the thread holding the lock. If it is a "start" slide, we must wait for the lock, but only for 0.2 # seconds, since we don't want to cause a deadlock timeout = 0.2 if start else -1 - if not self.slide_selected_lock.acquire(start, timeout): + if not self.slide_selected_lock.acquire(start, timeout): # pylint: disable=too-many-function-args if start: self.log_debug('Could not get lock in slide_selected after waiting %f, skip to avoid deadlock.' % timeout) From 588cf96b5af06b3e461cb73030ea115b57809f1f Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Thu, 5 Jul 2018 21:18:16 +0200 Subject: [PATCH 12/18] Fix a test --- tests/functional/openlp_core/lib/test_htmlbuilder.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/openlp_core/lib/test_htmlbuilder.py b/tests/functional/openlp_core/lib/test_htmlbuilder.py index a1c749d50..02a390783 100644 --- a/tests/functional/openlp_core/lib/test_htmlbuilder.py +++ b/tests/functional/openlp_core/lib/test_htmlbuilder.py @@ -121,7 +121,7 @@ HTML = r""" } function show_text(new_text){ - var match = /-webkit-text-fill-color:[^;"]+/gi; + var match = /-webkit-text-fill-color:[^;\"]+/gi; if(timer != null) clearTimeout(timer); /* From ac14c0186d89193a64098a52b4f5d6fdd63d45ec Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Thu, 5 Jul 2018 22:33:07 +0200 Subject: [PATCH 13/18] fix pylint test --- tests/utils/test_pylint.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/utils/test_pylint.py b/tests/utils/test_pylint.py index 8ef9425c9..da83fc1e7 100644 --- a/tests/utils/test_pylint.py +++ b/tests/utils/test_pylint.py @@ -68,7 +68,7 @@ class TestPylint(TestCase): # WHEN: Running pylint (pylint_stdout, pylint_stderr) = \ - lint.py_run('openlp --errors-only --disable={disabled} --enable={enabled} ' + lint.py_run('openlp --errors-only -j 4 --disable={disabled} --enable={enabled} ' '--reports=no --output-format=parseable'.format(disabled=disabled_checks, enabled=enabled_checks), **pylint_kwargs) @@ -88,7 +88,7 @@ class TestPylint(TestCase): filtered_output = '' for line in pylint_output.splitlines(): # Filter out module info lines - if line.startswith('**'): + if '***' in line: continue # Filter out undefined-variable error releated to WindowsError elif 'undefined-variable' in line and 'WindowsError' in line: From 4dd16d1bd6e25aba6350c41baa0f3565258814f5 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Thu, 5 Jul 2018 22:43:55 +0200 Subject: [PATCH 14/18] pep8 fix --- openlp/core/ui/slidecontroller.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/core/ui/slidecontroller.py b/openlp/core/ui/slidecontroller.py index fa204f8e8..53588d574 100644 --- a/openlp/core/ui/slidecontroller.py +++ b/openlp/core/ui/slidecontroller.py @@ -1126,7 +1126,7 @@ class SlideController(DisplayController, LogMixin, RegistryProperties): # done by the thread holding the lock. If it is a "start" slide, we must wait for the lock, but only for 0.2 # seconds, since we don't want to cause a deadlock timeout = 0.2 if start else -1 - if not self.slide_selected_lock.acquire(start, timeout): # pylint: disable=too-many-function-args + if not self.slide_selected_lock.acquire(start, timeout): # pylint: disable=too-many-function-args if start: self.log_debug('Could not get lock in slide_selected after waiting %f, skip to avoid deadlock.' % timeout) From e3d163bf7a179953cfa4de9fca5f1db3573c3778 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Fri, 6 Jul 2018 22:23:33 +0200 Subject: [PATCH 15/18] Remove debug print --- tests/functional/openlp_core/common/test_init.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/functional/openlp_core/common/test_init.py b/tests/functional/openlp_core/common/test_init.py index 552bb9f29..f30215397 100644 --- a/tests/functional/openlp_core/common/test_init.py +++ b/tests/functional/openlp_core/common/test_init.py @@ -245,7 +245,6 @@ class TestInit(TestCase, TestMixin): result = clean_filename(invalid_name) # THEN: The file name should be cleaned. - print(result) assert wanted_name == result, 'The file name should not contain any special characters.' def test_delete_file_no_path(self): From 6adf749e9988ac3af3f2264371fda5cd25e23698 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Sat, 14 Jul 2018 21:56:11 +0200 Subject: [PATCH 16/18] Change appveyor integration to not rely on bzr. --- scripts/appveyor-webhook.py | 13 ++++++++----- scripts/appveyor.yml | 13 ++++++------- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/scripts/appveyor-webhook.py b/scripts/appveyor-webhook.py index f969b0e70..f5b148c55 100755 --- a/scripts/appveyor-webhook.py +++ b/scripts/appveyor-webhook.py @@ -92,21 +92,24 @@ def get_yml(branch, build_type): f = open('appveyor.yml') yml_text = f.read() f.close() - yml_text = yml_text.replace('BRANCHNAME', branch) + version_string, version = get_version() + yml_text = yml_text.replace('TAG', version_string) if build_type in ['openlp', 'trunk']: + yml_text = yml_text.replace('BRANCHPATH', '~openlp-core/openlp/trunk') yml_text = yml_text.replace('BUILD_DOCS', '$TRUE') else: + yml_text = yml_text.replace('BRANCHPATH', branch.split(':')[1]) yml_text = yml_text.replace('BUILD_DOCS', '$FALSE') - return yml_text + return yml_text, version_string -def hook(webhook_url, yml): +def hook(webhook_url, branch, build_type): """ Activate the webhook to start the build """ + yml, version_string = get_yml(branch, build_type) webhook_element['config'] = yml webhook_element['commit']['message'] = 'Building ' + branch - version_string, version = get_version() webhook_element['commit']['id'] = version_string request = urllib.request.Request(webhook_url) request.add_header('Content-Type', 'application/json;charset=utf-8') @@ -137,7 +140,7 @@ else: if build_type not in ['dev', 'trunk', 'openlp']: print('Invalid build type\nUsage: %s ' % sys.argv[0]) exit() - hook(webhook_url, get_yml(branch, build_type)) + hook(webhook_url, branch, build_type) # Wait 5 seconds to make sure the hook has been triggered time.sleep(5) get_appveyor_build_url(build_type) diff --git a/scripts/appveyor.yml b/scripts/appveyor.yml index 316e5f73b..a7704fb15 100644 --- a/scripts/appveyor.yml +++ b/scripts/appveyor.yml @@ -1,11 +1,10 @@ version: OpenLP-win-ci-b{build} -init: - - choco install -y --force bzr - - set PATH=C:\Program Files (x86)\Bazaar;%PATH% - clone_script: - - bzr checkout --lightweight BRANCHNAME openlp-branch + - curl -L https://bazaar.launchpad.net/~tomasgroth/openlp/pco-24/tarball -o pco.tar.gz + - 7z e pco.tar.gz + - 7z x pco.tar + - mv ~tomasgroth/openlp/pco-24 openlp-branch environment: PYTHON: C:\\Python34 @@ -74,10 +73,10 @@ after_test: 7z x documentation.tar mv ~openlp-core/openlp/documentation documentation cd packaging - &"$env:PYTHON\python.exe" builders/windows-builder.py --skip-update -c windows/config-appveyor.ini -b ../openlp-branch -d ../documentation --portable + &"$env:PYTHON\python.exe" builders/windows-builder.py --skip-update -c windows/config-appveyor.ini -b ../openlp-branch -d ../documentation --portable --tag-override TAG } else { cd packaging - &"$env:PYTHON\python.exe" builders/windows-builder.py --skip-update --skip-translations -c windows/config-appveyor.ini -b ../openlp-branch --portable + &"$env:PYTHON\python.exe" builders/windows-builder.py --skip-update --skip-translations -c windows/config-appveyor.ini -b ../openlp-branch --portable --tag-override TAG } artifacts: From 378ad41747baf47c2fb5d7bf0649fa19eb12a1bb Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Mon, 16 Jul 2018 22:01:47 +0200 Subject: [PATCH 17/18] More improvements to appveyor integration. --- scripts/appveyor-webhook.py | 4 ++-- scripts/appveyor.yml | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/scripts/appveyor-webhook.py b/scripts/appveyor-webhook.py index f5b148c55..ce02b7aee 100755 --- a/scripts/appveyor-webhook.py +++ b/scripts/appveyor-webhook.py @@ -81,7 +81,7 @@ def get_version(): latest = output.decode('utf-8').split(':')[0] version_string = latest == revision and tag or 'r%s' % latest # Save decimal version in case we need to do a portable build. - version = latest == revision and tag or '%s.%s' % (tag, latest) + version = latest == revision and tag or '%s-bzr%s' % (tag, latest) return version_string, version @@ -93,7 +93,7 @@ def get_yml(branch, build_type): yml_text = f.read() f.close() version_string, version = get_version() - yml_text = yml_text.replace('TAG', version_string) + yml_text = yml_text.replace('TAG', version) if build_type in ['openlp', 'trunk']: yml_text = yml_text.replace('BRANCHPATH', '~openlp-core/openlp/trunk') yml_text = yml_text.replace('BUILD_DOCS', '$TRUE') diff --git a/scripts/appveyor.yml b/scripts/appveyor.yml index a7704fb15..168b99fb5 100644 --- a/scripts/appveyor.yml +++ b/scripts/appveyor.yml @@ -1,17 +1,17 @@ version: OpenLP-win-ci-b{build} clone_script: - - curl -L https://bazaar.launchpad.net/~tomasgroth/openlp/pco-24/tarball -o pco.tar.gz - - 7z e pco.tar.gz - - 7z x pco.tar - - mv ~tomasgroth/openlp/pco-24 openlp-branch + - curl -L https://bazaar.launchpad.net/BRANCHPATH/tarball -o sourcecode.tar.gz + - 7z e sourcecode.tar.gz + - 7z x sourcecode.tar + - mv BRANCHPATH openlp-branch environment: PYTHON: C:\\Python34 install: # Install dependencies from pypi - - "%PYTHON%\\python.exe -m pip install sqlalchemy alembic chardet beautifulsoup4 Mako nose mock pyodbc==4.0.8 psycopg2 pypiwin32 pyenchant websockets asyncio waitress six webob requests" + - "%PYTHON%\\python.exe -m pip install sqlalchemy alembic chardet beautifulsoup4 Mako nose mock pyodbc==4.0.8 psycopg2 pypiwin32==219 pyenchant websockets asyncio waitress six webob requests" # Install mysql dependency - "%PYTHON%\\python.exe -m pip install http://cdn.mysql.com/Downloads/Connector-Python/mysql-connector-python-2.0.4.zip#md5=3df394d89300db95163f17c843ef49df" # Download and install lxml and pyicu (originally from http://www.lfd.uci.edu/~gohlke/pythonlibs/) From edada54d7e7d6c885a7a61a60e8f489eb84ca45c Mon Sep 17 00:00:00 2001 From: Ken Roberts Date: Fri, 3 Aug 2018 15:32:32 -0700 Subject: [PATCH 18/18] Fix windows not using lo as network interface --- openlp/core/common/__init__.py | 15 +++- .../common/test_network_interfaces.py | 78 +++++++++++++++++++ 2 files changed, 91 insertions(+), 2 deletions(-) create mode 100644 tests/functional/openlp_core/common/test_network_interfaces.py diff --git a/openlp/core/common/__init__.py b/openlp/core/common/__init__.py index a6e7620b5..0148bce99 100644 --- a/openlp/core/common/__init__.py +++ b/openlp/core/common/__init__.py @@ -64,13 +64,17 @@ def get_local_ip4(): log.debug('Getting local IPv4 interface(es) information') my_ip4 = {} for iface in QNetworkInterface.allInterfaces(): + log.debug('Checking for isValid and flags == IsUP | IsRunning') if not iface.isValid() or not (iface.flags() & (QNetworkInterface.IsUp | QNetworkInterface.IsRunning)): continue + log.debug('Checking address(es) protocol') for address in iface.addressEntries(): ip = address.ip() # NOTE: Next line will skip if interface is localhost - keep for now until we decide about it later # if (ip.protocol() == QAbstractSocket.IPv4Protocol) and (ip != QHostAddress.LocalHost): + log.debug('Checking for protocol == IPv4Protocol') if ip.protocol() == QAbstractSocket.IPv4Protocol: + log.debug('Getting interface information') my_ip4[iface.name()] = {'ip': ip.toString(), 'broadcast': address.broadcast().toString(), 'netmask': address.netmask().toString(), @@ -79,14 +83,21 @@ def get_local_ip4(): ip.toIPv4Address()).toString() } log.debug('Adding {iface} to active list'.format(iface=iface.name())) + if 'localhost' in my_ip4: + log.debug('Renaming windows localhost to lo') + my_ip4['lo'] = my_ip4['localhost'] + my_ip4.pop('localhost') + if len(my_ip4) == 0: + log.warning('No active IPv4 network interfaces detected') if len(my_ip4) == 1: if 'lo' in my_ip4: # No active interfaces - so leave localhost in there log.warning('No active IPv4 interfaces found except localhost') else: # Since we have a valid IP4 interface, remove localhost - log.debug('Found at least one IPv4 interface, removing localhost') - my_ip4.pop('lo') + if 'lo' in my_ip4: + log.debug('Found at least one IPv4 interface, removing localhost') + my_ip4.pop('lo') return my_ip4 diff --git a/tests/functional/openlp_core/common/test_network_interfaces.py b/tests/functional/openlp_core/common/test_network_interfaces.py new file mode 100644 index 000000000..d1547bd0a --- /dev/null +++ b/tests/functional/openlp_core/common/test_network_interfaces.py @@ -0,0 +1,78 @@ +# -*- coding: utf-8 -*- +# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4 + +############################################################################### +# OpenLP - Open Source Lyrics Projection # +# --------------------------------------------------------------------------- # +# Copyright (c) 2008-2017 OpenLP Developers # +# --------------------------------------------------------------------------- # +# This program is free software; you can redistribute it and/or modify it # +# under the terms of the GNU General Public License as published by the Free # +# Software Foundation; version 2 of the License. # +# # +# This program is distributed in the hope that it will be useful, but WITHOUT # +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or # +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for # +# more details. # +# # +# You should have received a copy of the GNU General Public License along # +# with this program; if not, write to the Free Software Foundation, Inc., 59 # +# Temple Place, Suite 330, Boston, MA 02111-1307 USA # +############################################################################### +""" +Functional tests to test calls for network interfaces. +""" +from unittest import TestCase +from unittest.mock import MagicMock, call, patch + +import openlp.core.common +from openlp.core.common import get_local_ip4 + +from tests.helpers.testmixin import TestMixin + +lo_address_attrs = {'isValid.return_value': True, + 'flags.return_value': True, + 'InterfaceFlags.return_value': True, + 'name.return_value': 'lo', + 'broadcast.toString.return_value': '127.0.0.255', + 'netmask.toString.return_value': '255.0.0.0', + 'prefixLength.return_value': 8, + 'ip.protocol.return_value': True} + + +class TestInterfaces(TestCase, TestMixin): + """ + A test suite to test out functions/methods that use network interface(s). + """ + def setUp(self): + """ + Create an instance and a few example actions. + """ + self.build_settings() + + self.ip4_lo_address = MagicMock() + self.ip4_lo_address.configure_mock(**lo_address_attrs) + + def tearDown(self): + """ + Clean up + """ + self.destroy_settings() + + @patch.object(openlp.core.common, 'log') + def test_ip4_no_interfaces(self, mock_log): + """ + Test no interfaces available + """ + # GIVEN: Test environment + call_warning = [call('No active IPv4 network interfaces detected')] + + with patch('openlp.core.common.QNetworkInterface') as mock_newtork_interface: + mock_newtork_interface.allInterfaces.return_value = [] + + # WHEN: get_local_ip4 is called + ifaces = get_local_ip4() + + # THEN: There should not be any interfaces detected + assert not ifaces, 'There should have been no active interfaces' + mock_log.warning.assert_has_calls(call_warning)