fix(android): report a package that is not installed when terminating it

devices/android.go

@@ -7,6 +7,7 @@ import (
 	"encoding/base64"
 	"encoding/json"
 	"encoding/xml"
+	"errors"
 	"fmt"
 	"os"
 	"os/exec"
@@ -509,7 +510,18 @@ func (d *AndroidDevice) LaunchApp(bundleID string, opts LaunchOptions) error {
 	return nil
 }
 
+// TerminateApp stops an app. `am force-stop` succeeds for any package name, so a
+// package that is not installed is reported the way IOSDevice and SimulatorDevice
+// report it. An installed app that is not running is still stopped without error.
 func (d *AndroidDevice) TerminateApp(bundleID string) error {
+	installed, err := d.isPackageInstalled(bundleID)
+	if err != nil {
+		return err
+	}
+	if !installed {
+		return fmt.Errorf("%s not installed", bundleID)
+	}
+
 	output, err := d.runAdbShell("am", "force-stop", bundleID)
 	if err != nil {
 		return fmt.Errorf("failed to terminate app %s: %v\nOutput: %s", bundleID, err, string(output))
@@ -1246,6 +1258,30 @@ func (d *AndroidDevice) Info() (*FullDeviceInfo, error) {
 	}, nil
 }
 
+// isPackageInstalled reports whether packageName is installed on the device.
+func (d *AndroidDevice) isPackageInstalled(packageName string) (bool, error) {
+	output, err := d.runAdbShell("pm", "path", packageName)
+	return pmPathReportsInstalled(output, err)
+}
+
+// pmPathReportsInstalled reads the result of `pm path <package>`, which prints
+// "package:<apk path>" for an installed package and exits 1 with no output for
+// one that is not installed. Any other failure, such as an offline device or
+// an adb that could not be started, is returned as an error rather than read
+// as "not installed".
+func pmPathReportsInstalled(output []byte, err error) (bool, error) {
+	text := strings.TrimSpace(string(output))
+	if strings.HasPrefix(text, "package:") {
+		return true, nil
+	}
+	var exitErr *exec.ExitError
+	exitedWithoutAMatch := errors.As(err, &exitErr) && exitErr.ExitCode() == 1 && text == ""
+	if err != nil && !exitedWithoutAMatch {
+		return false, fmt.Errorf("failed to check whether the package is installed: %w\nOutput: %s", err, text)
+	}
+	return false, nil
+}
+
 func (d *AndroidDevice) GetAppPath(packageName string) (string, error) {
 	output, err := d.runAdbShell("pm", "path", packageName)
 	if err != nil {

devices/android_apps_test.go

@@ -1,6 +1,70 @@
 package devices
 
-import "testing"
+import (
+	"context"
+	"errors"
+	"fmt"
+	"os/exec"
+	"testing"
+)
+
+func TestPmPathReportsAnInstalledPackage(t *testing.T) {
+	installed, err := pmPathReportsInstalled([]byte("package:/system_ext/priv-app/SettingsGoogle/SettingsGoogle.apk\n"), nil)
+
+	if err != nil || !installed {
+		t.Fatalf("got installed=%v err=%v, want installed with no error", installed, err)
+	}
+}
+
+func TestPmPathReportsAnInstalledSplitApkPackage(t *testing.T) {
+	output := []byte("package:/data/app/com.example/base.apk\npackage:/data/app/com.example/split_config.arm64_v8a.apk\n")
+
+	installed, err := pmPathReportsInstalled(output, nil)
+
+	if err != nil || !installed {
+		t.Fatalf("got installed=%v err=%v, want installed with no error", installed, err)
+	}
+}
+
+// processExiting returns the error of a process that exited with the given
+// status, as adb's does when pm path reports a missing package.
+func processExiting(t *testing.T, status int) error {
+	t.Helper()
+	err := exec.CommandContext(context.Background(), "sh", "-c", fmt.Sprintf("exit %d", status)).Run()
+	if err == nil {
+		t.Fatalf("expected the shell to exit with status %d", status)
+	}
+	return err
+}
+
+func TestPmPathReportsAMissingPackageAsNotInstalled(t *testing.T) {
+	// pm path exits 1 and prints nothing for a package that is not installed
+	installed, err := pmPathReportsInstalled([]byte(""), processExiting(t, 1))
+
+	if err != nil || installed {
+		t.Fatalf("got installed=%v err=%v, want not installed with no error", installed, err)
+	}
+}
+
+// adb that could not be started prints nothing either, and must not pass for
+// a package that is not installed.
+func TestPmPathReportsAnAdbThatCouldNotStartAsAnError(t *testing.T) {
+	adbNotFound := &exec.Error{Name: "adb", Err: exec.ErrNotFound}
+
+	_, err := pmPathReportsInstalled([]byte(""), adbNotFound)
+
+	if err == nil || !errors.Is(err, exec.ErrNotFound) {
+		t.Fatalf("expected the launch failure to be reported, got %v", err)
+	}
+}
+
+func TestPmPathReportsAnAdbFailureAsAnError(t *testing.T) {
+	_, err := pmPathReportsInstalled([]byte("adb: device 'emulator-5554' not found"), errors.New("exit status 1"))
+
+	if err == nil {
+		t.Fatal("an adb failure must not be read as 'not installed'")
+	}
+}
 
 func TestParsePackageListerOutput(t *testing.T) {
 	output := []byte(`[{"packageName":"com.mobilenext.devicekit","appName":"DeviceKit","version":"1.2.5","versionCode":10205}]`)

test/android.spec.ts

@@ -195,6 +195,22 @@ test.describe('Android Tests', () => {
 		await expectLauncherToBeInForeground(device!.id);
 	});
 
+	test('should report that a package which is not installed cannot be terminated', () => {
+		test.skip(!device, 'No Android device found');
+
+		const error = appsCommandError(device!.id, 'terminate', ABSENT_PACKAGE);
+
+		expect(error).toContain(`${ABSENT_PACKAGE} not installed`);
+	});
+
+	test('should terminate an installed app that is not running without an error', () => {
+		test.skip(!device, 'No Android device found');
+
+		// force-stop is idempotent, and callers stop an app before relaunching it
+		terminateApp(device!.id, SETTINGS_PACKAGE);
+		terminateApp(device!.id, SETTINGS_PACKAGE);
+	});
+
 	test('should handle launching app twice (idempotency)', async () => {
 		test.skip(!device, 'No Android device found');
 
@@ -983,6 +999,23 @@ function terminateApp(deviceId: string, packageName: string): void {
 	mobilecli(['apps', 'terminate', packageName, '--device', deviceId]);
 }
 
+// runs an `apps` subcommand that is expected to fail and returns its error message
+function appsCommandError(deviceId: string, subcommand: string, packageName: string): string {
+	try {
+		mobilecliJson(['apps', subcommand, packageName, '--device', deviceId]);
+	} catch (error: unknown) {
+		// a timeout or a missing binary fails without printing an envelope, and
+		// parsing that as json would bury the real cause
+		const stdout = (error as {stdout?: string}).stdout ?? '';
+		if (stdout.trim() === '') {
+			throw error;
+		}
+		return expectErrorEnvelope(JSON.parse(stdout));
+	}
+
+	throw new Error(`apps ${subcommand} ${packageName} unexpectedly succeeded`);
+}
+
 // force-stops every package that can hold an activity in the settings task, so the
 // next launch starts from a clean root instead of resuming whatever was left open
 // agent subcommands report a missing agent with status "fail" and still exit 0, so