From a24519412058587bc4be317be3f43ceca8603cdc Mon Sep 17 00:00:00 2001 From: PJ Date: Sun, 16 Aug 2026 17:42:00 +0530 Subject: [PATCH] fix(sidecar): map the driver's refusals onto the gesture errors OUT_OF_RANGE becomes ErrGestureUndelivered on tap, long press, double tap, swipe and the selector fallback; NOT_FOUND on TapSelector becomes ErrSelectorMatchedNothing. without this the runner reads either as a plain apply failure and counts it toward the failure streak. --- internal/driver/sidecar/client.go | 26 ++++++-- internal/driver/sidecar/client_test.go | 84 +++++++++++++++++++++++++- 2 files changed, 103 insertions(+), 7 deletions(-) diff --git a/internal/driver/sidecar/client.go b/internal/driver/sidecar/client.go index 0214b29..1673103 100644 --- a/internal/driver/sidecar/client.go +++ b/internal/driver/sidecar/client.go @@ -8,7 +8,9 @@ import ( "time" "google.golang.org/grpc" + "google.golang.org/grpc/codes" "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/status" "github.com/priyanshujain/sanderling/internal/android" "github.com/priyanshujain/sanderling/internal/driver" @@ -129,19 +131,33 @@ func (c *Client) Terminate(ctx context.Context) error { return err } +// asGestureError translates the sidecar's OUT_OF_RANGE refusal of a point the +// device has no surface under into driver.ErrGestureUndelivered. Only the +// sidecar knows the screen extent, and the platform silently drops such a +// gesture, so without this the step reads as an action that landed. +func asGestureError(err error) error { + if status.Code(err) != codes.OutOfRange { + return err + } + return fmt.Errorf("%w: %s", driver.ErrGestureUndelivered, status.Convert(err).Message()) +} + func (c *Client) Tap(ctx context.Context, x, y int) error { _, err := c.stub.Tap(ctx, &driverpb.Point{X: int32(x), Y: int32(y)}) - return err + return asGestureError(err) } func (c *Client) LongPress(ctx context.Context, x, y int) error { _, err := c.stub.LongPress(ctx, &driverpb.Point{X: int32(x), Y: int32(y)}) - return err + return asGestureError(err) } func (c *Client) TapSelector(ctx context.Context, selector string) error { _, err := c.stub.TapSelector(ctx, &driverpb.Selector{Value: selector}) - return err + if status.Code(err) == codes.NotFound { + return fmt.Errorf("%w: %s", driver.ErrSelectorMatchedNothing, status.Convert(err).Message()) + } + return asGestureError(err) } // doubleTapGap is the inter-tap delay for the selector fallback: short enough @@ -155,7 +171,7 @@ const doubleTapGap = 50 * time.Millisecond // navigation to interleave between the taps. func (c *Client) DoubleTap(ctx context.Context, x, y int) error { _, err := c.stub.DoubleTap(ctx, &driverpb.Point{X: int32(x), Y: int32(y)}) - return err + return asGestureError(err) } func (c *Client) DoubleTapSelector(ctx context.Context, selector string) error { @@ -192,7 +208,7 @@ func (c *Client) Swipe(ctx context.Context, fromX, fromY, toX, toY int, duration To: &driverpb.Point{X: int32(toX), Y: int32(toY)}, DurationMillis: duration.Milliseconds(), }) - return err + return asGestureError(err) } func (c *Client) PressKey(ctx context.Context, key string) error { diff --git a/internal/driver/sidecar/client_test.go b/internal/driver/sidecar/client_test.go index 2cee6c6..d9c5693 100644 --- a/internal/driver/sidecar/client_test.go +++ b/internal/driver/sidecar/client_test.go @@ -2,6 +2,7 @@ package sidecar import ( "context" + "errors" "io" "net" "strings" @@ -13,6 +14,7 @@ import ( "google.golang.org/grpc/codes" "google.golang.org/grpc/status" + "github.com/priyanshujain/sanderling/internal/driver" driverpb "github.com/priyanshujain/sanderling/proto/driverpb" ) @@ -46,8 +48,10 @@ type fakeServer struct { logEntries []*driverpb.LogEntry metrics *driverpb.MetricsResponse - healthError error - tapError error + healthError error + tapError error + gestureError error + selectorError error } func (s *fakeServer) Health(_ context.Context, _ *driverpb.Empty) (*driverpb.HealthStatus, error) { @@ -85,6 +89,9 @@ func (s *fakeServer) Tap(_ context.Context, point *driverpb.Point) (*driverpb.Em if s.tapError != nil { return nil, s.tapError } + if s.gestureError != nil { + return nil, s.gestureError + } s.taps = append(s.taps, point.GetX(), point.GetY()) return &driverpb.Empty{}, nil } @@ -92,6 +99,9 @@ func (s *fakeServer) Tap(_ context.Context, point *driverpb.Point) (*driverpb.Em func (s *fakeServer) TapSelector(_ context.Context, selector *driverpb.Selector) (*driverpb.Empty, error) { s.mutex.Lock() defer s.mutex.Unlock() + if s.selectorError != nil { + return nil, s.selectorError + } s.tapSelectors = append(s.tapSelectors, selector.GetValue()) return &driverpb.Empty{}, nil } @@ -113,6 +123,9 @@ func (s *fakeServer) WaitForIdle(_ context.Context, duration *driverpb.Duration) func (s *fakeServer) LongPress(_ context.Context, point *driverpb.Point) (*driverpb.Empty, error) { s.mutex.Lock() defer s.mutex.Unlock() + if s.gestureError != nil { + return nil, s.gestureError + } s.longPresses = append(s.longPresses, point.GetX(), point.GetY()) return &driverpb.Empty{}, nil } @@ -120,6 +133,9 @@ func (s *fakeServer) LongPress(_ context.Context, point *driverpb.Point) (*drive func (s *fakeServer) DoubleTap(_ context.Context, point *driverpb.Point) (*driverpb.Empty, error) { s.mutex.Lock() defer s.mutex.Unlock() + if s.gestureError != nil { + return nil, s.gestureError + } s.doubleTaps = append(s.doubleTaps, point.GetX(), point.GetY()) return &driverpb.Empty{}, nil } @@ -127,6 +143,9 @@ func (s *fakeServer) DoubleTap(_ context.Context, point *driverpb.Point) (*drive func (s *fakeServer) Swipe(_ context.Context, request *driverpb.SwipeRequest) (*driverpb.Empty, error) { s.mutex.Lock() defer s.mutex.Unlock() + if s.gestureError != nil { + return nil, s.gestureError + } s.swipes = append(s.swipes, request) return &driverpb.Empty{}, nil } @@ -682,3 +701,64 @@ func TestClient_RecentLogsSinceBranches(t *testing.T) { }) } } + +// TestClient_SelectorThatMatchesNothingReportsIt keeps a by-selector tap that +// named no element distinguishable from a gesture that reached no point: the +// sidecar refuses it with NOT_FOUND and the client names the resolution +// failure rather than the delivery one. +func TestClient_SelectorThatMatchesNothingReportsIt(t *testing.T) { + taps := map[string]func(*Client) error{ + "TapSelector": func(c *Client) error { return c.TapSelector(context.Background(), "id:absent") }, + "DoubleTapSelector": func(c *Client) error { return c.DoubleTapSelector(context.Background(), "id:absent") }, + } + for name, tap := range taps { + t.Run(name, func(t *testing.T) { + state := newHarness(t) + state.fake.mutex.Lock() + state.fake.selectorError = status.Error( + codes.NotFound, "selector id:absent matched no element") + state.fake.mutex.Unlock() + client, _ := Dial(state.address) + defer client.Close() + + err := tap(client) + if !errors.Is(err, driver.ErrSelectorMatchedNothing) { + t.Fatalf("err = %v, want ErrSelectorMatchedNothing", err) + } + if errors.Is(err, driver.ErrGestureUndelivered) { + t.Fatal("a selector that matched nothing must not read as an undelivered gesture") + } + }) + } +} + +// TestClient_OffScreenGestureReportsUndelivered holds the Android client to the +// contract Chrome and iOS already meet: a gesture the device had no surface +// under reports driver.ErrGestureUndelivered rather than returning nil and +// letting the step read as an action that landed. +func TestClient_OffScreenGestureReportsUndelivered(t *testing.T) { + gestures := map[string]func(*Client) error{ + "Tap": func(c *Client) error { return c.Tap(context.Background(), 160, 900) }, + "DoubleTap": func(c *Client) error { return c.DoubleTap(context.Background(), 160, 900) }, + "LongPress": func(c *Client) error { return c.LongPress(context.Background(), 160, 900) }, + "Swipe": func(c *Client) error { + return c.Swipe(context.Background(), 160, 900, 160, 700, time.Second) + }, + } + for name, gesture := range gestures { + t.Run(name, func(t *testing.T) { + state := newHarness(t) + state.fake.mutex.Lock() + state.fake.gestureError = status.Error( + codes.OutOfRange, "gesture point (160,900) is outside the 320x640 screen") + state.fake.mutex.Unlock() + client, _ := Dial(state.address) + defer client.Close() + + err := gesture(client) + if !errors.Is(err, driver.ErrGestureUndelivered) { + t.Fatalf("err = %v, want ErrGestureUndelivered", err) + } + }) + } +}