From 3cdc0a1a2a81d695dca4cd043d95b30c367fc4aa Mon Sep 17 00:00:00 2001 From: PJ Date: Sun, 16 Aug 2026 17:44:40 +0530 Subject: [PATCH] feat(runner): bound every device call and record the actions that never reached the app observation and apply now run under a timeout, so a driver that stops answering ends the step rather than the run. an undelivered gesture and a selector that matched nothing are recorded as their own skip reasons instead of counting toward the apply-failure streak, a failed observation is counted apart from a screen with nothing on it, and the summary names both. resolveCoordinates hands a point outside the viewport to the driver rather than dropping it: only the driver knows whether it can scroll that point back into reach. exceptions and navigations are collected per step and a Scroll goes to a driver's Scroller when it has one. --- internal/runner/error_surface_trace_test.go | 96 ++++ internal/runner/navigation_trace_test.go | 82 ++++ internal/runner/runner.go | 229 ++++++++-- internal/runner/runner_test.go | 467 ++++++++++++++++++-- 4 files changed, 814 insertions(+), 60 deletions(-) create mode 100644 internal/runner/error_surface_trace_test.go create mode 100644 internal/runner/navigation_trace_test.go diff --git a/internal/runner/error_surface_trace_test.go b/internal/runner/error_surface_trace_test.go new file mode 100644 index 0000000..61991d0 --- /dev/null +++ b/internal/runner/error_surface_trace_test.go @@ -0,0 +1,96 @@ +package runner + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/priyanshujain/sanderling/internal/driver" + mockdriver "github.com/priyanshujain/sanderling/internal/driver/mock" + "github.com/priyanshujain/sanderling/internal/trace" +) + +// throwingDriver reports one captured uncaught error, the way the chrome +// driver reports the page's buffer. +type throwingDriver struct { + *mockdriver.Driver +} + +func (d *throwingDriver) Exceptions( + context.Context, +) ([]driver.Exception, error) { + return []driver.Exception{{ + Class: "TypeError", + Message: "cannot read balance of null", + StackTrace: "at render (app.js:12)", + UnixMillis: 1700000000000, + }}, nil +} + +// TestRunner_LogsAndExceptionsLandInTheTrace covers the error surface an +// offline oracle has no other source for: the default properties read +// state.logs and state.exceptions, and neither used to survive the step. +func TestRunner_LogsAndExceptionsLandInTheTrace(t *testing.T) { + state := newHarness(t) + state.mock.LogEntries = []driver.LogEntry{{ + UnixMillis: 1700000000123, + Level: "E", + Tag: "AndroidRuntime", + Message: "FATAL EXCEPTION: main", + }} + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + if _, err := Run(ctx, Options{ + Duration: 100 * time.Millisecond, + IdleTimeout: 20 * time.Millisecond, + MaxSteps: 1, + Driver: &throwingDriver{Driver: state.mock}, + Verifier: state.verifier, + TraceWriter: state.writer, + }); err != nil { + t.Fatalf("Run: %v", err) + } + + body, err := os.ReadFile( + filepath.Join(state.writer.Directory(), "trace.jsonl"), + ) + if err != nil { + t.Fatal(err) + } + line := strings.SplitN(strings.TrimSpace(string(body)), "\n", 2)[0] + var stored struct { + TraceVersion int `json:"trace_version"` + Logs []trace.LogEntry `json:"logs"` + Exceptions []trace.Exception `json:"exceptions"` + } + if err := json.Unmarshal([]byte(line), &stored); err != nil { + t.Fatalf("decode trace line: %v\n%s", err, line) + } + if stored.TraceVersion != trace.TraceVersion { + t.Errorf( + "trace_version = %d, want %d; an old trace could not be told apart", + stored.TraceVersion, + trace.TraceVersion, + ) + } + want := trace.LogEntry{ + UnixMillis: 1700000000123, + Level: "E", + Tag: "AndroidRuntime", + Message: "FATAL EXCEPTION: main", + } + if len(stored.Logs) != 1 || stored.Logs[0] != want { + t.Errorf("logs on disk = %+v, want [%+v]", stored.Logs, want) + } + if len(stored.Exceptions) != 1 || + stored.Exceptions[0].Class != "TypeError" || + stored.Exceptions[0].Message != "cannot read balance of null" || + stored.Exceptions[0].StackTrace != "at render (app.js:12)" { + t.Errorf("exceptions on disk = %+v", stored.Exceptions) + } +} diff --git a/internal/runner/navigation_trace_test.go b/internal/runner/navigation_trace_test.go new file mode 100644 index 0000000..ebac4ae --- /dev/null +++ b/internal/runner/navigation_trace_test.go @@ -0,0 +1,82 @@ +package runner + +import ( + "bufio" + "context" + "encoding/json" + "os" + "path/filepath" + "testing" + "time" + + "github.com/priyanshujain/sanderling/internal/driver" + mockdriver "github.com/priyanshujain/sanderling/internal/driver/mock" + "github.com/priyanshujain/sanderling/internal/trace" +) + +// navigatingDriver reports one navigation per step, the way a page that submits +// a form or reloads does. +type navigatingDriver struct { + *mockdriver.Driver + url string +} + +func (d *navigatingDriver) Navigations(context.Context) ([]driver.Navigation, error) { + return []driver.Navigation{{URL: d.url, UnixMillis: 1700000000000}}, nil +} + +// A run whose app replaced its own document has to say so on the step it +// happened. Without it the trace shows a generator repeating one action and no +// reason for it, and an analysis cannot tell that from a seed that chose badly. +func TestRunner_TheTraceRecordsThatThePageNavigated(t *testing.T) { + state := newHarness(t) + const url = "http://127.0.0.1/index.html?" + navigating := &navigatingDriver{Driver: state.mock, url: url} + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + if _, err := Run(ctx, Options{ + Duration: 100 * time.Millisecond, + IdleTimeout: 20 * time.Millisecond, + MaxSteps: 3, + Driver: navigating, + Verifier: state.verifier, + TraceWriter: state.writer, + }); err != nil { + t.Fatalf("Run: %v", err) + } + if err := state.writer.Close(); err != nil { + t.Fatalf("close trace: %v", err) + } + + file, err := os.Open(filepath.Join(state.writer.Directory(), "trace.jsonl")) + if err != nil { + t.Fatal(err) + } + defer file.Close() + + recorded := 0 + scanner := bufio.NewScanner(file) + scanner.Buffer(make([]byte, 0, 64*1024), 8*1024*1024) + for scanner.Scan() { + var step struct { + Index int `json:"step"` + Navigations []trace.Navigation `json:"navigations"` + } + if err := json.Unmarshal(scanner.Bytes(), &step); err != nil { + t.Fatalf("trace line decode: %v", err) + } + for _, navigation := range step.Navigations { + recorded++ + if navigation.URL != url { + t.Errorf("step %d records navigation to %q, want %q", step.Index, navigation.URL, url) + } + if navigation.UnixMillis == 0 { + t.Errorf("step %d records a navigation with no timestamp", step.Index) + } + } + } + if recorded == 0 { + t.Fatal("the page navigated on every step and the trace holds no record of it") + } +} diff --git a/internal/runner/runner.go b/internal/runner/runner.go index 97872ca..f26528a 100644 --- a/internal/runner/runner.go +++ b/internal/runner/runner.go @@ -61,6 +61,15 @@ type Summary struct { // could not dispatch, deduped, so the report can flag a spec exercising // gestures this target does not support. UnsupportedVerbs []string + // SkippedActions counts, by reason, the actions a step chose that never + // reached the app. Without it a run that dropped most of what it generated + // reads exactly like one that exercised it: the reasons reach the trace and + // a warn line, and nothing else. + SkippedActions map[string]int + // FailedObservations counts the steps whose device read produced no tree at + // all. Such a step verifies nothing, so a run that failed every observation + // finishes with no violations and reads as a clean one. + FailedObservations int } type ViolationRecord struct { @@ -94,6 +103,8 @@ func Run(ctx context.Context, options Options) (Summary, error) { return Summary{}, err } _, pageExtractors := extractorSource.(webSource) + exceptionReporter, _ := options.Driver.(driver.ExceptionReporter) + navigationReporter, _ := options.Driver.(driver.NavigationReporter) summary := Summary{StartTime: time.Now()} deadline := summary.StartTime.Add(options.Duration) @@ -130,8 +141,10 @@ func Run(ctx context.Context, options Options) (Summary, error) { // gctx is bound to the errgroup so a returned error (or outer // cancellation) propagates to every sibling read rather than leaving - // one blocked on a hung device. - g, gctx := errgroup.WithContext(ctx) + // one blocked on a hung device, and to observationTimeout so a read + // that never answers ends the step instead of the run. + observeCtx, observeCancel := context.WithTimeout(ctx, observationTimeout) + g, gctx := errgroup.WithContext(observeCtx) si := stepIndex // fetchSyncedState issues a single Snapshot RPC so hierarchy and // screenshot describe the same frame, then re-fetches the pair @@ -152,12 +165,18 @@ func Run(ctx context.Context, options Options) (Summary, error) { // All goroutines write to local variables and return nil, so the Wait // error is always nil; ignored intentionally. _ = g.Wait() + observeCancel() + navigations := collectNavigations(ctx, navigationReporter, logger, stepIndex) + + observationError := "" if hierarchyErr != nil { if isWDADrop(hierarchyErr) { return summary, fmt.Errorf("WDA connection permanently lost at step %d - re-run the test: %w", stepIndex, hierarchyErr) } logger.Warn("hierarchy fetch failed", "step", stepIndex, "err", hierarchyErr) + observationError = hierarchyErr.Error() + summary.FailedObservations++ } treeSize := 0 if tree != nil { @@ -189,6 +208,7 @@ func Run(ctx context.Context, options Options) (Summary, error) { var violations []string var extractorChanges map[string]trace.ExtractorChange var witnesses map[string]trace.Witness + var exceptions []verifier.Exception skippedVerification := false if !transitional { // The page-side extractors evaluate only on steps the verifier will @@ -205,7 +225,9 @@ func Run(ctx context.Context, options Options) (Summary, error) { // // lastAction is the same value PushSnapshot hands the goja state // below: the two engines evaluate this step against one action. - v8Overrides, overridesErr := extractorSource.ExtractorOverrides(ctx, lastAction) + overridesCtx, overridesCancel := context.WithTimeout(ctx, observationTimeout) + v8Overrides, overridesErr := extractorSource.ExtractorOverrides(overridesCtx, lastAction) + overridesCancel() if overridesErr != nil { // Not a warning. Without the page's values this step's // extractors keep goja's dump-derived readings while the @@ -214,6 +236,12 @@ func Run(ctx context.Context, options Options) (Summary, error) { // wrong. return summary, fmt.Errorf("step %d extractor overrides: %w", stepIndex, overridesErr) } + exceptions = collectExceptions( + ctx, + exceptionReporter, + logger, + stepIndex, + ) if err := options.Verifier.PushSnapshot(verifier.SnapshotInput{ Tree: tree, ScreenshotPNG: screenshotPNG, @@ -222,6 +250,7 @@ func Run(ctx context.Context, options Options) (Summary, error) { StepIndex: stepIndex, RunStart: summary.StartTime, Logs: logs, + Exceptions: exceptions, }); err != nil { return summary, fmt.Errorf("step %d push: %w", stepIndex, err) } @@ -285,8 +314,33 @@ func Run(ctx context.Context, options Options) (Summary, error) { actionSkipped = actionSkippedForeground lastAction = nil } else if nextErr == nil { - notDispatched, err := applyAction(ctx, options.Driver, nextAction, tree) - if err != nil { + applyCtx, applyCancel := context.WithTimeout(ctx, applyBound(nextAction)) + notDispatched, err := applyAction(applyCtx, options.Driver, nextAction, tree) + applyCancel() + if errors.Is(err, driver.ErrGestureUndelivered) { + // The gesture reached no element, so the app cannot have + // responded to it and the screen is still the one already + // verified. The device is healthy, so the step is neither + // transitional nor part of the apply-failure streak; it records + // that the action landed on nothing, which is what separates it + // from an action the app received and ignored. + logger.Warn("gesture reached no element", + "step", stepIndex, "action", nextAction.Kind, "err", err) + applySkipped = true + actionSkipped = actionSkippedGestureUndelivered + lastAction = nil + } else if errors.Is(err, driver.ErrSelectorMatchedNothing) { + // The selector named no element, so no point was resolved and + // nothing was dispatched. The screen is the one already + // verified and the device is healthy, so this is the same + // non-action the runner records when it cannot resolve a + // selector itself, not a device fault worth a failure streak. + logger.Warn("selector matched no element", + "step", stepIndex, "action", nextAction.Kind, "err", err) + applySkipped = true + actionSkipped = actionSkippedUnresolvedSelector + lastAction = nil + } else 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) } @@ -303,10 +357,14 @@ func Run(ctx context.Context, options Options) (Summary, error) { 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) + actionSkipped = actionSkippedApplyError + if errors.Is(applyCtx.Err(), context.DeadlineExceeded) { + actionSkipped = actionSkippedApplyTimeout + } + logger.Warn("apply error; marking step transitional", + "step", stepIndex, "reason", actionSkipped, "err", err) transitional = true applySkipped = true - actionSkipped = actionSkippedApplyError lastAction = nil } else if notDispatched != "" { // The action was chosen but nothing reached the driver, so the @@ -333,12 +391,16 @@ func Run(ctx context.Context, options Options) (Summary, error) { Timestamp: stepStart, Screen: screen, NextAction: traceAction, + Logs: traceLogs(logs), + Exceptions: traceExceptions(exceptions), + Navigations: navigations, Violations: violations, Hierarchy: tree, Residuals: residuals, Metrics: metrics, ExtractorChanges: extractorChanges, Transitional: transitional, + ObservationError: observationError, ActionSkipped: string(actionSkipped), SkippedVerification: skippedVerification, Witnesses: witnesses, @@ -346,6 +408,12 @@ func Run(ctx context.Context, options Options) (Summary, error) { if err := options.TraceWriter.WriteStep(step); err != nil { return summary, fmt.Errorf("step %d trace: %w", stepIndex, err) } + if actionSkipped != "" { + if summary.SkippedActions == nil { + summary.SkippedActions = map[string]int{} + } + summary.SkippedActions[string(actionSkipped)]++ + } summary.Steps = stepIndex if len(violations) > 0 { summary.Violations = append(summary.Violations, violationRecords(violations, witnesses, stepIndex)...) @@ -410,6 +478,21 @@ func RenderSummary(w io.Writer, summary Summary, platform string) { fmt.Fprintf(w, " step %d: %v\n", violation.StepIndex, violation.Properties) } } + if len(summary.SkippedActions) > 0 { + total := 0 + byReason := make([]string, 0, len(summary.SkippedActions)) + for _, reason := range slices.Sorted(maps.Keys(summary.SkippedActions)) { + total += summary.SkippedActions[reason] + byReason = append(byReason, + fmt.Sprintf("%s %d", reason, summary.SkippedActions[reason])) + } + fmt.Fprintf(w, "%d action(s) never reached the app: %s\n", + total, strings.Join(byReason, ", ")) + } + if summary.FailedObservations > 0 { + fmt.Fprintf(w, "%d step(s) observed nothing: the device state could not be read\n", + summary.FailedObservations) + } if len(summary.UnsupportedVerbs) > 0 { fmt.Fprintf(w, "unsupported on %s: %s\n", platform, strings.Join(summary.UnsupportedVerbs, ", ")) @@ -655,27 +738,18 @@ func applyAction(ctx context.Context, drv driver.DeviceDriver, action verifier.A case verifier.ActionKindTap: x, y, ok := resolveCoordinates(action, tree) if !ok { - if action.On == "" { - return actionSkippedNoTarget, nil - } return "", drv.TapSelector(ctx, action.On) } return "", drv.Tap(ctx, x, y) case verifier.ActionKindDoubleTap: x, y, ok := resolveCoordinates(action, tree) if !ok { - if action.On == "" { - return actionSkippedNoTarget, nil - } return "", drv.DoubleTapSelector(ctx, action.On) } return "", drv.DoubleTap(ctx, x, y) case verifier.ActionKindLongPress: x, y, ok := resolveCoordinates(action, tree) if !ok { - 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 @@ -688,6 +762,9 @@ func applyAction(ctx context.Context, drv driver.DeviceDriver, action verifier.A if duration <= 0 { duration = 300 * time.Millisecond } + if scroller, ok := drv.(driver.Scroller); ok { + return "", scroller.Scroll(ctx, fromX, fromY, toX, toY, duration) + } return "", drv.Swipe(ctx, fromX, fromY, toX, toY, duration) case verifier.ActionKindInputText: tapped := false @@ -782,6 +859,81 @@ func collectLogs(ctx context.Context, drv driver.DeviceDriver, since time.Time) return result } +// collectExceptions reads the app's captured uncaught errors. Like log +// capture it is best-effort: a failed read is warned on rather than ending +// the run, and a driver that cannot report them yields none. +func collectExceptions( + ctx context.Context, + reporter driver.ExceptionReporter, + logger *slog.Logger, + stepIndex int, +) []verifier.Exception { + if reporter == nil { + return nil + } + captured, err := reporter.Exceptions(ctx) + if err != nil { + logger.Warn("exception fetch failed", "step", stepIndex, "err", err) + return nil + } + result := make([]verifier.Exception, 0, len(captured)) + for _, entry := range captured { + result = append(result, verifier.Exception{ + Class: entry.Class, + Message: entry.Message, + StackTrace: entry.StackTrace, + UnixMillis: entry.UnixMillis, + }) + } + return result +} + +func collectNavigations( + ctx context.Context, + reporter driver.NavigationReporter, + logger *slog.Logger, + stepIndex int, +) []trace.Navigation { + if reporter == nil { + return nil + } + observed, err := reporter.Navigations(ctx) + if err != nil { + logger.Warn("navigation fetch failed", "step", stepIndex, "err", err) + return nil + } + records := make([]trace.Navigation, 0, len(observed)) + for _, entry := range observed { + records = append(records, trace.Navigation{URL: entry.URL, UnixMillis: entry.UnixMillis}) + } + if len(records) == 0 { + return nil + } + return records +} + +func traceLogs(entries []verifier.LogEntry) []trace.LogEntry { + if len(entries) == 0 { + return nil + } + result := make([]trace.LogEntry, 0, len(entries)) + for _, entry := range entries { + result = append(result, trace.LogEntry(entry)) + } + return result +} + +func traceExceptions(entries []verifier.Exception) []trace.Exception { + if len(entries) == 0 { + return nil + } + result := make([]trace.Exception, 0, len(entries)) + for _, entry := range entries { + result = append(result, trace.Exception(entry)) + } + 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 { @@ -894,13 +1046,12 @@ func resolveCoordinates(action verifier.Action, tree *hierarchy.Tree) (int, int, // When On is empty, X/Y are authoritative (web V8 path emits coordinates // directly from getBoundingClientRect; the runtime nullifies unresolved // actions upstream so a non-null InputText here always has real coords, - // even at (0,0)). When On is set, prefer the tree lookup so stale coords - // don't leak from earlier ticks. + // even at (0,0)). A point outside the viewport is off screen, not absent: + // only the driver knows whether it can scroll that point back into reach, + // so the judgement belongs there and not here. When On is set, prefer the + // tree lookup so stale coords don't leak from earlier ticks. if action.On == "" { - if action.X >= 0 && action.Y >= 0 { - return action.X, action.Y, true - } - return 0, 0, false + return action.X, action.Y, true } if tree != nil { // An ambiguous selector names several elements while the action's own @@ -1268,15 +1419,21 @@ type actionSkipReason string const ( 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 was dispatched and the driver never came back inside the + // step's bound. Distinct from apply_error, which is a call that answered + // and said no, and from the reasons below, which are actions that never + // reached the device at all. + actionSkippedApplyTimeout actionSkipReason = "apply_timeout" // The action named a selector that resolved to no on-screen coordinates, - // and its verb has no by-selector dispatch to fall back to. + // either because its verb has no by-selector dispatch to fall back to or + // because the driver's own lookup found nothing to tap. actionSkippedUnresolvedSelector actionSkipReason = "unresolved_selector" actionSkippedMissingKey actionSkipReason = "missing_key" actionSkippedZeroDurationWait actionSkipReason = "zero_duration_wait" + // The driver resolved the action's point and found no element there, so + // the gesture was never dispatched. Recorded rather than counted as a + // device fault: a run that acts on nothing has to say so. + actionSkippedGestureUndelivered actionSkipReason = "gesture_undelivered" ) // maxConsecutiveApplyFailures bounds how many transient apply failures in a @@ -1285,6 +1442,24 @@ const ( // would be spent doing nothing. const maxConsecutiveApplyFailures = 3 +// applyTimeout bounds one dispatched action and observationTimeout the device +// reads a step opens with. Options.Duration is a loop condition checked between +// steps and the drivers add no deadline of their own, so without these a call +// that never returns holds the run for as long as the process lives. Both are +// far above any healthy call and far below the timeout a campaign runner puts +// on a whole run. Variables so the timeout tests can shrink them. +var ( + applyTimeout = 60 * time.Second + observationTimeout = 60 * time.Second +) + +// applyBound is how long one dispatched action may take. An action that names +// its own duration carries it on top: the bound exists to end a call that +// stopped answering, not to cut a gesture the spec asked for. +func applyBound(action verifier.Action) time.Duration { + return applyTimeout + time.Duration(action.DurationMillis)*time.Millisecond +} + // 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 diff --git a/internal/runner/runner_test.go b/internal/runner/runner_test.go index 08b2fd1..61c0e90 100644 --- a/internal/runner/runner_test.go +++ b/internal/runner/runner_test.go @@ -47,6 +47,16 @@ globalThis.properties = { globalThis.actions = actions(() => [Wait({ durationMillis: 0 })]); ` +// absentSelectorSpec names an element the tree never holds, so every step +// dispatches by selector and the driver is the layer that finds nothing. +const absentSelectorSpec = ` +import { actions, always, Tap } from "@sanderling/spec"; +globalThis.properties = { + alwaysHolds: always(() => true), +}; +globalThis.actions = actions(() => [Tap({ on: "id:absent" })]); +` + const violationSpec = ` import { actions, always } from "@sanderling/spec"; globalThis.properties = { @@ -739,20 +749,6 @@ func TestApplyAction_InputTextErasesExistingTextBeforeTyping(t *testing.T) { } } -// TestApplyAction_InputTextWithoutTargetSkipsFocusTap pins that with no -// resolvable target there is no focus tap (and so no settle), and InputText -// still runs at the cursor. -func TestApplyAction_InputTextWithoutTargetSkipsFocusTap(t *testing.T) { - driverMock := mockdriver.New() - action := verifier.Action{Kind: verifier.ActionKindInputText, X: -1, Y: -1, Text: "alice"} - - 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) - } -} - // 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. @@ -1156,6 +1152,47 @@ func TestApplyAction_ScrollWithPrecomputedEndpointsSwipes(t *testing.T) { } } +// scrollingDriver is a driver that scrolls by something other than a drag, +// which is what the web driver is: a browser scrolls on wheel input and only +// ever treats a drag as a drag. +type scrollingDriver struct { + *mockdriver.Driver + scrolls [][4]int +} + +func (d *scrollingDriver) Scroll( + _ context.Context, + fromX, fromY, toX, toY int, + _ time.Duration, +) error { + d.scrolls = append(d.scrolls, [4]int{fromX, fromY, toX, toY}) + return nil +} + +func TestApplyAction_ScrollPrefersTheScrollCapabilityOverASwipe(t *testing.T) { + drv := &scrollingDriver{Driver: mockdriver.New()} + action := verifier.Action{ + Kind: verifier.ActionKindScroll, + Direction: "down", + FromX: 100, + FromY: 500, + ToX: 100, + ToY: 300, + DurationMillis: 300, + } + + mustDispatch(t, drv, action, nil) + want := [4]int{100, 500, 100, 300} + if len(drv.scrolls) != 1 || drv.scrolls[0] != want { + t.Fatalf("scrolls = %v, want one %v", drv.scrolls, want) + } + for _, a := range drv.Actions() { + if a.Kind == mockdriver.ActionSwipe { + t.Errorf("the scroll was dispatched as a swipe: %v", drv.Actions()) + } + } +} + func TestApplyAction_ScrollDirectionUsesInversion(t *testing.T) { driverMock := mockdriver.New() treeJSON := `{"attributes":{"resource-id":"com.fixture:id/list","bounds":"[0,0,400,800]"},"children":[],"enabled":true}` @@ -1258,21 +1295,6 @@ func TestApplyAction_NonDispatchPathsReportWhy(t *testing.T) { 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"}, @@ -1379,10 +1401,11 @@ func TestRunner_DispatchedActionRecordsNoSkipReason(t *testing.T) { } type traceStepLine struct { - Step int `json:"step"` - NextAction *trace.Action `json:"next_action"` - ActionSkipped string `json:"action_skipped"` - Transitional bool `json:"transitional"` + Step int `json:"step"` + NextAction *trace.Action `json:"next_action"` + ActionSkipped string `json:"action_skipped"` + Transitional bool `json:"transitional"` + ObservationError string `json:"observation_error"` } func readTraceLines(t *testing.T, directory string) []traceStepLine { @@ -2575,3 +2598,381 @@ func TestRunner_SiblingTapsReachTheDriverAtTheirOwnCoordinates(t *testing.T) { t.Errorf("driver saw %d distinct tap points, want %d: %v", len(tapped), len(want), slices.Sorted(maps.Keys(tapped))) } } + +// tapReachesNoElement wraps a mock driver so every tap reports what the chrome +// driver reports when the action's point holds no element: nothing was +// dispatched, so the app cannot have responded. +type tapReachesNoElement struct { + *mockdriver.Driver +} + +func (d *tapReachesNoElement) TapSelector( + _ context.Context, + selector string, +) error { + return fmt.Errorf("%w: %s", driver.ErrGestureUndelivered, selector) +} + +func (d *tapReachesNoElement) Tap(_ context.Context, x, y int) error { + return fmt.Errorf("%w: (%d,%d)", driver.ErrGestureUndelivered, x, y) +} + +// TestRunner_UndeliveredGestureIsRecordedNotSilentlyClean is the runner half of +// the silent-actuation bug: a run whose every tap reached nothing used to look +// exactly like a run that exercised the app and found no violations. The step +// now names the reason, and the run says how many actions did nothing. +func TestRunner_UndeliveredGestureIsRecordedNotSilentlyClean(t *testing.T) { + state := newHarness(t) + wrapped := &tapReachesNoElement{Driver: state.mock} + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + summary, err := Run(ctx, Options{ + Duration: 10 * time.Second, + MaxSteps: maxConsecutiveApplyFailures + 2, + IdleTimeout: 20 * time.Millisecond, + Driver: wrapped, + Verifier: state.verifier, + TraceWriter: state.writer, + }) + if err != nil { + t.Fatalf( + "a gesture that reached nothing is not a device fault: %v", + err, + ) + } + if summary.Steps <= maxConsecutiveApplyFailures { + t.Fatalf( + "the run must outlive the apply-failure cap, got %d steps", + summary.Steps, + ) + } + undelivered := summary.SkippedActions[string(actionSkippedGestureUndelivered)] + if undelivered != summary.Steps { + t.Errorf( + "gesture_undelivered count = %d, want %d (every tap reached nothing)", + undelivered, + summary.Steps, + ) + } + + var rendered bytes.Buffer + RenderSummary(&rendered, summary, "web") + if !strings.Contains(rendered.String(), "gesture_undelivered") { + t.Errorf( + "the summary must not read clean while nothing was actuated, got:\n%s", + rendered.String(), + ) + } + + type traceLine struct { + Step int `json:"step"` + ActionSkipped string `json:"action_skipped"` + 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.ActionSkipped != "gesture_undelivered" { + t.Errorf("step %d: action_skipped = %q, want %q", + line.Step, line.ActionSkipped, "gesture_undelivered") + } + if line.Transitional { + t.Errorf( + "step %d: an undelivered gesture leaves the verified screen intact, "+ + "so the step must not be transitional", + line.Step, + ) + } + } +} + +// selectorMatchesNothing wraps a mock driver so every by-selector tap reports +// what the drivers report when the selector names no element on the screen. +type selectorMatchesNothing struct { + *mockdriver.Driver +} + +func (d *selectorMatchesNothing) TapSelector(_ context.Context, selector string) error { + return fmt.Errorf("%w: %q", driver.ErrSelectorMatchedNothing, selector) +} + +// TestRunner_SelectorThatMatchesNothingIsRecordedApartFromAnUndeliveredGesture +// keeps the two silent paths distinguishable. A selector that named no element +// is a resolution failure, so the step records unresolved_selector, keeps the +// verified screen (not transitional), and does not spend the apply-failure +// budget that a wedged device is meant to exhaust. +func TestRunner_SelectorThatMatchesNothingIsRecordedApartFromAnUndeliveredGesture(t *testing.T) { + state := newHarnessWithSpec(t, absentSelectorSpec) + wrapped := &selectorMatchesNothing{Driver: state.mock} + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + summary, err := Run(ctx, Options{ + Duration: 10 * time.Second, + MaxSteps: maxConsecutiveApplyFailures + 2, + IdleTimeout: 20 * time.Millisecond, + Driver: wrapped, + Verifier: state.verifier, + TraceWriter: state.writer, + }) + if err != nil { + t.Fatalf("a selector that matched nothing is not a device fault: %v", err) + } + if summary.Steps <= maxConsecutiveApplyFailures { + t.Fatalf("the run must outlive the apply-failure cap, got %d steps", summary.Steps) + } + if got := summary.SkippedActions[string(actionSkippedUnresolvedSelector)]; got != summary.Steps { + t.Errorf("unresolved_selector count = %d, want %d", got, summary.Steps) + } + if got := summary.SkippedActions[string(actionSkippedGestureUndelivered)]; got != 0 { + t.Errorf("gesture_undelivered count = %d, want 0: nothing was dispatched to a point", got) + } + + var rendered bytes.Buffer + RenderSummary(&rendered, summary, "android") + if !strings.Contains(rendered.String(), "unresolved_selector") { + t.Errorf("the summary must name the actions that found no target, got:\n%s", rendered.String()) + } + + type traceLine struct { + Step int `json:"step"` + ActionSkipped string `json:"action_skipped"` + 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.ActionSkipped != "unresolved_selector" { + t.Errorf("step %d: action_skipped = %q, want %q", + line.Step, line.ActionSkipped, "unresolved_selector") + } + if line.Transitional { + t.Errorf("step %d: nothing was dispatched, so the step must not be transitional", line.Step) + } + } +} + +// TestApplyAction_TapAboveTheViewportReachesTheDriver is the other half of the +// off-screen reach fix. The web host names an unnamed candidate by coordinates +// alone, and a candidate the growing document pushed above the fold carries a +// negative y; dropping it here denies the driver the scroll that would bring it +// back, so reach below the fold works and reach above it does not. +func TestApplyAction_TapAboveTheViewportReachesTheDriver(t *testing.T) { + mock := mockdriver.New() + mustDispatch(t, mock, verifier.Action{ + Kind: verifier.ActionKindTap, + X: 622, + Y: -208, + }, nil) + + dispatched := mock.Actions() + if len(dispatched) != 1 { + t.Fatalf("driver saw %d actions, want 1: %v", len(dispatched), dispatched) + } + if dispatched[0].Kind != mockdriver.ActionTap || + dispatched[0].X != 622 || dispatched[0].Y != -208 { + t.Errorf("driver saw %v, want a tap at (622,-208)", dispatched[0]) + } +} + +// TestApplyAction_TapAboveTheViewportKeepsTheUndeliveredReport holds the +// distinction the fix must not collapse: a point the driver cannot put an +// element under is still a failure, reported as ErrGestureUndelivered rather +// than as an action the runner declined to try. +func TestApplyAction_TapAboveTheViewportKeepsTheUndeliveredReport(t *testing.T) { + drv := &tapReachesNoElement{Driver: mockdriver.New()} + skipped, err := applyAction(context.Background(), drv, verifier.Action{ + Kind: verifier.ActionKindTap, + X: 622, + Y: -208, + }, nil) + if !errors.Is(err, driver.ErrGestureUndelivered) { + t.Errorf("err = %v, want ErrGestureUndelivered", err) + } + if skipped != "" { + t.Errorf("skip reason = %q, want none: the driver was called", skipped) + } +} + +// TestRenderSummary_NamesTheActionsThatNeverReachedTheApp is the report half of +// the silent-actuation class. A run that chose an action every step and dropped +// every one of them printed the same "no violations" as a run that exercised +// the app, because the reasons lived only in the trace and a warn line. +func TestRenderSummary_NamesTheActionsThatNeverReachedTheApp(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: 3, + Driver: state.mock, + Verifier: state.verifier, + TraceWriter: state.writer, + }) + if err != nil { + t.Fatalf("Run: %v", err) + } + reason := string(actionSkippedZeroDurationWait) + if summary.SkippedActions[reason] != 3 { + t.Errorf("SkippedActions[%s] = %d, want 3", reason, summary.SkippedActions[reason]) + } + + var rendered bytes.Buffer + RenderSummary(&rendered, summary, "web") + want := "3 action(s) never reached the app: " + reason + " 3" + if !strings.Contains(rendered.String(), want) { + t.Errorf("summary must carry %q, got:\n%s", want, rendered.String()) + } + + var clean bytes.Buffer + RenderSummary(&clean, Summary{Steps: 3}, "web") + if strings.Contains(clean.String(), "never reached the app") { + t.Errorf("a run that dropped nothing must not carry the line, got:\n%s", clean.String()) + } +} + +// wedgedTapSelector never answers the first by-selector tap, which is what a +// driver call that has stopped returning looks like from the step loop. +type wedgedTapSelector struct { + *mockdriver.Driver + calls int +} + +func (d *wedgedTapSelector) TapSelector(ctx context.Context, selector string) error { + d.calls++ + if d.calls == 1 { + <-ctx.Done() + return ctx.Err() + } + return d.Driver.TapSelector(ctx, selector) +} + +// Duration is a loop condition checked between steps, so an action that never +// returns held the run for as long as the process lived and only a kill from +// outside ended it. The step is what must fail, under a reason of its own: a +// wedge is not the gesture that reached nothing and not the action the runner +// declined to dispatch, and an analysis that cannot tell them apart cannot say +// whether the device answered at all. +func TestRunner_AnActionThatNeverReturnsFailsTheStepNotTheRun(t *testing.T) { + state := newHarness(t) + previousApplyTimeout := applyTimeout + applyTimeout = 50 * time.Millisecond + t.Cleanup(func() { applyTimeout = previousApplyTimeout }) + wrapped := &wedgedTapSelector{Driver: state.mock} + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + summary, err := Run(ctx, Options{ + Duration: time.Hour, + IdleTimeout: 20 * time.Millisecond, + MaxSteps: 3, + Driver: wrapped, + Verifier: state.verifier, + TraceWriter: state.writer, + }) + if err != nil { + t.Fatalf("Run must outlive an action that never returns, got %v", err) + } + if summary.Steps != 3 { + t.Fatalf("Steps = %d, want 3: the wedge costs one step, not the run", summary.Steps) + } + lines := readTraceLines(t, state.writer.Directory()) + if lines[0].ActionSkipped != string(actionSkippedApplyTimeout) { + t.Errorf("step 1 action_skipped = %q, want %q", + lines[0].ActionSkipped, actionSkippedApplyTimeout) + } + if !lines[0].Transitional { + t.Error("step 1 must be transitional: the action was dispatched and its effect is unknown") + } + for _, line := range lines[1:] { + if line.ActionSkipped != "" { + t.Errorf("step %d action_skipped = %q, want the wedge confined to the step that wedged", + line.Step, line.ActionSkipped) + } + } + if summary.SkippedActions[string(actionSkippedApplyTimeout)] != 1 { + t.Errorf("SkippedActions = %v, want one %s", summary.SkippedActions, actionSkippedApplyTimeout) + } +} + +// snapshotFailThenEmpty fails the first observation outright and answers the +// second with a dump holding no elements at all. +type snapshotFailThenEmpty struct { + *mockdriver.Driver + calls int +} + +func (d *snapshotFailThenEmpty) Snapshot(ctx context.Context) (string, driver.Image, error) { + d.calls++ + switch d.calls { + case 1: + return "", driver.Image{}, errors.New("Timeout while fetching view hierarchy") + case 2: + return "", driver.Image{}, nil + } + return d.Driver.Snapshot(ctx) +} + +// A step that read nothing because the read failed and a step that read a +// screen with nothing on it are recorded identically as a nil tree, so a run +// that observed nothing at all reports exactly like a run that observed an +// empty app and found no violation in it. +func TestRunner_AFailedObservationIsNotAnObservationOfAnEmptyScreen(t *testing.T) { + state := newHarness(t) + state.mock.HierarchyJSON = `{"attributes":{"resource-id":"HomeScreen"},"children":[]}` + wrapped := &snapshotFailThenEmpty{Driver: state.mock} + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + summary, err := Run(ctx, Options{ + Duration: time.Hour, + IdleTimeout: 20 * time.Millisecond, + MaxSteps: 3, + Driver: wrapped, + Verifier: state.verifier, + TraceWriter: state.writer, + }) + if err != nil { + t.Fatalf("Run: %v", err) + } + if summary.Steps != 3 { + t.Fatalf("Steps = %d, want 3", summary.Steps) + } + lines := readTraceLines(t, state.writer.Directory()) + if !strings.Contains(lines[0].ObservationError, "Timeout while fetching view hierarchy") { + t.Errorf("step 1 observation_error = %q, want the driver's own failure", + lines[0].ObservationError) + } + if lines[1].ObservationError != "" { + t.Errorf("step 2 observation_error = %q, want none: the screen was read and held no elements", + lines[1].ObservationError) + } + if lines[2].ObservationError != "" { + t.Errorf("step 3 observation_error = %q, want none", lines[2].ObservationError) + } + if summary.FailedObservations != 1 { + t.Errorf("FailedObservations = %d, want 1", summary.FailedObservations) + } + var rendered bytes.Buffer + RenderSummary(&rendered, summary, "android") + if !strings.Contains(rendered.String(), "1 step(s) observed nothing") { + t.Errorf("summary hides the failed observation: %q", rendered.String()) + } +}