Repository: beeware/briefcase
Pull request #1543 (2023-11-16 13:44:56)

Upgrade RCEdit from v1.1.1 to v2.0.0
## Changes
- Start using RCEdit [v2.0.0](https://github.com/electron/rcedit/releases/tag/v2.0.0) released recently

## PR Checklist:
<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
- [X] All new features have been tested
- [X] All new features have been documented
- [X] I have read the **CONTRIBUTING.md** file
- [X] I will abide by the code of conduct


Patch:
diff --git a/changes/1541.misc.rst b/changes/1541.misc.rst
deleted file mode 100644
index 6043dbdc..00000000
--- a/changes/1541.misc.rst
+++ /dev/null
@@ -1,1 +0,0 @@
-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/changes/1543.feature.rst b/changes/1543.feature.rst
new file mode 100644
index 00000000..5bc6aba0
--- /dev/null
+++ b/changes/1543.feature.rst
@@ -0,0 +1,1 @@
+RCEdit v2.0.0 is now used to build Windows apps. Run ``briefcase upgrade`` to use this latest version.
diff --git a/src/briefcase/commands/run.py b/src/briefcase/commands/run.py
index 4977b341..6a54b499 100644
--- a/src/briefcase/commands/run.py
+++ b/src/briefcase/commands/run.py
@@ -1,9 +1,7 @@
 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
@@ -81,14 +79,6 @@ class LogFilter:
             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
@@ -141,8 +131,7 @@ class RunAppMixin:
         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; this Popen
-            process will be closed after log streaming completes.
+        :param popen: The Popen object for the stream we are monitoring
         :param test_mode: Are we launching in test mode?
         :param clean_filter: The log cleaning filter to use; see ``LogFilter``
             for details.
@@ -169,13 +158,12 @@ class RunAppMixin:
 
             # Start streaming logs for the app.
             self.logger.info("=" * 75)
-            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,
-                )
+            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.
@@ -198,7 +186,7 @@ class RunAppMixin:
             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.poll() != 0:
+                if popen.returncode != 0:
                     raise BriefcaseCommandError(f"Problem running app {app.app_name}.")
 
         except KeyboardInterrupt:
diff --git a/src/briefcase/integrations/rcedit.py b/src/briefcase/integrations/rcedit.py
index e59189f9..59b3f2d8 100644
--- a/src/briefcase/integrations/rcedit.py
+++ b/src/briefcase/integrations/rcedit.py
@@ -14,7 +14,7 @@ class RCEdit(ManagedTool):
     @property
     def download_url(self) -> str:
         return (
-            "https://github.com/electron/rcedit/releases/download/v1.1.1/rcedit-x64.exe"
+            "https://github.com/electron/rcedit/releases/download/v2.0.0/rcedit-x64.exe"
         )
 
     @property


Test patch:
diff --git a/tests/commands/run/test__stream_app_logs.py b/tests/commands/run/test__stream_app_logs.py
index 74c1287b..d8b6188c 100644
--- a/tests/commands/run/test__stream_app_logs.py
+++ b/tests/commands/run/test__stream_app_logs.py
@@ -9,7 +9,7 @@ from briefcase.exceptions import BriefcaseCommandError, BriefcaseTestSuiteFailur
 def test_run_app(run_command, first_app):
     """An app can have its logs streamed."""
     popen = mock.MagicMock()
-    popen.poll = mock.MagicMock(return_value=0)
+    popen.returncode = 0
     clean_filter = mock.MagicMock()
 
     # Invoke the stop func as part of streaming. This is to satisfy coverage.
@@ -47,7 +47,7 @@ def test_run_app(run_command, first_app):
 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.poll = mock.MagicMock(return_value=0)
+    popen.returncode = 0
     clean_filter = mock.MagicMock()
     stop_func = mock.MagicMock()
     run_command.tools.subprocess.stream_output = mock.MagicMock()
diff --git a/tests/integrations/rcedit/test_RCEdit__upgrade.py b/tests/integrations/rcedit/test_RCEdit__upgrade.py
index ba98dcac..3403d16f 100644
--- a/tests/integrations/rcedit/test_RCEdit__upgrade.py
+++ b/tests/integrations/rcedit/test_RCEdit__upgrade.py
@@ -26,7 +26,7 @@ def test_upgrade_exists(mock_tools, rcedit, tmp_path):
     # A download is invoked
     mock_tools.download.file.assert_called_with(
         url="https://github.com/electron/rcedit/"
-        "releases/download/v1.1.1/rcedit-x64.exe",
+        "releases/download/v2.0.0/rcedit-x64.exe",
         download_path=tmp_path / "tools",
         role="RCEdit",
     )
@@ -60,7 +60,7 @@ def test_upgrade_rcedit_download_failure(mock_tools, rcedit, tmp_path):
     # A download was invoked
     mock_tools.download.file.assert_called_with(
         url="https://github.com/electron/rcedit/"
-        "releases/download/v1.1.1/rcedit-x64.exe",
+        "releases/download/v2.0.0/rcedit-x64.exe",
         download_path=tmp_path / "tools",
         role="RCEdit",
     )
diff --git a/tests/integrations/rcedit/test_RCEdit__verify.py b/tests/integrations/rcedit/test_RCEdit__verify.py
index 887dbc65..801cc1e7 100644
--- a/tests/integrations/rcedit/test_RCEdit__verify.py
+++ b/tests/integrations/rcedit/test_RCEdit__verify.py
@@ -76,7 +76,7 @@ def test_verify_does_not_exist(mock_tools, tmp_path):
     # A download is invoked
     mock_tools.download.file.assert_called_with(
         url="https://github.com/electron/rcedit/"
-        "releases/download/v1.1.1/rcedit-x64.exe",
+        "releases/download/v2.0.0/rcedit-x64.exe",
         download_path=tmp_path / "tools",
         role="RCEdit",
     )
@@ -95,7 +95,7 @@ def test_verify_rcedit_download_failure(mock_tools, tmp_path):
     # A download was invoked
     mock_tools.download.file.assert_called_with(
         url="https://github.com/electron/rcedit/"
-        "releases/download/v1.1.1/rcedit-x64.exe",
+        "releases/download/v2.0.0/rcedit-x64.exe",
         download_path=tmp_path / "tools",
         role="RCEdit",
     )