openlp-core team mailing list archive
-
openlp-core team
-
Mailing list archive
-
Message #23116
[Merge] lp:~strada/openlp/songbeamer-import-enhancements into lp:openlp
Stefan Strasser has proposed merging lp:~strada/openlp/songbeamer-import-enhancements into lp:openlp.
Requested reviews:
Samuel Mehrbrodt (sam92)
OpenLP Core (openlp-core)
For more details, see:
https://code.launchpad.net/~strada/openlp/songbeamer-import-enhancements/+merge/215812
Some enhancements for songbeamer-import:
- added some missing verse-marks to be recognized by the importer (Misc,Part,Teil,$$M=); all of them set to be translated to VerseType.Other (I would prefer a special marker for Part/Teil but this is currently not existing in openlp)
- special handling for mark $$M= in check_verse_marks because the name of this mark may be different (after the =) and it is not countable; this mark is in songbeamer used for custom marks (on importing from other formats or manual)
- page-break-recognition now accepts also -- (in songbeamer -- and --- are both identically used to separate slides/verses, the only difference is in the layout of a printout)
- added a functional test for the function check_verse_marks (checking the recognition of different possible lines in one test)
- replaced all occurrences in the code of the deprecated alias assertEquals() with assertEqual()
--
https://code.launchpad.net/~strada/openlp/songbeamer-import-enhancements/+merge/215812
Your team OpenLP Core is requested to review the proposed merge of lp:~strada/openlp/songbeamer-import-enhancements into lp:openlp.
=== modified file 'openlp/plugins/songs/lib/songbeamerimport.py'
--- openlp/plugins/songs/lib/songbeamerimport.py 2014-03-29 13:31:28 +0000
+++ openlp/plugins/songs/lib/songbeamerimport.py 2014-04-15 05:48:25 +0000
@@ -56,11 +56,15 @@
'Zwischenspiel': VerseType.tags[VerseType.Bridge],
'Pre-Chorus': VerseType.tags[VerseType.PreChorus],
'Pre-Refrain': VerseType.tags[VerseType.PreChorus],
+ 'Misc': VerseType.tags[VerseType.Other],
'Pre-Bridge': VerseType.tags[VerseType.Other],
'Pre-Coda': VerseType.tags[VerseType.Other],
+ 'Part': VerseType.tags[VerseType.Other],
+ 'Teil': VerseType.tags[VerseType.Other],
'Unbekannt': VerseType.tags[VerseType.Other],
'Unknown': VerseType.tags[VerseType.Other],
- 'Unbenannt': VerseType.tags[VerseType.Other]
+ 'Unbenannt': VerseType.tags[VerseType.Other],
+ '$$M=': VerseType.tags[VerseType.Other]
}
@@ -132,7 +136,8 @@
line = str(line).strip()
if line.startswith('#') and not read_verses:
self.parseTags(line)
- elif line.startswith('---'):
+ elif line.startswith('--'):
+ # --- 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)
@@ -282,4 +287,7 @@
if marks[1].isdigit():
self.current_verse_type += marks[1]
return True
+ elif marks[0].startswith('$$M='): # this verse-mark cannot be numbered
+ self.current_verse_type = SongBeamerTypes.MarkTypes['$$M=']
+ return True
return False
=== modified file 'tests/functional/openlp_core_common/test_registryproperties.py'
--- tests/functional/openlp_core_common/test_registryproperties.py 2014-04-02 19:35:09 +0000
+++ tests/functional/openlp_core_common/test_registryproperties.py 2014-04-15 05:48:25 +0000
@@ -52,7 +52,7 @@
# GIVEN an Empty Registry
# WHEN there is no Application
# THEN the application should be none
- self.assertEquals(self.application, None, 'The application value should be None')
+ self.assertEqual(self.application, None, 'The application value should be None')
def application_test(self):
"""
@@ -63,4 +63,4 @@
# WHEN the application is registered
Registry().register('application', application)
# THEN the application should be none
- self.assertEquals(self.application, application, 'The application value should match')
+ self.assertEqual(self.application, application, 'The application value should match')
=== modified file 'tests/functional/openlp_plugins/bibles/test_http.py'
--- tests/functional/openlp_plugins/bibles/test_http.py 2014-04-02 18:51:21 +0000
+++ tests/functional/openlp_plugins/bibles/test_http.py 2014-04-15 05:48:25 +0000
@@ -180,4 +180,4 @@
'http://m.bibleserver.com/overlay/selectBook?translation=NIV')
self.assertFalse(self.mock_log.error.called, 'log.error should not have been called')
self.assertFalse(self.mock_send_error_message.called, 'send_error_message should not have been called')
- self.assertEquals(result, ['Genesis', 'Leviticus'])
+ self.assertEqual(result, ['Genesis', 'Leviticus'])
=== modified file 'tests/functional/openlp_plugins/songs/test_ewimport.py'
--- tests/functional/openlp_plugins/songs/test_ewimport.py 2014-04-02 19:35:09 +0000
+++ tests/functional/openlp_plugins/songs/test_ewimport.py 2014-04-15 05:48:25 +0000
@@ -139,10 +139,10 @@
# THEN:
self.assertIsNotNone(field_desc_entry, 'Import should not be none')
- self.assertEquals(field_desc_entry.name, name, 'FieldDescEntry.name should be the same as the name argument')
- self.assertEquals(field_desc_entry.field_type, field_type,
+ 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')
- self.assertEquals(field_desc_entry.size, size, 'FieldDescEntry.size should be the same as the size argument')
+ self.assertEqual(field_desc_entry.size, size, 'FieldDescEntry.size should be the same as the size argument')
def create_importer_test(self):
"""
@@ -174,7 +174,7 @@
for field_name in existing_fields:
# THEN: The item corresponding the index returned should have the same name attribute
- self.assertEquals(importer.field_descriptions[importer.find_field(field_name)].name, field_name)
+ self.assertEqual(importer.field_descriptions[importer.find_field(field_name)].name, field_name)
def find_non_existing_field_test(self):
"""
@@ -230,7 +230,7 @@
return_value = importer.get_field(field_index)
# THEN: get_field should return the known results
- self.assertEquals(return_value, result,
+ self.assertEqual(return_value, result,
'get_field should return "%s" when called with "%s"' %
(result, TEST_FIELDS[field_index]))
@@ -257,7 +257,7 @@
get_field_seek_calls = test_results[2]['seek']
# THEN: get_field should return the appropriate value with the appropriate mocked objects being called
- self.assertEquals(importer.get_field(field_index), get_field_result)
+ self.assertEqual(importer.get_field(field_index), get_field_result)
for call in get_field_read_calls:
mocked_memo_file.read.assert_any_call(call)
for call in get_field_seek_calls:
@@ -403,11 +403,11 @@
if song_copyright:
self.assertEqual(importer.copyright, song_copyright)
if ccli_number:
- self.assertEquals(importer.ccli_number, ccli_number, 'ccli_number for %s should be %s'
+ 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.assertEquals(importer.verse_order_list, verse_order_list,
+ self.assertEqual(importer.verse_order_list, 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_songbeamerimport.py'
--- tests/functional/openlp_plugins/songs/test_songbeamerimport.py 2014-04-02 19:35:09 +0000
+++ tests/functional/openlp_plugins/songs/test_songbeamerimport.py 2014-04-15 05:48:25 +0000
@@ -35,6 +35,7 @@
from tests.functional import MagicMock, patch
from openlp.plugins.songs.lib.songbeamerimport import SongBeamerImport
+from openlp.plugins.songs.lib import VerseType
TEST_PATH = os.path.abspath(os.path.join(os.path.dirname(__file__),
'..', '..', '..', 'resources', 'songbeamersongs'))
@@ -89,7 +90,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.assertEquals(mocked_import_wizard.progress_bar.setMaximum.called, False,
+ self.assertEqual(mocked_import_wizard.progress_bar.setMaximum.called, False,
'setMaxium on import_wizard.progress_bar should not have been called')
def valid_import_source_test(self):
@@ -143,13 +144,90 @@
# THEN: do_import should return none, the song data should be as expected, and finish should have been
# called.
self.assertIsNone(importer.do_import(), 'do_import should return None when it has completed')
- self.assertEquals(importer.title, title, 'title for %s should be "%s"' % (song_file, title))
+ self.assertEqual(importer.title, title, 'title for %s should be "%s"' % (song_file, title))
for verse_text, verse_tag in add_verse_calls:
mocked_add_verse.assert_any_call(verse_text, verse_tag)
if song_book_name:
- self.assertEquals(importer.song_book_name, song_book_name, 'song_book_name for %s should be "%s"' %
+ 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.assertEquals(importer.song_number, song_number, 'song_number for %s should be %s' %
+ self.assertEqual(importer.song_number, song_number, 'song_number for %s should be %s' %
(song_file, song_number))
mocked_finish.assert_called_with()
+
+ def check_verse_marks_test(self):
+ """
+ Tests different lines to see if a verse mark is detected or not
+ """
+
+ # GIVEN: line with unnumbered verse-type
+ line = 'Refrain'
+ self.current_verse_type = None
+ # WHEN: line is being checked for verse marks
+ result = SongBeamerImport.check_verse_marks(self, line)
+ # THEN: we should get back true and c as self.current_verse_type
+ self.assertTrue(result, 'Versemark for <Refrain> should be found, value true')
+ self.assertEqual(self.current_verse_type, 'c', '<Refrain> should be interpreted as <c>')
+
+ # GIVEN: line with unnumbered verse-type and trailing space
+ line = 'Refrain '
+ self.current_verse_type = None
+ # WHEN: line is being checked for verse marks
+ result = SongBeamerImport.check_verse_marks(self, line)
+ # THEN: we should get back true and c as self.current_verse_type
+ self.assertTrue(result, 'Versemark for <Refrain > should be found, value true')
+ self.assertEqual(self.current_verse_type, 'c', '<Refrain > should be interpreted as <c>')
+
+ # GIVEN: line with numbered verse-type
+ line = 'Verse 1'
+ self.current_verse_type = None
+ # WHEN: line is being checked for verse marks
+ result = SongBeamerImport.check_verse_marks(self, line)
+ # THEN: we should get back true and v1 as self.current_verse_type
+ self.assertTrue(result, 'Versemark for <Verse 1> should be found, value true')
+ self.assertEqual(self.current_verse_type, 'v1', u'<Verse 1> should be interpreted as <v1>')
+
+ # GIVEN: line with special unnumbered verse-mark (used in Songbeamer to allow usage of non-supported tags)
+ line = '$$M=special'
+ self.current_verse_type = None
+ # WHEN: line is being checked for verse marks
+ result = SongBeamerImport.check_verse_marks(self, line)
+ # THEN: we should get back true and o as self.current_verse_type
+ self.assertTrue(result, 'Versemark for <$$M=special> should be found, value true')
+ self.assertEqual(self.current_verse_type, 'o', u'<$$M=special> should be interpreted as <o>')
+
+ # GIVEN: line with song-text with 3 words
+ line = 'Jesus my saviour'
+ self.current_verse_type = None
+ # WHEN: line is being checked for verse marks
+ result = SongBeamerImport.check_verse_marks(self, line)
+ # THEN: we should get back false and none as self.current_verse_type
+ self.assertFalse(result, 'No versemark for <Jesus my saviour> should be found, value false')
+ self.assertIsNone(self.current_verse_type, '<Jesus my saviour> should be interpreted as none versemark')
+
+ # GIVEN: line with song-text with 2 words
+ line = 'Praise him'
+ self.current_verse_type = None
+ # WHEN: line is being checked for verse marks
+ result = SongBeamerImport.check_verse_marks(self, line)
+ # THEN: we should get back false and none as self.current_verse_type
+ self.assertFalse(result, 'No versemark for <Praise him> should be found, value false')
+ self.assertIsNone(self.current_verse_type, '<Praise him> should be interpreted as none versemark')
+
+ # GIVEN: line with only a space (could occur, nothing regular)
+ line = ' '
+ self.current_verse_type = None
+ # WHEN: line is being checked for verse marks
+ result = SongBeamerImport.check_verse_marks(self, line)
+ # THEN: we should get back false and none as self.current_verse_type
+ self.assertFalse(result, 'No versemark for < > should be found, value false')
+ self.assertIsNone(self.current_verse_type, '< > should be interpreted as none versemark')
+
+ # GIVEN: blank line (could occur, nothing regular)
+ line = ''
+ self.current_verse_type = None
+ # WHEN: line is being checked for verse marks
+ result = SongBeamerImport.check_verse_marks(self, line)
+ # THEN: we should get back false and none as self.current_verse_type
+ self.assertFalse(result, 'No versemark for <> should be found, value false')
+ self.assertIsNone(self.current_verse_type, '<> should be interpreted as none versemark')
=== modified file 'tests/functional/openlp_plugins/songs/test_songshowplusimport.py'
--- tests/functional/openlp_plugins/songs/test_songshowplusimport.py 2014-04-02 19:35:09 +0000
+++ tests/functional/openlp_plugins/songs/test_songshowplusimport.py 2014-04-15 05:48:25 +0000
@@ -95,7 +95,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.assertEquals(mocked_import_wizard.progress_bar.setMaximum.called, False,
+ self.assertEqual(mocked_import_wizard.progress_bar.setMaximum.called, False,
'setMaximum on import_wizard.progress_bar should not have been called')
def valid_import_source_test(self):
@@ -143,7 +143,7 @@
# THEN: The returned value should should correlate with the input arguments
for original_tag, openlp_tag in test_values:
- self.assertEquals(importer.to_openlp_verse_tag(original_tag), openlp_tag,
+ 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))
@@ -172,6 +172,6 @@
# THEN: The returned value should should correlate with the input arguments
for original_tag, openlp_tag in test_values:
- self.assertEquals(importer.to_openlp_verse_tag(original_tag, ignore_unique=True), openlp_tag,
+ 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))
=== modified file 'tests/helpers/songfileimport.py'
--- tests/helpers/songfileimport.py 2014-04-02 19:35:09 +0000
+++ tests/helpers/songfileimport.py 2014-04-15 05:48:25 +0000
@@ -110,29 +110,29 @@
# THEN: do_import should return none, the song data should be as expected, and finish should have been
# called.
self.assertIsNone(importer.do_import(), 'do_import should return None when it has completed')
- self.assertEquals(importer.title, title, 'title for %s should be "%s"' % (source_file_name, title))
+ self.assertEqual(importer.title, title, 'title for %s should be "%s"' % (source_file_name, title))
for author in author_calls:
self.mocked_parse_author.assert_any_call(author)
if song_copyright:
self.mocked_add_copyright.assert_called_with(song_copyright)
if ccli_number:
- self.assertEquals(importer.ccli_number, ccli_number, 'ccli_number for %s should be %s' %
+ 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.assertEquals(importer.topics, topics, 'topics for %s should be %s' % (source_file_name, topics))
+ self.assertEqual(importer.topics, topics, 'topics for %s should be %s' % (source_file_name, topics))
if comments:
- self.assertEquals(importer.comments, comments, 'comments for %s should be "%s"' %
+ self.assertEqual(importer.comments, comments, 'comments for %s should be "%s"' %
(source_file_name, comments))
if song_book_name:
- self.assertEquals(importer.song_book_name, song_book_name, 'song_book_name for %s should be "%s"' %
+ 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.assertEquals(importer.song_number, song_number, 'song_number for %s should be %s' %
+ self.assertEqual(importer.song_number, song_number, 'song_number for %s should be %s' %
(source_file_name, song_number))
if verse_order_list:
- self.assertEquals(importer.verse_order_list, [], 'verse_order_list for %s should be %s' %
+ 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()
=== modified file 'tests/interfaces/openlp_plugins/bibles/test_lib_parse_reference.py'
--- tests/interfaces/openlp_plugins/bibles/test_lib_parse_reference.py 2014-04-02 19:35:09 +0000
+++ tests/interfaces/openlp_plugins/bibles/test_lib_parse_reference.py 2014-04-15 05:48:25 +0000
@@ -83,7 +83,7 @@
# WHEN asking to parse the bible reference
results = parse_reference('1 Timothy 1', self.manager.db_cache['tests'], MagicMock(), 54)
# THEN a verse array should be returned
- self.assertEquals([(54, 1, 1, -1)], results, "The bible verses should matches the expected results")
+ self.assertEqual([(54, 1, 1, -1)], results, "The bible verses should matches the expected results")
def parse_reference_two_test(self):
"""
@@ -93,7 +93,7 @@
# WHEN asking to parse the bible reference
results = parse_reference('1 Timothy 1:1-2', self.manager.db_cache['tests'], MagicMock(), 54)
# THEN a verse array should be returned
- self.assertEquals([(54, 1, 1, 2)], results, "The bible verses should matches the expected results")
+ self.assertEqual([(54, 1, 1, 2)], results, "The bible verses should matches the expected results")
def parse_reference_three_test(self):
"""
@@ -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.assertEquals([(54, 1, 1, -1), (54, 2, 1, 1)], results, "The bible verses should matches the expected "
+ self.assertEqual([(54, 1, 1, -1), (54, 2, 1, 1)], results, "The bible verses should matches the expected "
"results")
Follow ups