diff --git a/CHANGELOG.md b/CHANGELOG.md index 3523394..ee6d099 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed +- `runFlow: when` conditions with variable expressions (e.g., `${output.element.id}`) were never expanded, causing conditions to always evaluate as false and silently skip conditional blocks + ## [1.0.7] - 2026-02-20 ### Added diff --git a/pkg/core/driver_test.go b/pkg/core/driver_test.go index 9f852c5..9bb59e2 100644 --- a/pkg/core/driver_test.go +++ b/pkg/core/driver_test.go @@ -235,8 +235,8 @@ func TestHasNonASCII(t *testing.T) { {"Hello World 123!", false}, {"", false}, {"abc\t\n", false}, - {"\x7f", false}, // DEL is ASCII (127) - {"\x80", true}, // first non-ASCII byte + {"\x7f", false}, // DEL is ASCII (127) + {"\x80", true}, // first non-ASCII byte {"cafe\u0301", true}, // e with combining accent {"hello world", false}, } diff --git a/pkg/driver/appium/driver.go b/pkg/driver/appium/driver.go index 95ae8cc..41fb3b8 100644 --- a/pkg/driver/appium/driver.go +++ b/pkg/driver/appium/driver.go @@ -693,7 +693,14 @@ func (d *Driver) findElementRelativeWithElements(sel flow.Selector, allElements return nil, fmt.Errorf("no candidates after sorting") } - selected := SelectByIndex(candidates, sel.Index) + var selected *ParsedElement + if sel.Index == "" && (filterType == filterBelow || filterType == filterAbove || filterType == filterLeftOf || filterType == filterRightOf) { + // Directional filters sort candidates by distance. Pick the closest + // (first) element to match Maestro's .firstOrNull() behavior. + selected = candidates[0] + } else { + selected = SelectByIndex(candidates, sel.Index) + } // If element isn't clickable, try to find a clickable parent // This handles React Native pattern where text nodes aren't clickable but containers are diff --git a/pkg/driver/appium/driver_test.go b/pkg/driver/appium/driver_test.go index 3915f3d..c028d72 100644 --- a/pkg/driver/appium/driver_test.go +++ b/pkg/driver/appium/driver_test.go @@ -1264,6 +1264,74 @@ func TestFindElementRelativeWithElementsContainsDescendants(t *testing.T) { } } +// mockAppiumServerForRelativeDepthTest creates a server with elements that test +// distance vs. depth selection in directional relative selectors. +func mockAppiumServerForRelativeDepthTest() *httptest.Server { + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + path := r.URL.Path + + if strings.HasSuffix(path, "/source") { + writeJSON(w, map[string]interface{}{ + "value": ` + + + + + + + + + + + + +`, + }) + return + } + + if strings.Contains(path, "/window/rect") { + writeJSON(w, map[string]interface{}{ + "value": map[string]interface{}{"width": 1080.0, "height": 2340.0, "x": 0.0, "y": 0.0}, + }) + return + } + + writeJSON(w, map[string]interface{}{"value": nil}) + })) +} + +// TestFindElementRelativePrefersClosestOverDeepest verifies that directional +// relative selectors pick the closest element by distance rather than the +// deepest in the DOM. +func TestFindElementRelativePrefersClosestOverDeepest(t *testing.T) { + server := mockAppiumServerForRelativeDepthTest() + defer server.Close() + driver := createTestAppiumDriver(server) + + source, _ := driver.client.Source() + elements, platform, _ := ParsePageSource(source) + + sel := flow.Selector{ + Below: &flow.Selector{Text: "Email Address"}, + } + + info, err := driver.findElementRelativeWithElements(sel, elements, platform) + if err != nil { + t.Fatalf("Expected success, got: %v", err) + } + if info == nil { + t.Fatal("Expected element info") + } + + // The closest element below "Email Address" (bottom at y=130) is the + // EditText at y=140, not the deeply-nested TextView at y=350. + if info.Bounds.Y != 140 { + t.Errorf("Expected element at y=140, got y=%d", info.Bounds.Y) + } +} + // TestFindElementRelativeWithNestedRelative tests nested relative selector func TestFindElementRelativeWithNestedRelative(t *testing.T) { server := mockAppiumServerForRelativeElements() diff --git a/pkg/driver/uiautomator2/driver.go b/pkg/driver/uiautomator2/driver.go index 89c8dc7..d110191 100644 --- a/pkg/driver/uiautomator2/driver.go +++ b/pkg/driver/uiautomator2/driver.go @@ -783,7 +783,14 @@ func (d *Driver) resolveRelativeSelector(sel flow.Selector) (*core.ElementInfo, // Prioritize clickable elements candidates = SortClickableFirst(candidates) - selected := SelectByIndex(candidates, sel.Index) + var selected *ParsedElement + if sel.Index == "" && (filterType == filterBelow || filterType == filterAbove || filterType == filterLeftOf || filterType == filterRightOf) { + // Directional filters sort candidates by distance. Pick the closest + // (first) element to match Maestro's .firstOrNull() behavior. + selected = candidates[0] + } else { + selected = SelectByIndex(candidates, sel.Index) + } // If element isn't clickable, try to find a clickable parent // This handles React Native pattern where text nodes aren't clickable but containers are @@ -872,7 +879,14 @@ func (d *Driver) findElementRelativeWithElements(sel flow.Selector, allElements // Prioritize clickable elements candidates = SortClickableFirst(candidates) - selected := SelectByIndex(candidates, sel.Index) + var selected *ParsedElement + if sel.Index == "" && (filterType == filterBelow || filterType == filterAbove || filterType == filterLeftOf || filterType == filterRightOf) { + // Directional filters sort candidates by distance. Pick the closest + // (first) element to match Maestro's .firstOrNull() behavior. + selected = candidates[0] + } else { + selected = SelectByIndex(candidates, sel.Index) + } // If element isn't clickable, try to find a clickable parent // This handles React Native pattern where text nodes aren't clickable but containers are diff --git a/pkg/driver/uiautomator2/driver_test.go b/pkg/driver/uiautomator2/driver_test.go index cc83be4..6d25ccf 100644 --- a/pkg/driver/uiautomator2/driver_test.go +++ b/pkg/driver/uiautomator2/driver_test.go @@ -2010,6 +2010,52 @@ func TestTapOnRelativeSelectorBelow(t *testing.T) { } } +// TestResolveRelativeSelectorPrefersClosestOverDeepest verifies that directional +// relative selectors pick the closest element by distance rather than the +// deepest in the DOM. +func TestResolveRelativeSelectorPrefersClosestOverDeepest(t *testing.T) { + pageSource := ` + + + + + + + + + + +` + + server := setupMockServer(t, map[string]func(w http.ResponseWriter, r *http.Request){ + "GET /source": func(w http.ResponseWriter, r *http.Request) { + writeJSON(w, map[string]interface{}{"value": pageSource}) + }, + }) + defer server.Close() + + client := newMockHTTPClient(server.URL) + driver := New(client.Client, nil, nil) + + sel := flow.Selector{ + Below: &flow.Selector{Text: "Email Address"}, + } + + info, err := driver.resolveRelativeSelector(sel) + if err != nil { + t.Fatalf("Expected success, got: %v", err) + } + if info == nil { + t.Fatal("Expected element info") + } + + // The closest element below "Email Address" (bottom at y=130) is the + // EditText at y=140, not the deeply-nested TextView at y=350. + if info.Bounds.Y != 140 { + t.Errorf("Expected element at y=140, got y=%d", info.Bounds.Y) + } +} + func TestTapOnRelativeSelectorClickError(t *testing.T) { pageSource := ` diff --git a/pkg/driver/wda/driver.go b/pkg/driver/wda/driver.go index addfb0f..3932e88 100644 --- a/pkg/driver/wda/driver.go +++ b/pkg/driver/wda/driver.go @@ -655,7 +655,14 @@ func (d *Driver) resolveRelativeSelector(sel flow.Selector, allElements []*Parse // Prioritize clickable/interactive elements candidates = SortClickableFirst(candidates) - selected := SelectByIndex(candidates, sel.Index) + var selected *ParsedElement + if sel.Index == "" && (filterType == filterBelow || filterType == filterAbove || filterType == filterLeftOf || filterType == filterRightOf) { + // Directional filters sort candidates by distance. Pick the closest + // (first) element to match Maestro's .firstOrNull() behavior. + selected = candidates[0] + } else { + selected = SelectByIndex(candidates, sel.Index) + } return &core.ElementInfo{ Text: selected.Label, diff --git a/pkg/driver/wda/driver_test.go b/pkg/driver/wda/driver_test.go index facf7a2..b07b30a 100644 --- a/pkg/driver/wda/driver_test.go +++ b/pkg/driver/wda/driver_test.go @@ -1569,6 +1569,80 @@ func TestResolveRelativeSelectorContainsDescendants(t *testing.T) { } } +// mockWDAServerForRelativeDepthTest creates a server with elements that test +// distance vs. depth selection in directional relative selectors. +// The page source has a close TextField (depth 2) and a far-but-deeply-nested +// Link (depth 5) below the anchor. The correct behavior is to select the closer one. +func mockWDAServerForRelativeDepthTest() *httptest.Server { + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + path := r.URL.Path + + if strings.HasSuffix(path, "/source") { + jsonResponse(w, map[string]interface{}{ + "value": ` + + + + + + + + + + + + +`, + }) + return + } + + if strings.Contains(path, "/window/size") { + jsonResponse(w, map[string]interface{}{ + "value": map[string]interface{}{"width": 390.0, "height": 844.0}, + }) + return + } + + jsonResponse(w, map[string]interface{}{"status": 0}) + })) +} + +// TestResolveRelativeSelectorPrefersClosestOverDeepest verifies that directional +// relative selectors (below/above/leftOf/rightOf) pick the closest element by +// distance rather than the deepest in the DOM. This matches Maestro's +// .firstOrNull() behavior on the distance-sorted candidate list. +func TestResolveRelativeSelectorPrefersClosestOverDeepest(t *testing.T) { + server := mockWDAServerForRelativeDepthTest() + defer server.Close() + driver := createTestDriver(server) + + source, _ := driver.client.Source() + elements, _ := ParsePageSource(source) + + sel := flow.Selector{ + Below: &flow.Selector{Text: "Email Address"}, + } + + info, err := driver.resolveRelativeSelector(sel, elements) + if err != nil { + t.Fatalf("Expected success, got: %v", err) + } + if info == nil { + t.Fatal("Expected element info") + } + + // The closest element below "Email Address" (bottom at y=130) is the + // TextField at y=140, not the deeply-nested Link at y=350 (depth 5). + if info.Text != "email input" { + t.Errorf("Expected closest element 'email input', got '%s'", info.Text) + } + if info.Bounds.Y != 140 { + t.Errorf("Expected element at y=140, got y=%d", info.Bounds.Y) + } +} + // TestEraseTextWithActiveElement tests eraseText with active element func TestEraseTextWithActiveElement(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/pkg/executor/flow_runner.go b/pkg/executor/flow_runner.go index ccd1ec0..afa7352 100644 --- a/pkg/executor/flow_runner.go +++ b/pkg/executor/flow_runner.go @@ -583,6 +583,7 @@ func (fr *FlowRunner) executeNestedStep(step flow.Step) *core.CommandResult { case *flow.RetryStep: result = fr.executeRetry(s) case *flow.RunFlowStep: + fr.script.ExpandStep(step) result = fr.executeRunFlow(s) case *flow.TakeScreenshotStep: fr.script.ExpandStep(step) diff --git a/pkg/executor/scripting.go b/pkg/executor/scripting.go index 5b684bd..c6407cb 100644 --- a/pkg/executor/scripting.go +++ b/pkg/executor/scripting.go @@ -530,6 +530,25 @@ func (se *ScriptEngine) ExpandStep(step flow.Step) { s.Link = se.ExpandVariables(s.Link) case *flow.PressKeyStep: s.Key = se.ExpandVariables(s.Key) + case *flow.RunFlowStep: + s.File = se.ExpandVariables(s.File) + if s.When != nil { + if s.When.Visible != nil { + s.When.Visible = se.expandSelector(s.When.Visible) + } + if s.When.NotVisible != nil { + s.When.NotVisible = se.expandSelector(s.When.NotVisible) + } + if s.When.Script != "" { + s.When.Script = se.ExpandVariables(s.When.Script) + } + if s.When.Platform != "" { + s.When.Platform = se.ExpandVariables(s.When.Platform) + } + } + for k, v := range s.Env { + s.Env[k] = se.ExpandVariables(v) + } } } diff --git a/pkg/executor/scripting_test.go b/pkg/executor/scripting_test.go index 9089898..1b5d0a3 100644 --- a/pkg/executor/scripting_test.go +++ b/pkg/executor/scripting_test.go @@ -1804,3 +1804,69 @@ func TestScriptEngine_EvalCondition_UndefinedVariable(t *testing.T) { t.Error("EvalCondition(SOME_UNDEFINED_VAR) should return false for undefined variable") } } + +// =========================================== +// ExpandStep: RunFlowStep +// =========================================== + +func TestScriptEngine_ExpandStep_RunFlowStep(t *testing.T) { + se := NewScriptEngine() + defer se.Close() + + se.SetVariable("FLOW_FILE", "auth.yaml") + se.SetVariable("BUTTON_ID", "profile_button") + se.SetVariable("LABEL_TEXT", "Welcome") + se.SetVariable("ENV_VAL", "production") + + step := &flow.RunFlowStep{ + File: "${FLOW_FILE}", + When: &flow.Condition{ + Visible: &flow.Selector{ID: "${BUTTON_ID}"}, + NotVisible: &flow.Selector{Text: "${LABEL_TEXT}"}, + Script: "${BUTTON_ID} !== undefined", + Platform: "${ENV_VAL}", + }, + Env: map[string]string{ + "MODE": "${ENV_VAL}", + }, + } + + se.ExpandStep(step) + + if step.File != "auth.yaml" { + t.Errorf("File = %q, want %q", step.File, "auth.yaml") + } + if step.When.Visible.ID != "profile_button" { + t.Errorf("When.Visible.ID = %q, want %q", step.When.Visible.ID, "profile_button") + } + if step.When.NotVisible.Text != "Welcome" { + t.Errorf("When.NotVisible.Text = %q, want %q", step.When.NotVisible.Text, "Welcome") + } + if step.When.Script != "profile_button !== undefined" { + t.Errorf("When.Script = %q, want %q", step.When.Script, "profile_button !== undefined") + } + if step.When.Platform != "production" { + t.Errorf("When.Platform = %q, want %q", step.When.Platform, "production") + } + if step.Env["MODE"] != "production" { + t.Errorf("Env[MODE] = %q, want %q", step.Env["MODE"], "production") + } +} + +func TestScriptEngine_ExpandStep_RunFlowStep_NilWhen(t *testing.T) { + se := NewScriptEngine() + defer se.Close() + + se.SetVariable("FILE", "test.yaml") + + // RunFlowStep with no When condition should not panic + step := &flow.RunFlowStep{ + File: "${FILE}", + } + + se.ExpandStep(step) + + if step.File != "test.yaml" { + t.Errorf("File = %q, want %q", step.File, "test.yaml") + } +}