From 6b553d23a11bfd42bccc075aa1923c54d8a1950c Mon Sep 17 00:00:00 2001 From: Helio Perroni Filho Date: Fri, 19 Jul 2024 16:44:51 -0400 Subject: [PATCH 1/5] Added option to run ADB in no-streaming mode ADB can install an app either directly from the host ("streamed install"), or by copying the APK to the device and then running the installation process. The former is the current default, but the latter is still needed for some devices. Added an option allowing streamed install to be disabled. Signed-off-by: Helio Perroni Filho --- src/briefcase/integrations/android_sdk.py | 10 ++++++++-- src/briefcase/platforms/android/gradle.py | 10 +++++++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/briefcase/integrations/android_sdk.py b/src/briefcase/integrations/android_sdk.py index ac35e3ca5..eed7b0c46 100644 --- a/src/briefcase/integrations/android_sdk.py +++ b/src/briefcase/integrations/android_sdk.py @@ -1493,14 +1493,20 @@ def run(self, *arguments: SubprocessArgT, quiet: bool = False) -> str: raise InvalidDeviceError("device id", self.device) from e raise - def install_apk(self, apk_path: str | Path): + def install_apk(self, apk_path: str | Path, no_streaming=False): """Install an APK file on an Android device. :param apk_path: The path of the Android APK file to install. + :param no_streaming: Whether to push APK to device and invoke Package Manager as separate steps. :returns: `None` on success; raises an exception on failure. """ + command = ( + ["install", "-r"] + + (["--no-streaming"] if no_streaming else []) + + [apk_path] + ) try: - self.run("install", "-r", apk_path) + self.run(*command) except subprocess.CalledProcessError as e: raise BriefcaseCommandError( f"Unable to install APK {apk_path} on {self.device}" diff --git a/src/briefcase/platforms/android/gradle.py b/src/briefcase/platforms/android/gradle.py index 4f1631445..d6098f6f1 100644 --- a/src/briefcase/platforms/android/gradle.py +++ b/src/briefcase/platforms/android/gradle.py @@ -353,6 +353,12 @@ def add_options(self, parser): help="Additional arguments to use when starting the emulator", required=False, ) + parser.add_argument( + "--no-streaming", + action="store_true", + help="Push APK to device and invoke Package Manager as separate steps", + required=False, + ) parser.add_argument( "--shutdown-on-exit", action="store_true", @@ -367,6 +373,7 @@ def run_app( passthrough: list[str], device_or_avd=None, extra_emulator_args=None, + no_streaming=False, shutdown_on_exit=False, **kwargs, ): @@ -378,6 +385,7 @@ def run_app( :param device_or_avd: The device to target. If ``None``, the user will be asked to re-run the command selecting a specific device. :param extra_emulator_args: Any additional arguments to pass to the emulator. + :param no_streaming: Whether to push APK to device and invoke Package Manager as separate steps. :param shutdown_on_exit: Should the emulator be shut down on exit? """ device, name, avd = self.tools.android_sdk.select_target_device(device_or_avd) @@ -425,7 +433,7 @@ def run_app( # Install the latest APK file onto the device. with self.input.wait_bar("Installing new app version..."): - adb.install_apk(self.binary_path(app)) + adb.install_apk(self.binary_path(app), no_streaming=no_streaming) # To start the app, we launch `org.beeware.android.MainActivity`. with self.input.wait_bar(f"Launching {label}..."): From 9157aad9ae6c9b409a9d481dee2a0197acedb8b2 Mon Sep 17 00:00:00 2001 From: Helio Perroni Filho Date: Sat, 20 Jul 2024 14:29:20 -0400 Subject: [PATCH 2/5] Added missing change note Signed-off-by: Helio Perroni Filho --- changes/1922.feature.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 changes/1922.feature.rst diff --git a/changes/1922.feature.rst b/changes/1922.feature.rst new file mode 100644 index 000000000..362312eff --- /dev/null +++ b/changes/1922.feature.rst @@ -0,0 +1 @@ +Added to ``briefcase run android`` a ``--no-streaming`` option which makes ``adb install`` run in no-streaming mode. ADB can install an app either directly from the host ("streamed install") or by copying the APK to the device and then running the installation process ("no-streaming"). The former is the current default, but the latter has to be specified for installation to work in some devices, for example those running recent versions of LineageOS. From 2b392c4c3ebdcd1afb4ef3efc5c95b57d2c06da6 Mon Sep 17 00:00:00 2001 From: Helio Perroni Filho Date: Sat, 20 Jul 2024 15:46:12 -0400 Subject: [PATCH 3/5] Updated android integration tests to cover new --no-streaming option Signed-off-by: Helio Perroni Filho --- tests/platforms/android/gradle/test_run.py | 113 +++++++++++++++++++-- 1 file changed, 106 insertions(+), 7 deletions(-) diff --git a/tests/platforms/android/gradle/test_run.py b/tests/platforms/android/gradle/test_run.py index de5e986ab..f91538933 100644 --- a/tests/platforms/android/gradle/test_run.py +++ b/tests/platforms/android/gradle/test_run.py @@ -93,6 +93,7 @@ def test_device_option(run_command): "test_mode": False, "passthrough": [], "extra_emulator_args": None, + "no_streaming": False, "shutdown_on_exit": False, } assert overrides == {} @@ -116,6 +117,29 @@ def test_extra_emulator_args_option(run_command): "test_mode": False, "passthrough": [], "extra_emulator_args": ["-no-window", "-no-audio"], + "no_streaming": False, + "shutdown_on_exit": False, + } + assert overrides == {} + + +def test_no_streaming_option(run_command): + """The --no-streaming option can be parsed.""" + options, overrides = run_command.parse_options(["--no-streaming"]) + + assert options == { + "device_or_avd": None, + "appname": None, + "update": False, + "update_requirements": False, + "update_resources": False, + "update_support": False, + "update_stub": False, + "no_update": False, + "test_mode": False, + "passthrough": [], + "extra_emulator_args": None, + "no_streaming": True, "shutdown_on_exit": False, } assert overrides == {} @@ -137,6 +161,7 @@ def test_shutdown_on_exit_option(run_command): "test_mode": False, "passthrough": [], "extra_emulator_args": None, + "no_streaming": False, "shutdown_on_exit": True, } assert overrides == {} @@ -207,7 +232,81 @@ def mock_stream_output(app, stop_func, **kwargs): # The adb wrapper is invoked with the expected arguments run_command.tools.mock_adb.install_apk.assert_called_once_with( - run_command.binary_path(first_app_config) + run_command.binary_path(first_app_config), no_streaming=False + ) + run_command.tools.mock_adb.force_stop_app.assert_called_once_with( + f"{first_app_config.package_name}.{first_app_config.module_name}", + ) + + run_command.tools.mock_adb.start_app.assert_called_once_with( + f"{first_app_config.package_name}.{first_app_config.module_name}", + "org.beeware.android.MainActivity", + [], + ) + + run_command.tools.mock_adb.pidof.assert_called_once_with( + f"{first_app_config.package_name}.{first_app_config.module_name}", + quiet=True, + ) + run_command.tools.mock_adb.logcat.assert_called_once_with(pid="777") + + run_command._stream_app_logs.assert_called_once_with( + first_app_config, + popen=log_popen, + test_mode=False, + clean_filter=android_log_clean_filter, + clean_output=False, + stop_func=mock.ANY, + log_stream=True, + ) + + # The emulator was not killed at the end of the test + run_command.tools.mock_adb.kill.assert_not_called() + + +def test_run_no_streaming(run_command, first_app_config): + """An app can be run on an existing device after being installed in no-streaming + mode.""" + # Set up device selection to return a running physical device. + run_command.tools.android_sdk.select_target_device = mock.MagicMock( + return_value=("exampleDevice", "ExampleDevice", None) + ) + + # Set up the log streamer to return a known stream + log_popen = mock.MagicMock() + run_command.tools.mock_adb.logcat.return_value = log_popen + + # To satisfy coverage, the stop function must be invoked at least once + # when invoking stream_output. + def mock_stream_output(app, stop_func, **kwargs): + stop_func() + + run_command._stream_app_logs.side_effect = mock_stream_output + + # Set up app config to have a `-` in the `bundle`, to ensure it gets + # normalized into a `_` via `package_name`. + first_app_config.bundle = "com.ex-ample" + + # Invoke run_app + run_command.run_app( + first_app_config, + device_or_avd="exampleDevice", + no_streaming=True, + test_mode=False, + passthrough=[], + ) + + # select_target_device was invoked with a specific device + run_command.tools.android_sdk.select_target_device.assert_called_once_with( + "exampleDevice" + ) + + # The ADB wrapper is created + run_command.tools.android_sdk.adb.assert_called_once_with(device="exampleDevice") + + # The adb wrapper is invoked with the expected arguments + run_command.tools.mock_adb.install_apk.assert_called_once_with( + run_command.binary_path(first_app_config), no_streaming=True ) run_command.tools.mock_adb.force_stop_app.assert_called_once_with( f"{first_app_config.package_name}.{first_app_config.module_name}", @@ -279,7 +378,7 @@ def mock_stream_output(app, stop_func, **kwargs): # The adb wrapper is invoked with the expected arguments run_command.tools.mock_adb.install_apk.assert_called_once_with( - run_command.binary_path(first_app_config) + run_command.binary_path(first_app_config), no_streaming=False ) run_command.tools.mock_adb.force_stop_app.assert_called_once_with( f"{first_app_config.package_name}.{first_app_config.module_name}", @@ -439,7 +538,7 @@ def test_run_created_emulator(run_command, first_app_config): # The adb wrapper is invoked with the expected arguments run_command.tools.mock_adb.install_apk.assert_called_once_with( - run_command.binary_path(first_app_config) + run_command.binary_path(first_app_config), no_streaming=False ) run_command.tools.mock_adb.force_stop_app.assert_called_once_with( f"{first_app_config.package_name}.{first_app_config.module_name}", @@ -501,7 +600,7 @@ def test_run_idle_device(run_command, first_app_config): # The adb wrapper is invoked with the expected arguments run_command.tools.mock_adb.install_apk.assert_called_once_with( - run_command.binary_path(first_app_config) + run_command.binary_path(first_app_config), no_streaming=False ) run_command.tools.mock_adb.force_stop_app.assert_called_once_with( f"{first_app_config.package_name}.{first_app_config.module_name}", @@ -602,7 +701,7 @@ def mock_stream_output(app, stop_func, **kwargs): # The adb wrapper is invoked with the expected arguments run_command.tools.mock_adb.install_apk.assert_called_once_with( - run_command.binary_path(first_app_config) + run_command.binary_path(first_app_config), no_streaming=False ) run_command.tools.mock_adb.force_stop_app.assert_called_once_with( f"{first_app_config.package_name}.{first_app_config.module_name}", @@ -675,7 +774,7 @@ def mock_stream_output(app, stop_func, **kwargs): # The adb wrapper is invoked with the expected arguments run_command.tools.mock_adb.install_apk.assert_called_once_with( - run_command.binary_path(first_app_config) + run_command.binary_path(first_app_config), no_streaming=False ) run_command.tools.mock_adb.force_stop_app.assert_called_once_with( f"{first_app_config.package_name}.{first_app_config.module_name}", @@ -752,7 +851,7 @@ def test_run_test_mode_created_emulator(run_command, first_app_config): # The adb wrapper is invoked with the expected arguments run_command.tools.mock_adb.install_apk.assert_called_once_with( - run_command.binary_path(first_app_config) + run_command.binary_path(first_app_config), no_streaming=False ) run_command.tools.mock_adb.force_stop_app.assert_called_once_with( f"{first_app_config.package_name}.{first_app_config.module_name}", From 22ad33ec05f022b46ebdbb5b480ea9e102bcb862 Mon Sep 17 00:00:00 2001 From: Helio Perroni Filho Date: Mon, 22 Jul 2024 09:03:33 -0400 Subject: [PATCH 4/5] Replaced --no-streaming option with --Xadb-install Replaced the --no-streaming option with the more general --Xadb-install, which allows passing arbitrary options to adb install. Signed-off-by: Helio Perroni Filho --- changes/1922.feature.rst | 2 +- src/briefcase/integrations/android_sdk.py | 14 +++--- src/briefcase/platforms/android/gradle.py | 19 ++++---- tests/platforms/android/gradle/test_run.py | 50 +++++++++++----------- 4 files changed, 43 insertions(+), 42 deletions(-) diff --git a/changes/1922.feature.rst b/changes/1922.feature.rst index 362312eff..389c5b6ab 100644 --- a/changes/1922.feature.rst +++ b/changes/1922.feature.rst @@ -1 +1 @@ -Added to ``briefcase run android`` a ``--no-streaming`` option which makes ``adb install`` run in no-streaming mode. ADB can install an app either directly from the host ("streamed install") or by copying the APK to the device and then running the installation process ("no-streaming"). The former is the current default, but the latter has to be specified for installation to work in some devices, for example those running recent versions of LineageOS. +Added to ``briefcase run android`` a ``--Xadb-install`` option for passing extra arguments to ``adb install``. This makes it possible to handle various corner cases, for example, setting the ``--no-streaming`` option to make installation work for devices running recent versions of LineageOS. diff --git a/src/briefcase/integrations/android_sdk.py b/src/briefcase/integrations/android_sdk.py index eed7b0c46..565328da8 100644 --- a/src/briefcase/integrations/android_sdk.py +++ b/src/briefcase/integrations/android_sdk.py @@ -1493,20 +1493,18 @@ def run(self, *arguments: SubprocessArgT, quiet: bool = False) -> str: raise InvalidDeviceError("device id", self.device) from e raise - def install_apk(self, apk_path: str | Path, no_streaming=False): + def install_apk(self, apk_path: str | Path, extra_args: list[str] | None = None): """Install an APK file on an Android device. :param apk_path: The path of the Android APK file to install. - :param no_streaming: Whether to push APK to device and invoke Package Manager as separate steps. + :param extra_args: Any additional arguments to pass to adb install. :returns: `None` on success; raises an exception on failure. """ - command = ( - ["install", "-r"] - + (["--no-streaming"] if no_streaming else []) - + [apk_path] - ) + if extra_args is None: + extra_args = [] + try: - self.run(*command) + self.run("install", "-r", *extra_args, apk_path) except subprocess.CalledProcessError as e: raise BriefcaseCommandError( f"Unable to install APK {apk_path} on {self.device}" diff --git a/src/briefcase/platforms/android/gradle.py b/src/briefcase/platforms/android/gradle.py index d6098f6f1..ebf070650 100644 --- a/src/briefcase/platforms/android/gradle.py +++ b/src/briefcase/platforms/android/gradle.py @@ -347,16 +347,17 @@ def add_options(self, parser): required=False, ) parser.add_argument( - "--Xemulator", + "--Xadb-install", action="append", - dest="extra_emulator_args", - help="Additional arguments to use when starting the emulator", + dest="extra_adb_install_args", + help="Additional arguments passed to adb install", required=False, ) parser.add_argument( - "--no-streaming", - action="store_true", - help="Push APK to device and invoke Package Manager as separate steps", + "--Xemulator", + action="append", + dest="extra_emulator_args", + help="Additional arguments to use when starting the emulator", required=False, ) parser.add_argument( @@ -372,8 +373,8 @@ def run_app( test_mode: bool, passthrough: list[str], device_or_avd=None, + extra_adb_install_args=None, extra_emulator_args=None, - no_streaming=False, shutdown_on_exit=False, **kwargs, ): @@ -384,8 +385,8 @@ def run_app( :param passthrough: The list of arguments to pass to the app :param device_or_avd: The device to target. If ``None``, the user will be asked to re-run the command selecting a specific device. + :param extra_adb_install_args: Any additional arguments to pass to adb install. :param extra_emulator_args: Any additional arguments to pass to the emulator. - :param no_streaming: Whether to push APK to device and invoke Package Manager as separate steps. :param shutdown_on_exit: Should the emulator be shut down on exit? """ device, name, avd = self.tools.android_sdk.select_target_device(device_or_avd) @@ -433,7 +434,7 @@ def run_app( # Install the latest APK file onto the device. with self.input.wait_bar("Installing new app version..."): - adb.install_apk(self.binary_path(app), no_streaming=no_streaming) + adb.install_apk(self.binary_path(app), extra_adb_install_args) # To start the app, we launch `org.beeware.android.MainActivity`. with self.input.wait_bar(f"Launching {label}..."): diff --git a/tests/platforms/android/gradle/test_run.py b/tests/platforms/android/gradle/test_run.py index f91538933..f01b17f18 100644 --- a/tests/platforms/android/gradle/test_run.py +++ b/tests/platforms/android/gradle/test_run.py @@ -92,17 +92,17 @@ def test_device_option(run_command): "no_update": False, "test_mode": False, "passthrough": [], + "extra_adb_install_args": None, "extra_emulator_args": None, - "no_streaming": False, "shutdown_on_exit": False, } assert overrides == {} -def test_extra_emulator_args_option(run_command): - """The -d option can be parsed.""" +def test_extra_adb_install_args_option(run_command): + """The --Xadb-install option can be parsed.""" options, overrides = run_command.parse_options( - ["--Xemulator=-no-window", "--Xemulator=-no-audio"] + ["--Xadb-install=--no-streaming", "--Xadb-install=-g"] ) assert options == { @@ -116,16 +116,18 @@ def test_extra_emulator_args_option(run_command): "no_update": False, "test_mode": False, "passthrough": [], - "extra_emulator_args": ["-no-window", "-no-audio"], - "no_streaming": False, + "extra_adb_install_args": ["--no-streaming", "-g"], + "extra_emulator_args": None, "shutdown_on_exit": False, } assert overrides == {} -def test_no_streaming_option(run_command): - """The --no-streaming option can be parsed.""" - options, overrides = run_command.parse_options(["--no-streaming"]) +def test_extra_emulator_args_option(run_command): + """The -d option can be parsed.""" + options, overrides = run_command.parse_options( + ["--Xemulator=-no-window", "--Xemulator=-no-audio"] + ) assert options == { "device_or_avd": None, @@ -138,8 +140,8 @@ def test_no_streaming_option(run_command): "no_update": False, "test_mode": False, "passthrough": [], - "extra_emulator_args": None, - "no_streaming": True, + "extra_adb_install_args": None, + "extra_emulator_args": ["-no-window", "-no-audio"], "shutdown_on_exit": False, } assert overrides == {} @@ -160,8 +162,8 @@ def test_shutdown_on_exit_option(run_command): "no_update": False, "test_mode": False, "passthrough": [], + "extra_adb_install_args": None, "extra_emulator_args": None, - "no_streaming": False, "shutdown_on_exit": True, } assert overrides == {} @@ -232,7 +234,7 @@ def mock_stream_output(app, stop_func, **kwargs): # The adb wrapper is invoked with the expected arguments run_command.tools.mock_adb.install_apk.assert_called_once_with( - run_command.binary_path(first_app_config), no_streaming=False + run_command.binary_path(first_app_config), None ) run_command.tools.mock_adb.force_stop_app.assert_called_once_with( f"{first_app_config.package_name}.{first_app_config.module_name}", @@ -264,9 +266,9 @@ def mock_stream_output(app, stop_func, **kwargs): run_command.tools.mock_adb.kill.assert_not_called() -def test_run_no_streaming(run_command, first_app_config): - """An app can be run on an existing device after being installed in no-streaming - mode.""" +def test_run_extra_adb_install_args(run_command, first_app_config): + """An app can be run on an existing device after being installed with extra adb + install arguments.""" # Set up device selection to return a running physical device. run_command.tools.android_sdk.select_target_device = mock.MagicMock( return_value=("exampleDevice", "ExampleDevice", None) @@ -291,7 +293,7 @@ def mock_stream_output(app, stop_func, **kwargs): run_command.run_app( first_app_config, device_or_avd="exampleDevice", - no_streaming=True, + extra_adb_install_args=["--no-streaming", "-g"], test_mode=False, passthrough=[], ) @@ -306,7 +308,7 @@ def mock_stream_output(app, stop_func, **kwargs): # The adb wrapper is invoked with the expected arguments run_command.tools.mock_adb.install_apk.assert_called_once_with( - run_command.binary_path(first_app_config), no_streaming=True + run_command.binary_path(first_app_config), ["--no-streaming", "-g"] ) run_command.tools.mock_adb.force_stop_app.assert_called_once_with( f"{first_app_config.package_name}.{first_app_config.module_name}", @@ -378,7 +380,7 @@ def mock_stream_output(app, stop_func, **kwargs): # The adb wrapper is invoked with the expected arguments run_command.tools.mock_adb.install_apk.assert_called_once_with( - run_command.binary_path(first_app_config), no_streaming=False + run_command.binary_path(first_app_config), None ) run_command.tools.mock_adb.force_stop_app.assert_called_once_with( f"{first_app_config.package_name}.{first_app_config.module_name}", @@ -538,7 +540,7 @@ def test_run_created_emulator(run_command, first_app_config): # The adb wrapper is invoked with the expected arguments run_command.tools.mock_adb.install_apk.assert_called_once_with( - run_command.binary_path(first_app_config), no_streaming=False + run_command.binary_path(first_app_config), None ) run_command.tools.mock_adb.force_stop_app.assert_called_once_with( f"{first_app_config.package_name}.{first_app_config.module_name}", @@ -600,7 +602,7 @@ def test_run_idle_device(run_command, first_app_config): # The adb wrapper is invoked with the expected arguments run_command.tools.mock_adb.install_apk.assert_called_once_with( - run_command.binary_path(first_app_config), no_streaming=False + run_command.binary_path(first_app_config), None ) run_command.tools.mock_adb.force_stop_app.assert_called_once_with( f"{first_app_config.package_name}.{first_app_config.module_name}", @@ -701,7 +703,7 @@ def mock_stream_output(app, stop_func, **kwargs): # The adb wrapper is invoked with the expected arguments run_command.tools.mock_adb.install_apk.assert_called_once_with( - run_command.binary_path(first_app_config), no_streaming=False + run_command.binary_path(first_app_config), None ) run_command.tools.mock_adb.force_stop_app.assert_called_once_with( f"{first_app_config.package_name}.{first_app_config.module_name}", @@ -774,7 +776,7 @@ def mock_stream_output(app, stop_func, **kwargs): # The adb wrapper is invoked with the expected arguments run_command.tools.mock_adb.install_apk.assert_called_once_with( - run_command.binary_path(first_app_config), no_streaming=False + run_command.binary_path(first_app_config), None ) run_command.tools.mock_adb.force_stop_app.assert_called_once_with( f"{first_app_config.package_name}.{first_app_config.module_name}", @@ -851,7 +853,7 @@ def test_run_test_mode_created_emulator(run_command, first_app_config): # The adb wrapper is invoked with the expected arguments run_command.tools.mock_adb.install_apk.assert_called_once_with( - run_command.binary_path(first_app_config), no_streaming=False + run_command.binary_path(first_app_config), None ) run_command.tools.mock_adb.force_stop_app.assert_called_once_with( f"{first_app_config.package_name}.{first_app_config.module_name}", From 3eebad7c0a609b460f29c2e1301a2f804e4c0484 Mon Sep 17 00:00:00 2001 From: Helio Perroni Filho Date: Mon, 22 Jul 2024 10:04:32 -0400 Subject: [PATCH 5/5] Updated install_apk() logic to address branch coverage failure Signed-off-by: Helio Perroni Filho --- src/briefcase/integrations/android_sdk.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/briefcase/integrations/android_sdk.py b/src/briefcase/integrations/android_sdk.py index 565328da8..de30af2b8 100644 --- a/src/briefcase/integrations/android_sdk.py +++ b/src/briefcase/integrations/android_sdk.py @@ -1500,11 +1500,10 @@ def install_apk(self, apk_path: str | Path, extra_args: list[str] | None = None) :param extra_args: Any additional arguments to pass to adb install. :returns: `None` on success; raises an exception on failure. """ - if extra_args is None: - extra_args = [] + command = ["install", "-r"] + (extra_args if extra_args else []) + [apk_path] try: - self.run("install", "-r", *extra_args, apk_path) + self.run(*command) except subprocess.CalledProcessError as e: raise BriefcaseCommandError( f"Unable to install APK {apk_path} on {self.device}"