From 22115a6c25aacd0fab313bc7242092fbbcfc3642 Mon Sep 17 00:00:00 2001 From: PJ Date: Sat, 15 Aug 2026 22:38:23 +0530 Subject: [PATCH] fix(doctor): resolve adb and emulator the way a run does --- cmd/sanderling/doctor.go | 31 ++++++++++---------------- cmd/sanderling/doctor_test.go | 41 ++++++++++++++++++++++++++++++++++- 2 files changed, 52 insertions(+), 20 deletions(-) diff --git a/cmd/sanderling/doctor.go b/cmd/sanderling/doctor.go index d642c3e..e206ce5 100644 --- a/cmd/sanderling/doctor.go +++ b/cmd/sanderling/doctor.go @@ -5,15 +5,14 @@ import ( "flag" "fmt" "io" - "os" "os/exec" - "path/filepath" "regexp" "strconv" "time" "github.com/chromedp/chromedp" + "github.com/priyanshujain/sanderling/internal/android" "github.com/priyanshujain/sanderling/internal/driver/ioscompanion" "github.com/priyanshujain/sanderling/internal/ios" "github.com/priyanshujain/sanderling/internal/sidecarassets" @@ -52,8 +51,8 @@ func webChecks() []doctorCheck { func androidChecks() []doctorCheck { return []doctorCheck{ - {Name: "adb on PATH", Run: checkExecutableOnPath("adb")}, - {Name: "emulator on PATH or under ANDROID_HOME", Run: checkEmulator}, + {Name: "adb on PATH or under the Android SDK", Run: checkAdb}, + {Name: "emulator on PATH or under the Android SDK", Run: checkEmulator}, {Name: "java 17+ on PATH", Run: checkJavaVersion}, {Name: "sidecar JAR is real (not placeholder)", Run: checkSidecarJAR}, } @@ -235,22 +234,16 @@ func checkExecutableOnPath(name string) func(context.Context) error { } } +// checkAdb and checkEmulator resolve through the same helpers a run uses, so a +// host the doctor passes is a host a run can drive. +func checkAdb(_ context.Context) error { + _, err := android.AdbBinary() + return err +} + func checkEmulator(_ context.Context) error { - if _, err := exec.LookPath("emulator"); err == nil { - return nil - } - androidHome := os.Getenv("ANDROID_HOME") - if androidHome == "" { - androidHome = os.Getenv("ANDROID_SDK_ROOT") - } - if androidHome == "" { - return fmt.Errorf("not on PATH and ANDROID_HOME is unset") - } - candidate := filepath.Join(androidHome, "emulator", "emulator") - if _, err := os.Stat(candidate); err != nil { - return fmt.Errorf("not found at %s", candidate) - } - return nil + _, err := android.EmulatorBinary() + return err } var javaVersionPattern = regexp.MustCompile(`(?:java|openjdk)[^"]*"(\d+)(?:\.(\d+))?`) diff --git a/cmd/sanderling/doctor_test.go b/cmd/sanderling/doctor_test.go index 3155180..489b498 100644 --- a/cmd/sanderling/doctor_test.go +++ b/cmd/sanderling/doctor_test.go @@ -6,6 +6,8 @@ import ( "errors" "flag" "io" + "os" + "path/filepath" "strings" "testing" @@ -110,6 +112,43 @@ func TestDoctorChecksFor_Android_IncludesADB(t *testing.T) { } } +// The SDK tools a run invokes are resolved through $ANDROID_HOME and the +// standard install locations, never PATH alone, so a doctor that turns away a +// host on a PATH lookup condemns a setup every run on it would drive fine. +func TestAndroidChecks_AcceptSDKToolsThatAreNotOnPath(t *testing.T) { + sdk := t.TempDir() + for _, tool := range []string{"platform-tools/adb", "emulator/emulator"} { + path := filepath.Join(sdk, 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) + } + } + t.Setenv("PATH", t.TempDir()) + t.Setenv("ANDROID_HOME", sdk) + t.Setenv("ANDROID_SDK_ROOT", "") + + var sdkChecks []doctorCheck + for _, check := range doctorChecksFor("android") { + if strings.Contains(check.Name, "adb") || strings.Contains(check.Name, "emulator") { + sdkChecks = append(sdkChecks, check) + } + } + if len(sdkChecks) != 2 { + t.Fatalf("expected the adb and emulator checks, got %d", len(sdkChecks)) + } + + var stdout bytes.Buffer + if err := runDoctorChecks(context.Background(), sdkChecks, &stdout); err != nil { + t.Fatalf("doctor: %v\n%s", err, stdout.String()) + } + if strings.Contains(stdout.String(), "FAIL") { + t.Errorf("doctor rejected an SDK it can resolve:\n%s", stdout.String()) + } +} + func TestDoctorChecksFor_iOS_IncludesXcrun(t *testing.T) { checks := doctorChecksFor("ios") found := false @@ -129,7 +168,7 @@ func TestDoctorChecksFor_All_IsUnion(t *testing.T) { for _, c := range all { names[c.Name]++ } - for _, name := range []string{"adb on PATH", "xcrun on PATH (ios simulator)", "headless chromium can launch"} { + for _, name := range []string{"adb on PATH or under the Android SDK", "xcrun on PATH (ios simulator)", "headless chromium can launch"} { if names[name] != 1 { t.Errorf("expected %q in 'all' exactly once, got %d", name, names[name]) }