diff --git a/internal/driver/ioscompanion/device.go b/internal/driver/ioscompanion/device.go index 1193636..b8dbdff 100644 --- a/internal/driver/ioscompanion/device.go +++ b/internal/driver/ioscompanion/device.go @@ -83,7 +83,6 @@ 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, @@ -104,20 +103,24 @@ func NewDevice(ctx context.Context, options DeviceOptions) (*Driver, error) { } } if options.pickAddress != nil { - d.pickDeviceAddress = options.pickAddress + d.pickRunnerAddress = options.pickAddress } else { - d.pickDeviceAddress = pickLoopbackAddress + d.pickRunnerAddress = pickLoopbackAddress } // 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. + // natively, so no paste prompt is ever hit. Stopping the app before the + // clear is a no-op too: devicectl addresses processes by pid rather than by + // bundle, and the uninstall that is the device's only clear takes the + // running app with it, which is what a terminate here would be for. 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.terminateApp = func(context.Context) error { return nil } d.restart = d.respawnDevice d.processContext, d.processCancel = context.WithCancel(ctx) @@ -133,6 +136,7 @@ func NewDevice(ctx context.Context, options DeviceOptions) (*Driver, error) { d.Close() return nil, err } + d.clearedBundleID = options.BundleID } if err := d.bringUpDevice(ctx); err != nil { @@ -155,7 +159,7 @@ func NewDevice(ctx context.Context, options DeviceOptions) (*Driver, error) { // health. The build runs inside spawnRunner under the process context, so the // startup timeout only bounds the post-spawn wait, not the build. func (d *Driver) bringUpDevice(ctx context.Context) error { - address, err := d.pickDeviceAddress() + address, err := d.pickRunnerAddress() if err != nil { return err } @@ -239,13 +243,11 @@ func (d *Driver) devicectlReinstall(ctx context.Context) error { return nil } -// deviceResetContainerUnsupported warns once that device clear-state needs an -// app path for a devicectl reinstall: there is no simulator-style data-container -// wipe on a physical device. +// deviceResetContainerUnsupported ends the run: there is no simulator-style +// data-container wipe on a physical device, so a clear-state with no app path +// to reinstall from cannot happen. Warning and carrying on hands the run every +// previous run's data while the flag says it started clean. func (d *Driver) deviceResetContainerUnsupported(context.Context) error { - if !d.clearStateWarned { - fmt.Fprintln(d.output, "clear-state on a physical device requires --ios-app-path for a reinstall; skipping (state not cleared)") - d.clearStateWarned = true - } - return nil + return errors.New("clear-state on a physical device requires --ios-app-path for a reinstall; " + + "there is no data-container wipe on a device, so the run would start on the previous run's state") } diff --git a/internal/driver/ioscompanion/device_test.go b/internal/driver/ioscompanion/device_test.go index abc5ced..15bf23a 100644 --- a/internal/driver/ioscompanion/device_test.go +++ b/internal/driver/ioscompanion/device_test.go @@ -83,6 +83,25 @@ func TestNewDeviceWiresRunnerOnlyMode(t *testing.T) { } } +// TestNewDeviceWiresTheAddressPickerEveryBringUpUses keeps the device driver +// whole. bringUpRunner reads its picker from a field rather than calling the +// package function, and NewDevice left that field nil, so the only thing +// standing between a device run and a nil call was which restart path ran. +func TestNewDeviceWiresTheAddressPickerEveryBringUpUses(t *testing.T) { + address := startLoopbackListener(t) + options := testDeviceOptions(address, newDeviceCompanion()) + options.HardwareUDID = "00008140-PICKER" + d, err := NewDevice(context.Background(), options) + if err != nil { + t.Fatalf("NewDevice: %v", err) + } + defer d.Close() + + if err := d.bringUpRunner(context.Background()); err != nil { + t.Fatalf("bringUpRunner: %v", err) + } +} + func TestNewDeviceRequiresIdentifiers(t *testing.T) { if _, err := NewDevice(context.Background(), DeviceOptions{CoreDeviceID: "x"}); err == nil { t.Fatal("missing HardwareUDID must error") @@ -222,17 +241,32 @@ func TestDevicectlReinstallProceedsWhenNothingIsInstalled(t *testing.T) { } } -func TestDeviceClearStateWithoutAppPathWarnsOnce(t *testing.T) { - output := &bytes.Buffer{} - d := &Driver{output: output, deviceMode: true} - d.resetContainer = d.deviceResetContainerUnsupported - for i := 0; i < 2; i++ { - if err := d.deviceResetContainerUnsupported(context.Background()); err != nil { - t.Fatal(err) - } +// TestNewDeviceRefusesClearStateWithoutAnAppPath keeps the device from starting +// a run whose clear-state cannot happen. There is no data-container wipe on a +// physical device, so without an app path to reinstall from, carrying on hands +// the run every previous run's data under a flag that says otherwise. +func TestNewDeviceRefusesClearStateWithoutAnAppPath(t *testing.T) { + address := startLoopbackListener(t) + options := testDeviceOptions(address, newDeviceCompanion()) + options.HardwareUDID = "00008140-NO-APP-PATH" + options.ClearState = true + spawned := false + spawn := options.spawnRunner + options.spawnRunner = func(ctx context.Context, runnerAddress string) (*exec.Cmd, error) { + spawned = true + return spawn(ctx, runnerAddress) } - if got := bytes.Count(output.Bytes(), []byte("requires --ios-app-path")); got != 1 { - t.Fatalf("warning emitted %d times, want once", got) + + d, err := NewDevice(context.Background(), options) + if err == nil { + d.Close() + t.Fatal("NewDevice returned a driver whose clear-state never happened") + } + if !strings.Contains(err.Error(), "--ios-app-path") { + t.Fatalf("err = %v, want it to name the flag that makes the clear possible", err) + } + if spawned { + t.Fatal("the run started anyway; a clear-state that cannot happen must end the run, not open it") } } diff --git a/internal/driver/ioscompanion/driver.go b/internal/driver/ioscompanion/driver.go index 125a421..aa12589 100644 --- a/internal/driver/ioscompanion/driver.go +++ b/internal/driver/ioscompanion/driver.go @@ -91,6 +91,7 @@ type Options struct { dialRunner func(address string) (transport.Companion, error) reinstallApp func(ctx context.Context) error resetContainer func(ctx context.Context) error + terminateApp func(ctx context.Context) error } // Driver implements driver.DeviceDriver against an iOS simulator companion. @@ -101,11 +102,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 + // clearedBundleID names the app New (or NewDevice) reset to first-launch + // state before attaching, which is the only point in a run where clearing + // is safe. Launch refuses a clear-state request for anything else rather + // than reinstalling under a live session or reporting a reset that only + // ever reached another bundle. + clearedBundleID string screenWidth int screenHeight int @@ -136,6 +138,10 @@ type Driver struct { // A seam so tests skip the simctl shell-outs. reinstallApp func(ctx context.Context) error + // terminateApp stops the app before its state is cleared. A seam so tests + // skip the simctl shell-out. + terminateApp func(ctx context.Context) error + // grantPaste pre-authorizes the app's pasteboard access. A seam so tests // skip the sqlite shell-out. grantPaste func(ctx context.Context) error @@ -151,23 +157,26 @@ 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) + 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 + + // pickRunnerAddress hands every bring-up a free loopback port, on the + // simulator and the device alike. One field, so no path can be wired + // without it. 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. // coreDeviceID feeds devicectl; tunnel is the in-process usbmux forwarder // bridging the host loopback port to the runner's device-side port. - deviceMode bool - coreDeviceID string - tunnel io.Closer - startTunnel func(ctx context.Context, hardwareUDID, localAddress, devicePort string) (io.Closer, error) - pickDeviceAddress func() (string, error) + deviceMode bool + coreDeviceID string + tunnel io.Closer + startTunnel func(ctx context.Context, hardwareUDID, localAddress, devicePort string) (io.Closer, error) // processContext owns the companion child's lifetime: it is derived from // New's context (so a canceled run still reaps the child) and canceled by @@ -228,7 +237,6 @@ 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, @@ -237,6 +245,7 @@ func New(ctx context.Context, options Options) (*Driver, error) { dialRunner: options.dialRunner, reinstallApp: options.reinstallApp, resetContainer: options.resetContainer, + terminateApp: options.terminateApp, hybrid: hybridCompanionEnabled(), } if driverInstance.spawnChild == nil { @@ -271,6 +280,9 @@ func New(ctx context.Context, options Options) (*Driver, error) { if driverInstance.reinstallApp == nil { driverInstance.reinstallApp = driverInstance.simctlReinstall } + if driverInstance.terminateApp == nil { + driverInstance.terminateApp = driverInstance.simctlTerminate + } driverInstance.grantPaste = driverInstance.grantPasteboardAccess driverInstance.processContext, driverInstance.processCancel = context.WithCancel(ctx) @@ -286,6 +298,7 @@ func New(ctx context.Context, options Options) (*Driver, error) { driverInstance.Close() return nil, err } + driverInstance.clearedBundleID = options.BundleID } if err := driverInstance.bringUp(ctx); err != nil { @@ -484,6 +497,26 @@ func isConnectionError(err error) bool { return false } +// isBudgetExpiry reports whether err is a call that outlived its bound rather +// than a failure the transport can name. Each transport says so differently: +// the runner wraps the context's error when cancellation has landed and the +// connection's i/o timeout when the deadline it armed from that context fires +// first, and the legacy companion returns a gRPC status. Only an expiry earns +// a session restart; an error the runner reports has already said what a fresh +// session would say. +func isBudgetExpiry(err error) bool { + if err == nil { + return false + } + if errors.Is(err, context.DeadlineExceeded) || errors.Is(err, os.ErrDeadlineExceeded) { + return true + } + if statusValue, ok := status.FromError(err); ok { + return statusValue.Code() == codes.DeadlineExceeded + } + return false +} + func (d *Driver) Launch(ctx context.Context, bundleID string, clearState bool, env map[string]string) error { if bundleID != "" { d.bundleID = bundleID @@ -495,12 +528,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 { + if clearState && (d.clearedBundleID == "" || d.clearedBundleID != d.bundleID) { // 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") + return fmt.Errorf("ios companion: clear-state must be requested when the driver is created (Options.ClearState) "+ + "for the bundle being launched; this backend cleared %q before its automation session existed, not %q", + d.clearedBundleID, d.bundleID) } // Terminate first so the launch is a clean cold start regardless of the @@ -545,7 +579,7 @@ func (d *Driver) launchWithSessionRecovery(ctx context.Context) error { err := d.lifecycleCall(ctx, launch) // A caller whose own budget ran out gets no restart: the bound that expired // was the caller's to spend, and the second attempt would inherit it dead. - if err == nil || !errors.Is(err, context.DeadlineExceeded) || ctx.Err() != nil || d.restart == nil { + if !isBudgetExpiry(err) || ctx.Err() != nil || d.restart == nil { return err } fmt.Fprintf(d.output, "launch %s blew its %v bound (%v); restarting the session and launching once more\n", @@ -599,6 +633,11 @@ func (d *Driver) lifecycleCompanion() transport.Companion { // 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 { + // Nothing may be writing to the state while it goes, which is the ordering + // Launch used to hold: uninstall copes with a running app, deleting the + // data container out from under one does not. Best effort, because an app + // that is not running reports a failure that means nothing here. + _ = d.terminateApp(ctx) if d.appPath != "" { if err := d.reinstallApp(ctx); err != nil { return fmt.Errorf("reinstall %s: %w", d.appPath, err) @@ -630,6 +669,18 @@ func (d *Driver) simctlReinstall(ctx context.Context) error { return nil } +// simctlTerminate stops the app under test. Launch used to terminate through +// the automation session before clearing; the clear now runs before any session +// exists, so simctl is what is left to stop the app with. An app that is not +// running reports a failure that means nothing to the caller, which is why +// clearAppState treats this as best effort. +func (d *Driver) simctlTerminate(ctx context.Context) error { + if output, err := exec.CommandContext(ctx, "xcrun", "simctl", "terminate", d.udid, d.bundleID).CombinedOutput(); err != nil { + return fmt.Errorf("simctl terminate %s: %w: %s", d.bundleID, err, strings.TrimSpace(string(output))) + } + return nil +} + // grantPasteboardAccess authorizes the app to read the pasteboard without the // iOS permission prompt, by writing an allow row into the simulator's privacy // (TCC) database. This is the simulator counterpart to `simctl privacy grant`, diff --git a/internal/driver/ioscompanion/driver_test.go b/internal/driver/ioscompanion/driver_test.go index f8ae5b9..21e85a8 100644 --- a/internal/driver/ioscompanion/driver_test.go +++ b/internal/driver/ioscompanion/driver_test.go @@ -18,10 +18,12 @@ import ( "testing" "time" + "google.golang.org/grpc" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" "github.com/priyanshujain/sanderling/internal/driver" + "github.com/priyanshujain/sanderling/internal/driver/ioscompanion/companionpb" "github.com/priyanshujain/sanderling/internal/driver/ioscompanion/transport" ) @@ -230,6 +232,7 @@ func clearStateOptions(t *testing.T, probe *clearStateProbe, udid string, clearS }, reinstallApp: func(context.Context) error { probe.record("reinstall"); return nil }, resetContainer: func(context.Context) error { probe.record("reset container"); return nil }, + terminateApp: func(context.Context) error { probe.record("stop app"); return nil }, } } @@ -247,9 +250,9 @@ func TestClearStateReinstallsOnceBeforeTheRunnerSession(t *testing.T) { t.Fatalf("Launch: %v", err) } - want := []string{"reinstall", "runner session"} + want := []string{"stop app", "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) + t.Fatalf("calls = %v, want %v: the reinstall must run once, on a stopped app, before the automation session attaches", got, want) } } @@ -268,9 +271,9 @@ func TestClearStateWithoutAppPathWipesContainerBeforeTheRunnerSession(t *testing t.Fatalf("Launch: %v", err) } - want := []string{"reset container", "runner session"} + want := []string{"stop app", "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) + t.Fatalf("calls = %v, want %v: the fallback must wipe a stopped app's 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) @@ -316,6 +319,25 @@ func TestLaunchRefusesClearStateTheDriverWasNotBuiltFor(t *testing.T) { } } +// TestLaunchRefusesClearStateForABundleItDidNotClear holds the guard to the +// fact it is guarding. A driver built to clear one bundle has cleared nothing +// for another, so reporting that launch as a clear-state launch is a reset the +// caller was told happened and did not. +func TestLaunchRefusesClearStateForABundleItDidNotClear(t *testing.T) { + companion := &fakeCompanion{accessibilityJSON: "[]"} + d := newTestDriver(companion) + d.clearedBundleID = "com.example.app" + + err := d.Launch(context.Background(), "com.other.app", true, nil) + + if err == nil || !strings.Contains(err.Error(), "clear-state") { + t.Fatalf("Launch err = %v, want a refusal naming clear-state: com.other.app was never cleared", err) + } + if indexOf(companion.recorded(), "launch") >= 0 { + t.Fatalf("a launch reporting a clear that never happened must not reach the companion; got %v", companion.recorded()) + } +} + func TestNewRejectsClearStateWithoutBundleID(t *testing.T) { probe := &clearStateProbe{} options := clearStateOptions(t, probe, "NO-BUNDLE-UDID", true) @@ -391,6 +413,55 @@ func TestSimctlReinstallProceedsWhenNothingIsInstalled(t *testing.T) { } } +// TestClearStateStopsTheAppBeforeWipingItsContainer covers the ordering Launch +// used to hold. simctl uninstall copes with a running app; deleting the data +// container out from under one does not, and the CI iOS leg passes no app path +// so it is the wipe that runs. A run whose previous run was interrupted finds +// the app still up. +func TestClearStateStopsTheAppBeforeWipingItsContainer(t *testing.T) { + container := t.TempDir() + stale := filepath.Join(container, "Documents") + if err := os.Mkdir(stale, 0o755); err != nil { + t.Fatal(err) + } + log := scriptedXcrun(t, `"simctl terminate "*) :;; +"simctl get_app_container "*) echo `+container+`;;`) + d := &Driver{udid: "SIM-UDID", bundleID: "app.example", output: &bytes.Buffer{}} + d.terminateApp = d.simctlTerminate + d.resetContainer = d.resetDataContainer + + if err := d.clearAppState(context.Background()); err != nil { + t.Fatalf("clearAppState: %v", err) + } + + want := []string{ + "simctl terminate SIM-UDID app.example", + "simctl get_app_container SIM-UDID app.example data", + } + if got := xcrunCalls(t, log); !slices.Equal(got, want) { + t.Fatalf("xcrun calls = %v, want %v: the app was still writing to the container being deleted", got, want) + } + if _, err := os.Stat(stale); !os.IsNotExist(err) { + t.Fatalf("stat %s = %v, want the previous run's state gone", stale, err) + } +} + +// TestClearStateSurvivesAnAppThatIsNotRunning holds the terminate to best +// effort. simctl exits non-zero when there is nothing to stop, and a first run +// on a fresh simulator must not fail on it. +func TestClearStateSurvivesAnAppThatIsNotRunning(t *testing.T) { + container := t.TempDir() + scriptedXcrun(t, `"simctl terminate "*) echo "No matching processes belonging to bundle identifier app.example"; exit 3;; +"simctl get_app_container "*) echo `+container+`;;`) + d := &Driver{udid: "SIM-UDID", bundleID: "app.example", output: &bytes.Buffer{}} + d.terminateApp = d.simctlTerminate + d.resetContainer = d.resetDataContainer + + if err := d.clearAppState(context.Background()); err != nil { + t.Fatalf("clearAppState: %v: an app that is not running is not a failure to clear", err) + } +} + func TestLaunchRejectsEnvironment(t *testing.T) { d := newTestDriver(&fakeCompanion{accessibilityJSON: "[]"}) err := d.Launch(context.Background(), "", false, map[string]string{"K": "V"}) @@ -1116,14 +1187,134 @@ func TestLaunchLeavesATighterCallerDeadlineAlone(t *testing.T) { } } +// blownBudgetShape is one way a transport the driver launches through reports +// a lifecycle call outliving its budget. +type blownBudgetShape struct { + name string + err error +} + +// blownBudgetShapes drives every such transport against a server that never +// answers and returns the error each one really produces. The runner transport +// has two: wrapTransport wraps the context's own error once cancellation has +// landed, and the connection's i/o timeout when the deadline it armed from +// that context fires first. The legacy transport reports the same expiry as a +// gRPC status. Only the first satisfies errors.Is(err, context.DeadlineExceeded), +// so a fake that returns ctx.Err() raw shows the driver a recovery that two +// thirds of production can never reach. +func blownBudgetShapes(t *testing.T) []blownBudgetShape { + t.Helper() + return []blownBudgetShape{ + {"runner context deadline", runnerContextDeadlineError(t)}, + {"runner connection deadline", silentRunnerLaunchError(t, connectionDeadlineOnly{time.Now().Add(100 * time.Millisecond)})}, + {"legacy grpc deadline", silentGRPCLaunchError(t)}, + } +} + +// runnerContextDeadlineError is the shape the runner transport produces once +// the context's own cancellation has landed. +func runnerContextDeadlineError(t *testing.T) error { + t.Helper() + ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(-time.Second)) + defer cancel() + return silentRunnerLaunchError(t, ctx) +} + +// connectionDeadlineOnly carries a deadline the runner transport arms the +// connection with, while its own cancellation never lands. That is the race +// wrapTransport's second branch exists for: the connection's deadline fires +// while ctx.Err() is still nil. +type connectionDeadlineOnly struct{ deadline time.Time } + +func (c connectionDeadlineOnly) Deadline() (time.Time, bool) { return c.deadline, true } +func (c connectionDeadlineOnly) Done() <-chan struct{} { return nil } +func (c connectionDeadlineOnly) Err() error { return nil } +func (c connectionDeadlineOnly) Value(any) any { return nil } + +// silentRunnerLaunchError returns what the real runner transport produces for a +// launch nobody ever answers. +func silentRunnerLaunchError(t *testing.T, ctx context.Context) error { + t.Helper() + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + closed := make(chan struct{}) + go func() { + conn, acceptErr := listener.Accept() + if acceptErr != nil { + return + } + defer conn.Close() + <-closed + }() + companion, err := transport.DialRunner(listener.Addr().String(), "SIM-UDID", "com.example.app") + if err != nil { + listener.Close() + t.Fatal(err) + } + t.Cleanup(func() { + close(closed) + _ = companion.Close() + listener.Close() + }) + + launchErr := companion.Launch(ctx, "com.example.app", true) + if launchErr == nil { + t.Fatal("the runner transport reported a launch no server ever answered") + } + return launchErr +} + +// silentCompanionServer is the legacy companion with a launch that never +// answers, so the caller's own deadline is what ends the call. +type silentCompanionServer struct { + companionpb.UnimplementedCompanionServiceServer +} + +func (silentCompanionServer) Launch(stream grpc.BidiStreamingServer[companionpb.LaunchRequest, companionpb.LaunchResponse]) error { + <-stream.Context().Done() + return stream.Context().Err() +} + +// silentGRPCLaunchError returns what the legacy transport produces for the same +// launch, which SANDERLING_SIMULATOR_COMPANION=legacy still runs on. +func silentGRPCLaunchError(t *testing.T) error { + t.Helper() + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + server := grpc.NewServer() + companionpb.RegisterCompanionServiceServer(server, silentCompanionServer{}) + go server.Serve(listener) + t.Cleanup(server.Stop) + + companion, err := transport.Dial(listener.Addr().String()) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { companion.Close() }) + + ctx, cancel := context.WithTimeout(context.Background(), 250*time.Millisecond) + defer cancel() + launchErr := companion.Launch(ctx, "com.example.app", true) + if launchErr == nil { + t.Fatal("the legacy transport reported a launch the companion never answered") + } + return launchErr +} + // wedgedUntilRestartCompanion models the session a refused launch leaves -// behind: the refusal is never reported, and no later launch is answered until -// the session itself is replaced. +// behind: the refusal is never reported, no later launch is answered until the +// session itself is replaced, and the expired bound reaches the driver in +// whatever shape its transport gives it. type wedgedUntilRestartCompanion struct { fakeCompanion - mutex sync.Mutex - replaced bool - attempted int + blownBudget error + mutex sync.Mutex + replaced bool + attempted int } func (w *wedgedUntilRestartCompanion) replaceSession() { @@ -1147,7 +1338,7 @@ func (w *wedgedUntilRestartCompanion) Launch(ctx context.Context, _ string, _ bo return nil } <-ctx.Done() - return ctx.Err() + return w.blownBudget } // TestLaunchReplacesTheSessionAfterALaunchBlowsItsBound covers the FrontBoard @@ -1155,34 +1346,40 @@ func (w *wedgedUntilRestartCompanion) Launch(ctx context.Context, _ string, _ bo // makes the session refuse the launch, and XCTest answers that refusal with // minutes of diagnostics instead of an error, so the bound expires and every // later call queues behind the same wedge. Calling launch again on that session -// cannot work; the run only recovers if the session is replaced first. +// cannot work; the run only recovers if the session is replaced first. The +// recovery has to fire on every shape the driver's transports report that +// expiry in, because which one arrives is a race the driver does not control. func TestLaunchReplacesTheSessionAfterALaunchBlowsItsBound(t *testing.T) { - previous := launchTimeout - launchTimeout = 100 * time.Millisecond - defer func() { launchTimeout = previous }() + for _, shape := range blownBudgetShapes(t) { + t.Run(shape.name, func(t *testing.T) { + previous := launchTimeout + launchTimeout = 100 * time.Millisecond + defer func() { launchTimeout = previous }() - companion := &wedgedUntilRestartCompanion{} - output := &bytes.Buffer{} - d := newTestDriver(companion) - d.output = output - restarts := 0 - d.restart = func(context.Context) error { - restarts++ - companion.replaceSession() - return nil - } + companion := &wedgedUntilRestartCompanion{blownBudget: shape.err} + output := &bytes.Buffer{} + d := newTestDriver(companion) + d.output = output + restarts := 0 + d.restart = func(context.Context) error { + restarts++ + companion.replaceSession() + return nil + } - if err := d.Launch(context.Background(), "", false, nil); err != nil { - t.Fatalf("Launch: %v", err) - } - if restarts != 1 { - t.Fatalf("session restarts = %d, want exactly 1", restarts) - } - if attempts := companion.launchAttempts(); attempts != 2 { - t.Fatalf("launch attempts = %d, want 2: one that wedged and one on the replaced session", attempts) - } - if !strings.Contains(output.String(), "restarting the session") { - t.Fatalf("the recovery was silent, so a run that needed it never says so; output was %q", output.String()) + if err := d.Launch(context.Background(), "", false, nil); err != nil { + t.Fatalf("Launch: %v", err) + } + if restarts != 1 { + t.Fatalf("session restarts = %d, want exactly 1 (the session reported %v)", restarts, shape.err) + } + if attempts := companion.launchAttempts(); attempts != 2 { + t.Fatalf("launch attempts = %d, want 2: one that wedged and one on the replaced session", attempts) + } + if !strings.Contains(output.String(), "restarting the session") { + t.Fatalf("the recovery was silent, so a run that needed it never says so; output was %q", output.String()) + } + }) } } @@ -1196,7 +1393,7 @@ func TestLaunchBoundsTheSessionRestartItTriggers(t *testing.T) { launchRecoveryTimeout = 200 * time.Millisecond defer func() { launchTimeout, launchRecoveryTimeout = previousLaunch, previousRecovery }() - d := newTestDriver(&wedgedUntilRestartCompanion{}) + d := newTestDriver(&wedgedUntilRestartCompanion{blownBudget: runnerContextDeadlineError(t)}) d.restart = func(restartCtx context.Context) error { <-restartCtx.Done() return restartCtx.Err() @@ -1222,7 +1419,7 @@ func TestLaunchKeepsTheSessionWhenTheCallersOwnDeadlineExpires(t *testing.T) { launchTimeout = 30 * time.Second defer func() { launchTimeout = previous }() - companion := &wedgedUntilRestartCompanion{} + companion := &wedgedUntilRestartCompanion{blownBudget: runnerContextDeadlineError(t)} d := newTestDriver(companion) restarts := 0 d.restart = func(context.Context) error {