Pull in OpenLP song importer fixes from Olli's branch

openlp/core/__init__.py

@@ -317,7 +317,7 @@ class OpenLP(OpenLPMixin, QtWidgets.QApplication):
         return QtWidgets.QApplication.event(self, event)
 
 
-def parse_options(args):
+def parse_options(args=None):
     """
     Parse the command line arguments
 

openlp/plugins/songs/lib/importers/openlp.py

@@ -221,11 +221,17 @@ class OpenLPSongImport(SongImport):
                     if not existing_book:
                         existing_book = Book.populate(name=entry.songbook.name, publisher=entry.songbook.publisher)
                     new_song.add_songbook_entry(existing_book, entry.entry)
-            elif song.book:
+            elif hasattr(song, 'book') and song.book:
                 existing_book = self.manager.get_object_filtered(Book, Book.name == song.book.name)
                 if not existing_book:
                     existing_book = Book.populate(name=song.book.name, publisher=song.book.publisher)
-                new_song.add_songbook_entry(existing_book, '')
+                # Get the song_number from "songs" table "song_number" field. (This is db structure from 2.2.1)
+                # If there's a number, add it to the song, otherwise it will be "".
+                existing_number = song.song_number if hasattr(song, 'song_number') else ''
+                if existing_number:
+                    new_song.add_songbook_entry(existing_book, existing_number)
+                else:
+                    new_song.add_songbook_entry(existing_book, "")
             # Find or create all the media files and add them to the new song object
             if has_media_files and song.media_files:
                 for media_file in song.media_files:

tests/functional/openlp_core/test_init.py

@@ -22,12 +22,12 @@
 
 import sys
 from unittest import TestCase
+from unittest.mock import MagicMock, patch
 
-from openlp.core import parse_options
+from openlp.core import OpenLP, parse_options
-from tests.helpers.testmixin import TestMixin
 
 
-class TestInitFunctions(TestMixin, TestCase):
+class TestInitFunctions(TestCase):
 
     def parse_options_basic_test(self):
         """
@@ -116,7 +116,7 @@ class TestInitFunctions(TestMixin, TestCase):
 
     def parse_options_file_and_debug_test(self):
         """
-        Test the parse options process works with a file
+        Test the parse options process works with a file and the debug log level
 
         """
         # GIVEN: a a set of system arguments.
@@ -131,14 +131,28 @@ class TestInitFunctions(TestMixin, TestCase):
         self.assertEquals(args.style, None, 'There are no style flags to be processed')
         self.assertEquals(args.rargs, 'dummy_temp', 'The service file should not be blank')
 
-    def parse_options_two_files_test(self):
-        """
-        Test the parse options process works with a file
 
+class TestOpenLP(TestCase):
+    """
+    Test the OpenLP app class
+    """
+    @patch('openlp.core.QtWidgets.QApplication.exec')
+    def test_exec(self, mocked_exec):
         """
-        # GIVEN: a a set of system arguments.
+        Test the exec method
-        sys.argv[1:] = ['dummy_temp', 'dummy_temp2']
+        """
-        # WHEN: We we parse them to expand to options
+        # GIVEN: An app
-        args = parse_options()
+        app = OpenLP([])
-        # THEN: the following fields will have been extracted.
+        app.shared_memory = MagicMock()
-        self.assertEquals(args, None, 'The args should be None')
+        mocked_exec.return_value = False
+
+        # WHEN: exec() is called
+        result = app.exec()
+
+        # THEN: The right things should be called
+        assert app.is_event_loop_active is True
+        mocked_exec.assert_called_once_with()
+        app.shared_memory.detach.assert_called_once_with()
+        assert result is False
+
+        del app