Merge lp:~raoul-snyman/openlp/bug-1742910 into lp:openlp

Proposed by Raoul Snyman on 2018-01-13
Status: Merged
Merged at revision: 2808
Proposed branch: lp:~raoul-snyman/openlp/bug-1742910
Merge into: lp:openlp
Diff against target: 755 lines (+333/-194)
10 files modified
openlp/core/app.py (+1/-1)
openlp/core/common/path.py (+2/-2)
openlp/core/threading.py (+14/-10)
openlp/core/ui/mainwindow.py (+3/-4)
tests/functional/openlp_core/api/http/test_error.py (+37/-35)
tests/functional/openlp_core/api/test_deploy.py (+94/-15)
tests/functional/openlp_core/common/test_path.py (+14/-1)
tests/functional/openlp_core/lib/test_path.py (+0/-87)
tests/functional/openlp_core/lib/test_ui.py (+55/-28)
tests/functional/openlp_core/test_threading.py (+113/-11)
To merge this branch: bzr merge lp:~raoul-snyman/openlp/bug-1742910
Reviewer Review Type Date Requested Status
Tim Bentley 2018-01-13 Approve on 2018-01-13
Review via email: mp+336066@code.launchpad.net

This proposal supersedes a proposal from 2018-01-13.

Description of the Change

Fix bug #1742910 by moving the threads to the application object instead of the main window object.

Add this to your merge proposal:
--------------------------------------------------------------------------------
lp:~raoul-snyman/openlp/bug-1742910 (revision 2810)
https://ci.openlp.io/job/Branch-01-Pull/2418/ [SUCCESS]
https://ci.openlp.io/job/Branch-02a-Linux-Tests/2319/ [SUCCESS]
https://ci.openlp.io/job/Branch-02b-macOS-Tests/114/ [SUCCESS]
https://ci.openlp.io/job/Branch-03a-Build-Source/36/ [SUCCESS]
https://ci.openlp.io/job/Branch-03b-Build-macOS/35/ [SUCCESS]
https://ci.openlp.io/job/Branch-04a-Code-Analysis/1498/ [SUCCESS]
https://ci.openlp.io/job/Branch-04b-Test-Coverage/1311/ [SUCCESS]
https://ci.openlp.io/job/Branch-05-AppVeyor-Tests/262/ [FAILURE]
Stopping after failure

Failed builds:
 - Branch-05-AppVeyor-Tests #262: https://ci.openlp.io/job/Branch-05-AppVeyor-Tests/262/console

