OpenLP

Merge lp:~alisonken1/openlp/bug-1407445 into lp:openlp

  1. bug-1407445
  2. Merge into trunk
Proposed by Ken Roberts
Status: Superseded
Proposed branch: lp:~alisonken1/openlp/bug-1407445
Merge into: lp:openlp
Diff against target: 98 lines (+49/-2)
3 files modified
openlp/core/ui/servicemanager.py (+2/-0)
openlp/plugins/remotes/lib/httprouter.py (+1/-1)
tests/functional/openlp_plugins/remotes/test_router.py (+46/-1)
To merge this branch: bzr merge lp:~alisonken1/openlp/bug-1407445
Reviewer Review Type Date Requested Status
Tim Bentley Needs Fixing
Review via email: mp+245896@code.launchpad.net

This proposal supersedes a proposal from 2015-01-08.

This proposal has been superseded by a proposal from 2015-01-09.

Description of the change

bugfix 1407445

Fix remote previous/next calls to servicemananger
Remove unneeded Registry() modification

lp:~alisonken1/openlp/bug-1407445 (revision 2470)
[SUCCESS] http://ci.openlp.org/job/Branch-01-Pull/845/
[SUCCESS] http://ci.openlp.org/job/Branch-02-Functional-Tests/777/
[SUCCESS] http://ci.openlp.org/job/Branch-03-Interface-Tests/723/
[FAILURE] http://ci.openlp.org/job/Branch-04a-Windows_Functional_Tests/627/
Stopping after failure

passed local nosetest
passed local pep8

To post a comment you must log in.
Revision history for this message
Tim Bentley (trb143) : Posted in a previous version of this proposal
review: Needs Information
Revision history for this message
Ken Roberts (alisonken1) wrote : Posted in a previous version of this proposal
Download full text (5.3 KiB)

I had refactored the setup - on first try
Registry().get('service_manager') returned None.
Fixed.

On Thu, Jan 8, 2015 at 10:46 AM, Tim Bentley <email address hidden> wrote:
> Review: Needs Information
>
>
>
> Diff comments:
>
>> === modified file 'openlp/core/ui/servicemanager.py'
>> --- openlp/core/ui/servicemanager.py 2014-12-31 10:58:13 +0000
>> +++ openlp/core/ui/servicemanager.py 2015-01-08 18:35:24 +0000
>> @@ -338,6 +338,8 @@
>> self.setup_ui(self)
>> # Need to use event as called across threads and UI is updated
>> QtCore.QObject.connect(self, QtCore.SIGNAL('servicemanager_set_item'), self.on_set_item)
>> + QtCore.QObject.connect(self, QtCore.SIGNAL('servicemanager_next_item'), self.next_item)
>> + QtCore.QObject.connect(self, QtCore.SIGNAL('servicemanager_previous_item'), self.previous_item)
>>
>> def bootstrap_post_set_up(self):
>> """
>>
>> === modified file 'openlp/plugins/remotes/lib/httprouter.py'
>> --- openlp/plugins/remotes/lib/httprouter.py 2014-12-31 10:58:13 +0000
>> +++ openlp/plugins/remotes/lib/httprouter.py 2015-01-08 18:35:24 +0000
>> @@ -582,7 +582,7 @@
>> return self.do_http_error()
>> self.service_manager.emit(QtCore.SIGNAL(event), data)
>> else:
>> - Registry().execute(event)
>> + self.service_manager.emit(QtCore.SIGNAL(event))
>> self.do_json_header()
>> return json.dumps({'results': {'success': True}}).encode()
>>
>>
>> === modified file 'tests/functional/openlp_plugins/remotes/test_router.py'
>> --- tests/functional/openlp_plugins/remotes/test_router.py 2014-12-31 10:58:13 +0000
>> +++ tests/functional/openlp_plugins/remotes/test_router.py 2015-01-08 18:35:24 +0000
>> @@ -32,8 +32,9 @@
>> import os
>> import urllib.request
>> from unittest import TestCase
>> -
>> +from PyQt4 import QtCore
>> from openlp.core.common import Settings, Registry
>> +from openlp.core.ui import ServiceManager
>> from openlp.plugins.remotes.lib.httpserver import HttpRouter
>> from urllib.parse import urlparse
>> from tests.functional import MagicMock, patch, mock_open
>> @@ -61,9 +62,12 @@
>> """
>> Create the UI
>> """
>> + Registry.create()
>> self.setup_application()
>> self.build_settings()
>> Settings().extend_default_settings(__default_settings__)
>> + self.service_manager = ServiceManager()
>> + self.service_manager = Registry().service_list['service_manager'] = self.service_manager
>
> Why do you need this the previous line should have created and registered a valid servicemanager object.
>
>> self.router = HttpRouter()
>>
>> def tearDown(self):
>> @@ -299,3 +303,41 @@
>> mocked_image_manager.assert_called_any(os.path.normpath('thumbnails\\another test'),
>> 'slide1.png', None, '120x90')
>> mocked_image_manager.assert_called_any(os.path.normpath('thumbnails\\another test'), 'slide1.png', '120x90')
>> +
>> + def remote_next_test(self):
>> + """
>> + Test service manager receives remote next click p...

Read more...

Revision history for this message
Tim Bentley (trb143) wrote :

needs blank line before #When etc

review: Needs Fixing
lp:~alisonken1/openlp/bug-1407445 updated
2471. By Ken Roberts

