Merge pull request #1541 from rmartin16/log-stream-exit-wait

Wait for app to exit normally after log streaming

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.

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
@@ -131,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.
@@ -158,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.
@@ -186,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:

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