Merge branch 'ios-frontboard-retry' into trial-merge

internal/driver/ioscompanion/driver.go

@@ -53,6 +53,14 @@ var shutdownGrace = 15 * time.Second
 // A variable so the timeout test can shrink it.
 var launchTimeout = 90 * time.Second
 
+// launchRecoveryTimeout bounds the whole recovery a blown launch bound
+// triggers, the session restart and the second attempt together. It keeps the
+// launch path inside the three minutes testrun allows it, so what a user sees
+// when the app really cannot be launched stays the driver's error rather than
+// that backstop firing over the top of it. A variable so the bound test can
+// shrink it.
+var launchRecoveryTimeout = 60 * time.Second
+
 // longPressHoldMilliseconds is how long LongPress holds the finger down.
 const longPressHoldMilliseconds = 600
 
@@ -477,14 +485,55 @@ func (d *Driver) Launch(ctx context.Context, bundleID string, clearState bool, e
 		}
 	}
 
-	if err := d.lifecycleCall(ctx, func(callCtx context.Context, companion transport.Companion) error {
-		return companion.Launch(callCtx, d.bundleID, true)
-	}); err != nil {
+	if err := d.launchWithSessionRecovery(ctx); err != nil {
 		return fmt.Errorf("launch %s: %w", d.bundleID, err)
 	}
 	return nil
 }
 
+// launchWithSessionRecovery runs the launch RPC and, when it blows its own
+// bound, replaces the session and launches again.
+//
+// A launch the simulator refuses, which is what a clear-state reinstall racing
+// FrontBoard's registration produces, never comes back as an error: XCTest
+// records the refusal as a test failure the runner cannot observe, then holds
+// the session's main thread for about four minutes walking a diagnostic chain
+// (a 120s accessibility wait, a spindump, an idle wait). So there is no error
+// text to key a retry on, only the expired bound, and every later call queues
+// behind the same wedge. Only a session that never served the refused launch
+// can serve the retry, which is why this restarts rather than calls again.
+func (d *Driver) launchWithSessionRecovery(ctx context.Context) error {
+	launch := func(callCtx context.Context, companion transport.Companion) error {
+		return companion.Launch(callCtx, d.bundleID, true)
+	}
+	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 {
+		return err
+	}
+	fmt.Fprintf(d.output, "launch %s blew its %v bound (%v); restarting the session and launching once more\n",
+		d.bundleID, launchTimeout, err)
+
+	// The restart runs under the driver's own lifetime context for the same
+	// reason withRecovery's does, while the second attempt stays on the
+	// caller's. Both end at one deadline, so a launch that already spent
+	// launchTimeout cannot then wait out a session cold start on top of it.
+	recoveryDeadline := time.Now().Add(launchRecoveryTimeout)
+	restartCtx := d.processContext
+	if restartCtx == nil {
+		restartCtx = ctx
+	}
+	restartCtx, cancelRestart := context.WithDeadline(restartCtx, recoveryDeadline)
+	defer cancelRestart()
+	if restartErr := d.restart(restartCtx); restartErr != nil {
+		return fmt.Errorf("session restart failed: %w (original: %v)", restartErr, err)
+	}
+	relaunchCtx, cancelRelaunch := context.WithDeadline(ctx, recoveryDeadline)
+	defer cancelRelaunch()
+	return d.lifecycleCall(relaunchCtx, launch)
+}
+
 // lifecycleCall runs an app lifecycle RPC against lifecycleCompanion under a
 // launchTimeout-bounded context, with the usual one-restart recovery. The
 // companion is resolved inside the retry so a restart's replacement client

internal/driver/ioscompanion/driver_test.go

@@ -951,6 +951,131 @@ func TestLaunchLeavesATighterCallerDeadlineAlone(t *testing.T) {
 	}
 }
 
+// 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.
+type wedgedUntilRestartCompanion struct {
+	fakeCompanion
+	mutex     sync.Mutex
+	replaced  bool
+	attempted int
+}
+
+func (w *wedgedUntilRestartCompanion) replaceSession() {
+	w.mutex.Lock()
+	defer w.mutex.Unlock()
+	w.replaced = true
+}
+
+func (w *wedgedUntilRestartCompanion) launchAttempts() int {
+	w.mutex.Lock()
+	defer w.mutex.Unlock()
+	return w.attempted
+}
+
+func (w *wedgedUntilRestartCompanion) Launch(ctx context.Context, _ string, _ bool) error {
+	w.mutex.Lock()
+	w.attempted++
+	replaced := w.replaced
+	w.mutex.Unlock()
+	if replaced {
+		return nil
+	}
+	<-ctx.Done()
+	return ctx.Err()
+}
+
+// TestLaunchReplacesTheSessionAfterALaunchBlowsItsBound covers the FrontBoard
+// race: a clear-state reinstall the simulator has not finished registering
+// 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.
+func TestLaunchReplacesTheSessionAfterALaunchBlowsItsBound(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
+	}
+
+	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())
+	}
+}
+
+// TestLaunchBoundsTheSessionRestartItTriggers keeps the recovery inside a
+// budget of its own. The restart deliberately runs on the driver's lifetime
+// context rather than the caller's, so without a deadline a session that never
+// comes back would hang the launch path exactly the way #73 stopped it hanging.
+func TestLaunchBoundsTheSessionRestartItTriggers(t *testing.T) {
+	previousLaunch, previousRecovery := launchTimeout, launchRecoveryTimeout
+	launchTimeout = 100 * time.Millisecond
+	launchRecoveryTimeout = 200 * time.Millisecond
+	defer func() { launchTimeout, launchRecoveryTimeout = previousLaunch, previousRecovery }()
+
+	d := newTestDriver(&wedgedUntilRestartCompanion{})
+	d.restart = func(restartCtx context.Context) error {
+		<-restartCtx.Done()
+		return restartCtx.Err()
+	}
+
+	done := make(chan error, 1)
+	go func() { done <- d.Launch(context.Background(), "", false, nil) }()
+	select {
+	case err := <-done:
+		if err == nil || !strings.Contains(err.Error(), "session restart failed") {
+			t.Fatalf("err = %v, want the failed restart named", err)
+		}
+	case <-time.After(10 * time.Second):
+		t.Fatal("Launch never returned: a session that never comes back hangs the launch path")
+	}
+}
+
+// TestLaunchKeepsTheSessionWhenTheCallersOwnDeadlineExpires holds the recovery
+// to the driver's own bound. Spending a session restart on a caller that has
+// already run out of budget cannot produce a launch, only a later failure.
+func TestLaunchKeepsTheSessionWhenTheCallersOwnDeadlineExpires(t *testing.T) {
+	previous := launchTimeout
+	launchTimeout = 30 * time.Second
+	defer func() { launchTimeout = previous }()
+
+	companion := &wedgedUntilRestartCompanion{}
+	d := newTestDriver(companion)
+	restarts := 0
+	d.restart = func(context.Context) error {
+		restarts++
+		companion.replaceSession()
+		return nil
+	}
+
+	ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
+	defer cancel()
+	if err := d.Launch(ctx, "", false, nil); !errors.Is(err, context.DeadlineExceeded) {
+		t.Fatalf("err = %v, want a deadline-exceeded error", err)
+	}
+	if restarts != 0 {
+		t.Fatalf("session restarts = %d, want 0", restarts)
+	}
+}
+
 // newLockTestOptions builds New options that dial a seamed companion, so the
 // device-lock tests exercise New without spawning anything.
 func newLockTestOptions(t *testing.T, udid string) Options {