From b80680772028a254b205c7414e5714c202f934a0 Mon Sep 17 00:00:00 2001 From: Russell Martin Date: Wed, 15 Nov 2023 09:58:53 -0500 Subject: [PATCH 1/2] Wait for app to exit normally after log streaming --- changes/1541.misc.rst | 1 + src/briefcase/commands/run.py | 10 ++++++++++ 2 files changed, 11 insertions(+) create mode 100644 changes/1541.misc.rst diff --git a/changes/1541.misc.rst b/changes/1541.misc.rst new file mode 100644 index 000000000..6043dbdc3 --- /dev/null +++ b/changes/1541.misc.rst @@ -0,0 +1 @@ +Once the exit condition is detected while log streaming, the app is now waited upon to exit normally instead of signaling it to exit immediately. diff --git a/src/briefcase/commands/run.py b/src/briefcase/commands/run.py index 6a54b499d..27968b87b 100644 --- a/src/briefcase/commands/run.py +++ b/src/briefcase/commands/run.py @@ -1,7 +1,9 @@ from __future__ import annotations import re +import subprocess from abc import abstractmethod +from contextlib import suppress from briefcase.config import AppConfig from briefcase.exceptions import BriefcaseCommandError, BriefcaseTestSuiteFailure @@ -79,6 +81,14 @@ def __call__(self, line): if self.exit_filter: self.returncode = self.exit_filter(tail) if self.returncode is not None: + # This returncode is captured from output from the app and does not + # necessarily mean the app has already exited. However, once + # StopStreaming is raised, the output streamer sends a signal to + # the app to exit immediately and that may result in the app + # exiting with a non-zero returncode. Therefore, wait for the app + # to close normally before raising StopStreaming. + with suppress(subprocess.TimeoutExpired): + self.log_popen.wait(timeout=3) raise StopStreaming() # Return the display line From 303331f9a67810d44c5e3b2d02285168a68e4d68 Mon Sep 17 00:00:00 2001 From: Russell Martin Date: Wed, 15 Nov 2023 10:25:45 -0500 Subject: [PATCH 2/2] Close log streaming `Popen` process and poll for returncode --- src/briefcase/commands/run.py | 18 ++++++++++-------- tests/commands/run/test__stream_app_logs.py | 4 ++-- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/briefcase/commands/run.py b/src/briefcase/commands/run.py index 27968b87b..4977b3419 100644 --- a/src/briefcase/commands/run.py +++ b/src/briefcase/commands/run.py @@ -141,7 +141,8 @@ def _stream_app_logs( Catches and cleans up after any Ctrl-C interrupts. :param app: The app to be launched - :param popen: The Popen object for the stream we are monitoring + :param popen: The Popen object for the stream we are monitoring; this Popen + process will be closed after log streaming completes. :param test_mode: Are we launching in test mode? :param clean_filter: The log cleaning filter to use; see ``LogFilter`` for details. @@ -168,12 +169,13 @@ def _stream_app_logs( # Start streaming logs for the app. self.logger.info("=" * 75) - self.tools.subprocess.stream_output( - label="log stream" if log_stream else app.app_name, - popen_process=popen, - stop_func=stop_func, - filter_func=log_filter, - ) + with popen: + self.tools.subprocess.stream_output( + label="log stream" if log_stream else app.app_name, + popen_process=popen, + stop_func=stop_func, + filter_func=log_filter, + ) # If we're in test mode, and log streaming ends, # check for the status of the test suite. @@ -196,7 +198,7 @@ def _stream_app_logs( else: # If we're monitoring an actual app (not just a log stream), # and the app didn't exit cleanly, surface the error to the user. - if popen.returncode != 0: + if popen.poll() != 0: raise BriefcaseCommandError(f"Problem running app {app.app_name}.") except KeyboardInterrupt: diff --git a/tests/commands/run/test__stream_app_logs.py b/tests/commands/run/test__stream_app_logs.py index d8b6188c1..74c1287ba 100644 --- a/tests/commands/run/test__stream_app_logs.py +++ b/tests/commands/run/test__stream_app_logs.py @@ -9,7 +9,7 @@ def test_run_app(run_command, first_app): """An app can have its logs streamed.""" popen = mock.MagicMock() - popen.returncode = 0 + popen.poll = mock.MagicMock(return_value=0) clean_filter = mock.MagicMock() # Invoke the stop func as part of streaming. This is to satisfy coverage. @@ -47,7 +47,7 @@ def mock_stream(label, popen_process, stop_func, filter_func): def test_run_app_custom_stop_func(run_command, first_app): """An app with a custom stop function can have its logs streamed.""" popen = mock.MagicMock() - popen.returncode = 0 + popen.poll = mock.MagicMock(return_value=0) clean_filter = mock.MagicMock() stop_func = mock.MagicMock() run_command.tools.subprocess.stream_output = mock.MagicMock()