diff --git a/internal/runner/runner.go b/internal/runner/runner.go index 5e322d8..e30ad04 100644 --- a/internal/runner/runner.go +++ b/internal/runner/runner.go @@ -240,7 +240,7 @@ func Run(ctx context.Context, options Options) (Summary, error) { } applySkipped := false - actionSkipped := "" + var actionSkipped actionSkipReason if nextErr == nil && !appIsForeground(ctx, options) { // The app left the foreground between observe and apply (a prior // action's gesture settling late, or an async navigation). The @@ -253,7 +253,8 @@ func Run(ctx context.Context, options Options) (Summary, error) { actionSkipped = actionSkippedForeground lastAction = nil } else if nextErr == nil { - if err := applyAction(ctx, options.Driver, nextAction, tree); err != nil { + notDispatched, err := applyAction(ctx, options.Driver, nextAction, tree) + if err != nil { if isWDADrop(err) { return summary, fmt.Errorf("step %d: the iOS XCTest runner could not be restarted - re-run the test: %w", stepIndex, err) } @@ -275,6 +276,17 @@ func Run(ctx context.Context, options Options) (Summary, error) { applySkipped = true actionSkipped = actionSkippedApplyError lastAction = nil + } else if notDispatched != "" { + // The action was chosen but nothing reached the driver, so the + // screen is exactly the one already verified: the step stays + // non-transitional and only records why it acted on nothing. + // The apply-failure streak is left alone; a step that never + // reached the device says nothing about the device's health. + logger.Warn("action not dispatched", + "step", stepIndex, "action", nextAction.Kind, "reason", notDispatched) + applySkipped = true + actionSkipped = notDispatched + lastAction = nil } else { consecutiveApplyFailures = 0 actionCopy := nextAction @@ -295,7 +307,7 @@ func Run(ctx context.Context, options Options) (Summary, error) { Metrics: metrics, ExtractorChanges: extractorChanges, Transitional: transitional, - ActionSkipped: actionSkipped, + ActionSkipped: string(actionSkipped), SkippedVerification: skippedVerification, Witnesses: witnesses, } @@ -572,34 +584,41 @@ func settleForForeground(ctx context.Context, options Options) { cancel() } -func applyAction(ctx context.Context, drv driver.DeviceDriver, action verifier.Action, tree *hierarchy.Tree) error { +// applyAction dispatches one chosen action to the driver. The returned reason is +// empty exactly when the driver was called; a non-empty reason means nothing was +// dispatched and names why, so the step can record that it acted on nothing +// instead of showing a next_action that looks executed. +func applyAction(ctx context.Context, drv driver.DeviceDriver, action verifier.Action, tree *hierarchy.Tree) (actionSkipReason, error) { switch action.Kind { case verifier.ActionKindTap: x, y, ok := resolveCoordinates(action, tree) if !ok { if action.On == "" { - return nil + return actionSkippedNoTarget, nil } - return drv.TapSelector(ctx, action.On) + return "", drv.TapSelector(ctx, action.On) } - return drv.Tap(ctx, x, y) + return "", drv.Tap(ctx, x, y) case verifier.ActionKindDoubleTap: x, y, ok := resolveCoordinates(action, tree) if !ok { if action.On == "" { - return nil + return actionSkippedNoTarget, nil } - return drv.DoubleTapSelector(ctx, action.On) + return "", drv.DoubleTapSelector(ctx, action.On) } - return drv.DoubleTap(ctx, x, y) + return "", drv.DoubleTap(ctx, x, y) case verifier.ActionKindLongPress: x, y, ok := resolveCoordinates(action, tree) if !ok { - // No long-press-by-selector RPC exists, so an unresolved target is - // nothing we can dispatch; skip rather than error. - return nil + if action.On == "" { + return actionSkippedNoTarget, nil + } + // No long-press-by-selector RPC exists, so a selector that resolves + // to no coordinates is nothing we can dispatch. + return actionSkippedUnresolvedSelector, nil } - return drv.LongPress(ctx, x, y) + return "", drv.LongPress(ctx, x, y) case verifier.ActionKindScroll: fromX, fromY, toX, toY := scrollEndpoints(action, tree) fromX, fromY, toX, toY = clampGestureToSafeArea(fromX, fromY, toX, toY, screenBounds(tree)) @@ -607,17 +626,17 @@ func applyAction(ctx context.Context, drv driver.DeviceDriver, action verifier.A if duration <= 0 { duration = 300 * time.Millisecond } - return drv.Swipe(ctx, fromX, fromY, toX, toY, duration) + 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 + return "", err } tapped = true } else if action.On != "" { if err := drv.TapSelector(ctx, action.On); err != nil { - return err + return "", err } tapped = true } @@ -631,7 +650,7 @@ func applyAction(ctx context.Context, drv driver.DeviceDriver, action verifier.A select { case <-ctx.Done(): timer.Stop() - return ctx.Err() + return "", ctx.Err() case <-timer.C: } } @@ -643,38 +662,38 @@ func applyAction(ctx context.Context, drv driver.DeviceDriver, action verifier.A if !inputReplacesText(drv) { if count := existingTextLength(action, tree); count > 0 { if err := drv.EraseText(ctx, count); err != nil { - return err + return "", err } } } - return drv.InputText(ctx, action.Text) + return "", drv.InputText(ctx, action.Text) case verifier.ActionKindSwipe: duration := time.Duration(action.DurationMillis) * time.Millisecond if duration <= 0 { duration = 250 * time.Millisecond } fromX, fromY, toX, toY := clampGestureToSafeArea(action.FromX, action.FromY, action.ToX, action.ToY, screenBounds(tree)) - return drv.Swipe(ctx, fromX, fromY, toX, toY, duration) + return "", drv.Swipe(ctx, fromX, fromY, toX, toY, duration) case verifier.ActionKindPressKey: if action.Key == "" { - return nil + return actionSkippedMissingKey, nil } - return drv.PressKey(ctx, action.Key) + return "", drv.PressKey(ctx, action.Key) case verifier.ActionKindWait: duration := time.Duration(action.DurationMillis) * time.Millisecond if duration <= 0 { - return nil + return actionSkippedZeroDurationWait, nil } timer := time.NewTimer(duration) defer timer.Stop() select { case <-ctx.Done(): - return ctx.Err() + return "", ctx.Err() case <-timer.C: - return nil + return "", nil } default: - return fmt.Errorf("unknown action kind %q", action.Kind) + return "", fmt.Errorf("unknown action kind %q", action.Kind) } } @@ -1082,12 +1101,23 @@ func encodeResiduals(residuals map[string]ltl.Formula) (map[string]json.RawMessa return encoded, firstErr } -// Reasons a chosen action was never dispatched, recorded on the step so a count -// of executed actions is not inflated by the next_action of a step that acted on -// nothing. +// actionSkipReason names why a chosen action was never dispatched. It is +// recorded on the step so a count of executed actions is not inflated by the +// next_action of a step that acted on nothing. Empty means the action ran. +type actionSkipReason string + const ( - actionSkippedForeground = "app_left_foreground" - actionSkippedApplyError = "apply_error" + actionSkippedForeground actionSkipReason = "app_left_foreground" + actionSkippedApplyError actionSkipReason = "apply_error" + // The action named no target at all: no selector, and no usable + // coordinates (a web candidate scrolled out of the viewport carries + // negative ones). + actionSkippedNoTarget actionSkipReason = "no_target" + // The action named a selector that resolved to no on-screen coordinates, + // and its verb has no by-selector dispatch to fall back to. + actionSkippedUnresolvedSelector actionSkipReason = "unresolved_selector" + actionSkippedMissingKey actionSkipReason = "missing_key" + actionSkippedZeroDurationWait actionSkipReason = "zero_duration_wait" ) // maxConsecutiveApplyFailures bounds how many transient apply failures in a diff --git a/internal/runner/runner_test.go b/internal/runner/runner_test.go index 35a9847..859497b 100644 --- a/internal/runner/runner_test.go +++ b/internal/runner/runner_test.go @@ -36,6 +36,16 @@ globalThis.properties = { globalThis.actions = actions(() => [Tap({ on: "id:next" })]); ` +// zeroWaitSpec's only action is a Wait the runner cannot perform, so every step +// chooses an action that never reaches the device. +const zeroWaitSpec = ` +import { actions, always, Wait } from "@sanderling/spec"; +globalThis.properties = { + alwaysHolds: always(() => true), +}; +globalThis.actions = actions(() => [Wait({ durationMillis: 0 })]); +` + const violationSpec = ` import { actions, always } from "@sanderling/spec"; globalThis.properties = { @@ -61,6 +71,17 @@ func fastFocusSettle(t *testing.T) { t.Cleanup(func() { focusTapSettle = prev }) } +func mustDispatch(t *testing.T, drv driver.DeviceDriver, action verifier.Action, tree *hierarchy.Tree) { + t.Helper() + skipped, err := applyAction(context.Background(), drv, action, tree) + if err != nil { + t.Fatalf("applyAction: %v", err) + } + if skipped != "" { + t.Fatalf("applyAction reported %q: the action never reached the driver", skipped) + } +} + // bundleSpec compiles an authored TS spec with the goja runtime entry so the // loaded bundle installs __sanderlingNextAction__ (the shared picker). func bundleSpec(t *testing.T, specSource string) string { @@ -699,9 +720,7 @@ 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 { - t.Fatalf("applyAction: %v", err) - } + mustDispatch(t, driverMock, action, tree) // The post-tap settle is now a brief internal sleep, not a WaitForIdle RPC, // so the recorded driver actions are tap, erase, input. actions := driverMock.Actions() @@ -726,9 +745,7 @@ func TestApplyAction_InputTextWithoutTargetSkipsFocusTap(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); err != nil { - t.Fatalf("applyAction: %v", err) - } + mustDispatch(t, driverMock, action, nil) actions := driverMock.Actions() if len(actions) != 1 || actions[0].Kind != mockdriver.ActionInputText { t.Errorf("no target: want input_text only (no focus tap), got %v", actions) @@ -750,9 +767,7 @@ func TestApplyAction_InputTextSkipsEraseForReplacingDriver(t *testing.T) { driverMock.ReplacesText = true action := verifier.Action{Kind: verifier.ActionKindInputText, On: "id:username", Text: "alice"} - if err := applyAction(context.Background(), driverMock, action, tree); err != nil { - t.Fatalf("applyAction: %v", err) - } + mustDispatch(t, driverMock, action, tree) if containsAction(driverMock.Actions(), mockdriver.ActionEraseText, "") { t.Errorf("replacing driver must not be asked to erase: %v", driverMock.Actions()) } @@ -772,9 +787,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 { - t.Fatalf("applyAction: %v", err) - } + mustDispatch(t, driverMock, action, tree) if containsAction(driverMock.Actions(), mockdriver.ActionEraseText, "") { t.Errorf("empty field must not be erased: %v", driverMock.Actions()) } @@ -786,7 +799,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) if err == nil { t.Fatalf("expected focus tap failure to surface, got nil") } @@ -799,7 +812,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) if err == nil { t.Fatalf("expected focus tap failure to surface, got nil") } @@ -814,9 +827,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 { - t.Fatalf("apply action: %v", err) - } + mustDispatch(t, driverMock, action, nil) actions := driverMock.Actions() if !containsAction(actions, mockdriver.ActionTap, "") { t.Errorf("expected focus Tap before InputText, got %v", actions) @@ -834,9 +845,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 { - t.Fatalf("apply action: %v", err) - } + mustDispatch(t, driverMock, action, nil) if !containsAction(driverMock.Actions(), mockdriver.ActionTap, "") { t.Errorf("expected focus Tap at (0,0), got %v", driverMock.Actions()) } @@ -846,9 +855,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 { - t.Fatalf("apply action: %v", err) - } + mustDispatch(t, driverMock, action, nil) taps := 0 for _, a := range driverMock.Actions() { if a.Kind == mockdriver.ActionDoubleTap && a.X == 100 && a.Y == 200 { @@ -864,9 +871,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 { - t.Fatalf("apply action: %v", err) - } + mustDispatch(t, driverMock, action, nil) taps := 0 for _, a := range driverMock.Actions() { if a.Kind == mockdriver.ActionDoubleTapSelector && a.Selector == "id:save" { @@ -882,9 +887,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 { - t.Fatalf("apply action: %v", err) - } + mustDispatch(t, driverMock, action, nil) found := false for _, a := range driverMock.Actions() { if a.Kind == mockdriver.ActionLongPress && a.X == 120 && a.Y == 240 { @@ -908,9 +911,7 @@ func TestApplyAction_ScrollWithPrecomputedEndpointsSwipes(t *testing.T) { DurationMillis: 300, } - if err := applyAction(context.Background(), driverMock, action, nil); err != nil { - t.Fatalf("apply action: %v", err) - } + mustDispatch(t, driverMock, action, nil) found := false for _, a := range driverMock.Actions() { if a.Kind == mockdriver.ActionSwipe && a.FromX == 100 && a.FromY == 500 && a.ToX == 100 && a.ToY == 300 { @@ -931,9 +932,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 { - t.Fatalf("apply action: %v", err) - } + mustDispatch(t, driverMock, action, tree) var swipe *mockdriver.Action for i := range driverMock.Actions() { if driverMock.Actions()[i].Kind == mockdriver.ActionSwipe { @@ -963,9 +962,7 @@ func TestApplyAction_ScrollNearTopKeepsDirectionAfterClamp(t *testing.T) { } action := verifier.Action{Kind: verifier.ActionKindScroll, Direction: "up", On: "id:toplist"} - if err := applyAction(context.Background(), driverMock, action, tree); err != nil { - t.Fatalf("apply action: %v", err) - } + mustDispatch(t, driverMock, action, tree) var swipe *mockdriver.Action for i := range driverMock.Actions() { if driverMock.Actions()[i].Kind == mockdriver.ActionSwipe { @@ -994,9 +991,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 { - t.Fatalf("apply action: %v", err) - } + mustDispatch(t, driverMock, action, tree) var swipe *mockdriver.Action for i := range driverMock.Actions() { if driverMock.Actions()[i].Kind == mockdriver.ActionSwipe { @@ -1016,6 +1011,164 @@ func TestApplyAction_ScrollScreenFallback(t *testing.T) { } } +// Every shape applyAction cannot dispatch has to name why. Returning a bare nil +// leaves the step recording a next_action that reached no driver at all, which +// an executed-action count then reads as work done. +func TestApplyAction_NonDispatchPathsReportWhy(t *testing.T) { + tree, err := hierarchy.Parse(`{"attributes":{"resource-id":"root","bounds":"[0,0,400,800]"},"children":[]}`) + if err != nil { + t.Fatalf("parse tree: %v", err) + } + cases := []struct { + name string + action verifier.Action + tree *hierarchy.Tree + want actionSkipReason + }{ + { + name: "tap with neither selector nor coordinates", + action: verifier.Action{Kind: verifier.ActionKindTap, X: -1, Y: -1}, + want: actionSkippedNoTarget, + }, + { + name: "double tap with neither selector nor coordinates", + action: verifier.Action{Kind: verifier.ActionKindDoubleTap, X: -1, Y: -1}, + want: actionSkippedNoTarget, + }, + { + name: "long press with neither selector nor coordinates", + action: verifier.Action{Kind: verifier.ActionKindLongPress, X: -1, Y: -1}, + want: actionSkippedNoTarget, + }, + { + name: "long press whose selector is not on screen", + action: verifier.Action{Kind: verifier.ActionKindLongPress, On: "id:gone"}, + tree: tree, + want: actionSkippedUnresolvedSelector, + }, + { + name: "press key without a key", + action: verifier.Action{Kind: verifier.ActionKindPressKey}, + want: actionSkippedMissingKey, + }, + { + name: "wait without a duration", + action: verifier.Action{Kind: verifier.ActionKindWait}, + want: actionSkippedZeroDurationWait, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + driverMock := mockdriver.New() + skipped, err := applyAction(context.Background(), driverMock, tc.action, tc.tree) + if err != nil { + t.Fatalf("applyAction: %v", err) + } + if skipped != tc.want { + t.Errorf("skip reason = %q, want %q", skipped, tc.want) + } + if len(driverMock.Actions()) != 0 { + t.Errorf("nothing must reach the driver, got %v", driverMock.Actions()) + } + }) + } +} + +// TestRunner_RecordsWhyAChosenActionNeverRan drives a spec whose only action is +// undispatchable and pins that each step says so on its trace line. The step is +// left non-transitional: nothing was dispatched, so the verified screen still +// describes the device. +func TestRunner_RecordsWhyAChosenActionNeverRan(t *testing.T) { + state := newHarnessWithSpec(t, zeroWaitSpec) + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + summary, err := Run(ctx, Options{ + Duration: 5 * time.Second, + IdleTimeout: 20 * time.Millisecond, + MaxSteps: 2, + Driver: state.mock, + Verifier: state.verifier, + TraceWriter: state.writer, + }) + if err != nil { + t.Fatalf("Run: %v", err) + } + if summary.Steps != 2 { + t.Fatalf("Steps = %d, want 2", summary.Steps) + } + lines := readTraceLines(t, state.writer.Directory()) + acted := 0 + for _, line := range lines { + if line.NextAction == nil { + continue + } + acted++ + if line.ActionSkipped != string(actionSkippedZeroDurationWait) { + t.Errorf("step %d action_skipped = %q, want %q", + line.Step, line.ActionSkipped, actionSkippedZeroDurationWait) + } + if line.Transitional { + t.Errorf("step %d marked transitional; nothing was dispatched, so the screen is unchanged", line.Step) + } + } + if acted != 2 { + t.Fatalf("want 2 steps carrying a next_action, got %d", acted) + } +} + +// The other half of the contract: a step whose action really was dispatched +// must carry no reason at all, or every step looks skipped. +func TestRunner_DispatchedActionRecordsNoSkipReason(t *testing.T) { + state := newHarness(t) + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + if _, err := Run(ctx, Options{ + Duration: 5 * time.Second, + IdleTimeout: 20 * time.Millisecond, + MaxSteps: 2, + Driver: state.mock, + Verifier: state.verifier, + TraceWriter: state.writer, + }); err != nil { + t.Fatalf("Run: %v", err) + } + if !containsAction(state.mock.Actions(), mockdriver.ActionTapSelector, "id:next") { + t.Fatalf("fixture tap never dispatched, got %v", state.mock.Actions()) + } + for _, line := range readTraceLines(t, state.writer.Directory()) { + if line.NextAction != nil && line.ActionSkipped != "" { + t.Errorf("step %d recorded action_skipped=%q for a dispatched action", + line.Step, line.ActionSkipped) + } + } +} + +type traceStepLine struct { + Step int `json:"step"` + NextAction *trace.Action `json:"next_action"` + ActionSkipped string `json:"action_skipped"` + Transitional bool `json:"transitional"` +} + +func readTraceLines(t *testing.T, directory string) []traceStepLine { + t.Helper() + body, err := os.ReadFile(filepath.Join(directory, "trace.jsonl")) + if err != nil { + t.Fatal(err) + } + var lines []traceStepLine + for _, raw := range bytes.Split(bytes.TrimSpace(body), []byte("\n")) { + var line traceStepLine + if err := json.Unmarshal(raw, &line); err != nil { + t.Fatalf("decode trace line: %v", err) + } + lines = append(lines, line) + } + return lines +} + func TestRunner_ParallelFetchCallsAllDriverMethods(t *testing.T) { state := newHarness(t) state.mock.MetricsData = driver.Metrics{CPUPercent: 5.0, HeapBytes: 1024, TotalMemoryBytes: 4096} @@ -1470,7 +1623,7 @@ func TestRunner_TransientApplyErrorMarksTransitional(t *testing.T) { if err := json.Unmarshal(lines[0], &firstSkip); err != nil { t.Fatalf("decode first trace line: %v", err) } - if firstSkip.ActionSkipped != actionSkippedApplyError { + if firstSkip.ActionSkipped != string(actionSkippedApplyError) { t.Errorf("step 1 action_skipped = %q, want %q", firstSkip.ActionSkipped, actionSkippedApplyError) } if len(first.Violations) != 0 { @@ -2023,7 +2176,7 @@ func TestRunner_SkipsActionWhenOverlayStealsFocusAtApplyTime(t *testing.T) { if err := json.Unmarshal(line, &step); err != nil { t.Fatalf("decode trace line: %v", err) } - skipped = skipped || step.ActionSkipped == actionSkippedForeground + skipped = skipped || step.ActionSkipped == string(actionSkippedForeground) } if !skipped { t.Errorf("no step recorded action_skipped=%q, so the undispatched action looks executed", actionSkippedForeground)