Merge branch 'android-home-diagnostic' into trial-merge

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+))?`)

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

docs/manual/cli.md

@@ -56,7 +56,7 @@ sanderling doctor [--platform web|android|ios|ios-device|all]
 | Platform | Checks |
 |---|---|
 | `web` | headless Chromium can launch (the bundled CDP surface boots a real browser). |
-| `android` | `adb` on PATH; `emulator` on PATH or under `ANDROID_HOME`; Java 17+; embedded native sidecar JAR is real. |
+| `android` | `adb` and `emulator` on PATH, or under `$ANDROID_HOME`, `$ANDROID_SDK_ROOT` or a standard SDK install location; Java 17+; embedded native sidecar JAR is real. |
 | `ios` | `xcrun` on PATH; `simctl` on PATH. The simulator path drives the native companion with no JVM. |
 | `ios-device` | the `ios` checks plus `devicectl`; the macOS `usbmuxd` socket; a connected, paired device; App Store Connect signing credentials present. |
 

internal/android/android.go

@@ -221,11 +221,7 @@ func navOverlayCommand(ctx context.Context, adb, serial, overlay string) *exec.C
 // EnvWithAndroidPlatformTools returns env with the directory containing adb
 // prepended to PATH, so child processes (the sidecar) can invoke adb even
 // when the user hasn't set up their shell PATH.
-func EnvWithAndroidPlatformTools(env []string) []string {
-	adb, err := AdbBinary()
-	if err != nil {
-		return env
-	}
+func EnvWithAndroidPlatformTools(env []string, adb string) []string {
 	adbDir := filepath.Dir(adb)
 	result := make([]string, 0, len(env))
 	found := false
@@ -247,7 +243,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/<subdir>/<name> and $ANDROID_SDK_ROOT/<subdir>/<name>,
@@ -264,7 +262,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 +310,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 +353,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 +418,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
 	}

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") {

internal/testrun/driver.go

@@ -161,10 +161,14 @@ func buildDriver(ctx context.Context, options Options, stdout io.Writer) (driver
 	if options.Device != "" {
 		sidecarArgs = append(sidecarArgs, "--serial", options.Device)
 	}
+	adbPath, err := android.AdbBinary()
+	if err != nil {
+		return nil, nil, preflightFailure("android", err)
+	}
 	sidecarCommand := exec.CommandContext(ctx, "java", sidecarArgs...)
 	sidecarCommand.Stdout = stdout
 	sidecarCommand.Stderr = stdout
-	sidecarCommand.Env = android.EnvWithAndroidPlatformTools(os.Environ())
+	sidecarCommand.Env = android.EnvWithAndroidPlatformTools(os.Environ(), adbPath)
 	// SIGTERM lets the sidecar's shutdown hook stop the iOS XCTest runner.
 	// SIGKILL skips the hook and orphans an xcodebuild session that later
 	// restarts its runner and hijacks the simulator mid-run.
@@ -175,11 +179,13 @@ func buildDriver(ctx context.Context, options Options, stdout io.Writer) (driver
 	if err := sidecarCommand.Start(); err != nil {
 		return nil, nil, fmt.Errorf("spawn sidecar: %w", err)
 	}
-	fmt.Fprintf(stdout, "sidecar pid=%d listening on 127.0.0.1:%d\n", sidecarCommand.Process.Pid, sidecarPort)
+	sidecarExited := watchSidecar(sidecarCommand)
+	address := fmt.Sprintf("127.0.0.1:%d", sidecarPort)
+	fmt.Fprintf(stdout, "sidecar pid=%d listening on %s (adb: %s)\n", sidecarCommand.Process.Pid, address, adbPath)
 
-	driverClient, err := driverSidecar.Dial(fmt.Sprintf("127.0.0.1:%d", sidecarPort))
+	driverClient, err := driverSidecar.Dial(address)
 	if err != nil {
-		stopSidecar(sidecarCommand)
+		stopSidecar(sidecarCommand, sidecarExited)
 		return nil, nil, fmt.Errorf("dial sidecar: %w", err)
 	}
 	driverClient.SetPlatform(options.Platform)
@@ -188,22 +194,82 @@ func buildDriver(ctx context.Context, options Options, stdout io.Writer) (driver
 	// (absorbing the XCUITest startup race) runs inside IosDriverBackend.init
 	// in the sidecar - no additional sleep needed here.
 	healthCtx, healthCancel := context.WithTimeout(ctx, sidecarStartupTimeout)
-	if err := driverClient.WaitForHealth(healthCtx, 250e6); err != nil {
-		healthCancel()
-		stopSidecar(sidecarCommand)
-		_ = driverClient.Close()
-		return nil, nil, fmt.Errorf("sidecar health check: %w", err)
-	}
+	healthErr := awaitSidecar(healthCtx, address, sidecarStartupTimeout, func(pollCtx context.Context) error {
+		return driverClient.WaitForHealth(pollCtx, 250e6)
+	}, sidecarExited)
 	healthCancel()
+	if healthErr != nil {
+		stopSidecar(sidecarCommand, sidecarExited)
+		_ = driverClient.Close()
+		return nil, nil, healthErr
+	}
 	fmt.Fprintln(stdout, "sidecar is healthy")
 
 	cleanup := func() {
 		_ = driverClient.Close()
-		stopSidecar(sidecarCommand)
+		stopSidecar(sidecarCommand, sidecarExited)
 	}
 	return driverClient, cleanup, nil
 }
 
+// watchSidecar reaps the sidecar and publishes its exit status. The channel is
+// closed after the send so the shutdown path can still receive once the startup
+// path has taken the status.
+func watchSidecar(sidecarCommand *exec.Cmd) <-chan error {
+	exited := make(chan error, 1)
+	go func() {
+		exited <- sidecarCommand.Wait()
+		close(exited)
+	}()
+	return exited
+}
+
+// awaitSidecar waits for the sidecar to answer a health check, racing that
+// against the process exiting so a sidecar that dies during startup is reported
+// as the exit it was rather than as a deadline half a minute later. Neither
+// failure knows why the sidecar was unhappy, so both name what to look at
+// instead of picking a cause.
+func awaitSidecar(
+	ctx context.Context,
+	address string,
+	timeout time.Duration,
+	health func(context.Context) error,
+	exited <-chan error,
+) error {
+	healthy := make(chan error, 1)
+	go func() { healthy <- health(ctx) }()
+	select {
+	case exitErr := <-exited:
+		return sidecarExitedError(address, exitErr)
+	case err := <-healthy:
+		if err == nil {
+			return nil
+		}
+		select {
+		case exitErr := <-exited:
+			return sidecarExitedError(address, exitErr)
+		default:
+			return fmt.Errorf(
+				"sidecar did not answer a health check on %s within %s and is still running\n%s",
+				address, timeout, sidecarWhatToCheck,
+			)
+		}
+	}
+}
+
+const sidecarWhatToCheck = "check the sidecar output above, then `sanderling doctor --platform=android` (java 17+, adb, Android SDK)"
+
+func sidecarExitedError(address string, exitErr error) error {
+	status := "exit status 0"
+	if exitErr != nil {
+		status = exitErr.Error()
+	}
+	return fmt.Errorf(
+		"sidecar exited before it answered a health check on %s: %s\n%s",
+		address, status, sidecarWhatToCheck,
+	)
+}
+
 // sidecarShutdownGrace bounds how long the sidecar gets to run its shutdown
 // hook (terminate the app, stop the XCTest runner) before being killed.
 const sidecarShutdownGrace = 15 * time.Second
@@ -211,25 +277,20 @@ const sidecarShutdownGrace = 15 * time.Second
 // stopSidecar terminates the sidecar gracefully so its shutdown hook can stop
 // the device-side runner processes, escalating to SIGKILL when it does not
 // exit within the grace window.
-func stopSidecar(sidecarCommand *exec.Cmd) {
+func stopSidecar(sidecarCommand *exec.Cmd, exited <-chan error) {
 	if sidecarCommand.Process == nil {
 		return
 	}
 	if err := sidecarCommand.Process.Signal(syscall.SIGTERM); err != nil {
 		_ = sidecarCommand.Process.Kill()
-		_ = sidecarCommand.Wait()
+		<-exited
 		return
 	}
-	done := make(chan struct{})
-	go func() {
-		_ = sidecarCommand.Wait()
-		close(done)
-	}()
 	select {
-	case <-done:
+	case <-exited:
 	case <-time.After(sidecarShutdownGrace):
 		_ = sidecarCommand.Process.Kill()
-		<-done
+		<-exited
 	}
 }
 

internal/testrun/driver_test.go

@@ -4,7 +4,10 @@ import (
 	"context"
 	"errors"
 	"io"
+	"os/exec"
+	"strings"
 	"testing"
+	"time"
 
 	"github.com/priyanshujain/sanderling/internal/driver"
 	"github.com/priyanshujain/sanderling/internal/driver/ioscompanion"
@@ -94,6 +97,130 @@ func TestBuildDriverSurfacesDeviceConstructionError(t *testing.T) {
 	}
 }
 
+// A sidecar that dies during startup leaves the health poll with nothing to
+// talk to, and reporting that as a deadline sends the reader after a gRPC
+// timeout instead of the exit that already happened.
+func TestAwaitSidecarReportsTheExitItSaw(t *testing.T) {
+	exited := make(chan error, 1)
+	exited <- errors.New("exit status 1")
+	close(exited)
+
+	err := awaitSidecar(
+		context.Background(),
+		"127.0.0.1:54321",
+		30*time.Second,
+		func(ctx context.Context) error { <-ctx.Done(); return ctx.Err() },
+		exited,
+	)
+	if err == nil {
+		t.Fatal("expected an error when the sidecar exits before it is healthy")
+	}
+	for _, want := range []string{
+		"sidecar exited before it answered a health check on 127.0.0.1:54321: exit status 1",
+		"check the sidecar output above",
+		"sanderling doctor --platform=android",
+	} {
+		if !strings.Contains(err.Error(), want) {
+			t.Errorf("error %q missing %q", err, want)
+		}
+	}
+}
+
+func TestAwaitSidecarTimeoutSaysOnlyWhatItObserved(t *testing.T) {
+	ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond)
+	defer cancel()
+
+	err := awaitSidecar(
+		ctx,
+		"127.0.0.1:54321",
+		50*time.Millisecond,
+		func(ctx context.Context) error { <-ctx.Done(); return ctx.Err() },
+		make(chan error, 1),
+	)
+	if err == nil {
+		t.Fatal("expected an error when the sidecar never answers")
+	}
+	for _, want := range []string{
+		"sidecar did not answer a health check on 127.0.0.1:54321 within 50ms and is still running",
+		"check the sidecar output above",
+		"sanderling doctor --platform=android",
+	} {
+		if !strings.Contains(err.Error(), want) {
+			t.Errorf("error %q missing %q", err, want)
+		}
+	}
+	if strings.Contains(err.Error(), "context deadline exceeded") {
+		t.Errorf("error %q must not hand the reader a bare gRPC deadline", err)
+	}
+}
+
+func TestAwaitSidecarHealthyReturnsNil(t *testing.T) {
+	err := awaitSidecar(
+		context.Background(),
+		"127.0.0.1:54321",
+		30*time.Second,
+		func(context.Context) error { return nil },
+		make(chan error, 1),
+	)
+	if err != nil {
+		t.Fatalf("expected a healthy sidecar to pass, got %v", err)
+	}
+}
+
+func TestStopSidecarTerminatesARunningSidecar(t *testing.T) {
+	command := exec.Command("sleep", "60")
+	if err := command.Start(); err != nil {
+		t.Fatalf("start: %v", err)
+	}
+	exited := watchSidecar(command)
+
+	stopped := make(chan struct{})
+	go func() {
+		stopSidecar(command, exited)
+		close(stopped)
+	}()
+	select {
+	case <-stopped:
+	case <-time.After(10 * time.Second):
+		t.Fatal("stopSidecar never returned for a running sidecar")
+	}
+	if got := command.ProcessState.String(); got != "signal: terminated" {
+		t.Errorf("sidecar ended as %q, want the SIGTERM its shutdown hook needs", got)
+	}
+}
+
+// The startup path takes the exit status to report it, so the shutdown path
+// that follows must not sit waiting for a status nobody will send again.
+func TestStopSidecarAfterTheStartupPathTookTheExitStatus(t *testing.T) {
+	command := exec.Command("sh", "-c", "exit 3")
+	if err := command.Start(); err != nil {
+		t.Fatalf("start: %v", err)
+	}
+	exited := watchSidecar(command)
+
+	err := awaitSidecar(
+		context.Background(),
+		"127.0.0.1:54321",
+		30*time.Second,
+		func(ctx context.Context) error { <-ctx.Done(); return ctx.Err() },
+		exited,
+	)
+	if err == nil || !strings.Contains(err.Error(), "exit status 3") {
+		t.Fatalf("expected the sidecar's real exit status, got %v", err)
+	}
+
+	stopped := make(chan struct{})
+	go func() {
+		stopSidecar(command, exited)
+		close(stopped)
+	}()
+	select {
+	case <-stopped:
+	case <-time.After(10 * time.Second):
+		t.Fatal("stopSidecar blocked on an exit status the startup path had already taken")
+	}
+}
+
 // stubPreflight bypasses the host-readiness checks so routing tests exercise
 // driver construction on a Linux CI runner that lacks xcrun/java.
 func stubPreflight(t *testing.T) {

internal/testrun/preflight.go

@@ -4,6 +4,8 @@ import (
 	"context"
 	"fmt"
 	"os/exec"
+
+	"github.com/priyanshujain/sanderling/internal/android"
 )
 
 // Preflight runs platform-specific host checks before sidecar/driver setup.
@@ -18,7 +20,15 @@ func Preflight(ctx context.Context, platform string) error {
 
 type preflightFunc func(name string) error
 
+// preflightCheck resolves adb through the same helper every adb call in a run
+// uses, so a host whose SDK is only reachable through $ANDROID_HOME or a
+// standard install location is not turned away here and then driven fine by
+// the rest of the pipeline.
 func preflightCheck(name string) error {
+	if name == "adb" {
+		_, err := android.AdbBinary()
+		return err
+	}
 	if _, err := exec.LookPath(name); err != nil {
 		return fmt.Errorf("%s not found on PATH: %w", name, err)
 	}

internal/testrun/preflight_test.go

@@ -3,6 +3,8 @@ package testrun
 import (
 	"context"
 	"errors"
+	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 )
@@ -49,6 +51,33 @@ func TestPreflight_AndroidNeedsAdbAndJava(t *testing.T) {
 	}
 }
 
+// Every adb call in an android run resolves through $ANDROID_HOME and the
+// standard SDK locations, so a preflight that only looks at PATH turns away a
+// host the run itself would drive.
+func TestPreflight_AndroidAcceptsAdbUnderAndroidHome(t *testing.T) {
+	sdk := t.TempDir()
+	writeExecutable(t, filepath.Join(sdk, "platform-tools", "adb"))
+	pathDirectory := t.TempDir()
+	writeExecutable(t, filepath.Join(pathDirectory, "java"))
+	t.Setenv("PATH", pathDirectory)
+	t.Setenv("ANDROID_HOME", sdk)
+	t.Setenv("ANDROID_SDK_ROOT", "")
+
+	if err := Preflight(context.Background(), "android"); err != nil {
+		t.Fatalf("Preflight with adb under $ANDROID_HOME: %v", err)
+	}
+}
+
+func writeExecutable(t *testing.T, path string) {
+	t.Helper()
+	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)
+	}
+}
+
 func TestPreflight_iOSNeedsXcrun(t *testing.T) {
 	check := func(name string) error {
 		if name == "xcrun" {