From 1ee8223d55ab37dbe8fc6feb5be97bdc88fd4a99 Mon Sep 17 00:00:00 2001 From: Raoul Snyman Date: Wed, 18 Jan 2023 21:57:55 -0700 Subject: [PATCH] Display the closing progress dialog during plugin shutdown --- openlp/core/ui/mainwindow.py | 42 ++++++--- tests/conftest.py | 20 ++-- tests/openlp_core/ui/test_mainwindow.py | 116 ++++++++++++++++++++++++ 3 files changed, 154 insertions(+), 24 deletions(-) diff --git a/openlp/core/ui/mainwindow.py b/openlp/core/ui/mainwindow.py index 772df3045..f5ff34a0b 100644 --- a/openlp/core/ui/mainwindow.py +++ b/openlp/core/ui/mainwindow.py @@ -22,6 +22,7 @@ This is the main window, where all the action happens. """ import shutil +from contextlib import contextmanager from datetime import datetime, date from pathlib import Path from tempfile import gettempdir @@ -529,20 +530,32 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, LogMixin, RegistryPropert self.ws_server = WebSocketServer() self.screen_updating_lock = Lock() + @contextmanager + def _show_wait_dialog(self, title, message): + """ + Show a wait dialog, wait for some tasks to complete, and then close it. + """ + try: + # Display a progress dialog with a message + wait_dialog = QtWidgets.QProgressDialog(message, '', 0, 0, self) + wait_dialog.setWindowTitle(title) + for window_flag in [QtCore.Qt.WindowContextHelpButtonHint]: + wait_dialog.setWindowFlag(window_flag, False) + wait_dialog.setWindowModality(QtCore.Qt.WindowModal) + wait_dialog.setAutoClose(False) + wait_dialog.setCancelButton(None) + wait_dialog.show() + QtWidgets.QApplication.processEvents() + yield + finally: + # Finally close the message window + wait_dialog.close() + def _wait_for_threads(self): """ Wait for the threads """ # Sometimes the threads haven't finished, let's wait for them - wait_dialog = QtWidgets.QProgressDialog(translate('OpenLP.MainWindow', 'Waiting for some things to finish...'), - '', 0, 0, self) - wait_dialog.setWindowTitle(translate('OpenLP.MainWindow', 'Please Wait')) - for window_flag in [QtCore.Qt.WindowContextHelpButtonHint]: - wait_dialog.setWindowFlag(window_flag, False) - wait_dialog.setWindowModality(QtCore.Qt.WindowModal) - wait_dialog.setAutoClose(False) - wait_dialog.setCancelButton(None) - wait_dialog.show() thread_names = list(self.application.worker_threads.keys()) for thread_name in thread_names: if thread_name not in self.application.worker_threads.keys(): @@ -569,7 +582,6 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, LogMixin, RegistryPropert except RuntimeError: # Ignore the RuntimeError that is thrown when Qt has already deleted the C++ thread object pass - wait_dialog.close() def bootstrap_post_set_up(self): """ @@ -1085,10 +1097,12 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, LogMixin, RegistryPropert else: event.accept() if event.isAccepted(): - # Wait for all the threads to complete - self._wait_for_threads() - # If we just did a settings import, close without saving changes. - self.clean_up(save_settings=not self.settings_imported) + with self._show_wait_dialog(translate('OpenLP.MainWindow', 'Please Wait'), + translate('OpenLP.MainWindow', 'Waiting for some things to finish...')): + # Wait for all the threads to complete + self._wait_for_threads() + # If we just did a settings import, close without saving changes. + self.clean_up(save_settings=not self.settings_imported) def eventFilter(self, obj, event): if event.type() == QtCore.QEvent.FileOpen: diff --git a/tests/conftest.py b/tests/conftest.py index d78e8e79b..00d997b67 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -78,14 +78,14 @@ def settings(qapp, registry): # Needed on windows to make sure a Settings object is available during the tests sets = Settings() sets.setValue('themes/global theme', 'my_theme') - Registry().register('settings', sets) - Registry().register('settings_thread', sets) - Registry().register('application', qapp) + registry.register('settings', sets) + registry.register('settings_thread', sets) + registry.register('application', qapp) qapp.settings = sets yield sets del sets - Registry().remove('settings') - Registry().remove('settings_thread') + registry.remove('settings') + registry.remove('settings_thread') os.close(fd) os.unlink(Settings().fileName()) @@ -95,12 +95,12 @@ def mock_settings(qapp, registry): """A Mock Settings() instance""" # Create and register a mock settings object to work with mk_settings = MagicMock() - Registry().register('settings', mk_settings) - Registry().register('application', qapp) - Registry().register('settings_thread', mk_settings) + registry.register('settings', mk_settings) + registry.register('application', qapp) + registry.register('settings_thread', mk_settings) yield mk_settings - Registry().remove('settings') - Registry().remove('settings_thread') + registry.remove('settings') + registry.remove('settings_thread') del mk_settings diff --git a/tests/openlp_core/ui/test_mainwindow.py b/tests/openlp_core/ui/test_mainwindow.py index b0efde327..491fc4ae5 100644 --- a/tests/openlp_core/ui/test_mainwindow.py +++ b/tests/openlp_core/ui/test_mainwindow.py @@ -831,3 +831,119 @@ def test_update_recent_files_menu(mocked_create_action, mocked_add_actions, Mock # THEN: There should be no errors assert mocked_create_action.call_count == 2 + + +@patch('openlp.core.ui.mainwindow.QtWidgets.QProgressDialog') +def test_show_wait_dialog(MockProcessDialog, main_window_reduced): + """Test that the show wait dialog context manager works correctly""" + # GIVEN: A mocked out QProgressDialog and a minimal main window + mocked_wait_dialog = MagicMock() + MockProcessDialog.return_value = mocked_wait_dialog + + # WHEN: Calling _show_wait_dialog() + with main_window_reduced._show_wait_dialog('Test', 'This is a test'): + pass + + # THEN: The correct methods should have been called + MockProcessDialog.assert_called_once_with('This is a test', '', 0, 0, main_window_reduced) + mocked_wait_dialog.setWindowTitle.assert_called_once_with('Test') + mocked_wait_dialog.setWindowFlag.assert_called_once_with(QtCore.Qt.WindowContextHelpButtonHint, False) + mocked_wait_dialog.setWindowModality.assert_called_once_with(QtCore.Qt.WindowModal) + mocked_wait_dialog.setAutoClose.assert_called_once_with(False) + mocked_wait_dialog.setCancelButton.assert_called_once_with(None) + mocked_wait_dialog.show.assert_called_once_with() + mocked_wait_dialog.close.assert_called_once_with() + + +@patch('openlp.core.ui.mainwindow.QtWidgets.QApplication') +def test_wait_for_threads(MockApp, main_window_reduced): + """Test that the wait_for_threads() method correctly stops the threads""" + # GIVEN: A mocked application, and a reduced main window + mocked_http_thread = MagicMock() + mocked_http_thread.isRunning.side_effect = [True, True, False, False] + mocked_http_worker = MagicMock() + mocked_http_worker.stop = MagicMock() + main_window_reduced.application.worker_threads = { + 'http': {'thread': mocked_http_thread, 'worker': mocked_http_worker} + } + + # WHEN: _wait_for_threads() is called + main_window_reduced._wait_for_threads() + + # THEN: The correct methods should have been called + assert MockApp.processEvents.call_count == 2, 'processEvents() should have been called twice' + mocked_http_worker.stop.assert_called_once() + assert mocked_http_thread.isRunning.call_count == 4, 'isRunning() should have been called 4 times' + mocked_http_thread.wait.assert_called_once_with(100) + + +@patch('openlp.core.ui.mainwindow.QtWidgets.QApplication') +def test_wait_for_threads_no_threads(MockApp, main_window_reduced): + """Test that the wait_for_threads() method exits early when there are no threads""" + # GIVEN: A mocked application, and a reduced main window + main_window_reduced.application.worker_threads = {} + + # WHEN: _wait_for_threads() is called + main_window_reduced._wait_for_threads() + + # THEN: The correct methods should have been called + assert MockApp.processEvents.call_count == 0, 'processEvents() should not have been called' + + +@patch('openlp.core.ui.mainwindow.QtWidgets.QApplication') +def test_wait_for_threads_disappearing_thread(MockApp, main_window_reduced): + """Test that the wait_for_threads() method correctly ignores threads that resolve themselves""" + # GIVEN: A mocked application, and a reduced main window + main_window_reduced.application.worker_threads = MagicMock(**{'keys.side_effect': [['http'], []]}) + + # WHEN: _wait_for_threads() is called + main_window_reduced._wait_for_threads() + + # THEN: The correct methods should have been called + assert MockApp.processEvents.call_count == 0, 'processEvents() should not have been called' + + +@patch('openlp.core.ui.mainwindow.QtWidgets.QApplication') +def test_wait_for_threads_stuck_thread(MockApp, main_window_reduced): + """Test that the wait_for_threads() method correctly stops the threads""" + # GIVEN: A mocked application, and a reduced main window + mocked_http_thread = MagicMock() + mocked_http_thread.isRunning.return_value = True + mocked_http_worker = MagicMock() + mocked_http_worker.stop = MagicMock() + main_window_reduced.application.worker_threads = { + 'http': {'thread': mocked_http_thread, 'worker': mocked_http_worker} + } + + # WHEN: _wait_for_threads() is called + main_window_reduced._wait_for_threads() + + # THEN: The correct methods should have been called + assert MockApp.processEvents.call_count == 51, 'processEvents() should have been called 51 times' + mocked_http_worker.stop.assert_called_once() + assert mocked_http_thread.isRunning.call_count == 53, 'isRunning() should have been called 53 times' + mocked_http_thread.wait.assert_called_with(100) + mocked_http_thread.terminate.assert_called_once() + + +@patch('openlp.core.ui.mainwindow.QtWidgets.QApplication') +def test_wait_for_threads_runtime_error(MockApp, main_window_reduced): + """Test that the wait_for_threads() method handles a runtime error""" + # GIVEN: A mocked application, and a reduced main window + mocked_http_thread = MagicMock() + mocked_http_thread.isRunning.side_effect = RuntimeError + mocked_http_worker = MagicMock() + mocked_http_worker.stop = MagicMock() + main_window_reduced.application.worker_threads = { + 'http': {'thread': mocked_http_thread, 'worker': mocked_http_worker} + } + + # WHEN: _wait_for_threads() is called + main_window_reduced._wait_for_threads() + + # THEN: The correct methods should have been called + assert MockApp.processEvents.call_count == 1, 'processEvents() should have been called once' + mocked_http_worker.stop.assert_called_once() + assert mocked_http_thread.isRunning.call_count == 1, 'isRunning() should have been called once' + mocked_http_thread.wait.assert_not_called() + mocked_http_thread.terminate.assert_not_called()