From 513b33c0e51ac29de988a061ffd4119171681d89 Mon Sep 17 00:00:00 2001 From: PJ Date: Tue, 18 Aug 2026 14:16:47 +0530 Subject: [PATCH] feat(testrun): refuse a run that dispatched none of its actions Same argument as the zero-property refusal: an instrument that drove nothing must not report a clean result. A first-screen violation still wins under --exit-on-violation, --allow-no-properties exempts the extraction sweeps that measure reach rather than judge, and one dispatched action is enough, so a generator quiet on some screens is untouched. --- internal/testrun/testrun.go | 52 +++++++++++++++++++++++++ internal/testrun/testrun_test.go | 65 ++++++++++++++++++++++++++++++-- 2 files changed, 113 insertions(+), 4 deletions(-) diff --git a/internal/testrun/testrun.go b/internal/testrun/testrun.go index 1440f5a..b4d36d9 100644 --- a/internal/testrun/testrun.go +++ b/internal/testrun/testrun.go @@ -5,9 +5,12 @@ import ( "context" "fmt" "io" + "maps" "os" "path/filepath" + "slices" "strconv" + "strings" "time" "github.com/priyanshujain/sanderling/internal/android" @@ -261,6 +264,14 @@ func Execute(ctx context.Context, options Options, stdout io.Writer) error { // because it holds no verdict to report. The threshold is every step and not a // fraction of them: a screen that composes now and then costs a healthy android // run a step or two, and a check that fired on those would be red on every run. +// +// A run that dispatched no action at all fails on the same grounds, and the +// threshold is zero for the same reason: a generator with nothing to offer on +// some screens is ordinary, one with nothing to offer on every screen of a whole +// run drove nothing. --exit-on-violation keeps precedence over it so a run that +// found something still exits on its evidence, and the property-free opt-out +// exempts the sweeps, whose measurement is where a generator reaches and for +// which "nowhere on this build" is a result rather than a broken run. func runOutcome(options Options, summary runner.Summary) error { if summary.Steps > 0 && summary.SkippedVerification == summary.Steps { return VacuousRunError{Steps: summary.Steps} @@ -268,6 +279,12 @@ func runOutcome(options Options, summary runner.Summary) error { if options.ExitOnViolation && len(summary.Violations) > 0 { return ViolationsError{Count: len(summary.Violations)} } + if !options.AllowNoProperties && summary.Steps > 0 && summary.DispatchedActions == 0 { + return NoActionsDispatchedError{ + Steps: summary.Steps, + SkippedActions: summary.SkippedActions, + } + } return nil } @@ -335,6 +352,41 @@ func (e NoPropertiesError) Error() string { e.Spec) } +// NoActionsDispatchedError reports a run not one of whose steps drove the app. +// Every screen it judged was the one it launched on, so its empty violation list +// says as much about the app as a spec with no properties would: the run +// observed, judged the same state over and over, and exercised nothing. It stays +// untyped to the CLI's violation path like VacuousRunError, so it exits 1 as a +// run that holds no verdict rather than 2. +type NoActionsDispatchedError struct { + Steps int + // SkippedActions is the runner's per-reason count of actions that never + // reached the app, which is where the cause is: a picker with no candidate + // reads differently from one whose every model call failed. + SkippedActions map[string]int +} + +func (e NoActionsDispatchedError) Error() string { + return fmt.Sprintf( + "%d step(s) ran and none of them dispatched an action: the run observed the app "+ + "and never drove it, so it judged its launch screen over and over and its "+ + "violation count says nothing about the rest of the app%s", + e.Steps, skipReasonSuffix(e.SkippedActions)) +} + +// skipReasonSuffix renders the skip-reason tally as a trailing clause, empty +// when the run recorded none. +func skipReasonSuffix(skipped map[string]int) string { + if len(skipped) == 0 { + return "" + } + reasons := make([]string, 0, len(skipped)) + for _, reason := range slices.Sorted(maps.Keys(skipped)) { + reasons = append(reasons, fmt.Sprintf("%s %d", reason, skipped[reason])) + } + return ". Actions that never reached the app: " + strings.Join(reasons, ", ") +} + // bundleInputs holds the pre-driver assembly: alias map, seed, esbuild defines, // and the resolved spec-API/goja-runtime paths the bundler consumes. type bundleInputs struct { diff --git a/internal/testrun/testrun_test.go b/internal/testrun/testrun_test.go index 2f7f4bb..5c7ac94 100644 --- a/internal/testrun/testrun_test.go +++ b/internal/testrun/testrun_test.go @@ -297,10 +297,11 @@ func TestBuildRunMeta_OmitsModelWhenSpecDeclaresNoLLMGenerator(t *testing.T) { // stays a successful run, which is what every existing invocation expects. func TestRunOutcome_ReportsViolationsOnlyUnderTheFlag(t *testing.T) { violated := runner.Summary{ - Steps: 7, - Violations: []runner.ViolationRecord{{StepIndex: 3, Properties: []string{"balanceMoves"}}}, + Steps: 7, + DispatchedActions: 7, + Violations: []runner.ViolationRecord{{StepIndex: 3, Properties: []string{"balanceMoves"}}}, } - clean := runner.Summary{Steps: 7} + clean := runner.Summary{Steps: 7, DispatchedActions: 7} if err := runOutcome(Options{}, violated); err != nil { t.Errorf("without --exit-on-violation a violated run must succeed, got %v", err) @@ -338,12 +339,68 @@ func TestRunOutcome_ARunThatJudgedNothingIsNotASuccess(t *testing.T) { // A screen that composes now and then costs a run steps, not its verdict. A // check that fired here would turn every healthy android run red. - mostlyJudged := runner.Summary{Steps: 6, SkippedVerification: 5} + mostlyJudged := runner.Summary{Steps: 6, SkippedVerification: 5, DispatchedActions: 1} if err := runOutcome(Options{}, mostlyJudged); err != nil { t.Errorf("a run that judged one of its 6 steps must succeed, got %v", err) } } +// A run that dispatched no action at all observed one screen for its whole life +// and never drove the app, so its empty violation list is what an unplugged +// instrument reports. The provider rate-limiting a model picker is the way this +// happens in practice: every step ends with the picker handing back nothing. +func TestRunOutcome_ARunThatDroveNothingIsNotASuccess(t *testing.T) { + droveNothing := runner.Summary{ + Steps: 200, + SkippedActions: map[string]int{"no_action_produced": 200}, + } + err := runOutcome(Options{}, droveNothing) + var dead NoActionsDispatchedError + if !errors.As(err, &dead) { + t.Fatalf("a run that dispatched none of its 200 steps' actions came back %v, "+ + "want a NoActionsDispatchedError", err) + } + if dead.Steps != 200 { + t.Errorf("steps: got %d, want 200", dead.Steps) + } + if !strings.Contains(dead.Error(), "no_action_produced") { + t.Errorf("the error never names why nothing was dispatched: %v", dead) + } + + // One action is exploration, however little. A check that fired here would + // be red on any run whose screen offers the generator nothing for a while. + droveOnce := runner.Summary{Steps: 200, DispatchedActions: 1} + if err := runOutcome(Options{}, droveOnce); err != nil { + t.Errorf("a run that dispatched one action must succeed, got %v", err) + } + + // The sweeps that measure what a spec extracts and where a generator reaches + // ask for a run that judges nothing by name, and "the generator reached + // nothing here" is their measurement rather than their failure. + if err := runOutcome(Options{AllowNoProperties: true}, droveNothing); err != nil { + t.Errorf("the property-free opt-out no longer carries a run through, got %v", err) + } + + // A run cut short before it took a step never got going, which the deadline + // and the step count already say. + if err := runOutcome(Options{}, runner.Summary{}); err != nil { + t.Errorf("a run with no steps at all must not report as dead-on-arrival, got %v", err) + } + + // CI reads exit 2 as "the run found the bug". A run whose first screen + // already violated must keep reporting that, or the found bug is downgraded + // to a broken harness. + foundOnTheFirstScreen := droveNothing + foundOnTheFirstScreen.Violations = []runner.ViolationRecord{ + {StepIndex: 1, Properties: []string{"balanceNonNegative"}}, + } + err = runOutcome(Options{ExitOnViolation: true}, foundOnTheFirstScreen) + var violations ViolationsError + if !errors.As(err, &violations) { + t.Errorf("a run that found a violation came back %v, want a ViolationsError", err) + } +} + // wedgedLaunchDriver never returns from Launch, standing in for a driver whose // device-side session is stuck. type wedgedLaunchDriver struct {