diff --git a/examples/folio/justfile b/examples/folio/justfile index 856a3bd..2bc82f7 100644 --- a/examples/folio/justfile +++ b/examples/folio/justfile @@ -123,7 +123,9 @@ ios: # Run 'sanderling test' against the folio app. Uses a connected device if one is # online; otherwise boots AVD= when provided, or auto-boots a bootable AVD. -test: _ensure-device +# Depends on install so the run always fuzzes the current build, matching +# test-ios which rebuilds and reinstalls the app first. +test: install #!/usr/bin/env bash set -euo pipefail avd_flag=() diff --git a/internal/driver/chrome/driver.go b/internal/driver/chrome/driver.go index df8aad1..829a8a2 100644 --- a/internal/driver/chrome/driver.go +++ b/internal/driver/chrome/driver.go @@ -193,6 +193,12 @@ func (d *Driver) InputText(_ context.Context, text string) error { ) } +// ReplacesTextOnInput reports that InputText replaces existing content via +// select-all, so the runner skips its pre-erase. +func (d *Driver) ReplacesTextOnInput() bool { + return true +} + // EraseText clears the focused field. InputText above already replaces via // select-all, so the character count is not needed to bound the deletion. func (d *Driver) EraseText(_ context.Context, _ int) error { diff --git a/internal/driver/driver.go b/internal/driver/driver.go index e4757d2..649d673 100644 --- a/internal/driver/driver.go +++ b/internal/driver/driver.go @@ -60,6 +60,16 @@ type ForegroundChecker interface { ForegroundApp(ctx context.Context) (string, error) } +// TextReplacer is the optional capability for drivers whose InputText already +// replaces the field's content instead of appending to it. The runner must +// skip its pre-erase for such drivers: the erase would be a redundant +// round-trip on every InputText. +type TextReplacer interface { + // ReplacesTextOnInput reports whether InputText replaces existing + // content, making the runner's pre-erase unnecessary. + ReplacesTextOnInput() bool +} + // FocusedWindowChecker is the optional capability for reporting which app owns // the focused (on-screen) window. The startup gate prefers it over // ForegroundChecker: the resumed-activity signal flips to a freshly launched diff --git a/internal/driver/mock/mock.go b/internal/driver/mock/mock.go index f60c323..b0e8f27 100644 --- a/internal/driver/mock/mock.go +++ b/internal/driver/mock/mock.go @@ -63,6 +63,10 @@ type Driver struct { MetricsData driver.Metrics Failures map[ActionKind]error + // ReplacesText makes the mock assert the TextReplacer capability, so + // tests cover both the erase-before-type and replace-on-input paths. + ReplacesText bool + // ForegroundResults is consumed one entry per ForegroundApp call (the // last entry repeats). Empty yields "", which disables the runner's // app-scope guard so tests that don't care are unaffected. @@ -212,6 +216,12 @@ func (d *Driver) InputText(ctx context.Context, text string) error { return nil } +func (d *Driver) ReplacesTextOnInput() bool { + d.mutex.Lock() + defer d.mutex.Unlock() + return d.ReplacesText +} + func (d *Driver) EraseText(ctx context.Context, characterCount int) error { if err := d.failure(ActionEraseText); err != nil { return err diff --git a/internal/hierarchy/hierarchy.go b/internal/hierarchy/hierarchy.go index 9135649..9972cb4 100644 --- a/internal/hierarchy/hierarchy.go +++ b/internal/hierarchy/hierarchy.go @@ -28,6 +28,7 @@ import ( "fmt" "maps" "regexp" + "sort" "strconv" "strings" ) @@ -426,9 +427,22 @@ func (n *Node) scopedNodes(accept func(*Element) bool) []*Node { result = append(result, candidate) } } + // Spatial containment alone lets a large container outrank the intended + // small element; the most specific (smallest) match wins instead. + sortBySpecificity(result) return result } +// sortBySpecificity orders nodes ascending by bounds area, so the smallest +// (most specific) containing match comes first. Equal-area nodes keep their +// pre-order position. +func sortBySpecificity(nodes []*Node) { + sort.SliceStable(nodes, func(i, j int) bool { + return nodes[i].Bounds.Width()*nodes[i].Bounds.Height() < + nodes[j].Bounds.Width()*nodes[j].Bounds.Height() + }) +} + func collectMatches(node *Node, accept func(*Element) bool, result *[]*Node) { if accept(&node.Element) { *result = append(*result, node) diff --git a/internal/hierarchy/hierarchy_test.go b/internal/hierarchy/hierarchy_test.go index ebcf8ea..6eff3b7 100644 --- a/internal/hierarchy/hierarchy_test.go +++ b/internal/hierarchy/hierarchy_test.go @@ -856,6 +856,74 @@ func TestIOSFlatSpatialScopeExcludesOutsideBounds(t *testing.T) { } } +// spatialSpecificityDump scopes a leaf testTag (iOS-flat pattern) over a +// screen where both a screen-sized container and the small element inside it +// match the same attribute, plus two equal-bounds siblings for tie ordering. +const spatialSpecificityDump = `{ + "attributes": {"bounds": "[0,0][402,874]"}, + "children": [ + { + "attributes": {"resource-id": "FormScreen", "bounds": "[0,62][402,840]"}, + "children": [] + }, + { + "attributes": {"resource-id": "FieldContainer", "accessibilityText": "Amount", "bounds": "[0,62][402,840]"}, + "children": [ + { + "attributes": {"resource-id": "AmountField", "accessibilityText": "Amount", "bounds": "[34,125][368,173]"}, + "children": [] + }, + { + "attributes": {"resource-id": "FirstTab", "accessibilityText": "Tab", "bounds": "[20,297][382,345]"}, + "children": [] + }, + { + "attributes": {"resource-id": "SecondTab", "accessibilityText": "Tab", "bounds": "[20,297][382,345]"}, + "children": [] + } + ] + } + ] +}` + +func TestSpatialFallbackPrefersSmallestContainingMatch(t *testing.T) { + tree, err := Parse(spatialSpecificityDump) + if err != nil { + t.Fatalf("Parse: %v", err) + } + screen := tree.FindNode("id:FormScreen") + if screen == nil { + t.Fatal("expected FormScreen node") + } + // Both FieldContainer (screen-sized, earlier in pre-order) and + // AmountField (small) match; the most specific match must win. + node := screen.Find("desc:Amount") + if node == nil { + t.Fatal("expected a spatial-fallback match") + } + if node.ResourceID != "AmountField" { + t.Fatalf("expected the smallest containing match AmountField, got id=%q", node.ResourceID) + } +} + +func TestSpatialFallbackEqualAreaKeepsPreOrder(t *testing.T) { + tree, err := Parse(spatialSpecificityDump) + if err != nil { + t.Fatalf("Parse: %v", err) + } + screen := tree.FindNode("id:FormScreen") + if screen == nil { + t.Fatal("expected FormScreen node") + } + node := screen.Find("desc:Tab") + if node == nil { + t.Fatal("expected a spatial-fallback match") + } + if node.ResourceID != "FirstTab" { + t.Fatalf("equal-area matches must keep pre-order, got id=%q", node.ResourceID) + } +} + func TestIOSFlatStructuralChildStillPreferred(t *testing.T) { tree, _ := Parse(iosFlatDump) submit := tree.FindNode("id:LoginSubmit") diff --git a/internal/runner/runner.go b/internal/runner/runner.go index a2d8237..ae5d640 100644 --- a/internal/runner/runner.go +++ b/internal/runner/runner.go @@ -14,8 +14,6 @@ import ( "time" "golang.org/x/sync/errgroup" - "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" "github.com/priyanshujain/sanderling/internal/driver" "github.com/priyanshujain/sanderling/internal/hierarchy" @@ -81,6 +79,7 @@ func Run(ctx context.Context, options Options) (Summary, error) { summary := Summary{StartTime: time.Now()} deadline := summary.StartTime.Add(options.Duration) stepIndex := 0 + consecutiveApplyFailures := 0 var lastAction *verifier.Action var lastLogTime time.Time for time.Now().Before(deadline) { @@ -227,19 +226,29 @@ func Run(ctx context.Context, options Options) (Summary, error) { applySkipped := false if nextErr == nil { - if err := applyAction(ctx, options.Driver, nextAction, tree); err != nil { + if err := applyAction(ctx, options.Driver, nextAction, tree, options.IdleTimeout); err != nil { if isWDADrop(err) { - return summary, fmt.Errorf("step %d: iOS XCTest runner lost connection - known WDA startup flake, re-run the test: %w", stepIndex, err) + return summary, fmt.Errorf("step %d: the iOS XCTest runner could not be restarted - re-run the test: %w", stepIndex, err) } - if isTransientApplyError(ctx, err) { - logger.Warn("transient apply error; marking step transitional", "step", stepIndex, "err", err) - transitional = true - applySkipped = true - lastAction = nil - } else { + if ctx.Err() != nil { return summary, fmt.Errorf("step %d apply: %w", stepIndex, err) } + // Every apply error is a device-side condition (a dropped + // gesture, a typing request the runner's input handler choked + // on, an RPC deadline). None of them individually justify + // killing a fuzz run; what does is an unbroken streak, which + // means the device is wedged. The step is marked transitional + // so the verifier never sees a state the action did not reach. + consecutiveApplyFailures++ + if consecutiveApplyFailures >= maxConsecutiveApplyFailures { + return summary, fmt.Errorf("step %d apply: %d consecutive failures; the device is not recovering: %w", stepIndex, consecutiveApplyFailures, err) + } + logger.Warn("apply error; marking step transitional", "step", stepIndex, "err", err) + transitional = true + applySkipped = true + lastAction = nil } else { + consecutiveApplyFailures = 0 actionCopy := nextAction lastAction = &actionCopy } @@ -452,7 +461,7 @@ func settleForForeground(ctx context.Context, options Options) { cancel() } -func applyAction(ctx context.Context, drv driver.DeviceDriver, action verifier.Action, tree *hierarchy.Tree) error { +func applyAction(ctx context.Context, drv driver.DeviceDriver, action verifier.Action, tree *hierarchy.Tree, idleTimeout time.Duration) error { switch action.Kind { case verifier.ActionKindTap: x, y, ok := resolveCoordinates(action, tree) @@ -488,22 +497,36 @@ func applyAction(ctx context.Context, drv driver.DeviceDriver, action verifier.A } return drv.Swipe(ctx, fromX, fromY, toX, toY, duration) case verifier.ActionKindInputText: + tapped := false if x, y, ok := resolveCoordinates(action, tree); ok { if err := drv.Tap(ctx, x, y); err != nil { return err } + tapped = true } else if action.On != "" { if err := drv.TapSelector(ctx, action.On); err != nil { return err } + tapped = true + } + // The focus tap raises the keyboard. Settle before sending key + // events so the keyboard animation cannot race them into the wrong + // field (or drop them entirely). + if tapped { + idleCtx, idleCancel := context.WithTimeout(ctx, idleTimeout) + _ = drv.WaitForIdle(idleCtx, idleTimeout) + idleCancel() } // InputText replaces the field's content: erase what the target // holds before typing. Appending instead lets repeated draws grow // the field without bound (e.g. into a max-length validation error // the fuzzer can never escape) and makes retried typing land twice. - if count := existingTextLength(action, tree); count > 0 { - if err := drv.EraseText(ctx, count); err != nil { - return err + // Drivers whose InputText already replaces skip the erase entirely. + if !inputReplacesText(drv) { + if count := existingTextLength(action, tree); count > 0 { + if err := drv.EraseText(ctx, count); err != nil { + return err + } } } return drv.InputText(ctx, action.Text) @@ -556,6 +579,13 @@ func collectLogs(ctx context.Context, drv driver.DeviceDriver, since time.Time) return result } +// inputReplacesText reports whether the driver's InputText replaces existing +// content, making the runner's pre-erase redundant. +func inputReplacesText(drv driver.DeviceDriver) bool { + replacer, ok := drv.(driver.TextReplacer) + return ok && replacer.ReplacesTextOnInput() +} + // existingTextLength returns the character count of the InputText target's // current text, so the runner can erase it before typing. Zero when the // target cannot be resolved or holds no text. @@ -671,6 +701,7 @@ const ( // transient state. func fetchSyncedState(ctx context.Context, options Options, logger *slog.Logger, stepIndex int) (tree *hierarchy.Tree, transitional bool, err error) { var pngBytes []byte + var previousJSON string retryLoop: for attempt := range transitionalRetryAttempts { hierarchyJSON, image, snapshotErr := options.Driver.Snapshot(ctx) @@ -684,6 +715,14 @@ retryLoop: if err != nil || !isTransitionalHierarchy(tree) { break } + // A tree unchanged since the previous attempt is a settled state + // that merely matches the heuristic (persistent overlay, both route + // ids alive at rest), not a cross-fade in flight: verify it instead + // of burning the retry budget and skipping the verifier forever. + if attempt > 0 && hierarchyJSON == previousJSON { + break + } + previousJSON = hierarchyJSON if attempt == transitionalRetryAttempts-1 { transitional = true break @@ -760,29 +799,23 @@ func traceActionFor(action verifier.Action, tree *hierarchy.Tree) *trace.Action return traceAction } -// stampSelectorTarget mirrors applyAction's coordinate-resolution rule so the -// trace records the same point the runner taps. +// stampSelectorTarget records the element bounds the selector resolved to and +// derives the tap point through resolveCoordinates, the same rule applyAction +// dispatches with, so the trace can never record a different point than the +// one tapped. func stampSelectorTarget(traceAction *trace.Action, action verifier.Action, tree *hierarchy.Tree) { - if action.X > 0 && action.Y > 0 { - traceAction.TapPoint = &trace.PointRecord{X: action.X, Y: action.Y} - return + if action.On != "" && tree != nil { + if element := tree.Find(action.On); element != nil { + bounds := element.Bounds + traceAction.ResolvedBounds = &trace.BoundsRecord{ + X: bounds.Left, + Y: bounds.Top, + Width: bounds.Width(), + Height: bounds.Height(), + } + } } - if tree == nil || action.On == "" { - return - } - element := tree.Find(action.On) - if element == nil { - return - } - bounds := element.Bounds - traceAction.ResolvedBounds = &trace.BoundsRecord{ - X: bounds.Left, - Y: bounds.Top, - Width: bounds.Width(), - Height: bounds.Height(), - } - x, y := bounds.Center() - if x > 0 && y > 0 { + if x, y, ok := resolveCoordinates(action, tree); ok { traceAction.TapPoint = &trace.PointRecord{X: x, Y: y} } } @@ -888,34 +921,18 @@ func encodeResiduals(residuals map[string]ltl.Formula) (map[string]json.RawMessa return encoded, firstErr } +// maxConsecutiveApplyFailures bounds how many transient apply failures in a +// row the run tolerates before aborting. One or two absorb a runner restart; +// an unbroken streak means the device is wedged and the rest of the budget +// would be spent doing nothing. +const maxConsecutiveApplyFailures = 3 + +// isWDADrop reports that the sidecar could not restart the iOS XCTest +// runner: the channel is gone for good and the run must abort. Transient +// drops are classified by the sidecar itself (it reconnects and surfaces +// UNAVAILABLE), so matching on raw exception text like "ConnectException" +// here would kill runs the sidecar already recovered. func isWDADrop(err error) bool { - msg := err.Error() - return strings.Contains(msg, "ConnectException") || - (strings.Contains(msg, "code = Internal") && strings.Contains(msg, "SocketException")) + return strings.Contains(err.Error(), "WDA reconnect failed") } -// isTransientApplyError reports whether an applyAction failure is a transient -// device-side hang (sidecar RPC deadline, momentary unavailability) rather than -// a fatal condition. Such steps are recorded as transitional and the loop -// continues. The run context being cancelled is never transient: it means the -// caller wants to stop. -func isTransientApplyError(runCtx context.Context, err error) bool { - if err == nil || runCtx.Err() != nil { - return false - } - if s, ok := status.FromError(err); ok { - switch s.Code() { - case codes.DeadlineExceeded, codes.Unavailable: - return true - case codes.Internal: - message := s.Message() - if strings.Contains(message, "DEADLINE_EXCEEDED") || strings.Contains(message, "UNAVAILABLE") { - return true - } - } - } - if errors.Is(err, context.DeadlineExceeded) { - return true - } - return false -} diff --git a/internal/runner/runner_test.go b/internal/runner/runner_test.go index 1ee7eb6..c2977de 100644 --- a/internal/runner/runner_test.go +++ b/internal/runner/runner_test.go @@ -522,6 +522,36 @@ func TestRunner_StampsHierarchyResolvedBoundsAndResiduals(t *testing.T) { } } +// TestTraceActionFor_StaleCoordinatesDoNotOverrideTreeCenter pins the stamp +// to applyAction's resolution rule: when On resolves in the tree, the trace +// tap point must be the tree center even if the action carries stale X/Y +// from an earlier tick. +func TestTraceActionFor_StaleCoordinatesDoNotOverrideTreeCenter(t *testing.T) { + tree, err := hierarchy.Parse(`{"attributes":{"resource-id":"root","bounds":"[0,0,1080,2340]"},"children":[ + {"attributes":{"resource-id":"next","bounds":"[100,200,300,400]"},"children":[]} + ]}`) + if err != nil { + t.Fatalf("Parse: %v", err) + } + action := verifier.Action{Kind: verifier.ActionKindTap, On: "id:next", X: 50, Y: 60} + + traceAction := traceActionFor(action, tree) + if traceAction.TapPoint == nil { + t.Fatal("expected a tap point") + } + if traceAction.TapPoint.X != 200 || traceAction.TapPoint.Y != 300 { + t.Errorf("tap point = (%d,%d), want tree center (200,300)", + traceAction.TapPoint.X, traceAction.TapPoint.Y) + } + if traceAction.ResolvedBounds == nil { + t.Fatal("expected resolved bounds") + } + if traceAction.ResolvedBounds.X != 100 || traceAction.ResolvedBounds.Y != 200 { + t.Errorf("resolved bounds origin = (%d,%d), want (100,200)", + traceAction.ResolvedBounds.X, traceAction.ResolvedBounds.Y) + } +} + func TestRunner_LogsWaitForIdleDriverErrors(t *testing.T) { state := newHarness(t) state.mock.Failures[mockdriver.ActionWaitForIdle] = errors.New("sidecar lost gRPC stream") @@ -560,21 +590,66 @@ func TestApplyAction_InputTextErasesExistingTextBeforeTyping(t *testing.T) { driverMock := mockdriver.New() action := verifier.Action{Kind: verifier.ActionKindInputText, On: "id:username", Text: "alice"} - if err := applyAction(context.Background(), driverMock, action, tree); err != nil { + if err := applyAction(context.Background(), driverMock, action, tree, time.Millisecond); err != nil { t.Fatalf("applyAction: %v", err) } actions := driverMock.Actions() - if len(actions) != 3 { - t.Fatalf("want tap, erase, input; got %v", actions) + if len(actions) != 4 { + t.Fatalf("want tap, wait_for_idle, erase, input; got %v", actions) } if actions[0].Kind != mockdriver.ActionTap { t.Errorf("first action = %v, want tap", actions[0].Kind) } - if actions[1].Kind != mockdriver.ActionEraseText || actions[1].CharacterCount != len("stale-value") { - t.Errorf("second action = %+v, want erase_text of %d characters", actions[1], len("stale-value")) + if actions[1].Kind != mockdriver.ActionWaitForIdle { + t.Errorf("second action = %v, want wait_for_idle (settle after focus tap)", actions[1].Kind) } - if actions[2].Kind != mockdriver.ActionInputText || actions[2].Text != "alice" { - t.Errorf("third action = %+v, want input_text alice", actions[2]) + if actions[2].Kind != mockdriver.ActionEraseText || actions[2].CharacterCount != len("stale-value") { + t.Errorf("third action = %+v, want erase_text of %d characters", actions[2], len("stale-value")) + } + if actions[3].Kind != mockdriver.ActionInputText || actions[3].Text != "alice" { + t.Errorf("fourth action = %+v, want input_text alice", actions[3]) + } +} + +// TestApplyAction_InputTextWithoutTargetSkipsSettle pins that the post-tap +// settle only runs when a focus tap actually happened: with no resolvable +// target there is no keyboard animation to absorb. +func TestApplyAction_InputTextWithoutTargetSkipsSettle(t *testing.T) { + driverMock := mockdriver.New() + action := verifier.Action{Kind: verifier.ActionKindInputText, X: -1, Y: -1, Text: "alice"} + + if err := applyAction(context.Background(), driverMock, action, nil, time.Millisecond); err != nil { + t.Fatalf("applyAction: %v", err) + } + for _, recorded := range driverMock.Actions() { + if recorded.Kind == mockdriver.ActionWaitForIdle { + t.Errorf("no focus tap happened; settle must be skipped: %v", driverMock.Actions()) + } + } +} + +// TestApplyAction_InputTextSkipsEraseForReplacingDriver pins that a driver +// asserting the TextReplacer capability never pays the pre-erase round-trip: +// its InputText already replaces the field's content. +func TestApplyAction_InputTextSkipsEraseForReplacingDriver(t *testing.T) { + tree, err := hierarchy.Parse(`{"attributes":{"resource-id":"root","bounds":"[0,0,1080,2340]"},"children":[ + {"attributes":{"resource-id":"username","text":"stale-value","bounds":"[10,10,500,100]"},"children":[]} + ]}`) + if err != nil { + t.Fatalf("Parse: %v", err) + } + driverMock := mockdriver.New() + driverMock.ReplacesText = true + action := verifier.Action{Kind: verifier.ActionKindInputText, On: "id:username", Text: "alice"} + + if err := applyAction(context.Background(), driverMock, action, tree, time.Millisecond); err != nil { + t.Fatalf("applyAction: %v", err) + } + if containsAction(driverMock.Actions(), mockdriver.ActionEraseText, "") { + t.Errorf("replacing driver must not be asked to erase: %v", driverMock.Actions()) + } + if !containsAction(driverMock.Actions(), mockdriver.ActionInputText, "") { + t.Errorf("expected InputText, got %v", driverMock.Actions()) } } @@ -588,7 +663,7 @@ func TestApplyAction_InputTextSkipsEraseWhenTargetEmpty(t *testing.T) { driverMock := mockdriver.New() action := verifier.Action{Kind: verifier.ActionKindInputText, On: "id:username", Text: "alice"} - if err := applyAction(context.Background(), driverMock, action, tree); err != nil { + if err := applyAction(context.Background(), driverMock, action, tree, time.Millisecond); err != nil { t.Fatalf("applyAction: %v", err) } if containsAction(driverMock.Actions(), mockdriver.ActionEraseText, "") { @@ -602,7 +677,7 @@ func TestApplyAction_InputTextSurfacesFocusTapError(t *testing.T) { driverMock.Failures[mockdriver.ActionTapSelector] = errors.New("adb unreachable") action := verifier.Action{Kind: verifier.ActionKindInputText, On: "id:username", Text: "alice"} - err := applyAction(context.Background(), driverMock, action, nil) + err := applyAction(context.Background(), driverMock, action, nil, time.Millisecond) if err == nil { t.Fatalf("expected focus tap failure to surface, got nil") } @@ -615,7 +690,7 @@ func TestApplyAction_InputTextSurfacesFocusTapError(t *testing.T) { driverMock.Failures[mockdriver.ActionTap] = errors.New("tap driver error") action := verifier.Action{Kind: verifier.ActionKindInputText, X: 10, Y: 20, Text: "alice"} - err := applyAction(context.Background(), driverMock, action, nil) + err := applyAction(context.Background(), driverMock, action, nil, time.Millisecond) if err == nil { t.Fatalf("expected focus tap failure to surface, got nil") } @@ -629,7 +704,7 @@ func TestApplyAction_V8InputTextTapsAtCoordinates(t *testing.T) { driverMock := mockdriver.New() action := verifier.Action{Kind: verifier.ActionKindInputText, X: 50, Y: 100, Text: "alice"} - if err := applyAction(context.Background(), driverMock, action, nil); err != nil { + if err := applyAction(context.Background(), driverMock, action, nil, time.Millisecond); err != nil { t.Fatalf("apply action: %v", err) } actions := driverMock.Actions() @@ -648,7 +723,7 @@ func TestApplyAction_V8InputTextAtOriginStillTaps(t *testing.T) { // InputText with (0,0) is a deliberate edge tap, not a sentinel). action := verifier.Action{Kind: verifier.ActionKindInputText, X: 0, Y: 0, Text: "alice"} - if err := applyAction(context.Background(), driverMock, action, nil); err != nil { + if err := applyAction(context.Background(), driverMock, action, nil, time.Millisecond); err != nil { t.Fatalf("apply action: %v", err) } if !containsAction(driverMock.Actions(), mockdriver.ActionTap, "") { @@ -660,7 +735,7 @@ func TestApplyAction_DoubleTapDispatchesDoubleTapAtCoordinates(t *testing.T) { driverMock := mockdriver.New() action := verifier.Action{Kind: verifier.ActionKindDoubleTap, X: 100, Y: 200} - if err := applyAction(context.Background(), driverMock, action, nil); err != nil { + if err := applyAction(context.Background(), driverMock, action, nil, time.Millisecond); err != nil { t.Fatalf("apply action: %v", err) } taps := 0 @@ -678,7 +753,7 @@ func TestApplyAction_DoubleTapDispatchesDoubleTapSelector(t *testing.T) { driverMock := mockdriver.New() action := verifier.Action{Kind: verifier.ActionKindDoubleTap, On: "id:save"} - if err := applyAction(context.Background(), driverMock, action, nil); err != nil { + if err := applyAction(context.Background(), driverMock, action, nil, time.Millisecond); err != nil { t.Fatalf("apply action: %v", err) } taps := 0 @@ -696,7 +771,7 @@ func TestApplyAction_LongPressDispatchesAtResolvedCoordinates(t *testing.T) { driverMock := mockdriver.New() action := verifier.Action{Kind: verifier.ActionKindLongPress, X: 120, Y: 240} - if err := applyAction(context.Background(), driverMock, action, nil); err != nil { + if err := applyAction(context.Background(), driverMock, action, nil, time.Millisecond); err != nil { t.Fatalf("apply action: %v", err) } found := false @@ -722,7 +797,7 @@ func TestApplyAction_ScrollWithPrecomputedEndpointsSwipes(t *testing.T) { DurationMillis: 300, } - if err := applyAction(context.Background(), driverMock, action, nil); err != nil { + if err := applyAction(context.Background(), driverMock, action, nil, time.Millisecond); err != nil { t.Fatalf("apply action: %v", err) } found := false @@ -745,7 +820,7 @@ func TestApplyAction_ScrollDirectionUsesInversion(t *testing.T) { } action := verifier.Action{Kind: verifier.ActionKindScroll, Direction: "down", On: "id:list"} - if err := applyAction(context.Background(), driverMock, action, tree); err != nil { + if err := applyAction(context.Background(), driverMock, action, tree, time.Millisecond); err != nil { t.Fatalf("apply action: %v", err) } var swipe *mockdriver.Action @@ -774,7 +849,7 @@ func TestApplyAction_ScrollScreenFallback(t *testing.T) { // On unset: container falls back to whole-screen (root) bounds. action := verifier.Action{Kind: verifier.ActionKindScroll, Direction: "up"} - if err := applyAction(context.Background(), driverMock, action, tree); err != nil { + if err := applyAction(context.Background(), driverMock, action, tree, time.Millisecond); err != nil { t.Fatalf("apply action: %v", err) } var swipe *mockdriver.Action @@ -956,17 +1031,18 @@ func TestIsTransitionalHierarchy_DetectsMultipleScreens(t *testing.T) { } } -// TestRunner_TransitionalSkipsVerifier feeds a driver whose hierarchy stays -// transitional (multiple route-level *Screen ids) on every Snapshot call. -// Every step must be marked transitional in the trace, no violations may be -// emitted (the verifier never ran), and the summary must stay clean even -// though the spec is a guaranteed always-false predicate. -func TestRunner_TransitionalSkipsVerifier(t *testing.T) { +// TestRunner_StableTransitionalTreeIsVerified feeds a driver whose hierarchy +// constantly carries two route-level *Screen ids but never changes between +// retry attempts. Such a tree is a settled state that merely matches the +// transitional heuristic, so the runner must verify it (the always-false +// predicate's violation surfaces) instead of skipping the verifier forever. +func TestRunner_StableTransitionalTreeIsVerified(t *testing.T) { state := newHarnessWithSpec(t, violationSpec) state.mock.HierarchyJSON = `{"attributes":{"resource-id":"root"},"children":[ {"attributes":{"resource-id":"AddAccountScreen"},"children":[]}, {"attributes":{"resource-id":"HomeScreen"},"children":[]} ]}` + state.mock.ImageData = driver.Image{PNG: []byte("fakepng"), Width: 100, Height: 200} ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() @@ -983,8 +1059,77 @@ func TestRunner_TransitionalSkipsVerifier(t *testing.T) { if summary.Steps == 0 { t.Fatal("expected at least one step") } + if !containsProperty(summary.Violations, "balanceNonNegative") { + t.Fatalf("expected verifier to run on a stable two-screen tree, got %v", summary.Violations) + } + + type traceLine struct { + Step int `json:"step"` + Transitional bool `json:"transitional"` + } + body, err := os.ReadFile(filepath.Join(state.writer.Directory(), "trace.jsonl")) + if err != nil { + t.Fatal(err) + } + for _, raw := range bytes.Split(bytes.TrimSpace(body), []byte("\n")) { + var line traceLine + if err := json.Unmarshal(raw, &line); err != nil { + t.Fatalf("decode trace line: %v", err) + } + if line.Transitional { + t.Errorf("step %d: stable tree must not be marked transitional", line.Step) + } + } + + // The early break must still persist the step's screenshot. + screenshotPath := filepath.Join(state.writer.Directory(), "screenshots", "step-00001.png") + if _, err := os.Stat(screenshotPath); err != nil { + t.Errorf("expected screenshot for step 1 at %s: %v", screenshotPath, err) + } +} + +// snapshotCrossFade wraps a mock driver so every Snapshot call returns a +// transitional two-screen tree whose JSON differs from the previous call, +// mimicking a genuine cross-fade in flight. +type snapshotCrossFade struct { + *mockdriver.Driver + calls int +} + +func (d *snapshotCrossFade) Snapshot(ctx context.Context) (string, driver.Image, error) { + d.calls++ + _, image, err := d.Driver.Snapshot(ctx) + hierarchyJSON := fmt.Sprintf(`{"attributes":{"resource-id":"root"},"children":[ + {"attributes":{"resource-id":"AddAccountScreen","text":"frame-%d"},"children":[]}, + {"attributes":{"resource-id":"HomeScreen"},"children":[]} + ]}`, d.calls) + return hierarchyJSON, image, err +} + +// TestRunner_GenuineCrossFadeStillRetried pins the existing behavior for real +// transitions: a tree that keeps changing between retry attempts exhausts the +// budget, stays transitional, and the verifier is skipped for the step. +func TestRunner_GenuineCrossFadeStillRetried(t *testing.T) { + state := newHarnessWithSpec(t, violationSpec) + wrapped := &snapshotCrossFade{Driver: state.mock} + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + summary, err := Run(ctx, Options{ + Duration: 200 * time.Millisecond, + IdleTimeout: 20 * time.Millisecond, + Driver: wrapped, + Verifier: state.verifier, + TraceWriter: state.writer, + }) + if err != nil { + t.Fatalf("Run: %v", err) + } + if summary.Steps == 0 { + t.Fatal("expected at least one step") + } if len(summary.Violations) != 0 { - t.Fatalf("verifier must be skipped on transitional steps; got %v", summary.Violations) + t.Fatalf("verifier must be skipped on cross-fade steps; got %v", summary.Violations) } type traceLine struct { @@ -1176,8 +1321,8 @@ func TestRunner_TransientApplyErrorMarksTransitional(t *testing.T) { if len(summary.Violations) != 0 { t.Errorf("transient apply error must not surface as a violation, got %v", summary.Violations) } - if !strings.Contains(logBuf.String(), "transient apply error") { - t.Errorf("expected transient-apply WARN log, got %q", logBuf.String()) + if !strings.Contains(logBuf.String(), "apply error; marking step transitional") { + t.Errorf("expected apply-error WARN log, got %q", logBuf.String()) } type traceLine struct { @@ -1208,37 +1353,119 @@ func TestRunner_TransientApplyErrorMarksTransitional(t *testing.T) { } } -// TestIsTransientApplyError_Classification covers the helper's matching rules -// directly so future code changes don't quietly drop a transient case. -func TestIsTransientApplyError_Classification(t *testing.T) { - cleanCtx := context.Background() - cancelledCtx, cancel := context.WithCancel(context.Background()) - cancel() +// internalApplyErrorFailFirst wraps a mock driver so the first InputText call +// fails with the bare Internal error the iOS runner's input handler emits +// when it chokes (HTTP 500 with an empty body), then recovers. +type internalApplyErrorFailFirst struct { + *mockdriver.Driver + calls int +} +func (d *internalApplyErrorFailFirst) TapSelector(ctx context.Context, selector string) error { + d.calls++ + if d.calls == 1 { + return status.Error(codes.Internal, "UnknownFailure(errorResponse=Request for inputText failed, code: 500, body: )") + } + return d.Driver.TapSelector(ctx, selector) +} + +// TestRunner_InternalApplyErrorMarksTransitional pins the policy that a +// one-off device-side failure (e.g. the iOS input handler's bare 500) is +// absorbed as a transitional step instead of killing the run. Persistent +// failure is covered by the consecutive-failure cap. +func TestRunner_InternalApplyErrorMarksTransitional(t *testing.T) { + state := newHarness(t) + wrapped := &internalApplyErrorFailFirst{Driver: state.mock} + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + summary, err := Run(ctx, Options{ + Duration: 300 * time.Millisecond, + IdleTimeout: 20 * time.Millisecond, + Driver: wrapped, + Verifier: state.verifier, + TraceWriter: state.writer, + }) + if err != nil { + t.Fatalf("Run must not return on a one-off internal apply error, got %v", err) + } + if summary.Steps < 2 { + t.Fatalf("need at least 2 steps to prove the loop continued, got %d", summary.Steps) + } +} + +// TestIsWDADrop_Classification pins the fatal-vs-recoverable boundary: only +// the sidecar's explicit reconnect-failure signal is a drop. A structured +// UNAVAILABLE that happens to embed raw exception text (e.g. ConnectException +// from the original failure) means the sidecar already recovered and the run +// must continue. +func TestIsWDADrop_Classification(t *testing.T) { cases := []struct { name string - ctx context.Context err error want bool }{ - {"nil error", cleanCtx, nil, false}, - {"deadline exceeded", cleanCtx, status.Error(codes.DeadlineExceeded, "boom"), true}, - {"unavailable", cleanCtx, status.Error(codes.Unavailable, "boom"), true}, - {"internal wrapping deadline", cleanCtx, status.Error(codes.Internal, "io.grpc.StatusRuntimeException: DEADLINE_EXCEEDED: ..."), true}, - {"internal wrapping unavailable", cleanCtx, status.Error(codes.Internal, "io.grpc.StatusRuntimeException: UNAVAILABLE: ..."), true}, - {"internal generic", cleanCtx, status.Error(codes.Internal, "boom"), false}, - {"raw context deadline", cleanCtx, context.DeadlineExceeded, true}, - {"run context cancelled overrides", cancelledCtx, status.Error(codes.DeadlineExceeded, "boom"), false}, + { + "unavailable with embedded ConnectException is recovered", + status.Error(codes.Unavailable, "connection dropped mid-action; the action may have applied: java.net.ConnectException: Failed to connect to /127.0.0.1:22161"), + false, + }, + { + "reconnect failure is a drop", + status.Error(codes.Internal, "java.lang.IllegalStateException: WDA reconnect failed: IOSDriverTimeoutException"), + true, + }, + {"generic internal is not a drop", status.Error(codes.Internal, "boom"), false}, } for _, testCase := range cases { t.Run(testCase.name, func(t *testing.T) { - if got := isTransientApplyError(testCase.ctx, testCase.err); got != testCase.want { + if got := isWDADrop(testCase.err); got != testCase.want { t.Errorf("got %v, want %v", got, testCase.want) } }) } } +// tapSelectorAlwaysUnavailable wraps a mock driver so every TapSelector call +// fails with a transient Unavailable error, mimicking a device whose channel +// never recovers between steps. +type tapSelectorAlwaysUnavailable struct { + *mockdriver.Driver +} + +func (d *tapSelectorAlwaysUnavailable) TapSelector(ctx context.Context, selector string) error { + return status.Error(codes.Unavailable, "connection dropped mid-action; the action may have applied") +} + +// TestRunner_ConsecutiveTransientApplyFailuresAbort verifies the run fails +// fast once transient apply errors form an unbroken streak instead of burning +// the whole budget on a wedged device. +func TestRunner_ConsecutiveTransientApplyFailuresAbort(t *testing.T) { + state := newHarness(t) + wrapped := &tapSelectorAlwaysUnavailable{Driver: state.mock} + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + summary, err := Run(ctx, Options{ + Duration: 5 * time.Second, + IdleTimeout: 20 * time.Millisecond, + Driver: wrapped, + Verifier: state.verifier, + TraceWriter: state.writer, + }) + if err == nil { + t.Fatal("Run must abort after consecutive transient apply failures") + } + if !strings.Contains(err.Error(), "consecutive failures") { + t.Errorf("expected consecutive-failure abort, got %v", err) + } + // The aborting step returns before it is recorded, so the summary holds + // the steps before the cap-hitting one. + if summary.Steps != maxConsecutiveApplyFailures-1 { + t.Errorf("run must stop at the failure cap, got %d recorded steps", summary.Steps) + } +} + // TestRunner_WaitActionSkipsIdle ensures the runner does not call WaitForIdle // after a Wait action - the action already provides settling time. func TestRunner_WaitActionSkipsIdle(t *testing.T) { diff --git a/internal/testrun/driver.go b/internal/testrun/driver.go index d58f41c..af4c108 100644 --- a/internal/testrun/driver.go +++ b/internal/testrun/driver.go @@ -8,6 +8,8 @@ import ( "os" "os/exec" "strconv" + "syscall" + "time" "github.com/priyanshujain/sanderling/internal/android" "github.com/priyanshujain/sanderling/internal/driver" @@ -53,6 +55,13 @@ func buildDriver(ctx context.Context, options Options, stdout io.Writer) (driver sidecarCommand.Stdout = stdout sidecarCommand.Stderr = stdout sidecarCommand.Env = android.EnvWithAndroidPlatformTools(os.Environ()) + // SIGTERM lets the sidecar's shutdown hook stop the iOS XCTest runner. + // SIGKILL skips the hook and orphans an xcodebuild session that later + // restarts its runner and hijacks the simulator mid-run. + sidecarCommand.Cancel = func() error { + return sidecarCommand.Process.Signal(syscall.SIGTERM) + } + sidecarCommand.WaitDelay = sidecarShutdownGrace if err := sidecarCommand.Start(); err != nil { return nil, nil, fmt.Errorf("spawn sidecar: %w", err) } @@ -60,7 +69,7 @@ func buildDriver(ctx context.Context, options Options, stdout io.Writer) (driver driverClient, err := driverSidecar.Dial(fmt.Sprintf("127.0.0.1:%d", sidecarPort)) if err != nil { - _ = sidecarCommand.Process.Kill() + stopSidecar(sidecarCommand) return nil, nil, fmt.Errorf("dial sidecar: %w", err) } driverClient.SetPlatform(options.Platform) @@ -70,7 +79,7 @@ func buildDriver(ctx context.Context, options Options, stdout io.Writer) (driver healthCtx, healthCancel := context.WithTimeout(ctx, sidecarStartupTimeout) if err := driverClient.WaitForHealth(healthCtx, 250e6); err != nil { healthCancel() - _ = sidecarCommand.Process.Kill() + stopSidecar(sidecarCommand) _ = driverClient.Close() return nil, nil, fmt.Errorf("sidecar health check: %w", err) } @@ -79,13 +88,40 @@ func buildDriver(ctx context.Context, options Options, stdout io.Writer) (driver cleanup := func() { _ = driverClient.Close() - if sidecarCommand.Process != nil { - _ = sidecarCommand.Process.Kill() - } + stopSidecar(sidecarCommand) } return driverClient, cleanup, nil } +// sidecarShutdownGrace bounds how long the sidecar gets to run its shutdown +// hook (terminate the app, stop the XCTest runner) before being killed. +const sidecarShutdownGrace = 15 * time.Second + +// stopSidecar terminates the sidecar gracefully so its shutdown hook can stop +// the device-side runner processes, escalating to SIGKILL when it does not +// exit within the grace window. +func stopSidecar(sidecarCommand *exec.Cmd) { + if sidecarCommand.Process == nil { + return + } + if err := sidecarCommand.Process.Signal(syscall.SIGTERM); err != nil { + _ = sidecarCommand.Process.Kill() + _ = sidecarCommand.Wait() + return + } + done := make(chan struct{}) + go func() { + _ = sidecarCommand.Wait() + close(done) + }() + select { + case <-done: + case <-time.After(sidecarShutdownGrace): + _ = sidecarCommand.Process.Kill() + <-done + } +} + func pickFreePort() (int, error) { listener, err := net.Listen("tcp", "127.0.0.1:0") if err != nil { diff --git a/replay-ui/src/__tests__/device-space.test.ts b/replay-ui/src/__tests__/device-space.test.ts new file mode 100644 index 0000000..db3f453 --- /dev/null +++ b/replay-ui/src/__tests__/device-space.test.ts @@ -0,0 +1,47 @@ +import { describe, it, expect } from "bun:test"; +import { deviceSpaceOf } from "../lib/device-space"; +import type { Hierarchy } from "../types"; + +function hierarchyWithRoot(right: number, bottom: number): Hierarchy { + return { + elements: [ + { + bounds: { left: 0, top: 0, right, bottom }, + }, + ], + }; +} + +describe("deviceSpaceOf", () => { + it("returns the root bounds extent", () => { + expect(deviceSpaceOf(hierarchyWithRoot(393, 852))).toEqual({ + width: 393, + height: 852, + }); + }); + + it("returns undefined without a hierarchy", () => { + expect(deviceSpaceOf(undefined)).toBeUndefined(); + }); + + it("returns undefined for an empty hierarchy", () => { + expect(deviceSpaceOf({ elements: [] })).toBeUndefined(); + }); + + it("returns undefined for non-positive root bounds", () => { + expect(deviceSpaceOf(hierarchyWithRoot(0, 852))).toBeUndefined(); + expect(deviceSpaceOf(hierarchyWithRoot(393, 0))).toBeUndefined(); + expect(deviceSpaceOf(hierarchyWithRoot(-1, -1))).toBeUndefined(); + }); + + it("skips the iOS synthetic zero-bounds root", () => { + const hierarchy: Hierarchy = { + elements: [ + { bounds: { left: 0, top: 0, right: 0, bottom: 0 } }, + { bounds: { left: 0, top: 0, right: 402, bottom: 874 } }, + { bounds: { left: 20, top: 100, right: 380, bottom: 150 } }, + ], + }; + expect(deviceSpaceOf(hierarchy)).toEqual({ width: 402, height: 874 }); + }); +}); diff --git a/replay-ui/src/lib/device-space.ts b/replay-ui/src/lib/device-space.ts new file mode 100644 index 0000000..5c89828 --- /dev/null +++ b/replay-ui/src/lib/device-space.ts @@ -0,0 +1,20 @@ +import type { Hierarchy } from "../types"; + +// Tap points and resolved bounds in the trace share the hierarchy root's +// coordinate space (iOS points, Android pixels, web CSS px). Screenshots may +// be scaled (iOS 3x, web DPR>1), so the overlay viewBox must come from the +// root bounds, not the image's natural pixel size. Elements are in pre-order; +// the first one with positive extent is the root window (iOS prepends a +// synthetic zero-bounds node, so plain elements[0] is not enough). +export function deviceSpaceOf( + hierarchy?: Hierarchy, +): { width: number; height: number } | undefined { + for (const element of hierarchy?.elements ?? []) { + const width = element.bounds.right; + const height = element.bounds.bottom; + if (width > 0 && height > 0) { + return { width, height }; + } + } + return undefined; +} diff --git a/replay-ui/src/routes/RunDetail.tsx b/replay-ui/src/routes/RunDetail.tsx index eab5ca9..bade4e5 100644 --- a/replay-ui/src/routes/RunDetail.tsx +++ b/replay-ui/src/routes/RunDetail.tsx @@ -14,6 +14,7 @@ import Tabs, { type TabDefinition } from "../components/Tabs"; import { useStep } from "../hooks/useStep"; import { useKeyboardNav } from "../hooks/useKeyboardNav"; import { useTheme } from "../hooks/useTheme"; +import { deviceSpaceOf } from "../lib/device-space"; function basename(specPath: string): string { const index = specPath.lastIndexOf("/"); @@ -129,12 +130,21 @@ export default function RunDetail() { const witnessesBefore = currentStep?.witnesses; const witnessesAfter = nextStep?.witnesses ?? witnessesBefore; const exceptionsForStep = currentStep?.exceptions; + const beforeSpace = deviceSpaceOf(currentStep?.hierarchy); + const afterSpace = deviceSpaceOf(nextStep?.hierarchy ?? currentStep?.hierarchy); const beforeTabs: TabDefinition[] = [ { id: "screenshot", label: "Screenshot", - content: , + content: ( + + ), }, { id: "snapshots", @@ -194,7 +204,14 @@ export default function RunDetail() { { id: "screenshot", label: "Screenshot", - content: , + content: ( + + ), }, { id: "snapshots", diff --git a/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverBackend.kt b/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverBackend.kt index e21f5a5..e75ed56 100644 --- a/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverBackend.kt +++ b/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverBackend.kt @@ -31,6 +31,11 @@ interface DriverBackend { // serialized pair from the same on-device frame. Backends may override // to fuse the two reads more tightly when their native API allows. fun snapshot(): SnapshotSample = SnapshotSample(hierarchy(), screenshot()) + + // close releases device-side resources on shutdown. The iOS backend must + // stop its XCTest runner here: an orphaned runner session auto-restarts + // later and hijacks the simulator's gesture daemon mid-run. + fun close() {} } data class SnapshotSample( @@ -179,6 +184,29 @@ private fun walkForStructuralHash(node: com.fasterxml.jackson.databind.JsonNode, out.append(')') } +// overlappedDoubleTap fires the second tap while the first is still in +// flight, so the on-device gap stays tight on transports with high per-tap +// latency. The overlap can collide with the other tap still executing ("only +// one gesture can be performed at a time") on either leg; the colliding leg +// then lands sequentially after the surviving one instead of failing the +// step. +internal fun overlappedDoubleTap(tapAction: () -> Unit) { + val firstTap = java.util.concurrent.CompletableFuture.runAsync { tapAction() } + Thread.sleep(40) + try { + tapAction() + } catch (_: Throwable) { + runCatching { firstTap.join() } + tapAction() + return + } + try { + firstTap.join() + } catch (_: Throwable) { + tapAction() + } +} + data class MetricsSample( val cpuPercent: Double, val heapBytes: Long, @@ -595,6 +623,11 @@ class MaestroDriverBackend(private val serial: String?) : DriverBackend { override fun healthy() = runCatching { driver.contentDescriptor(false); true }.getOrElse { false } override fun metrics(bundleId: String) = readProcMetrics(serial, bundleId) + + override fun close() { + runCatching { driver.close() } + runCatching { dadb.close() } + } } private fun buildDadb(serial: String?): dadb.Dadb { @@ -649,12 +682,103 @@ private fun pngHeight(bytes: ByteArray): Int { (bytes[22].toInt() and 0xFF shl 8) or (bytes[23].toInt() and 0xFF) } +internal const val IOS_XCTEST_RUNNER_BUNDLE_ID = "dev.mobile.maestro-driver-iosUITests.xctrunner" + +// reapOrphanIosRunners kills XCTest runner sessions left over from a prior +// run. A sidecar that died without its shutdown hook leaves its xcodebuild +// session alive; xcodebuild later restarts its dead runner, which terminates +// the active run's session and steals the simulator's gesture daemon. Returns +// true when an orphaned xcodebuild session was found and killed. +internal fun reapOrphanIosRunners(udid: String, execute: (List) -> Int): Boolean { + val killed = execute(listOf("pkill", "-f", "xcodebuild.*test-without-building.*$udid")) == 0 + execute(listOf("xcrun", "simctl", "terminate", udid, IOS_XCTEST_RUNNER_BUNDLE_ID)) + return killed +} + +// WdaRecovery serializes XCTest runner recovery across concurrent RPCs. An +// IOException on one call does not prove the runner is down (an overlapped +// gesture can reset a single connection), and a full runner restart costs +// around 50 seconds of downtime, so recovery probes channel liveness first +// and only restarts a dead channel. The probe re-runs under the lock so +// threads queued behind an in-flight restart do not restart again. +internal class WdaRecovery( + private val isAlive: () -> Boolean, + private val restart: () -> Unit, + private val log: (String) -> Unit = ::println, +) { + private val lock = java.util.concurrent.locks.ReentrantLock() + + // run executes block, recovering the channel on IO failure. replay re-runs + // the block afterwards and is only safe for idempotent reads: an action + // can fail client-side after the device already applied it, so replaying + // types text or taps twice. Non-idempotent actions surface UNAVAILABLE, + // which the runner treats as transient. + fun run(replay: Boolean, block: () -> T): T { + return try { + block() + } catch (e: Exception) { + if (!isIoFailure(e)) throw e + recover(e) + if (!replay) { + throw io.grpc.Status.UNAVAILABLE + .withDescription("connection dropped mid-action; the action may have applied: ${e.message}") + .withCause(e).asRuntimeException() + } + try { + block() + } catch (retryErr: Exception) { + if (!isIoFailure(retryErr)) throw retryErr + throw io.grpc.Status.UNAVAILABLE + .withDescription("read retry failed after channel recovery: ${retryErr.message}") + .withCause(retryErr).asRuntimeException() + } + } + } + + private fun isIoFailure(e: Exception): Boolean = + generateSequence(e as Throwable) { it.cause }.any { it is java.io.IOException } + + private fun recover(cause: Exception) { + lock.lock() + try { + if (isAlive()) { + log("channel alive after $cause; skipping runner restart") + return + } + log("channel dead after $cause; restarting the XCTest runner") + val startedAt = System.currentTimeMillis() + try { + restart() + } catch (restartErr: Exception) { + throw IllegalStateException("WDA reconnect failed: $restartErr", cause) + } + log("XCTest runner restarted in ${System.currentTimeMillis() - startedAt} ms") + } finally { + lock.unlock() + } + } +} + class IosDriverBackend(private val udid: String) : DriverBackend { private lateinit var driver: maestro.drivers.IOSDriver private lateinit var localDevice: ios.LocalIOSDevice - private val reconnectLock = java.util.concurrent.locks.ReentrantLock() + private lateinit var installer: xcuitest.installer.LocalXCTestInstaller + private val recovery by lazy { + WdaRecovery( + isAlive = { runCatching { installer.isChannelAlive() }.getOrElse { false } }, + restart = { driver.open(); warmup() }, + ) + } init { + val reaped = reapOrphanIosRunners(udid) { command -> + runCatching { ProcessBuilder(command).start().waitFor() }.getOrDefault(1) + } + if (reaped) { + println("terminated orphaned XCTest runner session for $udid") + // Give the killed session a beat to tear down before installing ours. + Thread.sleep(1000) + } val wdaPort = maestro.utils.SocketUtils.nextFreePort(22000, 23000) val tempFileHandler = maestro.utils.TempFileHandler() val simctlDevice = device.SimctlIOSDevice( @@ -667,7 +791,7 @@ class IosDriverBackend(private val udid: String) : DriverBackend { context = xcuitest.installer.Context.CLI, snapshotKeyHonorModalViews = null, ) - val installer = xcuitest.installer.LocalXCTestInstaller( + installer = xcuitest.installer.LocalXCTestInstaller( deviceId = udid, host = "127.0.0.1", deviceType = util.IOSDeviceType.SIMULATOR, @@ -714,25 +838,8 @@ class IosDriverBackend(private val udid: String) : DriverBackend { warmupErr?.let { throw IllegalStateException("WDA warmup failed after 3 attempts: $it") } } - private fun withReconnect(block: () -> T): T { - return try { - block() - } catch (e: Exception) { - val isIoFailure = generateSequence(e as Throwable) { it.cause } - .any { it is java.io.IOException } - if (!isIoFailure) throw e - reconnectLock.lock() - try { - try { driver.open(); warmup() } - catch (reconnectErr: Exception) { - throw IllegalStateException("WDA reconnect failed: $reconnectErr", e) - } - } finally { - reconnectLock.unlock() - } - block() - } - } + private fun withReconnect(replay: Boolean = true, block: () -> T): T = + recovery.run(replay, block) override fun launch(bundleId: String, clearState: Boolean, env: Map) = withReconnect { runCatching { driver.stopApp(bundleId) } @@ -742,38 +849,33 @@ class IosDriverBackend(private val udid: String) : DriverBackend { override fun terminate(bundleId: String) = withReconnect { driver.stopApp(bundleId) } - override fun tap(x: Int, y: Int) = withReconnect { driver.tap(maestro.Point(x, y)) } + override fun tap(x: Int, y: Int) = withReconnect(replay = false) { driver.tap(maestro.Point(x, y)) } // The second tap request is already queued at the XCTest runner while the // first executes, so the on-device gap collapses to the runner's // turnaround instead of a full transport round trip. Sequential requests // leave a gap wide enough for the app to navigate between the taps. - override fun doubleTap(x: Int, y: Int): Unit = withReconnect { - val point = maestro.Point(x, y) - val firstTap = java.util.concurrent.CompletableFuture.runAsync { driver.tap(point) } - Thread.sleep(40) - driver.tap(point) - firstTap.join() - Unit + override fun doubleTap(x: Int, y: Int): Unit = withReconnect(replay = false) { + overlappedDoubleTap { driver.tap(maestro.Point(x, y)) } } - override fun longPress(x: Int, y: Int) = withReconnect { driver.longPress(maestro.Point(x, y)) } + override fun longPress(x: Int, y: Int) = withReconnect(replay = false) { driver.longPress(maestro.Point(x, y)) } - override fun tapSelector(selector: String) = withReconnect { + override fun tapSelector(selector: String) = withReconnect(replay = false) { val root = driver.contentDescriptor(false) val bounds = findBoundsBySelector(root, selector) ?: return@withReconnect driver.tap(maestro.Point((bounds[0] + bounds[2]) / 2, (bounds[1] + bounds[3]) / 2)) } - override fun inputText(text: String) = withReconnect { driver.inputText(text) } + override fun inputText(text: String) = withReconnect(replay = false) { driver.inputText(text) } - override fun eraseText(characterCount: Int) = withReconnect { driver.eraseText(characterCount) } + override fun eraseText(characterCount: Int) = withReconnect(replay = false) { driver.eraseText(characterCount) } - override fun swipe(fromX: Int, fromY: Int, toX: Int, toY: Int, durationMillis: Long) = withReconnect { + override fun swipe(fromX: Int, fromY: Int, toX: Int, toY: Int, durationMillis: Long) = withReconnect(replay = false) { driver.swipe(maestro.Point(fromX, fromY), maestro.Point(toX, toY), maxOf(durationMillis, 250L)) } - override fun pressKey(key: String) = withReconnect { + override fun pressKey(key: String) = withReconnect(replay = false) { StubDriverBackend.KEY_MAP[key]?.let { keyCode -> keyCodeToMaestro(keyCode)?.let { driver.pressKey(it) } } @@ -808,6 +910,13 @@ class IosDriverBackend(private val udid: String) : DriverBackend { override fun healthy() = runCatching { driver.contentDescriptor(false); true }.getOrElse { false } override fun metrics(bundleId: String) = MetricsSample(0.0, 0L, 0L) + + // close stops the XCTest runner session (kills the xcodebuild process and + // uninstalls the runner app). Skipping this leaves an orphaned session + // that xcodebuild later restarts, killing the next run's session. + override fun close() { + runCatching { driver.close() } + } } private fun keyCodeToMaestro(adbKeyCode: String): maestro.KeyCode? { diff --git a/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverService.kt b/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverService.kt index 3fc703d..cfe11d0 100644 --- a/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverService.kt +++ b/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverService.kt @@ -186,11 +186,29 @@ class DriverService( } } + // shutdown runs on the JVM shutdown path (SIGTERM from the runner). It + // terminates the app under test so the simulator is not left showing a + // stale session, then closes the backend so the iOS XCTest runner process + // dies with us instead of being orphaned. + fun shutdown() { + runCatching { launchedBundleId.getAndSet(null)?.let { backend.terminate(it) } } + runCatching { backend.close() } + } + private inline fun runRpc(observer: StreamObserver, block: () -> T) { try { observer.onNext(block()) observer.onCompleted() - } catch (cause: Exception) { + } catch (cause: io.grpc.StatusRuntimeException) { + // A backend that already chose a status code (e.g. UNAVAILABLE for + // a dropped-mid-action connection) keeps it, so the runner can + // tell transient failures from fatal ones. + observer.onError(cause) + } catch (cause: Throwable) { + // Throwable, not Exception: the vendored iOS client throws + // failures that do not extend Exception, and an uncaught one + // kills the RPC as a channel-level Unknown instead of a status + // the runner can classify. observer.onError(io.grpc.Status.INTERNAL.withDescription(cause.toString()) .withCause(cause).asRuntimeException()) } diff --git a/sidecar/src/main/kotlin/dev/sanderling/sidecar/Main.kt b/sidecar/src/main/kotlin/dev/sanderling/sidecar/Main.kt index 4aeed6c..7cd3c1d 100644 --- a/sidecar/src/main/kotlin/dev/sanderling/sidecar/Main.kt +++ b/sidecar/src/main/kotlin/dev/sanderling/sidecar/Main.kt @@ -30,11 +30,33 @@ class SidecarServer( fun stop() { grpcServer?.shutdown() + service.shutdown() shutdownLatch.countDown() } } +// quietExpectedDriverNoise silences vendored loggers whose ERROR lines fire +// on expected paths: CommandLineUtils logs every nonzero simctl exit even +// when the caller absorbs it (terminating an app that is not running), and +// XCTestDriverClient logs every non-2xx response including gesture +// collisions the double-tap path retries. AndroidDriver logs an ERROR for +// every view-hierarchy fetch the on-device server cancels or times out, which +// happens routinely while the UI is animating; stabilitySnapshot polls the +// hierarchy on a sub-second cadence and swallows those throws to keep polling, +// so each absorbed failure produces a log line with no effect on the run. +// Real failures still reach the runner as gRPC status errors, so nothing is +// lost from run output. +private fun quietExpectedDriverNoise() { + org.apache.logging.log4j.core.config.Configurator.setLevel( + "util.CommandLineUtils", org.apache.logging.log4j.Level.OFF) + org.apache.logging.log4j.core.config.Configurator.setLevel( + "xcuitest.XCTestDriverClient", org.apache.logging.log4j.Level.OFF) + org.apache.logging.log4j.core.config.Configurator.setLevel( + "maestro.drivers.AndroidDriver", org.apache.logging.log4j.Level.OFF) +} + fun main(arguments: Array) { + quietExpectedDriverNoise() val port = arguments.indexOf("--port").let { index -> if (index >= 0 && index + 1 < arguments.size) arguments[index + 1].toInt() else 0 } diff --git a/sidecar/src/test/kotlin/dev/sanderling/sidecar/DriverServiceTest.kt b/sidecar/src/test/kotlin/dev/sanderling/sidecar/DriverServiceTest.kt index ab52bab..77f148f 100644 --- a/sidecar/src/test/kotlin/dev/sanderling/sidecar/DriverServiceTest.kt +++ b/sidecar/src/test/kotlin/dev/sanderling/sidecar/DriverServiceTest.kt @@ -54,6 +54,45 @@ class DriverServiceTest { assertEquals(null, backend.lastBundleId) } + @Test fun shutdownTerminatesLaunchedAppAndClosesBackend() { + var terminated: String? = null + var closed = false + val backend = object : DriverBackend by StubDriverBackend("android") { + override fun terminate(bundleId: String) { terminated = bundleId } + override fun close() { closed = true } + } + val serverName = InProcessServerBuilder.generateName() + val service = DriverService(platform = "android", backend = backend) + grpcCleanup.register( + InProcessServerBuilder.forName(serverName).directExecutor().addService(service).build().start() + ) + val channel: ManagedChannel = grpcCleanup.register( + InProcessChannelBuilder.forName(serverName).directExecutor().build() + ) + val client = DriverGrpc.newBlockingStub(channel) + + client.launch(LaunchRequest.newBuilder().setBundleId("com.example").build()) + service.shutdown() + + assertEquals("com.example", terminated) + assertTrue(closed) + } + + @Test fun shutdownWithoutLaunchedAppStillClosesBackend() { + var terminated: String? = null + var closed = false + val backend = object : DriverBackend by StubDriverBackend("android") { + override fun terminate(bundleId: String) { terminated = bundleId } + override fun close() { closed = true } + } + val service = DriverService(platform = "android", backend = backend) + + service.shutdown() + + assertEquals(null, terminated) + assertTrue(closed) + } + @Test fun tapForwardsCoordinates() { val backend = StubDriverBackend("android") val client = newClient(backend) @@ -83,6 +122,103 @@ class DriverServiceTest { assertEquals("hello world", backend.lastInputText) } + // A backend that already chose a status code (the iOS backend surfaces + // UNAVAILABLE when the connection dropped mid-action) must keep it, so + // the runner can tell transient failures from fatal ones. + @Test fun backendStatusCodePassesThrough() { + val backend = object : DriverBackend by StubDriverBackend("android") { + override fun inputText(text: String) { + throw io.grpc.Status.UNAVAILABLE + .withDescription("connection dropped mid-action") + .asRuntimeException() + } + } + val client = newClient(backend) + + val thrown = kotlin.test.assertFailsWith { + client.inputText(Text.newBuilder().setValue("hello").build()) + } + assertEquals(io.grpc.Status.Code.UNAVAILABLE, thrown.status.code) + } + + // The vendored iOS client throws failures that do not extend Exception; + // they must still map to a status error instead of killing the RPC as a + // channel-level Unknown the runner cannot classify. + @Test fun nonExceptionThrowableMapsToInternal() { + val backend = object : DriverBackend by StubDriverBackend("android") { + override fun inputText(text: String) { + throw Throwable("only one gesture can be performed at a time") + } + } + val client = newClient(backend) + + val thrown = kotlin.test.assertFailsWith { + client.inputText(Text.newBuilder().setValue("hello").build()) + } + assertEquals(io.grpc.Status.Code.INTERNAL, thrown.status.code) + assertTrue(thrown.status.description.orEmpty().contains("only one gesture")) + } + + @Test fun reapOrphanIosRunnersKillsStrayXcodebuildAndRunnerApp() { + val commands = mutableListOf>() + val reaped = reapOrphanIosRunners("UDID-1234") { command -> + commands.add(command) + 0 + } + assertTrue(reaped) + assertEquals(2, commands.size) + assertEquals("pkill", commands[0][0]) + assertTrue(commands[0][2].contains("test-without-building")) + assertTrue(commands[0][2].contains("UDID-1234")) + assertEquals(listOf("xcrun", "simctl", "terminate", "UDID-1234", IOS_XCTEST_RUNNER_BUNDLE_ID), commands[1]) + } + + @Test fun reapOrphanIosRunnersReportsNothingFound() { + val reaped = reapOrphanIosRunners("UDID-1234") { 1 } + assertEquals(false, reaped) + } + + @Test fun overlappedDoubleTapLandsTwoTaps() { + val invocations = java.util.concurrent.atomic.AtomicInteger(0) + overlappedDoubleTap { invocations.incrementAndGet() } + assertEquals(2, invocations.get()) + } + + @Test fun overlappedDoubleTapRetriesSequentiallyOnGestureCollision() { + val invocations = java.util.concurrent.atomic.AtomicInteger(0) + val inFlight = java.util.concurrent.atomic.AtomicBoolean(false) + // Mimic the XCTest runner: a tap issued while another gesture is + // still executing fails instead of queuing. + val tapAction = { + if (!inFlight.compareAndSet(false, true)) { + throw IllegalStateException("only one gesture can be performed at a time") + } + invocations.incrementAndGet() + Thread.sleep(150) + inFlight.set(false) + } + overlappedDoubleTap(tapAction) + assertEquals(2, invocations.get()) + } + + @Test fun overlappedDoubleTapRetriesWhenFirstLegCollides() { + val landed = java.util.concurrent.atomic.AtomicInteger(0) + val failedFirst = java.util.concurrent.atomic.AtomicBoolean(false) + // The async first tap loses the race and collides; the second tap + // succeeds. The collision must be absorbed with a sequential retry, + // not propagated out of the join. + val tapAction = { + if (failedFirst.compareAndSet(false, true)) { + Thread.sleep(60) + throw IllegalStateException("only one gesture can be performed at a time") + } + landed.incrementAndGet() + Unit + } + overlappedDoubleTap(tapAction) + assertEquals(2, landed.get()) + } + @Test fun doubleTapDefaultComposesTwoTaps() { // Interface delegation would bind the default doubleTap to the // delegate, bypassing the tap override, so implement the interface diff --git a/sidecar/src/test/kotlin/dev/sanderling/sidecar/WdaRecoveryTest.kt b/sidecar/src/test/kotlin/dev/sanderling/sidecar/WdaRecoveryTest.kt new file mode 100644 index 0000000..e3eb3d8 --- /dev/null +++ b/sidecar/src/test/kotlin/dev/sanderling/sidecar/WdaRecoveryTest.kt @@ -0,0 +1,135 @@ +package dev.sanderling.sidecar + +import java.io.IOException +import java.util.concurrent.CountDownLatch +import java.util.concurrent.atomic.AtomicBoolean +import java.util.concurrent.atomic.AtomicInteger +import kotlin.concurrent.thread +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertTrue + +class WdaRecoveryTest { + + private fun recovery( + isAlive: () -> Boolean, + restart: () -> Unit, + ) = WdaRecovery(isAlive = isAlive, restart = restart, log = {}) + + @Test fun aliveChannelSkipsRestartAndRetriesReads() { + val restarts = AtomicInteger(0) + val recovery = recovery(isAlive = { true }, restart = { restarts.incrementAndGet() }) + var calls = 0 + + val result = recovery.run(replay = true) { + calls++ + if (calls == 1) throw IOException("connection reset") + "ok" + } + + assertEquals("ok", result) + assertEquals(2, calls) + assertEquals(0, restarts.get()) + } + + @Test fun aliveChannelSurfacesUnavailableForActions() { + val restarts = AtomicInteger(0) + val recovery = recovery(isAlive = { true }, restart = { restarts.incrementAndGet() }) + + val thrown = assertFailsWith { + recovery.run(replay = false) { throw IOException("connection reset") } + } + + assertEquals(io.grpc.Status.Code.UNAVAILABLE, thrown.status.code) + assertEquals(0, restarts.get()) + } + + @Test fun deadChannelRestartsOnceThenRetries() { + val alive = AtomicBoolean(false) + val restarts = AtomicInteger(0) + val recovery = recovery( + isAlive = { alive.get() }, + restart = { + restarts.incrementAndGet() + alive.set(true) + }, + ) + var calls = 0 + + val result = recovery.run(replay = true) { + calls++ + if (calls == 1) throw IOException("connection refused") + "ok" + } + + assertEquals("ok", result) + assertEquals(1, restarts.get()) + } + + @Test fun concurrentFailuresRestartOnly() { + val alive = AtomicBoolean(false) + val restarts = AtomicInteger(0) + val recovery = recovery( + isAlive = { alive.get() }, + restart = { + Thread.sleep(100) + restarts.incrementAndGet() + alive.set(true) + }, + ) + val started = CountDownLatch(2) + val threads = (1..2).map { + thread { + started.countDown() + started.await() + recovery.run(replay = true) { + if (!alive.get()) throw IOException("connection refused") + "ok" + } + } + } + threads.forEach { it.join() } + + assertEquals(1, restarts.get()) + } + + @Test fun restartFailureSurfacesWdaReconnectFailed() { + val recovery = recovery( + isAlive = { false }, + restart = { throw IllegalStateException("xcodebuild died") }, + ) + + val thrown = assertFailsWith { + recovery.run(replay = true) { throw IOException("connection refused") } + } + + assertTrue(thrown.message.orEmpty().contains("WDA reconnect failed")) + } + + @Test fun nonIoFailurePropagatesWithoutRecovery() { + val restarts = AtomicInteger(0) + val probes = AtomicInteger(0) + val recovery = recovery( + isAlive = { probes.incrementAndGet() > 0 }, + restart = { restarts.incrementAndGet() }, + ) + + assertFailsWith { + recovery.run(replay = true) { throw IllegalArgumentException("bad selector") } + } + + assertEquals(0, restarts.get()) + assertEquals(0, probes.get()) + } + + @Test fun readRetryFailureSurfacesUnavailable() { + val recovery = recovery(isAlive = { true }, restart = {}) + + val thrown = assertFailsWith { + recovery.run(replay = true) { throw IOException("connection reset") } + } + + assertEquals(io.grpc.Status.Code.UNAVAILABLE, thrown.status.code) + } +}