fix(doctor): resolve adb and emulator the way a run does

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: "adb on PATH or under the Android SDK", Run: checkAdb},
-		{Name: "emulator on PATH or under ANDROID_HOME", Run: checkEmulator},
+		{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 {
+	_, err := android.EmulatorBinary()
-		return nil
+	return err
-	}
-	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
 }
 
 var javaVersionPattern = regexp.MustCompile(`(?:java|openjdk)[^"]*"(\d+)(?:\.(\d+))?`)

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])
 		}