From a5f86334aff26fe4b581d0ad809d13f560c3cac8 Mon Sep 17 00:00:00 2001 From: PJ Date: Sat, 15 Aug 2026 22:36:50 +0530 Subject: [PATCH] fix(android): say what the sdk lookup checked, not just to set ANDROID_HOME --- internal/android/android.go | 52 +++++++++++++-- internal/android/android_test.go | 110 +++++++++++++++++++++++++++++++ 2 files changed, 156 insertions(+), 6 deletions(-) diff --git a/internal/android/android.go b/internal/android/android.go index b8038a2..17470ac 100644 --- a/internal/android/android.go +++ b/internal/android/android.go @@ -247,7 +247,9 @@ func EnvWithAndroidPlatformTools(env []string) []string { // AdbBinary locates the adb binary via PATH or known Android SDK locations. func AdbBinary() (string, error) { return findAndroidTool("adb", "platform-tools") } -func emulatorBinary() (string, error) { return findAndroidTool("emulator", "emulator") } +// EmulatorBinary locates the emulator binary via PATH or known Android SDK +// locations. +func EmulatorBinary() (string, error) { return findAndroidTool("emulator", "emulator") } // findAndroidTool locates a binary from the Android SDK. It checks PATH, // then $ANDROID_HOME// and $ANDROID_SDK_ROOT//, @@ -264,7 +266,36 @@ func findAndroidTool(name, subdir string) (string, error) { } tried = append(tried, candidate) } - return "", fmt.Errorf("could not locate %q: not on PATH and not under any known Android SDK root (set $ANDROID_HOME to point at your SDK; tried %v)", name, tried) + return "", fmt.Errorf( + "%s not found: not on PATH, and not at [%s]; %s\nput %s on PATH, or point $ANDROID_HOME at an Android SDK that has %s/%s", + name, strings.Join(tried, ", "), sdkRootStatus(), name, subdir, name, + ) +} + +// sdkRootStatus reports what the SDK root variables hold, so a lookup failure +// says whether they were unset or pointed somewhere that is not an SDK instead +// of leaving the reader to work out which from a list of paths. +func sdkRootStatus() string { + var reported []string + for _, variable := range []string{"ANDROID_HOME", "ANDROID_SDK_ROOT"} { + value := os.Getenv(variable) + if value == "" { + continue + } + info, err := os.Stat(value) + switch { + case err != nil: + reported = append(reported, fmt.Sprintf("$%s=%s does not exist", variable, value)) + case !info.IsDir(): + reported = append(reported, fmt.Sprintf("$%s=%s is not a directory", variable, value)) + default: + reported = append(reported, fmt.Sprintf("$%s=%s", variable, value)) + } + } + if len(reported) == 0 { + return "$ANDROID_HOME and $ANDROID_SDK_ROOT are unset" + } + return strings.Join(reported, ", ") } func androidSDKCandidates() []string { @@ -283,11 +314,20 @@ func androidSDKCandidates() []string { addRoot(filepath.Join(home, "Library", "Android", "sdk")) addRoot(filepath.Join(home, "Android", "Sdk")) } - addRoot("/opt/homebrew/share/android-commandlinetools") - addRoot("/usr/local/share/android-commandlinetools") + for _, root := range standardSDKRoots { + addRoot(root) + } return roots } +// standardSDKRoots are the install locations checked after the environment and +// the home directory. A var so a resolution test can point it at a fixture +// instead of whatever SDK the host running the test happens to have. +var standardSDKRoots = []string{ + "/opt/homebrew/share/android-commandlinetools", + "/usr/local/share/android-commandlinetools", +} + func listAdbDevices(ctx context.Context) ([]string, error) { adb, err := AdbBinary() if err != nil { @@ -317,7 +357,7 @@ func parseAdbDevices(output string) []string { } func listAVDs(ctx context.Context) ([]string, error) { - emulator, err := emulatorBinary() + emulator, err := EmulatorBinary() if err != nil { return nil, err } @@ -382,7 +422,7 @@ func pickAVD(requested string, available []string) (string, error) { } func bootAVD(_ context.Context, name string) error { - emulator, err := emulatorBinary() + emulator, err := EmulatorBinary() if err != nil { return err } diff --git a/internal/android/android_test.go b/internal/android/android_test.go index 01a017e..2161cd3 100644 --- a/internal/android/android_test.go +++ b/internal/android/android_test.go @@ -1,6 +1,8 @@ package android import ( + "os" + "path/filepath" "reflect" "slices" "strings" @@ -136,6 +138,114 @@ func TestPickAVD_NoneAvailable(t *testing.T) { } } +// fakeSDK writes an SDK layout holding only the named tools ("emulator/emulator"), +// so a lookup test never resolves against the host's own SDK. +func fakeSDK(t *testing.T, tools ...string) string { + t.Helper() + root := t.TempDir() + for _, tool := range tools { + path := filepath.Join(root, filepath.FromSlash(tool)) + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatalf("mkdir %s: %v", filepath.Dir(path), err) + } + if err := os.WriteFile(path, nil, 0o755); err != nil { + t.Fatalf("write %s: %v", path, err) + } + } + return root +} + +// isolateSDKLookup closes every route to the host's own SDK: an empty PATH, no +// root variables, a home with nothing under it, and the standard install +// locations replaced by the given roots. It returns the fake home directory. +func isolateSDKLookup(t *testing.T, roots ...string) string { + t.Helper() + home := t.TempDir() + t.Setenv("PATH", t.TempDir()) + t.Setenv("ANDROID_HOME", "") + t.Setenv("ANDROID_SDK_ROOT", "") + t.Setenv("HOME", home) + original := standardSDKRoots + t.Cleanup(func() { standardSDKRoots = original }) + standardSDKRoots = roots + return home +} + +func TestAdbBinary_ResolvesUnderStandardSDKRootWithAndroidHomeUnset(t *testing.T) { + sdk := fakeSDK(t, "platform-tools/adb") + isolateSDKLookup(t, sdk) + + adb, err := AdbBinary() + if err != nil { + t.Fatalf("AdbBinary: %v", err) + } + if want := filepath.Join(sdk, "platform-tools", "adb"); adb != want { + t.Errorf("AdbBinary() = %q, want %q", adb, want) + } +} + +func TestEmulatorBinary_ResolvesUnderStandardSDKRootWithAndroidHomeUnset(t *testing.T) { + sdk := fakeSDK(t, "emulator/emulator") + isolateSDKLookup(t, sdk) + + emulator, err := EmulatorBinary() + if err != nil { + t.Fatalf("EmulatorBinary: %v", err) + } + if want := filepath.Join(sdk, "emulator", "emulator"); emulator != want { + t.Errorf("EmulatorBinary() = %q, want %q", emulator, want) + } +} + +func TestAdbBinary_UsesAndroidHome(t *testing.T) { + sdk := fakeSDK(t, "platform-tools/adb") + isolateSDKLookup(t) + t.Setenv("ANDROID_HOME", sdk) + + adb, err := AdbBinary() + if err != nil { + t.Fatalf("AdbBinary: %v", err) + } + if want := filepath.Join(sdk, "platform-tools", "adb"); adb != want { + t.Errorf("AdbBinary() = %q, want %q", adb, want) + } +} + +func TestAdbBinary_NoSDKAnywhereReportsWhereItLooked(t *testing.T) { + empty := t.TempDir() + home := isolateSDKLookup(t, empty) + + _, err := AdbBinary() + if err == nil { + t.Fatal("expected an error with no SDK anywhere") + } + for _, want := range []string{ + "adb not found: not on PATH", + filepath.Join(home, "Library", "Android", "sdk", "platform-tools", "adb"), + filepath.Join(empty, "platform-tools", "adb"), + "$ANDROID_HOME and $ANDROID_SDK_ROOT are unset", + "put adb on PATH, or point $ANDROID_HOME at an Android SDK that has platform-tools/adb", + } { + if !strings.Contains(err.Error(), want) { + t.Errorf("error %q missing %q", err, want) + } + } +} + +func TestAdbBinary_AndroidHomePointingNowhereIsNamed(t *testing.T) { + isolateSDKLookup(t, t.TempDir()) + missing := filepath.Join(t.TempDir(), "no-such-sdk") + t.Setenv("ANDROID_HOME", missing) + + _, err := AdbBinary() + if err == nil { + t.Fatal("expected an error when ANDROID_HOME points nowhere") + } + if want := "$ANDROID_HOME=" + missing + " does not exist"; !strings.Contains(err.Error(), want) { + t.Errorf("error %q must say %q rather than leaving it in the tried list", err, want) + } +} + func TestPathContains(t *testing.T) { path := "/usr/bin:/opt/tools:/usr/local/bin" if !pathContains(path, "/opt/tools") {