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

Proposed by Ken Roberts
Status: Merged
Approved by: Raoul Snyman
Approved revision: 2471
Merged at revision: 2469
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
Raoul Snyman Approve
Tim Bentley Approve
Review via email: mp+246000@code.launchpad.net

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

Description of the change

bugfix 1407445

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

lp:~alisonken1/openlp/bug-1407445 (revision 2471)
[SUCCESS] http://ci.openlp.org/job/Branch-01-Pull/854/
[SUCCESS] http://ci.openlp.org/job/Branch-02-Functional-Tests/786/
[SUCCESS] http://ci.openlp.org/job/Branch-03-Interface-Tests/732/
[FAILURE] http://ci.openlp.org/job/Branch-04a-Windows_Functional_Tests/636/
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 : Posted in a previous version of this proposal

needs blank line before #When etc

review: Needs Fixing
Revision history for this message
Tim Bentley (trb143) :
review: Approve
Revision history for this message
Raoul Snyman (raoul-snyman) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'openlp/core/ui/servicemanager.py'
2--- openlp/core/ui/servicemanager.py 2014-12-31 10:58:13 +0000
3+++ openlp/core/ui/servicemanager.py 2015-01-09 18:57:35 +0000
4@@ -338,6 +338,8 @@
5 self.setup_ui(self)
6 # Need to use event as called across threads and UI is updated
7 QtCore.QObject.connect(self, QtCore.SIGNAL('servicemanager_set_item'), self.on_set_item)
8+ QtCore.QObject.connect(self, QtCore.SIGNAL('servicemanager_next_item'), self.next_item)
9+ QtCore.QObject.connect(self, QtCore.SIGNAL('servicemanager_previous_item'), self.previous_item)
10
11 def bootstrap_post_set_up(self):
12 """
13
14=== modified file 'openlp/plugins/remotes/lib/httprouter.py'
15--- openlp/plugins/remotes/lib/httprouter.py 2014-12-31 10:58:13 +0000
16+++ openlp/plugins/remotes/lib/httprouter.py 2015-01-09 18:57:35 +0000
17@@ -582,7 +582,7 @@
18 return self.do_http_error()
19 self.service_manager.emit(QtCore.SIGNAL(event), data)
20 else:
21- Registry().execute(event)
22+ self.service_manager.emit(QtCore.SIGNAL(event))
23 self.do_json_header()
24 return json.dumps({'results': {'success': True}}).encode()
25
26
27=== modified file 'tests/functional/openlp_plugins/remotes/test_router.py'
28--- tests/functional/openlp_plugins/remotes/test_router.py 2014-12-31 10:58:13 +0000
29+++ tests/functional/openlp_plugins/remotes/test_router.py 2015-01-09 18:57:35 +0000
30@@ -32,8 +32,9 @@
31 import os
32 import urllib.request
33 from unittest import TestCase
34-
35+from PyQt4 import QtCore
36 from openlp.core.common import Settings, Registry
37+from openlp.core.ui import ServiceManager
38 from openlp.plugins.remotes.lib.httpserver import HttpRouter
39 from urllib.parse import urlparse
40 from tests.functional import MagicMock, patch, mock_open
41@@ -61,9 +62,11 @@
42 """
43 Create the UI
44 """
45+ Registry.create()
46 self.setup_application()
47 self.build_settings()
48 Settings().extend_default_settings(__default_settings__)
49+ self.service_manager = ServiceManager()
50 self.router = HttpRouter()
51
52 def tearDown(self):
53@@ -299,3 +302,45 @@
54 mocked_image_manager.assert_called_any(os.path.normpath('thumbnails\\another test'),
55 'slide1.png', None, '120x90')
56 mocked_image_manager.assert_called_any(os.path.normpath('thumbnails\\another test'), 'slide1.png', '120x90')
57+
58+ def remote_next_test(self):
59+ """
60+ Test service manager receives remote next click properly (bug 1407445)
61+ """
62+ # GIVEN: initial setup and mocks
63+ self.router.routes = [(r'^/api/service/(.*)$', {'function': self.router.service, 'secure': False})]
64+ self.router.request_data = False
65+ mocked_next_item = MagicMock()
66+ self.service_manager.next_item = mocked_next_item
67+ with patch.object(self.service_manager, 'setup_ui'), \
68+ patch.object(self.router, 'do_json_header'):
69+ self.service_manager.bootstrap_initialise()
70+ self.app.processEvents()
71+
72+ # WHEN: Remote next is received
73+ self.router.service(action='next')
74+ self.app.processEvents()
75+
76+ # THEN: service_manager.next_item() should have been called
77+ self.assertTrue(mocked_next_item.called, 'next_item() should have been called in service_manager')
78+
79+ def remote_previous_test(self):
80+ """
81+ Test service manager receives remote previous click properly (bug 1407445)
82+ """
83+ # GIVEN: initial setup and mocks
84+ self.router.routes = [(r'^/api/service/(.*)$', {'function': self.router.service, 'secure': False})]
85+ self.router.request_data = False
86+ mocked_previous_item = MagicMock()
87+ self.service_manager.previous_item = mocked_previous_item
88+ with patch.object(self.service_manager, 'setup_ui'), \
89+ patch.object(self.router, 'do_json_header'):
90+ self.service_manager.bootstrap_initialise()
91+ self.app.processEvents()
92+
93+ # WHEN: Remote next is received
94+ self.router.service(action='previous')
95+ self.app.processEvents()
96+
97+ # THEN: service_manager.next_item() should have been called
98+ self.assertTrue(mocked_previous_item.called, 'previous_item() should have been called in service_manager')