To post a comment you must log in.
Tim Bentley (trb143) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'openlp/core/app.py'
2--- openlp/core/app.py 2018-01-07 04:40:40 +0000
3+++ openlp/core/app.py 2018-01-13 06:00:49 +0000
4@@ -63,8 +63,8 @@
5 The core application class. This class inherits from Qt's QApplication
6 class in order to provide the core of the application.
7 """
8-
9 args = []
10+ worker_threads = {}
11
12 def exec(self):
13 """
14
15=== modified file 'openlp/core/common/path.py'
16--- openlp/core/common/path.py 2017-12-29 09:15:48 +0000
17+++ openlp/core/common/path.py 2018-01-13 06:00:49 +0000
18@@ -26,9 +26,9 @@
19 from openlp.core.common import is_win
20
21 if is_win():
22- from pathlib import WindowsPath as PathVariant
23+ from pathlib import WindowsPath as PathVariant # pragma: nocover
24 else:
25- from pathlib import PosixPath as PathVariant
26+ from pathlib import PosixPath as PathVariant # pragma: nocover
27
28 log = logging.getLogger(__name__)
29
30
31=== modified file 'openlp/core/threading.py'
32--- openlp/core/threading.py 2018-01-07 17:50:29 +0000
33+++ openlp/core/threading.py 2018-01-13 06:00:49 +0000
34@@ -50,12 +50,12 @@
35 """
36 if not thread_name:
37 raise ValueError('A thread_name is required when calling the "run_thread" function')
38- main_window = Registry().get('main_window')
39- if thread_name in main_window.threads:
40+ application = Registry().get('application')
41+ if thread_name in application.worker_threads:
42 raise KeyError('A thread with the name "{}" has already been created, please use another'.format(thread_name))
43 # Create the thread and add the thread and the worker to the parent
44 thread = QtCore.QThread()
45- main_window.threads[thread_name] = {
46+ application.worker_threads[thread_name] = {
47 'thread': thread,
48 'worker': worker
49 }
50@@ -78,7 +78,10 @@
51 :param str thread_name: The name of the thread
52 :returns: The worker for this thread name
53 """
54- return Registry().get('main_window').threads.get(thread_name)
55+ thread_info = Registry().get('application').worker_threads.get(thread_name)
56+ if not thread_info:
57+ raise KeyError('No thread named "{}" exists'.format(thread_name))
58+ return thread_info.get('worker')
59
60
61 def is_thread_finished(thread_name):
62@@ -88,8 +91,8 @@
63 :param str thread_name: The name of the thread
64 :returns: True if the thread is finished, False if it is still running
65 """
66- main_window = Registry().get('main_window')
67- return thread_name not in main_window.threads or main_window.threads[thread_name]['thread'].isFinished()
68+ app = Registry().get('application')
69+ return thread_name not in app.worker_threads or app.worker_threads[thread_name]['thread'].isFinished()
70
71
72 def make_remove_thread(thread_name):
73@@ -99,13 +102,14 @@
74 :param str thread_name: The name of the thread which should be removed from the thread registry.
75 :returns: A function which will remove the thread from the thread registry.
76 """
77- def remove_thread():
78+
79+ def remove_thread(): # pragma: nocover
80 """
81 Stop and remove a registered thread
82
83 :param str thread_name: The name of the thread to stop and remove
84 """
85- main_window = Registry().get('main_window')
86- if thread_name in main_window.threads:
87- del main_window.threads[thread_name]
88+ application = Registry().get('application')
89+ if thread_name in application.worker_threads:
90+ del application.worker_threads[thread_name]
91 return remove_thread
92
93=== modified file 'openlp/core/ui/mainwindow.py'
94--- openlp/core/ui/mainwindow.py 2018-01-07 17:50:29 +0000
95+++ openlp/core/ui/mainwindow.py 2018-01-13 06:00:49 +0000
96@@ -477,7 +477,6 @@
97 """
98 super(MainWindow, self).__init__()
99 Registry().register('main_window', self)
100- self.threads = {}
101 self.clipboard = self.application.clipboard()
102 self.arguments = ''.join(self.application.args)
103 # Set up settings sections for the main application (not for use by plugins).
104@@ -557,11 +556,11 @@
105 wait_dialog.setAutoClose(False)
106 wait_dialog.setCancelButton(None)
107 wait_dialog.show()
108- for thread_name in self.threads.keys():
109+ for thread_name in self.application.worker_threads.keys():
110 log.debug('Waiting for thread %s', thread_name)
111 self.application.processEvents()
112- thread = self.threads[thread_name]['thread']
113- worker = self.threads[thread_name]['worker']
114+ thread = self.application.worker_threads[thread_name]['thread']
115+ worker = self.application.worker_threads[thread_name]['worker']
116 try:
117 if worker and hasattr(worker, 'stop'):
118 # If the worker has a stop method, run it
119
120=== modified file 'tests/functional/openlp_core/api/http/test_error.py'
121--- tests/functional/openlp_core/api/http/test_error.py 2017-12-29 09:15:48 +0000
122+++ tests/functional/openlp_core/api/http/test_error.py 2018-01-13 06:00:49 +0000
123@@ -22,38 +22,40 @@
124 """
125 Functional tests to test the API Error Class.
126 """
127-
128-from unittest import TestCase
129-
130-from openlp.core.api.http.errors import NotFound, ServerError
131-
132-
133-class TestApiError(TestCase):
134- """
135- A test suite to test out the Error in the API code
136- """
137- def test_not_found(self):
138- """
139- Test the Not Found error displays the correct information
140- """
141- # GIVEN:
142- # WHEN: I raise an exception
143- with self.assertRaises(Exception) as context:
144- raise NotFound()
145-
146- # THEN: we get an error and a status
147- assert 'Not Found' == context.exception.message, 'A Not Found exception should be thrown'
148- assert 404 == context.exception.status, 'A 404 status should be thrown'
149-
150- def test_server_error(self):
151- """
152- Test the server error displays the correct information
153- """
154- # GIVEN:
155- # WHEN: I raise an exception
156- with self.assertRaises(Exception) as context:
157- raise ServerError()
158-
159- # THEN: we get an error and a status
160- assert'Server Error' == context.exception.message, 'A Not Found exception should be thrown'
161- assert 500 == context.exception.status, 'A 500 status should be thrown'
162+from openlp.core.api.http.errors import HttpError, NotFound, ServerError
163+
164+
165+def test_http_error():
166+ """
167+ Test the HTTPError class
168+ """
169+ # GIVEN: An HTTPError class
170+ # WHEN: An instance is created
171+ error = HttpError(400, 'Access Denied')
172+
173+ # THEN: The to_response() method should return the correct information
174+ assert error.to_response() == ('Access Denied', 400), 'to_response() should have returned the correct info'
175+
176+
177+def test_not_found():
178+ """
179+ Test the Not Found error displays the correct information
180+ """
181+ # GIVEN: A NotFound class
182+ # WHEN: An instance is created
183+ error = NotFound()
184+
185+ # THEN: The to_response() method should return the correct information
186+ assert error.to_response() == ('Not Found', 404), 'to_response() should have returned the correct info'
187+
188+
189+def test_server_error():
190+ """
191+ Test the server error displays the correct information
192+ """
193+ # GIVEN: A ServerError class
194+ # WHEN: An instance of the class is created
195+ error = ServerError()
196+
197+ # THEN: The to_response() method should return the correct information
198+ assert error.to_response() == ('Server Error', 500), 'to_response() should have returned the correct info'
199
200=== modified file 'tests/functional/openlp_core/api/test_deploy.py'
201--- tests/functional/openlp_core/api/test_deploy.py 2017-12-29 09:15:48 +0000
202+++ tests/functional/openlp_core/api/test_deploy.py 2018-01-13 06:00:49 +0000
203@@ -21,11 +21,12 @@
204 ###############################################################################
205 from tempfile import mkdtemp
206 from unittest import TestCase
207-
208-from openlp.core.api.deploy import deploy_zipfile
209-from openlp.core.common.path import Path, copyfile
210-
211-TEST_PATH = (Path(__file__).parent / '..' / '..' / '..' / 'resources').resolve()
212+from unittest.mock import MagicMock, patch
213+
214+from openlp.core.api.deploy import deploy_zipfile, download_sha256, download_and_check
215+from openlp.core.common.path import Path
216+
217+CONFIG_FILE = '2c266badff1e3d140664c50fd1460a2b332b24d5ad8c267fa62e506b5eb6d894 deploy/site.zip\n2017_06_27'
218
219
220 class TestRemoteDeploy(TestCase):
221@@ -45,17 +46,95 @@
222 """
223 self.app_root_path.rmtree()
224
225- def test_deploy_zipfile(self):
226+ @patch('openlp.core.api.deploy.ZipFile')
227+ def test_deploy_zipfile(self, MockZipFile):
228 """
229 Remote Deploy tests - test the dummy zip file is processed correctly
230 """
231 # GIVEN: A new downloaded zip file
232- zip_path = TEST_PATH / 'remotes' / 'site.zip'
233- app_root_path = self.app_root_path / 'site.zip'
234- copyfile(zip_path, app_root_path)
235-
236- # WHEN: I process the zipfile
237- deploy_zipfile(self.app_root_path, 'site.zip')
238-
239- # THEN: test if www directory has been created
240- assert (self.app_root_path / 'www').is_dir(), 'We should have a www directory'
241+ mocked_zipfile = MagicMock()
242+ MockZipFile.return_value = mocked_zipfile
243+ root_path = Path('/tmp/remotes')
244+
245+ # WHEN: deploy_zipfile() is called
246+ deploy_zipfile(root_path, 'site.zip')
247+
248+ # THEN: the zip file should have been extracted to the right location
249+ MockZipFile.assert_called_once_with('/tmp/remotes/site.zip')
250+ mocked_zipfile.extractall.assert_called_once_with('/tmp/remotes')
251+
252+ @patch('openlp.core.api.deploy.Registry')
253+ @patch('openlp.core.api.deploy.get_web_page')
254+ def test_download_sha256_connection_error(self, mocked_get_web_page, MockRegistry):
255+ """
256+ Test that if a ConnectionError occurs while downloading a sha256 False is returned
257+ """
258+ # GIVEN: A bunch of mocks
259+ MockRegistry.return_value.get.return_value.applicationVersion.return_value = '1.0'
260+ mocked_get_web_page.side_effect = ConnectionError()
261+
262+ # WHEN: download_sha256() is called
263+ result = download_sha256()
264+
265+ # THEN: The result should be False
266+ assert result is False, 'download_sha256() should return False when encountering ConnectionError'
267+
268+ @patch('openlp.core.api.deploy.Registry')
269+ @patch('openlp.core.api.deploy.get_web_page')
270+ def test_download_sha256_no_config(self, mocked_get_web_page, MockRegistry):
271+ """
272+ Test that if there's no config when downloading a sha256 None is returned
273+ """
274+ # GIVEN: A bunch of mocks
275+ MockRegistry.return_value.get.return_value.applicationVersion.return_value = '1.0'
276+ mocked_get_web_page.return_value = None
277+
278+ # WHEN: download_sha256() is called
279+ result = download_sha256()
280+
281+ # THEN: The result should be Nonw
282+ assert result is None, 'download_sha256() should return None when there is a problem downloading the page'
283+
284+ @patch('openlp.core.api.deploy.Registry')
285+ @patch('openlp.core.api.deploy.get_web_page')
286+ def test_download_sha256(self, mocked_get_web_page, MockRegistry):
287+ """
288+ Test that the sha256 and the version are returned
289+ """
290+ # GIVEN: A bunch of mocks
291+ MockRegistry.return_value.get.return_value.applicationVersion.return_value = '1.0'
292+ mocked_get_web_page.return_value = CONFIG_FILE
293+
294+ # WHEN: download_sha256() is called
295+ result = download_sha256()
296+
297+ # THEN: The result should be Nonw
298+ assert result == ('2c266badff1e3d140664c50fd1460a2b332b24d5ad8c267fa62e506b5eb6d894', '2017_06_27'), \
299+ 'download_sha256() should return a tuple of sha256 and version'
300+
301+ @patch('openlp.core.api.deploy.Registry')
302+ @patch('openlp.core.api.deploy.download_sha256')
303+ @patch('openlp.core.api.deploy.get_url_file_size')
304+ @patch('openlp.core.api.deploy.download_file')
305+ @patch('openlp.core.api.deploy.AppLocation.get_section_data_path')
306+ @patch('openlp.core.api.deploy.deploy_zipfile')
307+ def test_download_and_check(self, mocked_deploy_zipfile, mocked_get_data_path, mocked_download_file,
308+ mocked_get_url_file_size, mocked_download_sha256, MockRegistry):
309+ # GIVEN: A bunch of mocks
310+ mocked_get_data_path.return_value = Path('/tmp/remotes')
311+ mocked_download_file.return_value = True
312+ mocked_get_url_file_size.return_value = 5
313+ mocked_download_sha256.return_value = ('asdfgh', '0.1')
314+ MockRegistry.return_value.get.return_value.applicationVersion.return_value = '1.0'
315+ mocked_callback = MagicMock()
316+
317+ # WHEN: download_and_check() is called
318+ download_and_check(mocked_callback)
319+
320+ # THEN: The correct things should have been done
321+ mocked_download_sha256.assert_called_once_with()
322+ mocked_get_url_file_size.assert_called_once_with('https://get.openlp.org/webclient/site.zip')
323+ mocked_callback.setRange.assert_called_once_with(0, 5)
324+ mocked_download_file.assert_called_once_with(mocked_callback, 'https://get.openlp.org/webclient/site.zip',
325+ Path('/tmp/remotes/site.zip'), sha256='asdfgh')
326+ mocked_deploy_zipfile.assert_called_once_with(Path('/tmp/remotes'), 'site.zip')
327
328=== modified file 'tests/functional/openlp_core/common/test_path.py'
329--- tests/functional/openlp_core/common/test_path.py 2017-12-29 09:15:48 +0000
330+++ tests/functional/openlp_core/common/test_path.py 2018-01-13 06:00:49 +0000
331@@ -27,7 +27,7 @@
332 from unittest.mock import ANY, MagicMock, patch
333
334 from openlp.core.common.path import Path, copy, copyfile, copytree, create_paths, path_to_str, replace_params, \
335- str_to_path, which
336+ str_to_path, which, files_to_paths
337
338
339 class TestShutil(TestCase):
340@@ -401,3 +401,16 @@
341 except:
342 # THEN: `create_paths` raises an exception
343 pass
344+
345+ def test_files_to_paths(self):
346+ """
347+ Test the files_to_paths() method
348+ """
349+ # GIVEN: A list of string filenames
350+ test_files = ['/tmp/openlp/file1.txt', '/tmp/openlp/file2.txt']
351+
352+ # WHEN: files_to_paths is called
353+ result = files_to_paths(test_files)
354+
355+ # THEN: The result should be a list of Paths
356+ assert result == [Path('/tmp/openlp/file1.txt'), Path('/tmp/openlp/file2.txt')]
357
358=== removed file 'tests/functional/openlp_core/lib/test_path.py'
359--- tests/functional/openlp_core/lib/test_path.py 2017-12-23 09:09:45 +0000
360+++ tests/functional/openlp_core/lib/test_path.py 1970-01-01 00:00:00 +0000
361@@ -1,87 +0,0 @@
362-# -*- coding: utf-8 -*-
363-# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4
364-
365-###############################################################################
366-# OpenLP - Open Source Lyrics Projection #
367-# --------------------------------------------------------------------------- #
368-# Copyright (c) 2008-2017 OpenLP Developers #
369-# --------------------------------------------------------------------------- #
370-# This program is free software; you can redistribute it and/or modify it #
371-# under the terms of the GNU General Public License as published by the Free #
372-# Software Foundation; version 2 of the License. #
373-# #
374-# This program is distributed in the hope that it will be useful, but WITHOUT #
375-# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or #
376-# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for #
377-# more details. #
378-# #
379-# You should have received a copy of the GNU General Public License along #
380-# with this program; if not, write to the Free Software Foundation, Inc., 59 #
381-# Temple Place, Suite 330, Boston, MA 02111-1307 USA #
382-###############################################################################
383-"""
384-Package to test the openlp.core.lib.path package.
385-"""
386-import os
387-from unittest import TestCase
388-
389-from openlp.core.common.path import Path, path_to_str, str_to_path
390-
391-
392-class TestPath(TestCase):
393- """
394- Tests for the :mod:`openlp.core.lib.path` module
395- """
396-
397- def test_path_to_str_type_error(self):
398- """
399- Test that `path_to_str` raises a type error when called with an invalid type
400- """
401- # GIVEN: The `path_to_str` function
402- # WHEN: Calling `path_to_str` with an invalid Type
403- # THEN: A TypeError should have been raised
404- with self.assertRaises(TypeError):
405- path_to_str(str())
406-
407- def test_path_to_str_none(self):
408- """
409- Test that `path_to_str` correctly converts the path parameter when passed with None
410- """
411- # GIVEN: The `path_to_str` function
412- # WHEN: Calling the `path_to_str` function with None
413- result = path_to_str(None)
414-
415- # THEN: `path_to_str` should return an empty string
416- assert result == ''
417-
418- def test_path_to_str_path_object(self):
419- """
420- Test that `path_to_str` correctly converts the path parameter when passed a Path object
421- """
422- # GIVEN: The `path_to_str` function
423- # WHEN: Calling the `path_to_str` function with a Path object
424- result = path_to_str(Path('test/path'))
425-
426- # THEN: `path_to_str` should return a string representation of the Path object
427- assert result == os.path.join('test', 'path')
428-
429- def test_str_to_path_type_error(self):
430- """
431- Test that `str_to_path` raises a type error when called with an invalid type
432- """
433- # GIVEN: The `str_to_path` function
434- # WHEN: Calling `str_to_path` with an invalid Type
435- # THEN: A TypeError should have been raised
436- with self.assertRaises(TypeError):
437- str_to_path(Path())
438-
439- def test_str_to_path_empty_str(self):
440- """
441- Test that `str_to_path` correctly converts the string parameter when passed with and empty string
442- """
443- # GIVEN: The `str_to_path` function
444- # WHEN: Calling the `str_to_path` function with None
445- result = str_to_path('')
446-
447- # THEN: `path_to_str` should return None
448- assert result is None
449
450=== modified file 'tests/functional/openlp_core/lib/test_ui.py'
451--- tests/functional/openlp_core/lib/test_ui.py 2017-12-17 20:19:19 +0000
452+++ tests/functional/openlp_core/lib/test_ui.py 2018-01-13 06:00:49 +0000
453@@ -23,14 +23,14 @@
454 Package to test the openlp.core.lib.ui package.
455 """
456 from unittest import TestCase
457-from unittest.mock import MagicMock, patch
458+from unittest.mock import MagicMock, patch, call
459
460 from PyQt5 import QtCore, QtGui, QtWidgets
461
462 from openlp.core.common.i18n import UiStrings, translate
463 from openlp.core.lib.ui import add_welcome_page, create_button_box, create_horizontal_adjusting_combo_box, \
464 create_button, create_action, create_valign_selection_widgets, find_and_set_in_combo_box, create_widget_action, \
465- set_case_insensitive_completer
466+ set_case_insensitive_completer, critical_error_message_box
467
468
469 class TestUi(TestCase):
470@@ -80,6 +80,34 @@
471 assert 1 == len(btnbox.buttons())
472 assert QtWidgets.QDialogButtonBox.HelpRole, btnbox.buttonRole(btnbox.buttons()[0])
473
474+ @patch('openlp.core.lib.ui.Registry')
475+ def test_critical_error_message_box(self, MockRegistry):
476+ """
477+ Test the critical_error_message_box() function
478+ """
479+ # GIVEN: A mocked Registry
480+ # WHEN: critical_error_message_box() is called
481+ critical_error_message_box('Error', 'This is an error')
482+
483+ # THEN: The error_message() method on the main window should be called
484+ MockRegistry.return_value.get.return_value.error_message.assert_called_once_with('Error', 'This is an error')
485+
486+ @patch('openlp.core.lib.ui.QtWidgets.QMessageBox.critical')
487+ def test_critical_error_question(self, mocked_critical):
488+ """
489+ Test the critical_error_message_box() function
490+ """
491+ # GIVEN: A mocked critical() method and some other mocks
492+ mocked_parent = MagicMock()
493+
494+ # WHEN: critical_error_message_box() is called
495+ critical_error_message_box(None, 'This is a question', mocked_parent, True)
496+
497+ # THEN: The error_message() method on the main window should be called
498+ mocked_critical.assert_called_once_with(mocked_parent, 'Error', 'This is a question',
499+ QtWidgets.QMessageBox.StandardButtons(QtWidgets.QMessageBox.Yes |
500+ QtWidgets.QMessageBox.No))
501+
502 def test_create_horizontal_adjusting_combo_box(self):
503 """
504 Test creating a horizontal adjusting combo box
505@@ -92,65 +120,64 @@
506
507 # THEN: We should get a ComboBox
508 assert isinstance(combo, QtWidgets.QComboBox)
509- assert 'combo1' == combo.objectName()
510+ assert combo.objectName() == 'combo1'
511 assert QtWidgets.QComboBox.AdjustToMinimumContentsLength == combo.sizeAdjustPolicy()
512
513- def test_create_button(self):
514+ @patch('openlp.core.lib.ui.log')
515+ def test_create_button(self, mocked_log):
516 """
517 Test creating a button
518 """
519 # GIVEN: A dialog
520 dialog = QtWidgets.QDialog()
521
522- # WHEN: We create the button
523- btn = create_button(dialog, 'my_btn')
524-
525- # THEN: We should get a button with a name
526- assert isinstance(btn, QtWidgets.QPushButton)
527- assert 'my_btn' == btn.objectName()
528- assert btn.isEnabled() is True
529-
530 # WHEN: We create a button with some attributes
531- btn = create_button(dialog, 'my_btn', text='Hello', tooltip='How are you?', enabled=False)
532+ btn = create_button(dialog, 'my_btn', text='Hello', tooltip='How are you?', enabled=False, role='test', test=1)
533
534 # THEN: We should get a button with those attributes
535 assert isinstance(btn, QtWidgets.QPushButton)
536- assert 'Hello' == btn.text()
537- assert 'How are you?' == btn.toolTip()
538+ assert btn.objectName() == 'my_btn'
539+ assert btn.text() == 'Hello'
540+ assert btn.toolTip() == 'How are you?'
541 assert btn.isEnabled() is False
542+ assert mocked_log.warning.call_args_list == [call('The role "test" is not defined in create_push_button().'),
543+ call('Parameter test was not consumed in create_button().')]
544+
545+ def test_create_tool_button(self):
546+ """
547+ Test creating a toolbutton
548+ """
549+ # GIVEN: A dialog
550+ dialog = QtWidgets.QDialog()
551
552 # WHEN: We create a toolbutton
553 btn = create_button(dialog, 'my_btn', btn_class='toolbutton')
554
555 # THEN: We should get a toolbutton
556 assert isinstance(btn, QtWidgets.QToolButton)
557- assert 'my_btn' == btn.objectName()
558+ assert btn.objectName() == 'my_btn'
559 assert btn.isEnabled() is True
560
561- def test_create_action(self):
562+ @patch('openlp.core.lib.ui.log')
563+ def test_create_action(self, mocked_log):
564 """
565 Test creating an action
566 """
567 # GIVEN: A dialog
568 dialog = QtWidgets.QDialog()
569
570- # WHEN: We create an action
571- action = create_action(dialog, 'my_action')
572-
573- # THEN: We should get a QAction
574- assert isinstance(action, QtWidgets.QAction)
575- assert 'my_action' == action.objectName()
576-
577 # WHEN: We create an action with some properties
578 action = create_action(dialog, 'my_action', text='my text', icon=':/wizards/wizard_firsttime.bmp',
579- tooltip='my tooltip', statustip='my statustip')
580+ tooltip='my tooltip', statustip='my statustip', test=1)
581
582 # THEN: These properties should be set
583 assert isinstance(action, QtWidgets.QAction)
584- assert 'my text' == action.text()
585+ assert action.objectName() == 'my_action'
586+ assert action.text() == 'my text'
587 assert isinstance(action.icon(), QtGui.QIcon)
588- assert 'my tooltip' == action.toolTip()
589- assert 'my statustip' == action.statusTip()
590+ assert action.toolTip() == 'my tooltip'
591+ assert action.statusTip() == 'my statustip'
592+ mocked_log.warning.assert_called_once_with('Parameter test was not consumed in create_action().')
593
594 def test_create_action_on_mac_osx(self):
595 """
596
597=== modified file 'tests/functional/openlp_core/test_threading.py'
598--- tests/functional/openlp_core/test_threading.py 2018-01-07 17:50:29 +0000
599+++ tests/functional/openlp_core/test_threading.py 2018-01-13 06:00:49 +0000
600@@ -22,9 +22,10 @@
601 """
602 Package to test the openlp.core.threading package.
603 """
604+from inspect import isfunction
605 from unittest.mock import MagicMock, call, patch
606
607-from openlp.core.version import run_thread
608+from openlp.core.threading import ThreadWorker, run_thread, get_thread_worker, is_thread_finished, make_remove_thread
609
610
611 def test_run_thread_no_name():
612@@ -47,9 +48,9 @@
613 Test that trying to run a thread with a name that already exists will throw a KeyError
614 """
615 # GIVEN: A mocked registry with a main window object
616- mocked_main_window = MagicMock()
617- mocked_main_window.threads = {'test_thread': MagicMock()}
618- MockRegistry.return_value.get.return_value = mocked_main_window
619+ mocked_application = MagicMock()
620+ mocked_application.worker_threads = {'test_thread': MagicMock()}
621+ MockRegistry.return_value.get.return_value = mocked_application
622
623 # WHEN: run_thread() is called
624 try:
625@@ -66,18 +67,19 @@
626 Test that running a thread works correctly
627 """
628 # GIVEN: A mocked registry with a main window object
629- mocked_main_window = MagicMock()
630- mocked_main_window.threads = {}
631- MockRegistry.return_value.get.return_value = mocked_main_window
632+ mocked_application = MagicMock()
633+ mocked_application.worker_threads = {}
634+ MockRegistry.return_value.get.return_value = mocked_application
635
636 # WHEN: run_thread() is called
637 run_thread(MagicMock(), 'test_thread')
638
639 # THEN: The thread should be in the threads list and the correct methods should have been called
640- assert len(mocked_main_window.threads.keys()) == 1, 'There should be 1 item in the list of threads'
641- assert list(mocked_main_window.threads.keys()) == ['test_thread'], 'The test_thread item should be in the list'
642- mocked_worker = mocked_main_window.threads['test_thread']['worker']
643- mocked_thread = mocked_main_window.threads['test_thread']['thread']
644+ assert len(mocked_application.worker_threads.keys()) == 1, 'There should be 1 item in the list of threads'
645+ assert list(mocked_application.worker_threads.keys()) == ['test_thread'], \
646+ 'The test_thread item should be in the list'
647+ mocked_worker = mocked_application.worker_threads['test_thread']['worker']
648+ mocked_thread = mocked_application.worker_threads['test_thread']['thread']
649 mocked_worker.moveToThread.assert_called_once_with(mocked_thread)
650 mocked_thread.started.connect.assert_called_once_with(mocked_worker.start)
651 expected_quit_calls = [call(mocked_thread.quit), call(mocked_worker.deleteLater)]
652@@ -87,3 +89,103 @@
653 'The threads finished signal should be connected to its deleteLater slot'
654 assert mocked_thread.finished.connect.call_count == 2, 'The signal should have been connected twice'
655 mocked_thread.start.assert_called_once_with()
656+
657+
658+def test_thread_worker():
659+ """
660+ Test that creating a thread worker object and calling start throws and NotImplementedError
661+ """
662+ # GIVEN: A ThreadWorker class
663+ worker = ThreadWorker()
664+
665+ try:
666+ # WHEN: calling start()
667+ worker.start()
668+ assert False, 'A NotImplementedError should have been thrown'
669+ except NotImplementedError:
670+ # A NotImplementedError should be thrown
671+ pass
672+ except Exception:
673+ assert False, 'A NotImplementedError should have been thrown'
674+
675+
676+@patch('openlp.core.threading.Registry')
677+def test_get_thread_worker(MockRegistry):
678+ """
679+ Test that calling the get_thread_worker() function returns the correct worker
680+ """
681+ # GIVEN: A mocked thread worker
682+ mocked_worker = MagicMock()
683+ MockRegistry.return_value.get.return_value.worker_threads = {'test_thread': {'worker': mocked_worker}}
684+
685+ # WHEN: get_thread_worker() is called
686+ worker = get_thread_worker('test_thread')
687+
688+ # THEN: The mocked worker is returned
689+ assert worker is mocked_worker, 'The mocked worker should have been returned'
690+
691+
692+@patch('openlp.core.threading.Registry')
693+def test_get_thread_worker_mising(MockRegistry):
694+ """
695+ Test that calling the get_thread_worker() function raises a KeyError if it does not exist
696+ """
697+ # GIVEN: A mocked thread worker
698+ MockRegistry.return_value.get.return_value.worker_threads = {}
699+
700+ try:
701+ # WHEN: get_thread_worker() is called
702+ get_thread_worker('test_thread')
703+ assert False, 'A KeyError should have been raised'
704+ except KeyError:
705+ # THEN: The mocked worker is returned
706+ pass
707+ except Exception:
708+ assert False, 'A KeyError should have been raised'
709+
710+
711+@patch('openlp.core.threading.Registry')
712+def test_is_thread_finished(MockRegistry):
713+ """
714+ Test the is_thread_finished() function
715+ """
716+ # GIVEN: A mock thread and worker
717+ mocked_thread = MagicMock()
718+ mocked_thread.isFinished.return_value = False
719+ MockRegistry.return_value.get.return_value.worker_threads = {'test': {'thread': mocked_thread}}
720+
721+ # WHEN: is_thread_finished() is called
722+ result = is_thread_finished('test')
723+
724+ # THEN: The result should be correct
725+ assert result is False, 'is_thread_finished should have returned False'
726+
727+
728+@patch('openlp.core.threading.Registry')
729+def test_is_thread_finished_missing(MockRegistry):
730+ """
731+ Test that calling the is_thread_finished() function returns True if the thread doesn't exist
732+ """
733+ # GIVEN: A mocked thread worker
734+ MockRegistry.return_value.get.return_value.worker_threads = {}
735+
736+ # WHEN: get_thread_worker() is called
737+ result = is_thread_finished('test_thread')
738+
739+ # THEN: The result should be correct
740+ assert result is True, 'is_thread_finished should return True when a thread is missing'
741+
742+
743+def test_make_remove_thread():
744+ """
745+ Test the make_remove_thread() function
746+ """
747+ # GIVEN: A thread name
748+ thread_name = 'test_thread'
749+
750+ # WHEN: make_remove_thread() is called
751+ rm_func = make_remove_thread(thread_name)
752+
753+ # THEN: The result should be a function
754+ assert isfunction(rm_func), 'make_remove_thread should return a function'
755+ assert rm_func.__name__ == 'remove_thread'