Merge branch 'spec-parity-holes' into trial-merge

internal/hierarchy/hierarchy.go

@@ -11,7 +11,8 @@
 //	  descPrefix:<prefix>  - starts-with on content-desc / accessibilityText
 //
 //	Object selectors (multi-attribute AND, element-scoped or global):
-//	  { attr: value, ... } - all key/value pairs must match; substring / boolean semantics
+//	  { attr: value, ... } - all key/value pairs must match, each key resolved by
+//	                         the same rule its string form above uses
 //
 //	Path queries (global scan only, string form):
 //	  <sel> > <sel> > ...  - each segment matched within subtree of previous match
@@ -156,10 +157,17 @@ func matchAttr(element *Element, attr, value string) bool {
 	return false
 }
 
-// matchSelector returns true when all filters in sel match the element (AND semantics).
+// matchSelector returns true when all filters in sel match the element (AND
+// semantics). Each filter goes through match, the same rule the string form
+// resolves a "kind:value" segment by, so {id: "Submit"} and "id:Submit" can
+// never resolve to different elements. Applying matchAttr directly here made
+// the object form skip the kind arms entirely: id, desc and descPrefix name no
+// attribute any producer writes, so those keys matched NOTHING through an
+// object selector while the string form matched, and every property over the
+// missing element passed vacuously.
 func matchSelector(element *Element, sel Selector) bool {
 	for _, f := range sel.Filters {
-		if !matchAttr(element, f.Attr, f.Value) {
+		if !match(element, f.Attr, f.Value) {
 			return false
 		}
 	}

internal/hierarchy/hierarchy_test.go

@@ -1035,3 +1035,83 @@ func TestTreeTransitional(t *testing.T) {
 		t.Error("nil tree must not be flagged as transitional")
 	}
 }
+
+// selectorFormsDump carries one node per id shape a real dump produces, plus
+// nodes carrying a description in the ", " form the desc rule knows about and a
+// text the text rule matches on a substring.
+const selectorFormsDump = `{
+  "attributes": {"resource-id": "root", "bounds": "[0,0,400,800]"},
+  "children": [
+    {"attributes": {"resource-id": "BareThing", "bounds": "[0,0,100,50]"}, "children": []},
+    {"attributes": {"resource-id": "com.example.app:id/AndroidThing", "bounds": "[0,50,100,100]"},
+     "children": []},
+    {"attributes": {"accessibilityIdentifier": "IosThing", "bounds": "[0,100,100,150]"},
+     "children": []},
+    {"attributes": {"resource-id": "Described", "content-desc": "Save, button", "bounds": "[0,150,100,200]"},
+     "children": []},
+    {"attributes": {"resource-id": "Labelled", "text": "Total balance", "bounds": "[0,200,100,250]"},
+     "children": []}
+  ]
+}`
+
+// TestSelectorFormsResolveTheSameElement holds the two selector forms a spec can
+// write to ONE rule per key. A spec reaches these through state.ax.find: a
+// string goes to FindNode, an object to FindBySelector, and the two ran
+// different matchers. `id` has a kind arm that knows an Android resource id is
+// package-qualified (com.example.app:id/Thing) and that a spec names the bare
+// tail; the object form had no such arm and looked for a literal `id` attribute
+// no producer writes, so {id: "Thing"} silently matched nothing on every
+// platform while "id:Thing" matched. `desc` and `descPrefix` had the same
+// split. A selector that resolves nothing makes every property over it
+// vacuously true, which is the failure that reports a green run while checking
+// nothing.
+func TestSelectorFormsResolveTheSameElement(t *testing.T) {
+	tree, err := Parse(selectorFormsDump)
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, test := range []struct {
+		key   string
+		value string
+		want  string
+	}{
+		{"id", "BareThing", "BareThing"},
+		// A spec names the tail; an Android dump carries the package prefix.
+		{"id", "AndroidThing", "com.example.app:id/AndroidThing"},
+		{"id", "com.example.app:id/AndroidThing", "com.example.app:id/AndroidThing"},
+		{"id", "IosThing", "IosThing"},
+		{"desc", "Save, button", "Described"},
+		// The ", " form an accessibility label takes when a role is appended.
+		{"desc", "Save", "Described"},
+		{"descPrefix", "Sav", "Described"},
+		{"text", "Total", "Labelled"},
+		{"resource-id", "BareThing", "BareThing"},
+		{"testTag", "IosThing", "IosThing"},
+	} {
+		t.Run(test.key+":"+test.value, func(t *testing.T) {
+			stringForm := test.key + ":" + test.value
+			fromString := tree.FindNode(stringForm)
+			if fromString == nil {
+				t.Fatalf("the string form %q resolved nothing", stringForm)
+			}
+			if fromString.ResourceID != test.want {
+				t.Fatalf("the string form resolved %q, want %q", fromString.ResourceID, test.want)
+			}
+			fromObject := tree.Root.FindBySelector(
+				Selector{Filters: []AttrFilter{{Attr: test.key, Value: test.value}}},
+			)
+			if fromObject == nil {
+				t.Fatalf(
+					"the object form {%s: %q} resolved nothing while %q resolved %q",
+					test.key, test.value, stringForm, fromString.ResourceID,
+				)
+			}
+			if fromObject != fromString {
+				t.Errorf(
+					"one selector, two answers: {%s: %q} resolved %q and %q resolved %q",
+					test.key, test.value, fromObject.ResourceID, stringForm, fromString.ResourceID,
+				)
+			}
+		})
+	}
+}

internal/verifier/ax_integration_test.go

@@ -2,6 +2,7 @@ package verifier
 
 import (
 	"os"
+	"strconv"
 	"testing"
 
 	"github.com/priyanshujain/sanderling/internal/hierarchy"
@@ -99,3 +100,77 @@ func TestStateAxFindWorks(t *testing.T) {
 		t.Fatalf("findAll count = %d, want 1", count)
 	}
 }
+
+// axSelectorFormsTree carries one node per id shape a dump produces: the bare
+// tag Compose and the web driver emit, the package-qualified resource id
+// Android emits, and the iOS accessibility identifier.
+const axSelectorFormsTree = `{
+	"attributes": {"resource-id": "root", "bounds": "[0,0,400,800]"},
+	"children": [
+		{"attributes": {"resource-id": "BareThing", "text": "bare", "bounds": "[0,0,100,50]"},
+		 "children": []},
+		{"attributes": {"resource-id": "com.example.app:id/AndroidThing", "text": "android",
+		 "bounds": "[0,50,100,100]"}, "children": []},
+		{"attributes": {"accessibilityIdentifier": "IosThing", "text": "ios",
+		 "bounds": "[0,100,100,150]"}, "children": []}
+	]
+}`
+
+// TestStateAxSelectorFormsAgree drives both selector forms a spec can write
+// through state.ax.find and holds them to the same element. The two forms
+// dispatch to different lookups (findNodeFromJS sends a string to FindNode and
+// an object to FindBySelector), and the object one used to skip the id rule
+// that knows an Android resource id is package-qualified, so a spec that wrote
+// ax.find({id: "AddAccountSubmit"}) got undefined on Android and every property
+// reading it passed while checking nothing.
+func TestStateAxSelectorFormsAgree(t *testing.T) {
+	tree, err := hierarchy.Parse(axSelectorFormsTree)
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, test := range []struct {
+		value string
+		want  string
+	}{
+		{"BareThing", "bare"},
+		{"AndroidThing", "android"},
+		{"com.example.app:id/AndroidThing", "android"},
+		{"IosThing", "ios"},
+	} {
+		t.Run(test.value, func(t *testing.T) {
+			verifier := newVerifier(t)
+			mustLoad(t, verifier, `
+				globalThis.fromObject = __sanderling__.extract(
+					state => state.ax.find({ id: `+strconv.Quote(test.value)+` })?.text, "fromObject");
+				globalThis.fromString = __sanderling__.extract(
+					state => state.ax.find("id:" + `+strconv.Quote(test.value)+`)?.text, "fromString");
+				globalThis.properties = {};
+			`)
+			if err := verifier.PushSnapshot(SnapshotInput{Tree: tree}); err != nil {
+				t.Fatal(err)
+			}
+			fromObject := readCurrent(t, verifier, "fromObject")
+			fromString := readCurrent(t, verifier, "fromString")
+			if fromString != test.want {
+				t.Fatalf(`ax.find("id:%s") read %v, want %q`, test.value, fromString, test.want)
+			}
+			if fromObject != fromString {
+				t.Errorf(
+					`one selector, two answers: ax.find({id: %q}) read %v and ax.find("id:%s") read %v`,
+					test.value, fromObject, test.value, fromString,
+				)
+			}
+		})
+	}
+}
+
+// readCurrent returns a named extractor's current value, or nil when the getter
+// returned undefined, which is what an unresolved selector produces.
+func readCurrent(t *testing.T, verifier *Verifier, name string) any {
+	t.Helper()
+	handle := verifier.runtime.GlobalObject().Get(name)
+	if handle == nil {
+		t.Fatalf("%s is not defined", name)
+	}
+	return handle.ToObject(verifier.runtime).Get("current").Export()
+}

internal/verifier/extractor_encoding_test.go

@@ -177,3 +177,79 @@ func compactJSON(t *testing.T, source string) string {
 	}
 	return compact.String()
 }
