openlp-core team mailing list archive

[Jeffrey Smith <whydoubt@xxxxxxxxx>]

Jeffrey Smith has proposed merging lp:~whydoubt/openlp/fix_ewimport_py3 into lp:openlp.

Requested reviews:
  Phill (phill-ridout)
  OpenLP Core (openlp-core)

For more details, see:
https://code.launchpad.net/~whydoubt/openlp/fix_ewimport_py3/+merge/184453

Fixed EasyWorship song importer and its tests for Python3.

On Fedora, tested with:
  nosetests-3.3 -v tests/functional/openlp_plugins/songs/test_ewimport.py
-- 
https://code.launchpad.net/~whydoubt/openlp/fix_ewimport_py3/+merge/184453
Your team OpenLP Core is requested to review the proposed merge of lp:~whydoubt/openlp/fix_ewimport_py3 into lp:openlp.

=== modified file 'openlp/plugins/songs/lib/ewimport.py'
--- openlp/plugins/songs/lib/ewimport.py	2013-08-31 18:17:38 +0000
+++ openlp/plugins/songs/lib/ewimport.py	2013-09-07 21:13:32 +0000
@@ -122,7 +122,7 @@
         db_file.seek(120)
         field_info = db_file.read(num_fields * 2)
         db_file.seek(4 + (num_fields * 4) + 261, os.SEEK_CUR)
-        field_names = db_file.read(header_size - db_file.tell()).split('\0', num_fields)
+        field_names = db_file.read(header_size - db_file.tell()).split(b'\0', num_fields)
         field_names.pop()
         field_descs = []
         for i, field_name in enumerate(field_names):
@@ -132,12 +132,12 @@
         # Pick out the field description indexes we will need
         try:
             success = True
-            fi_title = self.findField('Title')
-            fi_author = self.findField('Author')
-            fi_copy = self.findField('Copyright')
-            fi_admin = self.findField('Administrator')
-            fi_words = self.findField('Words')
-            fi_ccli = self.findField('Song Number')
+            fi_title = self.findField(b'Title')
+            fi_author = self.findField(b'Author')
+            fi_copy = self.findField(b'Copyright')
+            fi_admin = self.findField(b'Administrator')
+            fi_words = self.findField(b'Words')
+            fi_ccli = self.findField(b'Song Number')
         except IndexError:
             # This is the wrong table
             success = False
@@ -150,7 +150,7 @@
             cur_block_pos = header_size + ((cur_block - 1) * 1024 * block_size)
             db_file.seek(cur_block_pos)
             cur_block, rec_count = struct.unpack('<h2xh', db_file.read(6))
-            rec_count = (rec_count + record_size) / record_size
+            rec_count = (rec_count + record_size) // record_size
             block_list.append((cur_block_pos, rec_count))
             total_count += rec_count
         self.import_wizard.progress_bar.setMaximum(total_count)
@@ -164,7 +164,7 @@
                 raw_record = db_file.read(record_size)
                 self.fields = self.recordStruct.unpack(raw_record)
                 self.setDefaults()
-                self.title = self.getField(fi_title)
+                self.title = self.getField(fi_title).decode()
                 # Get remaining fields.
                 copy = self.getField(fi_copy)
                 admin = self.getField(fi_admin)
@@ -173,25 +173,26 @@
                 words = self.getField(fi_words)
                 # Set the SongImport object members.
                 if copy:
-                    self.copyright = copy
+                    self.copyright = copy.decode()
                 if admin:
                     if copy:
                         self.copyright += ', '
-                    self.copyright += translate('SongsPlugin.EasyWorshipSongImport', 'Administered by %s') % admin
+                    self.copyright += translate('SongsPlugin.EasyWorshipSongImport',
+                                                'Administered by %s') % admin.decode()
                 if ccli:
-                    self.ccliNumber = ccli
+                    self.ccliNumber = ccli.decode()
                 if authors:
                     # Split up the authors
