From 04cb827911d945d401dedc5cc6aee2452c9c47cd Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Mon, 27 Jun 2016 23:19:53 +0200 Subject: [PATCH 01/21] Started work on using pylint for code analysing. --- tests/utils/test_pylint.py | 52 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 tests/utils/test_pylint.py diff --git a/tests/utils/test_pylint.py b/tests/utils/test_pylint.py new file mode 100644 index 000000000..cf09a387a --- /dev/null +++ b/tests/utils/test_pylint.py @@ -0,0 +1,52 @@ +# -*- coding: utf-8 -*- +# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4 + +############################################################################### +# OpenLP - Open Source Lyrics Projection # +# --------------------------------------------------------------------------- # +# Copyright (c) 2008-2016 OpenLP Developers # +# --------------------------------------------------------------------------- # +# This program is free software; you can redistribute it and/or modify it # +# under the terms of the GNU General Public License as published by the Free # +# Software Foundation; version 2 of the License. # +# # +# This program is distributed in the hope that it will be useful, but WITHOUT # +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or # +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for # +# more details. # +# # +# You should have received a copy of the GNU General Public License along # +# with this program; if not, write to the Free Software Foundation, Inc., 59 # +# Temple Place, Suite 330, Boston, MA 02111-1307 USA # +############################################################################### +""" +Package to test for proper bzr tags. +""" +import os +import logging +from unittest import TestCase + +from pylint import epylint as lint +import multiprocessing + +log = logging.getLogger(__name__) + + +class TestPylint(TestCase): + + def test_pylint(self): + """ + Test for pylint errors + """ + # GIVEN: The openlp base folder + path = os.path.normpath(os.path.join(os.path.dirname(__file__), '..', '..', 'openlp')) + + # WHEN: Running pylint + (pylint_stdout, pylint_stderr) = lint.py_run('{path} --disable=all --enable=missing-format-argument-key,unused-format-string-argument -r n -j {cpu_count}'.format(path='openlp', cpu_count=multiprocessing.cpu_count()), return_std=True) + stdout = pylint_stdout.read() + stderr = pylint_stderr.read() + log.debug(stdout) + log.debug(stderr) + + # THEN: The output should be empty + self.assertEqual(stdout, '', 'PyLint should find no errors') From 668f10a14b52131da74e9d198486438cf063778d Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Tue, 28 Jun 2016 22:44:50 +0200 Subject: [PATCH 02/21] more pylint --- tests/utils/test_pylint.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/tests/utils/test_pylint.py b/tests/utils/test_pylint.py index cf09a387a..6f98e9a33 100644 --- a/tests/utils/test_pylint.py +++ b/tests/utils/test_pylint.py @@ -27,9 +27,6 @@ import logging from unittest import TestCase from pylint import epylint as lint -import multiprocessing - -log = logging.getLogger(__name__) class TestPylint(TestCase): @@ -39,14 +36,18 @@ class TestPylint(TestCase): Test for pylint errors """ # GIVEN: The openlp base folder - path = os.path.normpath(os.path.join(os.path.dirname(__file__), '..', '..', 'openlp')) + enabled_checks = 'missing-format-argument-key,unused-format-string-argument' + disabled_checks = 'all' - # WHEN: Running pylint - (pylint_stdout, pylint_stderr) = lint.py_run('{path} --disable=all --enable=missing-format-argument-key,unused-format-string-argument -r n -j {cpu_count}'.format(path='openlp', cpu_count=multiprocessing.cpu_count()), return_std=True) + # WHEN: Running pylint + (pylint_stdout, pylint_stderr) = \ + lint.py_run('{path} --disable={disabled} --enable={enabled} --reports=no'.format(path='openlp', + disabled=disabled_checks, + enabled=enabled_checks), + return_std=True) stdout = pylint_stdout.read() stderr = pylint_stderr.read() - log.debug(stdout) - log.debug(stderr) + print(stdout) # THEN: The output should be empty - self.assertEqual(stdout, '', 'PyLint should find no errors') + self.assertTrue(stdout == '', 'PyLint should find no errors') From 1cf078be76b903a72e09f22b15f426ecafe02e98 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Fri, 1 Jul 2016 23:14:24 +0200 Subject: [PATCH 03/21] Add pylintrc file --- pylintrc | 379 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 379 insertions(+) create mode 100644 pylintrc diff --git a/pylintrc b/pylintrc new file mode 100644 index 000000000..367695e13 --- /dev/null +++ b/pylintrc @@ -0,0 +1,379 @@ +[MASTER] + +# Specify a configuration file. +#rcfile= + +# Python code to execute, usually for sys.path manipulation such as +# pygtk.require(). +#init-hook= + +# Add files or directories to the blacklist. They should be base names, not +# paths. +ignore=vlc.py + +# Pickle collected data for later comparisons. +persistent=yes + +# List of plugins (as comma separated values of python modules names) to load, +# usually to register additional checkers. +load-plugins= + +# Use multiple processes to speed up Pylint. +jobs=4 + +# Allow loading of arbitrary C extensions. Extensions are imported into the +# active Python interpreter and may run arbitrary code. +unsafe-load-any-extension=no + +# A comma-separated list of package or module names from where C extensions may +# be loaded. Extensions are loading into the active Python interpreter and may +# run arbitrary code +extension-pkg-whitelist= + +# Allow optimization of some AST trees. This will activate a peephole AST +# optimizer, which will apply various small optimizations. For instance, it can +# be used to obtain the result of joining multiple strings with the addition +# operator. Joining a lot of strings can lead to a maximum recursion error in +# Pylint and this flag can prevent that. It has one side effect, the resulting +# AST will be different than the one from reality. +optimize-ast=no + + +[MESSAGES CONTROL] + +# Only show warnings with the listed confidence levels. Leave empty to show +# all. Valid levels: HIGH, INFERENCE, INFERENCE_FAILURE, UNDEFINED +confidence= + +# Enable the message, report, category or checker with the given id(s). You can +# either give multiple identifier separated by comma (,) or put this option +# multiple time. See also the "--disable" option for examples. +#enable= + +# Disable the message, report, category or checker with the given id(s). You +# can either give multiple identifiers separated by comma (,) or put this +# option multiple times (only on the command line, not in the configuration +# file where it should appear only once).You can also use "--disable=all" to +# disable everything first and then reenable specific checks. For example, if +# you want to run only the similarities checker, you can use "--disable=all +# --enable=similarities". If you want to run only the classes checker, but have +# no Warning level messages displayed, use"--disable=all --enable=classes +# --disable=W" +disable=reload-builtin,too-many-locals,no-name-in-module,hex-method,bad-builtin,old-raise-syntax,bad-continuation,reduce-builtin,unicode-builtin,unused-wildcard-import,apply-builtin,no-member,filter-builtin-not-iterating,execfile-builtin,import-star-module-level,input-builtin,duplicate-code,old-division,print-statement,cmp-method,fixme,no-absolute-import,cmp-builtin,no-self-use,too-many-nested-blocks,standarderror-builtin,long-builtin,raising-string,delslice-method,metaclass-assignment,coerce-builtin,old-octal-literal,basestring-builtin,xrange-builtin,line-too-long,suppressed-message,unused-variable,round-builtin,raw_input-builtin,too-many-instance-attributes,unused-argument,next-method-called,oct-method,attribute-defined-outside-init,too-many-public-methods,too-many-statements,logging-format-interpolation,map-builtin-not-iterating,buffer-builtin,parameter-unpacking,range-builtin-not-iterating,intern-builtin,wrong-import-position,coerce-method,useless-suppression,using-cmp-argument,dict-view-method,backtick,old-ne-operator,missing-docstring,setslice-method,access-member-before-definition,long-suffix,too-few-public-methods,file-builtin,zip-builtin-not-iterating,invalid-name,getslice-method,unpacking-in-except,too-many-arguments,dict-iter-method,unichr-builtin,too-many-lines,too-many-branches,wildcard-import,indexing-exception,nonzero-method + + +[REPORTS] + +# Set the output format. Available formats are text, parseable, colorized, msvs +# (visual studio) and html. You can also give a reporter class, eg +# mypackage.mymodule.MyReporterClass. +output-format=text + +# Put messages in a separate file for each module / package specified on the +# command line instead of printing them on stdout. Reports (if any) will be +# written in a file name "pylint_global.[txt|html]". +files-output=no + +# Tells whether to display a full report or only the messages +reports=no + +# Python expression which should return a note less than 10 (10 is the highest +# note). You have access to the variables errors warning, statement which +# respectively contain the number of errors / warnings messages and the total +# number of statements analyzed. This is used by the global evaluation report +# (RP0004). +evaluation=10.0 - ((float(5 * error + warning + refactor + convention) / statement) * 10) + +# Template used to display messages. This is a python new-style format string +# used to format the message information. See doc for all details +#msg-template= + + +[SPELLING] + +# Spelling dictionary name. Available dictionaries: en_ZA (myspell), en_US +# (myspell), en_GB (myspell), en_AU (myspell), da_DK (myspell), en (aspell), +# en_CA (aspell). +spelling-dict= + +# List of comma separated words that should not be checked. +spelling-ignore-words= + +# A path to a file that contains private dictionary; one word per line. +spelling-private-dict-file= + +# Tells whether to store unknown words to indicated private dictionary in +# --spelling-private-dict-file option instead of raising a message. +spelling-store-unknown-words=no + + +[SIMILARITIES] + +# Minimum lines number of a similarity. +min-similarity-lines=4 + +# Ignore comments when computing similarities. +ignore-comments=yes + +# Ignore docstrings when computing similarities. +ignore-docstrings=yes + +# Ignore imports when computing similarities. +ignore-imports=no + + +[LOGGING] + +# Logging modules to check that the string format arguments are in logging +# function parameter format +logging-modules=logging + + +[BASIC] + +# List of builtins function names that should not be used, separated by a comma +bad-functions=map,filter + +# Good variable names which should always be accepted, separated by a comma +good-names=i,j,k,ex,Run,_ + +# Bad variable names which should always be refused, separated by a comma +bad-names=foo,bar,baz,toto,tutu,tata + +# Colon-delimited sets of names that determine each other's naming style when +# the name regexes allow several styles. +name-group= + +# Include a hint for the correct naming format with invalid-name +include-naming-hint=no + +# Regular expression matching correct class attribute names +class-attribute-rgx=([A-Za-z_][A-Za-z0-9_]{2,30}|(__.*__))$ + +# Naming hint for class attribute names +class-attribute-name-hint=([A-Za-z_][A-Za-z0-9_]{2,30}|(__.*__))$ + +# Regular expression matching correct attribute names +attr-rgx=[a-z_][a-z0-9_]{2,30}$ + +# Naming hint for attribute names +attr-name-hint=[a-z_][a-z0-9_]{2,30}$ + +# Regular expression matching correct variable names +variable-rgx=[a-z_][a-z0-9_]{2,30}$ + +# Naming hint for variable names +variable-name-hint=[a-z_][a-z0-9_]{2,30}$ + +# Regular expression matching correct argument names +argument-rgx=[a-z_][a-z0-9_]{2,30}$ + +# Naming hint for argument names +argument-name-hint=[a-z_][a-z0-9_]{2,30}$ + +# Regular expression matching correct function names +function-rgx=[a-z_][a-z0-9_]{2,30}$ + +# Naming hint for function names +function-name-hint=[a-z_][a-z0-9_]{2,30}$ + +# Regular expression matching correct method names +method-rgx=[a-z_][a-z0-9_]{2,30}$ + +# Naming hint for method names +method-name-hint=[a-z_][a-z0-9_]{2,30}$ + +# Regular expression matching correct class names +class-rgx=[A-Z_][a-zA-Z0-9]+$ + +# Naming hint for class names +class-name-hint=[A-Z_][a-zA-Z0-9]+$ + +# Regular expression matching correct module names +module-rgx=(([a-z_][a-z0-9_]*)|([A-Z][a-zA-Z0-9]+))$ + +# Naming hint for module names +module-name-hint=(([a-z_][a-z0-9_]*)|([A-Z][a-zA-Z0-9]+))$ + +# Regular expression matching correct inline iteration names +inlinevar-rgx=[A-Za-z_][A-Za-z0-9_]*$ + +# Naming hint for inline iteration names +inlinevar-name-hint=[A-Za-z_][A-Za-z0-9_]*$ + +# Regular expression matching correct constant names +const-rgx=(([A-Z_][A-Z0-9_]*)|(__.*__))$ + +# Naming hint for constant names +const-name-hint=(([A-Z_][A-Z0-9_]*)|(__.*__))$ + +# Regular expression which should only match function or class names that do +# not require a docstring. +no-docstring-rgx=^_ + +# Minimum line length for functions/classes that require docstrings, shorter +# ones are exempt. +docstring-min-length=-1 + + +[ELIF] + +# Maximum number of nested blocks for function / method body +max-nested-blocks=5 + + +[FORMAT] + +# Maximum number of characters on a single line. +max-line-length=100 + +# Regexp for a line that is allowed to be longer than the limit. +ignore-long-lines=^\s*(# )??$ + +# Allow the body of an if to be on the same line as the test if there is no +# else. +single-line-if-stmt=no + +# List of optional constructs for which whitespace checking is disabled. `dict- +# separator` is used to allow tabulation in dicts, etc.: {1 : 1,\n222: 2}. +# `trailing-comma` allows a space between comma and closing bracket: (a, ). +# `empty-line` allows space-only lines. +no-space-check=trailing-comma,dict-separator + +# Maximum number of lines in a module +max-module-lines=1000 + +# String used as indentation unit. This is usually " " (4 spaces) or "\t" (1 +# tab). +indent-string=' ' + +# Number of spaces of indent required inside a hanging or continued line. +indent-after-paren=4 + +# Expected format of line ending, e.g. empty (any line ending), LF or CRLF. +expected-line-ending-format= + + +[MISCELLANEOUS] + +# List of note tags to take in consideration, separated by a comma. +notes=FIXME,XXX,TODO + + +[VARIABLES] + +# Tells whether we should check for unused import in __init__ files. +init-import=no + +# A regular expression matching the name of dummy variables (i.e. expectedly +# not used). +dummy-variables-rgx=_$|dummy + +# List of additional names supposed to be defined in builtins. Remember that +# you should avoid to define new builtins when possible. +additional-builtins= + +# List of strings which can identify a callback function by name. A callback +# name must start or end with one of those strings. +callbacks=cb_,_cb + + +[TYPECHECK] + +# Tells whether missing members accessed in mixin class should be ignored. A +# mixin class is detected if its name ends with "mixin" (case insensitive). +ignore-mixin-members=yes + +# List of module names for which member attributes should not be checked +# (useful for modules/projects where namespaces are manipulated during runtime +# and thus existing member attributes cannot be deduced by static analysis. It +# supports qualified module names, as well as Unix pattern matching. +ignored-modules= + +# List of classes names for which member attributes should not be checked +# (useful for classes with attributes dynamically set). This supports can work +# with qualified names. +ignored-classes= + +# List of members which are set dynamically and missed by pylint inference +# system, and so shouldn't trigger E1101 when accessed. Python regular +# expressions are accepted. +generated-members= + + +[DESIGN] + +# Maximum number of arguments for function / method +max-args=5 + +# Argument names that match this expression will be ignored. Default to name +# with leading underscore +ignored-argument-names=_.* + +# Maximum number of locals for function / method body +max-locals=15 + +# Maximum number of return / yield for function / method body +max-returns=6 + +# Maximum number of branch for function / method body +max-branches=12 + +# Maximum number of statements in function / method body +max-statements=50 + +# Maximum number of parents for a class (see R0901). +max-parents=7 + +# Maximum number of attributes for a class (see R0902). +max-attributes=7 + +# Minimum number of public methods for a class (see R0903). +min-public-methods=2 + +# Maximum number of public methods for a class (see R0904). +max-public-methods=20 + +# Maximum number of boolean expressions in a if statement +max-bool-expr=5 + + +[IMPORTS] + +# Deprecated modules which should not be used, separated by a comma +deprecated-modules=optparse + +# Create a graph of every (i.e. internal and external) dependencies in the +# given file (report RP0402 must not be disabled) +import-graph= + +# Create a graph of external dependencies in the given file (report RP0402 must +# not be disabled) +ext-import-graph= + +# Create a graph of internal dependencies in the given file (report RP0402 must +# not be disabled) +int-import-graph= + + +[CLASSES] + +# List of method names used to declare (i.e. assign) instance attributes. +defining-attr-methods=__init__,__new__,setUp + +# List of valid names for the first argument in a class method. +valid-classmethod-first-arg=cls + +# List of valid names for the first argument in a metaclass class method. +valid-metaclass-classmethod-first-arg=mcs + +# List of member names, which should be excluded from the protected access +# warning. +exclude-protected=_asdict,_fields,_replace,_source,_make + + +[EXCEPTIONS] + +# Exceptions that will emit a warning when being caught. Defaults to +# "Exception" +overgeneral-exceptions=Exception From 8aa917c89c1a5e406682f3cd2eeb9717f71f4dc4 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Fri, 1 Jul 2016 23:17:20 +0200 Subject: [PATCH 04/21] A bunch of fixes suggested by pylint. --- openlp/core/common/actions.py | 2 +- openlp/core/common/db.py | 5 ++-- openlp/core/common/languagemanager.py | 2 +- openlp/core/common/settings.py | 2 +- openlp/core/common/uistrings.py | 2 +- openlp/core/common/versionchecker.py | 4 +-- openlp/core/lib/__init__.py | 2 +- openlp/core/lib/db.py | 4 +-- openlp/core/lib/htmlbuilder.py | 4 +-- openlp/core/lib/imagemanager.py | 2 +- openlp/core/lib/pluginmanager.py | 1 - openlp/core/lib/projector/db.py | 25 ++++++++--------- openlp/core/lib/projector/pjlink1.py | 28 +++++++++---------- openlp/core/lib/renderer.py | 2 +- openlp/core/lib/serviceitem.py | 2 +- openlp/core/lib/theme.py | 7 ++--- openlp/core/lib/webpagereader.py | 3 +- openlp/core/ui/exceptionform.py | 3 +- openlp/core/ui/firsttimeform.py | 4 +-- openlp/core/ui/formattingtagcontroller.py | 2 +- openlp/core/ui/lib/spelltextedit.py | 2 +- openlp/core/ui/maindisplay.py | 8 +++--- openlp/core/ui/mainwindow.py | 1 - openlp/core/ui/media/__init__.py | 4 +-- openlp/core/ui/media/mediacontroller.py | 5 ++-- openlp/core/ui/media/playertab.py | 4 ++- openlp/core/ui/media/vlcplayer.py | 3 +- openlp/core/ui/media/webkitplayer.py | 4 +-- openlp/core/ui/projector/sourceselectform.py | 14 +++++----- openlp/core/ui/projector/tab.py | 2 +- openlp/core/ui/servicemanager.py | 4 +-- openlp/core/ui/slidecontroller.py | 1 - openlp/core/ui/themeform.py | 4 +-- openlp/core/ui/thememanager.py | 2 +- .../presentations/lib/messagelistener.py | 2 +- .../presentations/lib/pdfcontroller.py | 6 ++-- .../presentations/lib/pptviewcontroller.py | 4 +-- .../presentations/presentationplugin.py | 2 +- .../openlp_core_lib/test_serviceitem.py | 2 -- tests/utils/test_pylint.py | 9 ++++-- 40 files changed, 92 insertions(+), 97 deletions(-) diff --git a/openlp/core/common/actions.py b/openlp/core/common/actions.py index d22ef8fd1..5e5dd2e05 100644 --- a/openlp/core/common/actions.py +++ b/openlp/core/common/actions.py @@ -138,7 +138,7 @@ class CategoryList(object): for category in self.categories: if category.name == key: return category - raise KeyError('Category "{keY}" does not exist.'.format(key=key)) + raise KeyError('Category "{key}" does not exist.'.format(key=key)) def __len__(self): """ diff --git a/openlp/core/common/db.py b/openlp/core/common/db.py index 1e18167ab..1fd7a6521 100644 --- a/openlp/core/common/db.py +++ b/openlp/core/common/db.py @@ -22,11 +22,12 @@ """ The :mod:`db` module provides helper functions for database related methods. """ -import sqlalchemy import logging - from copy import deepcopy +import sqlalchemy + + log = logging.getLogger(__name__) diff --git a/openlp/core/common/languagemanager.py b/openlp/core/common/languagemanager.py index 58262ffb5..099e84af6 100644 --- a/openlp/core/common/languagemanager.py +++ b/openlp/core/common/languagemanager.py @@ -168,7 +168,7 @@ def format_time(text, local_time): """ return local_time.strftime(match.group()) - return re.sub('\%[a-zA-Z]', match_formatting, text) + return re.sub(r'\%[a-zA-Z]', match_formatting, text) def get_locale_key(string): diff --git a/openlp/core/common/settings.py b/openlp/core/common/settings.py index d4e114c74..a812e856b 100644 --- a/openlp/core/common/settings.py +++ b/openlp/core/common/settings.py @@ -26,7 +26,7 @@ import datetime import logging import os -from PyQt5 import QtCore, QtGui, QtWidgets +from PyQt5 import QtCore, QtGui from openlp.core.common import ThemeLevel, SlideLimits, UiStrings, is_win, is_linux diff --git a/openlp/core/common/uistrings.py b/openlp/core/common/uistrings.py index 91db10fcf..dccc3bdb4 100644 --- a/openlp/core/common/uistrings.py +++ b/openlp/core/common/uistrings.py @@ -68,7 +68,7 @@ class UiStrings(object): self.Default = translate('OpenLP.Ui', 'Default') self.DefaultColor = translate('OpenLP.Ui', 'Default Color:') self.DefaultServiceName = translate('OpenLP.Ui', 'Service %Y-%m-%d %H-%M', - 'This may not contain any of the following characters: /\\?*|<>\[\]":+\n' + 'This may not contain any of the following characters: /\\?*|<>[]":+\n' 'See http://docs.python.org/library/datetime' '.html#strftime-strptime-behavior for more information.') self.Delete = translate('OpenLP.Ui', '&Delete') diff --git a/openlp/core/common/versionchecker.py b/openlp/core/common/versionchecker.py index fb706968b..25479884f 100644 --- a/openlp/core/common/versionchecker.py +++ b/openlp/core/common/versionchecker.py @@ -10,10 +10,10 @@ from datetime import datetime from distutils.version import LooseVersion from subprocess import Popen, PIPE -from openlp.core.common import AppLocation, Settings - from PyQt5 import QtCore +from openlp.core.common import AppLocation, Settings + log = logging.getLogger(__name__) APPLICATION_VERSION = {} diff --git a/openlp/core/lib/__init__.py b/openlp/core/lib/__init__.py index a7e01bd24..fed6df05c 100644 --- a/openlp/core/lib/__init__.py +++ b/openlp/core/lib/__init__.py @@ -95,7 +95,7 @@ def get_text_file_string(text_file): content = None try: file_handle = open(text_file, 'r', encoding='utf-8') - if not file_handle.read(3) == '\xEF\xBB\xBF': + if file_handle.read(3) != '\xEF\xBB\xBF': # no BOM was found file_handle.seek(0) content = file_handle.read() diff --git a/openlp/core/lib/db.py b/openlp/core/lib/db.py index 3decb0a3b..d77432181 100644 --- a/openlp/core/lib/db.py +++ b/openlp/core/lib/db.py @@ -178,9 +178,9 @@ def upgrade_db(url, upgrade): version_meta = Metadata.populate(key='version', value=int(upgrade.__version__)) session.commit() upgrade_version = upgrade.__version__ - version_meta = int(version_meta.value) + version = int(version_meta.value) session.close() - return version_meta, upgrade_version + return version, upgrade_version def delete_database(plugin_name, db_file_name=None): diff --git a/openlp/core/lib/htmlbuilder.py b/openlp/core/lib/htmlbuilder.py index 6f2fee68c..456925d5a 100644 --- a/openlp/core/lib/htmlbuilder.py +++ b/openlp/core/lib/htmlbuilder.py @@ -389,8 +389,8 @@ is the function which has to be called from outside. The generated and returned """ import logging -from PyQt5 import QtWebKit from string import Template +from PyQt5 import QtWebKit from openlp.core.common import Settings from openlp.core.lib.theme import BackgroundType, BackgroundGradientType, VerticalType, HorizontalType @@ -647,7 +647,7 @@ def webkit_version(): webkit_ver = float(QtWebKit.qWebKitVersion()) log.debug('Webkit version = {version}'.format(version=webkit_ver)) except AttributeError: - webkit_ver = 0 + webkit_ver = 0.0 return webkit_ver diff --git a/openlp/core/lib/imagemanager.py b/openlp/core/lib/imagemanager.py index 1c25fca25..7e0cd3212 100644 --- a/openlp/core/lib/imagemanager.py +++ b/openlp/core/lib/imagemanager.py @@ -272,7 +272,7 @@ class ImageManager(QtCore.QObject): Add image to cache if it is not already there. """ log.debug('add_image {path}'.format(path=path)) - if not (path, source, width, height) in self._cache: + if (path, source, width, height) not in self._cache: image = Image(path, source, background, width, height) self._cache[(path, source, width, height)] = image self._conversion_queue.put((image.priority, image.secondary_priority, image)) diff --git a/openlp/core/lib/pluginmanager.py b/openlp/core/lib/pluginmanager.py index 4eb4c2f01..98f49a2c6 100644 --- a/openlp/core/lib/pluginmanager.py +++ b/openlp/core/lib/pluginmanager.py @@ -23,7 +23,6 @@ Provide plugin management """ import os -import sys import imp from openlp.core.lib import Plugin, PluginStatus diff --git a/openlp/core/lib/projector/db.py b/openlp/core/lib/projector/db.py index 98778e695..9d223b0e1 100644 --- a/openlp/core/lib/projector/db.py +++ b/openlp/core/lib/projector/db.py @@ -40,13 +40,12 @@ log.debug('projector.lib.db module loaded') from sqlalchemy import Column, ForeignKey, Integer, MetaData, String, and_ from sqlalchemy.ext.declarative import declarative_base, declared_attr -from sqlalchemy.orm import backref, relationship +from sqlalchemy.orm import relationship from openlp.core.lib.db import Manager, init_db, init_url from openlp.core.lib.projector.constants import PJLINK_DEFAULT_CODES -metadata = MetaData() -Base = declarative_base(metadata) +Base = declarative_base(MetaData()) class CommonBase(object): @@ -54,8 +53,8 @@ class CommonBase(object): Base class to automate table name and ID column. """ @declared_attr - def __tablename__(cls): - return cls.__name__.lower() + def __tablename__(self): + return self.__name__.lower() id = Column(Integer, primary_key=True) @@ -257,7 +256,7 @@ class ProjectorDB(Manager): projector = self.get_object_filtered(Projector, Projector.id == dbid) if projector is None: # Not found - log.warn('get_projector_by_id() did not find {data}'.format(data=id)) + log.warning('get_projector_by_id() did not find {data}'.format(data=id)) return None log.debug('get_projectorby_id() returning 1 entry for "{entry}" id="{data}"'.format(entry=dbid, data=projector.id)) @@ -290,7 +289,7 @@ class ProjectorDB(Manager): projector = self.get_object_filtered(Projector, Projector.ip == ip) if projector is None: # Not found - log.warn('get_projector_by_ip() did not find {ip}'.format(ip=ip)) + log.warning('get_projector_by_ip() did not find {ip}'.format(ip=ip)) return None log.debug('get_projectorby_ip() returning 1 entry for "{ip}" id="{data}"'.format(ip=ip, data=projector.id)) @@ -307,7 +306,7 @@ class ProjectorDB(Manager): projector = self.get_object_filtered(Projector, Projector.name == name) if projector is None: # Not found - log.warn('get_projector_by_name() did not find "{name}"'.format(name=name)) + log.warning('get_projector_by_name() did not find "{name}"'.format(name=name)) return None log.debug('get_projector_by_name() returning one entry for "{name}" id="{data}"'.format(name=name, data=projector.id)) @@ -324,7 +323,7 @@ class ProjectorDB(Manager): """ old_projector = self.get_object_filtered(Projector, Projector.ip == projector.ip) if old_projector is not None: - log.warn('add_new() skipping entry ip="{ip}" (Already saved)'.format(ip=old_projector.ip)) + log.warning('add_new() skipping entry ip="{ip}" (Already saved)'.format(ip=old_projector.ip)) return False log.debug('add_new() saving new entry') log.debug('ip="{ip}", name="{name}", location="{location}"'.format(ip=projector.ip, @@ -408,10 +407,10 @@ class ProjectorDB(Manager): :param source: ProjectorSource id :returns: ProjetorSource instance or None """ - source_entry = self.get_object_filtered(ProjetorSource, ProjectorSource.id == source) + source_entry = self.get_object_filtered(ProjectorSource, ProjectorSource.id == source) if source_entry is None: # Not found - log.warn('get_source_by_id() did not find "{source}"'.format(source=source)) + log.warning('get_source_by_id() did not find "{source}"'.format(source=source)) return None log.debug('get_source_by_id() returning one entry for "{source}""'.format(source=source)) return source_entry @@ -430,8 +429,8 @@ class ProjectorDB(Manager): if source_entry is None: # Not found - log.warn('get_source_by_id() not found') - log.warn('code="{code}" projector_id="{data}"'.format(code=code, data=projector_id)) + log.warning('get_source_by_id() not found') + log.warning('code="{code}" projector_id="{data}"'.format(code=code, data=projector_id)) return None log.debug('get_source_by_id() returning one entry') log.debug('code="{code}" projector_id="{data}"'.format(code=code, data=projector_id)) diff --git a/openlp/core/lib/projector/pjlink1.py b/openlp/core/lib/projector/pjlink1.py index ce06b3625..834b589d8 100644 --- a/openlp/core/lib/projector/pjlink1.py +++ b/openlp/core/lib/projector/pjlink1.py @@ -310,10 +310,10 @@ class PJLink1(QTcpSocket): read = self.readLine(self.maxSize) dontcare = self.readLine(self.maxSize) # Clean out the trailing \r\n if read is None: - log.warn('({ip}) read is None - socket error?'.format(ip=self.ip)) + log.warning('({ip}) read is None - socket error?'.format(ip=self.ip)) return elif len(read) < 8: - log.warn('({ip}) Not enough data read)'.format(ip=self.ip)) + log.warning('({ip}) Not enough data read)'.format(ip=self.ip)) return data = decode(read, 'ascii') # Possibility of extraneous data on input when reading. @@ -402,7 +402,7 @@ class PJLink1(QTcpSocket): self.projectorReceivedData.emit() return elif '=' not in data: - log.warn('({ip}) get_data(): Invalid packet received'.format(ip=self.ip)) + log.warning('({ip}) get_data(): Invalid packet received'.format(ip=self.ip)) self.send_busy = False self.projectorReceivedData.emit() return @@ -410,15 +410,15 @@ class PJLink1(QTcpSocket): try: (prefix, class_, cmd, data) = (data_split[0][0], data_split[0][1], data_split[0][2:], data_split[1]) except ValueError as e: - log.warn('({ip}) get_data(): Invalid packet - expected header + command + data'.format(ip=self.ip)) - log.warn('({ip}) get_data(): Received data: "{data}"'.format(ip=self.ip, data=data_in.strip())) + log.warning('({ip}) get_data(): Invalid packet - expected header + command + data'.format(ip=self.ip)) + log.warning('({ip}) get_data(): Received data: "{data}"'.format(ip=self.ip, data=data_in.strip())) self.change_status(E_INVALID_DATA) self.send_busy = False self.projectorReceivedData.emit() return if not (self.pjlink_class in PJLINK_VALID_CMD and cmd in PJLINK_VALID_CMD[self.pjlink_class]): - log.warn('({ip}) get_data(): Invalid packet - unknown command "{data}"'.format(ip=self.ip, data=cmd)) + log.warning('({ip}) get_data(): Invalid packet - unknown command "{data}"'.format(ip=self.ip, data=cmd)) self.send_busy = False self.projectorReceivedData.emit() return @@ -461,7 +461,7 @@ class PJLink1(QTcpSocket): :param queue: Option to force add to queue rather than sending directly """ if self.state() != self.ConnectedState: - log.warn('({ip}) send_command(): Not connected - returning'.format(ip=self.ip)) + log.warning('({ip}) send_command(): Not connected - returning'.format(ip=self.ip)) self.send_queue = [] return self.projectorNetwork.emit(S_NETWORK_SENDING) @@ -577,7 +577,7 @@ class PJLink1(QTcpSocket): if cmd in self.PJLINK1_FUNC: self.PJLINK1_FUNC[cmd](data) else: - log.warn('({ip}) Invalid command {data}'.format(ip=self.ip, data=cmd)) + log.warning('({ip}) Invalid command {data}'.format(ip=self.ip, data=cmd)) self.send_busy = False self.projectorReceivedData.emit() @@ -596,7 +596,7 @@ class PJLink1(QTcpSocket): fill = {'Hours': int(data_dict[0]), 'On': False if data_dict[1] == '0' else True} except ValueError: # In case of invalid entry - log.warn('({ip}) process_lamp(): Invalid data "{data}"'.format(ip=self.ip, data=data)) + log.warning('({ip}) process_lamp(): Invalid data "{data}"'.format(ip=self.ip, data=data)) return lamps.append(fill) data_dict.pop(0) # Remove lamp hours @@ -623,7 +623,7 @@ class PJLink1(QTcpSocket): self.send_command('INST') else: # Log unknown status response - log.warn('({ip}) Unknown power response: {data}'.format(ip=self.ip, data=data)) + log.warning('({ip}) Unknown power response: {data}'.format(ip=self.ip, data=data)) return def process_avmt(self, data): @@ -648,7 +648,7 @@ class PJLink1(QTcpSocket): shutter = True mute = True else: - log.warn('({ip}) Unknown shutter response: {data}'.format(ip=self.ip, data=data)) + log.warning('({ip}) Unknown shutter response: {data}'.format(ip=self.ip, data=data)) update_icons = shutter != self.shutter update_icons = update_icons or mute != self.mute self.shutter = shutter @@ -797,7 +797,7 @@ class PJLink1(QTcpSocket): Initiate connection to projector. """ if self.state() == self.ConnectedState: - log.warn('({ip}) connect_to_host(): Already connected - returning'.format(ip=self.ip)) + log.warning('({ip}) connect_to_host(): Already connected - returning'.format(ip=self.ip)) return self.change_status(S_CONNECTING) self.connectToHost(self.ip, self.port if type(self.port) is int else int(self.port)) @@ -809,9 +809,9 @@ class PJLink1(QTcpSocket): """ if abort or self.state() != self.ConnectedState: if abort: - log.warn('({ip}) disconnect_from_host(): Aborting connection'.format(ip=self.ip)) + log.warning('({ip}) disconnect_from_host(): Aborting connection'.format(ip=self.ip)) else: - log.warn('({ip}) disconnect_from_host(): Not connected - returning'.format(ip=self.ip)) + log.warning('({ip}) disconnect_from_host(): Not connected - returning'.format(ip=self.ip)) self.reset_information() self.disconnectFromHost() try: diff --git a/openlp/core/lib/renderer.py b/openlp/core/lib/renderer.py index 0d233a9c4..48d1cd05b 100644 --- a/openlp/core/lib/renderer.py +++ b/openlp/core/lib/renderer.py @@ -531,7 +531,7 @@ def words_split(line): :param line: Line to be split """ # this parse we are to be wordy - return re.split('\s+', line) + return re.split(r'\s+', line) def get_start_tags(raw_text): diff --git a/openlp/core/lib/serviceitem.py b/openlp/core/lib/serviceitem.py index c0a819390..1344bfeea 100644 --- a/openlp/core/lib/serviceitem.py +++ b/openlp/core/lib/serviceitem.py @@ -34,7 +34,7 @@ import ntpath from PyQt5 import QtGui from openlp.core.common import RegistryProperties, Settings, translate, AppLocation, md5_hash -from openlp.core.lib import ImageSource, build_icon, clean_tags, expand_tags, create_thumb +from openlp.core.lib import ImageSource, build_icon, clean_tags, expand_tags log = logging.getLogger(__name__) diff --git a/openlp/core/lib/theme.py b/openlp/core/lib/theme.py index 4e84d353b..e24613aa6 100644 --- a/openlp/core/lib/theme.py +++ b/openlp/core/lib/theme.py @@ -23,7 +23,6 @@ Provide the theme XML and handling functions for OpenLP v2 themes. """ import os -import re import logging import json @@ -477,12 +476,12 @@ class ThemeXML(object): if element == 'weight': element = 'bold' if value == 'Normal': - value = False + ret_value = False else: - value = True + ret_value = True if element == 'proportion': element = 'size' - return False, master, element, value + return False, master, element, ret_value def _create_attr(self, master, element, value): """ diff --git a/openlp/core/lib/webpagereader.py b/openlp/core/lib/webpagereader.py index 260ef1556..52c98bbaf 100644 --- a/openlp/core/lib/webpagereader.py +++ b/openlp/core/lib/webpagereader.py @@ -179,5 +179,4 @@ def get_web_page(url, header=None, update_openlp=False): return page -__all__ = ['get_application_version', 'check_latest_version', - 'get_web_page'] +__all__ = ['get_web_page'] diff --git a/openlp/core/ui/exceptionform.py b/openlp/core/ui/exceptionform.py index 216780584..7e97cb796 100644 --- a/openlp/core/ui/exceptionform.py +++ b/openlp/core/ui/exceptionform.py @@ -32,8 +32,6 @@ import sqlalchemy from PyQt5 import Qt, QtCore, QtGui, QtWebKit, QtWidgets from lxml import etree -from openlp.core.common import RegistryProperties, is_linux - try: import migrate MIGRATE_VERSION = getattr(migrate, '__version__', '< 0.7') @@ -74,6 +72,7 @@ except ImportError: from openlp.core.common import Settings, UiStrings, translate from openlp.core.common.versionchecker import get_application_version +from openlp.core.common import RegistryProperties, is_linux from .exceptiondialog import Ui_ExceptionDialog diff --git a/openlp/core/ui/firsttimeform.py b/openlp/core/ui/firsttimeform.py index 9ae2e0898..a0eec54a4 100644 --- a/openlp/core/ui/firsttimeform.py +++ b/openlp/core/ui/firsttimeform.py @@ -666,14 +666,14 @@ class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties): if missed_files: file_list = '' for entry in missed_files: - file_list += '{text}
'.format(text=entry) + file_list += '{text}
'.format(text=entry) msg = QtWidgets.QMessageBox() msg.setIcon(QtWidgets.QMessageBox.Warning) msg.setWindowTitle(translate('OpenLP.FirstTimeWizard', 'Network Error')) msg.setText(translate('OpenLP.FirstTimeWizard', 'Unable to download some files')) msg.setInformativeText(translate('OpenLP.FirstTimeWizard', 'The following files were not able to be ' - 'downloaded:
{text}'.format(text=file_list))) + 'downloaded:
{text}'.format(text=file_list))) msg.setStandardButtons(msg.Ok) ans = msg.exec() return True diff --git a/openlp/core/ui/formattingtagcontroller.py b/openlp/core/ui/formattingtagcontroller.py index 161930cb6..5a0511842 100644 --- a/openlp/core/ui/formattingtagcontroller.py +++ b/openlp/core/ui/formattingtagcontroller.py @@ -84,7 +84,7 @@ class FormattingTagController(object): 'desc': desc, 'start tag': '{{{tag}}}'.format(tag=tag), 'start html': start_html, - 'end tag': '{/{tag}}}'.format(tag=tag), + 'end tag': '{{{tag}}}'.format(tag=tag), 'end html': end_html, 'protected': False, 'temporary': False diff --git a/openlp/core/ui/lib/spelltextedit.py b/openlp/core/ui/lib/spelltextedit.py index 8b6b552be..5fd983128 100644 --- a/openlp/core/ui/lib/spelltextedit.py +++ b/openlp/core/ui/lib/spelltextedit.py @@ -164,7 +164,7 @@ class Highlighter(QtGui.QSyntaxHighlighter): """ Provides a text highlighter for pointing out spelling errors in text. """ - WORDS = '(?iu)[\w\']+' + WORDS = r'(?iu)[\w\']+' def __init__(self, *args): """ diff --git a/openlp/core/ui/maindisplay.py b/openlp/core/ui/maindisplay.py index 00f4be7ca..ff5acb975 100644 --- a/openlp/core/ui/maindisplay.py +++ b/openlp/core/ui/maindisplay.py @@ -33,7 +33,7 @@ import html import logging import os -from PyQt5 import QtCore, QtWidgets, QtWebKit, QtWebKitWidgets, QtOpenGL, QtGui, QtMultimedia +from PyQt5 import QtCore, QtWidgets, QtWebKit, QtWebKitWidgets, QtGui, QtMultimedia from openlp.core.common import AppLocation, Registry, RegistryProperties, OpenLPMixin, Settings, translate,\ is_macosx, is_win @@ -468,9 +468,9 @@ class MainDisplay(OpenLPMixin, Display, RegistryProperties): self.service_item.theme_data.background_filename, ImageSource.Theme) if image_path: image_bytes = self.image_manager.get_image_bytes(image_path, ImageSource.ImagePlugin) - html = build_html(self.service_item, self.screen, self.is_live, background, image_bytes, - plugins=self.plugin_manager.plugins) - self.web_view.setHtml(html) + created_html = build_html(self.service_item, self.screen, self.is_live, background, image_bytes, + plugins=self.plugin_manager.plugins) + self.web_view.setHtml(created_html) if service_item.foot_text: self.footer(service_item.foot_text) # if was hidden keep it hidden diff --git a/openlp/core/ui/mainwindow.py b/openlp/core/ui/mainwindow.py index ccd12727c..63048eac5 100644 --- a/openlp/core/ui/mainwindow.py +++ b/openlp/core/ui/mainwindow.py @@ -46,7 +46,6 @@ from openlp.core.ui.firsttimeform import FirstTimeForm from openlp.core.ui.media import MediaController from openlp.core.ui.printserviceform import PrintServiceForm from openlp.core.ui.projector.manager import ProjectorManager -from openlp.core.ui.lib.toolbar import OpenLPToolbar from openlp.core.ui.lib.dockwidget import OpenLPDockWidget from openlp.core.ui.lib.mediadockmanager import MediaDockManager diff --git a/openlp/core/ui/media/__init__.py b/openlp/core/ui/media/__init__.py index 248aca6f2..b51391583 100644 --- a/openlp/core/ui/media/__init__.py +++ b/openlp/core/ui/media/__init__.py @@ -24,10 +24,10 @@ The :mod:`~openlp.core.ui.media` module contains classes and objects for media p """ import logging -from openlp.core.common import Settings - from PyQt5 import QtCore +from openlp.core.common import Settings + log = logging.getLogger(__name__ + '.__init__') diff --git a/openlp/core/ui/media/mediacontroller.py b/openlp/core/ui/media/mediacontroller.py index 021ea5281..d404ee02e 100644 --- a/openlp/core/ui/media/mediacontroller.py +++ b/openlp/core/ui/media/mediacontroller.py @@ -38,7 +38,6 @@ from openlp.core.ui.media.mediaplayer import MediaPlayer from openlp.core.ui.media import MediaState, MediaInfo, MediaType, get_media_players, set_media_players,\ parse_optical_path from openlp.core.ui.lib.toolbar import OpenLPToolbar -from openlp.core.ui.lib.dockwidget import OpenLPDockWidget log = logging.getLogger(__name__) @@ -175,7 +174,7 @@ class MediaController(RegistryMixin, OpenLPMixin, RegistryProperties): log.debug('_check_available_media_players') controller_dir = os.path.join(AppLocation.get_directory(AppLocation.AppDir), 'core', 'ui', 'media') for filename in os.listdir(controller_dir): - if filename.endswith('player.py') and not filename == 'mediaplayer.py': + if filename.endswith('player.py') and filename != 'mediaplayer.py': path = os.path.join(controller_dir, filename) if os.path.isfile(path): module_name = 'openlp.core.ui.media.' + os.path.splitext(filename)[0] @@ -554,7 +553,7 @@ class MediaController(RegistryMixin, OpenLPMixin, RegistryProperties): default_player = [used_players[0]] if service_item.processor and service_item.processor != UiStrings().Automatic: # check to see if the player is usable else use the default one. - if not service_item.processor.lower() in used_players: + if service_item.processor.lower() not in used_players: used_players = default_player else: used_players = [service_item.processor.lower()] diff --git a/openlp/core/ui/media/playertab.py b/openlp/core/ui/media/playertab.py index 1fca21450..c76fc3300 100644 --- a/openlp/core/ui/media/playertab.py +++ b/openlp/core/ui/media/playertab.py @@ -224,9 +224,11 @@ class PlayerTab(SettingsTab): self.settings_form.register_post_process('mediaitem_media_rebuild') self.settings_form.register_post_process('config_screen_changed') - def post_set_up(self): + def post_set_up(self, post_update=False): """ Late setup for players as the MediaController has to be initialised first. + + :param post_update: Indicates if called before or after updates. """ for key, player in self.media_players.items(): player = self.media_players[key] diff --git a/openlp/core/ui/media/vlcplayer.py b/openlp/core/ui/media/vlcplayer.py index 9c2110e22..48b0602fe 100644 --- a/openlp/core/ui/media/vlcplayer.py +++ b/openlp/core/ui/media/vlcplayer.py @@ -112,7 +112,6 @@ def get_vlc(): # This needs to happen on module load and not in get_vlc(), otherwise it can cause crashes on some DE on some setups # (reported on Gnome3, Unity, Cinnamon, all GTK+ based) when using native filedialogs... if is_linux() and 'nose' not in sys.argv[0] and get_vlc(): - import ctypes try: try: x11 = ctypes.cdll.LoadLibrary('libX11.so.6') @@ -233,7 +232,7 @@ class VlcPlayer(MediaPlayer): """ vlc = get_vlc() start = datetime.now() - while not media_state == display.vlc_media.get_state(): + while media_state != display.vlc_media.get_state(): if display.vlc_media.get_state() == vlc.State.Error: return False self.application.process_events() diff --git a/openlp/core/ui/media/webkitplayer.py b/openlp/core/ui/media/webkitplayer.py index 19221ace0..5cd982951 100644 --- a/openlp/core/ui/media/webkitplayer.py +++ b/openlp/core/ui/media/webkitplayer.py @@ -22,10 +22,10 @@ """ The :mod:`~openlp.core.ui.media.webkit` module contains our WebKit video player """ -from PyQt5 import QtGui, QtWebKitWidgets - import logging +from PyQt5 import QtGui, QtWebKitWidgets + from openlp.core.common import Settings from openlp.core.lib import translate from openlp.core.ui.media import MediaState diff --git a/openlp/core/ui/projector/sourceselectform.py b/openlp/core/ui/projector/sourceselectform.py index 7d73f6a5a..5ee1ac403 100644 --- a/openlp/core/ui/projector/sourceselectform.py +++ b/openlp/core/ui/projector/sourceselectform.py @@ -141,23 +141,23 @@ def Build_Tab(group, source_key, default, projector, projectordb, edit=False): return widget, button_count, buttonchecked -def set_button_tooltip(bar): +def set_button_tooltip(button_bar): """ Set the toolip for the standard buttons used - :param bar: QDialogButtonBar instance to update + :param button_bar: QDialogButtonBar instance to update """ - for button in bar.buttons(): - if bar.standardButton(button) == QDialogButtonBox.Cancel: + for button in button_bar.buttons(): + if button_bar.standardButton(button) == QDialogButtonBox.Cancel: button.setToolTip(translate('OpenLP.SourceSelectForm', 'Ignoring current changes and return to OpenLP')) - elif bar.standardButton(button) == QDialogButtonBox.Reset: + elif button_bar.standardButton(button) == QDialogButtonBox.Reset: button.setToolTip(translate('OpenLP.SourceSelectForm', 'Delete all user-defined text and revert to PJLink default text')) - elif bar.standardButton(button) == QDialogButtonBox.Discard: + elif button_bar.standardButton(button) == QDialogButtonBox.Discard: button.setToolTip(translate('OpenLP.SourceSelectForm', 'Discard changes and reset to previous user-defined text')) - elif bar.standardButton(button) == QDialogButtonBox.Ok: + elif button_bar.standardButton(button) == QDialogButtonBox.Ok: button.setToolTip(translate('OpenLP.SourceSelectForm', 'Save changes and return to OpenLP')) else: diff --git a/openlp/core/ui/projector/tab.py b/openlp/core/ui/projector/tab.py index 1b87209c0..ff4bdee17 100644 --- a/openlp/core/ui/projector/tab.py +++ b/openlp/core/ui/projector/tab.py @@ -133,7 +133,7 @@ class ProjectorTab(SettingsTab): settings.setValue('socket timeout', self.socket_timeout_spin_box.value()) settings.setValue('poll time', self.socket_poll_spin_box.value()) settings.setValue('source dialog type', self.dialog_type_combo_box.currentIndex()) - settings.endGroup + settings.endGroup() def on_dialog_type_combo_box_changed(self): self.dialog_type = self.dialog_type_combo_box.currentIndex() diff --git a/openlp/core/ui/servicemanager.py b/openlp/core/ui/servicemanager.py index 907cb49a6..aa31dfbf8 100644 --- a/openlp/core/ui/servicemanager.py +++ b/openlp/core/ui/servicemanager.py @@ -774,7 +774,7 @@ class ServiceManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ServiceMa else: critical_error_message_box(message=translate('OpenLP.ServiceManager', 'File is not a valid service.')) self.log_error('File contains no service data') - except (IOError, NameError, zipfile.BadZipfile): + except (IOError, NameError): self.log_exception('Problem loading service file {name}'.format(name=file_name)) critical_error_message_box(message=translate('OpenLP.ServiceManager', 'File could not be opened because it is corrupt.')) @@ -1327,7 +1327,7 @@ class ServiceManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ServiceMa """ The theme may have changed in the settings dialog so make sure the theme combo box is in the correct state. """ - visible = not self.renderer.theme_level == ThemeLevel.Global + visible = self.renderer.theme_level != ThemeLevel.Global self.toolbar.actions['theme_combo_box'].setVisible(visible) self.toolbar.actions['theme_label'].setVisible(visible) self.regenerate_service_items() diff --git a/openlp/core/ui/slidecontroller.py b/openlp/core/ui/slidecontroller.py index 9379bb96f..8b5ce5e34 100644 --- a/openlp/core/ui/slidecontroller.py +++ b/openlp/core/ui/slidecontroller.py @@ -37,7 +37,6 @@ from openlp.core.lib import ItemCapabilities, ServiceItem, ImageSource, ServiceI build_html from openlp.core.lib.ui import create_action from openlp.core.ui.lib.toolbar import OpenLPToolbar -from openlp.core.ui.lib.dockwidget import OpenLPDockWidget from openlp.core.ui.lib.listpreviewwidget import ListPreviewWidget from openlp.core.ui import HideMode, MainDisplay, Display, DisplayControllerType diff --git a/openlp/core/ui/themeform.py b/openlp/core/ui/themeform.py index 475bfc0b7..a0ab5f45b 100644 --- a/openlp/core/ui/themeform.py +++ b/openlp/core/ui/themeform.py @@ -249,7 +249,7 @@ class ThemeForm(QtWidgets.QWizard, Ui_ThemeWizard, RegistryProperties): NOTE the font_main_override is the inverse of the check box value """ if self.update_theme_allowed: - self.theme.font_main_override = not (value == QtCore.Qt.Checked) + self.theme.font_main_override = (value != QtCore.Qt.Checked) def on_footer_position_check_box_state_changed(self, value): """ @@ -257,7 +257,7 @@ class ThemeForm(QtWidgets.QWizard, Ui_ThemeWizard, RegistryProperties): NOTE the font_footer_override is the inverse of the check box value """ if self.update_theme_allowed: - self.theme.font_footer_override = not (value == QtCore.Qt.Checked) + self.theme.font_footer_override = (value != QtCore.Qt.Checked) def exec(self, edit=False): """ diff --git a/openlp/core/ui/thememanager.py b/openlp/core/ui/thememanager.py index 70ca9fd88..341a8061e 100644 --- a/openlp/core/ui/thememanager.py +++ b/openlp/core/ui/thememanager.py @@ -583,7 +583,7 @@ class ThemeManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ThemeManage out_file.write(theme_zip.read(name)) out_file.close() except (IOError, zipfile.BadZipfile): - self.log_exception('Importing theme from zip failed {name|'.format(name=file_name)) + self.log_exception('Importing theme from zip failed {name}'.format(name=file_name)) raise ValidationError except ValidationError: critical_error_message_box(translate('OpenLP.ThemeManager', 'Validation Error'), diff --git a/openlp/plugins/presentations/lib/messagelistener.py b/openlp/plugins/presentations/lib/messagelistener.py index 992ba9b5c..097b9dc38 100644 --- a/openlp/plugins/presentations/lib/messagelistener.py +++ b/openlp/plugins/presentations/lib/messagelistener.py @@ -28,7 +28,7 @@ from PyQt5 import QtCore from openlp.core.common import Registry from openlp.core.ui import HideMode -from openlp.core.lib import ServiceItemContext, ServiceItem +from openlp.core.lib import ServiceItemContext from openlp.plugins.presentations.lib.pdfcontroller import PDF_CONTROLLER_FILETYPES log = logging.getLogger(__name__) diff --git a/openlp/plugins/presentations/lib/pdfcontroller.py b/openlp/plugins/presentations/lib/pdfcontroller.py index dd67031bd..6045d5326 100644 --- a/openlp/plugins/presentations/lib/pdfcontroller.py +++ b/openlp/plugins/presentations/lib/pdfcontroller.py @@ -80,7 +80,7 @@ class PdfController(PresentationController): found_mutool = re.search('usage: mutool.*', decoded_line, re.IGNORECASE) if found_mutool: # Test that mutool contains mudraw - if re.search('draw\s+--\s+convert document.*', runlog.decode(), re.IGNORECASE | re.MULTILINE): + if re.search(r'draw\s+--\s+convert document.*', runlog.decode(), re.IGNORECASE | re.MULTILINE): program_type = 'mutool' break found_gs = re.search('GPL Ghostscript.*', decoded_line, re.IGNORECASE) @@ -215,8 +215,8 @@ class PdfDocument(PresentationDocument): height = 0.0 for line in runlog.splitlines(): try: - width = float(re.search('.*Size: x: (\d+\.?\d*), y: \d+.*', line.decode()).group(1)) - height = float(re.search('.*Size: x: \d+\.?\d*, y: (\d+\.?\d*).*', line.decode()).group(1)) + width = float(re.search(r'.*Size: x: (\d+\.?\d*), y: \d+.*', line.decode()).group(1)) + height = float(re.search(r'.*Size: x: \d+\.?\d*, y: (\d+\.?\d*).*', line.decode()).group(1)) break except AttributeError: continue diff --git a/openlp/plugins/presentations/lib/pptviewcontroller.py b/openlp/plugins/presentations/lib/pptviewcontroller.py index 54d8f5170..43eb69454 100644 --- a/openlp/plugins/presentations/lib/pptviewcontroller.py +++ b/openlp/plugins/presentations/lib/pptviewcontroller.py @@ -181,13 +181,13 @@ class PptviewDocument(PresentationDocument): index = -1 list_to_add = None # check if it is a slide - match = re.search("slides/slide(.+)\.xml", zip_info.filename) + match = re.search(r'slides/slide(.+)\.xml', zip_info.filename) if match: index = int(match.group(1)) - 1 node_type = 'ctrTitle' list_to_add = titles # or a note - match = re.search("notesSlides/notesSlide(.+)\.xml", zip_info.filename) + match = re.search(r'notesSlides/notesSlide(.+)\.xml', zip_info.filename) if match: index = int(match.group(1)) - 1 node_type = 'body' diff --git a/openlp/plugins/presentations/presentationplugin.py b/openlp/plugins/presentations/presentationplugin.py index dc0614086..ca0ecba82 100644 --- a/openlp/plugins/presentations/presentationplugin.py +++ b/openlp/plugins/presentations/presentationplugin.py @@ -124,7 +124,7 @@ class PresentationPlugin(Plugin): log.debug('check_pre_conditions') controller_dir = os.path.join(AppLocation.get_directory(AppLocation.PluginsDir), 'presentations', 'lib') for filename in os.listdir(controller_dir): - if filename.endswith('controller.py') and not filename == 'presentationcontroller.py': + if filename.endswith('controller.py') and filename != 'presentationcontroller.py': path = os.path.join(controller_dir, filename) if os.path.isfile(path): module_name = 'openlp.plugins.presentations.lib.' + os.path.splitext(filename)[0] diff --git a/tests/functional/openlp_core_lib/test_serviceitem.py b/tests/functional/openlp_core_lib/test_serviceitem.py index e1c73b3ca..d18a4d049 100644 --- a/tests/functional/openlp_core_lib/test_serviceitem.py +++ b/tests/functional/openlp_core_lib/test_serviceitem.py @@ -112,7 +112,6 @@ class TestServiceItem(TestCase): # WHEN: adding an image from a saved Service and mocked exists line = convert_file_service_item(TEST_PATH, 'serviceitem_image_1.osj') with patch('openlp.core.ui.servicemanager.os.path.exists') as mocked_exists,\ - patch('openlp.core.lib.serviceitem.create_thumb') as mocked_create_thumb,\ patch('openlp.core.lib.serviceitem.AppLocation.get_section_data_path') as \ mocked_get_section_data_path: mocked_exists.return_value = True @@ -164,7 +163,6 @@ class TestServiceItem(TestCase): line2 = convert_file_service_item(TEST_PATH, 'serviceitem_image_2.osj', 1) with patch('openlp.core.ui.servicemanager.os.path.exists') as mocked_exists, \ - patch('openlp.core.lib.serviceitem.create_thumb') as mocked_create_thumb, \ patch('openlp.core.lib.serviceitem.AppLocation.get_section_data_path') as \ mocked_get_section_data_path: mocked_exists.return_value = True diff --git a/tests/utils/test_pylint.py b/tests/utils/test_pylint.py index 6f98e9a33..6db43464e 100644 --- a/tests/utils/test_pylint.py +++ b/tests/utils/test_pylint.py @@ -27,6 +27,7 @@ import logging from unittest import TestCase from pylint import epylint as lint +from pylint.__pkginfo__ import pylint_version class TestPylint(TestCase): @@ -37,17 +38,19 @@ class TestPylint(TestCase): """ # GIVEN: The openlp base folder enabled_checks = 'missing-format-argument-key,unused-format-string-argument' - disabled_checks = 'all' + #disabled_checks = 'all' + disabled_checks = '' # WHEN: Running pylint (pylint_stdout, pylint_stderr) = \ lint.py_run('{path} --disable={disabled} --enable={enabled} --reports=no'.format(path='openlp', disabled=disabled_checks, enabled=enabled_checks), - return_std=True) + return_std=True, script='pylint3') stdout = pylint_stdout.read() stderr = pylint_stderr.read() print(stdout) + print(stderr) # THEN: The output should be empty - self.assertTrue(stdout == '', 'PyLint should find no errors') + self.assertTrue(stdout == 's', 'PyLint should find no errors') From 9294aadae9eb25a2a11396e4c6cb1836a8410e29 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Tue, 5 Jul 2016 22:30:33 +0200 Subject: [PATCH 05/21] Changes to pylint test --- tests/utils/test_pylint.py | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/tests/utils/test_pylint.py b/tests/utils/test_pylint.py index 6db43464e..d4e744e55 100644 --- a/tests/utils/test_pylint.py +++ b/tests/utils/test_pylint.py @@ -24,11 +24,14 @@ Package to test for proper bzr tags. """ import os import logging -from unittest import TestCase - -from pylint import epylint as lint -from pylint.__pkginfo__ import pylint_version +import platform +from unittest import TestCase, SkipTest +try: + from pylint import epylint as lint + from pylint.__pkginfo__ import version +except ImportError: + raise SkipTest('pylint not installed - skipping tests using pylint.') class TestPylint(TestCase): @@ -36,17 +39,20 @@ class TestPylint(TestCase): """ Test for pylint errors """ - # GIVEN: The openlp base folder + # GIVEN: Some checks to disable and enable, and the pylint script + disabled_checks = 'no-member,import-error,no-name-in-module' enabled_checks = 'missing-format-argument-key,unused-format-string-argument' - #disabled_checks = 'all' - disabled_checks = '' + if 'arch' in platform.dist()[0].lower(): + pylint_script = 'pylint' + else: + pylint_script = 'pylint3' # WHEN: Running pylint (pylint_stdout, pylint_stderr) = \ - lint.py_run('{path} --disable={disabled} --enable={enabled} --reports=no'.format(path='openlp', - disabled=disabled_checks, - enabled=enabled_checks), - return_std=True, script='pylint3') + lint.py_run('openlp --errors-only --disable={disabled} --enable={enabled} --reports=no --output-format=parseable'.format( + disabled=disabled_checks, + enabled=enabled_checks), + return_std=True, script=pylint_script) stdout = pylint_stdout.read() stderr = pylint_stderr.read() print(stdout) From 99e6ab6657eb1dec65d9a31aca04619214798d24 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Tue, 5 Jul 2016 22:31:29 +0200 Subject: [PATCH 06/21] Fix various issues as suggested by pylint --- openlp/core/lib/theme.py | 1 + openlp/plugins/bibles/lib/manager.py | 2 +- openlp/plugins/bibles/lib/opensong.py | 2 +- openlp/plugins/songs/lib/importers/cclifile.py | 1 + openlp/plugins/songs/lib/importers/dreambeam.py | 4 ++-- openlp/plugins/songs/lib/importers/easyworship.py | 3 +++ openlp/plugins/songs/lib/importers/openlyrics.py | 2 +- openlp/plugins/songs/lib/importers/presentationmanager.py | 1 + pylintrc | 2 +- 9 files changed, 12 insertions(+), 6 deletions(-) diff --git a/openlp/core/lib/theme.py b/openlp/core/lib/theme.py index e24613aa6..fc20037e7 100644 --- a/openlp/core/lib/theme.py +++ b/openlp/core/lib/theme.py @@ -164,6 +164,7 @@ class ThemeXML(object): jsn = get_text_file_string(json_file) jsn = json.loads(jsn) self.expand_json(jsn) + self.background_filename = None def expand_json(self, var, prev=None): """ diff --git a/openlp/plugins/bibles/lib/manager.py b/openlp/plugins/bibles/lib/manager.py index d1465475d..1c55222f2 100644 --- a/openlp/plugins/bibles/lib/manager.py +++ b/openlp/plugins/bibles/lib/manager.py @@ -370,7 +370,7 @@ class BibleManager(RegistryProperties): """ log.debug('save_meta data {bible}, {version}, {copyright}, {perms}'.format(bible=bible, version=version, - cr=copyright, + copyright=copyright, perms=permissions)) self.db_cache[bible].save_meta('name', version) self.db_cache[bible].save_meta('copyright', copyright) diff --git a/openlp/plugins/bibles/lib/opensong.py b/openlp/plugins/bibles/lib/opensong.py index af3ef64c0..9c01a1fe0 100644 --- a/openlp/plugins/bibles/lib/opensong.py +++ b/openlp/plugins/bibles/lib/opensong.py @@ -117,7 +117,7 @@ class OpenSongBible(BibleDB): if len(verse_parts) > 1: number = int(verse_parts[0]) except TypeError: - log.warning('Illegal verse number: {verse:d}'.format(verse.attrib['n'])) + log.warning('Illegal verse number: {verse:d}'.format(verse=verse.attrib['n'])) verse_number = number else: verse_number += 1 diff --git a/openlp/plugins/songs/lib/importers/cclifile.py b/openlp/plugins/songs/lib/importers/cclifile.py index 800509ab2..b17f78129 100644 --- a/openlp/plugins/songs/lib/importers/cclifile.py +++ b/openlp/plugins/songs/lib/importers/cclifile.py @@ -255,6 +255,7 @@ class CCLIFileImport(SongImport): song_author = '' verse_start = False for line in text_list: + verse_type= 'v' clean_line = line.strip() if not clean_line: if line_number == 0: diff --git a/openlp/plugins/songs/lib/importers/dreambeam.py b/openlp/plugins/songs/lib/importers/dreambeam.py index 9371adc56..8c435bfd5 100644 --- a/openlp/plugins/songs/lib/importers/dreambeam.py +++ b/openlp/plugins/songs/lib/importers/dreambeam.py @@ -124,8 +124,8 @@ class DreamBeamImport(SongImport): if hasattr(song_xml, 'Sequence'): for lyrics_sequence_item in (song_xml.Sequence.iterchildren()): item = lyrics_sequence_item.get('Type')[:1] - self.verse_order_list.append("{item}{number}".format(item=item), - lyrics_sequence_item.get('Number')) + number = lyrics_sequence_item.get('Number') + self.verse_order_list.append("{item}{number}".format(item=item, number=number)) if hasattr(song_xml, 'Notes'): self.comments = str(song_xml.Notes.text) else: diff --git a/openlp/plugins/songs/lib/importers/easyworship.py b/openlp/plugins/songs/lib/importers/easyworship.py index cdffe9292..9db30838f 100644 --- a/openlp/plugins/songs/lib/importers/easyworship.py +++ b/openlp/plugins/songs/lib/importers/easyworship.py @@ -27,6 +27,7 @@ import os import struct import re import zlib +import logging from openlp.core.lib import translate from openlp.plugins.songs.lib import VerseType @@ -38,6 +39,8 @@ SLIDE_BREAK_REGEX = re.compile(r'\n *?\n[\n ]*') NUMBER_REGEX = re.compile(r'[0-9]+') NOTE_REGEX = re.compile(r'\(.*?\)') +log = logging.getLogger(__name__) + class FieldDescEntry: def __init__(self, name, field_type, size): diff --git a/openlp/plugins/songs/lib/importers/openlyrics.py b/openlp/plugins/songs/lib/importers/openlyrics.py index 84b667ce4..fcbe8fbfb 100644 --- a/openlp/plugins/songs/lib/importers/openlyrics.py +++ b/openlp/plugins/songs/lib/importers/openlyrics.py @@ -67,7 +67,7 @@ class OpenLyricsImport(SongImport): xml = etree.tostring(parsed_file).decode() self.open_lyrics.xml_to_song(xml) except etree.XMLSyntaxError: - log.exception('XML syntax error in file {path}'.format(file_path)) + log.exception('XML syntax error in file {path}'.format(path=file_path)) self.log_error(file_path, SongStrings.XMLSyntaxError) except OpenLyricsError as exception: log.exception('OpenLyricsException {error:d} in file {name}: {text}'.format(error=exception.type, diff --git a/openlp/plugins/songs/lib/importers/presentationmanager.py b/openlp/plugins/songs/lib/importers/presentationmanager.py index 34a94dec2..f10bd0ed6 100644 --- a/openlp/plugins/songs/lib/importers/presentationmanager.py +++ b/openlp/plugins/songs/lib/importers/presentationmanager.py @@ -30,6 +30,7 @@ import re import chardet from lxml import objectify, etree +from openlp.core.common import translate from openlp.core.ui.lib.wizard import WizardStrings from .songimport import SongImport diff --git a/pylintrc b/pylintrc index 367695e13..92d83ffaa 100644 --- a/pylintrc +++ b/pylintrc @@ -19,7 +19,7 @@ persistent=yes load-plugins= # Use multiple processes to speed up Pylint. -jobs=4 +#jobs=4 # Allow loading of arbitrary C extensions. Extensions are imported into the # active Python interpreter and may run arbitrary code. From 1992a513394a8e661824fbdb8f0c821219535906 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Wed, 6 Jul 2016 21:48:57 +0200 Subject: [PATCH 07/21] More pylint-fixes --- openlp/core/lib/db.py | 30 +++++------ openlp/plugins/bibles/lib/osis.py | 2 - openlp/plugins/bibles/lib/zefania.py | 2 - .../plugins/songs/lib/importers/cclifile.py | 2 +- tests/utils/test_pylint.py | 53 ++++++++++++++++--- 5 files changed, 62 insertions(+), 27 deletions(-) diff --git a/openlp/core/lib/db.py b/openlp/core/lib/db.py index d77432181..f42c3b5fc 100644 --- a/openlp/core/lib/db.py +++ b/openlp/core/lib/db.py @@ -122,6 +122,21 @@ def get_upgrade_op(session): return Operations(context) +class BaseModel(object): + """ + BaseModel provides a base object with a set of generic functions + """ + @classmethod + def populate(cls, **kwargs): + """ + Creates an instance of a class and populates it, returning the instance + """ + instance = cls() + for key, value in kwargs.items(): + instance.__setattr__(key, value) + return instance + + def upgrade_db(url, upgrade): """ Upgrade a database. @@ -197,21 +212,6 @@ def delete_database(plugin_name, db_file_name=None): return delete_file(db_file_path) -class BaseModel(object): - """ - BaseModel provides a base object with a set of generic functions - """ - @classmethod - def populate(cls, **kwargs): - """ - Creates an instance of a class and populates it, returning the instance - """ - instance = cls() - for key, value in kwargs.items(): - instance.__setattr__(key, value) - return instance - - class Manager(object): """ Provide generic object persistence management diff --git a/openlp/plugins/bibles/lib/osis.py b/openlp/plugins/bibles/lib/osis.py index c0a3bec81..85aea70b3 100644 --- a/openlp/plugins/bibles/lib/osis.py +++ b/openlp/plugins/bibles/lib/osis.py @@ -126,8 +126,6 @@ class OSISBible(BibleDB): # Remove div-tags in the book etree.strip_tags(book, ('{http://www.bibletechnologies.net/2003/OSIS/namespace}div')) book_ref_id = self.get_book_ref_id_by_name(book.get('osisID'), num_books, language_id) - if not book_ref_id: - book_ref_id = self.get_book_ref_id_by_localised_name(book.get('osisID')) if not book_ref_id: log.error('Importing books from "{name}" failed'.format(name=self.filename)) return False diff --git a/openlp/plugins/bibles/lib/zefania.py b/openlp/plugins/bibles/lib/zefania.py index 0ea96a2a3..482d98107 100644 --- a/openlp/plugins/bibles/lib/zefania.py +++ b/openlp/plugins/bibles/lib/zefania.py @@ -86,8 +86,6 @@ class ZefaniaBible(BibleDB): continue if bname: book_ref_id = self.get_book_ref_id_by_name(bname, num_books, language_id) - if not book_ref_id: - book_ref_id = self.get_book_ref_id_by_localised_name(bname) else: log.debug('Could not find a name, will use number, basically a guess.') book_ref_id = int(bnumber) diff --git a/openlp/plugins/songs/lib/importers/cclifile.py b/openlp/plugins/songs/lib/importers/cclifile.py index b17f78129..58ceefebc 100644 --- a/openlp/plugins/songs/lib/importers/cclifile.py +++ b/openlp/plugins/songs/lib/importers/cclifile.py @@ -255,7 +255,7 @@ class CCLIFileImport(SongImport): song_author = '' verse_start = False for line in text_list: - verse_type= 'v' + verse_type = 'v' clean_line = line.strip() if not clean_line: if line_number == 0: diff --git a/tests/utils/test_pylint.py b/tests/utils/test_pylint.py index d4e744e55..dc6c83909 100644 --- a/tests/utils/test_pylint.py +++ b/tests/utils/test_pylint.py @@ -33,6 +33,13 @@ try: except ImportError: raise SkipTest('pylint not installed - skipping tests using pylint.') +from openlp.core.common import is_win + +TOLERATED_ERRORS = {'registryproperties.py': ['access-member-before-definition'], + 'opensong.py': ['no-name-in-module'], + 'maindisplay.py': ['no-name-in-module']} + + class TestPylint(TestCase): def test_pylint(self): @@ -40,23 +47,55 @@ class TestPylint(TestCase): Test for pylint errors """ # GIVEN: Some checks to disable and enable, and the pylint script - disabled_checks = 'no-member,import-error,no-name-in-module' + disabled_checks = 'import-error,no-member' enabled_checks = 'missing-format-argument-key,unused-format-string-argument' - if 'arch' in platform.dist()[0].lower(): + if is_win() or 'arch' in platform.dist()[0].lower(): pylint_script = 'pylint' else: pylint_script = 'pylint3' # WHEN: Running pylint (pylint_stdout, pylint_stderr) = \ - lint.py_run('openlp --errors-only --disable={disabled} --enable={enabled} --reports=no --output-format=parseable'.format( - disabled=disabled_checks, - enabled=enabled_checks), + lint.py_run('openlp --errors-only --disable={disabled} --enable={enabled} ' + '--reports=no --output-format=parseable'.format(disabled=disabled_checks, + enabled=enabled_checks), return_std=True, script=pylint_script) stdout = pylint_stdout.read() stderr = pylint_stderr.read() - print(stdout) + filtered_stdout = self._filter_tolerated_errors(stdout) + print(filtered_stdout) print(stderr) # THEN: The output should be empty - self.assertTrue(stdout == 's', 'PyLint should find no errors') + self.assertTrue(filtered_stdout == '', 'PyLint should find no errors') + + def _filter_tolerated_errors(self, pylint_output): + """ + Filter out errors we tolerate. + """ + filtered_output = '' + for line in pylint_output.splitlines(): + # Filter out module info lines + if line.startswith('**'): + continue + # Filter out undefined-variable error releated to WindowsError + elif 'undefined-variable' in line and 'WindowsError' in line: + continue + # Filter out PyQt related errors + elif ('no-name-in-module' in line or 'no-member' in line) and 'PyQt5' in line: + continue + elif self._is_line_tolerated(line): + continue + else: + filtered_output += line + '\n' + return filtered_output.strip() + + def _is_line_tolerated(self, line): + """ + Check if line constains a tolerated error + """ + for filename in TOLERATED_ERRORS: + for tolerated_error in TOLERATED_ERRORS[filename]: + if filename in line and tolerated_error in line: + return True + return False From 69951133fe8d8973110092b8b0f7d174c90e5be5 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Thu, 7 Jul 2016 22:47:57 +0200 Subject: [PATCH 08/21] Fix handeling of control chars and escaped chars in VideoPsalm import. Fixes bug 1594945. Fixes: https://launchpad.net/bugs/1594945 --- .../plugins/songs/lib/importers/videopsalm.py | 8 ++++ .../openlp_plugins/songs/test_videopsalm.py | 2 + .../videopsalm-as-safe-a-stronghold2.json | 47 +++++++++++++++++++ 3 files changed, 57 insertions(+) create mode 100644 tests/resources/videopsalmsongs/videopsalm-as-safe-a-stronghold2.json diff --git a/openlp/plugins/songs/lib/importers/videopsalm.py b/openlp/plugins/songs/lib/importers/videopsalm.py index 25fd4d8eb..b536dd678 100644 --- a/openlp/plugins/songs/lib/importers/videopsalm.py +++ b/openlp/plugins/songs/lib/importers/videopsalm.py @@ -73,6 +73,14 @@ class VideoPsalmImport(SongImport): processed_content += c c = next(file_content_it) processed_content += '"' + c + # Remove control characters + elif (c < chr(32)): + processed_content += ' ' + # Handle escaped characters + elif c == '\\': + processed_content += c + c = next(file_content_it) + processed_content += c else: processed_content += c songbook = json.loads(processed_content.strip()) diff --git a/tests/functional/openlp_plugins/songs/test_videopsalm.py b/tests/functional/openlp_plugins/songs/test_videopsalm.py index 1bf13241d..ff1a81db5 100644 --- a/tests/functional/openlp_plugins/songs/test_videopsalm.py +++ b/tests/functional/openlp_plugins/songs/test_videopsalm.py @@ -43,3 +43,5 @@ class TestVideoPsalmFileImport(SongImportTestHelper): """ self.file_import(os.path.join(TEST_PATH, 'videopsalm-as-safe-a-stronghold.json'), self.load_external_result_data(os.path.join(TEST_PATH, 'as-safe-a-stronghold.json'))) + self.file_import(os.path.join(TEST_PATH, 'videopsalm-as-safe-a-stronghold2.json'), + self.load_external_result_data(os.path.join(TEST_PATH, 'as-safe-a-stronghold2.json'))) diff --git a/tests/resources/videopsalmsongs/videopsalm-as-safe-a-stronghold2.json b/tests/resources/videopsalmsongs/videopsalm-as-safe-a-stronghold2.json new file mode 100644 index 000000000..11bc082e6 --- /dev/null +++ b/tests/resources/videopsalmsongs/videopsalm-as-safe-a-stronghold2.json @@ -0,0 +1,47 @@ +{Abbreviation:"SB1",Copyright:"Public domain",Songs:[{ID:3,Composer:"Unknown",Author:"Martin Luther",Copyright:"Public +Domain",Theme:"tema1 +tema2",CCLI:"12345",Alias:"A safe stronghold",Memo1:"This is +the first comment +",Memo2:"This is +the second comment +",Memo3:"This is +the third comment +",Reference:"reference",Guid:"jtCkrJdPIUOmECjaQylg/g",Verses:[{ +Text:"As safe a stronghold our God is still, +A trusty shield and weapon; +He’ll help us clear from all the ill +That hath us now o’ertaken. +The ancient prince of hell +Hath risen with purpose fell; +Strong mail of craft and power +He weareth in this hour; +On earth is not His fellow."},{ID:2, +Text:"With \"force\" of arms we nothing can, +Full soon were we down-ridden; +But for us fights \\ the proper Man, +Whom God Himself hath bidden. +Ask ye: Who is this same? +Christ Jesus is His name, +The Lord Sabaoth’s Son; +He, and no other one, +Shall conquer in the battle."},{ID:3, +Text:"And were this world all devils o’er, +And watching to devour us, +We lay it not to heart so sore; +Not they can overpower us. +And let the prince of ill +Look grim as e’er he will, +He harms us not a whit; +For why? his doom is writ; +A word shall quickly slay him."},{ID:4, +Text:"God’s word, for all their craft and force, +One moment will not linger, +But, spite of hell, shall have its course; +’Tis written by His finger. +And though they take our life, +Goods, honour, children, wife, +Yet is their profit small: +These things shall vanish all; +The city of God remaineth."}],AudioFile:"282.mp3",IsAudioFileEnabled:1, +Text:"A Safe Stronghold Our God is Still"}],Guid:"khiHU2blX0Kb41dGdbDLhA",VersionDate:"20121012000000", +Text:"SongBook1"} From 8009bfef855192a36c6d225bdd5bcfef299a27d5 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Thu, 7 Jul 2016 22:48:52 +0200 Subject: [PATCH 09/21] forgot test file --- .../as-safe-a-stronghold2.json | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 tests/resources/videopsalmsongs/as-safe-a-stronghold2.json diff --git a/tests/resources/videopsalmsongs/as-safe-a-stronghold2.json b/tests/resources/videopsalmsongs/as-safe-a-stronghold2.json new file mode 100644 index 000000000..f6becc92a --- /dev/null +++ b/tests/resources/videopsalmsongs/as-safe-a-stronghold2.json @@ -0,0 +1,35 @@ +{ + "authors": [ + ["Martin Luther", "words"], + ["Unknown", "music"] + ], + "ccli_number": "12345", + "comments": "This is\nthe first comment\nThis is\nthe second comment\nThis is\nthe third comment\n", + "copyright": "Public Domain", + "song_book_name": "SongBook1", + "song_number": 0, + "title": "A Safe Stronghold Our God is Still", + "topics": [ + "tema1", + "tema2" + ], + "verse_order_list": [], + "verses": [ + [ + "As safe a stronghold our God is still,\nA trusty shield and weapon;\nHe’ll help us clear from all the ill\nThat hath us now o’ertaken.\nThe ancient prince of hell\nHath risen with purpose fell;\nStrong mail of craft and power\nHe weareth in this hour;\nOn earth is not His fellow.", + "v" + ], + [ + "With \"force\" of arms we nothing can,\nFull soon were we down-ridden;\nBut for us fights \\ the proper Man,\nWhom God Himself hath bidden.\nAsk ye: Who is this same?\nChrist Jesus is His name,\nThe Lord Sabaoth’s Son;\nHe, and no other one,\nShall conquer in the battle.", + "v" + ], + [ + "And were this world all devils o’er,\nAnd watching to devour us,\nWe lay it not to heart so sore;\nNot they can overpower us.\nAnd let the prince of ill\nLook grim as e’er he will,\nHe harms us not a whit;\nFor why? his doom is writ;\nA word shall quickly slay him.", + "v" + ], + [ + "God’s word, for all their craft and force,\nOne moment will not linger,\nBut, spite of hell, shall have its course;\n’Tis written by His finger.\nAnd though they take our life,\nGoods, honour, children, wife,\nYet is their profit small:\nThese things shall vanish all;\nThe city of God remaineth.", + "v" + ] + ] +} From f5da7e2a2b96b50bd6294accf1adf343be0695fc Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Thu, 7 Jul 2016 22:51:25 +0200 Subject: [PATCH 10/21] Make easyslide importer try to recover when reading non-standard xml. Fixes bug 1588822. Fixes: https://launchpad.net/bugs/1588822 --- openlp/plugins/songs/lib/importers/easyslides.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/plugins/songs/lib/importers/easyslides.py b/openlp/plugins/songs/lib/importers/easyslides.py index 907a6c90f..2ae489208 100644 --- a/openlp/plugins/songs/lib/importers/easyslides.py +++ b/openlp/plugins/songs/lib/importers/easyslides.py @@ -46,7 +46,7 @@ class EasySlidesImport(SongImport): def do_import(self): log.info('Importing EasySlides XML file {source}'.format(source=self.import_source)) - parser = etree.XMLParser(remove_blank_text=True) + parser = etree.XMLParser(remove_blank_text=True, recover=True) parsed_file = etree.parse(self.import_source, parser) xml = etree.tostring(parsed_file).decode() song_xml = objectify.fromstring(xml) From ba80fe653ca89893d458139b5370b34f1d665baa Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Thu, 7 Jul 2016 22:52:00 +0200 Subject: [PATCH 11/21] Fix format error --- openlp/plugins/bibles/lib/manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/plugins/bibles/lib/manager.py b/openlp/plugins/bibles/lib/manager.py index d1465475d..1c55222f2 100644 --- a/openlp/plugins/bibles/lib/manager.py +++ b/openlp/plugins/bibles/lib/manager.py @@ -370,7 +370,7 @@ class BibleManager(RegistryProperties): """ log.debug('save_meta data {bible}, {version}, {copyright}, {perms}'.format(bible=bible, version=version, - cr=copyright, + copyright=copyright, perms=permissions)) self.db_cache[bible].save_meta('name', version) self.db_cache[bible].save_meta('copyright', copyright) From 93fc6e014537b0959fd09e6dee1ce3fe24961958 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Thu, 7 Jul 2016 22:56:50 +0200 Subject: [PATCH 12/21] Update Crosswalk webpage parser to match new layout. Fixes bug 1599999. Fixes: https://launchpad.net/bugs/1599999 --- openlp/plugins/bibles/lib/http.py | 40 +++++++++++-------- .../openlp_plugins/bibles/test_lib_http.py | 1 - 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/openlp/plugins/bibles/lib/http.py b/openlp/plugins/bibles/lib/http.py index c50745c2f..fce5d3285 100644 --- a/openlp/plugins/bibles/lib/http.py +++ b/openlp/plugins/bibles/lib/http.py @@ -532,28 +532,26 @@ class CWExtract(RegistryProperties): returns a list in the form [(biblename, biblekey, language_code)] """ log.debug('CWExtract.get_bibles_from_http') - bible_url = 'http://www.biblestudytools.com/' + bible_url = 'http://www.biblestudytools.com/bible-versions/' soup = get_soup_for_bible_ref(bible_url) if not soup: return None - bible_select = soup.find('select') - if not bible_select: - log.debug('No select tags found - did site change?') - return None - option_tags = bible_select.find_all('option', {'class': 'log-translation'}) - if not option_tags: - log.debug('No option tags found - did site change?') + h4_tags = soup.find_all('h4', {'class': 'small-header'}) + if not h4_tags: + log.debug('No h4 tags found - did site change?') return None bibles = [] - for ot in option_tags: - tag_text = ot.get_text().strip() - try: - tag_value = ot['value'] - except KeyError: - log.exception('No value attribute found - did site change?') + for h4t in h4_tags: + short_name = None + if h4t.span: + short_name = h4t.span.get_text().strip().lower() + else: + log.error('No span tag found - did site change?') return None - if not tag_value: + if not short_name: continue + h4t.span.extract() + tag_text = h4t.get_text().strip() # The names of non-english bibles has their language in parentheses at the end if tag_text.endswith(')'): language = tag_text[tag_text.rfind('(') + 1:-1] @@ -561,12 +559,20 @@ class CWExtract(RegistryProperties): language_code = CROSSWALK_LANGUAGES[language] else: language_code = '' - # ... except for the latin vulgate + # ... except for those that don't... elif 'latin' in tag_text.lower(): language_code = 'la' + elif 'la biblia' in tag_text.lower() or 'nueva' in tag_text.lower(): + language_code = 'es' + elif 'chinese' in tag_text.lower(): + language_code = 'zh' + elif 'greek' in tag_text.lower(): + language_code = 'el' + elif 'nova' in tag_text.lower(): + language_code = 'pt' else: language_code = 'en' - bibles.append((tag_text, tag_value, language_code)) + bibles.append((tag_text, short_name, language_code)) return bibles diff --git a/tests/interfaces/openlp_plugins/bibles/test_lib_http.py b/tests/interfaces/openlp_plugins/bibles/test_lib_http.py index 4a7fb4af3..4ca4a8b0f 100644 --- a/tests/interfaces/openlp_plugins/bibles/test_lib_http.py +++ b/tests/interfaces/openlp_plugins/bibles/test_lib_http.py @@ -146,7 +146,6 @@ class TestBibleHTTP(TestCase): self.assertIsNotNone(bibles) self.assertIn(('Holman Christian Standard Bible', 'HCSB', 'en'), bibles) - @skip("Waiting for Crosswalk to fix their server") def test_crosswalk_get_bibles(self): """ Test getting list of bibles from Crosswalk.com From de3f4046b7a452120cb90d147eee46adfb99ed00 Mon Sep 17 00:00:00 2001 From: Ken Roberts Date: Fri, 8 Jul 2016 12:19:08 -0700 Subject: [PATCH 13/21] Convert md5sum calls to utf-8 for non-ascii pins --- openlp/core/common/__init__.py | 22 ++++++++++++++----- openlp/core/lib/projector/pjlink1.py | 11 +++++----- .../test_projector_utilities.py | 10 ++++----- 3 files changed, 27 insertions(+), 16 deletions(-) diff --git a/openlp/core/common/__init__.py b/openlp/core/common/__init__.py index a5c86314c..9afc08c8f 100644 --- a/openlp/core/common/__init__.py +++ b/openlp/core/common/__init__.py @@ -195,7 +195,7 @@ def verify_ip_address(addr): return True if verify_ipv4(addr) else verify_ipv6(addr) -def md5_hash(salt, data=None): +def md5_hash(salt=None, data=None): """ Returns the hashed output of md5sum on salt,data using Python3 hashlib @@ -205,8 +205,11 @@ def md5_hash(salt, data=None): :returns: str """ log.debug('md5_hash(salt="{text}")'.format(text=salt)) + if not salt and not data: + return None hash_obj = hashlib.new('md5') - hash_obj.update(salt) + if salt: + hash_obj.update(salt) if data: hash_obj.update(data) hash_value = hash_obj.hexdigest() @@ -214,18 +217,25 @@ def md5_hash(salt, data=None): return hash_value -def qmd5_hash(salt, data=None): +def qmd5_hash(salt=None, data=None): """ Returns the hashed output of MD5Sum on salt, data - using PyQt5.QCryptographicHash. + using PyQt5.QCryptographicHash. Function returns a + QByteArray instead of a text string. + If you need a string instead, call with + + result = str(qmd5_hash(salt=..., data=...), encoding='ascii') :param salt: Initial salt :param data: OPTIONAL Data to hash - :returns: str + :returns: QByteArray """ log.debug('qmd5_hash(salt="{text}"'.format(text=salt)) + if salt is None and data is None: + return None hash_obj = QHash(QHash.Md5) - hash_obj.addData(salt) + if salt: + hash_obj.addData(salt) if data: hash_obj.addData(data) hash_value = hash_obj.result().toHex() diff --git a/openlp/core/lib/projector/pjlink1.py b/openlp/core/lib/projector/pjlink1.py index 330e02b73..71c3e450d 100644 --- a/openlp/core/lib/projector/pjlink1.py +++ b/openlp/core/lib/projector/pjlink1.py @@ -49,7 +49,7 @@ from codecs import decode from PyQt5.QtCore import pyqtSignal, pyqtSlot from PyQt5.QtNetwork import QAbstractSocket, QTcpSocket -from openlp.core.common import translate, md5_hash +from openlp.core.common import translate, qmd5_hash from openlp.core.lib.projector.constants import * # Shortcuts @@ -364,14 +364,15 @@ class PJLink1(QTcpSocket): else: log.debug('({ip}) Setting hash with salt="{data}"'.format(ip=self.ip, data=data_check[2])) log.debug('({ip}) pin="{data}"'.format(ip=self.ip, data=self.pin)) - salt = md5_hash(salt=data_check[2].encode('ascii'), data=self.pin.encode('ascii')) + data_hash = str(qmd5_hash(salt=data_check[2].encode('utf-8'), data=self.pin.encode('utf-8')), + encoding='ascii') else: - salt = None - # We're connected at this point, so go ahead and do regular I/O + data_hash = None + # We're connected at this point, so go ahead and setup regular I/O self.readyRead.connect(self.get_data) self.projectorReceivedData.connect(self._send_command) # Initial data we should know about - self.send_command(cmd='CLSS', salt=salt) + self.send_command(cmd='CLSS', salt=data_hash) self.waitForReadyRead() if (not self.no_poll) and (self.state() == self.ConnectedState): log.debug('({ip}) Starting timer'.format(ip=self.ip)) diff --git a/tests/functional/openlp_core_common/test_projector_utilities.py b/tests/functional/openlp_core_common/test_projector_utilities.py index 72609956d..75ecd582a 100644 --- a/tests/functional/openlp_core_common/test_projector_utilities.py +++ b/tests/functional/openlp_core_common/test_projector_utilities.py @@ -124,7 +124,7 @@ class testProjectorUtilities(TestCase): Test MD5 hash from salt+data pass (python) """ # WHEN: Given a known salt+data - hash_ = md5_hash(salt=salt.encode('ascii'), data=pin.encode('ascii')) + hash_ = md5_hash(salt=salt.encode('utf-8'), data=pin.encode('utf-8')) # THEN: Validate return has is same self.assertEquals(hash_, test_hash, 'MD5 should have returned a good hash') @@ -134,7 +134,7 @@ class testProjectorUtilities(TestCase): Test MD5 hash from salt+data fail (python) """ # WHEN: Given a different salt+hash - hash_ = md5_hash(salt=pin.encode('ascii'), data=salt.encode('ascii')) + hash_ = md5_hash(salt=pin.encode('utf-8'), data=salt.encode('utf-8')) # THEN: return data is different self.assertNotEquals(hash_, test_hash, 'MD5 should have returned a bad hash') @@ -144,7 +144,7 @@ class testProjectorUtilities(TestCase): Test MD5 hash from salt+data pass (Qt) """ # WHEN: Given a known salt+data - hash_ = qmd5_hash(salt=salt.encode('ascii'), data=pin.encode('ascii')) + hash_ = qmd5_hash(salt=salt.encode('utf-8'), data=pin.encode('utf-8')) # THEN: Validate return has is same self.assertEquals(hash_, test_hash, 'Qt-MD5 should have returned a good hash') @@ -154,7 +154,7 @@ class testProjectorUtilities(TestCase): Test MD5 hash from salt+hash fail (Qt) """ # WHEN: Given a different salt+hash - hash_ = qmd5_hash(salt=pin.encode('ascii'), data=salt.encode('ascii')) + hash_ = qmd5_hash(salt=pin.encode('utf-8'), data=salt.encode('utf-8')) # THEN: return data is different self.assertNotEquals(hash_, test_hash, 'Qt-MD5 should have returned a bad hash') @@ -174,7 +174,7 @@ class testProjectorUtilities(TestCase): Test MD5 hash with non-ascii string - bug 1417809 """ # WHEN: Non-ascii string is hashed - hash_ = md5_hash(salt=test_non_ascii_string.encode('utf-8'), data=None) + hash_ = md5_hash(data=test_non_ascii_string.encode('utf-8')) # THEN: Valid MD5 hash should be returned self.assertEqual(hash_, test_non_ascii_hash, 'Qt-MD5 should have returned a valid hash') From 3cdd42bfe0180504c7971f596af98e9adb0a88ce Mon Sep 17 00:00:00 2001 From: Raoul Snyman Date: Sun, 17 Jul 2016 00:20:56 +0200 Subject: [PATCH 14/21] Catch the PermissionError too --- openlp/core/ui/servicemanager.py | 9 +- .../openlp_core_ui/test_servicemanager.py | 310 ++++++++++-------- 2 files changed, 176 insertions(+), 143 deletions(-) diff --git a/openlp/core/ui/servicemanager.py b/openlp/core/ui/servicemanager.py index 907cb49a6..7c01b4191 100644 --- a/openlp/core/ui/servicemanager.py +++ b/openlp/core/ui/servicemanager.py @@ -480,9 +480,10 @@ class ServiceManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ServiceMa :return: service array """ service = [] - core = {'lite-service': self._save_lite, - 'service-theme': self.service_theme - } + core = { + 'lite-service': self._save_lite, + 'service-theme': self.service_theme + } service.append({'openlp_core': core}) return service @@ -658,7 +659,7 @@ class ServiceManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ServiceMa if success: try: shutil.copy(temp_file_name, path_file_name) - except shutil.Error: + except (shutil.Error, PermissionError): return self.save_file_as() self.main_window.add_recent_file(path_file_name) self.set_modified(False) diff --git a/tests/functional/openlp_core_ui/test_servicemanager.py b/tests/functional/openlp_core_ui/test_servicemanager.py index 7882d8a24..1697eb1f6 100644 --- a/tests/functional/openlp_core_ui/test_servicemanager.py +++ b/tests/functional/openlp_core_ui/test_servicemanager.py @@ -22,10 +22,12 @@ """ Package to test the openlp.core.ui.slidecontroller package. """ -import PyQt5 +import os from unittest import TestCase -from openlp.core.common import Registry, ThemeLevel, Settings +import PyQt5 + +from openlp.core.common import Registry, ThemeLevel from openlp.core.lib import ServiceItem, ServiceItemType, ItemCapabilities from openlp.core.ui import ServiceManager @@ -33,6 +35,9 @@ from tests.functional import MagicMock, patch class TestServiceManager(TestCase): + """ + Test the service manager + """ def setUp(self): """ @@ -108,20 +113,20 @@ class TestServiceManager(TestCase): # WHEN I define a context menu service_manager.context_menu(1) # THEN the following calls should have occurred. - self.assertEquals(service_manager.edit_action.setVisible.call_count, 1, 'Should have been called once') - self.assertEquals(service_manager.rename_action.setVisible.call_count, 1, 'Should have been called once') - self.assertEquals(service_manager.create_custom_action.setVisible.call_count, 1, 'Should have been called once') - self.assertEquals(service_manager.maintain_action.setVisible.call_count, 1, 'Should have been called once') - self.assertEquals(service_manager.notes_action.setVisible.call_count, 1, 'Should have been called once') - self.assertEquals(service_manager.time_action.setVisible.call_count, 1, 'Should have been called once') - self.assertEquals(service_manager.auto_start_action.setVisible.call_count, 1, 'Should have been called once') - self.assertEquals(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 1, - 'Should have been called once') - self.assertEquals(service_manager.auto_play_slides_once.setChecked.call_count, 0, 'Should not be called') - self.assertEquals(service_manager.auto_play_slides_loop.setChecked.call_count, 0, 'Should not be called') - self.assertEquals(service_manager.timed_slide_interval.setChecked.call_count, 0, 'Should not be called') - self.assertEquals(service_manager.theme_menu.menuAction().setVisible.call_count, 1, - 'Should have been called once') + self.assertEqual(service_manager.edit_action.setVisible.call_count, 1, 'Should have been called once') + self.assertEqual(service_manager.rename_action.setVisible.call_count, 1, 'Should have been called once') + self.assertEqual(service_manager.create_custom_action.setVisible.call_count, 1, 'Should have been called once') + self.assertEqual(service_manager.maintain_action.setVisible.call_count, 1, 'Should have been called once') + self.assertEqual(service_manager.notes_action.setVisible.call_count, 1, 'Should have been called once') + self.assertEqual(service_manager.time_action.setVisible.call_count, 1, 'Should have been called once') + self.assertEqual(service_manager.auto_start_action.setVisible.call_count, 1, 'Should have been called once') + self.assertEqual(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 1, + 'Should have been called once') + self.assertEqual(service_manager.auto_play_slides_once.setChecked.call_count, 0, 'Should not be called') + self.assertEqual(service_manager.auto_play_slides_loop.setChecked.call_count, 0, 'Should not be called') + self.assertEqual(service_manager.timed_slide_interval.setChecked.call_count, 0, 'Should not be called') + self.assertEqual(service_manager.theme_menu.menuAction().setVisible.call_count, 1, + 'Should have been called once') def test_build_song_context_menu(self): """ @@ -139,12 +144,10 @@ class TestServiceManager(TestCase): service_manager.service_manager_list = MagicMock() service_manager.service_manager_list.itemAt.return_value = item service_item = ServiceItem(None) - service_item.add_capability(ItemCapabilities.CanEdit) - service_item.add_capability(ItemCapabilities.CanPreview) - service_item.add_capability(ItemCapabilities.CanLoop) - service_item.add_capability(ItemCapabilities.OnLoadUpdate) - service_item.add_capability(ItemCapabilities.AddIfNewItem) - service_item.add_capability(ItemCapabilities.CanSoftBreak) + for capability in [ItemCapabilities.CanEdit, ItemCapabilities.CanPreview, ItemCapabilities.CanLoop, + ItemCapabilities.OnLoadUpdate, ItemCapabilities.AddIfNewItem, + ItemCapabilities.CanSoftBreak]: + service_item.add_capability(capability) service_item.service_item_type = ServiceItemType.Text service_item.edit_id = 1 service_item._display_frames.append(MagicMock()) @@ -165,29 +168,29 @@ class TestServiceManager(TestCase): # WHEN I define a context menu service_manager.context_menu(1) # THEN the following calls should have occurred. - self.assertEquals(service_manager.edit_action.setVisible.call_count, 2, 'Should have be called twice') - self.assertEquals(service_manager.rename_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.create_custom_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.maintain_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.notes_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.time_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.auto_start_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 1, - 'Should have be called once') - self.assertEquals(service_manager.auto_play_slides_once.setChecked.call_count, 0, 'Should not be called') - self.assertEquals(service_manager.auto_play_slides_loop.setChecked.call_count, 0, 'Should not be called') - self.assertEquals(service_manager.timed_slide_interval.setChecked.call_count, 0, 'Should not be called') - self.assertEquals(service_manager.theme_menu.menuAction().setVisible.call_count, 2, - 'Should have be called twice') + self.assertEqual(service_manager.edit_action.setVisible.call_count, 2, 'Should have be called twice') + self.assertEqual(service_manager.rename_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.create_custom_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.maintain_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.notes_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.time_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.auto_start_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 1, + 'Should have be called once') + self.assertEqual(service_manager.auto_play_slides_once.setChecked.call_count, 0, 'Should not be called') + self.assertEqual(service_manager.auto_play_slides_loop.setChecked.call_count, 0, 'Should not be called') + self.assertEqual(service_manager.timed_slide_interval.setChecked.call_count, 0, 'Should not be called') + self.assertEqual(service_manager.theme_menu.menuAction().setVisible.call_count, 2, + 'Should have be called twice') # THEN we add a 2nd display frame service_item._display_frames.append(MagicMock()) service_manager.context_menu(1) # THEN the following additional calls should have occurred. - self.assertEquals(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 2, - 'Should have be called twice') - self.assertEquals(service_manager.auto_play_slides_once.setChecked.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.auto_play_slides_loop.setChecked.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.timed_slide_interval.setChecked.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 2, + 'Should have be called twice') + self.assertEqual(service_manager.auto_play_slides_once.setChecked.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.auto_play_slides_loop.setChecked.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.timed_slide_interval.setChecked.call_count, 1, 'Should have be called once') def test_build_bible_context_menu(self): """ @@ -205,11 +208,10 @@ class TestServiceManager(TestCase): service_manager.service_manager_list = MagicMock() service_manager.service_manager_list.itemAt.return_value = item service_item = ServiceItem(None) - service_item.add_capability(ItemCapabilities.NoLineBreaks) - service_item.add_capability(ItemCapabilities.CanPreview) - service_item.add_capability(ItemCapabilities.CanLoop) - service_item.add_capability(ItemCapabilities.CanWordSplit) - service_item.add_capability(ItemCapabilities.CanEditTitle) + for capability in [ItemCapabilities.NoLineBreaks, ItemCapabilities.CanPreview, + ItemCapabilities.CanLoop, ItemCapabilities.CanWordSplit, + ItemCapabilities.CanEditTitle]: + service_item.add_capability(capability) service_item.service_item_type = ServiceItemType.Text service_item.edit_id = 1 service_item._display_frames.append(MagicMock()) @@ -230,29 +232,29 @@ class TestServiceManager(TestCase): # WHEN I define a context menu service_manager.context_menu(1) # THEN the following calls should have occurred. - self.assertEquals(service_manager.edit_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.rename_action.setVisible.call_count, 2, 'Should have be called twice') - self.assertEquals(service_manager.create_custom_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.maintain_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.notes_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.time_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.auto_start_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 1, - 'Should have be called once') - self.assertEquals(service_manager.auto_play_slides_once.setChecked.call_count, 0, 'Should not be called') - self.assertEquals(service_manager.auto_play_slides_loop.setChecked.call_count, 0, 'Should not be called') - self.assertEquals(service_manager.timed_slide_interval.setChecked.call_count, 0, 'Should not be called') - self.assertEquals(service_manager.theme_menu.menuAction().setVisible.call_count, 2, - 'Should have be called twice') + self.assertEqual(service_manager.edit_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.rename_action.setVisible.call_count, 2, 'Should have be called twice') + self.assertEqual(service_manager.create_custom_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.maintain_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.notes_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.time_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.auto_start_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 1, + 'Should have be called once') + self.assertEqual(service_manager.auto_play_slides_once.setChecked.call_count, 0, 'Should not be called') + self.assertEqual(service_manager.auto_play_slides_loop.setChecked.call_count, 0, 'Should not be called') + self.assertEqual(service_manager.timed_slide_interval.setChecked.call_count, 0, 'Should not be called') + self.assertEqual(service_manager.theme_menu.menuAction().setVisible.call_count, 2, + 'Should have be called twice') # THEN we add a 2nd display frame service_item._display_frames.append(MagicMock()) service_manager.context_menu(1) # THEN the following additional calls should have occurred. - self.assertEquals(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 2, - 'Should have be called twice') - self.assertEquals(service_manager.auto_play_slides_once.setChecked.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.auto_play_slides_loop.setChecked.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.timed_slide_interval.setChecked.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 2, + 'Should have be called twice') + self.assertEqual(service_manager.auto_play_slides_once.setChecked.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.auto_play_slides_loop.setChecked.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.timed_slide_interval.setChecked.call_count, 1, 'Should have be called once') def test_build_custom_context_menu(self): """ @@ -295,29 +297,29 @@ class TestServiceManager(TestCase): # WHEN I define a context menu service_manager.context_menu(1) # THEN the following calls should have occurred. - self.assertEquals(service_manager.edit_action.setVisible.call_count, 2, 'Should have be called twice') - self.assertEquals(service_manager.rename_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.create_custom_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.maintain_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.notes_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.time_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.auto_start_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 1, + self.assertEqual(service_manager.edit_action.setVisible.call_count, 2, 'Should have be called twice') + self.assertEqual(service_manager.rename_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.create_custom_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.maintain_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.notes_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.time_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.auto_start_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.auto_play_slides_once.setChecked.call_count, 0, 'Should not be called') - self.assertEquals(service_manager.auto_play_slides_loop.setChecked.call_count, 0, 'Should not be called') - self.assertEquals(service_manager.timed_slide_interval.setChecked.call_count, 0, 'Should not be called') - self.assertEquals(service_manager.theme_menu.menuAction().setVisible.call_count, 2, + self.assertEqual(service_manager.auto_play_slides_once.setChecked.call_count, 0, 'Should not be called') + self.assertEqual(service_manager.auto_play_slides_loop.setChecked.call_count, 0, 'Should not be called') + self.assertEqual(service_manager.timed_slide_interval.setChecked.call_count, 0, 'Should not be called') + self.assertEqual(service_manager.theme_menu.menuAction().setVisible.call_count, 2, 'Should have be called twice') # THEN we add a 2nd display frame service_item._display_frames.append(MagicMock()) service_manager.context_menu(1) # THEN the following additional calls should have occurred. - self.assertEquals(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 2, + self.assertEqual(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 2, 'Should have be called twice') - self.assertEquals(service_manager.auto_play_slides_once.setChecked.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.auto_play_slides_loop.setChecked.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.timed_slide_interval.setChecked.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.auto_play_slides_once.setChecked.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.auto_play_slides_loop.setChecked.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.timed_slide_interval.setChecked.call_count, 1, 'Should have be called once') def test_build_image_context_menu(self): """ @@ -358,29 +360,29 @@ class TestServiceManager(TestCase): # WHEN I define a context menu service_manager.context_menu(1) # THEN the following calls should have occurred. - self.assertEquals(service_manager.edit_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.rename_action.setVisible.call_count, 2, 'Should have be called twice') - self.assertEquals(service_manager.create_custom_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.maintain_action.setVisible.call_count, 2, 'Should have be called twice') - self.assertEquals(service_manager.notes_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.time_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.auto_start_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 1, + self.assertEqual(service_manager.edit_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.rename_action.setVisible.call_count, 2, 'Should have be called twice') + self.assertEqual(service_manager.create_custom_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.maintain_action.setVisible.call_count, 2, 'Should have be called twice') + self.assertEqual(service_manager.notes_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.time_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.auto_start_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.auto_play_slides_once.setChecked.call_count, 0, 'Should not be called') - self.assertEquals(service_manager.auto_play_slides_loop.setChecked.call_count, 0, 'Should not be called') - self.assertEquals(service_manager.timed_slide_interval.setChecked.call_count, 0, 'Should not be called') - self.assertEquals(service_manager.theme_menu.menuAction().setVisible.call_count, 1, + self.assertEqual(service_manager.auto_play_slides_once.setChecked.call_count, 0, 'Should not be called') + self.assertEqual(service_manager.auto_play_slides_loop.setChecked.call_count, 0, 'Should not be called') + self.assertEqual(service_manager.timed_slide_interval.setChecked.call_count, 0, 'Should not be called') + self.assertEqual(service_manager.theme_menu.menuAction().setVisible.call_count, 1, 'Should have be called once') # THEN we add a 2nd display frame and regenerate the menu. service_item._raw_frames.append(MagicMock()) service_manager.context_menu(1) # THEN the following additional calls should have occurred. - self.assertEquals(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 2, + self.assertEqual(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 2, 'Should have be called twice') - self.assertEquals(service_manager.auto_play_slides_once.setChecked.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.auto_play_slides_loop.setChecked.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.timed_slide_interval.setChecked.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.auto_play_slides_once.setChecked.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.auto_play_slides_loop.setChecked.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.timed_slide_interval.setChecked.call_count, 1, 'Should have be called once') def test_build_media_context_menu(self): """ @@ -419,25 +421,25 @@ class TestServiceManager(TestCase): # WHEN I define a context menu service_manager.context_menu(1) # THEN the following calls should have occurred. - self.assertEquals(service_manager.edit_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.rename_action.setVisible.call_count, 2, 'Should have be called twice') - self.assertEquals(service_manager.create_custom_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.maintain_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.notes_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.time_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.auto_start_action.setVisible.call_count, 2, 'Should have be called twice') - self.assertEquals(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 1, + self.assertEqual(service_manager.edit_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.rename_action.setVisible.call_count, 2, 'Should have be called twice') + self.assertEqual(service_manager.create_custom_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.maintain_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.notes_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.time_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.auto_start_action.setVisible.call_count, 2, 'Should have be called twice') + self.assertEqual(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.auto_play_slides_once.setChecked.call_count, 0, 'Should not be called') - self.assertEquals(service_manager.auto_play_slides_loop.setChecked.call_count, 0, 'Should not be called') - self.assertEquals(service_manager.timed_slide_interval.setChecked.call_count, 0, 'Should not be called') - self.assertEquals(service_manager.theme_menu.menuAction().setVisible.call_count, 1, + self.assertEqual(service_manager.auto_play_slides_once.setChecked.call_count, 0, 'Should not be called') + self.assertEqual(service_manager.auto_play_slides_loop.setChecked.call_count, 0, 'Should not be called') + self.assertEqual(service_manager.timed_slide_interval.setChecked.call_count, 0, 'Should not be called') + self.assertEqual(service_manager.theme_menu.menuAction().setVisible.call_count, 1, 'Should have be called once') # THEN I change the length of the media and regenerate the menu. service_item.set_media_length(5) service_manager.context_menu(1) # THEN the following additional calls should have occurred. - self.assertEquals(service_manager.time_action.setVisible.call_count, 3, 'Should have be called three times') + self.assertEqual(service_manager.time_action.setVisible.call_count, 3, 'Should have be called three times') def test_build_presentation_pdf_context_menu(self): """ @@ -477,19 +479,19 @@ class TestServiceManager(TestCase): # WHEN I define a context menu service_manager.context_menu(1) # THEN the following calls should have occurred. - self.assertEquals(service_manager.edit_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.rename_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.create_custom_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.maintain_action.setVisible.call_count, 2, 'Should have be called twice') - self.assertEquals(service_manager.notes_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.time_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.auto_start_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 1, + self.assertEqual(service_manager.edit_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.rename_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.create_custom_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.maintain_action.setVisible.call_count, 2, 'Should have be called twice') + self.assertEqual(service_manager.notes_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.time_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.auto_start_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.auto_play_slides_once.setChecked.call_count, 0, 'Should not be called') - self.assertEquals(service_manager.auto_play_slides_loop.setChecked.call_count, 0, 'Should not be called') - self.assertEquals(service_manager.timed_slide_interval.setChecked.call_count, 0, 'Should not be called') - self.assertEquals(service_manager.theme_menu.menuAction().setVisible.call_count, 1, + self.assertEqual(service_manager.auto_play_slides_once.setChecked.call_count, 0, 'Should not be called') + self.assertEqual(service_manager.auto_play_slides_loop.setChecked.call_count, 0, 'Should not be called') + self.assertEqual(service_manager.timed_slide_interval.setChecked.call_count, 0, 'Should not be called') + self.assertEqual(service_manager.theme_menu.menuAction().setVisible.call_count, 1, 'Should have be called once') def test_build_presentation_non_pdf_context_menu(self): @@ -527,19 +529,19 @@ class TestServiceManager(TestCase): # WHEN I define a context menu service_manager.context_menu(1) # THEN the following calls should have occurred. - self.assertEquals(service_manager.edit_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.rename_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.create_custom_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.maintain_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.notes_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.time_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.auto_start_action.setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 1, + self.assertEqual(service_manager.edit_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.rename_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.create_custom_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.maintain_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.notes_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.time_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.auto_start_action.setVisible.call_count, 1, 'Should have be called once') + self.assertEqual(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 1, 'Should have be called once') - self.assertEquals(service_manager.auto_play_slides_once.setChecked.call_count, 0, 'Should not be called') - self.assertEquals(service_manager.auto_play_slides_loop.setChecked.call_count, 0, 'Should not be called') - self.assertEquals(service_manager.timed_slide_interval.setChecked.call_count, 0, 'Should not be called') - self.assertEquals(service_manager.theme_menu.menuAction().setVisible.call_count, 1, + self.assertEqual(service_manager.auto_play_slides_once.setChecked.call_count, 0, 'Should not be called') + self.assertEqual(service_manager.auto_play_slides_loop.setChecked.call_count, 0, 'Should not be called') + self.assertEqual(service_manager.timed_slide_interval.setChecked.call_count, 0, 'Should not be called') + self.assertEqual(service_manager.theme_menu.menuAction().setVisible.call_count, 1, 'Should have be called once') @patch(u'openlp.core.ui.servicemanager.Settings') @@ -573,7 +575,7 @@ class TestServiceManager(TestCase): # WHEN: on_single_click_preview() is called service_manager.on_single_click_preview() # THEN: timer should not be started - self.assertEquals(mocked_singleShot.call_count, 0, 'Should not be called') + self.assertEqual(mocked_singleShot.call_count, 0, 'Should not be called') @patch(u'openlp.core.ui.servicemanager.Settings') @patch(u'PyQt5.QtCore.QTimer.singleShot') @@ -591,7 +593,8 @@ class TestServiceManager(TestCase): service_manager.on_double_click_live() service_manager.on_single_click_preview() # THEN: timer should not be started - self.assertEquals(mocked_singleShot.call_count, 0, 'Should not be called') + mocked_make_live.assert_called_with() + self.assertEqual(mocked_singleShot.call_count, 0, 'Should not be called') @patch(u'openlp.core.ui.servicemanager.ServiceManager.make_preview') def test_single_click_timeout_single(self, mocked_make_preview): @@ -603,7 +606,7 @@ class TestServiceManager(TestCase): # WHEN: on_single_click_preview() is called service_manager.on_single_click_preview_timeout() # THEN: make_preview() should have been called - self.assertEquals(mocked_make_preview.call_count, 1, 'Should have been called once') + self.assertEqual(mocked_make_preview.call_count, 1, 'ServiceManager.make_preview() should have been called once') @patch(u'openlp.core.ui.servicemanager.ServiceManager.make_preview') @patch(u'openlp.core.ui.servicemanager.ServiceManager.make_live') @@ -616,5 +619,34 @@ class TestServiceManager(TestCase): # WHEN: on_single_click_preview() is called after a double click service_manager.on_double_click_live() service_manager.on_single_click_preview_timeout() - # THEN: make_preview() should have been called - self.assertEquals(mocked_make_preview.call_count, 0, 'Should not be called') + # THEN: make_preview() should not have been called + self.assertEqual(mocked_make_preview.call_count, 0, 'ServiceManager.make_preview() should not be called') + + @patch(u'openlp.core.ui.servicemanager.shutil.copy') + @patch(u'openlp.core.ui.servicemanager.zipfile') + @patch(u'openlp.core.ui.servicemanager.ServiceManager.save_file_as') + def test_save_raises_permission_error(self, mocked_save_file_as, mocked_zipfile, mocked_shutil_copy): + """ + Test that when a PermissionError is raised when trying to save a file, it is handled correctly + """ + # GIVEN: A service manager, a service to save + mocked_main_window = MagicMock() + mocked_main_window.service_manager_settings_section = 'servicemanager' + Registry().register('main_window', mocked_main_window) + Registry().register('application', MagicMock()) + service_manager = ServiceManager(None) + service_manager._file_name = os.path.join('temp', 'filename.osz') + service_manager._save_lite = False + service_manager.service_items = [] + service_manager.service_theme = 'Default' + mocked_save_file_as.return_value = True + mocked_zipfile.ZipFile.return_value = MagicMock() + mocked_shutil_copy.side_effect = PermissionError + + # WHEN: The service is saved and a PermissionError is thrown + result = service_manager.save_local_file() + + # THEN: The "save_as" method is called to save the service + self.assertTrue(result) + mocked_save_file_as.assert_called_with() + From 97d65864623b308021d9a1a628e962e8d1403998 Mon Sep 17 00:00:00 2001 From: Raoul Snyman Date: Sun, 17 Jul 2016 21:12:00 +0200 Subject: [PATCH 15/21] Fixed another part of the permission denied error --- openlp/core/ui/servicemanager.py | 2 +- .../openlp_core_ui/test_servicemanager.py | 31 ++++++++++++++++++- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/openlp/core/ui/servicemanager.py b/openlp/core/ui/servicemanager.py index 7c01b4191..578217ea6 100644 --- a/openlp/core/ui/servicemanager.py +++ b/openlp/core/ui/servicemanager.py @@ -598,7 +598,7 @@ class ServiceManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ServiceMa if success: try: shutil.copy(temp_file_name, path_file_name) - except shutil.Error: + except (shutil.Error, PermissionError): return self.save_file_as() except OSError as ose: QtWidgets.QMessageBox.critical(self, translate('OpenLP.ServiceManager', 'Error Saving File'), diff --git a/tests/functional/openlp_core_ui/test_servicemanager.py b/tests/functional/openlp_core_ui/test_servicemanager.py index 1697eb1f6..5a337d786 100644 --- a/tests/functional/openlp_core_ui/test_servicemanager.py +++ b/tests/functional/openlp_core_ui/test_servicemanager.py @@ -625,7 +625,7 @@ class TestServiceManager(TestCase): @patch(u'openlp.core.ui.servicemanager.shutil.copy') @patch(u'openlp.core.ui.servicemanager.zipfile') @patch(u'openlp.core.ui.servicemanager.ServiceManager.save_file_as') - def test_save_raises_permission_error(self, mocked_save_file_as, mocked_zipfile, mocked_shutil_copy): + def test_save_file_raises_permission_error(self, mocked_save_file_as, mocked_zipfile, mocked_shutil_copy): """ Test that when a PermissionError is raised when trying to save a file, it is handled correctly """ @@ -639,6 +639,35 @@ class TestServiceManager(TestCase): service_manager._save_lite = False service_manager.service_items = [] service_manager.service_theme = 'Default' + service_manager.service_manager_list = MagicMock() + mocked_save_file_as.return_value = True + mocked_zipfile.ZipFile.return_value = MagicMock() + mocked_shutil_copy.side_effect = PermissionError + + # WHEN: The service is saved and a PermissionError is thrown + result = service_manager.save_file() + + # THEN: The "save_as" method is called to save the service + self.assertTrue(result) + mocked_save_file_as.assert_called_with() + + @patch(u'openlp.core.ui.servicemanager.shutil.copy') + @patch(u'openlp.core.ui.servicemanager.zipfile') + @patch(u'openlp.core.ui.servicemanager.ServiceManager.save_file_as') + def test_save_local_file_raises_permission_error(self, mocked_save_file_as, mocked_zipfile, mocked_shutil_copy): + """ + Test that when a PermissionError is raised when trying to save a local file, it is handled correctly + """ + # GIVEN: A service manager, a service to save + mocked_main_window = MagicMock() + mocked_main_window.service_manager_settings_section = 'servicemanager' + Registry().register('main_window', mocked_main_window) + Registry().register('application', MagicMock()) + service_manager = ServiceManager(None) + service_manager._file_name = os.path.join('temp', 'filename.osz') + service_manager._save_lite = False + service_manager.service_items = [] + service_manager.service_theme = 'Default' mocked_save_file_as.return_value = True mocked_zipfile.ZipFile.return_value = MagicMock() mocked_shutil_copy.side_effect = PermissionError From 65fe62d69f1ae10ffc3c11676bbccc34106caa5b Mon Sep 17 00:00:00 2001 From: Raoul Snyman Date: Sun, 17 Jul 2016 21:33:10 +0200 Subject: [PATCH 16/21] Fix some linting issues --- .../openlp_core_ui/test_servicemanager.py | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/tests/functional/openlp_core_ui/test_servicemanager.py b/tests/functional/openlp_core_ui/test_servicemanager.py index 5a337d786..853baf1f0 100644 --- a/tests/functional/openlp_core_ui/test_servicemanager.py +++ b/tests/functional/openlp_core_ui/test_servicemanager.py @@ -305,18 +305,18 @@ class TestServiceManager(TestCase): self.assertEqual(service_manager.time_action.setVisible.call_count, 1, 'Should have be called once') self.assertEqual(service_manager.auto_start_action.setVisible.call_count, 1, 'Should have be called once') self.assertEqual(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 1, - 'Should have be called once') + 'Should have be called once') self.assertEqual(service_manager.auto_play_slides_once.setChecked.call_count, 0, 'Should not be called') self.assertEqual(service_manager.auto_play_slides_loop.setChecked.call_count, 0, 'Should not be called') self.assertEqual(service_manager.timed_slide_interval.setChecked.call_count, 0, 'Should not be called') self.assertEqual(service_manager.theme_menu.menuAction().setVisible.call_count, 2, - 'Should have be called twice') + 'Should have be called twice') # THEN we add a 2nd display frame service_item._display_frames.append(MagicMock()) service_manager.context_menu(1) # THEN the following additional calls should have occurred. self.assertEqual(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 2, - 'Should have be called twice') + 'Should have be called twice') self.assertEqual(service_manager.auto_play_slides_once.setChecked.call_count, 1, 'Should have be called once') self.assertEqual(service_manager.auto_play_slides_loop.setChecked.call_count, 1, 'Should have be called once') self.assertEqual(service_manager.timed_slide_interval.setChecked.call_count, 1, 'Should have be called once') @@ -368,18 +368,18 @@ class TestServiceManager(TestCase): self.assertEqual(service_manager.time_action.setVisible.call_count, 1, 'Should have be called once') self.assertEqual(service_manager.auto_start_action.setVisible.call_count, 1, 'Should have be called once') self.assertEqual(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 1, - 'Should have be called once') + 'Should have be called once') self.assertEqual(service_manager.auto_play_slides_once.setChecked.call_count, 0, 'Should not be called') self.assertEqual(service_manager.auto_play_slides_loop.setChecked.call_count, 0, 'Should not be called') self.assertEqual(service_manager.timed_slide_interval.setChecked.call_count, 0, 'Should not be called') self.assertEqual(service_manager.theme_menu.menuAction().setVisible.call_count, 1, - 'Should have be called once') + 'Should have be called once') # THEN we add a 2nd display frame and regenerate the menu. service_item._raw_frames.append(MagicMock()) service_manager.context_menu(1) # THEN the following additional calls should have occurred. self.assertEqual(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 2, - 'Should have be called twice') + 'Should have be called twice') self.assertEqual(service_manager.auto_play_slides_once.setChecked.call_count, 1, 'Should have be called once') self.assertEqual(service_manager.auto_play_slides_loop.setChecked.call_count, 1, 'Should have be called once') self.assertEqual(service_manager.timed_slide_interval.setChecked.call_count, 1, 'Should have be called once') @@ -429,12 +429,12 @@ class TestServiceManager(TestCase): self.assertEqual(service_manager.time_action.setVisible.call_count, 1, 'Should have be called once') self.assertEqual(service_manager.auto_start_action.setVisible.call_count, 2, 'Should have be called twice') self.assertEqual(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 1, - 'Should have be called once') + 'Should have be called once') self.assertEqual(service_manager.auto_play_slides_once.setChecked.call_count, 0, 'Should not be called') self.assertEqual(service_manager.auto_play_slides_loop.setChecked.call_count, 0, 'Should not be called') self.assertEqual(service_manager.timed_slide_interval.setChecked.call_count, 0, 'Should not be called') self.assertEqual(service_manager.theme_menu.menuAction().setVisible.call_count, 1, - 'Should have be called once') + 'Should have be called once') # THEN I change the length of the media and regenerate the menu. service_item.set_media_length(5) service_manager.context_menu(1) @@ -487,12 +487,12 @@ class TestServiceManager(TestCase): self.assertEqual(service_manager.time_action.setVisible.call_count, 1, 'Should have be called once') self.assertEqual(service_manager.auto_start_action.setVisible.call_count, 1, 'Should have be called once') self.assertEqual(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 1, - 'Should have be called once') + 'Should have be called once') self.assertEqual(service_manager.auto_play_slides_once.setChecked.call_count, 0, 'Should not be called') self.assertEqual(service_manager.auto_play_slides_loop.setChecked.call_count, 0, 'Should not be called') self.assertEqual(service_manager.timed_slide_interval.setChecked.call_count, 0, 'Should not be called') self.assertEqual(service_manager.theme_menu.menuAction().setVisible.call_count, 1, - 'Should have be called once') + 'Should have be called once') def test_build_presentation_non_pdf_context_menu(self): """ @@ -537,12 +537,12 @@ class TestServiceManager(TestCase): self.assertEqual(service_manager.time_action.setVisible.call_count, 1, 'Should have be called once') self.assertEqual(service_manager.auto_start_action.setVisible.call_count, 1, 'Should have be called once') self.assertEqual(service_manager.auto_play_slides_menu.menuAction().setVisible.call_count, 1, - 'Should have be called once') + 'Should have be called once') self.assertEqual(service_manager.auto_play_slides_once.setChecked.call_count, 0, 'Should not be called') self.assertEqual(service_manager.auto_play_slides_loop.setChecked.call_count, 0, 'Should not be called') self.assertEqual(service_manager.timed_slide_interval.setChecked.call_count, 0, 'Should not be called') self.assertEqual(service_manager.theme_menu.menuAction().setVisible.call_count, 1, - 'Should have be called once') + 'Should have be called once') @patch(u'openlp.core.ui.servicemanager.Settings') @patch(u'PyQt5.QtCore.QTimer.singleShot') @@ -606,7 +606,8 @@ class TestServiceManager(TestCase): # WHEN: on_single_click_preview() is called service_manager.on_single_click_preview_timeout() # THEN: make_preview() should have been called - self.assertEqual(mocked_make_preview.call_count, 1, 'ServiceManager.make_preview() should have been called once') + self.assertEqual(mocked_make_preview.call_count, 1, + 'ServiceManager.make_preview() should have been called once') @patch(u'openlp.core.ui.servicemanager.ServiceManager.make_preview') @patch(u'openlp.core.ui.servicemanager.ServiceManager.make_live') From 785257020dc6fd24e00b119640b132e62058cd78 Mon Sep 17 00:00:00 2001 From: Raoul Snyman Date: Sun, 17 Jul 2016 21:46:06 +0200 Subject: [PATCH 17/21] Fix some linting issues --- tests/functional/openlp_core_ui/test_servicemanager.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/functional/openlp_core_ui/test_servicemanager.py b/tests/functional/openlp_core_ui/test_servicemanager.py index 853baf1f0..3e4af8c97 100644 --- a/tests/functional/openlp_core_ui/test_servicemanager.py +++ b/tests/functional/openlp_core_ui/test_servicemanager.py @@ -679,4 +679,3 @@ class TestServiceManager(TestCase): # THEN: The "save_as" method is called to save the service self.assertTrue(result) mocked_save_file_as.assert_called_with() - From ebefc03674ecb0c59d715b19cc78233c724aa325 Mon Sep 17 00:00:00 2001 From: Raoul Snyman Date: Sat, 23 Jul 2016 23:41:24 +0200 Subject: [PATCH 18/21] Fix the disappearance of a variable :-) --- openlp/core/lib/theme.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/openlp/core/lib/theme.py b/openlp/core/lib/theme.py index fc20037e7..c27b08cd0 100644 --- a/openlp/core/lib/theme.py +++ b/openlp/core/lib/theme.py @@ -474,6 +474,7 @@ class ThemeXML(object): if element.startswith('shadow') or element.startswith('outline'): master = 'font_main' # fix bold font + ret_value = None if element == 'weight': element = 'bold' if value == 'Normal': @@ -482,7 +483,7 @@ class ThemeXML(object): ret_value = True if element == 'proportion': element = 'size' - return False, master, element, ret_value + return False, master, element, ret_value if ret_value is not None else value def _create_attr(self, master, element, value): """ From 031ae9ebc181765c1e70e47cda5a13da86d92b1b Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Sun, 24 Jul 2016 21:49:29 +0200 Subject: [PATCH 19/21] Use BibleGateway standard site instead of the legacy site. Fixes bug 1562384. Fixes: https://launchpad.net/bugs/1562384 --- openlp/plugins/bibles/lib/http.py | 10 +++++----- .../openlp_plugins/bibles/test_lib_http.py | 18 +++++++++++------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/openlp/plugins/bibles/lib/http.py b/openlp/plugins/bibles/lib/http.py index fce5d3285..392cce05a 100644 --- a/openlp/plugins/bibles/lib/http.py +++ b/openlp/plugins/bibles/lib/http.py @@ -252,7 +252,7 @@ class BGExtract(RegistryProperties): chapter=chapter, version=version) soup = get_soup_for_bible_ref( - 'http://legacy.biblegateway.com/passage/?{url}'.format(url=url_params), + 'http://biblegateway.com/passage/?{url}'.format(url=url_params), pre_parse_regex=r'', pre_parse_substitute='') if not soup: return None @@ -281,7 +281,7 @@ class BGExtract(RegistryProperties): """ log.debug('BGExtract.get_books_from_http("{version}")'.format(version=version)) url_params = urllib.parse.urlencode({'action': 'getVersionInfo', 'vid': '{version}'.format(version=version)}) - reference_url = 'http://legacy.biblegateway.com/versions/?{url}#books'.format(url=url_params) + reference_url = 'http://biblegateway.com/versions/?{url}#books'.format(url=url_params) page = get_web_page(reference_url) if not page: send_error_message('download') @@ -312,7 +312,7 @@ class BGExtract(RegistryProperties): for book in content: book = book.find('td') if book: - books.append(book.contents[0]) + books.append(book.contents[1]) return books def get_bibles_from_http(self): @@ -322,11 +322,11 @@ class BGExtract(RegistryProperties): returns a list in the form [(biblename, biblekey, language_code)] """ log.debug('BGExtract.get_bibles_from_http') - bible_url = 'https://legacy.biblegateway.com/versions/' + bible_url = 'https://biblegateway.com/versions/' soup = get_soup_for_bible_ref(bible_url) if not soup: return None - bible_select = soup.find('select', {'class': 'translation-dropdown'}) + bible_select = soup.find('select', {'class': 'search-translation-select'}) if not bible_select: log.debug('No select tags found - did site change?') return None diff --git a/tests/interfaces/openlp_plugins/bibles/test_lib_http.py b/tests/interfaces/openlp_plugins/bibles/test_lib_http.py index 4ca4a8b0f..de4d4bba8 100644 --- a/tests/interfaces/openlp_plugins/bibles/test_lib_http.py +++ b/tests/interfaces/openlp_plugins/bibles/test_lib_http.py @@ -50,7 +50,8 @@ class TestBibleHTTP(TestCase): books = handler.get_books_from_http('NIV') # THEN: We should get back a valid service item - assert len(books) == 66, 'The bible should not have had any books added or removed' + self.assertEqual(len(books), 66, 'The bible should not have had any books added or removed') + self.assertEqual(books[0], 'Genesis', 'The first bible book should be Genesis') def test_bible_gateway_extract_books_support_redirect(self): """ @@ -63,7 +64,7 @@ class TestBibleHTTP(TestCase): books = handler.get_books_from_http('DN1933') # THEN: We should get back a valid service item - assert len(books) == 66, 'This bible should have 66 books' + self.assertEqual(len(books), 66, 'This bible should have 66 books') def test_bible_gateway_extract_verse(self): """ @@ -76,7 +77,8 @@ class TestBibleHTTP(TestCase): results = handler.get_bible_chapter('NIV', 'John', 3) # THEN: We should get back a valid service item - assert len(results.verse_list) == 36, 'The book of John should not have had any verses added or removed' + self.assertEqual(len(results.verse_list), 36, + 'The book of John should not have had any verses added or removed') def test_bible_gateway_extract_verse_nkjv(self): """ @@ -89,7 +91,8 @@ class TestBibleHTTP(TestCase): results = handler.get_bible_chapter('NKJV', 'John', 3) # THEN: We should get back a valid service item - assert len(results.verse_list) == 36, 'The book of John should not have had any verses added or removed' + self.assertEqual(len(results.verse_list), 36, + 'The book of John should not have had any verses added or removed') def test_crosswalk_extract_books(self): """ @@ -102,7 +105,7 @@ class TestBibleHTTP(TestCase): books = handler.get_books_from_http('niv') # THEN: We should get back a valid service item - assert len(books) == 66, 'The bible should not have had any books added or removed' + self.assertEqual(len(books), 66, 'The bible should not have had any books added or removed') def test_crosswalk_extract_verse(self): """ @@ -115,7 +118,8 @@ class TestBibleHTTP(TestCase): results = handler.get_bible_chapter('niv', 'john', 3) # THEN: We should get back a valid service item - assert len(results.verse_list) == 36, 'The book of John should not have had any verses added or removed' + self.assertEqual(len(results.verse_list), 36, + 'The book of John should not have had any verses added or removed') def test_bibleserver_get_bibles(self): """ @@ -144,7 +148,7 @@ class TestBibleHTTP(TestCase): # THEN: The list should not be None, and some known bibles should be there self.assertIsNotNone(bibles) - self.assertIn(('Holman Christian Standard Bible', 'HCSB', 'en'), bibles) + self.assertIn(('Holman Christian Standard Bible (HCSB)', 'HCSB', 'en'), bibles) def test_crosswalk_get_bibles(self): """ From ea455d9b3201f485cbf3ff38bcf9877b1b58f810 Mon Sep 17 00:00:00 2001 From: Raoul Snyman Date: Sun, 24 Jul 2016 22:41:27 +0200 Subject: [PATCH 20/21] Write some tests --- .../functional/openlp_core_lib/test_theme.py | 95 +++++++++++++------ 1 file changed, 67 insertions(+), 28 deletions(-) diff --git a/tests/functional/openlp_core_lib/test_theme.py b/tests/functional/openlp_core_lib/test_theme.py index c006abb78..abacface8 100644 --- a/tests/functional/openlp_core_lib/test_theme.py +++ b/tests/functional/openlp_core_lib/test_theme.py @@ -22,43 +22,82 @@ """ Package to test the openlp.core.lib.theme package. """ -from tests.functional import MagicMock, patch from unittest import TestCase +import os from openlp.core.lib.theme import ThemeXML -class TestTheme(TestCase): +class TestThemeXML(TestCase): """ - Test the functions in the Theme module + Test the ThemeXML class """ - def setUp(self): - """ - Create the UI - """ - pass - - def tearDown(self): - """ - Delete all the C++ objects at the end so that we don't have a segfault - """ - pass - def test_new_theme(self): """ - Test the theme creation - basic test + Test the ThemeXML constructor """ - # GIVEN: A new theme - - # WHEN: A theme is created + # GIVEN: The ThemeXML class + # WHEN: A theme object is created default_theme = ThemeXML() - # 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', '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') - self.assertTrue(default_theme.font_footer_name == "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') + # THEN: The default values should be correct + self.assertEqual('#000000', default_theme.background_border_color, 'background_border_color should be "#000000"') + self.assertEqual('solid', default_theme.background_type, 'background_type should be "solid"') + self.assertEqual(0, default_theme.display_vertical_align, 'display_vertical_align should be 0') + self.assertEqual('Arial', default_theme.font_footer_name, 'font_footer_name should be "Arial"') + self.assertFalse(default_theme.font_main_bold, 'font_main_bold should be False') + self.assertEqual(47, len(default_theme.__dict__), 'The theme should have 47 attributes') + + def test_expand_json(self): + """ + Test the expand_json method + """ + # GIVEN: A ThemeXML object and some JSON to "expand" + theme = ThemeXML() + theme_json = { + 'background': { + 'border_color': '#000000', + 'type': 'solid' + }, + 'display': { + 'vertical_align': 0 + }, + 'font': { + 'footer': { + 'bold': False + }, + 'main': { + 'name': 'Arial' + } + } + } + + # WHEN: ThemeXML.expand_json() is run + theme.expand_json(theme_json) + + # THEN: The attributes should be set on the object + self.assertEqual('#000000', theme.background_border_color, 'background_border_color should be "#000000"') + self.assertEqual('solid', theme.background_type, 'background_type should be "solid"') + self.assertEqual(0, theme.display_vertical_align, 'display_vertical_align should be 0') + self.assertFalse(theme.font_footer_bold, 'font_footer_bold should be False') + self.assertEqual('Arial', theme.font_main_name, 'font_main_name should be "Arial"') + + def test_extend_image_filename(self): + """ + Test the extend_image_filename method + """ + # GIVEN: A theme object + theme = ThemeXML() + theme.theme_name = 'MyBeautifulTheme ' + theme.background_filename = ' video.mp4' + theme.background_type = 'video' + path = os.path.expanduser('~') + + # WHEN: ThemeXML.extend_image_filename is run + theme.extend_image_filename(path) + + # THEN: The filename of the background should be correct + expected_filename = os.path.join(path, 'MyBeautifulTheme', 'video.mp4') + self.assertEqual(expected_filename, theme.background_filename) + self.assertEqual('MyBeautifulTheme', theme.theme_name) + From 0dcae022bcf59f06bfb630605854153903bf0348 Mon Sep 17 00:00:00 2001 From: Raoul Snyman Date: Sun, 24 Jul 2016 22:49:00 +0200 Subject: [PATCH 21/21] some pep8 fixes --- 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 abacface8..db6b78d02 100644 --- a/tests/functional/openlp_core_lib/test_theme.py +++ b/tests/functional/openlp_core_lib/test_theme.py @@ -41,7 +41,8 @@ class TestThemeXML(TestCase): default_theme = ThemeXML() # THEN: The default values should be correct - self.assertEqual('#000000', default_theme.background_border_color, 'background_border_color should be "#000000"') + self.assertEqual('#000000', default_theme.background_border_color, + 'background_border_color should be "#000000"') self.assertEqual('solid', default_theme.background_type, 'background_type should be "solid"') self.assertEqual(0, default_theme.display_vertical_align, 'display_vertical_align should be 0') self.assertEqual('Arial', default_theme.font_footer_name, 'font_footer_name should be "Arial"') @@ -100,4 +101,3 @@ class TestThemeXML(TestCase): expected_filename = os.path.join(path, 'MyBeautifulTheme', 'video.mp4') self.assertEqual(expected_filename, theme.background_filename) self.assertEqual('MyBeautifulTheme', theme.theme_name) -