diff --git a/internal/driver/ioscompanion/driver.go b/internal/driver/ioscompanion/driver.go index 49b33c5..74a2de3 100644 --- a/internal/driver/ioscompanion/driver.go +++ b/internal/driver/ioscompanion/driver.go @@ -741,17 +741,27 @@ func (d *Driver) Screenshot(ctx context.Context) (driver.Image, error) { func (d *Driver) Snapshot(ctx context.Context) (string, driver.Image, error) { d.mu.Lock() defer d.mu.Unlock() + + // The hierarchy and the screenshot ride different transports on the + // hybrid path, so they are captured concurrently. Only the hierarchy leg + // runs under withRecovery: two concurrent recoveries would race the + // restart bookkeeping, and a screenshot connection failure surfaces as a + // plain error that the next serialized call recovers from. + var data []byte + screenshotDone := make(chan error, 1) + go func() { + imageData, _, callErr := d.companion.Screenshot(ctx) + data = imageData + screenshotDone <- callErr + }() + dump, err := d.describeAll(ctx) + screenshotErr := <-screenshotDone if err != nil { return "", driver.Image{}, err } - var data []byte - if err := d.withRecovery(ctx, func() error { - var screenshotErr error - data, _, screenshotErr = d.companion.Screenshot(ctx) - return screenshotErr - }); err != nil { - return "", driver.Image{}, fmt.Errorf("screenshot: %w", err) + if screenshotErr != nil { + return "", driver.Image{}, fmt.Errorf("screenshot: %w", screenshotErr) } mapped, err := MapHierarchy(dump, d.screenWidth, d.screenHeight) if err != nil { diff --git a/internal/driver/ioscompanion/driver_test.go b/internal/driver/ioscompanion/driver_test.go index f163bf5..cced6f5 100644 --- a/internal/driver/ioscompanion/driver_test.go +++ b/internal/driver/ioscompanion/driver_test.go @@ -13,6 +13,7 @@ import ( "os/exec" "path/filepath" "strings" + "sync" "testing" "time" @@ -27,7 +28,10 @@ import ( // the order of calls and returns scripted results, so the driver's decision // logic is testable without a live simulator. type fakeCompanion struct { - calls []string + // callsMutex guards calls: Snapshot captures the hierarchy and the + // screenshot concurrently. + callsMutex sync.Mutex + calls []string accessibilityJSON string accessibilityErr error @@ -40,7 +44,19 @@ type fakeCompanion struct { hidErr error } -func (f *fakeCompanion) record(name string) { f.calls = append(f.calls, name) } +func (f *fakeCompanion) record(name string) { + f.callsMutex.Lock() + defer f.callsMutex.Unlock() + f.calls = append(f.calls, name) +} + +func (f *fakeCompanion) recorded() []string { + f.callsMutex.Lock() + defer f.callsMutex.Unlock() + out := make([]string, len(f.calls)) + copy(out, f.calls) + return out +} func (f *fakeCompanion) AccessibilityInfo(context.Context) (string, error) { f.record("accessibility") @@ -218,7 +234,7 @@ func TestLaunchRejectsEnvironment(t *testing.T) { } } -func TestSnapshotPairsHierarchyThenScreenshot(t *testing.T) { +func TestSnapshotPairsHierarchyAndScreenshot(t *testing.T) { companion := &fakeCompanion{ accessibilityJSON: "[]", screenshotData: samplePNG(t, 390, 844), @@ -231,8 +247,11 @@ func TestSnapshotPairsHierarchyThenScreenshot(t *testing.T) { if image.Width != 390 || image.Height != 844 { t.Fatalf("image dims = %dx%d, want 390x844", image.Width, image.Height) } - if indexOf(companion.calls, "accessibility") > indexOf(companion.calls, "screenshot") { - t.Fatalf("accessibility must precede screenshot; got %v", companion.calls) + // The two captures run concurrently (they ride different transports on + // the hybrid path), so both must happen but in no particular order. + calls := companion.recorded() + if indexOf(calls, "accessibility") < 0 || indexOf(calls, "screenshot") < 0 { + t.Fatalf("snapshot must capture hierarchy and screenshot; got %v", calls) } } diff --git a/internal/driver/ioscompanion/settle.go b/internal/driver/ioscompanion/settle.go index b40babb..b28287e 100644 --- a/internal/driver/ioscompanion/settle.go +++ b/internal/driver/ioscompanion/settle.go @@ -56,33 +56,30 @@ func SystemClock() Clock { return systemClock{} } // capped at StabilityPollCap. fetch returns the current hierarchy tree (nil on // fetch failure, treated like a transitional snapshot so the streak resets). // -// The loop mirrors the companion's JVM implementation: it samples a prior -// snapshot, then on each tick sleeps the poll interval, fetches a fresh -// snapshot, and grows the streak only while consecutive snapshots are both -// non-transitional and structurally identical. A transitional snapshot, a -// changed snapshot, or a fetch failure resets the streak. The function returns -// when the streak reaches MinStableStreak or the cap elapses, and respects -// context cancellation. +// The stable stretch is measured from the start of the earliest read in the +// current run of identical snapshots: a fetch is not instantaneous (a runner +// snapshot takes a fair fraction of the streak itself), and the UI changing +// mid-read would change the snapshot, so the read's own duration is evidence +// of stability. A transitional snapshot, a changed snapshot, or a fetch +// failure resets the run. The function returns when the stretch reaches +// MinStableStreak or the cap elapses, and respects context cancellation. func PollUntilStable(ctx context.Context, clock Clock, fetch func() *hierarchy.Tree) { deadline := clock.Now().Add(StabilityPollCap) + runStart := clock.Now() prior := snapshot(fetch) - var streakStart time.Time for clock.Now().Before(deadline) { if ctx.Err() != nil { return } clock.Sleep(StabilityPollInterval) + currentStart := clock.Now() current := snapshot(fetch) - now := clock.Now() if prior.valid && current.valid && prior.hash == current.hash { - if streakStart.IsZero() { - streakStart = now - } - if now.Sub(streakStart) >= MinStableStreak { + if clock.Now().Sub(runStart) >= MinStableStreak { return } } else { - streakStart = time.Time{} + runStart = currentStart } prior = current }