-                    author_list = authors.split('/')
-                    if len(author_list) < 2:
-                        author_list = authors.split(';')
-                    if len(author_list) < 2:
-                        author_list = authors.split(',')
+                    author_list = authors.split(b'/')
+                    if len(author_list) < 2:
+                        author_list = authors.split(b';')
+                    if len(author_list) < 2:
+                        author_list = authors.split(b',')
                     for author_name in author_list:
-                        self.addAuthor(author_name.strip())
+                        self.addAuthor(author_name.decode().strip())
                 if words:
                     # Format the lyrics
-                    result = strip_rtf(words, self.encoding)
+                    result = strip_rtf(words.decode(), self.encoding)
                     if result is None:
                         return
                     words, self.encoding = result
@@ -267,14 +268,14 @@
         field = self.fields[field_desc_index]
         field_desc = self.fieldDescs[field_desc_index]
         # Return None in case of 'blank' entries
-        if isinstance(field, str):
-            if not field.rstrip('\0'):
+        if isinstance(field, bytes):
+            if not field.rstrip(b'\0'):
                 return None
         elif field == 0:
             return None
         # Format the field depending on the field type
         if field_desc.field_type == FieldType.String:
-            return field.rstrip('\0')
+            return field.rstrip(b'\0')
         elif field_desc.field_type == FieldType.Int16:
             return field ^ 0x8000
         elif field_desc.field_type == FieldType.Int32:
@@ -291,12 +292,12 @@
                 self.memoFile.seek(8, os.SEEK_CUR)
             elif memo_block_type == 3:
                 if sub_block > 63:
-                    return ''
+                    return b''
                 self.memoFile.seek(11 + (5 * sub_block), os.SEEK_CUR)
                 sub_block_start, = struct.unpack('B', self.memoFile.read(1))
                 self.memoFile.seek(block_start + (sub_block_start * 16))
             else:
-                return ''
+                return b''
             return self.memoFile.read(blob_size)
         else:
             return 0

=== modified file 'tests/functional/openlp_plugins/songs/test_ewimport.py'
--- tests/functional/openlp_plugins/songs/test_ewimport.py	2013-08-31 18:17:38 +0000
+++ tests/functional/openlp_plugins/songs/test_ewimport.py	2013-09-07 21:13:32 +0000
@@ -74,18 +74,18 @@
     TestFieldDesc('Default Background', FieldType.Logical, 1), TestFieldDesc('Words', FieldType.Memo, 250),
     TestFieldDesc('Words', FieldType.Memo, 250), TestFieldDesc('BK Bitmap', FieldType.Blob, 10),
     TestFieldDesc('Last Modified', FieldType.Timestamp, 10)]
