fix(runner): fail when the page reports fewer readings than extractors

the comment here already claimed a partial override was fatal. it was
not: the skipped check only catches indices outside the extractor list,
so a page reporting values for some extractors and not others left the
rest holding goja's reading of the dump with nothing said.
This commit is contained in:
pj committed 2026-08-15 12:45:31 +05:30
1 parent 4b7a9e2fe2
commit 85338aeaf1
2 files changed
+55 -2

No files matched your search

+9 -2
View File
@@ -88,6 +88,7 @@ func Run(ctx context.Context, options Options) (Summary, error) {
if err != nil { if err != nil {
return Summary{}, err return Summary{}, err
} }
_, pageExtractors := extractorSource.(webSource)
summary := Summary{StartTime: time.Now()} summary := Summary{StartTime: time.Now()}
deadline := summary.StartTime.Add(options.Duration) deadline := summary.StartTime.Add(options.Duration)
@@ -219,11 +220,17 @@ func Run(ctx context.Context, options Options) (Summary, error) {
}); err != nil { }); err != nil {
return summary, fmt.Errorf("step %d push: %w", stepIndex, err) return summary, fmt.Errorf("step %d push: %w", stepIndex, err)
} }
// Both failures below leave some extractors holding the page's // Every failure below leaves some extractors holding the page's
// value and the rest holding goja's reading of the dump, and a // value and the rest holding goja's reading of the dump, and a
// property comparing previous to current across that split fires // property comparing previous to current across that split fires
// on a healthy app. A skip also means the two engines loaded // on a healthy app. Each also means the two engines loaded
// different bundles, which nothing downstream can reconcile. // different bundles, which nothing downstream can reconcile.
if pageExtractors && len(v8Overrides) != options.Verifier.ExtractorCount() {
return summary, fmt.Errorf(
"step %d: the page reported values for %d of the spec's %d extractors; "+
"the page and the host are running different bundles",
stepIndex, len(v8Overrides), options.Verifier.ExtractorCount())
}
skipped, overrideErr := options.Verifier.OverrideExtractorValues(v8Overrides) skipped, overrideErr := options.Verifier.OverrideExtractorValues(v8Overrides)
if overrideErr != nil { if overrideErr != nil {
return summary, fmt.Errorf("step %d apply extractor overrides: %w", stepIndex, overrideErr) return summary, fmt.Errorf("step %d apply extractor overrides: %w", stepIndex, overrideErr)
@@ -6,6 +6,7 @@ import (
"encoding/json" "encoding/json"
"os" "os"
"path/filepath" "path/filepath"
"strings"
"testing" "testing"
"time" "time"
@@ -118,3 +119,48 @@ func TestRunner_TraceRecordsTheValueTheVerdictUsed(t *testing.T) {
t.Error("no witness reached the trace; nothing was compared") t.Error("no witness reached the trace; nothing was compared")
} }
} }
// splitTableSpec registers two extractors whose goja bodies both answer "goja".
// The page below reports only the first, so index 1 keeps goja's dump-derived
// reading while index 0 holds the page's.
const splitTableSpec = `
import { actions, extract } from "@sanderling/spec";
extract("first", () => "goja");
extract("second", () => "goja");
globalThis.properties = {};
globalThis.actions = actions(() => []);
`
// TestRunner_PartialExtractorTableIsFatal pins the failure the runner used to
// let through. JSON.stringify drops an undefined-valued key, so a page whose
// extractors are mostly undefined off their own screen reported a table with
// holes in it, and the run completed with half the extractors reading from V8
// and half from goja. A delta property spanning that split convicts an app that
// did nothing wrong, which is worse than a crash: it is a green report of a bug
// that is not there, or a red one for a bug nobody can reproduce.
func TestRunner_PartialExtractorTableIsFatal(t *testing.T) {
state := newHarnessWithSpec(t, splitTableSpec)
web := &webMockDriver{
Driver: state.mock,
overrides: map[int]json.RawMessage{0: json.RawMessage(`"v8"`)},
}
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
_, err := Run(ctx, Options{
Duration: time.Hour,
IdleTimeout: 20 * time.Millisecond,
MaxSteps: 2,
Driver: web,
Verifier: state.verifier,
TraceWriter: state.writer,
})
if err == nil {
t.Fatal("the run completed on a page that reported 1 of 2 extractors; " +
"the second extractor silently kept goja's value")
}
const want = "the page reported values for 1 of the spec's 2 extractors"
if !strings.Contains(err.Error(), want) {
t.Errorf("Run failed with %q, want it to name the split: %q", err, want)
}
}