openlp-core team mailing list archive

[Phill <phill.ridout@xxxxxxxxx>]

Review: Needs Fixing

See in line comments

Diff comments:

> === modified file 'openlp/core/ui/firsttimeform.py'
> --- openlp/core/ui/firsttimeform.py	2014-12-31 10:58:13 +0000
> +++ openlp/core/ui/firsttimeform.py	2015-01-13 19:11:49 +0000
> @@ -182,7 +182,18 @@
>          self.config = ConfigParser()
>          user_agent = 'OpenLP/' + Registry().get('application').applicationVersion()
>          self.application.process_events()
> -        web_config = get_web_page('%s%s' % (self.web, 'download.cfg'), header=('User-Agent', user_agent))
> +        try:
> +            web_config = get_web_page('%s%s' % (self.web, 'download.cfg'), header=('User-Agent', user_agent))
> +        except (urllib.error.URLError, ConnectionError) as err:
> +            msg = QtGui.QMessageBox()
> +            title = translate('OpenLP.FirstTimeWizard', 'Network Error')
> +            msg.setText('{} {}'.format(title, err.code if hasattr(err, 'code') else ''))
> +            msg.setInformativeText(translate('OpenLP.FirstTimeWizard',
> +                                             'There was a network error attempting to\n'

Could you remove the new line please? On my system its wrapping after 'attempting' and 'configuration' so there are four lines in the message box like this:
"""
There was a network error attempting
to
connect to retrieve initial configuration
inforamtion
"""

also "inforamtion" should be "information" can we get a period after it?

> +                                             'connect to retrieve initial configuration inforamtion'))
> +            msg.setStandardButtons(msg.Ok)
> +            ans = msg.exec_()
> +            web_config = False
>          if web_config:
>              files = web_config.read()
>              try:
> 
> === modified file 'tests/functional/openlp_core_ui/test_firsttimeform.py'
> --- tests/functional/openlp_core_ui/test_firsttimeform.py	2015-01-11 19:46:41 +0000
> +++ tests/functional/openlp_core_ui/test_firsttimeform.py	2015-01-13 19:11:49 +0000
> @@ -30,6 +30,7 @@
>  Package to test the openlp.core.ui.firsttimeform package.
>  """
>  import os
> +import urllib
>  from unittest import TestCase
>  
>  from openlp.core.common import Registry
> @@ -214,3 +215,24 @@
>  
>              # THEN: The First Time Form should not have web access
>              self.assertFalse(first_time_form.web_access, 'There should not be web access with an invalid config file')
> +
> +    @patch('openlp.core.ui.firsttimeform.get_web_page')
> +    @patch('openlp.core.ui.firsttimeform.QtGui.QMessageBox')
> +    def network_error_test(self, mocked_message_box, mocked_get_web_page):
> +        """
> +        Test we catch a network error in First Time Wizard - bug 1409627
> +        """
> +        # GIVEN: Initial setup and mocks
> +        first_time_form = FirstTimeForm(None)
> +        first_time_form.initialize(MagicMock())
> +        mocked_get_web_page.side_effect = urllib.error.HTTPError(url='http//localhost',
> +                                                                 code=407,
> +                                                                 msg='Network proxy error',
> +                                                                 hdrs=None,
> +                                                                 fp=None)
> +        # WHEN: the First Time Wizard calls to get the initial configuration
> +        first_time_form._download_index()
> +
> +        # THEN: the critical_error_message_box should have been called
> +        self.assertEquals(mocked_message_box.mock_calls[1][1][0], 'Network Error 407',
> +                          'first_time_form should have caught Network Error')
> 


-- 
https://code.launchpad.net/~alisonken1/openlp/bug-1409627/+merge/246343
Your team OpenLP Core is subscribed to branch lp:openlp.