Repository navigation
feat: add --install-only and --pick-device to Appium runners - #65
Conversation
Builds, launches the device, resets, and installs the app, then exits without starting Appium or running tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Prompts for an iOS simulator and/or Android AVD in run-local.sh and, up front, in run-all.sh. Also lets --device override .env device names and syncs OS_VERSION with the running device. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
abdulraqeeb33
left a comment
There was a problem hiding this comment.
Nice addition, both flags are genuinely useful and the implementation follows the existing runner conventions closely. Two things worth a look before merge, neither blocking.
Also three minor notes:
run-local/runtime.sh:409— a failed version lookup and "version already matches" are both a silentreturn 0. Whenadb getproptimes out or the simctl/python read fails,versionis empty, the regex fails, andOS_VERSIONquietly stays at the default. That is the exact condition the function exists to prevent, and it surfaces later as a confusing AppiumplatformVersionmismatch rather than "couldn't read the device OS version". Worth splitting the empty-lookup case out with awarn.README.md:3— the intro still reads "builds the app, boots a simulator/emulator, starts Appium, resets app state, and runs the E2E test suite." With--install-onlythat is no longer the only path. The options table covers the flag, just the lead sentence that is stale.README.md:138-148— therun-all.shsection does not mention--pick-device, thoughrun-all.sh --helpdocuments it.
- Fail the device picker on EOF instead of looping - Make --device fully override .env on iOS (UDID, runtime) - Forward --install-only from run-all, one SDK per platform - Resolve the booted simulator instead of simctl's `booted` target - Warn when the device OS version can't be read - Extract ios_sim_version and combo_runs helpers Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks @abdulraqeeb33. Addressed the three notes in d99ae07: empty version lookup now warns, README intro mentions --install-only, and the run-all section shows --pick-device and --install-only. |
sherwinski
left a comment
There was a problem hiding this comment.
I noticed that the second fallback block in run-local/runtime.sh:82 still splices $IOS_SIMULATOR into Python source inside single quotes. The same quote-in-value failure applies as in #65 (comment). That code is outside the scope of this PR so probably worth addressing in a separate PR, or just make the update here.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks @sherwinski, fixed the fallback lookup here in 3897959, simulator name is passed as argv now. |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
abdulraqeeb33
left a comment
There was a problem hiding this comment.
Re-reviewed at a8547f8. My earlier pass was against 328f6c3, so this supersedes it.
Approving. The three commits since then fix the serious half of what I had, including the one finding that would have bitten in CI.
Fixed since 328f6c3
- The EOF busy-loop is gone.
readfailure now returns 1 instead of spinning. This was the one I'd have blocked on: both call sites run the function in a||/if !context, which suppresses errexit for the whole body, sorun-local.sh'sset -euo pipefailwasn't saving it either. --device=now clearsIOS_RUNTIMEandstart_ios_simulatorsorts runtimes newest-first, so an explicitly named device is no longer silently replaced by whichever simulator happens to be booted. Nice that the empty-filter-matches-any approach avoids the trap where clearing the runtime to''would have matched everything unsorted.${UDID:-booted}is gone from the install path.resolve_booted_simulatorpins the target on--skip-device, andinstall_appnow hard-errors rather than inferring. The 0-booted and multiple-booted cases both get a real message.$UDIDand$IOS_SIMULATORare passed via argv instead of interpolated into Python string literals.- The OS-version sync warns instead of silently keeping a stale value, which was the whole point of the function.
run-all.shparses--install-onlyand guards against installing several SDKs over one bundle id, instead of warning "Unknown option" and then running the full matrix.
I checked the new code rather than taking the commit messages for it. The [[ "$PLATFORM" == "ios" ]] && resolve_booted_simulator line in start_device is errexit-safe under set -euo pipefail (tested, a non-final && component failing doesn't trip it), and run-all.sh's error() only echoes, so the exit 2 after it is load-bearing rather than dead. combo_runs is a good extraction; my truth table on the old inline form said it was correct, and the named version is much easier to confirm.
Still open, none blocking
The vpx one is inline and is the only one I'd prioritize. Three things that don't anchor to a diff line:
No test harness, and that's load-bearing here. No bats, no shellcheck, no bash -n anywhere in .github/workflows, across roughly 2,500 lines of shell that CI never invokes. Worth noting bash -n wouldn't have caught the EOF loop, the sort, or the UDID quoting, so a lint step alone isn't the fix. The seam: list_ios_simulators / list_android_avds that only print udid|name|label to stdout, and a choose_from_list taking those lines plus a prompt, with the simctl transforms as programs reading JSON on stdin and taking arguments via argv. Now that ios_sim_version already takes its UDID that way, it's close to directly testable with fixture JSON. combo_runs is a free one.
The README environment table doesn't reflect the new precedence. It still presents DEVICE, IOS_SIMULATOR, IOS_RUNTIME, AVD_NAME, and OS_VERSION as the source of truth, with no note that --device= now clears the middle three or that the OS version is re-read from the running device. That table is outside the diff, so I couldn't anchor it.
config.sh is at 466 lines and runtime.sh at 492. Moving preflight / preflight_test_tooling into a run-local/preflight.sh, sibling to device-picker.sh, and the USAGE heredoc into its own file, would take roughly 170 lines out of config.sh and leave it as parse plus resolve.
Residual risk
Still nothing run end-to-end against a device, so the two paths you flagged stay unverified, and resolve_booted_simulator is new since the last pass. Specifically open: whether xcrun simctl install against a shutdown simulator succeeds, which decides what --pick-device --skip-device actually does on iOS now that it pins a UDID and then never boots it.
| # calls `vpx wdio run` directly on BrowserStack — so these checks are | ||
| # local-only by construction. | ||
| preflight() { | ||
| if [[ "$INSTALL_ONLY" != true ]]; then |
There was a problem hiding this comment.
This still skips all of preflight_test_tooling for --install-only, and that includes the vpx check, which install-only does need.
build.sh calls vp run build and vpx cap sync for Cordova, Capacitor, and Expo. So ./run-local.sh --install-only --sdk=capacitor without the vpx symlink spends the entire web build and then dies on a bare vpx: command not found, which is the failure the comment just above says this block exists to prevent. vp can exist while the vpx symlink doesn't, and that's exactly the case the error message in preflight_test_tooling is written for.
Appium, the driver check, the Appium node_modules bootstrap, and the Node-version WDIO_USE_NATIVE_FETCH export are all genuinely test-only and right to skip. vpx is a build dependency, so the split is one line off.
Suggestion: keep the vpx block unconditional and gate the rest behind [[ "$INSTALL_ONLY" != true ]].
| error "--release and --main are mutually exclusive" | ||
| fi | ||
|
|
||
| if [[ "$PICK_DEVICE" == true ]]; then |
There was a problem hiding this comment.
Small gap left over: this block unconditionally overwrites DEVICE, AVD_NAME, IOS_SIMULATOR, and UDID, so --pick-device --device=X and --device=X --pick-device both end on the picked device and the explicit --device= is dropped with no warning. Erroring on the combination would match the --device-real rejection on the line below.
Minor follow-on from the new DEVICE_FLAG handling: when both are passed, IOS_RUNTIME_ANY stays true from the --device= branch even though the picker supplies a UDID. Harmless today, since a non-empty UDID takes the early branch in start_ios_simulator and never consults the runtime, but it's the kind of leftover that stops being harmless if that early branch ever changes.
| if d["state"] == "Booted": | ||
| label += " [booted]" | ||
| rows.append((ver, d["name"], d["udid"], label)) | ||
| for ver, name, udid, label in sorted(rows, key=lambda r: (tuple(-v for v in r[0]), not r[1].startswith("iPhone"), r[1])): |
There was a problem hiding this comment.
Newest-first breaks when runtime ids differ in length, because (-18, 0) sorts before (-18, 0, -1). I confirmed 18.0 lists above 18.0.1. Equal-length ids sort correctly, so this needs a three-component runtime installed to show up, and live ids are usually two components.
Padding before negating fixes it, e.g. ver + (0,) * (4 - len(ver)).
Worth noting start_ios_simulator's new version() helper has the same shape, though it's only used to order candidates for an exact name match, so the consequence there is smaller.
| label += " [booted]" | ||
| rows.append((ver, d["name"], d["udid"], label)) | ||
| for ver, name, udid, label in sorted(rows, key=lambda r: (tuple(-v for v in r[0]), not r[1].startswith("iPhone"), r[1])): | ||
| print(udid + "|" + name + "|" + label) |
There was a problem hiding this comment.
Nit: | as the field separator breaks on a simulator renamed to contain one. iPhone | QA reads back as name iPhone , and boot plus the Appium capability then use the truncated name. UDIDs from simctl never contain |, so only the name is at risk.
A tab or \x1f separator, or printing just udid and name and rebuilding the label in bash, would close it.
| info "Installed $BUNDLE_ID" | ||
| } | ||
|
|
||
| prepare_install_only() { |
There was a problem hiding this comment.
These two sequences have now actually diverged, which is the thing I was worried about a pass ago: sync_device_os_version was dropped from here but kept in prepare_runtime. That's the right call on the merits, since nothing downstream of install-only reads OS_VERSION, but nothing in the code records that it was a decision rather than an omission, and the next step added to prepare_runtime has the same coin-flip.
Suggestion: extract the shared head (validate_existing_app, start_device, check_android_storage, reset_app) into one prepare_device, and keep the version sync, test tooling, and explicit install as mode-specific tails. Then the difference between the two paths is visible in one place instead of inferred from two lists.
Description
Adds
--install-onlytorun-local.shand--pick-devicetorun-local.shandrun-all.sh, so you can install a demo app without running tests and choose which simulator or emulator to use.Details
--install-only: builds, launches the device, resets, and installs the app (adb/simctl/devicectl), then exits without starting Appium or running tests. It skips the Appium/vpx checks. With--skip-buildit reinstalls the existing artifact. It's not inrun-all.shbecause every demo has the bundle IDcom.onesignal.example, so each install would overwrite the last.--pick-device: shows a numbered list of iOS simulators (newest iOS first, iPhones first) or Android AVDs, with booted/running ones marked.run-local.shasks after the platform is chosen.run-all.shasks once up front, only for platforms that have SDKs to run (e.g.--sdks=iosonly asks for a simulator), and forwards--device(plus--udidon iOS) to each run.--device=NAMEnow overridesAVD_NAME/IOS_SIMULATORfrom.env.OS_VERSIONis reset to the running device's OS version, so Appium'splatformVersioncheck passes on a non-default device.Testing:
bash -nandvp checkpass. I tested the picker against local simulators and AVDs, tested the iOS OS version lookup, and testedrun-all.shasking only for the relevant platforms with--sdks=ios,android, andios,flutter. Not yet tested: a full test run on a picked device, and--install-onlyon a device.🤖 Generated with Claude Code