+
+// TestExtractorEncoding_NestedUndefinedIsNotOnTheWire pins the one reading
+// shape the two hosts do NOT encode alike, rather than hiding it.
+//
+// JSON has no undefined, so the page loses the whole key (asserted in
+// pkg/spec/test/web-runtime.test.ts) while goja writes null. goja cannot mirror
+// the drop: Export reports an undefined member and a null member identically as
+// nil, so dropping those keys here would drop the genuine nulls the page keeps.
+// Mirroring the other way, by writing null on the page, would break the one
+// thing that does agree. Carrying the member across takes a wire format that
+// can express undefined, which is a change to every layer that parses a reading
+// and to the replay UI that renders one.
+//
+// So the guarantee is narrower than "the same object": both hosts answer
+// undefined when a property READS the member. Key presence (`in`, Object.keys)
+// is not part of it, and this test says so out loud, so closing the gap has to
+// be a deliberate change to both hosts at once.
+func TestExtractorEncoding_NestedUndefinedIsNotOnTheWire(t *testing.T) {
+	const reading = `({ absent: undefined, empty: null, present: 1 })`
+	const fromGoja = `{"absent":null,"empty":null,"present":1}`
+	// What the page sends for the same getter, with the key gone.
+	const fromWeb = `{"empty":null,"present":1}`
+
+	if got := encodeSpecValue(t, reading); got != fromGoja {
+		t.Errorf("goja encoded the reading as %s, want %s", got, fromGoja)
+	}
+
+	native := newVerifier(t)
+	mustLoad(t, native, "__sanderling__.extract(state => "+reading+", \"value\");\nglobalThis.properties = {};")
+	if err := native.PushSnapshot(SnapshotInput{}); err != nil {
+		t.Fatal(err)
+	}
+
+	web := newVerifier(t)
+	mustLoad(t, web, "__sanderling__.extract(state => null, \"value\");\nglobalThis.properties = {};")
+	if err := web.PushSnapshot(SnapshotInput{}); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := web.OverrideExtractorValues(map[int]json.RawMessage{0: json.RawMessage(fromWeb)}); err != nil {
+		t.Fatal(err)
+	}
+
+	for _, probe := range []struct {
+		expression string
+		native     bool
+		web        bool
+	}{
+		{"reading.absent === undefined", true, true},
+		{"reading.empty === null", true, true},
+		{"reading.present === 1", true, true},
+		// The half that does not survive the wire.
+		{`"absent" in reading`, true, false},
+	} {
+		if got := evaluateAgainstReading(t, native, probe.expression); got != probe.native {
+			t.Errorf("goja host: %s is %v, want %v", probe.expression, got, probe.native)
+		}
+		if got := evaluateAgainstReading(t, web, probe.expression); got != probe.web {
+			t.Errorf("web host: %s is %v, want %v", probe.expression, got, probe.web)
+		}
+	}
+}
+
+// evaluateAgainstReading answers a boolean expression over the value a property
+// would read out of the first extractor, which is where the two hosts have to
+// agree.
+func evaluateAgainstReading(t *testing.T, verifier *Verifier, expression string) bool {
+	t.Helper()
+	if err := verifier.runtime.GlobalObject().Set("reading", verifier.extractors[0].currentValue); err != nil {
+		t.Fatal(err)
+	}
+	value, err := verifier.runtime.RunString(expression)
+	if err != nil {
+		t.Fatalf("evaluate %s: %v", expression, err)
+	}
+	return value.ToBoolean()
+}