Merge pull request #6619 from kozlovsky/fix/gui_tests_stability

Fix gui tests stability

src/run_tribler.py

@@ -93,7 +93,7 @@ def init_boot_logger():
         version_history = VersionHistory(root_state_dir)
         state_dir = version_history.code_version.directory
         try:
-            start_core.start_tribler_core(api_port, api_key, state_dir, gui_test_mode=parsed_args.gui_test_mode)
+            start_core.run_tribler_core(api_port, api_key, state_dir, gui_test_mode=parsed_args.gui_test_mode)
         finally:
             logger.info('Remove lock file')
             process_checker.remove_lock_file()

src/tribler-core/tribler_core/start_core.py

@@ -104,7 +104,7 @@ async def core_session(config: TriblerConfig, components: List[Component]):
         config.write()
 
 
-def start_tribler_core(api_port, api_key, state_dir, gui_test_mode=False):
+def run_tribler_core(api_port, api_key, state_dir, gui_test_mode=False):
     """
     This method will start a new Tribler session.
     Note that there is no direct communication between the GUI process and the core: all communication is performed

src/tribler-core/tribler_core/tests/test_start_core.py

@@ -1,6 +1,6 @@
 from unittest.mock import MagicMock, patch
 
-from tribler_core.start_core import start_tribler_core
+from tribler_core.start_core import run_tribler_core
 from tribler_core.utilities.path_util import Path
 
 # pylint: disable=
@@ -15,5 +15,5 @@
 @patch('tribler_core.start_core.core_session')
 def test_start_tribler_core_no_exceptions(mocked_core_session):
     # test that base logic of tribler core runs without exceptions
-    start_tribler_core(1, 'key', Path('.'), False)
+    run_tribler_core(1, 'key', Path('.'), False)
     mocked_core_session.assert_called_once()

src/tribler-gui/tribler_gui/core_manager.py

@@ -17,8 +17,6 @@ class CoreManager(QObject):
     a fake API will be started.
     """
 
-    tribler_stopped = pyqtSignal()
-
     def __init__(self, root_state_dir, api_port, api_key, error_handler):
         QObject.__init__(self, None)
 
@@ -30,52 +28,35 @@ def __init__(self, root_state_dir, api_port, api_key, error_handler):
         self.api_key = api_key
         self.events_manager = EventRequestManager(self.api_port, self.api_key, error_handler)
 
+        self.upgrade_manager = None
+        self.core_args = None
+        self.core_env = None
+
+        self.core_started = False
+        self.core_running = False
+        self.core_connected = False
         self.shutting_down = False
-        self.should_stop_on_shutdown = False
+        self.core_finished = False
+        self.quitting_app = False
+
+        self.should_quit_app_on_core_finished = False
+
         self.use_existing_core = True
-        self.is_core_running = False
         self.last_core_stdout_output: str = ''
         self.last_core_stderr_output: str = ''
 
-        connect(self.events_manager.tribler_started, self._set_core_running)
+        connect(self.events_manager.tribler_started, self.on_core_connected)
+        app = QApplication.instance()
+        if app is not None:
+            # app can be None in tests where Qt application is not created
+            connect(app.aboutToQuit, self.on_about_to_quit)
 
-    def _set_core_running(self, _):
-        self.is_core_running = True
+    def on_about_to_quit(self):
+        self.quitting_app = True
 
