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.
_______________________________________________
Mailing list: https://launchpad.net/~openlp-core
Post to : [email protected]
Unsubscribe : https://launchpad.net/~openlp-core
More help : https://help.launchpad.net/ListHelp
