diff --git a/drivers/android/devicelab-android-driver.apk b/drivers/android/devicelab-android-driver.apk index 4f3d4f3..6077362 100644 Binary files a/drivers/android/devicelab-android-driver.apk and b/drivers/android/devicelab-android-driver.apk differ diff --git a/pkg/cli/cli_test.go b/pkg/cli/cli_test.go index ce9884f..c59733d 100644 --- a/pkg/cli/cli_test.go +++ b/pkg/cli/cli_test.go @@ -1,6 +1,7 @@ package cli import ( + "context" "fmt" "net" "os" @@ -31,6 +32,7 @@ func (m *mockDriver) GetState() *core.StateSnapshot { return nil } func (m *mockDriver) GetPlatformInfo() *core.PlatformInfo { return m.platformInfo } func (m *mockDriver) SetFindTimeout(int) {} func (m *mockDriver) SetWaitForIdleTimeout(int) error { return nil } +func (m *mockDriver) SetContext(context.Context) {} func TestResolveOutputDir_Default(t *testing.T) { dir, err := resolveOutputDir("", false) diff --git a/pkg/cli/ios.go b/pkg/cli/ios.go index f6d32db..1bef02a 100644 --- a/pkg/cli/ios.go +++ b/pkg/cli/ios.go @@ -4,9 +4,11 @@ import ( "context" "encoding/json" "fmt" + "os" "os/exec" "path/filepath" "strings" + "time" goios "github.com/danielpaulus/go-ios/ios" "github.com/danielpaulus/go-ios/ios/zipconduit" @@ -16,6 +18,11 @@ import ( "github.com/devicelab-dev/maestro-runner/pkg/logger" ) +// installTimeout is the max time we wait for an iOS app install to complete. +// Big apps on slow USB can legitimately take a while; 3 minutes is a generous +// upper bound that still catches infinite hangs. +const installTimeout = 3 * time.Minute + // simulatorInfo holds iOS simulator information. type simulatorInfo struct { Name string @@ -342,16 +349,81 @@ func getIOSDeviceInfo(udid string) (*iosDeviceInfo, error) { } // installIOSApp installs an app on an iOS device (simulator or physical). +// +// Strategy for physical devices: +// 1. Prefer `xcrun devicectl device install app` (Apple's modern CoreDevice +// installer, iOS 17+ friendly) — set MAESTRO_RUNNER_IOS_INSTALLER=zipconduit +// to skip. +// 2. Fall back to go-ios zipconduit for older Xcode / macOS or on devicectl +// failure. Both paths run with installTimeout so an unresponsive install +// service surfaces as an error instead of hanging forever. func installIOSApp(udid string, appPath string, isSimulator bool) error { if isSimulator { - out, err := runCommand("xcrun", "simctl", "install", udid, appPath) - if err != nil { - return fmt.Errorf("simctl install failed: %w\nOutput: %s", err, out) + return installViaSimctl(udid, appPath) + } + + installer := strings.ToLower(strings.TrimSpace(os.Getenv("MAESTRO_RUNNER_IOS_INSTALLER"))) + + // Default: prefer devicectl, fall back to zipconduit. + if installer != "zipconduit" && devicectlAvailable() { + if err := installViaDevicectl(udid, appPath); err == nil { + return nil + } else if installer == "devicectl" { + // User explicitly forced devicectl — propagate the error, don't silently fall back. + return err + } else { + logger.Warn("devicectl install failed, falling back to zipconduit: %v", err) } - return nil } - // Physical device - use go-ios zipconduit + return installViaZipconduit(udid, appPath) +} + +// installViaSimctl installs on a simulator. Wrapped in a timeout so a stuck +// simulator can't freeze the whole run. +func installViaSimctl(udid, appPath string) error { + ctx, cancel := context.WithTimeout(context.Background(), installTimeout) + defer cancel() + out, err := exec.CommandContext(ctx, "xcrun", "simctl", "install", udid, appPath).CombinedOutput() + if ctx.Err() == context.DeadlineExceeded { + return fmt.Errorf("simctl install timed out after %v", installTimeout) + } + if err != nil { + return fmt.Errorf("simctl install failed: %w\nOutput: %s", err, out) + } + return nil +} + +// devicectlAvailable reports whether `xcrun devicectl` works on this host. +// Requires macOS 14 / Xcode 15+. +func devicectlAvailable() bool { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + err := exec.CommandContext(ctx, "xcrun", "devicectl", "--version").Run() + return err == nil +} + +// installViaDevicectl uses Apple's modern CoreDevice installer. +// Supported on iOS 17+ real devices; also works on earlier iOS via Xcode 15+. +func installViaDevicectl(udid, appPath string) error { + ctx, cancel := context.WithTimeout(context.Background(), installTimeout) + defer cancel() + out, err := exec.CommandContext(ctx, "xcrun", "devicectl", "device", "install", "app", + "--device", udid, appPath).CombinedOutput() + if ctx.Err() == context.DeadlineExceeded { + return fmt.Errorf("devicectl install timed out after %v", installTimeout) + } + if err != nil { + return fmt.Errorf("devicectl install failed: %w\nOutput: %s", err, string(out)) + } + return nil +} + +// installViaZipconduit uses the go-ios Go-native installer. Kept as a fallback +// for hosts without devicectl (older macOS / Xcode). Wrapped in a timeout — +// without it, SendFile can hang indefinitely when the install service accepts +// the connection but never acks completion (observed on iOS 26 / iPhone 13). +func installViaZipconduit(udid, appPath string) error { entry, err := goios.GetDevice(udid) if err != nil { return fmt.Errorf("device %s not found: %w", udid, err) @@ -360,10 +432,20 @@ func installIOSApp(udid string, appPath string, isSimulator bool) error { if err != nil { return fmt.Errorf("failed to connect to device install service: %w", err) } - if err := conn.SendFile(appPath); err != nil { - return fmt.Errorf("failed to install app: %w", err) + + done := make(chan error, 1) + go func() { done <- conn.SendFile(appPath) }() + + select { + case err := <-done: + if err != nil { + return fmt.Errorf("failed to install app: %w", err) + } + return nil + case <-time.After(installTimeout): + return fmt.Errorf("app install timed out after %v (try: xcrun devicectl device install app --device %s %s)", + installTimeout, udid, appPath) } - return nil } // getSimulatorInfo gets information about an iOS simulator. diff --git a/pkg/cli/test.go b/pkg/cli/test.go index 79f956c..c1044c3 100644 --- a/pkg/cli/test.go +++ b/pkg/cli/test.go @@ -1283,13 +1283,14 @@ func executeSingleDevice(cfg *RunConfig, flows []flow.Flow) (*executor.RunResult WaitForIdleTimeout: cfg.WaitForIdleTimeout, TypingFrequency: cfg.TypingFrequency, DeviceInfo: &deviceInfo, - OnFlowStart: onFlowStart, + OnFlowStart: onFlowStartWithCloud(cfg), OnStepComplete: onStepComplete, OnNestedStep: onNestedStep, OnNestedFlowStart: onNestedFlowStart, - OnFlowEnd: onFlowEnd, + OnFlowEnd: onFlowEndWithCloud(cfg), }) + notifyCloudRunStart(cfg, len(flows)) return runner.Run(context.Background(), flows) } @@ -1323,13 +1324,14 @@ func ExecuteFlowWithDriver(driver core.Driver, cfg *RunConfig, f flow.Flow) (*ex WaitForIdleTimeout: cfg.WaitForIdleTimeout, TypingFrequency: cfg.TypingFrequency, DeviceInfo: &deviceInfo, - OnFlowStart: onFlowStart, + OnFlowStart: onFlowStartWithCloud(cfg), OnStepComplete: onStepComplete, OnNestedStep: onNestedStep, OnNestedFlowStart: onNestedFlowStart, - OnFlowEnd: onFlowEnd, + OnFlowEnd: onFlowEndWithCloud(cfg), }) + notifyCloudRunStart(cfg, 1) return runner.Run(context.Background(), []flow.Flow{f}) } @@ -1383,6 +1385,51 @@ func onFlowStart(flowIdx, totalFlows int, name, file string) { fmt.Println(strings.Repeat("─", 60)) } +// notifyCloudRunStart fires CloudProvider.OnRunStart once before the first flow. +// Errors are logged and do not abort the run. +func notifyCloudRunStart(cfg *RunConfig, totalFlows int) { + if cfg.CloudProvider == nil { + return + } + if err := cfg.CloudProvider.OnRunStart(cfg.CloudMeta, totalFlows); err != nil { + logger.Warn("%s OnRunStart failed: %v", cfg.CloudProvider.Name(), err) + } +} + +// onFlowStartWithCloud returns a flow-start callback that logs console progress +// and fires CloudProvider.OnFlowStart when a cloud provider is configured. +func onFlowStartWithCloud(cfg *RunConfig) func(flowIdx, totalFlows int, name, file string) { + return func(flowIdx, totalFlows int, name, file string) { + onFlowStart(flowIdx, totalFlows, name, file) + if cfg.CloudProvider == nil { + return + } + if err := cfg.CloudProvider.OnFlowStart(cfg.CloudMeta, flowIdx, totalFlows, name, file); err != nil { + logger.Warn("%s OnFlowStart failed: %v", cfg.CloudProvider.Name(), err) + } + } +} + +// onFlowEndWithCloud returns a flow-end callback that logs console progress +// and fires CloudProvider.OnFlowEnd when a cloud provider is configured. +func onFlowEndWithCloud(cfg *RunConfig) func(name string, passed bool, durationMs int64, errMsg string) { + return func(name string, passed bool, durationMs int64, errMsg string) { + onFlowEnd(name, passed, durationMs, errMsg) + if cfg.CloudProvider == nil { + return + } + fr := &cloud.FlowResult{ + Name: name, + Passed: passed, + Duration: durationMs, + Error: errMsg, + } + if err := cfg.CloudProvider.OnFlowEnd(cfg.CloudMeta, fr); err != nil { + logger.Warn("%s OnFlowEnd failed: %v", cfg.CloudProvider.Name(), err) + } + } +} + func onStepComplete(idx int, desc string, passed bool, durationMs int64, errMsg string) { // Don't mark runFlow/repeat/retry as slow - they contain multiple steps isCompoundStep := strings.HasPrefix(desc, "runFlow:") || @@ -1595,13 +1642,14 @@ func executeAppiumSingleSession(cfg *RunConfig, flows []flow.Flow) (*executor.Ru WaitForIdleTimeout: cfg.WaitForIdleTimeout, TypingFrequency: cfg.TypingFrequency, DeviceInfo: &deviceInfo, - OnFlowStart: onFlowStart, + OnFlowStart: onFlowStartWithCloud(cfg), OnStepComplete: onStepComplete, OnNestedStep: onNestedStep, OnNestedFlowStart: onNestedFlowStart, - OnFlowEnd: onFlowEnd, + OnFlowEnd: onFlowEndWithCloud(cfg), }) + notifyCloudRunStart(cfg, len(flows)) return runner.Run(context.Background(), flows) } diff --git a/pkg/cloud/example_provider.go b/pkg/cloud/example_provider.go index cb10cfd..06729cd 100644 --- a/pkg/cloud/example_provider.go +++ b/pkg/cloud/example_provider.go @@ -39,6 +39,25 @@ func (p *exampleProvider) ExtractMeta(sessionID string, caps map[string]interfac meta["jobID"] = sessionID } +func (p *exampleProvider) OnRunStart(meta map[string]string, totalFlows int) error { + // TODO (optional): signal run start to your provider's dashboard + // (e.g., create a suite, tag the job with totalFlows, etc.) + return nil +} + +func (p *exampleProvider) OnFlowStart(meta map[string]string, flowIdx, totalFlows int, name, file string) error { + // TODO (optional): mark test case as "started" in your provider's dashboard + // flowIdx is 0-based, totalFlows is the count, name is the flow title + return nil +} + +func (p *exampleProvider) OnFlowEnd(meta map[string]string, result *FlowResult) error { + // TODO (optional): mark test case as completed (pass/fail) in your + // provider's dashboard. Useful for live updates before the run ends. + // result.Name, result.File, result.Passed, result.Duration, result.Error + return nil +} + func (p *exampleProvider) ReportResult(appiumURL string, meta map[string]string, result *TestResult) error { jobID := meta["jobID"] if jobID == "" { diff --git a/pkg/cloud/provider.go b/pkg/cloud/provider.go index d75a4ea..77a2858 100644 --- a/pkg/cloud/provider.go +++ b/pkg/cloud/provider.go @@ -18,6 +18,23 @@ type Provider interface { // from the session response; meta is the output map to populate. ExtractMeta(sessionID string, caps map[string]interface{}, meta map[string]string) + // OnRunStart fires once, after ExtractMeta and before the first flow runs. + // Providers can use this to record run-level metadata (tags, build ID, total + // flow count) with the upstream dashboard. Errors are logged but do not + // abort the run. + OnRunStart(meta map[string]string, totalFlows int) error + + // OnFlowStart fires before each flow begins executing. Providers can use + // this to mark a test case as "started" in the upstream dashboard. Errors + // are logged but do not abort the flow. + OnFlowStart(meta map[string]string, flowIdx, totalFlows int, name, file string) error + + // OnFlowEnd fires after each flow finishes (pass, fail, or skip). + // Providers can use this to mark a test case as completed in the upstream + // dashboard before the full run has finished. Errors are logged but do not + // abort subsequent flows. + OnFlowEnd(meta map[string]string, result *FlowResult) error + // ReportResult reports the test result to the cloud provider. // Called once after all flows and reports have completed. ReportResult(appiumURL string, meta map[string]string, result *TestResult) error diff --git a/pkg/cloud/provider_test.go b/pkg/cloud/provider_test.go index 9a99b86..c68f690 100644 --- a/pkg/cloud/provider_test.go +++ b/pkg/cloud/provider_test.go @@ -94,6 +94,106 @@ type testProvider struct { func (t *testProvider) Name() string { return t.name } func (t *testProvider) ExtractMeta(sessionID string, caps map[string]interface{}, meta map[string]string) { } +func (t *testProvider) OnRunStart(meta map[string]string, totalFlows int) error { return nil } +func (t *testProvider) OnFlowStart(meta map[string]string, flowIdx, totalFlows int, name, file string) error { + return nil +} +func (t *testProvider) OnFlowEnd(meta map[string]string, result *FlowResult) error { return nil } func (t *testProvider) ReportResult(appiumURL string, meta map[string]string, result *TestResult) error { return nil } + +// TestProvider_Lifecycle_FiresInOrder verifies the full lifecycle sequence: +// ExtractMeta → OnRunStart → OnFlowStart → OnFlowEnd (per flow) → ReportResult. +func TestProvider_Lifecycle_FiresInOrder(t *testing.T) { + type event struct { + kind string + idx int + name string + passed bool + } + var events []event + + p := &lifecycleProvider{ + extract: func() { events = append(events, event{kind: "extract"}) }, + runStart: func(total int) { + events = append(events, event{kind: "run", idx: total}) + }, + flowStart: func(flowIdx int, name string) { + events = append(events, event{kind: "flowStart", idx: flowIdx, name: name}) + }, + flowEnd: func(name string, passed bool) { + events = append(events, event{kind: "flowEnd", name: name, passed: passed}) + }, + report: func(passed bool) { events = append(events, event{kind: "report", passed: passed}) }, + } + + // Simulate the CLI lifecycle sequence for two flows. + meta := map[string]string{} + p.ExtractMeta("sess-1", nil, meta) + if err := p.OnRunStart(meta, 2); err != nil { + t.Fatalf("OnRunStart: %v", err) + } + if err := p.OnFlowStart(meta, 0, 2, "A", "a.yaml"); err != nil { + t.Fatalf("OnFlowStart A: %v", err) + } + if err := p.OnFlowEnd(meta, &FlowResult{Name: "A", File: "a.yaml", Passed: true}); err != nil { + t.Fatalf("OnFlowEnd A: %v", err) + } + if err := p.OnFlowStart(meta, 1, 2, "B", "b.yaml"); err != nil { + t.Fatalf("OnFlowStart B: %v", err) + } + if err := p.OnFlowEnd(meta, &FlowResult{Name: "B", File: "b.yaml", Passed: false}); err != nil { + t.Fatalf("OnFlowEnd B: %v", err) + } + if err := p.ReportResult("http://x", meta, &TestResult{Passed: false}); err != nil { + t.Fatalf("ReportResult: %v", err) + } + + want := []event{ + {kind: "extract"}, + {kind: "run", idx: 2}, + {kind: "flowStart", idx: 0, name: "A"}, + {kind: "flowEnd", name: "A", passed: true}, + {kind: "flowStart", idx: 1, name: "B"}, + {kind: "flowEnd", name: "B", passed: false}, + {kind: "report", passed: false}, + } + if len(events) != len(want) { + t.Fatalf("event count = %d, want %d (%+v)", len(events), len(want), events) + } + for i, e := range events { + if e != want[i] { + t.Errorf("event[%d] = %+v, want %+v", i, e, want[i]) + } + } +} + +type lifecycleProvider struct { + extract func() + runStart func(total int) + flowStart func(flowIdx int, name string) + flowEnd func(name string, passed bool) + report func(passed bool) +} + +func (p *lifecycleProvider) Name() string { return "lifecycle" } +func (p *lifecycleProvider) ExtractMeta(sessionID string, caps map[string]interface{}, meta map[string]string) { + p.extract() +} +func (p *lifecycleProvider) OnRunStart(meta map[string]string, totalFlows int) error { + p.runStart(totalFlows) + return nil +} +func (p *lifecycleProvider) OnFlowStart(meta map[string]string, flowIdx, totalFlows int, name, file string) error { + p.flowStart(flowIdx, name) + return nil +} +func (p *lifecycleProvider) OnFlowEnd(meta map[string]string, result *FlowResult) error { + p.flowEnd(result.Name, result.Passed) + return nil +} +func (p *lifecycleProvider) ReportResult(appiumURL string, meta map[string]string, result *TestResult) error { + p.report(result.Passed) + return nil +} diff --git a/pkg/cloud/saucelabs.go b/pkg/cloud/saucelabs.go index db54e99..ad9deb6 100644 --- a/pkg/cloud/saucelabs.go +++ b/pkg/cloud/saucelabs.go @@ -40,6 +40,24 @@ func (s *sauceLabs) ExtractMeta(sessionID string, caps map[string]interface{}, m } } +// OnRunStart is a no-op — Sauce Labs jobs are created server-side by the +// Appium session, so there's nothing to signal at run start. +func (s *sauceLabs) OnRunStart(meta map[string]string, totalFlows int) error { + return nil +} + +// OnFlowStart is a no-op — Sauce's dashboard renders test cases from the +// final result payload, so there's no per-flow API to ping at flow start. +func (s *sauceLabs) OnFlowStart(meta map[string]string, flowIdx, totalFlows int, name, file string) error { + return nil +} + +// OnFlowEnd is a no-op — Sauce's job result is updated once at run end via +// ReportResult. There's no per-test-case live update API. +func (s *sauceLabs) OnFlowEnd(meta map[string]string, result *FlowResult) error { + return nil +} + func (s *sauceLabs) ReportResult(appiumURL string, meta map[string]string, result *TestResult) error { jobID := strings.TrimSpace(meta["jobID"]) if jobID == "" { diff --git a/pkg/core/driver.go b/pkg/core/driver.go index 96ee67c..79696e4 100644 --- a/pkg/core/driver.go +++ b/pkg/core/driver.go @@ -1,6 +1,7 @@ package core import ( + "context" "time" "github.com/devicelab-dev/maestro-runner/pkg/flow" @@ -33,6 +34,11 @@ type Driver interface { // 0 = disabled, >0 = wait up to N ms for device to be idle. // This is used by waitForIdleTimeout in flow config. SetWaitForIdleTimeout(ms int) error + + // SetContext sets the parent context for element-finding operations. + // When set, driver polling loops use this as the parent context so that + // external cancellation (e.g. runFlow timeout) can interrupt them. + SetContext(ctx context.Context) } // CommandResult represents the outcome of executing a single command diff --git a/pkg/driver/appium/commands.go b/pkg/driver/appium/commands.go index 733e4e9..214bba6 100644 --- a/pkg/driver/appium/commands.go +++ b/pkg/driver/appium/commands.go @@ -252,6 +252,10 @@ func (d *Driver) scrollUntilVisible(step *flow.ScrollUntilVisibleStep) *core.Com } for i := 0; i < maxScrolls && time.Now().Before(deadline); i++ { + if err := d.parentContext().Err(); err != nil { + return errorResult(fmt.Errorf("scroll cancelled: %w", err), "") + } + // Check if element is visible info, err := d.findElement(step.Element, 1*time.Second) if err == nil && info != nil { @@ -737,7 +741,7 @@ func (d *Driver) waitUntil(step *flow.WaitUntilStep) *core.CommandResult { timeout = time.Duration(step.TimeoutMs) * time.Millisecond } - ctx, cancel := context.WithTimeout(context.Background(), timeout) + ctx, cancel := context.WithTimeout(d.parentContext(), timeout) defer cancel() var selector *flow.Selector diff --git a/pkg/driver/appium/driver.go b/pkg/driver/appium/driver.go index c970e59..a02f19c 100644 --- a/pkg/driver/appium/driver.go +++ b/pkg/driver/appium/driver.go @@ -22,6 +22,7 @@ type Driver struct { capabilities map[string]interface{} // stored for session recreation (deep copy of original) platform string // detected from page source or capabilities appID string // current app ID + ctx context.Context // parent context for element-finding operations (nil = context.Background()) findTimeout time.Duration // configurable timeout for finding elements currentWaitForIdleTimeout int // track current value to skip redundant calls waitForIdleTimeoutSet bool // whether waitForIdleTimeout has been set @@ -235,6 +236,19 @@ func (d *Driver) GetPlatformInfo() *core.PlatformInfo { } } +// SetContext sets the parent context for element-finding operations. +func (d *Driver) SetContext(ctx context.Context) { + d.ctx = ctx +} + +// parentContext returns the parent context for element-finding operations. +func (d *Driver) parentContext() context.Context { + if d.ctx != nil { + return d.ctx + } + return context.Background() +} + // SetFindTimeout implements core.Driver. // Sets the default timeout (in ms) for finding elements. func (d *Driver) SetFindTimeout(ms int) { @@ -306,6 +320,10 @@ func (d *Driver) findElement(sel flow.Selector, timeout time.Duration) (*core.El deadline := time.Now().Add(timeout) for { + if err := d.parentContext().Err(); err != nil { + return nil, fmt.Errorf("element '%s' not found: %w", sel.Describe(), err) + } + info, err := d.findElementDirect(sel) if err == nil && info != nil { return info, nil @@ -387,29 +405,34 @@ func (d *Driver) findElementDirect(sel flow.Selector) (*core.ElementInfo, error) return d.getElementInfo(elemID) } } else { - // Try exact text match first + // Try case-sensitive first (preserves existing behavior) uiSelector := fmt.Sprintf(`new UiSelector().text("%s")`, escaped) if elemID, err := d.client.FindElement("-android uiautomator", uiSelector); err == nil && elemID != "" { return d.getElementInfo(elemID) } - - // Try textContains uiSelector = fmt.Sprintf(`new UiSelector().textContains("%s")`, escaped) if elemID, err := d.client.FindElement("-android uiautomator", uiSelector); err == nil && elemID != "" { return d.getElementInfo(elemID) } - - // Try description (content-desc) uiSelector = fmt.Sprintf(`new UiSelector().description("%s")`, escaped) if elemID, err := d.client.FindElement("-android uiautomator", uiSelector); err == nil && elemID != "" { return d.getElementInfo(elemID) } - - // Try descriptionContains uiSelector = fmt.Sprintf(`new UiSelector().descriptionContains("%s")`, escaped) if elemID, err := d.client.FindElement("-android uiautomator", uiSelector); err == nil && elemID != "" { return d.getElementInfo(elemID) } + + // Case-insensitive fallback + ciPattern := fmt.Sprintf(`(?is).*\Q%s\E.*`, escaped) + uiSelector = fmt.Sprintf(`new UiSelector().textMatches("%s")`, ciPattern) + if elemID, err := d.client.FindElement("-android uiautomator", uiSelector); err == nil && elemID != "" { + return d.getElementInfo(elemID) + } + uiSelector = fmt.Sprintf(`new UiSelector().descriptionMatches("%s")`, ciPattern) + if elemID, err := d.client.FindElement("-android uiautomator", uiSelector); err == nil && elemID != "" { + return d.getElementInfo(elemID) + } } } } @@ -475,6 +498,10 @@ func (d *Driver) findElementByPageSource(sel flow.Selector) (*core.ElementInfo, func (d *Driver) findElementByPageSourceWithPolling(sel flow.Selector, timeout time.Duration) (*core.ElementInfo, error) { deadline := time.Now().Add(timeout) for { + if err := d.parentContext().Err(); err != nil { + return nil, fmt.Errorf("element '%s' not found: %w", sel.Describe(), err) + } + info, err := d.findElementByPageSource(sel) if err == nil { return info, nil @@ -511,6 +538,10 @@ func (d *Driver) findElementForTap(sel flow.Selector, timeout time.Duration) (*c deadline := time.Now().Add(timeout) for { + if err := d.parentContext().Err(); err != nil { + return nil, fmt.Errorf("element '%s' not found: %w", sel.Describe(), err) + } + var info *core.ElementInfo var err error @@ -539,29 +570,44 @@ func (d *Driver) findElementForTap(sel flow.Selector, timeout time.Duration) (*c // findElementForTapDirect finds element for tap, trying clickable first then fallback to page source. func (d *Driver) findElementForTapDirect(sel flow.Selector) (*core.ElementInfo, error) { escaped := escapeUIAutomatorString(sel.Text) + ciPattern := fmt.Sprintf(`(?is).*\Q%s\E.*`, escaped) - // Step 1: Try clickable elements first (fast path) - // Try textContains with clickable filter + // Step 1: Try clickable elements first — case-sensitive, then case-insensitive fallback uiSelector := fmt.Sprintf(`new UiSelector().textContains("%s").clickable(true)`, escaped) if elemID, err := d.client.FindElement("-android uiautomator", uiSelector); err == nil && elemID != "" { return d.getElementInfo(elemID) } - - // Try descriptionContains with clickable filter uiSelector = fmt.Sprintf(`new UiSelector().descriptionContains("%s").clickable(true)`, escaped) if elemID, err := d.client.FindElement("-android uiautomator", uiSelector); err == nil && elemID != "" { return d.getElementInfo(elemID) } + // Case-insensitive clickable fallback + uiSelector = fmt.Sprintf(`new UiSelector().textMatches("%s").clickable(true)`, ciPattern) + if elemID, err := d.client.FindElement("-android uiautomator", uiSelector); err == nil && elemID != "" { + return d.getElementInfo(elemID) + } + uiSelector = fmt.Sprintf(`new UiSelector().descriptionMatches("%s").clickable(true)`, ciPattern) + if elemID, err := d.client.FindElement("-android uiautomator", uiSelector); err == nil && elemID != "" { + return d.getElementInfo(elemID) + } // Step 2: Check if text exists at all (without clickable filter) uiSelector = fmt.Sprintf(`new UiSelector().textContains("%s")`, escaped) _, textExistsErr := d.client.FindElement("-android uiautomator", uiSelector) if textExistsErr != nil { - // Also try description uiSelector = fmt.Sprintf(`new UiSelector().descriptionContains("%s")`, escaped) _, textExistsErr = d.client.FindElement("-android uiautomator", uiSelector) } + if textExistsErr != nil { + // Case-insensitive fallback + uiSelector = fmt.Sprintf(`new UiSelector().textMatches("%s")`, ciPattern) + _, textExistsErr = d.client.FindElement("-android uiautomator", uiSelector) + } + if textExistsErr != nil { + uiSelector = fmt.Sprintf(`new UiSelector().descriptionMatches("%s")`, ciPattern) + _, textExistsErr = d.client.FindElement("-android uiautomator", uiSelector) + } if textExistsErr != nil { // Text not found via UiAutomator - try page source as fallback @@ -602,7 +648,7 @@ func (d *Driver) findElementForTapIOS(sel flow.Selector) (*core.ElementInfo, err // findElementRelative handles relative selectors (below, above, etc.) // Deprecated: Use findElementRelativeWithContext for new code. func (d *Driver) findElementRelative(sel flow.Selector, timeout time.Duration) (*core.ElementInfo, error) { - ctx, cancel := context.WithTimeout(context.Background(), timeout) + ctx, cancel := context.WithTimeout(d.parentContext(), timeout) defer cancel() return d.findElementRelativeWithContext(ctx, sel) } diff --git a/pkg/driver/browser/cdp/driver.go b/pkg/driver/browser/cdp/driver.go index 56013e1..232bb58 100644 --- a/pkg/driver/browser/cdp/driver.go +++ b/pkg/driver/browser/cdp/driver.go @@ -1,6 +1,7 @@ package cdp import ( + "context" "encoding/json" "fmt" "log" @@ -659,6 +660,9 @@ func (d *Driver) SetWaitForIdleTimeout(ms int) error { return nil } +// SetContext is a no-op for browser — Rod waits use their own timeouts. +func (d *Driver) SetContext(ctx context.Context) {} + // successResult creates a success result. func successResult(msg string, elem *core.ElementInfo) *core.CommandResult { return &core.CommandResult{ diff --git a/pkg/driver/devicelab/commands.go b/pkg/driver/devicelab/commands.go index 0798c1a..4e174b3 100644 --- a/pkg/driver/devicelab/commands.go +++ b/pkg/driver/devicelab/commands.go @@ -43,12 +43,18 @@ func (d *Driver) tapOn(step *flow.TapOnStep) *core.CommandResult { return d.tapOnBrowser(step) } - strategies, err := buildSelectors(step.Selector, 0) + // Clickable-first: prefer buttons over labels. The agent promotes a text + // match to its nearest clickable ancestor when .clickable(true) is set, + // so "tapOn: SIGN IN" hits the clickable login-button ViewGroup even + // when the text lives on a non-clickable TextView child. + clickableStrategies, _ := buildClickableOnlyStrategies(step.Selector) + allStrategies, err := buildSelectors(step.Selector, 0) if err != nil { return errorResult(err, fmt.Sprintf("Failed to build selectors: %v", err)) } + strategies := append(clickableStrategies, allStrategies...) timeout := d.calculateTimeout(step.IsOptional(), step.TimeoutMs) - ctx, cancel := context.WithTimeout(context.Background(), timeout) + ctx, cancel := context.WithTimeout(d.parentContext(), timeout) defer cancel() var lastErr error @@ -410,22 +416,9 @@ func (d *Driver) inputText(step *flow.InputTextStep) *core.CommandResult { } } } else { - focused, err := d.findFocused() - if err != nil { - // Fallback: try finding by focused selector - focusedTrue := true - focusedSel := flow.Selector{Focused: &focusedTrue} - _, _, findErr := d.findElement(focusedSel, false, 2000) - if findErr != nil { - return errorResult(err, "No focused element to type into") - } - // Re-try findFocused after finding focused element - focused, err = d.findFocused() - if err != nil { - return errorResult(err, "No focused element to type into") - } - } - if err := focused.Input(text); err != nil { + // No selector — send key events directly to whatever the OS has focused. + // Matches Maestro's behavior: pressKeyCode for each character. + if err := d.client.SendKeyActions(text); err != nil { return errorResult(err, fmt.Sprintf("Failed to input text: %v", err)) } } @@ -722,37 +715,11 @@ func (d *Driver) swipe(step *flow.SwipeStep) *core.CommandResult { } } + // No selector — use screen coordinates directly (matches Maestro behavior) width, height, err := d.screenSize() if err != nil { return errorResult(err, "Failed to get screen size") } - - scrollableInfo, scrollableCount := d.findScrollableElement(10000) - - if scrollableInfo != nil { - b := scrollableInfo.Bounds - fmt.Printf("[swipe] Found %d scrollable(s), using: bounds=[%d,%d,%d,%d]\n", - scrollableCount, b.X, b.Y, b.Width, b.Height) - - centerX := b.X + b.Width/2 - var startY, endY int - switch direction { - case "up": - startY = b.Y + b.Height*70/100 - endY = b.Y + b.Height*30/100 - case "down": - startY = b.Y + b.Height*30/100 - endY = b.Y + b.Height*70/100 - default: - startY = b.Y + b.Height*70/100 - endY = b.Y + b.Height*30/100 - } - - fmt.Printf("[swipe] Coords in scrollable: (%d,%d) → (%d,%d)\n", centerX, startY, centerX, endY) - return d.swipeWithAbsoluteCoords(centerX, startY, centerX, endY, step.Duration) - } - - fmt.Printf("[swipe] No scrollable found, using screen coordinates (50%% center)\n") return d.swipeWithMaestroCoordinates(direction, width, height, step.Duration) } @@ -1660,7 +1627,7 @@ func (d *Driver) waitUntil(step *flow.WaitUntilStep) *core.CommandResult { } timeout := time.Duration(timeoutMs) * time.Millisecond - ctx, cancel := context.WithTimeout(context.Background(), timeout) + ctx, cancel := context.WithTimeout(d.parentContext(), timeout) defer cancel() for { diff --git a/pkg/driver/devicelab/commands_test.go b/pkg/driver/devicelab/commands_test.go index 8275091..e154ece 100644 --- a/pkg/driver/devicelab/commands_test.go +++ b/pkg/driver/devicelab/commands_test.go @@ -59,6 +59,7 @@ func (m *mockDeviceLabClient) ForceStop(string) error { func (m *mockDeviceLabClient) ClearAppData(string) error { return nil } func (m *mockDeviceLabClient) GrantPermissions(string, []string) error { return nil } func (m *mockDeviceLabClient) SetAppiumSettings(map[string]interface{}) error { return nil } +func (m *mockDeviceLabClient) WaitForSettle(int, int) (bool, error) { return true, nil } // Compile-time check var _ DeviceLabClient = (*mockDeviceLabClient)(nil) diff --git a/pkg/driver/devicelab/driver.go b/pkg/driver/devicelab/driver.go index 08a6549..af05430 100644 --- a/pkg/driver/devicelab/driver.go +++ b/pkg/driver/devicelab/driver.go @@ -66,6 +66,9 @@ type DeviceLabClient interface { // Settings SetAppiumSettings(settings map[string]interface{}) error + + // Settle detection + WaitForSettle(timeoutMs, quietMs int) (bool, error) } // Driver implements core.Driver using the DeviceLab Android Driver. @@ -74,6 +77,9 @@ type Driver struct { info *core.PlatformInfo device ShellExecutor // for ADB commands (fallback) + // Parent context for element-finding operations (nil = context.Background()) + ctx context.Context + // Timeouts (0 = use defaults) findTimeout int // ms, for required elements optionalFindTimeout int // ms, for optional elements @@ -104,6 +110,11 @@ func New(client DeviceLabClient, info *core.PlatformInfo, device ShellExecutor) } } +// WaitForSettle delegates to the on-device agent's tree-comparison settle detection. +func (d *Driver) WaitForSettle(timeoutMs, quietMs int) (bool, error) { + return d.client.WaitForSettle(timeoutMs, quietMs) +} + // SetCDPStateFunc sets the function used to retrieve real-time CDP socket state // from the background monitor. func (d *Driver) SetCDPStateFunc(fn func() *core.CDPInfo) { @@ -140,6 +151,20 @@ func (d *Driver) screenSize() (int, int, error) { return 0, 0, fmt.Errorf("screen dimensions not available") } +// SetContext sets the parent context for element-finding operations. +func (d *Driver) SetContext(ctx context.Context) { + d.ctx = ctx +} + +// parentContext returns the parent context for element-finding operations. +// Returns context.Background() if no context was set. +func (d *Driver) parentContext() context.Context { + if d.ctx != nil { + return d.ctx + } + return context.Background() +} + // SetFindTimeout sets the timeout for finding required elements. func (d *Driver) SetFindTimeout(ms int) { d.findTimeout = ms @@ -539,7 +564,7 @@ func (d *Driver) findElementForTap(sel flow.Selector, optional bool, stepTimeout // For relative selectors (below, above, etc.), use page source which handles them correctly if sel.HasRelativeSelector() { timeout := d.calculateTimeout(optional, stepTimeoutMs) - ctx, cancel := context.WithTimeout(context.Background(), timeout) + ctx, cancel := context.WithTimeout(d.parentContext(), timeout) defer cancel() return d.findElementRelativeWithContext(ctx, sel) } @@ -558,7 +583,7 @@ func (d *Driver) findElementForTap(sel flow.Selector, optional bool, stepTimeout // Non-clickable strategies first, then clickable fallback if sel.Text != "" { timeout := d.calculateTimeout(optional, stepTimeoutMs) - ctx, cancel := context.WithTimeout(context.Background(), timeout) + ctx, cancel := context.WithTimeout(d.parentContext(), timeout) defer cancel() return d.findElementDirectWithContext(ctx, sel) } @@ -570,26 +595,27 @@ func (d *Driver) findElementForTap(sel flow.Selector, optional bool, stepTimeout // findElementDirectWithContext finds an element for tap using only UiAutomator strategies. // Non-clickable strategies first (fastest match), then clickable fallback. No page source. func (d *Driver) findElementDirectWithContext(ctx context.Context, sel flow.Selector) (*uiautomator2.Element, *core.ElementInfo, error) { - // Non-clickable first: finds element in 1 round-trip when it exists. - // Clickable strategies appended as fallback for disambiguation. - allStrategies, _ := buildSelectors(sel, 0) + // Clickable first: for tap commands, prefer buttons over labels. + // General strategies as fallback when no clickable element matches. clickableStrategies, _ := buildClickableOnlyStrategies(sel) - combined := append(allStrategies, clickableStrategies...) + allStrategies, _ := buildSelectors(sel, 0) + combined := append(clickableStrategies, allStrategies...) - // When text triggers regex detection, also add literal textContains/descriptionContains + // When text triggers regex detection, also add literal textMatches // as fallback. Handles false positives like "alice@example.com (locked out)" where // parentheses trigger regex detection but the text is actually literal. if looksLikeRegex(sel.Text) { escaped := escapeUIAutomatorString(sel.Text) stateFilters := buildStateFilters(sel) + pattern := `(?is).*\Q` + escaped + `\E.*` combined = append(combined, LocatorStrategy{ Strategy: uiautomator2.StrategyUIAutomator, - Value: `new UiSelector().textContains("` + escaped + `")` + stateFilters, + Value: `new UiSelector().textMatches("` + pattern + `")` + stateFilters, }, LocatorStrategy{ Strategy: uiautomator2.StrategyUIAutomator, - Value: `new UiSelector().descriptionContains("` + escaped + `")` + stateFilters, + Value: `new UiSelector().descriptionMatches("` + pattern + `")` + stateFilters, }, ) } @@ -641,7 +667,9 @@ func buildClickableOnlyStrategies(sel flow.Selector) ([]LocatorStrategy, error) if sel.Text != "" { escaped := escapeUIAutomatorString(sel.Text) - // Always try textContains/descriptionContains first (no regex needed) + // Case-sensitive text/description/hint first, then case-insensitive fallback. + // hintContains is a DeviceLab-agent extension — matches EditText android:hint + // placeholder so "tapOn: 'Email'" finds an empty field by its hint text. strategies = append(strategies, LocatorStrategy{ Strategy: uiautomator2.StrategyUIAutomator, Value: `new UiSelector().textContains("` + escaped + `").clickable(true)` + stateFilters, @@ -650,6 +678,23 @@ func buildClickableOnlyStrategies(sel flow.Selector) ([]LocatorStrategy, error) Strategy: uiautomator2.StrategyUIAutomator, Value: `new UiSelector().descriptionContains("` + escaped + `").clickable(true)` + stateFilters, }) + strategies = append(strategies, LocatorStrategy{ + Strategy: uiautomator2.StrategyUIAutomator, + Value: `new UiSelector().hintContains("` + escaped + `").clickable(true)` + stateFilters, + }) + ciPattern := `(?is).*\Q` + escaped + `\E.*` + strategies = append(strategies, LocatorStrategy{ + Strategy: uiautomator2.StrategyUIAutomator, + Value: `new UiSelector().textMatches("` + ciPattern + `").clickable(true)` + stateFilters, + }) + strategies = append(strategies, LocatorStrategy{ + Strategy: uiautomator2.StrategyUIAutomator, + Value: `new UiSelector().descriptionMatches("` + ciPattern + `").clickable(true)` + stateFilters, + }) + strategies = append(strategies, LocatorStrategy{ + Strategy: uiautomator2.StrategyUIAutomator, + Value: `new UiSelector().hintMatches("` + ciPattern + `").clickable(true)` + stateFilters, + }) // Fall back to regex match (case-insensitive) for partial/pattern matches if looksLikeRegex(sel.Text) { regexEscaped := escapeUIAutomator(sel.Text) @@ -675,7 +720,7 @@ func buildClickableOnlyStrategies(sel flow.Selector) ([]LocatorStrategy, error) // findElementWithOptions is the internal implementation with clickable preference option. func (d *Driver) findElementWithOptions(sel flow.Selector, optional bool, stepTimeoutMs int, preferClickable bool, fastMode bool) (*uiautomator2.Element, *core.ElementInfo, error) { timeout := d.calculateTimeout(optional, stepTimeoutMs) - ctx, cancel := context.WithTimeout(context.Background(), timeout) + ctx, cancel := context.WithTimeout(d.parentContext(), timeout) defer cancel() return d.findElementWithContext(ctx, sel, preferClickable, fastMode) @@ -1314,10 +1359,12 @@ func buildSelectorsWithOptions(sel flow.Selector, timeoutMs int, preferClickable }) } - // Text-based selector + // Text-based selector: case-sensitive first, case-insensitive fallback. + // hintContains / hintMatches are DeviceLab-agent extensions — match EditText + // android:hint placeholder so "tapOn: 'Email'" finds an empty field by hint. if sel.Text != "" { escaped := escapeUIAutomatorString(sel.Text) - // Always try textContains/descriptionContains first (no regex needed, handles special chars) + ciPattern := `(?is).*\Q` + escaped + `\E.*` if preferClickable { strategies = append(strategies, LocatorStrategy{ Strategy: uiautomator2.StrategyUIAutomator, @@ -1327,6 +1374,10 @@ func buildSelectorsWithOptions(sel flow.Selector, timeoutMs int, preferClickable Strategy: uiautomator2.StrategyUIAutomator, Value: `new UiSelector().descriptionContains("` + escaped + `").clickable(true)` + stateFilters, }) + strategies = append(strategies, LocatorStrategy{ + Strategy: uiautomator2.StrategyUIAutomator, + Value: `new UiSelector().hintContains("` + escaped + `").clickable(true)` + stateFilters, + }) } strategies = append(strategies, LocatorStrategy{ Strategy: uiautomator2.StrategyUIAutomator, @@ -1336,6 +1387,37 @@ func buildSelectorsWithOptions(sel flow.Selector, timeoutMs int, preferClickable Strategy: uiautomator2.StrategyUIAutomator, Value: `new UiSelector().descriptionContains("` + escaped + `")` + stateFilters, }) + strategies = append(strategies, LocatorStrategy{ + Strategy: uiautomator2.StrategyUIAutomator, + Value: `new UiSelector().hintContains("` + escaped + `")` + stateFilters, + }) + // Case-insensitive fallback + if preferClickable { + strategies = append(strategies, LocatorStrategy{ + Strategy: uiautomator2.StrategyUIAutomator, + Value: `new UiSelector().textMatches("` + ciPattern + `").clickable(true)` + stateFilters, + }) + strategies = append(strategies, LocatorStrategy{ + Strategy: uiautomator2.StrategyUIAutomator, + Value: `new UiSelector().descriptionMatches("` + ciPattern + `").clickable(true)` + stateFilters, + }) + strategies = append(strategies, LocatorStrategy{ + Strategy: uiautomator2.StrategyUIAutomator, + Value: `new UiSelector().hintMatches("` + ciPattern + `").clickable(true)` + stateFilters, + }) + } + strategies = append(strategies, LocatorStrategy{ + Strategy: uiautomator2.StrategyUIAutomator, + Value: `new UiSelector().textMatches("` + ciPattern + `")` + stateFilters, + }) + strategies = append(strategies, LocatorStrategy{ + Strategy: uiautomator2.StrategyUIAutomator, + Value: `new UiSelector().descriptionMatches("` + ciPattern + `")` + stateFilters, + }) + strategies = append(strategies, LocatorStrategy{ + Strategy: uiautomator2.StrategyUIAutomator, + Value: `new UiSelector().hintMatches("` + ciPattern + `")` + stateFilters, + }) // Fall back to regex match (case-insensitive) with proper escaping if looksLikeRegex(sel.Text) { regexEscaped := escapeUIAutomator(sel.Text) diff --git a/pkg/driver/mock/mock.go b/pkg/driver/mock/mock.go index 6b5e411..fbe6b44 100644 --- a/pkg/driver/mock/mock.go +++ b/pkg/driver/mock/mock.go @@ -2,6 +2,7 @@ package mock import ( + "context" "fmt" "time" @@ -145,6 +146,11 @@ func (d *Driver) SetWaitForIdleTimeout(ms int) error { return nil } +// SetContext is a no-op for mock driver. +func (d *Driver) SetContext(ctx context.Context) { + // Mock driver doesn't use context +} + // needsElement returns true if the step type typically returns element info. func needsElement(step flow.Step) bool { switch step.Type() { diff --git a/pkg/driver/uiautomator2/commands.go b/pkg/driver/uiautomator2/commands.go index e964213..c4cc2c2 100644 --- a/pkg/driver/uiautomator2/commands.go +++ b/pkg/driver/uiautomator2/commands.go @@ -258,23 +258,9 @@ func (d *Driver) inputText(step *flow.InputTextStep) *core.CommandResult { return errorResult(err, fmt.Sprintf("Failed to input text: %v", err)) } } else { - // Type into focused element - // First try WebDriver activeElement endpoint - active, err := d.client.ActiveElement() - if err != nil { - // Fallback: find element with focused=true via page source - focusedTrue := true - focusedSel := flow.Selector{Focused: &focusedTrue} - elem, _, findErr := d.findElement(focusedSel, false, 2000) - if findErr != nil { - return errorResult(err, "No focused element to type into") - } - if err := elem.SendKeys(text); err != nil { - return errorResult(err, fmt.Sprintf("Failed to input text: %v", err)) - } - return successResult(fmt.Sprintf("Entered text: %s%s", text, unicodeWarning), nil) - } - if err := active.SendKeys(text); err != nil { + // No selector — send key events directly to whatever the OS has focused. + // Matches Maestro's behavior: pressKeyCode for each character. + if err := d.client.SendKeyActions(text); err != nil { return errorResult(err, fmt.Sprintf("Failed to input text: %v", err)) } } @@ -494,58 +480,11 @@ func (d *Driver) swipe(step *flow.SwipeStep) *core.CommandResult { } } - // Get screen size + // No selector — use screen coordinates directly (matches Maestro behavior) width, height, err := d.screenSize() if err != nil { return errorResult(err, "Failed to get screen size") } - - // No selector specified - try to find a scrollable element - // Wait up to 10 seconds for page to load and find scrollable - scrollableInfo, scrollableCount := d.findScrollableElement(10000) - - // Print debug info about scrollable elements found - if scrollableInfo != nil { - b := scrollableInfo.Bounds - fmt.Printf("[swipe] Found %d scrollable(s), using: bounds=[%d,%d,%d,%d]\n", - scrollableCount, b.X, b.Y, b.Width, b.Height) - - // Swipe within scrollable bounds using Maestro Android coordinates - var sX, sY, eX, eY int - switch direction { - case "up": - sX = b.X + b.Width*50/100 - sY = b.Y + b.Height*50/100 - eX = b.X + b.Width*50/100 - eY = b.Y + b.Height*10/100 - case "down": - sX = b.X + b.Width*50/100 - sY = b.Y + b.Height*20/100 - eX = b.X + b.Width*50/100 - eY = b.Y + b.Height*90/100 - case "left": - sX = b.X + b.Width*90/100 - sY = b.Y + b.Height*50/100 - eX = b.X + b.Width*10/100 - eY = b.Y + b.Height*50/100 - case "right": - sX = b.X + b.Width*10/100 - sY = b.Y + b.Height*50/100 - eX = b.X + b.Width*90/100 - eY = b.Y + b.Height*50/100 - default: - sX = b.X + b.Width*50/100 - sY = b.Y + b.Height*50/100 - eX = b.X + b.Width*50/100 - eY = b.Y + b.Height*10/100 - } - - fmt.Printf("[swipe] Coords in scrollable: (%d,%d) → (%d,%d)\n", sX, sY, eX, eY) - return d.swipeWithAbsoluteCoords(sX, sY, eX, eY, step.Duration) - } - - fmt.Printf("[swipe] No scrollable found, using screen coordinates (50%% center)\n") - // Fallback: Use coordinates starting from 50% center return d.swipeWithMaestroCoordinates(direction, width, height, step.Duration) } @@ -1483,7 +1422,7 @@ func (d *Driver) waitUntil(step *flow.WaitUntilStep) *core.CommandResult { timeout = time.Duration(step.TimeoutMs) * time.Millisecond } - ctx, cancel := context.WithTimeout(context.Background(), timeout) + ctx, cancel := context.WithTimeout(d.parentContext(), timeout) defer cancel() // Determine selector for error messages diff --git a/pkg/driver/uiautomator2/driver.go b/pkg/driver/uiautomator2/driver.go index da851e7..a565ed7 100644 --- a/pkg/driver/uiautomator2/driver.go +++ b/pkg/driver/uiautomator2/driver.go @@ -66,6 +66,9 @@ type Driver struct { info *core.PlatformInfo device ShellExecutor // for ADB commands (launchApp, stopApp, clearState) + // Parent context for element-finding operations (nil = context.Background()) + ctx context.Context + // Timeouts (0 = use defaults) findTimeout int // ms, for required elements optionalFindTimeout int // ms, for optional elements @@ -95,6 +98,19 @@ func (d *Driver) screenSize() (int, int, error) { return 0, 0, fmt.Errorf("screen dimensions not available") } +// SetContext sets the parent context for element-finding operations. +func (d *Driver) SetContext(ctx context.Context) { + d.ctx = ctx +} + +// parentContext returns the parent context for element-finding operations. +func (d *Driver) parentContext() context.Context { + if d.ctx != nil { + return d.ctx + } + return context.Background() +} + // SetFindTimeout sets the timeout for finding required elements. // Useful for testing with shorter timeouts. func (d *Driver) SetFindTimeout(ms int) { @@ -309,7 +325,7 @@ func (d *Driver) findElementForTap(sel flow.Selector, optional bool, stepTimeout // For relative selectors (below, above, etc.), use page source which handles them correctly if sel.HasRelativeSelector() { timeout := d.calculateTimeout(optional, stepTimeoutMs) - ctx, cancel := context.WithTimeout(context.Background(), timeout) + ctx, cancel := context.WithTimeout(d.parentContext(), timeout) defer cancel() return d.findElementRelativeWithContext(ctx, sel) } @@ -327,7 +343,7 @@ func (d *Driver) findElementForTap(sel flow.Selector, optional bool, stepTimeout // For text-based selectors, use smart fallback strategy if sel.Text != "" { timeout := d.calculateTimeout(optional, stepTimeoutMs) - ctx, cancel := context.WithTimeout(context.Background(), timeout) + ctx, cancel := context.WithTimeout(d.parentContext(), timeout) defer cancel() return d.findElementForTapWithContext(ctx, sel) @@ -431,7 +447,7 @@ func buildClickableOnlyStrategies(sel flow.Selector) ([]LocatorStrategy, error) // Set fastMode=true for visibility checks (1 HTTP call), false for full info (3 HTTP calls). func (d *Driver) findElementWithOptions(sel flow.Selector, optional bool, stepTimeoutMs int, preferClickable bool, fastMode bool) (*uiautomator2.Element, *core.ElementInfo, error) { timeout := d.calculateTimeout(optional, stepTimeoutMs) - ctx, cancel := context.WithTimeout(context.Background(), timeout) + ctx, cancel := context.WithTimeout(d.parentContext(), timeout) defer cancel() return d.findElementWithContext(ctx, sel, preferClickable, fastMode) @@ -1098,9 +1114,11 @@ func buildSelectorsWithOptions(sel flow.Selector, timeoutMs int, preferClickable Value: `new UiSelector().descriptionMatches("` + pattern + `")` + stateFilters, }) } else { - // Use textContains for literal text (case-insensitive by default) - // Escape only quotes for the string value + // Literal text: try case-sensitive textContains first (preserves existing behavior), + // then fall back to case-insensitive textMatches for cases like Android dialog + // buttons where textAllCaps displays "CANCEL" but hierarchy text is "Cancel". escaped := escapeUIAutomatorString(sel.Text) + ciPattern := `(?is).*\Q` + escaped + `\E.*` if preferClickable { strategies = append(strategies, LocatorStrategy{ Strategy: uiautomator2.StrategyUIAutomator, @@ -1119,6 +1137,25 @@ func buildSelectorsWithOptions(sel flow.Selector, timeoutMs int, preferClickable Strategy: uiautomator2.StrategyUIAutomator, Value: `new UiSelector().descriptionContains("` + escaped + `")` + stateFilters, }) + // Case-insensitive fallback + if preferClickable { + strategies = append(strategies, LocatorStrategy{ + Strategy: uiautomator2.StrategyUIAutomator, + Value: `new UiSelector().textMatches("` + ciPattern + `").clickable(true)` + stateFilters, + }) + strategies = append(strategies, LocatorStrategy{ + Strategy: uiautomator2.StrategyUIAutomator, + Value: `new UiSelector().descriptionMatches("` + ciPattern + `").clickable(true)` + stateFilters, + }) + } + strategies = append(strategies, LocatorStrategy{ + Strategy: uiautomator2.StrategyUIAutomator, + Value: `new UiSelector().textMatches("` + ciPattern + `")` + stateFilters, + }) + strategies = append(strategies, LocatorStrategy{ + Strategy: uiautomator2.StrategyUIAutomator, + Value: `new UiSelector().descriptionMatches("` + ciPattern + `")` + stateFilters, + }) } } diff --git a/pkg/driver/uiautomator2/driver_test.go b/pkg/driver/uiautomator2/driver_test.go index 5220b2e..1cfc149 100644 --- a/pkg/driver/uiautomator2/driver_test.go +++ b/pkg/driver/uiautomator2/driver_test.go @@ -204,55 +204,59 @@ func (m *MockShellExecutor) Shell(cmd string) (string, error) { // ============================================================================ func TestBuildSelectorsText(t *testing.T) { - // Literal text uses textContains (not regex) + // Literal text: textContains first, then case-insensitive textMatches fallback sel := flow.Selector{Text: "Login"} strategies, err := buildSelectors(sel, 5000) if err != nil { t.Fatalf("buildSelectors failed: %v", err) } - // Should have 2 strategies: text and description - if len(strategies) != 2 { - t.Errorf("expected 2 strategies, got %d", len(strategies)) + // Should have 4 strategies: textContains, descriptionContains, textMatches(ci), descriptionMatches(ci) + if len(strategies) != 4 { + t.Errorf("expected 4 strategies, got %d", len(strategies)) } - // First should be text-based with textContains (for literal text) + // First should be case-sensitive textContains (preserves existing behavior) s := strategies[0] if !strings.Contains(s.Value, "textContains") { - t.Errorf("expected textContains in selector, got: %s", s.Value) + t.Errorf("expected textContains as first strategy, got: %s", s.Value) } - if !strings.Contains(s.Value, `"Login"`) { - t.Errorf("expected Login in selector, got: %s", s.Value) + + // Third should be case-insensitive textMatches fallback + s = strategies[2] + if !strings.Contains(s.Value, "textMatches") { + t.Errorf("expected textMatches as fallback, got: %s", s.Value) + } + if !strings.Contains(s.Value, `\QLogin\E`) { + t.Errorf("expected \\QLogin\\E in fallback, got: %s", s.Value) } } func TestBuildSelectorsTextWithPeriod(t *testing.T) { - // Text with period (domain name) uses textContains, not regex + // Text with period: textContains first, then textMatches with \Q\E fallback sel := flow.Selector{Text: "Join mastodon.social"} strategies, err := buildSelectors(sel, 5000) if err != nil { t.Fatalf("buildSelectors failed: %v", err) } - // Should have 2 strategies: text and description - if len(strategies) != 2 { - t.Errorf("expected 2 strategies, got %d", len(strategies)) + if len(strategies) != 4 { + t.Errorf("expected 4 strategies, got %d", len(strategies)) } - // Should use textContains, NOT textMatches + // First: case-sensitive textContains s := strategies[0] if !strings.Contains(s.Value, "textContains") { - t.Errorf("expected textContains in selector, got: %s", s.Value) - } - if strings.Contains(s.Value, "textMatches") { - t.Errorf("should NOT use textMatches for literal text, got: %s", s.Value) - } - // The period should NOT be escaped (textContains doesn't use regex) - if strings.Contains(s.Value, `\.`) { - t.Errorf("period should not be escaped in textContains, got: %s", s.Value) + t.Errorf("expected textContains first, got: %s", s.Value) } if !strings.Contains(s.Value, "mastodon.social") { - t.Errorf("expected literal text 'mastodon.social' in selector, got: %s", s.Value) + t.Errorf("expected literal text in textContains, got: %s", s.Value) + } + + // Third: case-insensitive fallback with \Q\E + s = strategies[2] + if !strings.Contains(s.Value, `\QJoin mastodon.social\E`) { + t.Errorf("expected literal text in \\Q\\E fallback, got: %s", s.Value) } } diff --git a/pkg/driver/wda/commands.go b/pkg/driver/wda/commands.go index 5f3103d..65aecf1 100644 --- a/pkg/driver/wda/commands.go +++ b/pkg/driver/wda/commands.go @@ -330,7 +330,7 @@ func (d *Driver) waitForAlert(timeoutMs int, accept bool) *core.CommandResult { timeoutMs = 5000 } timeout := time.Duration(timeoutMs) * time.Millisecond - ctx, cancel := context.WithTimeout(context.Background(), timeout) + ctx, cancel := context.WithTimeout(d.parentContext(), timeout) defer cancel() action := "accept" @@ -1017,7 +1017,7 @@ func (d *Driver) waitUntil(step *flow.WaitUntilStep) *core.CommandResult { } timeout := time.Duration(timeoutMs) * time.Millisecond - ctx, cancel := context.WithTimeout(context.Background(), timeout) + ctx, cancel := context.WithTimeout(d.parentContext(), timeout) defer cancel() // Determine selector for error messages diff --git a/pkg/driver/wda/driver.go b/pkg/driver/wda/driver.go index e24d517..76281e9 100644 --- a/pkg/driver/wda/driver.go +++ b/pkg/driver/wda/driver.go @@ -18,6 +18,9 @@ type Driver struct { info *core.PlatformInfo udid string // Device UDID for simctl commands + // Parent context for element-finding operations (nil = context.Background()) + ctx context.Context + // App file path for clearState (uninstall+reinstall) appFile string @@ -71,6 +74,19 @@ func (d *Driver) screenSize() (int, int, error) { return 0, 0, fmt.Errorf("screen dimensions not available") } +// SetContext sets the parent context for element-finding operations. +func (d *Driver) SetContext(ctx context.Context) { + d.ctx = ctx +} + +// parentContext returns the parent context for element-finding operations. +func (d *Driver) parentContext() context.Context { + if d.ctx != nil { + return d.ctx + } + return context.Background() +} + // SetFindTimeout sets the timeout for finding required elements. func (d *Driver) SetFindTimeout(ms int) { d.findTimeout = ms @@ -272,7 +288,7 @@ func (d *Driver) findElement(sel flow.Selector, optional bool, stepTimeoutMs int } timeout := d.calculateTimeout(optional, stepTimeoutMs) - ctx, cancel := context.WithTimeout(context.Background(), timeout) + ctx, cancel := context.WithTimeout(d.parentContext(), timeout) defer cancel() return d.findElementWithContext(ctx, sel) @@ -321,7 +337,7 @@ func (d *Driver) findElementForTap(sel flow.Selector, optional bool, stepTimeout // For relative selectors, use page source which handles them correctly if sel.HasRelativeSelector() { timeout := d.calculateTimeout(optional, stepTimeoutMs) - ctx, cancel := context.WithTimeout(context.Background(), timeout) + ctx, cancel := context.WithTimeout(d.parentContext(), timeout) defer cancel() return d.findElementRelativeWithContext(ctx, sel) } @@ -340,7 +356,7 @@ func (d *Driver) findElementForTap(sel flow.Selector, optional bool, stepTimeout // For text-based selectors, use smart fallback strategy if sel.Text != "" { timeout := d.calculateTimeout(optional, stepTimeoutMs) - ctx, cancel := context.WithTimeout(context.Background(), timeout) + ctx, cancel := context.WithTimeout(d.parentContext(), timeout) defer cancel() return d.findElementForTapWithContext(ctx, sel) } diff --git a/pkg/executor/flow_runner.go b/pkg/executor/flow_runner.go index f694cc3..4eaf639 100644 --- a/pkg/executor/flow_runner.go +++ b/pkg/executor/flow_runner.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "path/filepath" + "strings" "time" "github.com/devicelab-dev/maestro-runner/pkg/core" @@ -31,6 +32,10 @@ type FlowRunner struct { stepsSkipped int // Sub-command tracking for compound steps (runFlow, repeat, retry) subCommands []report.Command + // Effective wait-for-idle timeout (0 = disabled, used to skip settle) + waitForIdleTimeout int + // Active runFlow timeout label (e.g. "3s") for enriching sub-step errors + runFlowTimeout string } // Run executes the flow and returns the result. @@ -48,6 +53,9 @@ func (fr *FlowRunner) Run() FlowResult { fr.script = NewScriptEngine() defer fr.script.Close() + // Set parent context on driver so element-finding respects cancellation + fr.driver.SetContext(fr.ctx) + // Import system environment variables fr.script.ImportSystemEnv() @@ -74,7 +82,10 @@ func (fr *FlowRunner) Run() FlowResult { if appID := fr.flow.Config.EffectiveAppID(); appID != "" { fr.script.SetVariable("APP_ID", appID) } - fr.script.SetVariables(fr.flow.Config.Env) + // Expand flow config env values to support ${VAR || "default"} syntax + for k, v := range fr.flow.Config.Env { + fr.script.SetVariable(k, fr.script.ExpandVariables(v)) + } // Apply commandTimeout if specified - overrides driver's default find timeout if fr.flow.Config.CommandTimeout > 0 { @@ -89,6 +100,7 @@ func (fr *FlowRunner) Run() FlowResult { if fr.flow.Config.WaitForIdleTimeout != nil { waitForIdleTimeout = *fr.flow.Config.WaitForIdleTimeout // flow override (highest priority) } + fr.waitForIdleTimeout = waitForIdleTimeout if err := fr.driver.SetWaitForIdleTimeout(waitForIdleTimeout); err != nil { // Log warning but continue - some drivers don't support this _ = err // ignore error, just continue @@ -626,6 +638,23 @@ func (fr *FlowRunner) executeRunFlow(step *flow.RunFlowStep) *core.CommandResult } } + // Apply runFlow timeout if specified + if step.TimeoutMs > 0 { + timeout := time.Duration(step.TimeoutMs) * time.Millisecond + ctx, cancel := context.WithTimeout(fr.ctx, timeout) + defer cancel() + origCtx := fr.ctx + origTimeout := fr.runFlowTimeout + fr.ctx = ctx + fr.runFlowTimeout = timeout.String() + fr.driver.SetContext(ctx) + defer func() { + fr.ctx = origCtx + fr.runFlowTimeout = origTimeout + fr.driver.SetContext(origCtx) + }() + } + // Report nested flow start if fr.config.OnNestedFlowStart != nil && step.File != "" { fr.config.OnNestedFlowStart(fr.depth+1, "Run "+step.File) @@ -638,12 +667,28 @@ func (fr *FlowRunner) executeRunFlow(step *flow.RunFlowStep) *core.CommandResult // Apply env variables with restore defer fr.script.withEnvVars(step.Env)() + // Format timeout limit for error messages + timeoutLimit := "" + if step.TimeoutMs > 0 { + timeoutLimit = (time.Duration(step.TimeoutMs) * time.Millisecond).String() + } + // Execute inline steps if present if len(step.Steps) > 0 { + var lastStep string for _, nestedStep := range step.Steps { + if fr.ctx.Err() != nil { + msg := fmt.Sprintf("runFlow timed out (%s timeout) — last completed: %s", timeoutLimit, lastStep) + return &core.CommandResult{ + Success: false, + Error: fmt.Errorf("%s", msg), + Message: msg, + } + } + lastStep = nestedStep.Describe() result := fr.executeNestedStep(nestedStep) if !result.Success && !nestedStep.IsOptional() { - return result + return fr.wrapRunFlowTimeout(result, step, lastStep, timeoutLimit) } } return &core.CommandResult{ @@ -671,7 +716,62 @@ func (fr *FlowRunner) executeRunFlow(step *flow.RunFlowStep) *core.CommandResult } } - return fr.executeSubFlow(*subFlow) + result := fr.executeSubFlow(*subFlow) + if !result.Success { + return fr.wrapRunFlowTimeout(result, step, "", timeoutLimit) + } + return result +} + +// wrapRunFlowTimeout replaces cryptic context errors with a clear timeout message +// when a runFlow step fails due to its timeout expiring. +func (fr *FlowRunner) wrapRunFlowTimeout(result *core.CommandResult, step *flow.RunFlowStep, lastStep, timeoutLimit string) *core.CommandResult { + if timeoutLimit == "" || fr.ctx.Err() == nil { + return result // not a timeout — return original error + } + + // Build informative message: include timeout value, flow file, and what was running + var msg string + if step.File != "" { + msg = fmt.Sprintf("runFlow '%s' timed out (%s timeout)", step.File, timeoutLimit) + } else { + msg = fmt.Sprintf("runFlow timed out (%s timeout)", timeoutLimit) + } + if lastStep != "" { + msg += fmt.Sprintf(" while executing: %s", lastStep) + } + + return &core.CommandResult{ + Success: false, + Error: fmt.Errorf("%s", msg), + Message: msg, + } +} + +// enrichTimeoutError replaces cryptic "context deadline exceeded" in sub-step errors +// with a message that references the active runFlow timeout. +func (fr *FlowRunner) enrichTimeoutError(result *core.CommandResult) *core.CommandResult { + timeout := fr.runFlowTimeout + enriched := *result // shallow copy + + reason := fmt.Sprintf("runFlow timeout (%s) exceeded", timeout) + + if enriched.Error != nil { + orig := enriched.Error.Error() + // Replace the cryptic Go context error with timeout context + cleaned := strings.ReplaceAll(orig, "context deadline exceeded", reason) + if cleaned == orig { + // No replacement happened — append the timeout info + cleaned = orig + " (" + reason + ")" + } + enriched.Error = fmt.Errorf("%s", cleaned) + } + + if enriched.Message != "" { + enriched.Message = strings.ReplaceAll(enriched.Message, "context deadline exceeded", reason) + } + + return &enriched } // executeNestedStep executes a step without report tracking (for nested execution). @@ -805,6 +905,11 @@ func (fr *FlowRunner) executeNestedStep(step flow.Step) *core.CommandResult { duration := time.Since(start).Milliseconds() + // Enrich sub-step errors with runFlow timeout context + if !result.Success && fr.runFlowTimeout != "" && fr.ctx.Err() != nil { + result = fr.enrichTimeoutError(result) + } + // Track nested step counts (compound steps like runFlow/repeat/retry don't count themselves) if !isCompoundStep { if result.Success { @@ -873,14 +978,20 @@ func (fr *FlowRunner) executeSubFlow(subFlow flow.Flow) *core.CommandResult { defer fr.script.withEnvVars(subFlow.Config.Env)() // Execute steps + var lastStepDesc string for _, step := range subFlow.Steps { if fr.ctx.Err() != nil { + msg := "Sub-flow cancelled" + if lastStepDesc != "" { + msg = fmt.Sprintf("Sub-flow cancelled after: %s", lastStepDesc) + } return &core.CommandResult{ Success: false, Error: fr.ctx.Err(), - Message: "Sub-flow cancelled", + Message: msg, } } + lastStepDesc = step.Describe() // Inject subflow's appId/url into app lifecycle steps (same as executeStep does for main flow) switch s := step.(type) { diff --git a/pkg/executor/runner_test.go b/pkg/executor/runner_test.go index 9a59b16..bd4d0c4 100644 --- a/pkg/executor/runner_test.go +++ b/pkg/executor/runner_test.go @@ -2,6 +2,7 @@ package executor import ( "context" + "fmt" "os" "path/filepath" "sync" @@ -66,6 +67,10 @@ func (m *mockDriver) SetWaitForIdleTimeout(ms int) error { return nil } +func (m *mockDriver) SetContext(ctx context.Context) { + // no-op for mock +} + func TestRunner_Run_AllPassed(t *testing.T) { tmpDir := t.TempDir() @@ -943,6 +948,224 @@ func TestRunner_RunFlowStep_NoFileOrSteps(t *testing.T) { } } +func TestRunner_RunFlowStep_Timeout(t *testing.T) { + tmpDir := t.TempDir() + + execCount := 0 + driver := &mockDriver{ + executeFunc: func(step flow.Step) *core.CommandResult { + execCount++ + // Each step takes 100ms + time.Sleep(100 * time.Millisecond) + return &core.CommandResult{Success: true} + }, + } + + runner := New(driver, RunnerConfig{ + OutputDir: tmpDir, + Parallelism: 0, + Artifacts: ArtifactNever, + Device: report.Device{ID: "test", Platform: "android"}, + }) + + // runFlow with 250ms timeout containing 5 steps (each takes 100ms) + // Should only complete ~2 steps before timeout + flows := []flow.Flow{ + { + SourcePath: "test.yaml", + Config: flow.Config{Name: "RunFlow Timeout Test"}, + Steps: []flow.Step{ + &flow.RunFlowStep{ + BaseStep: flow.BaseStep{StepType: flow.StepRunFlow, TimeoutMs: 250}, + Steps: []flow.Step{ + &flow.TapOnStep{BaseStep: flow.BaseStep{StepType: flow.StepTapOn}}, + &flow.TapOnStep{BaseStep: flow.BaseStep{StepType: flow.StepTapOn}}, + &flow.TapOnStep{BaseStep: flow.BaseStep{StepType: flow.StepTapOn}}, + &flow.TapOnStep{BaseStep: flow.BaseStep{StepType: flow.StepTapOn}}, + &flow.TapOnStep{BaseStep: flow.BaseStep{StepType: flow.StepTapOn}}, + }, + }, + }, + }, + } + + result, err := runner.Run(context.Background(), flows) + if err != nil { + t.Fatalf("Run() error = %v", err) + } + + // Should fail due to timeout + if result.Status != report.StatusFailed { + t.Errorf("Status = %v, want %v", result.Status, report.StatusFailed) + } + + // Should NOT have executed all 5 steps + if execCount >= 5 { + t.Errorf("execCount = %d, want < 5 (timeout should stop execution)", execCount) + } +} + +func TestRunner_RunFlowStep_TimeoutNotExceeded(t *testing.T) { + tmpDir := t.TempDir() + + execCount := 0 + driver := &mockDriver{ + executeFunc: func(step flow.Step) *core.CommandResult { + execCount++ + return &core.CommandResult{Success: true} + }, + } + + runner := New(driver, RunnerConfig{ + OutputDir: tmpDir, + Parallelism: 0, + Artifacts: ArtifactNever, + Device: report.Device{ID: "test", Platform: "android"}, + }) + + // runFlow with generous timeout — all steps should complete + flows := []flow.Flow{ + { + SourcePath: "test.yaml", + Config: flow.Config{Name: "RunFlow No Timeout Test"}, + Steps: []flow.Step{ + &flow.RunFlowStep{ + BaseStep: flow.BaseStep{StepType: flow.StepRunFlow, TimeoutMs: 5000}, + Steps: []flow.Step{ + &flow.TapOnStep{BaseStep: flow.BaseStep{StepType: flow.StepTapOn}}, + &flow.TapOnStep{BaseStep: flow.BaseStep{StepType: flow.StepTapOn}}, + &flow.TapOnStep{BaseStep: flow.BaseStep{StepType: flow.StepTapOn}}, + }, + }, + }, + }, + } + + result, err := runner.Run(context.Background(), flows) + if err != nil { + t.Fatalf("Run() error = %v", err) + } + + if result.Status != report.StatusPassed { + t.Errorf("Status = %v, want %v", result.Status, report.StatusPassed) + } + + if execCount != 3 { + t.Errorf("execCount = %d, want 3", execCount) + } +} + +func TestRunner_RunFlowStep_InlineStepFailure(t *testing.T) { + tmpDir := t.TempDir() + + execCount := 0 + driver := &mockDriver{ + executeFunc: func(step flow.Step) *core.CommandResult { + execCount++ + // Second step fails + if execCount == 2 { + return &core.CommandResult{Success: false, Error: fmt.Errorf("step failed")} + } + return &core.CommandResult{Success: true} + }, + } + + runner := New(driver, RunnerConfig{ + OutputDir: tmpDir, + Parallelism: 0, + Artifacts: ArtifactNever, + Device: report.Device{ID: "test", Platform: "android"}, + }) + + flows := []flow.Flow{ + { + SourcePath: "test.yaml", + Config: flow.Config{Name: "Inline Failure Test"}, + Steps: []flow.Step{ + &flow.RunFlowStep{ + BaseStep: flow.BaseStep{StepType: flow.StepRunFlow}, + Steps: []flow.Step{ + &flow.TapOnStep{BaseStep: flow.BaseStep{StepType: flow.StepTapOn}}, + &flow.TapOnStep{BaseStep: flow.BaseStep{StepType: flow.StepTapOn}}, // fails + &flow.TapOnStep{BaseStep: flow.BaseStep{StepType: flow.StepTapOn}}, // should not run + }, + }, + }, + }, + } + + result, err := runner.Run(context.Background(), flows) + if err != nil { + t.Fatalf("Run() error = %v", err) + } + + if result.Status != report.StatusFailed { + t.Errorf("Status = %v, want %v", result.Status, report.StatusFailed) + } + + // Third step should not execute + if execCount != 2 { + t.Errorf("execCount = %d, want 2", execCount) + } +} + +func TestRunner_RunFlowStep_ExternalFileWithCallback(t *testing.T) { + tmpDir := t.TempDir() + + // Create a sub-flow file + subFlowContent := `appId: com.test +--- +- tapOn: "OK" +` + subFlowPath := filepath.Join(tmpDir, "sub.yaml") + if err := os.WriteFile(subFlowPath, []byte(subFlowContent), 0644); err != nil { + t.Fatalf("Failed to write sub-flow: %v", err) + } + + nestedFlowStarted := false + driver := &mockDriver{ + executeFunc: func(step flow.Step) *core.CommandResult { + return &core.CommandResult{Success: true} + }, + } + + runner := New(driver, RunnerConfig{ + OutputDir: tmpDir, + Parallelism: 0, + Artifacts: ArtifactNever, + Device: report.Device{ID: "test", Platform: "android"}, + OnNestedFlowStart: func(depth int, name string) { + nestedFlowStarted = true + }, + }) + + flows := []flow.Flow{ + { + SourcePath: filepath.Join(tmpDir, "main.yaml"), + Config: flow.Config{Name: "Callback Test"}, + Steps: []flow.Step{ + &flow.RunFlowStep{ + BaseStep: flow.BaseStep{StepType: flow.StepRunFlow}, + File: "sub.yaml", + }, + }, + }, + } + + result, err := runner.Run(context.Background(), flows) + if err != nil { + t.Fatalf("Run() error = %v", err) + } + + if result.Status != report.StatusPassed { + t.Errorf("Status = %v, want %v", result.Status, report.StatusPassed) + } + + if !nestedFlowStarted { + t.Error("OnNestedFlowStart callback was not called") + } +} + func TestRunner_DefineVariablesStep(t *testing.T) { tmpDir := t.TempDir() diff --git a/pkg/executor/scripting.go b/pkg/executor/scripting.go index 2dc80f2..9978b0b 100644 --- a/pkg/executor/scripting.go +++ b/pkg/executor/scripting.go @@ -476,11 +476,12 @@ func conditionTimeout(cond flow.Condition, sel *flow.Selector) int { } // withEnvVars applies environment variables and returns a restore function. +// Values are expanded through ExpandVariables to support ${VAR || "default"} syntax. func (se *ScriptEngine) withEnvVars(env map[string]string) func() { oldVars := make(map[string]string) for k, v := range env { oldVars[k] = se.GetVariable(k) - se.SetVariable(k, v) + se.SetVariable(k, se.ExpandVariables(v)) } return func() { for k, v := range oldVars { diff --git a/pkg/executor/scripting_test.go b/pkg/executor/scripting_test.go index 1b5d0a3..9f73762 100644 --- a/pkg/executor/scripting_test.go +++ b/pkg/executor/scripting_test.go @@ -322,6 +322,30 @@ func TestScriptEngine_withEnvVars(t *testing.T) { } } +func TestScriptEngine_withEnvVars_DefaultValues(t *testing.T) { + se := NewScriptEngine() + defer se.Close() + + se.SetVariable("CLI_VAR", "from_cli") + + // withEnvVars should expand ${VAR || "default"} syntax + restore := se.withEnvVars(map[string]string{ + "APP_ID": `${APP_ID || "com.example.default"}`, + "USER_ID": `${CLI_VAR || "fallback"}`, + }) + + // APP_ID was undefined, should get default + if got := se.GetVariable("APP_ID"); got != "com.example.default" { + t.Errorf("APP_ID = %q, want %q", got, "com.example.default") + } + // CLI_VAR was defined, should get its value + if got := se.GetVariable("USER_ID"); got != "from_cli" { + t.Errorf("USER_ID = %q, want %q", got, "from_cli") + } + + restore() +} + func TestScriptEngine_GetOutput(t *testing.T) { se := NewScriptEngine() defer se.Close() diff --git a/pkg/executor/tap_options.go b/pkg/executor/tap_options.go index ff006fb..34e6f99 100644 --- a/pkg/executor/tap_options.go +++ b/pkg/executor/tap_options.go @@ -54,6 +54,49 @@ const ( settleInterval = 200 * time.Millisecond ) +// settleAfterAction waits for the UI to settle after a UI-mutating action. +// Matches Maestro's behavior of calling waitForAppToSettle() after every action. +// Uses DeviceLab's native event-based settle when available, falls back to hierarchy comparison. +func (fr *FlowRunner) settleAfterAction() { + // Check if driver supports native settle (DeviceLab) + if settler, ok := core.Unwrap(fr.driver).(interface { + WaitForSettle(timeoutMs, quietMs int) (bool, error) + }); ok { + settled, err := settler.WaitForSettle(2000, 150) + if err != nil { + logger.Warn("settleAfterAction: native settle error: %v", err) + } else if !settled { + logger.Debug("settleAfterAction: native settle timed out") + } + return + } + + // Fallback: hierarchy comparison (10 polls × 200ms, same as Maestro default) + fr.waitForSettle(2000) +} + +// isTapAction returns true if the step is a tap that may trigger a screen transition. +func isTapAction(step flow.Step) bool { + switch step.(type) { + case *flow.TapOnStep, *flow.DoubleTapOnStep, *flow.LongPressOnStep, *flow.TapOnPointStep: + return true + default: + return false + } +} + +// needsPreSettle returns true if the step needs the UI to be settled before executing. +// These steps don't call findElement (which has implicit idle wait), so they need +// explicit settle to avoid timing issues after screen transitions. +func needsPreSettle(step flow.Step) bool { + switch step.(type) { + case *flow.InputTextStep, *flow.InputRandomStep, *flow.EraseTextStep: + return true + default: + return false + } +} + // waitForSettle polls Hierarchy() until two consecutive snapshots match, // or the timeout is reached. Returns the final hierarchy snapshot. // If timeoutMs <= 0, returns the current hierarchy without polling. diff --git a/pkg/flow/parser.go b/pkg/flow/parser.go index 7553342..cefdbd2 100644 --- a/pkg/flow/parser.go +++ b/pkg/flow/parser.go @@ -985,6 +985,7 @@ func parseRunFlowStep(valueNode *yaml.Node, sourcePath string) (Step, error) { Env map[string]string `yaml:"env"` Optional bool `yaml:"optional"` Label string `yaml:"label"` + Timeout int `yaml:"timeout"` } if err := valueNode.Decode(&raw); err != nil { @@ -996,6 +997,7 @@ func parseRunFlowStep(valueNode *yaml.Node, sourcePath string) (Step, error) { s.Env = raw.Env s.Optional = raw.Optional s.StepLabel = raw.Label + s.TimeoutMs = raw.Timeout for _, cmdNode := range raw.Commands { step, err := parseStep(&cmdNode, sourcePath) diff --git a/pkg/flow/parser_test.go b/pkg/flow/parser_test.go index cdac14d..ac03eab 100644 --- a/pkg/flow/parser_test.go +++ b/pkg/flow/parser_test.go @@ -396,6 +396,55 @@ func TestParse_RunFlowWithInlineSteps(t *testing.T) { } } +func TestParse_RunFlowWithTimeout(t *testing.T) { + yaml := ` +- runFlow: + file: login.yaml + timeout: 5000 + env: + user: test +` + flow, err := Parse([]byte(yaml), "test.yaml") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + runFlow, ok := flow.Steps[0].(*RunFlowStep) + if !ok { + t.Fatalf("expected RunFlowStep, got %T", flow.Steps[0]) + } + if runFlow.TimeoutMs != 5000 { + t.Errorf("expected TimeoutMs=5000, got %d", runFlow.TimeoutMs) + } + if runFlow.File != "login.yaml" { + t.Errorf("expected file=login.yaml, got %q", runFlow.File) + } +} + +func TestParse_RunFlowInlineWithTimeout(t *testing.T) { + yaml := ` +- runFlow: + timeout: 3000 + commands: + - assertVisible: "Hello" +` + flow, err := Parse([]byte(yaml), "test.yaml") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + runFlow, ok := flow.Steps[0].(*RunFlowStep) + if !ok { + t.Fatalf("expected RunFlowStep, got %T", flow.Steps[0]) + } + if runFlow.TimeoutMs != 3000 { + t.Errorf("expected TimeoutMs=3000, got %d", runFlow.TimeoutMs) + } + if len(runFlow.Steps) != 1 { + t.Errorf("expected 1 inline step, got %d", len(runFlow.Steps)) + } +} + func TestParse_NestedRepeat(t *testing.T) { yaml := ` - repeat: @@ -889,7 +938,7 @@ func TestParse_AssertConditionStep(t *testing.T) { text: "Success" notVisible: text: "Error" - scriptCondition: "result === true" + true: "result === true" platform: Android ` flow, err := Parse([]byte(yaml), "test.yaml") diff --git a/pkg/flow/step.go b/pkg/flow/step.go index a52652a..42e8b70 100644 --- a/pkg/flow/step.go +++ b/pkg/flow/step.go @@ -307,7 +307,7 @@ type AssertTrueStep struct { type Condition struct { Visible *Selector `yaml:"visible"` NotVisible *Selector `yaml:"notVisible"` - Script string `yaml:"scriptCondition"` + Script string `yaml:"true"` Platform string `yaml:"platform"` Timeout int `yaml:"timeout"` // Timeout in ms for visible/notVisible checks } diff --git a/pkg/flutter/wrapper.go b/pkg/flutter/wrapper.go index 1fe1ca4..e603934 100644 --- a/pkg/flutter/wrapper.go +++ b/pkg/flutter/wrapper.go @@ -36,11 +36,12 @@ type FlutterDriver struct { client *VMServiceClient dev DeviceExecutor appID string - socketPath string // Unix socket path for VM Service forwarding (Android) - udid string // iOS simulator UDID (empty for Android) - isIOS bool // true = iOS reconnection path - attempted bool // true after first discovery attempt (avoids retrying every step) - findTimeoutMs int // current find timeout set by executor (0 = driver default) + socketPath string // Unix socket path for VM Service forwarding (Android) + udid string // iOS simulator UDID (empty for Android) + isIOS bool // true = iOS reconnection path + attempted bool // true after first discovery attempt (avoids retrying every step) + findTimeoutMs int // current find timeout set by executor (0 = driver default) + ctx context.Context // Parent context for element-finding operations (runFlow timeout) } // Wrap creates a FlutterDriver that wraps the given inner driver. @@ -115,7 +116,7 @@ func (d *FlutterDriver) Execute(step flow.Step) *core.CommandResult { // --- Parallel search: inner driver (1s polls) + Flutter (continuous goroutine) --- // Start Flutter polling in background goroutine - searchCtx, searchCancel := context.WithTimeout(context.Background(), time.Duration(effectiveTimeout)*time.Millisecond) + searchCtx, searchCancel := context.WithTimeout(d.parentContext(), time.Duration(effectiveTimeout)*time.Millisecond) flutterCh := make(chan *flutterSearchResult, 1) go d.flutterPollLoop(searchCtx, sel, flutterCh) @@ -125,6 +126,14 @@ func (d *FlutterDriver) Execute(step flow.Step) *core.CommandResult { var result *core.CommandResult for time.Now().Before(deadline) { + // Check if parent context (e.g. runFlow timeout) was cancelled + if err := d.parentContext().Err(); err != nil { + searchCancel() + <-flutterCh + d.inner.SetFindTimeout(d.findTimeoutMs) + return core.ErrorResult(fmt.Errorf("element %q not found: %w", sel.Describe(), err), "") + } + d.inner.SetFindTimeout(innerPollTimeout) result = d.inner.Execute(step) @@ -546,6 +555,17 @@ func (d *FlutterDriver) SetFindTimeout(ms int) { func (d *FlutterDriver) SetWaitForIdleTimeout(ms int) error { return d.inner.SetWaitForIdleTimeout(ms) } +func (d *FlutterDriver) SetContext(ctx context.Context) { + d.ctx = ctx + d.inner.SetContext(ctx) +} + +func (d *FlutterDriver) parentContext() context.Context { + if d.ctx != nil { + return d.ctx + } + return context.Background() +} // --- Optional interface forwarding --- diff --git a/pkg/flutter/wrapper_test.go b/pkg/flutter/wrapper_test.go index 45f8ab4..85e40f7 100644 --- a/pkg/flutter/wrapper_test.go +++ b/pkg/flutter/wrapper_test.go @@ -1,6 +1,7 @@ package flutter import ( + "context" "fmt" "testing" @@ -28,6 +29,7 @@ func (m *mockDriver) GetState() *core.StateSnapshot { return &core.Stat func (m *mockDriver) GetPlatformInfo() *core.PlatformInfo { return &core.PlatformInfo{} } func (m *mockDriver) SetFindTimeout(ms int) {} func (m *mockDriver) SetWaitForIdleTimeout(ms int) error { return nil } +func (m *mockDriver) SetContext(context.Context) {} func TestFlutterDriver_PassThrough_Success(t *testing.T) { inner := &mockDriver{ diff --git a/pkg/jsengine/engine.go b/pkg/jsengine/engine.go index 2df7664..8cbe41b 100644 --- a/pkg/jsengine/engine.go +++ b/pkg/jsengine/engine.go @@ -442,8 +442,8 @@ func (e *Engine) ExpandVariables(text string) (string, error) { // Extract expression expr := result[idx+2 : end-1] - // Evaluate expression - value, err := e.EvalString(expr) + // Evaluate expression, auto-defining undefined variables on ReferenceError + value, err := e.evalWithUndefinedFallback(expr) if err != nil { // If evaluation fails, leave as-is or return error start = end @@ -458,6 +458,45 @@ func (e *Engine) ExpandVariables(text string) (string, error) { return result, nil } +// evalWithUndefinedFallback evaluates a JS expression, automatically defining +// undefined variables to prevent ReferenceError. This matches Maestro's behavior +// where undeclared variables are treated as undefined (falsy) rather than errors. +// Supports patterns like: ${VAR || "default"}, ${VAR ?? "fallback"} +func (e *Engine) evalWithUndefinedFallback(expr string) (string, error) { + const maxRetries = 10 + for i := 0; i < maxRetries; i++ { + value, err := e.EvalString(expr) + if err == nil { + return value, nil + } + // Check if it's a ReferenceError for an undefined variable + varName := extractUndefinedVarName(err.Error()) + if varName == "" { + return "", err // Not a ReferenceError, return original error + } + e.DefineUndefinedIfMissing(varName) + } + return "", fmt.Errorf("too many undefined variables in expression: %s", expr) +} + +// extractUndefinedVarName extracts the variable name from a goja ReferenceError. +// Example: "JS eval error: ReferenceError: APP_ID is not defined at :1:1(0)" +// Returns "APP_ID", or empty string if not a ReferenceError. +func extractUndefinedVarName(errMsg string) string { + const prefix = "ReferenceError: " + const suffix = " is not defined" + idx := strings.Index(errMsg, prefix) + if idx == -1 { + return "" + } + after := errMsg[idx+len(prefix):] + endIdx := strings.Index(after, suffix) + if endIdx == -1 { + return "" + } + return after[:endIdx] +} + // Close cleans up the engine (stops timers, etc.) // Safe to call multiple times. func (e *Engine) Close() { diff --git a/pkg/jsengine/engine_test.go b/pkg/jsengine/engine_test.go index 1c81833..f480a45 100644 --- a/pkg/jsengine/engine_test.go +++ b/pkg/jsengine/engine_test.go @@ -107,6 +107,62 @@ func TestExpandVariables(t *testing.T) { } } +func TestExpandVariables_DefaultValues(t *testing.T) { + engine := New() + defer engine.Close() + + // Set one variable, leave others undefined + engine.SetVariable("DEFINED_VAR", "existing_value") + + tests := []struct { + name string + input string + expected string + }{ + {"undefined var with || fallback", `${APP_ID || "com.example.app"}`, "com.example.app"}, + {"undefined var with || single quotes", `${APP_ID || 'com.example.app'}`, "com.example.app"}, + {"defined var with || fallback", `${DEFINED_VAR || "fallback"}`, "existing_value"}, + {"undefined var with ?? fallback", `${UNDEF_VAR ?? "nullish_default"}`, "nullish_default"}, + {"multiple undefined in chain", `${UNDEF_A || UNDEF_B || "last"}`, "last"}, + {"default value in text", `App: ${APP_ID || "com.example.app"}`, "App: com.example.app"}, + {"ternary with undefined", `${UNDEF_X ? "yes" : "no"}`, "no"}, + {"mixed defined and default", `${DEFINED_VAR}-${UNDEF_VAR || "default"}`, "existing_value-default"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := engine.ExpandVariables(tt.input) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if result != tt.expected { + t.Errorf("expected %q, got %q", tt.expected, result) + } + }) + } +} + +func TestExtractUndefinedVarName(t *testing.T) { + tests := []struct { + errMsg string + expected string + }{ + {"JS eval error: ReferenceError: APP_ID is not defined at :1:1(0)", "APP_ID"}, + {"JS eval error: ReferenceError: myVar is not defined at :1:1(0)", "myVar"}, + {"JS eval error: TypeError: something went wrong", ""}, + {"random error", ""}, + } + + for _, tt := range tests { + t.Run(tt.errMsg, func(t *testing.T) { + result := extractUndefinedVarName(tt.errMsg) + if result != tt.expected { + t.Errorf("expected %q, got %q", tt.expected, result) + } + }) + } +} + func TestConsoleLog(t *testing.T) { engine := New() defer engine.Close() diff --git a/pkg/maestro/adapter.go b/pkg/maestro/adapter.go index a01b281..94240b3 100644 --- a/pkg/maestro/adapter.go +++ b/pkg/maestro/adapter.go @@ -405,6 +405,26 @@ func (a *Adapter) SetAppiumSettings(settings map[string]interface{}) error { return err } +// WaitForSettle waits for the UI to settle using accessibility event tracking. +// Returns true if settled within timeout, false if timed out. +func (a *Adapter) WaitForSettle(timeoutMs, quietMs int) (bool, error) { + resp, err := a.client.Call("UI.waitForSettle", map[string]interface{}{ + "timeout": timeoutMs, + "quiet": quietMs, + }) + if err != nil { + return false, err + } + + var result struct { + Settled bool `json:"settled"` + } + if err := json.Unmarshal(resp.Result, &result); err != nil { + return false, err + } + return result.Settled, nil +} + // --- Session management --- // CreateSession creates a session on the driver.