openlp-core team mailing list archive

[Phill <phill.ridout@xxxxxxxxx>]

Phill has proposed merging lp:~phill-ridout/openlp/bug1397570 into lp:openlp.

Requested reviews:
  Tim Bentley (trb143)
Related bugs:
  Bug #1397570 in OpenLP: "Libre Office no longer works on Fedora 21"
  https://bugs.launchpad.net/openlp/+bug/1397570

For more details, see:
https://code.launchpad.net/~phill-ridout/openlp/bug1397570/+merge/243949

Refactored get_uno_command & get_uno_instance to remove the use of a "constant" I couldn't quite understand the use of a "constant" as the sole condition in an if statement. This made the two functions refactored more "testable"
-- 
Your team OpenLP Core is subscribed to branch lp:openlp.

=== modified file 'openlp/core/utils/__init__.py'
--- openlp/core/utils/__init__.py	2014-12-07 08:20:30 +0000
+++ openlp/core/utils/__init__.py	2014-12-08 08:24:27 +0000
@@ -64,7 +64,6 @@
 APPLICATION_VERSION = {}
 IMAGES_FILTER = None
 ICU_COLLATOR = None
-UNO_CONNECTION_TYPE = 'pipe'
 CONTROL_CHARS = re.compile(r'[\x00-\x1F\x7F-\x9F]', re.UNICODE)
 INVALID_FILE_CHARS = re.compile(r'[\\/:\*\?"<>\|\+\[\]%]', re.UNICODE)
 DIGITS_OR_NONDIGITS = re.compile(r'\d+|\D+', re.UNICODE)
@@ -423,7 +422,7 @@
     return page
 
 
-def get_uno_command():
+def get_uno_command(connection_type='pipe'):
     """
     Returns the UNO command to launch an openoffice.org instance.
     """
@@ -434,21 +433,21 @@
         raise FileNotFoundError('Command not found')
 
     OPTIONS = '--nologo --norestore --minimized --nodefault --nofirststartwizard'
-    if UNO_CONNECTION_TYPE == 'pipe':
+    if connection_type == 'pipe':
         CONNECTION = '"--accept=pipe,name=openlp_pipe;urp;"'
     else:
         CONNECTION = '"--accept=socket,host=localhost,port=2002;urp;"'
     return '%s %s %s' % (command, OPTIONS, CONNECTION)
 
 
-def get_uno_instance(resolver):
+def get_uno_instance(resolver, connection_type='pipe'):
     """
     Returns a running openoffice.org instance.
 
     :param resolver: The UNO resolver to use to find a running instance.
     """
     log.debug('get UNO Desktop Openoffice - resolve')
-    if UNO_CONNECTION_TYPE == 'pipe':
+    if connection_type == 'pipe':
         return resolver.resolve('uno:pipe,name=openlp_pipe;urp;StarOffice.ComponentContext')
     else:
         return resolver.resolve('uno:socket,host=localhost,port=2002;urp;StarOffice.ComponentContext')

=== modified file 'tests/functional/openlp_core_utils/test_init.py'
--- tests/functional/openlp_core_utils/test_init.py	2014-10-21 20:05:08 +0000
+++ tests/functional/openlp_core_utils/test_init.py	2014-12-08 08:24:27 +0000
@@ -32,7 +32,7 @@
 from unittest import TestCase
 
 from openlp.core.common.settings import Settings
-from openlp.core.utils import VersionThread, get_application_version
+from openlp.core.utils import VersionThread, get_application_version, get_uno_command
 from tests.functional import MagicMock, patch
 from tests.helpers.testmixin import TestMixin
 
@@ -62,9 +62,74 @@
         # WHEN: We check to see if the version is different .
         with patch('PyQt4.QtCore.QThread'),\
                 patch('openlp.core.utils.get_application_version') as mocked_get_application_version:
-            mocked_get_application_version.return_value = \
-                {'version': '1.0.0', 'build': '', 'full': '2.0.4'}
+            mocked_get_application_version.return_value = {'version': '1.0.0', 'build': '', 'full': '2.0.4'}
             version_thread = VersionThread(mocked_main_window)
             version_thread.run()
         # THEN: If the version has changed the main window is notified
         self.assertTrue(mocked_main_window.emit.called, 'The main windows should have been notified')
+
+    def get_uno_command_libreoffice_command_exists_test(self):
+        """
+        Test the ``get_uno_command`` function uses the libreoffice command when available.
+        :return:
+        """
+
+        # GIVEN: A patched 'which' method which returns a path when called with 'libreoffice'
+        with patch('openlp.core.utils.which',
+                   **{'side_effect': lambda command: {'libreoffice': '/usr/bin/libreoffice'}[command]}):
+
+            # WHEN: Calling get_uno_command
+            result = get_uno_command()
+
+            # THEN: The command 'libreoffice' should be called with the appropriate parameters
+            self.assertEquals(result, 'libreoffice --nologo --norestore --minimized --nodefault --nofirststartwizard'
+                                       ' "--accept=pipe,name=openlp_pipe;urp;"')
+
+    def get_uno_command_only_soffice_command_exists_test(self):
+        """
+        Test the ``get_uno_command`` function uses the soffice command when the libreoffice command is not available.
+        :return:
+        """
+
+        # GIVEN: A patched 'which' method which returns None when called with 'libreoffice' and a path when called with
+        #        'soffice'
+        with patch('openlp.core.utils.which',
+                   **{'side_effect': lambda command: {'libreoffice': None, 'soffice': '/usr/bin/soffice'}[command]}):
+
+            # WHEN: Calling get_uno_command
+            result = get_uno_command()
+
+            # THEN: The command 'soffice' should be called with the appropriate parameters
+            self.assertEquals(result, 'soffice --nologo --norestore --minimized --nodefault --nofirststartwizard'
+                                       ' "--accept=pipe,name=openlp_pipe;urp;"')
+
+    def get_uno_command_when_no_command_exists_test(self):
+        """
+        Test the ``get_uno_command`` function raises an FileNotFoundError when neither the libreoffice or soffice
+        commands are available.
+        :return:
+        """
+
+        # GIVEN: A patched 'which' method which returns None
+        with patch('openlp.core.utils.which', **{'return_value': None}):
+
+            # WHEN: Calling get_uno_command
+
+            # THEN: a FileNotFoundError exception should be raised
+            self.assertRaises(FileNotFoundError, get_uno_command)
+
+    def get_uno_command_connection_type_test(self):
+        """
+        Test the ``get_uno_command`` function when the connection type is anything other than pipe.
+        :return:
+        """
+
+        # GIVEN: A patched 'which' method which returns 'libreoffice'
+        with patch('openlp.core.utils.which', **{'return_value': 'libreoffice'}):
+
+            # WHEN: Calling get_uno_command with a connection type other than pipe
+            result = get_uno_command('socket')
+
+            # THEN: The connection parameters should be set for socket
+            self.assertEqual(result, 'libreoffice --nologo --norestore --minimized --nodefault --nofirststartwizard'
+                                     ' "--accept=socket,host=localhost,port=2002;urp;"')