diff --git a/internal/android/android.go b/internal/android/android.go index 5983056..1ccd823 100644 --- a/internal/android/android.go +++ b/internal/android/android.go @@ -123,14 +123,17 @@ func antiFreezeCommands() [][]string { // reinstalling it. This replaces `pm clear` for clear-state: ColorOS and other // hardened OEM builds deny CLEAR_APP_USER_DATA even to the adb shell user, so a // clear aborts the launch, whereas uninstall+install is always permitted. -// The uninstall is best effort so a not-installed app is not an error. +// A failed uninstall is not passed over: `install -r` keeps the app's data, so +// the reinstall would report a clear-state that never happened. func ReinstallApp(ctx context.Context, serial, bundleID, apkPath string, stdout io.Writer) error { adb, err := AdbBinary() if err != nil { return err } if output, err := exec.CommandContext(ctx, adb, adbArgs(serial, "uninstall", bundleID)...).CombinedOutput(); err != nil { - fmt.Fprintf(stdout, "clear-state: uninstall %s skipped (%v: %s)\n", bundleID, err, strings.TrimSpace(string(output))) + if err := clearDataUninstallLeftBehind(ctx, adb, serial, bundleID, strings.TrimSpace(string(output)), stdout); err != nil { + return err + } } if output, err := exec.CommandContext(ctx, adb, adbArgs(serial, "install", "-r", apkPath)...).CombinedOutput(); err != nil { return fmt.Errorf("install %s: %w: %s", apkPath, err, strings.TrimSpace(string(output))) @@ -138,6 +141,37 @@ func ReinstallApp(ctx context.Context, serial, bundleID, apkPath string, stdout return nil } +// clearDataUninstallLeftBehind reaches first-launch state after `adb uninstall` +// failed. The failure text cannot say why: an API 34 emulator answers +// "Failure [DELETE_FAILED_INTERNAL_ERROR]" both for a package that was never +// installed and for one it refuses to remove. So ask the package manager which +// happened. Nothing installed means nothing to clear. Still installed +// means the data survives the reinstall, and `pm clear` is the one remaining +// way to reach first-launch state; when that fails too, so does clear-state. +func clearDataUninstallLeftBehind(ctx context.Context, adb, serial, bundleID, uninstallOutput string, stdout io.Writer) error { + if !packageInstalled(ctx, adb, serial, bundleID) { + return nil + } + output, err := exec.CommandContext(ctx, adb, adbArgs(serial, "shell", "pm", "clear", bundleID)...).CombinedOutput() + cleared := strings.TrimSpace(string(output)) + if err != nil || !strings.Contains(cleared, "Success") { + return fmt.Errorf( + "clear-state: %s is still installed after `adb uninstall` said %q, and `pm clear` said %q: its data was not cleared", + bundleID, uninstallOutput, cleared, + ) + } + fmt.Fprintf(stdout, "clear-state: uninstall %s said %q and left it installed; cleared its data with `pm clear` instead\n", bundleID, uninstallOutput) + return nil +} + +// packageInstalled reports whether the package manager resolves an APK path for +// bundleID. The printed path is the signal rather than the exit status, which +// `adb shell` does not forward from devices below API 24. +func packageInstalled(ctx context.Context, adb, serial, bundleID string) bool { + output, _ := exec.CommandContext(ctx, adb, adbArgs(serial, "shell", "pm", "path", bundleID)...).Output() + return strings.HasPrefix(strings.TrimSpace(string(output)), "package:") +} + const threeButtonNavOverlay = "com.android.internal.systemui.navbar.threebutton" // navModeOverlays are the system navigation-mode overlays. Only one is active at diff --git a/internal/android/android_test.go b/internal/android/android_test.go index 2161cd3..479c886 100644 --- a/internal/android/android_test.go +++ b/internal/android/android_test.go @@ -441,3 +441,134 @@ func TestNavModeToRestore(t *testing.T) { } } } + +// scriptedAdb puts an adb under a fake SDK root that logs each invocation's +// arguments and answers from replies, a `case "$*" in` body. SDK lookup is +// isolated onto that root, so ReinstallApp runs its real command sequence +// against the script and the log holds what reached adb. +func scriptedAdb(t *testing.T, replies string) string { + t.Helper() + root := t.TempDir() + log := filepath.Join(root, "adb.log") + adb := filepath.Join(root, "platform-tools", "adb") + if err := os.MkdirAll(filepath.Dir(adb), 0o755); err != nil { + t.Fatalf("mkdir %s: %v", filepath.Dir(adb), err) + } + script := "#!/bin/sh\necho \"$*\" >> " + log + "\ncase \"$*\" in\n" + replies + "\nesac\n" + if err := os.WriteFile(adb, []byte(script), 0o755); err != nil { + t.Fatalf("write %s: %v", adb, err) + } + isolateSDKLookup(t, root) + return log +} + +func adbCalls(t *testing.T, log string) []string { + t.Helper() + contents, err := os.ReadFile(log) + if err != nil { + if os.IsNotExist(err) { + return nil + } + t.Fatalf("read %s: %v", log, err) + } + return strings.Split(strings.TrimSpace(string(contents)), "\n") +} + +const ( + uninstallFails = `"uninstall "*) echo "Failure [DELETE_FAILED_INTERNAL_ERROR]"; exit 1;;` + stillInstalled = `"shell pm path "*) echo "package:/data/app/app.example-1/base.apk";;` + notInstalled = `"shell pm path "*) exit 1;;` + installSucceeds = `"install "*) echo "Success";;` +) + +func TestReinstallApp_RefusedUninstallClearsTheDataItLeftBehind(t *testing.T) { + log := scriptedAdb(t, strings.Join([]string{ + uninstallFails, + stillInstalled, + `"shell pm clear "*) echo "Success";;`, + installSucceeds, + }, "\n")) + output := &strings.Builder{} + + if err := ReinstallApp(t.Context(), "", "app.example", "/tmp/app.apk", output); err != nil { + t.Fatalf("ReinstallApp: %v", err) + } + + want := []string{ + "uninstall app.example", + "shell pm path app.example", + "shell pm clear app.example", + "install -r /tmp/app.apk", + } + if got := adbCalls(t, log); !slices.Equal(got, want) { + t.Fatalf("adb calls = %v, want %v", got, want) + } + if !strings.Contains(output.String(), "pm clear") { + t.Errorf("output %q does not say the data was cleared some other way", output.String()) + } +} + +func TestReinstallApp_RefusedUninstallThatCannotBeClearedIsFatal(t *testing.T) { + log := scriptedAdb(t, strings.Join([]string{ + uninstallFails, + stillInstalled, + `"shell pm clear "*) echo "Failed"; exit 1;;`, + installSucceeds, + }, "\n")) + + err := ReinstallApp(t.Context(), "", "app.example", "/tmp/app.apk", &strings.Builder{}) + + if err == nil { + t.Fatal("ReinstallApp reported success while app.example kept the data clear-state was asked to remove") + } + for _, want := range []string{"app.example", "DELETE_FAILED_INTERNAL_ERROR", "Failed"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error %q does not quote %q", err, want) + } + } + if slices.Contains(adbCalls(t, log), "install -r /tmp/app.apk") { + t.Error("installed over an app whose data survived, which is the reinstall reporting a clear it did not perform") + } +} + +func TestReinstallApp_UninstallFailureWithNothingInstalledIsQuiet(t *testing.T) { + log := scriptedAdb(t, strings.Join([]string{ + uninstallFails, + notInstalled, + installSucceeds, + }, "\n")) + output := &strings.Builder{} + + if err := ReinstallApp(t.Context(), "", "app.example", "/tmp/app.apk", output); err != nil { + t.Fatalf("ReinstallApp: %v", err) + } + + calls := adbCalls(t, log) + if !slices.Contains(calls, "install -r /tmp/app.apk") { + t.Fatalf("adb calls = %v, want the install to go ahead: a first run has no app to uninstall", calls) + } + if slices.Contains(calls, "shell pm clear app.example") { + t.Errorf("adb calls = %v, want no data clear: there was no app holding data", calls) + } + if output.String() != "" { + t.Errorf("output = %q, want nothing: a first run has no app to uninstall", output.String()) + } +} + +func TestReinstallApp_SuccessfulUninstallNeedsNoFallback(t *testing.T) { + log := scriptedAdb(t, strings.Join([]string{ + `"uninstall "*) echo "Success";;`, + stillInstalled, + `"shell pm clear "*) echo "Success";;`, + installSucceeds, + }, "\n")) + + if err := ReinstallApp(t.Context(), "", "app.example", "/tmp/app.apk", &strings.Builder{}); err != nil { + t.Fatalf("ReinstallApp: %v", err) + } + + want := []string{"uninstall app.example", "install -r /tmp/app.apk"} + if got := adbCalls(t, log); !slices.Equal(got, want) { + t.Fatalf("adb calls = %v, want %v", got, want) + } +} diff --git a/internal/driver/ioscompanion/device.go b/internal/driver/ioscompanion/device.go index 52903ab..1193636 100644 --- a/internal/driver/ioscompanion/device.go +++ b/internal/driver/ioscompanion/device.go @@ -224,8 +224,14 @@ func (d *Driver) respawnDevice(ctx context.Context) error { // devicectlReinstall uninstalls then installs the app bundle via devicectl, // keyed on the CoreDevice id. App lifecycle stays with devicectl: the runner's // own install path is simulator-specific. +// A failed uninstall ends the reinstall: installing over an app keeps its data, +// so clear-state would be reported without happening. Uninstalling an app that +// is not installed exits 0 ("App uninstalled." on a paired iPhone running iOS +// 26.5), so there is no benign failure here to sort out from a real one. func (d *Driver) devicectlReinstall(ctx context.Context) error { - _ = exec.CommandContext(ctx, "xcrun", "devicectl", "device", "uninstall", "app", "--device", d.coreDeviceID, d.bundleID).Run() + if output, err := exec.CommandContext(ctx, "xcrun", "devicectl", "device", "uninstall", "app", "--device", d.coreDeviceID, d.bundleID).CombinedOutput(); err != nil { + return fmt.Errorf("devicectl uninstall %s: %w: %s", d.bundleID, err, strings.TrimSpace(string(output))) + } output, err := exec.CommandContext(ctx, "xcrun", "devicectl", "device", "install", "app", "--device", d.coreDeviceID, d.appPath).CombinedOutput() if err != nil { return fmt.Errorf("devicectl install: %w: %s", err, strings.TrimSpace(string(output))) diff --git a/internal/driver/ioscompanion/device_test.go b/internal/driver/ioscompanion/device_test.go index 1ffd11d..abc5ced 100644 --- a/internal/driver/ioscompanion/device_test.go +++ b/internal/driver/ioscompanion/device_test.go @@ -7,6 +7,7 @@ import ( "net" "os/exec" "slices" + "strings" "testing" "github.com/priyanshujain/sanderling/internal/driver/ioscompanion/transport" @@ -182,6 +183,45 @@ func TestNewDeviceReinstallsOnceBeforeTheRunnerSession(t *testing.T) { } } +func TestDevicectlReinstallStopsWhenTheUninstallFails(t *testing.T) { + log := scriptedXcrun(t, `"devicectl device uninstall "*) echo "ERROR: Internal logic error: Connection was invalidated"; exit 1;; +"devicectl device install "*) :;;`) + d := &Driver{coreDeviceID: "CORE-DEVICE", bundleID: "app.example", appPath: "/tmp/Sample.app"} + + err := d.devicectlReinstall(context.Background()) + + if err == nil { + t.Fatal("devicectlReinstall reported success while app.example kept the data clear-state was asked to remove") + } + for _, want := range []string{"app.example", "Connection was invalidated"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error %q does not quote %q", err, want) + } + } + calls := xcrunCalls(t, log) + if slices.ContainsFunc(calls, func(call string) bool { return strings.HasPrefix(call, "devicectl device install") }) { + t.Errorf("xcrun calls = %v: installing over the app carries its data into the run", calls) + } +} + +func TestDevicectlReinstallProceedsWhenNothingIsInstalled(t *testing.T) { + log := scriptedXcrun(t, `"devicectl device uninstall "*) echo "App uninstalled.";; +"devicectl device install "*) :;;`) + d := &Driver{coreDeviceID: "CORE-DEVICE", bundleID: "app.example", appPath: "/tmp/Sample.app"} + + if err := d.devicectlReinstall(context.Background()); err != nil { + t.Fatalf("devicectlReinstall: %v", err) + } + + want := []string{ + "devicectl device uninstall app --device CORE-DEVICE app.example", + "devicectl device install app --device CORE-DEVICE /tmp/Sample.app", + } + if got := xcrunCalls(t, log); !slices.Equal(got, want) { + t.Fatalf("xcrun calls = %v, want %v", got, want) + } +} + func TestDeviceClearStateWithoutAppPathWarnsOnce(t *testing.T) { output := &bytes.Buffer{} d := &Driver{output: output, deviceMode: true} diff --git a/internal/driver/ioscompanion/driver.go b/internal/driver/ioscompanion/driver.go index 04b7d7c..125a421 100644 --- a/internal/driver/ioscompanion/driver.go +++ b/internal/driver/ioscompanion/driver.go @@ -615,8 +615,14 @@ func (d *Driver) clearAppState(ctx context.Context) error { // simctlReinstall uninstalls and reinstalls the app bundle via simctl. App // lifecycle stays with simctl: the companion's install RPC misreads current // simulator targets' architectures and rejects valid bundles. +// A failed uninstall ends the reinstall: `simctl install` over an installed app +// carries its data container across, so clear-state would be reported without +// happening. Uninstalling an app that is not installed exits 0, so there is no +// benign failure here to sort out from a real one. func (d *Driver) simctlReinstall(ctx context.Context) error { - _ = exec.CommandContext(ctx, "xcrun", "simctl", "uninstall", d.udid, d.bundleID).Run() + if output, err := exec.CommandContext(ctx, "xcrun", "simctl", "uninstall", d.udid, d.bundleID).CombinedOutput(); err != nil { + return fmt.Errorf("simctl uninstall %s: %w: %s", d.bundleID, err, strings.TrimSpace(string(output))) + } output, err := exec.CommandContext(ctx, "xcrun", "simctl", "install", d.udid, d.appPath).CombinedOutput() if err != nil { return fmt.Errorf("simctl install: %w: %s", err, strings.TrimSpace(string(output))) diff --git a/internal/driver/ioscompanion/driver_test.go b/internal/driver/ioscompanion/driver_test.go index db3b2f4..f8ae5b9 100644 --- a/internal/driver/ioscompanion/driver_test.go +++ b/internal/driver/ioscompanion/driver_test.go @@ -329,6 +329,68 @@ func TestNewRejectsClearStateWithoutBundleID(t *testing.T) { } } +// scriptedXcrun puts an xcrun on PATH that logs each invocation's arguments and +// answers from replies, a `case "$*" in` body, so a reinstall runs its real +// command sequence and the log holds what reached the tool. +func scriptedXcrun(t *testing.T, replies string) string { + t.Helper() + directory := t.TempDir() + log := filepath.Join(directory, "xcrun.log") + script := "#!/bin/sh\necho \"$*\" >> " + log + "\ncase \"$*\" in\n" + replies + "\nesac\n" + if err := os.WriteFile(filepath.Join(directory, "xcrun"), []byte(script), 0o755); err != nil { + t.Fatalf("write xcrun: %v", err) + } + t.Setenv("PATH", directory) + return log +} + +func xcrunCalls(t *testing.T, log string) []string { + t.Helper() + contents, err := os.ReadFile(log) + if err != nil { + if os.IsNotExist(err) { + return nil + } + t.Fatalf("read %s: %v", log, err) + } + return strings.Split(strings.TrimSpace(string(contents)), "\n") +} + +func TestSimctlReinstallStopsWhenTheUninstallFails(t *testing.T) { + log := scriptedXcrun(t, `"simctl uninstall "*) echo "Simulator device failed to uninstall app.example."; echo "Uninstall prohibited."; exit 22;; +"simctl install "*) :;;`) + d := &Driver{udid: "SIM-UDID", bundleID: "app.example", appPath: "/tmp/Sample.app"} + + err := d.simctlReinstall(context.Background()) + + if err == nil { + t.Fatal("simctlReinstall reported success while app.example kept the data clear-state was asked to remove") + } + for _, want := range []string{"app.example", "Uninstall prohibited."} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error %q does not quote %q", err, want) + } + } + if calls := xcrunCalls(t, log); slices.Contains(calls, "simctl install SIM-UDID /tmp/Sample.app") { + t.Errorf("xcrun calls = %v: installing over the app carries its data into the run", calls) + } +} + +func TestSimctlReinstallProceedsWhenNothingIsInstalled(t *testing.T) { + log := scriptedXcrun(t, `"simctl uninstall "*) :;; +"simctl install "*) :;;`) + d := &Driver{udid: "SIM-UDID", bundleID: "app.example", appPath: "/tmp/Sample.app"} + + if err := d.simctlReinstall(context.Background()); err != nil { + t.Fatalf("simctlReinstall: %v", err) + } + + want := []string{"simctl uninstall SIM-UDID app.example", "simctl install SIM-UDID /tmp/Sample.app"} + if got := xcrunCalls(t, log); !slices.Equal(got, want) { + t.Fatalf("xcrun calls = %v, want %v", got, want) + } +} + func TestLaunchRejectsEnvironment(t *testing.T) { d := newTestDriver(&fakeCompanion{accessibilityJSON: "[]"}) err := d.Launch(context.Background(), "", false, map[string]string{"K": "V"})