Add support for real device testing#83
Open
longtimeno-c wants to merge 3 commits into
Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds real-device mode: WDA lifecycle (build, launch, port forwarding, session, input), an HTTP relay server for MJPEG stream and input mapping with an embedded viewer, CLI command wiring ( ChangesPhysical Device Support via WebDriverAgent
Sequence DiagramsequenceDiagram
participant User as CLI User
participant WdaSession
participant Xcode as xcodebuild
participant usbmux as pymobiledevice3
participant WDA as WebDriverAgent
participant DeviceServer as Device Server
participant Browser as Browser/Viewer
User->>WdaSession: create & start()
WdaSession->>Xcode: build-for-testing (if needed)
WdaSession->>Xcode: test-without-building (launch runner)
WdaSession->>usbmux: usbmux forward control/mjpeg ports
WdaSession->>WDA: GET /status (poll until ready)
WdaSession->>WDA: POST /session
WdaSession->>WDA: GET /session/{id}/window/size
User->>DeviceServer: startDeviceServer(session)
Browser->>DeviceServer: GET /stream.mjpeg
DeviceServer->>WDA: GET /mjpeg (proxied)
Browser->>DeviceServer: POST /input (tap/drag/button)
DeviceServer->>WdaSession: tap/drag/pressButton
WdaSession->>WDA: POST session command
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/serve-sim/src/wda.ts (1)
334-345: 💤 Low valueConsider adding error handling for port forwarding failures.
The forwarder processes are spawned with stdio ignored, so a failure (e.g., port 8100 already in use) will go unnoticed until
pollStatusReadytimes out with a less specific error. Adding anerrorevent listener could provide clearer diagnostics:private startForward(localPort: number, devicePort: number): ChildProcess { const pmd = findPymobiledevice3(); if (!pmd) throw new WdaSetupError("pymobiledevice3 not found (needed for usbmux port forwarding)."); const child = spawn( pmd, ["usbmux", "forward", String(localPort), String(devicePort), "--serial", this.device.udid], - { stdio: ["ignore", "ignore", "ignore"] }, + { stdio: ["ignore", "ignore", "pipe"] }, ); + child.stderr?.on("data", (d) => debugDevice("forward stderr: %s", d.toString().trim())); + child.on("error", (err) => debugDevice("forward %d→%d spawn error: %s", localPort, devicePort, err.message)); this.forwards.push(child); return child; }This would surface port conflicts or process failures in debug logs rather than appearing as a generic timeout.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/serve-sim/src/wda.ts` around lines 334 - 345, The startForward method currently spawns the usbmux child with stdio ignored so failures (like port-in-use) are silent; modify the startForward function to attach listeners to the spawned ChildProcess (the variable child returned by spawn) to handle 'error' and 'exit' events, log descriptive diagnostics (including localPort, devicePort and child.pid) via the existing logger or throw a WdaSetupError when the process fails to start or exits with a non-zero code, and ensure the child is removed from this.forwards on exit to avoid stale references.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/serve-sim/src/device-server.ts`:
- Line 23: Remove the unused Server type from the import list so the import
reads only createServer, IncomingMessage and ServerResponse; the runtime
variable server (created via createServer) is already inferred so no explicit
Server annotation is needed—update the import line to drop Server and verify
there are no remaining references to the Server symbol (e.g., no type
annotations using Server).
- Around line 143-146: The data handler accumulating `raw` in the
`req.on("data", (chunk) => { ... })` callback currently calls `req.destroy()`
when `raw.length > 4096` without sending a response; change this so you first
send an HTTP 413 Payload Too Large to the client (e.g. set `res.statusCode =
413` or call `res.writeHead(413, ...)` and `res.end()` with a short message) and
then destroy the request to avoid the end handler race; update the
`req.on("data", ...)` branch that checks `raw.length > 4096` to perform these
steps instead of only calling `req.destroy()`.
In `@packages/serve-sim/src/wda.ts`:
- Line 30: The import list in packages/serve-sim/src/wda.ts includes an unused
symbol `execFile`; remove `execFile` from the named imports of "child_process"
so only `spawn`, `execFileSync`, and `type ChildProcess` remain (update the
import declaration that currently imports `spawn, execFile, execFileSync, type
ChildProcess`).
---
Nitpick comments:
In `@packages/serve-sim/src/wda.ts`:
- Around line 334-345: The startForward method currently spawns the usbmux child
with stdio ignored so failures (like port-in-use) are silent; modify the
startForward function to attach listeners to the spawned ChildProcess (the
variable child returned by spawn) to handle 'error' and 'exit' events, log
descriptive diagnostics (including localPort, devicePort and child.pid) via the
existing logger or throw a WdaSetupError when the process fails to start or
exits with a non-zero code, and ensure the child is removed from this.forwards
on exit to avoid stale references.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 525d5fd7-2240-40de-af85-a01973f34c38
📒 Files selected for processing (4)
packages/serve-sim/src/debug.tspackages/serve-sim/src/device-server.tspackages/serve-sim/src/index.tspackages/serve-sim/src/wda.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces a new "device mode" to the
serve-simtool, allowing users to stream the screen of a physical iOS device (iPhone/iPad) to their browser using WebDriverAgent (WDA). This is particularly useful for Intel Macs that can't run arm64-only simulators. The implementation includes a new relay server for device streaming, a minimal viewer UI, and a new CLI command for launching device mode.Device streaming support:
device-server.tsmodule that implements an HTTP relay server to bridge a browser to a physical iOS device via WDA. It provides endpoints for MJPEG streaming, input events, device config, and health checks, along with a minimal HTML viewer for live device interaction.debugDeviceindebug.tsfor real-device mode, aiding in troubleshooting device communication.CLI enhancements:
devicecommand to the CLI (index.ts) that detects connected iOS devices, sets up and launches a WDA session, and starts the device relay server. The command provides helpful error messages, setup instructions, and clean shutdown handling.devicecommand with the CLI, including documentation, usage examples, and environment variable overrides in the help text.Summary by CodeRabbit
New Features
Documentation