diff --git a/internal/driver/ioscompanion/device.go b/internal/driver/ioscompanion/device.go index 67536a7..52903ab 100644 --- a/internal/driver/ioscompanion/device.go +++ b/internal/driver/ioscompanion/device.go @@ -31,17 +31,22 @@ type DeviceOptions struct { BundleID string // AppPath is the .app bundle installed via devicectl for clear-state. AppPath string + // ClearState reinstalls the app while NewDevice runs, before the runner's + // test session exists. Clear state is a property of the driver rather than + // of a launch: see Launch. + ClearState bool // Output receives the runner session log path and driver warnings. Output io.Writer // DoubleTapGapMilliseconds overrides the synthesized double-tap gap. DoubleTapGapMilliseconds float64 // Test seams. Production leaves them nil and NewDevice wires the real - // build/spawn/tunnel/dial. - spawnRunner func(ctx context.Context, address string) (*exec.Cmd, error) - startTunnel func(ctx context.Context, hardwareUDID, localAddress, devicePort string) (io.Closer, error) - dialRunner func(address string) (transport.Companion, error) - pickAddress func() (string, error) + // build/spawn/tunnel/dial/devicectl. + spawnRunner func(ctx context.Context, address string) (*exec.Cmd, error) + startTunnel func(ctx context.Context, hardwareUDID, localAddress, devicePort string) (io.Closer, error) + dialRunner func(address string) (transport.Companion, error) + pickAddress func() (string, error) + reinstallApp func(ctx context.Context) error } // deviceStartupTimeout bounds the runner's startup once its hosting test @@ -61,6 +66,9 @@ func NewDevice(ctx context.Context, options DeviceOptions) (*Driver, error) { if options.CoreDeviceID == "" { return nil, errors.New("ios device: CoreDeviceID is required") } + if options.ClearState && options.BundleID == "" { + return nil, errors.New("ios device: clear-state needs BundleID: there is nothing to uninstall without it") + } output := options.Output if output == nil { output = io.Discard @@ -75,6 +83,7 @@ func NewDevice(ctx context.Context, options DeviceOptions) (*Driver, error) { coreDeviceID: options.CoreDeviceID, bundleID: options.BundleID, appPath: options.AppPath, + clearStateAtStartup: options.ClearState, output: output, doubleTapGapMilliseconds: gap, deviceMode: true, @@ -103,7 +112,10 @@ func NewDevice(ctx context.Context, options DeviceOptions) (*Driver, error) { // Device seams: clear-state reinstalls via devicectl; the container reset and // paste grant are simulator-only and become no-ops. The runner types // natively, so no paste prompt is ever hit. - d.reinstallApp = d.devicectlReinstall + d.reinstallApp = options.reinstallApp + if d.reinstallApp == nil { + d.reinstallApp = d.devicectlReinstall + } d.resetContainer = d.deviceResetContainerUnsupported d.grantPaste = func(context.Context) error { return nil } d.restart = d.respawnDevice @@ -116,6 +128,13 @@ func NewDevice(ctx context.Context, options DeviceOptions) (*Driver, error) { } d.deviceLock = lock + if options.ClearState { + if err := d.clearAppState(ctx); err != nil { + d.Close() + return nil, err + } + } + if err := d.bringUpDevice(ctx); err != nil { d.Close() return nil, err diff --git a/internal/driver/ioscompanion/device_test.go b/internal/driver/ioscompanion/device_test.go index 5cacfc0..1ffd11d 100644 --- a/internal/driver/ioscompanion/device_test.go +++ b/internal/driver/ioscompanion/device_test.go @@ -6,6 +6,7 @@ import ( "io" "net" "os/exec" + "slices" "testing" "github.com/priyanshujain/sanderling/internal/driver/ioscompanion/transport" @@ -152,6 +153,35 @@ func TestDeviceEraseAndPressKeyRouteThroughEditor(t *testing.T) { } } +func TestNewDeviceReinstallsOnceBeforeTheRunnerSession(t *testing.T) { + address := startLoopbackListener(t) + probe := &clearStateProbe{} + options := testDeviceOptions(address, newDeviceCompanion()) + options.HardwareUDID = "00008140-CLEAR" + options.AppPath = "/tmp/Sample.app" + options.ClearState = true + options.reinstallApp = func(context.Context) error { probe.record("reinstall"); return nil } + spawn := options.spawnRunner + options.spawnRunner = func(ctx context.Context, runnerAddress string) (*exec.Cmd, error) { + probe.record("runner session") + return spawn(ctx, runnerAddress) + } + + d, err := NewDevice(context.Background(), options) + if err != nil { + t.Fatalf("NewDevice: %v", err) + } + defer d.Close() + if err := d.Launch(context.Background(), "", true, nil); err != nil { + t.Fatalf("Launch: %v", err) + } + + want := []string{"reinstall", "runner session"} + if got := probe.recorded(); !slices.Equal(got, want) { + t.Fatalf("calls = %v, want %v: devicectl must reinstall once, before the runner's test session attaches", got, want) + } +} + func TestDeviceClearStateWithoutAppPathWarnsOnce(t *testing.T) { output := &bytes.Buffer{} d := &Driver{output: output, deviceMode: true} diff --git a/internal/driver/ioscompanion/driver.go b/internal/driver/ioscompanion/driver.go index 2db8019..04b7d7c 100644 --- a/internal/driver/ioscompanion/driver.go +++ b/internal/driver/ioscompanion/driver.go @@ -73,16 +73,24 @@ type Options struct { // AppPath is the .app bundle directory. Required for clear-state reinstall; // when empty, clear state falls back to resetting the data container. AppPath string + // ClearState resets the app to first-launch state while New runs, before + // any automation session attaches. Clear state is a property of the driver + // rather than of a launch: see Launch. + ClearState bool // Output receives companion stdout and stderr plus driver warnings. Output io.Writer // DoubleTapGapMilliseconds overrides the synthesized double-tap gap. DoubleTapGapMilliseconds float64 - // spawnChild, dialCompanion, and pickAddress are test seams. Production - // leaves them nil and New wires the real extraction, spawn, and dial. - spawnChild func(ctx context.Context, address string) (*exec.Cmd, error) - dialCompanion func(address string) (transport.Companion, error) - pickAddress func() (string, error) + // These are test seams. Production leaves them nil and New wires the real + // extraction, spawn, dial and simctl calls. + spawnChild func(ctx context.Context, address string) (*exec.Cmd, error) + dialCompanion func(address string) (transport.Companion, error) + pickAddress func() (string, error) + spawnRunner func(ctx context.Context, address string) (*exec.Cmd, error) + dialRunner func(address string) (transport.Companion, error) + reinstallApp func(ctx context.Context) error + resetContainer func(ctx context.Context) error } // Driver implements driver.DeviceDriver against an iOS simulator companion. @@ -93,6 +101,12 @@ type Driver struct { appPath string output io.Writer + // clearStateAtStartup records that New (or NewDevice) reset the app to + // first-launch state before attaching, which is the only point in a run + // where clearing is safe. Launch refuses a clear-state request the driver + // was not built for rather than reinstalling under a live session. + clearStateAtStartup bool + screenWidth int screenHeight int @@ -137,12 +151,13 @@ type Driver struct { // lifecycle, screenshot) with an in-simulator runner that serves // collapse-free accessibility snapshots and native unicode typing. // runnerClient is nil on the legacy-only path. - runnerClient transport.Companion - runnerChild *exec.Cmd - runnerAddress string - spawnRunner func(ctx context.Context, address string) (*exec.Cmd, error) - dialRunner func(address string) (transport.Companion, error) - hybrid bool + runnerClient transport.Companion + runnerChild *exec.Cmd + runnerAddress string + spawnRunner func(ctx context.Context, address string) (*exec.Cmd, error) + dialRunner func(address string) (transport.Companion, error) + pickRunnerAddress func() (string, error) + hybrid bool // Device-mode fields. On the physical-device path d.companion is the runner // dialed over a usbmux tunnel, hybrid is false, and runnerClient is nil. @@ -197,6 +212,9 @@ func New(ctx context.Context, options Options) (*Driver, error) { if options.UniqueDeviceIdentifier == "" { return nil, errors.New("ios companion: UniqueDeviceIdentifier is required") } + if options.ClearState && options.BundleID == "" { + return nil, errors.New("ios companion: clear-state needs BundleID: there is nothing to uninstall or wipe without it") + } output := options.Output if output == nil { output = io.Discard @@ -210,10 +228,15 @@ func New(ctx context.Context, options Options) (*Driver, error) { udid: options.UniqueDeviceIdentifier, bundleID: options.BundleID, appPath: options.AppPath, + clearStateAtStartup: options.ClearState, output: output, doubleTapGapMilliseconds: gap, spawnChild: options.spawnChild, dial: options.dialCompanion, + spawnRunner: options.spawnRunner, + dialRunner: options.dialRunner, + reinstallApp: options.reinstallApp, + resetContainer: options.resetContainer, hybrid: hybridCompanionEnabled(), } if driverInstance.spawnChild == nil { @@ -240,9 +263,14 @@ func New(ctx context.Context, options Options) (*Driver, error) { return nil, err } driverInstance.address = address + driverInstance.pickRunnerAddress = pickAddress driverInstance.restart = driverInstance.respawnAndRedial - driverInstance.resetContainer = driverInstance.resetDataContainer - driverInstance.reinstallApp = driverInstance.simctlReinstall + if driverInstance.resetContainer == nil { + driverInstance.resetContainer = driverInstance.resetDataContainer + } + if driverInstance.reinstallApp == nil { + driverInstance.reinstallApp = driverInstance.simctlReinstall + } driverInstance.grantPaste = driverInstance.grantPasteboardAccess driverInstance.processContext, driverInstance.processCancel = context.WithCancel(ctx) @@ -253,6 +281,13 @@ func New(ctx context.Context, options Options) (*Driver, error) { } driverInstance.deviceLock = lock + if options.ClearState { + if err := driverInstance.clearAppState(ctx); err != nil { + driverInstance.Close() + return nil, err + } + } + if err := driverInstance.bringUp(ctx); err != nil { driverInstance.Close() return nil, err @@ -366,7 +401,7 @@ func (d *Driver) bringUpRunner(ctx context.Context) error { // A fresh port every bring-up: after a restart the dying session's // listener may still answer on the old port and would satisfy the wait // below with a dead server. - address, err := pickLoopbackAddress() + address, err := d.pickRunnerAddress() if err != nil { return err } @@ -460,6 +495,13 @@ func (d *Driver) Launch(ctx context.Context, bundleID string, clearState bool, e // loudly rather than silently dropping the request. return errors.New("ios companion: launch with environment variables is unsupported on this backend") } + if clearState && !d.clearStateAtStartup { + // Clearing here would uninstall and reinstall the app underneath a live + // automation session, which is what races FrontBoard's registration and + // leaves the session launching a bundle FrontBoard has not registered. + return errors.New("ios companion: clear-state must be requested when the driver is created (Options.ClearState); " + + "this backend clears the app before its automation session exists") + } // Terminate first so the launch is a clean cold start regardless of the // app's prior state. A not-running app is not an error here. @@ -467,12 +509,6 @@ func (d *Driver) Launch(ctx context.Context, bundleID string, clearState bool, e return companion.Terminate(callCtx, d.bundleID) }) - if clearState { - if err := d.clearAppState(ctx); err != nil { - return err - } - } - // Grant the app pasteboard access before it runs so unicode input (which // must go through the pasteboard, since HID cannot express it) never trips // the iOS paste-permission prompt. clearState reinstall resets the grant, @@ -560,7 +596,8 @@ func (d *Driver) lifecycleCompanion() transport.Companion { // clearAppState resets the app to a first-launch state. With an app path it // uninstalls and reinstalls; without one it falls back to wiping the app's data -// container and warns once that a full reinstall needs the app path. +// container and warns once that a full reinstall needs the app path. Called +// only from construction, before any automation session is attached to the app. func (d *Driver) clearAppState(ctx context.Context) error { if d.appPath != "" { if err := d.reinstallApp(ctx); err != nil { diff --git a/internal/driver/ioscompanion/driver_test.go b/internal/driver/ioscompanion/driver_test.go index e69433c..db3b2f4 100644 --- a/internal/driver/ioscompanion/driver_test.go +++ b/internal/driver/ioscompanion/driver_test.go @@ -12,6 +12,7 @@ import ( "os" "os/exec" "path/filepath" + "slices" "strings" "sync" "testing" @@ -182,47 +183,149 @@ func TestLaunchContinuesWhenGrantFails(t *testing.T) { } } -func TestLaunchClearStateReinstallsWithAppPath(t *testing.T) { +// clearStateProbe records, in order, the calls a run makes to reset the app and +// to bring the runner's automation session up. A reinstall recorded after the +// session is the ordering that races FrontBoard. +type clearStateProbe struct { + mutex sync.Mutex + events []string +} + +func (p *clearStateProbe) record(event string) { + p.mutex.Lock() + defer p.mutex.Unlock() + p.events = append(p.events, event) +} + +func (p *clearStateProbe) recorded() []string { + p.mutex.Lock() + defer p.mutex.Unlock() + out := make([]string, len(p.events)) + copy(out, p.events) + return out +} + +// clearStateOptions wires every seam a hybrid bring-up needs, so New runs its +// real sequence against fakes: no simulator, no simctl, no XCTest session. +func clearStateOptions(t *testing.T, probe *clearStateProbe, udid string, clearState bool) Options { + t.Helper() + t.Setenv("SANDERLING_SIMULATOR_COMPANION", "") + address := startLoopbackListener(t) + return Options{ + UniqueDeviceIdentifier: udid, + BundleID: "com.example.app", + ClearState: clearState, + Output: &bytes.Buffer{}, + pickAddress: func() (string, error) { return address, nil }, + spawnChild: func(context.Context, string) (*exec.Cmd, error) { return &exec.Cmd{}, nil }, + dialCompanion: func(string) (transport.Companion, error) { + return &fakeCompanion{accessibilityJSON: "[]"}, nil + }, + spawnRunner: func(context.Context, string) (*exec.Cmd, error) { + probe.record("runner session") + return &exec.Cmd{}, nil + }, + dialRunner: func(string) (transport.Companion, error) { + return &fakeCompanion{accessibilityJSON: "[]"}, nil + }, + reinstallApp: func(context.Context) error { probe.record("reinstall"); return nil }, + resetContainer: func(context.Context) error { probe.record("reset container"); return nil }, + } +} + +func TestClearStateReinstallsOnceBeforeTheRunnerSession(t *testing.T) { + probe := &clearStateProbe{} + options := clearStateOptions(t, probe, "CLEAR-REINSTALL-UDID", true) + options.AppPath = "/tmp/Sample.app" + + d, err := New(context.Background(), options) + if err != nil { + t.Fatalf("New: %v", err) + } + defer d.Close() + if err := d.Launch(context.Background(), "", true, nil); err != nil { + t.Fatalf("Launch: %v", err) + } + + want := []string{"reinstall", "runner session"} + if got := probe.recorded(); !slices.Equal(got, want) { + t.Fatalf("calls = %v, want %v: the reinstall must run once, before the automation session attaches", got, want) + } +} + +func TestClearStateWithoutAppPathWipesContainerBeforeTheRunnerSession(t *testing.T) { + probe := &clearStateProbe{} + output := &bytes.Buffer{} + options := clearStateOptions(t, probe, "CLEAR-CONTAINER-UDID", true) + options.Output = output + + d, err := New(context.Background(), options) + if err != nil { + t.Fatalf("New: %v", err) + } + defer d.Close() + if err := d.Launch(context.Background(), "", true, nil); err != nil { + t.Fatalf("Launch: %v", err) + } + + want := []string{"reset container", "runner session"} + if got := probe.recorded(); !slices.Equal(got, want) { + t.Fatalf("calls = %v, want %v: the fallback must wipe the container once, before the session, and never reinstall", got, want) + } + if warnings := strings.Count(output.String(), "resetting the data container only"); warnings != 1 { + t.Fatalf("warning emitted %d times, want once", warnings) + } +} + +func TestWithoutClearStateTheAppIsLeftAlone(t *testing.T) { + probe := &clearStateProbe{} + options := clearStateOptions(t, probe, "NO-CLEAR-UDID", false) + options.AppPath = "/tmp/Sample.app" + + d, err := New(context.Background(), options) + if err != nil { + t.Fatalf("New: %v", err) + } + defer d.Close() + if err := d.Launch(context.Background(), "", false, nil); err != nil { + t.Fatalf("Launch: %v", err) + } + + want := []string{"runner session"} + if got := probe.recorded(); !slices.Equal(got, want) { + t.Fatalf("calls = %v, want %v: a run that did not ask for clear state must not touch the install", got, want) + } +} + +func TestLaunchRefusesClearStateTheDriverWasNotBuiltFor(t *testing.T) { companion := &fakeCompanion{accessibilityJSON: "[]"} d := newTestDriver(companion) d.appPath = "/tmp/Sample.app" reinstalls := 0 d.reinstallApp = func(context.Context) error { reinstalls++; return nil } - if err := d.Launch(context.Background(), "", true, nil); err != nil { - t.Fatalf("Launch: %v", err) + + err := d.Launch(context.Background(), "", true, nil) + if err == nil || !strings.Contains(err.Error(), "clear-state") { + t.Fatalf("Launch err = %v, want a refusal naming clear-state", err) } - if reinstalls != 1 { - t.Fatalf("clear-state with app path must reinstall exactly once; got %d", reinstalls) + if reinstalls != 0 { + t.Fatalf("reinstalls = %d, want 0: a live session must never have the app reinstalled under it", reinstalls) } - if indexOf(companion.calls, "launch") < indexOf(companion.calls, "terminate") { - t.Fatalf("launch must still follow terminate; got %v", companion.calls) + if indexOf(companion.recorded(), "launch") >= 0 { + t.Fatalf("a refused launch must not reach the companion; got %v", companion.recorded()) } } -func TestLaunchClearStateFallbackWarnsOnce(t *testing.T) { - companion := &fakeCompanion{accessibilityJSON: "[]"} - output := &bytes.Buffer{} - d := newTestDriver(companion) - d.output = output - resets := 0 - d.resetContainer = func(context.Context) error { resets++; return nil } +func TestNewRejectsClearStateWithoutBundleID(t *testing.T) { + probe := &clearStateProbe{} + options := clearStateOptions(t, probe, "NO-BUNDLE-UDID", true) + options.BundleID = "" - for i := 0; i < 2; i++ { - if err := d.Launch(context.Background(), "", true, nil); err != nil { - t.Fatalf("Launch %d: %v", i, err) - } + if _, err := New(context.Background(), options); err == nil || !strings.Contains(err.Error(), "BundleID") { + t.Fatalf("New err = %v, want a refusal naming BundleID", err) } - if resets != 2 { - t.Fatalf("resetContainer called %d times, want 2", resets) - } - warnings := strings.Count(output.String(), "resetting the data container only") - if warnings != 1 { - t.Fatalf("warning emitted %d times, want once", warnings) - } - for _, call := range companion.calls { - if call == "install" || call == "uninstall" { - t.Fatalf("fallback path must not install/uninstall; got %v", companion.calls) - } + if got := probe.recorded(); len(got) != 0 { + t.Fatalf("calls = %v, want none: clearing an unnamed bundle would reinstall without resetting anything", got) } } diff --git a/internal/testrun/driver.go b/internal/testrun/driver.go index a46a857..2bf3b5c 100644 --- a/internal/testrun/driver.go +++ b/internal/testrun/driver.go @@ -84,6 +84,17 @@ var newDeviceDriver = func(ctx context.Context, options ioscompanion.DeviceOptio return d, d.Close, nil } +// newSimulatorDriver constructs the iOS simulator driver and its cleanup. A +// seam so routing tests assert the run's options reach ioscompanion.Options +// without spawning a companion. +var newSimulatorDriver = func(ctx context.Context, options ioscompanion.Options) (driver.DeviceDriver, func(), error) { + d, err := ioscompanion.New(ctx, options) + if err != nil { + return nil, nil, err + } + return d, d.Close, nil +} + // buildDriver creates the appropriate DeviceDriver for the platform and returns // a cleanup function. For web, ChromeDriver is used directly. An iOS simulator // is driven by the native simulator companion (no JVM). A physical iOS device @@ -99,16 +110,17 @@ func buildDriver(ctx context.Context, options Options, stdout io.Writer) (driver } if options.Platform == "ios" && options.iosIsSimulator { - d, err := ioscompanion.New(ctx, ioscompanion.Options{ + d, cleanup, err := newSimulatorDriver(ctx, ioscompanion.Options{ UniqueDeviceIdentifier: options.iosUDID, BundleID: options.BundleID, AppPath: options.IosAppPath, + ClearState: options.ClearData, Output: stdout, }) if err != nil { return nil, nil, fmt.Errorf("ios simulator driver: %w", err) } - return d, d.Close, nil + return d, cleanup, nil } if options.Platform == "ios" { @@ -117,6 +129,7 @@ func buildDriver(ctx context.Context, options Options, stdout io.Writer) (driver CoreDeviceID: options.iosCoreDeviceID, BundleID: options.BundleID, AppPath: options.IosAppPath, + ClearState: options.ClearData, Output: stdout, }) if err != nil { diff --git a/internal/testrun/driver_test.go b/internal/testrun/driver_test.go index 5e461b3..653be9c 100644 --- a/internal/testrun/driver_test.go +++ b/internal/testrun/driver_test.go @@ -27,7 +27,7 @@ func TestBuildDriverRoutesPhysicalIOSToDeviceDriver(t *testing.T) { return stubDeviceDriver{}, func() { closed = true }, nil } - options := Options{Platform: "ios", BundleID: "app.folio", IosAppPath: "/tmp/iosApp.app"} + options := Options{Platform: "ios", BundleID: "app.folio", IosAppPath: "/tmp/iosApp.app", ClearData: true} options.iosIsSimulator = false options.iosUDID = "00008140-HW" options.iosCoreDeviceID = "CORE-1" @@ -45,12 +45,41 @@ func TestBuildDriverRoutesPhysicalIOSToDeviceDriver(t *testing.T) { if got.BundleID != "app.folio" || got.AppPath != "/tmp/iosApp.app" { t.Fatalf("DeviceOptions = %+v, want bundle and app path threaded through", got) } + if !got.ClearState { + t.Fatalf("DeviceOptions = %+v, want clear-data threaded through: the driver clears before its session, so a launch cannot", got) + } cleanup() if !closed { t.Fatal("cleanup must close the device driver") } } +func TestBuildDriverThreadsClearStateToTheSimulatorDriver(t *testing.T) { + stubPreflight(t) + original := newSimulatorDriver + t.Cleanup(func() { newSimulatorDriver = original }) + + var got ioscompanion.Options + newSimulatorDriver = func(_ context.Context, options ioscompanion.Options) (driver.DeviceDriver, func(), error) { + got = options + return stubDeviceDriver{}, func() {}, nil + } + + options := Options{Platform: "ios", BundleID: "app.folio", IosAppPath: "/tmp/iosApp.app", ClearData: true} + options.iosIsSimulator = true + options.iosUDID = "SIM-UDID" + + if _, _, err := buildDriver(context.Background(), options, io.Discard); err != nil { + t.Fatalf("buildDriver: %v", err) + } + if got.UniqueDeviceIdentifier != "SIM-UDID" || got.BundleID != "app.folio" || got.AppPath != "/tmp/iosApp.app" { + t.Fatalf("Options = %+v, want the resolved target, bundle and app path", got) + } + if !got.ClearState { + t.Fatalf("Options = %+v, want clear-data threaded through: the driver clears before its session, so a launch cannot", got) + } +} + func TestBuildDriverSurfacesDeviceConstructionError(t *testing.T) { stubPreflight(t) original := newDeviceDriver