Merge branch 'ios-clear-before-session' into trial-merge

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.
+	// 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 = 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

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}

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.
+	// 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
 
@@ -142,6 +156,7 @@ type Driver struct {
 	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
@@ -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
+	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 {

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 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 _, err := New(context.Background(), options); err == nil || !strings.Contains(err.Error(), "BundleID") {
+		t.Fatalf("New err = %v, want a refusal naming BundleID", err)
 	}
+	if got := probe.recorded(); len(got) != 0 {
+		t.Fatalf("calls = %v, want none: clearing an unnamed bundle would reinstall without resetting anything", got)
 	}
 }
 

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 {

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