Fix spacing

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'openlp/core/ui/servicemanager.py'
--- openlp/core/ui/servicemanager.py 2014-12-31 10:58:13 +0000
+++ openlp/core/ui/servicemanager.py 2015-01-09 18:53:54 +0000
@@ -338,6 +338,8 @@
338 self.setup_ui(self)338 self.setup_ui(self)
339 # Need to use event as called across threads and UI is updated339 # Need to use event as called across threads and UI is updated
340 QtCore.QObject.connect(self, QtCore.SIGNAL('servicemanager_set_item'), self.on_set_item)340 QtCore.QObject.connect(self, QtCore.SIGNAL('servicemanager_set_item'), self.on_set_item)
341 QtCore.QObject.connect(self, QtCore.SIGNAL('servicemanager_next_item'), self.next_item)
342 QtCore.QObject.connect(self, QtCore.SIGNAL('servicemanager_previous_item'), self.previous_item)
341343
342 def bootstrap_post_set_up(self):344 def bootstrap_post_set_up(self):
343 """345 """
344346
=== modified file 'openlp/plugins/remotes/lib/httprouter.py'
--- openlp/plugins/remotes/lib/httprouter.py 2014-12-31 10:58:13 +0000
+++ openlp/plugins/remotes/lib/httprouter.py 2015-01-09 18:53:54 +0000
@@ -582,7 +582,7 @@
582 return self.do_http_error()582 return self.do_http_error()
583 self.service_manager.emit(QtCore.SIGNAL(event), data)583 self.service_manager.emit(QtCore.SIGNAL(event), data)
584 else:584 else:
585 Registry().execute(event)585 self.service_manager.emit(QtCore.SIGNAL(event))
586 self.do_json_header()586 self.do_json_header()
587 return json.dumps({'results': {'success': True}}).encode()587 return json.dumps({'results': {'success': True}}).encode()
588588
589589
=== modified file 'tests/functional/openlp_plugins/remotes/test_router.py'
--- tests/functional/openlp_plugins/remotes/test_router.py 2014-12-31 10:58:13 +0000
+++ tests/functional/openlp_plugins/remotes/test_router.py 2015-01-09 18:53:54 +0000
@@ -32,8 +32,9 @@
32import os32import os
33import urllib.request33import urllib.request
34from unittest import TestCase34from unittest import TestCase
3535from PyQt4 import QtCore
36from openlp.core.common import Settings, Registry36from openlp.core.common import Settings, Registry
37from openlp.core.ui import ServiceManager
37from openlp.plugins.remotes.lib.httpserver import HttpRouter38from openlp.plugins.remotes.lib.httpserver import HttpRouter
38from urllib.parse import urlparse39from urllib.parse import urlparse
39from tests.functional import MagicMock, patch, mock_open40from tests.functional import MagicMock, patch, mock_open
@@ -61,9 +62,11 @@
61 """62 """
62 Create the UI63 Create the UI
63 """64 """
65 Registry.create()
64 self.setup_application()66 self.setup_application()
65 self.build_settings()67 self.build_settings()
66 Settings().extend_default_settings(__default_settings__)68 Settings().extend_default_settings(__default_settings__)
69 self.service_manager = ServiceManager()
67 self.router = HttpRouter()70 self.router = HttpRouter()
6871
69 def tearDown(self):72 def tearDown(self):
@@ -299,3 +302,45 @@
299 mocked_image_manager.assert_called_any(os.path.normpath('thumbnails\\another test'),302 mocked_image_manager.assert_called_any(os.path.normpath('thumbnails\\another test'),
300 'slide1.png', None, '120x90')303 'slide1.png', None, '120x90')
301 mocked_image_manager.assert_called_any(os.path.normpath('thumbnails\\another test'), 'slide1.png', '120x90')304 mocked_image_manager.assert_called_any(os.path.normpath('thumbnails\\another test'), 'slide1.png', '120x90')
305
306 def remote_next_test(self):
307 """
308 Test service manager receives remote next click properly (bug 1407445)
309 """
310 # GIVEN: initial setup and mocks
311 self.router.routes = [(r'^/api/service/(.*)$', {'function': self.router.service, 'secure': False})]
312 self.router.request_data = False
313 mocked_next_item = MagicMock()
314 self.service_manager.next_item = mocked_next_item
315 with patch.object(self.service_manager, 'setup_ui'), \
316 patch.object(self.router, 'do_json_header'):
317 self.service_manager.bootstrap_initialise()
318 self.app.processEvents()
319
320 # WHEN: Remote next is received
321 self.router.service(action='next')
322 self.app.processEvents()
323
324 # THEN: service_manager.next_item() should have been called
325 self.assertTrue(mocked_next_item.called, 'next_item() should have been called in service_manager')
326
327 def remote_previous_test(self):
328 """
329 Test service manager receives remote previous click properly (bug 1407445)
330 """
331 # GIVEN: initial setup and mocks
332 self.router.routes = [(r'^/api/service/(.*)$', {'function': self.router.service, 'secure': False})]
333 self.router.request_data = False
334 mocked_previous_item = MagicMock()
335 self.service_manager.previous_item = mocked_previous_item
336 with patch.object(self.service_manager, 'setup_ui'), \
337 patch.object(self.router, 'do_json_header'):
338 self.service_manager.bootstrap_initialise()
339 self.app.processEvents()
340
341 # WHEN: Remote next is received
342 self.router.service(action='previous')
343 self.app.processEvents()
344
345 # THEN: service_manager.next_item() should have been called
346 self.assertTrue(mocked_previous_item.called, 'previous_item() should have been called in service_manager')