diff --git a/CHANGELOG.md b/CHANGELOG.md index 0cbf4093..512a8a9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added +- **`--new-command-timeout `** (env `MAESTRO_NEW_COMMAND_TIMEOUT`) — sets `appium:newCommandTimeout` for the Appium session when it isn't already specified in `--caps`. An explicit `appium:newCommandTimeout` in the caps file stays authoritative and is never overridden. This lets you raise the session command timeout without editing the caps file — e.g. cloud `--parallel` runs where the earliest-created sessions sit idle during the serial pre-creation phase and would otherwise be reaped at the server default, failing the first flow with `invalid session id`. Off by default (`0` = leave unset). Requested by [@devrchoi](https://github.com/devrchoi) ([#124](https://github.com/devicelab-dev/maestro-runner/issues/124)). + ```bash + maestro-runner --driver appium --caps caps.json --new-command-timeout 300 --parallel 4 test flows/ + ``` + ## [1.1.19] - 2026-07-01 A reporter-driven follow-up focused on executor/`runScript` parity with Maestro and iOS simulator ergonomics. Headlines: `runScript`'s JavaScript environment now matches Maestro (env vars are scoped per script and undeclared variables read as `undefined` instead of throwing), `when:`/`while:` condition checks resolve **fast by default** instead of blocking on the 7s optional-find timeout, and `--auto-start-emulator` finally works for iOS simulators. Plus an iOS runner build-cache correctness fix. diff --git a/pkg/cli/appium_new_command_timeout_test.go b/pkg/cli/appium_new_command_timeout_test.go new file mode 100644 index 00000000..e24d06d3 --- /dev/null +++ b/pkg/cli/appium_new_command_timeout_test.go @@ -0,0 +1,155 @@ +package cli + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "sync" + "testing" + + "github.com/urfave/cli/v2" +) + +// captureAppiumSessionCaps stands up a mock Appium server, runs +// createAppiumDriver with the given config pointed at it, and returns the +// alwaysMatch capabilities block from the outgoing POST /session request. +func captureAppiumSessionCaps(t *testing.T, cfg *RunConfig) map[string]interface{} { + t.Helper() + + var mu sync.Mutex + var alwaysMatch map[string]interface{} + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch { + case r.URL.Path == "/session" && r.Method == "POST": + var body map[string]interface{} + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + t.Errorf("decode /session body: %v", err) + } + mu.Lock() + if capsWrap, ok := body["capabilities"].(map[string]interface{}); ok { + alwaysMatch, _ = capsWrap["alwaysMatch"].(map[string]interface{}) + } + mu.Unlock() + _ = json.NewEncoder(w).Encode(map[string]interface{}{ + "value": map[string]interface{}{ + "sessionId": "test-session", + "capabilities": map[string]interface{}{"platformName": "Android"}, + }, + }) + case r.URL.Path == "/session/test-session/window/rect": + _ = json.NewEncoder(w).Encode(map[string]interface{}{ + "value": map[string]interface{}{"width": 1080.0, "height": 1920.0}, + }) + default: + _ = json.NewEncoder(w).Encode(map[string]interface{}{"value": nil}) + } + })) + t.Cleanup(server.Close) + + cfg.AppiumURL = server.URL + _, cleanup, err := createAppiumDriver(cfg) + if err != nil { + t.Fatalf("createAppiumDriver: %v", err) + } + if cleanup != nil { + cleanup() + } + + mu.Lock() + defer mu.Unlock() + if alwaysMatch == nil { + t.Fatal("no alwaysMatch capabilities captured from /session request") + } + return alwaysMatch +} + +// TestGlobalFlags_NewCommandTimeoutFlag verifies the --new-command-timeout flag +// (with MAESTRO_NEW_COMMAND_TIMEOUT env var) is registered. Issue #124. +func TestGlobalFlags_NewCommandTimeoutFlag(t *testing.T) { + var found *cli.IntFlag + for _, f := range GlobalFlags { + if intFlag, ok := f.(*cli.IntFlag); ok && intFlag.Name == "new-command-timeout" { + found = intFlag + break + } + } + if found == nil { + t.Fatal("expected --new-command-timeout flag to be defined in GlobalFlags") + } + hasEnv := false + for _, e := range found.EnvVars { + if e == "MAESTRO_NEW_COMMAND_TIMEOUT" { + hasEnv = true + } + } + if !hasEnv { + t.Errorf("expected MAESTRO_NEW_COMMAND_TIMEOUT env var on --new-command-timeout, got %v", found.EnvVars) + } +} + +// TestCreateAppiumDriver_NewCommandTimeoutInjectedWhenAbsent verifies that when +// the caps file omits appium:newCommandTimeout, the --new-command-timeout value +// is injected into the outgoing session request. Issue #124 fix (b). +func TestCreateAppiumDriver_NewCommandTimeoutInjectedWhenAbsent(t *testing.T) { + caps := captureAppiumSessionCaps(t, &RunConfig{ + Platform: "Android", + NewCommandTimeout: 300, + Capabilities: map[string]interface{}{ + "platformName": "Android", + "appium:automationName": "UiAutomator2", + }, + }) + + got, ok := caps["appium:newCommandTimeout"] + if !ok { + t.Fatalf("expected appium:newCommandTimeout to be injected, got caps: %v", caps) + } + if n, _ := got.(float64); n != 300 { + // json round-trips numbers as float64 + if iv, isInt := got.(int); !isInt || iv != 300 { + t.Errorf("expected appium:newCommandTimeout=300, got %v (%T)", got, got) + } + } +} + +// TestCreateAppiumDriver_CapsNewCommandTimeoutPreserved verifies that an +// explicit appium:newCommandTimeout in the caps file is authoritative: it is +// preserved and NOT overridden by the --new-command-timeout flag. This is the +// core scenario from issue #124 — a user who sets 300 gets 300, never a smaller +// forced value. +func TestCreateAppiumDriver_CapsNewCommandTimeoutPreserved(t *testing.T) { + caps := captureAppiumSessionCaps(t, &RunConfig{ + Platform: "Android", + NewCommandTimeout: 90, // flag set to a smaller value must NOT win + Capabilities: map[string]interface{}{ + "platformName": "Android", + "appium:automationName": "UiAutomator2", + "appium:newCommandTimeout": float64(300), + }, + }) + + got := caps["appium:newCommandTimeout"] + n, _ := got.(float64) + if n != 300 { + t.Errorf("expected caps-file appium:newCommandTimeout=300 to be preserved, got %v (%T)", got, got) + } +} + +// TestCreateAppiumDriver_NewCommandTimeoutUnsetByDefault verifies the default +// behavior is unchanged: with no flag and no caps value, no newCommandTimeout is +// added to the session request (the Appium server default applies). +func TestCreateAppiumDriver_NewCommandTimeoutUnsetByDefault(t *testing.T) { + caps := captureAppiumSessionCaps(t, &RunConfig{ + Platform: "Android", + Capabilities: map[string]interface{}{ + "platformName": "Android", + "appium:automationName": "UiAutomator2", + }, + }) + + if v, ok := caps["appium:newCommandTimeout"]; ok { + t.Errorf("expected no appium:newCommandTimeout by default, got %v", v) + } +} diff --git a/pkg/cli/cli.go b/pkg/cli/cli.go index 113e94b6..15226e0a 100644 --- a/pkg/cli/cli.go +++ b/pkg/cli/cli.go @@ -119,6 +119,12 @@ var GlobalFlags = []cli.Flag{ Usage: "Override driver start timeout in seconds (0 = use driver default — 30s for UIAutomator2 / DeviceLab Android, 90s for WDA iOS). Bump to 120+ for slow cloud farms (AWS Device Farm, low-end shared devices) where APK install + dex2oat takes >30s.", EnvVars: []string{"MAESTRO_DRIVER_START_TIMEOUT"}, }, + &cli.IntFlag{ + Name: "new-command-timeout", + Value: 0, + Usage: "Set appium:newCommandTimeout (seconds) for the Appium session when it is not already set in --caps. 0 = leave unset (honor the caps-file value, else the Appium server default). Raise this for cloud farms where --parallel pre-creates sessions that sit idle during setup and would otherwise be reaped.", + EnvVars: []string{"MAESTRO_NEW_COMMAND_TIMEOUT"}, + }, } // Execute runs the CLI. diff --git a/pkg/cli/test.go b/pkg/cli/test.go index 4363b3ed..708b495c 100644 --- a/pkg/cli/test.go +++ b/pkg/cli/test.go @@ -496,6 +496,11 @@ type RunConfig struct { AppiumSessionFile string CapsFile string // Appium capabilities JSON file path Capabilities map[string]interface{} // Parsed Appium capabilities + // NewCommandTimeout, when > 0, sets appium:newCommandTimeout (seconds) for the + // Appium session unless the --caps file already specifies it (an explicit caps + // value is authoritative). Sourced from --new-command-timeout / + // MAESTRO_NEW_COMMAND_TIMEOUT. See issue #124. + NewCommandTimeout int // Driver settings WaitForIdleTimeout int // Wait for device idle in ms (0 = disabled, default 200) @@ -699,6 +704,7 @@ func runTest(c *cli.Context) error { AppiumSessionFile: getString("appium-session-file"), CapsFile: capsFile, Capabilities: caps, + NewCommandTimeout: getInt("new-command-timeout"), WaitForIdleTimeout: getInt("wait-for-idle-timeout"), ConditionTimeout: getInt("condition-timeout"), TypingFrequency: getInt("typing-frequency"), @@ -1902,6 +1908,19 @@ func createAppiumDriver(cfg *RunConfig) (core.Driver, func(), error) { if caps["appium:autoGrantPermissions"] == nil { caps["appium:autoGrantPermissions"] = true } + // Honor a user-supplied appium:newCommandTimeout. An explicit value in the + // --caps file is authoritative and is never overridden. When it is absent and + // --new-command-timeout / MAESTRO_NEW_COMMAND_TIMEOUT is set, inject it so the + // session command timeout can be raised without editing the caps file — e.g. + // cloud --parallel runs where the earliest-created sessions idle during the + // serial pre-creation phase and would otherwise be reaped. See issue #124. + if cfg.NewCommandTimeout > 0 { + _, hasPrefixed := caps["appium:newCommandTimeout"] + _, hasBare := caps["newCommandTimeout"] + if !hasPrefixed && !hasBare { + caps["appium:newCommandTimeout"] = cfg.NewCommandTimeout + } + } // Add waitForIdleTimeout to capabilities for session creation // Priority: Flow config > CLI flag > Workspace config > Cap file > Default (5000ms)