Review: Needs Fixing

See Below.

Diff comments:

> === 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-07 17:57:16 
> +0000
> @@ -32,6 +32,7 @@
>  from unittest import TestCase
>  
>  from openlp.core.common.settings import Settings
> +from openlp.core import utils
>  from openlp.core.utils import VersionThread, get_application_version
>  from tests.functional import MagicMock, patch
>  from tests.helpers.testmixin import TestMixin
> @@ -62,9 +63,79 @@
>          # 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 = utils.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 = utils.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: The command 'soffice' should be called with the 
> appropriate parameters

Description is wrong.

> +            self.assertRaises(FileNotFoundError, utils.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:
> +        """
> +
> +        original_type = utils.UNO_CONNECTION_TYPE
> +
> +        # GIVEN: A patched 'which' method which returns 'libreoffice'
> +        with patch('openlp.core.utils.which', **{'return_value': 
> 'libreoffice'}):
> +
> +            # WHEN: Calling get_uno_command
> +            utils.UNO_CONNECTION_TYPE = 'socket'
> +            result = utils.get_uno_command()
> +
> +            # THEN: The command 'soffice' should be called with the 
> appropriate parameters

And here.

> +            self.assertEqual(result, 'libreoffice --nologo --norestore 
> --minimized --nodefault --nofirststartwizard'
> +                                     ' 
> "--accept=socket,host=localhost,port=2002;urp;"')
> +
> +            utils.UNO_CONNECTION_TYPE = original_type

Can this not be patched or mocked.  Bit of a hack.

> 


-- 
https://code.launchpad.net/~phill-ridout/openlp/bug1397570/+merge/243914
Your team OpenLP Core is subscribed to branch lp:openlp.

_______________________________________________
Mailing list: https://launchpad.net/~openlp-core
Post to     : openlp-core@lists.launchpad.net
Unsubscribe : https://launchpad.net/~openlp-core
More help   : https://help.launchpad.net/ListHelp

Reply via email to