-    def on_core_stdout_read_ready(self):
-        raw_output = bytes(self.core_process.readAllStandardOutput())
-        self.last_core_stdout_output = raw_output.decode("utf-8").strip()
-        try:
-            print(self.last_core_stdout_output)  # print core output # noqa: T001
-        except OSError:
-            # Possible reason - cannot write to stdout as it was already closed during the application shutdown
-            if not self.shutting_down:
-                raise
-
-    def on_core_stderr_read_ready(self):
-        raw_output = bytes(self.core_process.readAllStandardError())
-        self.last_core_stderr_output = raw_output.decode("utf-8").strip()
-        try:
-            print(self.last_core_stderr_output, file=sys.stderr)  # print core output # noqa: T001
-        except OSError:
-            # Possible reason - cannot write to stdout as it was already closed during the application shutdown
-            if not self.shutting_down:
-                raise
-
-    def on_core_finished(self, exit_code, exit_status):
-        if self.shutting_down and self.should_stop_on_shutdown:
-            self.on_finished()
-        elif not self.shutting_down and exit_code != 0:
-            # Stop the event manager loop if it is running
-            if self.events_manager.connect_timer and self.events_manager.connect_timer.isActive():
-                self.events_manager.connect_timer.stop()
-
-            exception_message = (
-                f"The Tribler core has unexpectedly finished with exit code {exit_code} and status: {exit_status}!\n"
-                f"Last core output: \n {self.last_core_stderr_output or self.last_core_stdout_output}"
-            )
-
-            raise CoreCrashedError(exception_message)
+    def on_core_connected(self, _):
+        if not self.core_finished:
+            self.core_connected = True
 
     def start(self, core_args=None, core_env=None, upgrade_manager=None, run_core=True):
         """
@@ -86,49 +67,108 @@ def start(self, core_args=None, core_env=None, upgrade_manager=None, run_core=Tr
         self.events_manager.connect()
 
         if run_core:
-
-            def on_request_error(_):
-                if upgrade_manager:
-                    # Start Tribler Upgrader. When it finishes, start Tribler Core
-                    connect(
-                        upgrade_manager.upgrader_finished,
-                        lambda: self.start_tribler_core(core_args=core_args, core_env=core_env),
-                    )
-                    upgrade_manager.start()
-                else:
-                    self.start_tribler_core(core_args=core_args, core_env=core_env)
-
-            connect(self.events_manager.reply.error, on_request_error)
-
-    def start_tribler_core(self, core_args=None, core_env=None):
+            self.core_args = core_args
+            self.core_env = core_env
+            self.upgrade_manager = upgrade_manager
+            connect(self.events_manager.reply.error, self.on_event_manager_initial_error)
+
+    def on_event_manager_initial_error(self, _):
+        if self.upgrade_manager:
+            # Start Tribler Upgrader. When it finishes, start Tribler Core
+            connect(self.upgrade_manager.upgrader_finished, self.start_tribler_core)
+            self.upgrade_manager.start()
+        else:
+            self.start_tribler_core()
+
+    def start_tribler_core(self):
         self.use_existing_core = False
+
+        core_env = self.core_env
         if not core_env:
             core_env = QProcessEnvironment.systemEnvironment()
             core_env.insert("CORE_API_PORT", f"{self.api_port}")
             core_env.insert("CORE_API_KEY", self.api_key)
             core_env.insert("TSTATEDIR", str(self.root_state_dir))
+
+        core_args = self.core_args
         if not core_args:
             core_args = sys.argv + ['--core']
 
         self.core_process = QProcess()
         self.core_process.setProcessEnvironment(core_env)
         self.core_process.setProcessChannelMode(QProcess.SeparateChannels)
+        connect(self.core_process.started, self.on_core_started)
         connect(self.core_process.readyReadStandardOutput, self.on_core_stdout_read_ready)
         connect(self.core_process.readyReadStandardError, self.on_core_stderr_read_ready)
         connect(self.core_process.finished, self.on_core_finished)
         self.core_process.start(sys.executable, core_args)
 
-    def stop(self, stop_app_on_shutdown=True):
+    def on_core_started(self):
+        self.core_started = True
+        self.core_running = True
+
+    def on_core_stdout_read_ready(self):
+        if self.quitting_app:
+            # Reading at this stage can lead to the error "wrapped C/C++ object of type QProcess has been deleted"
+            return
+
+        raw_output = bytes(self.core_process.readAllStandardOutput())
+        self.last_core_stdout_output = raw_output.decode("utf-8").strip()
+
+        try:
+            print(self.last_core_stdout_output)  # print core output # noqa: T001
+        except OSError:
+            # Possible reason - cannot write to stdout as it was already closed during the application shutdown
+            pass
+
+    def on_core_stderr_read_ready(self):
+        if self.quitting_app:
+            # Reading at this stage can lead to the error "wrapped C/C++ object of type QProcess has been deleted"
+            return
+
+        raw_output = bytes(self.core_process.readAllStandardError())
+        self.last_core_stderr_output = raw_output.decode("utf-8").strip()
+
+        try:
+            print(self.last_core_stderr_output, file=sys.stderr)  # print core output # noqa: T001
+        except OSError:
+            # Possible reason - cannot write to stdout as it was already closed during the application shutdown
+            pass
+
+    def stop(self, quit_app_on_core_finished=True):
+        if quit_app_on_core_finished:
+            self.should_quit_app_on_core_finished = True
+
+        if self.shutting_down:
+            return
+
+        self.shutting_down = True
         self._logger.info("Stopping Core manager")
-        if self.core_process or self.is_core_running:
+        if self.core_process or self.core_connected:
             self._logger.info("Sending shutdown request to Tribler Core")
             self.events_manager.shutting_down = True
             TriblerNetworkRequest("shutdown", lambda _: None, method="PUT", priority=QNetworkRequest.HighPriority)
 
-            if stop_app_on_shutdown:
-                self.should_stop_on_shutdown = True
-
-    def on_finished(self):
-        self.tribler_stopped.emit()
+    def on_core_finished(self, exit_code, exit_status):
+        self.core_running = False
+        self.core_finished = True
         if self.shutting_down:
+            if self.should_quit_app_on_core_finished:
+                self.quit_application()
+        else:
+            error_message = (
+                f"The Tribler core has unexpectedly finished with exit code {exit_code} and status: {exit_status}!\n"
+                f"Last core output: \n {self.last_core_stderr_output or self.last_core_stdout_output}"
+            )
+            self._logger.warning(error_message)
+
+            # Stop the event manager loop if it is running
+            if self.events_manager.connect_timer and self.events_manager.connect_timer.isActive():
+                self.events_manager.connect_timer.stop()
+
+            raise CoreCrashedError(error_message)
+
+    def quit_application(self):
+        if not self.quitting_app:
+            self.quitting_app = True
             QApplication.quit()

src/tribler-gui/tribler_gui/error_handler.py

@@ -89,8 +89,7 @@ def _stop_tribler(self, text):
         self.tribler_window.downloads_page.stop_loading_downloads()
 
         # Add info about whether we are stopping Tribler or not
-        if not self.tribler_window.core_manager.shutting_down:
-            self.tribler_window.core_manager.stop(stop_app_on_shutdown=False)
+        self.tribler_window.core_manager.stop(quit_app_on_core_finished=False)
 
         self.tribler_window.setHidden(True)
 

src/tribler-gui/tribler_gui/tests/test_core_manager.py

@@ -9,17 +9,17 @@
 
 # fmt: off
 
-@patch.object(CoreManager, 'on_finished')
+@patch.object(CoreManager, 'quit_application')
 @patch('tribler_gui.core_manager.EventRequestManager', new=MagicMock())
-async def test_on_core_finished_call_on_finished(mocked_on_finished: MagicMock):
-    # test that in case of `shutting_down` and `should_stop_on_shutdown` flags have been set to True
+async def test_on_core_finished_call_on_finished(mocked_quit_application: MagicMock):
+    # test that in case of `shutting_down` and `should_quit_app_on_core_finished` flags have been set to True
     # then `on_finished` function will be called and Exception will not be raised
     core_manager = CoreManager(MagicMock(), MagicMock(), MagicMock(), MagicMock())
     core_manager.shutting_down = True
-    core_manager.should_stop_on_shutdown = True
+    core_manager.should_quit_app_on_core_finished = True
 
     core_manager.on_core_finished(exit_code=1, exit_status='exit status')
-    mocked_on_finished.assert_called_once()
+    mocked_quit_application.assert_called_once()
 
 
 @patch('tribler_gui.core_manager.EventRequestManager', new=MagicMock())
@@ -56,18 +56,11 @@ async def test_on_core_stderr_read_ready(mocked_stderr, mocked_print: MagicMock)
 @patch('tribler_gui.core_manager.EventRequestManager', new=MagicMock())
 @patch('builtins.print', MagicMock(side_effect=OSError()))
 def test_on_core_stdout_stderr_read_ready_os_error():
-    # test that OSError on writing to stdout is suppressed during shutting down
+    # test that OSError on writing to stdout is suppressed when quitting the application
 
     core_manager = CoreManager(MagicMock(), MagicMock(), MagicMock(), MagicMock())
     core_manager.core_process = MagicMock(read_all=MagicMock(return_value=''))
 
-    with pytest.raises(OSError):
-        core_manager.on_core_stdout_read_ready()
-
-    with pytest.raises(OSError):
-        core_manager.on_core_stderr_read_ready()
-
-    core_manager.shutting_down = True
-    # no exception during shutting down
+    # check that OSError exception is suppressed when writing to stdout and stderr
     core_manager.on_core_stdout_read_ready()
     core_manager.on_core_stderr_read_ready()

src/tribler-gui/tribler_gui/tribler_window.py

@@ -419,7 +419,7 @@ def close_tribler_gui():
                 QApplication.quit()
 
         self.downloads_page.stop_loading_downloads()
-        self.core_manager.stop(False)
+        self.core_manager.stop(quit_app_on_core_finished=False)
         close_dialog = ConfirmationDialog(
             self.window(),
             tr("<b>CRITICAL ERROR</b>"),
@@ -1099,7 +1099,6 @@ def show_force_shutdown():
             QApplication.quit()
 
         self.core_manager.stop()
-        self.core_manager.shutting_down = True
         self.downloads_page.stop_loading_downloads()
         request_manager.clear()