openlp-core team mailing list archive
-
openlp-core team
-
Mailing list archive
-
Message #23192
[Merge] lp:~sam92/openlp/bug-1247493 into lp:openlp
Samuel Mehrbrodt has proposed merging lp:~sam92/openlp/bug-1247493 into lp:openlp.
Requested reviews:
OpenLP Core (openlp-core)
Related bugs:
Bug #1247493 in OpenLP: "nightly build 2286 can't import Opensong NIV Bible"
https://bugs.launchpad.net/openlp/+bug/1247493
For more details, see:
https://code.launchpad.net/~sam92/openlp/bug-1247493/+merge/216201
Fix bug 1247493: OpenSong Importer fails
--
https://code.launchpad.net/~sam92/openlp/bug-1247493/+merge/216201
Your team OpenLP Core is requested to review the proposed merge of lp:~sam92/openlp/bug-1247493 into lp:openlp.
=== modified file 'openlp/plugins/bibles/lib/opensong.py'
--- openlp/plugins/bibles/lib/opensong.py 2014-03-09 10:26:28 +0000
+++ openlp/plugins/bibles/lib/opensong.py 2014-04-16 20:02:01 +0000
@@ -73,13 +73,13 @@
log.debug('Starting OpenSong import from "%s"' % self.filename)
if not isinstance(self.filename, str):
self.filename = str(self.filename, 'utf8')
- file = None
+ import_file = None
success = True
try:
# NOTE: We don't need to do any of the normal encoding detection here, because lxml does it's own encoding
# detection, and the two mechanisms together interfere with each other.
- file = open(self.filename, 'r')
- opensong = objectify.parse(file)
+ import_file = open(self.filename, 'rb')
+ opensong = objectify.parse(import_file)
bible = opensong.getroot()
language_id = self.get_language(bible_name)
if not language_id:
@@ -93,7 +93,7 @@
log.error('Importing books from "%s" failed' % self.filename)
return False
book_details = BiblesResourcesDB.get_book_by_id(book_ref_id)
- db_book = self.create_book(str(book.attrib['n']), book_ref_id, book_details['testament_id'])
+ db_book = self.create_book(book.attrib['n'], book_ref_id, book_details['testament_id'])
chapter_number = 0
for chapter in book.c:
if self.stop_import_flag:
@@ -122,8 +122,8 @@
verse_number += 1
self.create_verse(db_book.id, chapter_number, verse_number, self.get_text(verse))
self.wizard.increment_progress_bar(
- translate('BiblesPlugin.Opensong', 'Importing %s %s...',
- 'Importing <book name> <chapter>...')) % (db_book.name, chapter_number)
+ translate('BiblesPlugin.Opensong', 'Importing %(bookname)s %(chapter)s...' %
+ {'bookname': db_book.name, 'chapter': chapter_number}))
self.session.commit()
self.application.process_events()
except etree.XMLSyntaxError as inst:
@@ -137,8 +137,8 @@
log.exception('Loading Bible from OpenSong file failed')
success = False
finally:
- if file:
- file.close()
+ if import_file:
+ import_file.close()
if self.stop_import_flag:
return False
else:
=== modified file 'openlp/plugins/songs/lib/songbeamerimport.py'
--- openlp/plugins/songs/lib/songbeamerimport.py 2014-04-13 17:03:28 +0000
+++ openlp/plugins/songs/lib/songbeamerimport.py 2014-04-16 20:02:01 +0000
@@ -137,7 +137,7 @@
if line.startswith('#') and not read_verses:
self.parseTags(line)
elif line.startswith('--'):
- # --- and -- allowed for page-breaks (difference in Songbeamer only in printout)
+ # --- and -- allowed for page-breaks (difference in Songbeamer only in printout)
if self.current_verse:
self.replace_html_tags()
self.add_verse(self.current_verse, self.current_verse_type)
=== modified file 'scripts/translation_utils.py'
--- scripts/translation_utils.py 2014-04-02 18:51:21 +0000
+++ scripts/translation_utils.py 2014-04-16 20:02:01 +0000
@@ -96,7 +96,7 @@
return len(self.data)
def __getitem__(self, index):
- if not index in self.data:
+ if index not in self.data:
return None
elif self.data[index].get('arguments'):
return self.data[index]['command'], self.data[index]['arguments']
=== modified file 'tests/functional/openlp_core_common/test_common.py'
--- tests/functional/openlp_core_common/test_common.py 2014-04-04 21:12:44 +0000
+++ tests/functional/openlp_core_common/test_common.py 2014-04-16 20:02:01 +0000
@@ -79,5 +79,5 @@
trace_error_handler(mocked_logger)
# THEN: The mocked_logger.error() method should have been called with the correct parameters
- mocked_logger.error.assert_called_with('OpenLP Error trace\n File openlp.fake at line 56 \n\t called trace_error_handler_test')
-
+ mocked_logger.error.assert_called_with(
+ 'OpenLP Error trace\n File openlp.fake at line 56 \n\t called trace_error_handler_test')
=== modified file 'tests/functional/openlp_core_lib/test_file_dialog.py'
--- tests/functional/openlp_core_lib/test_file_dialog.py 2014-04-02 18:51:21 +0000
+++ tests/functional/openlp_core_lib/test_file_dialog.py 2014-04-16 20:02:01 +0000
@@ -54,7 +54,7 @@
self.mocked_qt_gui.reset()
# GIVEN: A List of known values as a return value from QFileDialog.getOpenFileNames and a list of valid
- # file names.
+ # file names.
self.mocked_qt_gui.QFileDialog.getOpenFileNames.return_value = [
'/Valid File', '/url%20encoded%20file%20%231', '/non-existing']
self.mocked_os.path.exists.side_effect = lambda file_name: file_name in [
=== modified file 'tests/functional/openlp_core_lib/test_lib.py'
--- tests/functional/openlp_core_lib/test_lib.py 2014-04-02 18:51:21 +0000
+++ tests/functional/openlp_core_lib/test_lib.py 2014-04-16 20:02:01 +0000
@@ -482,7 +482,7 @@
# WHEN: we run the validate_thumb() function
# THEN: we should have called a few functions, and the result should be True
- #mocked_os.path.exists.assert_called_with(thumb_path)
+ # mocked_os.path.exists.assert_called_with(thumb_path)
def validate_thumb_file_exists_and_older_test(self):
"""
=== modified file 'tests/functional/openlp_core_lib/test_ui.py'
--- tests/functional/openlp_core_lib/test_ui.py 2014-04-15 21:11:06 +0000
+++ tests/functional/openlp_core_lib/test_ui.py 2014-04-16 20:02:01 +0000
@@ -162,7 +162,7 @@
# WHEN: We create an action with some properties
action = create_action(dialog, 'my_action', text='my text', icon=':/wizards/wizard_firsttime.bmp',
- tooltip='my tooltip', statustip='my statustip')
+ tooltip='my tooltip', statustip='my statustip')
# THEN: These properties should be set
self.assertIsInstance(action, QtGui.QAction)
=== modified file 'tests/functional/openlp_core_ui/test_formattingtagsform.py'
--- tests/functional/openlp_core_ui/test_formattingtagsform.py 2014-04-02 19:35:09 +0000
+++ tests/functional/openlp_core_ui/test_formattingtagsform.py 2014-04-16 20:02:01 +0000
@@ -70,7 +70,7 @@
form.save_button = MagicMock()
# WHEN: on_text_edited is called with an arbitrary value
- #form.on_text_edited('text')
+ # form.on_text_edited('text')
# THEN: setEnabled and setDefault should have been called on save_push_button
- #form.save_button.setEnabled.assert_called_with(True)
+ # form.save_button.setEnabled.assert_called_with(True)
=== modified file 'tests/functional/openlp_core_ui/test_maindisplay.py'
--- tests/functional/openlp_core_ui/test_maindisplay.py 2014-04-08 15:23:40 +0000
+++ tests/functional/openlp_core_ui/test_maindisplay.py 2014-04-16 20:02:01 +0000
@@ -106,4 +106,4 @@
self.assertEqual('QGraphicsView {}', main_display.styleSheet(),
'MainDisplay instance should not be transparent')
self.assertFalse(main_display.testAttribute(QtCore.Qt.WA_TranslucentBackground),
- 'MainDisplay hasnt translucent background')
+ 'MainDisplay hasnt translucent background')
=== modified file 'tests/functional/openlp_plugins/bibles/test_http.py'
--- tests/functional/openlp_plugins/bibles/test_http.py 2014-04-15 05:28:51 +0000
+++ tests/functional/openlp_plugins/bibles/test_http.py 2014-04-16 20:02:01 +0000
@@ -35,7 +35,7 @@
from tests.functional import patch, MagicMock
from openlp.plugins.bibles.lib.http import BSExtract
-#TODO: Items left to test
+# TODO: Items left to test
# BGExtract
# __init__
# _remove_elements
@@ -68,7 +68,7 @@
"""
Test the BSExtractClass
"""
- #TODO: Items left to test
+ # TODO: Items left to test
# BSExtract
# __init__
# get_bible_chapter
=== modified file 'tests/functional/openlp_plugins/presentations/test_pptviewcontroller.py'
--- tests/functional/openlp_plugins/presentations/test_pptviewcontroller.py 2014-04-02 19:35:09 +0000
+++ tests/functional/openlp_plugins/presentations/test_pptviewcontroller.py 2014-04-16 20:02:01 +0000
@@ -47,7 +47,7 @@
"""
Test the PptviewController Class
"""
-#TODO: Items left to test
+# TODO: Items left to test
# PptviewController
# start_process(self)
# kill
@@ -108,7 +108,7 @@
"""
Test the PptviewDocument Class
"""
- #TODO: Items left to test
+ # TODO: Items left to test
# PptviewDocument
# __init__
# create_thumbnails
=== modified file 'tests/functional/openlp_plugins/songs/test_ewimport.py'
--- tests/functional/openlp_plugins/songs/test_ewimport.py 2014-04-15 05:28:51 +0000
+++ tests/functional/openlp_plugins/songs/test_ewimport.py 2014-04-16 20:02:01 +0000
@@ -141,7 +141,7 @@
self.assertIsNotNone(field_desc_entry, 'Import should not be none')
self.assertEqual(field_desc_entry.name, name, 'FieldDescEntry.name should be the same as the name argument')
self.assertEqual(field_desc_entry.field_type, field_type,
- 'FieldDescEntry.type should be the same as the type argument')
+ 'FieldDescEntry.type should be the same as the type argument')
self.assertEqual(field_desc_entry.size, size, 'FieldDescEntry.size should be the same as the size argument')
def create_importer_test(self):
@@ -229,10 +229,10 @@
for field_index, result in field_results:
return_value = importer.get_field(field_index)
- # THEN: get_field should return the known results
+ # THEN: get_field should return the known results
self.assertEqual(return_value, result,
- 'get_field should return "%s" when called with "%s"' %
- (result, TEST_FIELDS[field_index]))
+ 'get_field should return "%s" when called with "%s"' %
+ (result, TEST_FIELDS[field_index]))
def get_memo_field_test(self):
"""
@@ -403,11 +403,11 @@
if song_copyright:
self.assertEqual(importer.copyright, song_copyright)
if ccli_number:
- self.assertEqual(importer.ccli_number, ccli_number, 'ccli_number for %s should be %s'
- % (title, ccli_number))
+ self.assertEqual(importer.ccli_number, ccli_number,
+ 'ccli_number for %s should be %s' % (title, ccli_number))
for verse_text, verse_tag in add_verse_calls:
mocked_add_verse.assert_any_call(verse_text, verse_tag)
if verse_order_list:
self.assertEqual(importer.verse_order_list, verse_order_list,
- 'verse_order_list for %s should be %s' % (title, verse_order_list))
+ 'verse_order_list for %s should be %s' % (title, verse_order_list))
mocked_finish.assert_called_with()
=== modified file 'tests/functional/openlp_plugins/songs/test_foilpresenterimport.py'
--- tests/functional/openlp_plugins/songs/test_foilpresenterimport.py 2014-03-06 20:56:31 +0000
+++ tests/functional/openlp_plugins/songs/test_foilpresenterimport.py 2014-04-16 20:02:01 +0000
@@ -44,7 +44,7 @@
"""
Test the functions in the :mod:`foilpresenterimport` module.
"""
- #TODO: The following modules still need tests written for
+ # TODO: The following modules still need tests written for
# xml_to_song
# _child
# _process_authors
=== modified file 'tests/functional/openlp_plugins/songs/test_lib.py'
--- tests/functional/openlp_plugins/songs/test_lib.py 2014-04-14 19:36:07 +0000
+++ tests/functional/openlp_plugins/songs/test_lib.py 2014-04-16 20:02:01 +0000
@@ -129,7 +129,6 @@
# THEN: The result should be a tuple of songs..
assert result == (self.song1, self.song2), 'The result should be the tuble of songs'
-
def songs_probably_equal_different_song_test(self):
"""
Test the songs_probably_equal function with two different songs.
=== modified file 'tests/functional/openlp_plugins/songs/test_songbeamerimport.py'
--- tests/functional/openlp_plugins/songs/test_songbeamerimport.py 2014-04-14 18:33:34 +0000
+++ tests/functional/openlp_plugins/songs/test_songbeamerimport.py 2014-04-16 20:02:01 +0000
@@ -49,7 +49,8 @@
('4. Lobsingt seiner Treu´,\ndie immerdar neu,\nbis Er uns zur Herrlichket führet!\n\n', 'v')
],
'song_book_name': 'Glaubenslieder I',
- 'song_number': "1"
+ 'song_number': "1",
+ 'authors': ['Carl Brockhaus', 'Johann Jakob Vetter']
}
}
@@ -91,7 +92,7 @@
# THEN: do_import should return none and the progress bar maximum should not be set.
self.assertIsNone(importer.do_import(), 'do_import should return None when import_source is not a list')
self.assertEqual(mocked_import_wizard.progress_bar.setMaximum.called, False,
- 'setMaxium on import_wizard.progress_bar should not have been called')
+ 'setMaxium on import_wizard.progress_bar should not have been called')
def valid_import_source_test(self):
"""
@@ -140,6 +141,7 @@
add_verse_calls = SONG_TEST_DATA[song_file]['verses']
song_book_name = SONG_TEST_DATA[song_file]['song_book_name']
song_number = SONG_TEST_DATA[song_file]['song_number']
+ song_authors = SONG_TEST_DATA[song_file]['authors']
# THEN: do_import should return none, the song data should be as expected, and finish should have been
# called.
@@ -148,11 +150,14 @@
for verse_text, verse_tag in add_verse_calls:
mocked_add_verse.assert_any_call(verse_text, verse_tag)
if song_book_name:
- self.assertEqual(importer.song_book_name, song_book_name, 'song_book_name for %s should be "%s"' %
- (song_file, song_book_name))
+ self.assertEqual(importer.song_book_name, song_book_name,
+ 'song_book_name for %s should be "%s"' % (song_file, song_book_name))
if song_number:
- self.assertEqual(importer.song_number, song_number, 'song_number for %s should be %s' %
- (song_file, song_number))
+ self.assertEqual(importer.song_number, song_number,
+ 'song_number for %s should be %s' % (song_file, song_number))
+ if song_authors:
+ for author in importer.authors:
+ self.assertIn(author, song_authors)
mocked_finish.assert_called_with()
def check_verse_marks_test(self):
=== modified file 'tests/functional/openlp_plugins/songs/test_songshowplusimport.py'
--- tests/functional/openlp_plugins/songs/test_songshowplusimport.py 2014-04-15 05:28:51 +0000
+++ tests/functional/openlp_plugins/songs/test_songshowplusimport.py 2014-04-16 20:02:01 +0000
@@ -96,7 +96,7 @@
# THEN: do_import should return none and the progress bar maximum should not be set.
self.assertIsNone(importer.do_import(), 'do_import should return None when import_source is not a list')
self.assertEqual(mocked_import_wizard.progress_bar.setMaximum.called, False,
- 'setMaximum on import_wizard.progress_bar should not have been called')
+ 'setMaximum on import_wizard.progress_bar should not have been called')
def valid_import_source_test(self):
"""
@@ -116,7 +116,7 @@
# THEN: do_import should return none and the progress bar setMaximum should be called with the length of
# import_source.
self.assertIsNone(importer.do_import(), 'do_import should return None when import_source is a list '
- 'and stop_import_flag is True')
+ 'and stop_import_flag is True')
mocked_import_wizard.progress_bar.setMaximum.assert_called_with(len(importer.import_source))
def to_openlp_verse_tag_test(self):
@@ -144,8 +144,8 @@
# THEN: The returned value should should correlate with the input arguments
for original_tag, openlp_tag in test_values:
self.assertEqual(importer.to_openlp_verse_tag(original_tag), openlp_tag,
- 'SongShowPlusImport.to_openlp_verse_tag should return "%s" when called with "%s"' %
- (openlp_tag, original_tag))
+ 'SongShowPlusImport.to_openlp_verse_tag should return "%s" when called with "%s"' %
+ (openlp_tag, original_tag))
def to_openlp_verse_tag_verse_order_test(self):
"""
@@ -173,5 +173,5 @@
# THEN: The returned value should should correlate with the input arguments
for original_tag, openlp_tag in test_values:
self.assertEqual(importer.to_openlp_verse_tag(original_tag, ignore_unique=True), openlp_tag,
- 'SongShowPlusImport.to_openlp_verse_tag should return "%s" when called with "%s"' %
- (openlp_tag, original_tag))
+ 'SongShowPlusImport.to_openlp_verse_tag should return "%s" when called with "%s"' %
+ (openlp_tag, original_tag))
=== modified file 'tests/helpers/songfileimport.py'
--- tests/helpers/songfileimport.py 2014-04-15 05:28:51 +0000
+++ tests/helpers/songfileimport.py 2014-04-16 20:02:01 +0000
@@ -116,24 +116,24 @@
if song_copyright:
self.mocked_add_copyright.assert_called_with(song_copyright)
if ccli_number:
- self.assertEqual(importer.ccli_number, ccli_number, 'ccli_number for %s should be %s' %
- (source_file_name, ccli_number))
+ self.assertEqual(importer.ccli_number, ccli_number,
+ 'ccli_number for %s should be %s' % (source_file_name, ccli_number))
for verse_text, verse_tag in add_verse_calls:
self.mocked_add_verse.assert_any_call(verse_text, verse_tag)
if topics:
self.assertEqual(importer.topics, topics, 'topics for %s should be %s' % (source_file_name, topics))
if comments:
- self.assertEqual(importer.comments, comments, 'comments for %s should be "%s"' %
- (source_file_name, comments))
+ self.assertEqual(importer.comments, comments,
+ 'comments for %s should be "%s"' % (source_file_name, comments))
if song_book_name:
- self.assertEqual(importer.song_book_name, song_book_name, 'song_book_name for %s should be "%s"' %
- (source_file_name, song_book_name))
+ self.assertEqual(importer.song_book_name, song_book_name,
+ 'song_book_name for %s should be "%s"' % (source_file_name, song_book_name))
if song_number:
- self.assertEqual(importer.song_number, song_number, 'song_number for %s should be %s' %
- (source_file_name, song_number))
+ self.assertEqual(importer.song_number, song_number,
+ 'song_number for %s should be %s' % (source_file_name, song_number))
if verse_order_list:
- self.assertEqual(importer.verse_order_list, [], 'verse_order_list for %s should be %s' %
- (source_file_name, verse_order_list))
+ self.assertEqual(importer.verse_order_list, [],
+ 'verse_order_list for %s should be %s' % (source_file_name, verse_order_list))
self.mocked_finish.assert_called_with()
def _get_data(self, data, key):
=== modified file 'tests/interfaces/openlp_plugins/bibles/test_lib_parse_reference.py'
--- tests/interfaces/openlp_plugins/bibles/test_lib_parse_reference.py 2014-04-15 05:28:51 +0000
+++ tests/interfaces/openlp_plugins/bibles/test_lib_parse_reference.py 2014-04-16 20:02:01 +0000
@@ -103,5 +103,5 @@
# WHEN asking to parse the bible reference
results = parse_reference('1 Timothy 1:1-2:1', self.manager.db_cache['tests'], MagicMock(), 54)
# THEN a verse array should be returned
- self.assertEqual([(54, 1, 1, -1), (54, 2, 1, 1)], results, "The bible verses should matches the expected "
- "results")
+ self.assertEqual([(54, 1, 1, -1), (54, 2, 1, 1)], results,
+ "The bible verses should match the expected results")
=== modified file 'tests/resources/songbeamersongs/Lobsinget dem Herrn.sng'
--- tests/resources/songbeamersongs/Lobsinget dem Herrn.sng 2013-10-06 22:07:49 +0000
+++ tests/resources/songbeamersongs/Lobsinget dem Herrn.sng 2014-04-16 20:02:01 +0000
@@ -1,5 +1,7 @@
#LangCount=1
#Title=GL 1 - Lobsinget dem Herrn
+#Author=Carl Brockhaus
+#Melody=Johann Jakob Vetter
#Editor=SongBeamer 4.20
#Version=3
#Format=F/K//
Follow ups
