refactor: change package wda to devicekit - #312
Merged
Conversation
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (27)
WalkthroughThe change moves the WebDriverAgent implementation into the 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
devices/devicekit/gesture.go (1)
19-79: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReject unsupported gesture action types instead of silently dropping them.
convertActionshas no default case, so malformed or future actions are omitted and the RPC may execute a truncated gesture successfully. This is inconsistent withdevices/android.go, which returns an error for unsupported types.Proposed fix
-func convertActions(actions []TapAction) []gestureAction { +func convertActions(actions []TapAction) ([]gestureAction, error) { ... case "pause": if len(result) > 0 { result[len(result)-1].Duration += float64(a.Duration) / 1000.0 } + default: + return nil, fmt.Errorf("unsupported gesture action type: %s", a.Type) } } - return result + return result, nil } func (c *WdaClient) Gesture(actions []TapAction) error { + converted, err := convertActions(actions) + if err != nil { + return err + } params := map[string]any{ - "actions": convertActions(actions), + "actions": converted, }Also applies to: 81-87
🤖 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 `@devices/devicekit/gesture.go` around lines 19 - 79, Update convertActions to detect unsupported TapAction.Type values and return an error instead of silently omitting them. Propagate the error through its callers so the gesture RPC fails without executing a truncated action sequence, matching the unsupported-type handling in devices/android.go.
🤖 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 `@devices/devicekit/mjpeg/screencapture.go`:
- Around line 13-43: Bound the initial MJPEG connection and probe while
preserving an unbounded client for the established stream: in
devices/devicekit/mjpeg/screencapture.go lines 13-43, replace the unbounded
request context or client behavior used by the connection retry with a
deadline-limited probe, then switch to unbounded streaming only after the 200
response is established; in devices/devicekit/mjpeg/health.go lines 5-11, update
CheckHealth to use a bounded context or read deadline while waiting for the
first body bytes so health checks cannot hang.
---
Outside diff comments:
In `@devices/devicekit/gesture.go`:
- Around line 19-79: Update convertActions to detect unsupported TapAction.Type
values and return an error instead of silently omitting them. Propagate the
error through its callers so the gesture RPC fails without executing a truncated
action sequence, matching the unsupported-type handling in devices/android.go.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a06a1f9e-3eaa-49d4-bbb3-903c73588f03
📒 Files selected for processing (27)
commands/input.godevices/android.godevices/common.godevices/devicekit/active-app.godevices/devicekit/gesture.godevices/devicekit/longpress.godevices/devicekit/mjpeg/health.godevices/devicekit/mjpeg/screencapture.godevices/devicekit/mjpeg/types.godevices/devicekit/open-url.godevices/devicekit/orientation.godevices/devicekit/press-button.godevices/devicekit/press-keys.godevices/devicekit/requests.godevices/devicekit/screenshot.godevices/devicekit/send-keys.godevices/devicekit/source.godevices/devicekit/source_test.godevices/devicekit/status.godevices/devicekit/swipe.godevices/devicekit/tap.godevices/devicekit/types.godevices/devicekit/windowsize.godevices/ios.godevices/keys.godevices/remote.godevices/simulator.go
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
devices/devicekit/gesture.go (1)
19-79: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReject unsupported gesture action types instead of silently dropping them.
convertActionshas no default case, so malformed or future actions are omitted and the RPC may execute a truncated gesture successfully. This is inconsistent withdevices/android.go, which returns an error for unsupported types.Proposed fix
-func convertActions(actions []TapAction) []gestureAction { +func convertActions(actions []TapAction) ([]gestureAction, error) { ... case "pause": if len(result) > 0 { result[len(result)-1].Duration += float64(a.Duration) / 1000.0 } + default: + return nil, fmt.Errorf("unsupported gesture action type: %s", a.Type) } } - return result + return result, nil } func (c *WdaClient) Gesture(actions []TapAction) error { + converted, err := convertActions(actions) + if err != nil { + return err + } params := map[string]any{ - "actions": convertActions(actions), + "actions": converted, }Also applies to: 81-87
🤖 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 `@devices/devicekit/gesture.go` around lines 19 - 79, Update convertActions to detect unsupported TapAction.Type values and return an error instead of silently omitting them. Propagate the error through its callers so the gesture RPC fails without executing a truncated action sequence, matching the unsupported-type handling in devices/android.go.
🤖 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 `@devices/devicekit/mjpeg/screencapture.go`:
- Around line 13-43: Bound the initial MJPEG connection and probe while
preserving an unbounded client for the established stream: in
devices/devicekit/mjpeg/screencapture.go lines 13-43, replace the unbounded
request context or client behavior used by the connection retry with a
deadline-limited probe, then switch to unbounded streaming only after the 200
response is established; in devices/devicekit/mjpeg/health.go lines 5-11, update
CheckHealth to use a bounded context or read deadline while waiting for the
first body bytes so health checks cannot hang.
---
Outside diff comments:
In `@devices/devicekit/gesture.go`:
- Around line 19-79: Update convertActions to detect unsupported TapAction.Type
values and return an error instead of silently omitting them. Propagate the
error through its callers so the gesture RPC fails without executing a truncated
action sequence, matching the unsupported-type handling in devices/android.go.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a06a1f9e-3eaa-49d4-bbb3-903c73588f03
📒 Files selected for processing (27)
commands/input.godevices/android.godevices/common.godevices/devicekit/active-app.godevices/devicekit/gesture.godevices/devicekit/longpress.godevices/devicekit/mjpeg/health.godevices/devicekit/mjpeg/screencapture.godevices/devicekit/mjpeg/types.godevices/devicekit/open-url.godevices/devicekit/orientation.godevices/devicekit/press-button.godevices/devicekit/press-keys.godevices/devicekit/requests.godevices/devicekit/screenshot.godevices/devicekit/send-keys.godevices/devicekit/source.godevices/devicekit/source_test.godevices/devicekit/status.godevices/devicekit/swipe.godevices/devicekit/tap.godevices/devicekit/types.godevices/devicekit/windowsize.godevices/ios.godevices/keys.godevices/remote.godevices/simulator.go
🛑 Comments failed to post (1)
devices/devicekit/mjpeg/screencapture.go (1)
13-43: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== file list ==\n' git ls-files 'devices/devicekit/mjpeg/*' printf '\n== outlines ==\n' ast-grep outline devices/devicekit/mjpeg/screencapture.go --view expanded || true ast-grep outline devices/devicekit/mjpeg/health.go --view expanded || true printf '\n== screencapture.go ==\n' cat -n devices/devicekit/mjpeg/screencapture.go | sed -n '1,220p' printf '\n== health.go ==\n' cat -n devices/devicekit/mjpeg/health.go | sed -n '1,200p'Repository: mobile-next/mobilecli
Length of output: 2996
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== search for related timeouts / contexts ==\n' rg -n "Timeout: 0|context.Background\(|WithTimeout|WithDeadline|SetReadDeadline|CheckHealth\(|StartScreenCapture\(" devices/devicekit/mjpeg -S printf '\n== call sites of StartScreenCapture ==\n' rg -n "StartScreenCapture\(" -S .Repository: mobile-next/mobilecli
Length of output: 2368
🏁 Script executed:
#!/bin/bash set -euo pipefail sed -n '1,220p' devices/devicekit/mjpeg/screencapture.go | nl -ba printf '\n---\n' sed -n '1,120p' devices/devicekit/mjpeg/health.go | nl -baRepository: mobile-next/mobilecli
Length of output: 199
Bound the MJPEG connect/probe path.
Timeout: 0pluscontext.Background()letsclient.Doblock past the 10-second retry window, andCheckHealthcan hang waiting for the first body bytes after a 200 response. Use a bounded context or transport timeout for the initial probe, and keep the long-lived stream unbounded after it is established.
devices/devicekit/mjpeg/screencapture.go#L13-L43devices/devicekit/mjpeg/health.go#L5-L11📍 Affects 2 files
devices/devicekit/mjpeg/screencapture.go#L13-L43(this comment)devices/devicekit/mjpeg/health.go#L5-L11🤖 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 `@devices/devicekit/mjpeg/screencapture.go` around lines 13 - 43, Bound the initial MJPEG connection and probe while preserving an unbounded client for the established stream: in devices/devicekit/mjpeg/screencapture.go lines 13-43, replace the unbounded request context or client behavior used by the connection retry with a deadline-limited probe, then switch to unbounded streaming only after the 200 response is established; in devices/devicekit/mjpeg/health.go lines 5-11, update CheckHealth to use a bounded context or read deadline while waiting for the first body bytes so health checks cannot hang.
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.
No description provided.