-
Notifications
You must be signed in to change notification settings - Fork 8
feat: support raw view tree ui dumps #121
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds a --format flag to the dump UI CLI and routes requests to either a new raw-source path or the existing parsed-elements path. Devices and WDA client gain DumpSourceRaw/GetSourceRaw methods; command and CLI layers pass and return the new format and raw data, with improved error wrapping. Changes
sequenceDiagram
participant User
participant CLI as CLI (cli/dump.go)
participant Cmd as Command (commands/dump.go)
participant Device as Device (android/ios/simulator)
participant WDA as WDA Client (devices/wda/source.go)
User->>CLI: dump ui --format raw
CLI->>CLI: parse flags
CLI->>Cmd: DumpUI(DumpUIRequest{Format:"raw"})
alt Format == "raw"
Cmd->>Device: DumpSourceRaw()
Device->>WDA: GetSourceRaw()
WDA-->>Device: raw interface{}
Device-->>Cmd: raw interface{}
Cmd->>Cmd: set RawData
else Format != "raw"
Cmd->>Device: DumpSource()
Device->>Device: parse to ScreenElements
Device-->>Cmd: []ScreenElement
Cmd->>Cmd: set Elements
end
Cmd-->>CLI: DumpUIResponse
CLI-->>User: output
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
cli/dump.go (1)
46-46: Clarify the default format behavior in help text.The help text states "Default: json" but when the format is empty, the code returns parsed
Elements(not a "json" format per se). Consider updating the description to clarify the actual behavior:- dumpUICmd.Flags().StringVar(&dumpUIFormat, "format", "", "Output format: 'raw' for unprocessed tree from agent (Default: json)") + dumpUICmd.Flags().StringVar(&dumpUIFormat, "format", "", "Output format: 'raw' for unprocessed tree from agent (default: parsed elements)")devices/wda/source.go (1)
113-113: Consider log verbosity for large UI trees.The verbose logging of the entire WDA source response could produce very large output for complex UIs. If this becomes problematic, consider truncating or summarizing the logged data.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
cli/dump.go(2 hunks)commands/dump.go(1 hunks)devices/android.go(2 hunks)devices/common.go(1 hunks)devices/ios.go(1 hunks)devices/simulator.go(1 hunks)devices/wda/source.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
devices/common.go (2)
devices/android_display_test.go (1)
name(41-45)commands/screenshot.go (1)
DeviceID(16-21)
devices/android.go (2)
devices/common.go (1)
ScreenElement(38-38)types/screen.go (1)
ScreenElement(10-19)
commands/dump.go (3)
devices/common.go (2)
ScreenElement(38-38)StartAgentConfig(31-33)types/screen.go (1)
ScreenElement(10-19)commands/commands.go (3)
CommandResponse(11-15)FindDeviceOrAutoSelect(64-97)NewErrorResponse(26-31)
cli/dump.go (1)
commands/dump.go (1)
DumpUIRequest(9-12)
devices/wda/source.go (4)
devices/wda/types.go (1)
WdaClient(9-13)devices/common.go (1)
ScreenElement(38-38)types/screen.go (1)
ScreenElement(10-19)utils/logger.go (1)
Verbose(19-23)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint
- GitHub Check: Analyze (go)
🔇 Additional comments (12)
devices/simulator.go (1)
775-777: LGTM!The
DumpSourceRaw()implementation correctly delegates to the WDA client and follows the same pattern as the existingDumpSource()method.devices/ios.go (1)
665-667: LGTM!The
DumpSourceRaw()implementation is consistent with the simulator device and correctly delegates to the WDA client.devices/common.go (1)
68-68: LGTM!The interface extension properly adds
DumpSourceRaw()to support raw format output. Usinginterface{}as the return type is appropriate since Android returns XML strings while iOS/Simulator returns JSON structures.devices/android.go (2)
957-961: Good improvement on error wrapping.Using
%wfor error wrapping enables proper error unwrapping witherrors.Is()anderrors.As().
983-1015: LGTM!The refactoring cleanly separates raw data retrieval (
DumpSourceRaw) from parsing (DumpSource). The type assertion with a clear error message handles the interface{} to string conversion appropriately.devices/wda/source.go (3)
84-84: Good improvement on error wrapping.Using
%wenables proper error chain inspection.
90-105: LGTM!The
GetSourceRaw()method correctly extracts the raw value from the WDA response and handles missing "value" field appropriately.
107-128: LGTM!The refactoring to use
GetSourceRaw()is clean, and the improved error wrapping with%wis consistent throughout.commands/dump.go (4)
11-11: LGTM!The
Formatfield addition enables the raw dump functionality.
16-17: Good use of omitempty.The
omitemptytags on bothElementsandRawDataensure the JSON response only includes the relevant field based on the requested format.
25-31: Good error handling improvements.Error wrapping with
%wand including device ID in error messages improves debuggability.
34-56: LGTM!The conditional logic cleanly separates raw and parsed dump paths. The implementation correctly handles the format selection.
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
✏️ Tip: You can customize this high-level summary in your review settings.