diff --git a/internal/testrun/testrun.go b/internal/testrun/testrun.go index 893e5ce..7a46a94 100644 --- a/internal/testrun/testrun.go +++ b/internal/testrun/testrun.go @@ -242,7 +242,15 @@ func Execute(ctx context.Context, options Options, stdout io.Writer) error { // --exit-on-violation a run that found violations is still a successful run // (the summary reports them), which is the behaviour every existing caller // depends on. +// +// A run none of whose steps reached the verifier fails whatever the flags say, +// 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. func runOutcome(options Options, summary runner.Summary) error { + if summary.Steps > 0 && summary.SkippedVerification == summary.Steps { + return VacuousRunError{Steps: summary.Steps} + } if options.ExitOnViolation && len(summary.Violations) > 0 { return ViolationsError{Count: len(summary.Violations)} } @@ -261,6 +269,22 @@ func (e ViolationsError) Error() string { return fmt.Sprintf("%d violation record(s)", e.Count) } +// VacuousRunError reports a run in which no step reached the verifier, so no +// property ever judged anything. It is not a clean run and it is not a found +// bug: it is a run that produced no evidence either way, and the absence of +// violations in it says nothing about the app. It stays untyped to the CLI's +// violation path on purpose, so it exits 1 as a broken run rather than 2. +type VacuousRunError struct { + Steps int +} + +func (e VacuousRunError) Error() string { + return fmt.Sprintf( + "%d step(s) ran and none of them reached the verifier: the screen was "+ + "still moving every time it was read, so no property judged this run", + e.Steps) +} + // 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 fd6303e..52c77c1 100644 --- a/internal/testrun/testrun_test.go +++ b/internal/testrun/testrun_test.go @@ -266,6 +266,31 @@ func TestRunOutcome_ReportsViolationsOnlyUnderTheFlag(t *testing.T) { } } +// A step the verifier skipped was judged by nothing, so a run whose every step +// was skipped holds no verdict at all: "no violations" there is the absence of +// an answer rather than a clean one. Reporting it as a successful run is the +// green and vacuous outcome structuralShape's own design notes call worse than +// the composition it catches, and the runner's hold is what makes a fully +// skipped run reachable. +func TestRunOutcome_ARunThatJudgedNothingIsNotASuccess(t *testing.T) { + nothingJudged := runner.Summary{Steps: 6, SkippedVerification: 6} + err := runOutcome(Options{}, nothingJudged) + var vacuous VacuousRunError + if !errors.As(err, &vacuous) { + t.Fatalf("a run that judged none of its 6 steps came back %v, want a VacuousRunError", err) + } + if vacuous.Steps != 6 { + t.Errorf("steps: got %d, want 6", vacuous.Steps) + } + + // 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} + if err := runOutcome(Options{}, mostlyJudged); err != nil { + t.Errorf("a run that judged one of its 6 steps must succeed, got %v", err) + } +} + // wedgedLaunchDriver never returns from Launch, standing in for a driver whose // device-side session is stuck. type wedgedLaunchDriver struct {