← Back to team overview

openlp-core team mailing list archive

[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