Display the closing progress dialog during plugin shutdown

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:

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
 
 

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()