Repository: beeware/briefcase
Pull request #1630 (2024-02-03 20:31:11)

Improve Android emulator startup failure messages
## Changes
- Updates handling and management of information presented to the user when the Android emulator fails to start
- Related issues:
  - #1036
  - #1573

## UX
- Conceptually, I broke the errors in to two categories:
  - Briefcase detects the emulator closed
  - The user aborts the start up process
- In the first case, the emulator's output probably includes the recommended steps for the user.
- In the second case, Briefcase probably won't be able to retrieve the emulator's output because the emulator will likely still be running....and retrieving `stdout` from a running process without blocking is non-trivial....but potentially really valuable here

Presentation when Briefcase detects startup failure:
![image](https://github.com/beeware/briefcase/assets/387848/68f1ca21-ff04-45df-ab96-9dfa55fb7688)

Presentation when the User aborts the startup:
![image](https://github.com/beeware/briefcase/assets/387848/5735d8f2-6db7-4782-bc31-74ac9c920b7c)

## 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/1627.bugfix.rst b/changes/1627.bugfix.rst
deleted file mode 100644
index d5e474ff..00000000
--- a/changes/1627.bugfix.rst
+++ /dev/null
@@ -1,1 +0,0 @@
-The detection of physical Android devices on macOS was made more resilient.
diff --git a/changes/1630.feature.rst b/changes/1630.feature.rst
new file mode 100644
index 00000000..5c67090b
--- /dev/null
+++ b/changes/1630.feature.rst
@@ -0,0 +1,1 @@
+When the Android emulator fails to start up properly, users are now presented with additional resources to help resolve any issues.
diff --git a/src/briefcase/integrations/android_sdk.py b/src/briefcase/integrations/android_sdk.py
index 1ab02650..78759388 100644
--- a/src/briefcase/integrations/android_sdk.py
+++ b/src/briefcase/integrations/android_sdk.py
@@ -8,7 +8,6 @@ import shutil
 import subprocess
 import sys
 import time
-from contextlib import suppress
 from datetime import datetime
 from pathlib import Path
 
@@ -864,12 +863,8 @@ connection.
 
                     details = {}
                     for part in parts[2:]:
-                        try:
-                            key, value = part.split(":")
-                            details[key] = value
-                        except ValueError:
-                            # Ignore any entry that isn't in "key:value" format.
-                            pass
+                        key, value = part.split(":")
+                        details[key] = value
 
                     if parts[1] == "device":
                         try:
@@ -881,7 +876,7 @@ connection.
                         name = "Unknown device (offline)"
                         authorized = False
                     else:
-                        name = f"Device not available for development ({' '.join(parts[1:])})"
+                        name = "Unknown device (not authorized for development)"
                         authorized = False
 
                     devices[parts[0]] = {
@@ -1310,20 +1305,25 @@ In future, you can specify this device by running:
             for arg in map(str, emulator_popen.args)
         )
 
-        error_msg = (
-            "{prologue}"
-            + f"""
+        general_error_msg = f"""
+Review the emulator output above for:
+ - Troubleshooting or resolution steps such as enabling hardware acceleration
+ - Other errors or warnings that may be suggesting the cause of the startup failure
 
-Try starting the emulator manually by running:
+Ensure your Android SDK is up-to-date by running:
 
-    $ {emulator_command}
+    $ briefcase update {AndroidSDK.name}
+
+To review Google's general troubleshooting steps for the emulator, visit:
+
+    https://developer.android.com/studio/run/emulator-troubleshooting
 
-Resolve any problems you discover, then try running your app again. You may
-find this page helpful in diagnosing emulator problems.
+You can also start the emulator manually by running:
 
-    https://developer.android.com/studio/run/emulator-acceleration#accel-vm
+    $ {emulator_command}
 """
-        )
+
+        failed_startup_error_msg = f"{{prologue}}\n{general_error_msg}"
 
         # The boot process happens in 2 phases.
         # First, the emulator appears in the device list. However, it's
@@ -1339,7 +1339,7 @@ find this page helpful in diagnosing emulator problems.
                 while adb is None:
                     if emulator_popen.poll() is not None:
                         raise BriefcaseCommandError(
-                            error_msg.format(
+                            failed_startup_error_msg.format(
                                 prologue="Android emulator was unable to start!"
                             )
                         )
@@ -1370,7 +1370,7 @@ find this page helpful in diagnosing emulator problems.
                     while not adb.has_booted():
                         if emulator_popen.poll() is not None:
                             raise BriefcaseCommandError(
-                                error_msg.format(
+                                failed_startup_error_msg.format(
                                     prologue="Android emulator was unable to boot!"
                                 )
                             )
@@ -1378,19 +1378,32 @@ find this page helpful in diagnosing emulator problems.
                         # Try again in 2 seconds...
                         self.sleep(2)
         except BaseException as e:
-            # if the emulator exited, this should return its output immediately;
-            # if it is still running, this will quickly time out and print nothing.
-            with suppress(subprocess.TimeoutExpired):
+            self.tools.logger.warning(
+                "Emulator output log for startup failure",
+                prefix=self.name,
+            )
+            try:
+                # if the emulator exited, this should return its output immediately
                 self.tools.logger.info(emulator_popen.communicate(timeout=1)[0])
+            except subprocess.TimeoutExpired:
+                self.tools.logger.info(
+                    "Briefcase failed to retrieve emulator output "
+                    "(this is expected if the emulator is running)"
+                )
 
             # Provide troubleshooting steps if user gives up on the emulator starting
             if isinstance(e, KeyboardInterrupt):
-                self.tools.logger.warning()
                 self.tools.logger.warning(
-                    error_msg.format(
-                        prologue="Is the Android emulator not starting up properly?"
-                    )
+                    "Is the Android emulator not starting up properly?",
+                    prefix=self.name,
+                )
+                self.tools.logger.info(
+                    """
+If the emulator opened after pressing CTRL+C, then leave the emulator open and
+run Briefcase again. The running emulator can then be selected from the list.
+"""
                 )
+                self.tools.logger.info(general_error_msg)
 
             raise
 


Test patch:
diff --git a/tests/integrations/android_sdk/AndroidSDK/devices/no_permissions.out b/tests/integrations/android_sdk/AndroidSDK/devices/no_permissions.out
deleted file mode 100644
index f255cd0c..00000000
--- a/tests/integrations/android_sdk/AndroidSDK/devices/no_permissions.out
+++ /dev/null
@@ -1,3 +0,0 @@
-List of devices attached
-200ABCDEFGHIJK         no permissions (user russell is not in the plugdev group); see [http://developer.android.com/tools/device.html] usb:5-4.4.1 transport_id:1
-300ABCDEFGHIJK         no permissions (missing udev rules? user is in the plugdev group); see [http://developer.android.com/tools/device.html] usb:5-4.4.1 transport_id:1
diff --git a/tests/integrations/android_sdk/AndroidSDK/devices/physical_device_macOS.out b/tests/integrations/android_sdk/AndroidSDK/devices/physical_device_macOS.out
deleted file mode 100644
index 4d86d564..00000000
--- a/tests/integrations/android_sdk/AndroidSDK/devices/physical_device_macOS.out
+++ /dev/null
@@ -1,2 +0,0 @@
-List of devices attached
-200ABCDEFGHIJK         device 231-1 product:panther model:Pixel_7 device:panther transport_id:1
diff --git a/tests/integrations/android_sdk/AndroidSDK/test_devices.py b/tests/integrations/android_sdk/AndroidSDK/test_devices.py
index 9605391f..71dbf810 100644
--- a/tests/integrations/android_sdk/AndroidSDK/test_devices.py
+++ b/tests/integrations/android_sdk/AndroidSDK/test_devices.py
@@ -48,10 +48,7 @@ def test_multiple_devices(mock_tools, android_sdk):
 
     assert android_sdk.devices() == {
         "041234567892009a": {
-            "name": (
-                "Device not available for development "
-                "(unauthorized usb:336675328X transport_id:2)"
-            ),
+            "name": "Unknown device (not authorized for development)",
             "authorized": False,
         },
         "KABCDABCDA1513": {
@@ -84,44 +81,3 @@ def test_daemon_start(mock_tools, android_sdk):
     mock_tools.subprocess.check_output.return_value = devices_result("daemon_start")
 
     assert android_sdk.devices() == {}
-
-
-def test_physical_device_macOS(mock_tools, android_sdk):
-    """An extra piece of device detail is returned on macOS for physical devices."""
-    mock_tools.subprocess.check_output.return_value = devices_result(
-        "physical_device_macOS"
-    )
-
-    assert android_sdk.devices() == {
-        "200ABCDEFGHIJK": {
-            "authorized": True,
-            "name": "Pixel 7",
-        }
-    }
-
-
-def test_device_permissions(mock_tools, android_sdk):
-    """If AndroidSDK doesn't have access to the device, the error message can be
-    parsed."""
-    mock_tools.subprocess.check_output.return_value = devices_result("no_permissions")
-
-    assert android_sdk.devices() == {
-        "200ABCDEFGHIJK": {
-            "authorized": False,
-            "name": (
-                "Device not available for development (no permissions "
-                "(user russell is not in the plugdev group); "
-                "see [http://developer.android.com/tools/device.html] "
-                "usb:5-4.4.1 transport_id:1)"
-            ),
-        },
-        "300ABCDEFGHIJK": {
-            "authorized": False,
-            "name": (
-                "Device not available for development (no permissions "
-                "(missing udev rules? user is in the plugdev group); "
-                "see [http://developer.android.com/tools/device.html] "
-                "usb:5-4.4.1 transport_id:1)"
-            ),
-        },
-    }
diff --git a/tests/integrations/android_sdk/AndroidSDK/test_start_emulator.py b/tests/integrations/android_sdk/AndroidSDK/test_start_emulator.py
index 9237af52..a0339daf 100644
--- a/tests/integrations/android_sdk/AndroidSDK/test_start_emulator.py
+++ b/tests/integrations/android_sdk/AndroidSDK/test_start_emulator.py
@@ -423,7 +423,20 @@ def test_emulator_fail_to_boot(mock_tools, android_sdk):
     assert "Android emulator was unable to boot!" in exc_info.value.msg
 
 
-def test_emulator_ctrl_c(mock_tools, android_sdk, capsys):
+@pytest.mark.parametrize(
+    "emulator_comm, expected_log",
+    [
+        (
+            ("This is stdout", "this is stderr"),
+            "Emulator output log for startup failure\nThis is stdout",
+        ),
+        (
+            subprocess.TimeoutExpired(cmd="emulator", timeout=1),
+            "Emulator output log for startup failure\nBriefcase failed",
+        ),
+    ],
+)
+def test_emulator_ctrl_c(mock_tools, android_sdk, emulator_comm, expected_log, capsys):
     """If emulator startup is interrupted by the user, an error is displayed."""
     # Short circuit device loop by returning no devices
     android_sdk.devices = MagicMock(side_effect=[{}, {}])
@@ -435,6 +448,9 @@ def test_emulator_ctrl_c(mock_tools, android_sdk, capsys):
     emu_popen.poll.side_effect = [None, None, KeyboardInterrupt]
     mock_tools.subprocess.Popen.return_value = emu_popen
 
+    # Mock emulator output
+    emu_popen.communicate.side_effect = [emulator_comm]
+
     # Start the emulator
     with pytest.raises(KeyboardInterrupt):
         android_sdk.start_emulator("idleEmulator")
@@ -457,10 +473,13 @@ def test_emulator_ctrl_c(mock_tools, android_sdk, capsys):
     # Took a total of 2 naps before KeyboardInterrupt.
     assert android_sdk.sleep.call_count == 2
 
+    output = capsys.readouterr().out
+
+    # Emulator's log was printed.
+    assert expected_log in output
+
     # Expected error message was printed.
-    assert (
-        "Is the Android emulator not starting up properly?" in capsys.readouterr().out
-    )
+    assert "Is the Android emulator not starting up properly?" in output
 
 
 def test_start_emulator_extra_args(mock_tools, android_sdk):