From 743b62b2bdb8b5843dfb4c1158b895cd802e5ae4 Mon Sep 17 00:00:00 2001 From: PJ Date: Mon, 8 Jun 2026 23:20:35 +0530 Subject: [PATCH] feat(doctor): device prereqs replace java/sidecar for ios-device iosDeviceChecks now verifies devicectl, iproxy on PATH, a connected+paired device (via ios.ConnectedDevices), and App Store Connect signing creds (via ioscompanion.VerifyDeviceSigning). The retired JVM sidecar checks stay only under android. --- cmd/sanderling/doctor.go | 53 ++++++++++++++-- cmd/sanderling/doctor_test.go | 66 +++++++++++++++++--- internal/driver/ioscompanion/devicerunner.go | 8 +++ internal/ios/device.go | 6 ++ 4 files changed, 120 insertions(+), 13 deletions(-) diff --git a/cmd/sanderling/doctor.go b/cmd/sanderling/doctor.go index 40072ce..c7226c5 100644 --- a/cmd/sanderling/doctor.go +++ b/cmd/sanderling/doctor.go @@ -14,6 +14,8 @@ import ( "github.com/chromedp/chromedp" + "github.com/priyanshujain/sanderling/internal/driver/ioscompanion" + "github.com/priyanshujain/sanderling/internal/ios" "github.com/priyanshujain/sanderling/internal/sidecarassets" ) @@ -59,8 +61,8 @@ func androidChecks() []doctorCheck { // iosChecks covers the simulator path, which the native companion drives with // no JVM. A simulator host with no Java still passes. Physical-device runs -// additionally need java and the sidecar JAR, covered by iosDeviceChecks and -// surfaced through the "all" union. +// additionally need devicectl, iproxy, a connected device, and signing +// credentials, covered by iosDeviceChecks and surfaced through the "all" union. func iosChecks() []doctorCheck { return []doctorCheck{ {Name: "xcrun on PATH (ios simulator)", Run: checkExecutableOnPath("xcrun")}, @@ -68,12 +70,16 @@ func iosChecks() []doctorCheck { } } -// iosDeviceChecks covers the extra prerequisites a physical iOS device needs: -// the JVM and a real sidecar JAR for the sidecar driver path. +// iosDeviceChecks covers the prerequisites a physical iOS device needs: the +// runner is built and driven over a usbmux tunnel, so devicectl installs the +// app, iproxy forwards the tunnel, a device must be connected and paired, and +// App Store Connect signing credentials must be present for the no-UI build. func iosDeviceChecks() []doctorCheck { return []doctorCheck{ - {Name: "java 17+ on PATH (ios physical device)", Run: checkJavaVersion}, - {Name: "sidecar JAR is real (ios physical device)", Run: checkSidecarJAR}, + {Name: "devicectl available (ios physical device)", Run: checkDevicectl}, + {Name: "iproxy on PATH (ios physical device)", Run: checkExecutableOnPath("iproxy")}, + {Name: "an iOS device is connected and paired", Run: checkDeviceConnected}, + {Name: "App Store Connect signing credentials present", Run: checkDeviceSigning}, } } @@ -120,6 +126,41 @@ func checkSimctl(ctx context.Context) error { return nil } +// Device-check seams: package-level so the doctor's device checks run against +// canned results instead of a real device. +var ( + doctorConnectedDevices = ios.ConnectedDevices + doctorVerifySigning = ioscompanion.VerifyDeviceSigning +) + +// checkDevicectl exercises `xcrun devicectl --version`: devicectl is an xcrun +// subcommand, so a PATH lookup cannot find it. +func checkDevicectl(ctx context.Context) error { + if err := exec.CommandContext(ctx, "xcrun", "devicectl", "--version").Run(); err != nil { + return fmt.Errorf("xcrun devicectl --version: %w", err) + } + return nil +} + +// checkDeviceConnected confirms at least one physical iOS device is connected +// and paired, the prerequisite for the tunnel and the install. +func checkDeviceConnected(ctx context.Context) error { + devices, err := doctorConnectedDevices(ctx) + if err != nil { + return err + } + if len(devices) == 0 { + return fmt.Errorf("no connected iOS device; connect and pair an iPhone") + } + return nil +} + +// checkDeviceSigning confirms the App Store Connect signing environment is +// complete and the key file exists, so the no-UI device build can sign. +func checkDeviceSigning(_ context.Context) error { + return doctorVerifySigning() +} + func checkSidecarJAR(_ context.Context) error { if sidecarassets.IsPlaceholder() { return fmt.Errorf("placeholder JAR embedded; run `make sidecar && make sanderling` to embed the real fat JAR") diff --git a/cmd/sanderling/doctor_test.go b/cmd/sanderling/doctor_test.go index d6c464d..8e10d8e 100644 --- a/cmd/sanderling/doctor_test.go +++ b/cmd/sanderling/doctor_test.go @@ -6,8 +6,12 @@ import ( "errors" "flag" "io" + "os" + "path/filepath" "strings" "testing" + + "github.com/priyanshujain/sanderling/internal/ios" ) func TestRunDoctorChecks_AllPass(t *testing.T) { @@ -142,15 +146,63 @@ func TestDoctorChecksFor_iOSSimulator_OmitsJava(t *testing.T) { } } -func TestDoctorChecksFor_iOSDevice_IncludesJava(t *testing.T) { - found := false - for _, c := range doctorChecksFor("ios-device") { - if strings.Contains(c.Name, "java") { - found = true +func TestDoctorChecksFor_iOSDevice_CoversDevicePrereqs(t *testing.T) { + checks := doctorChecksFor("ios-device") + for _, c := range checks { + if strings.Contains(c.Name, "java") || strings.Contains(c.Name, "sidecar") { + t.Errorf("device checks must not include the retired %q", c.Name) } } - if !found { - t.Error("ios-device checks must include java for the sidecar path") + for _, want := range []string{"devicectl", "iproxy", "connected and paired", "signing credentials"} { + found := false + for _, c := range checks { + if strings.Contains(c.Name, want) { + found = true + } + } + if !found { + t.Errorf("ios-device checks missing %q: %+v", want, checks) + } + } +} + +func TestCheckDeviceConnected(t *testing.T) { + original := doctorConnectedDevices + t.Cleanup(func() { doctorConnectedDevices = original }) + + doctorConnectedDevices = func(context.Context) ([]ios.Device, error) { + return []ios.Device{{Name: "iPhone"}}, nil + } + if err := checkDeviceConnected(context.Background()); err != nil { + t.Fatalf("a connected device must pass: %v", err) + } + + doctorConnectedDevices = func(context.Context) ([]ios.Device, error) { return nil, nil } + if err := checkDeviceConnected(context.Background()); err == nil { + t.Fatal("no device must fail") + } +} + +func TestCheckDeviceSigning_EnvAndKeyFile(t *testing.T) { + // The signing check defers to the driver's credential verification, which + // reads the same environment a real build would. + for _, key := range []string{"SANDERLING_IOS_TEAM", "DEVELOPMENT_TEAM", "ASC_API_KEY_PATH", "ASC_API_KEY_ID", "ASC_API_ISSUER_ID"} { + t.Setenv(key, "") + } + if err := checkDeviceSigning(context.Background()); err == nil { + t.Fatal("missing signing env must fail") + } + + keyPath := filepath.Join(t.TempDir(), "AuthKey.p8") + if err := os.WriteFile(keyPath, []byte("key"), 0o600); err != nil { + t.Fatal(err) + } + t.Setenv("SANDERLING_IOS_TEAM", "TEAM1") + t.Setenv("ASC_API_KEY_ID", "KID") + t.Setenv("ASC_API_ISSUER_ID", "ISS") + t.Setenv("ASC_API_KEY_PATH", keyPath) + if err := checkDeviceSigning(context.Background()); err != nil { + t.Fatalf("complete signing env with a present key must pass: %v", err) } } diff --git a/internal/driver/ioscompanion/devicerunner.go b/internal/driver/ioscompanion/devicerunner.go index f193cc3..126a01e 100644 --- a/internal/driver/ioscompanion/devicerunner.go +++ b/internal/driver/ioscompanion/devicerunner.go @@ -75,6 +75,14 @@ func readSigningCredentials() (signingCredentials, error) { return creds, nil } +// VerifyDeviceSigning reports whether the device signing environment is complete +// and the App Store Connect key file exists. The doctor calls it so the device +// preflight surfaces missing credentials before a run reaches the build step. +func VerifyDeviceSigning() error { + _, err := readSigningCredentials() + return err +} + func firstNonEmpty(values ...string) string { for _, value := range values { if value != "" { diff --git a/internal/ios/device.go b/internal/ios/device.go index 816b8e2..86ea661 100644 --- a/internal/ios/device.go +++ b/internal/ios/device.go @@ -39,6 +39,12 @@ type coreDeviceList struct { // canned devicectl output without invoking xcrun. var listDevices = coreDevices +// ConnectedDevices lists the physical iOS devices devicectl reports. The doctor +// uses it to confirm at least one device is connected and paired. +func ConnectedDevices(ctx context.Context) ([]Device, error) { + return listDevices(ctx) +} + // ResolveDevice picks the physical iOS device a run drives. An empty query // resolves the single connected device (an error names them all when several // are connected). A non-empty query matches a device by name, hardware UDID, or