-TEST_FIELDS = ['A Heart Like Thine\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0', 32868, 2147483750,
-    129, '{\\rtf1\\ansi\\deff0\\deftab254{\\fonttbl{\\f0\\fnil\\fcharset0 Arial;}{\\f1\\fnil\\fcharset0 Verdana;}}'
-    '{\\colortbl\\red0\\green0\\blue0;\\red255\\green0\\blue0;\\red0\\green128\\blue0;\\red0\\green0\\blue255;'
-    '\\red255\\green255\\blue0;\\red255\\green0\\blue255;\\red128\\g��\7\0f\r\0\0\1\0',
-    '{\\rtf1\\ansi\\deff0\\deftab254{\\fonttbl{\\f0\\fnil\\fcharset0 Arial;}{\\f1\\fnil\\fcharset0 Verdana;}}'
-    '{\\colortbl\\red0\\green0\\blue0;\\red255\\green0\\blue0;\\red0\\green128\\blue0;\\red0\\green0\\blue255;\\red255'
-    '\\green255\\blue0;\\red255\\green0\\blue255;\\red128\\g>�\6\0�\6\0\0\1\0', '\0\0\0\0\0\0\0\0\0\0', 0]
+TEST_FIELDS = [b'A Heart Like Thine\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0', 32868, 2147483750,
+    129, b'{\\rtf1\\ansi\\deff0\\deftab254{\\fonttbl{\\f0\\fnil\\fcharset0 Arial;}{\\f1\\fnil\\fcharset0 Verdana;}}'
+    b'{\\colortbl\\red0\\green0\\blue0;\\red255\\green0\\blue0;\\red0\\green128\\blue0;\\red0\\green0\\blue255;'
+    b'\\red255\\green255\\blue0;\\red255\\green0\\blue255;\\red128\\g\xBF\xBD\7\0f\r\0\0\1\0',
+    b'{\\rtf1\\ansi\\deff0\\deftab254{\\fonttbl{\\f0\\fnil\\fcharset0 Arial;}{\\f1\\fnil\\fcharset0 Verdana;}}'
+    b'{\\colortbl\\red0\\green0\\blue0;\\red255\\green0\\blue0;\\red0\\green128\\blue0;\\red0\\green0\\blue255;\\red255'
+    b'\\green255\\blue0;\\red255\\green0\\blue255;\\red128\\g\6\0\xEF\xBF\xBD\6\0\0\1\0', b'\0\0\0\0\0\0\0\0\0\0', 0]
 GET_MEMO_FIELD_TEST_RESULTS = [
-    (4, '\2', {'return': '\2','read': (1, 3430), 'seek': (507136, (8, os.SEEK_CUR))}),
-    (4, '\3', {'return': '', 'read': (1, ), 'seek': (507136, )}),
-    (5, '\3', {'return': '\3', 'read': (1, 1725), 'seek': (3220111360, (41, os.SEEK_CUR), 3220111408)}),
-    (5, '\4', {'return': '', 'read': (), 'seek': ()})]
+    (4, b'\2', {'return': b'\2','read': (1, 3430), 'seek': (507136, (8, os.SEEK_CUR))}),
+    (4, b'\3', {'return': b'', 'read': (1, ), 'seek': (507136, )}),
+    (5, b'\3', {'return': b'\3', 'read': (1, 1725), 'seek': (3220111360, (41, os.SEEK_CUR), 3220111408)}),
+    (5, b'\4', {'return': b'', 'read': (), 'seek': ()})]
 
 class TestEasyWorshipSongImport(TestCase):
     """
@@ -189,7 +189,7 @@
             importer.encoding = TEST_DATA_ENCODING
             importer.fields = TEST_FIELDS
             importer.fieldDescs = TEST_FIELD_DESCS
-            field_results = [(0, 'A Heart Like Thine'), (1, 100), (2, 102), (3, True), (6, None), (7, None)]
+            field_results = [(0, b'A Heart Like Thine'), (1, 100), (2, 102), (3, True), (6, None), (7, None)]
 
             # WHEN: Called with test data
             for field_index, result in field_results:
@@ -276,7 +276,7 @@
         # GIVEN: A mocked out SongImport class, a mocked out "manager"
         with patch('openlp.plugins.songs.lib.ewimport.SongImport'), \
             patch('openlp.plugins.songs.lib.ewimport.os.path') as mocked_os_path, \
-            patch('__builtin__.open') as mocked_open, \
+            patch('builtins.open') as mocked_open, \
             patch('openlp.plugins.songs.lib.ewimport.struct') as mocked_struct:
             mocked_manager = MagicMock()
             importer = EasyWorshipSongImport(mocked_manager)
@@ -303,7 +303,7 @@
         # GIVEN: A mocked out SongImport class, a mocked out "manager"
         with patch('openlp.plugins.songs.lib.ewimport.SongImport'), \
             patch('openlp.plugins.songs.lib.ewimport.os.path') as mocked_os_path, \
-            patch('__builtin__.open'), patch('openlp.plugins.songs.lib.ewimport.struct') as mocked_struct, \
+            patch('builtins.open'), patch('openlp.plugins.songs.lib.ewimport.struct') as mocked_struct, \
             patch('openlp.plugins.songs.lib.ewimport.retrieve_windows_encoding') as mocked_retrieve_windows_encoding:
             mocked_manager = MagicMock()
             importer = EasyWorshipSongImport(mocked_manager)
@@ -354,7 +354,6 @@
             #       called.
             self.assertIsNone(importer.doImport(), 'doImport should return None when it has completed')
             for song_data in SONG_TEST_DATA:
-                print mocked_title.mocked_calls()
                 title = song_data['title']
                 author_calls = song_data['authors']
                 song_copyright = song_data['copyright']