diff --git a/internal/hierarchy/hierarchy.go b/internal/hierarchy/hierarchy.go index c54a052..1f1e9b4 100644 --- a/internal/hierarchy/hierarchy.go +++ b/internal/hierarchy/hierarchy.go @@ -11,7 +11,8 @@ // descPrefix: - 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): // > > ... - 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 } } diff --git a/internal/hierarchy/hierarchy_test.go b/internal/hierarchy/hierarchy_test.go index 91abd4c..395065f 100644 --- a/internal/hierarchy/hierarchy_test.go +++ b/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, + ) + } + }) + } +}