Skip to content
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

Upgrade appium solution #3876

Merged
merged 13 commits into from
Sep 4, 2024
Merged

Upgrade appium solution #3876

merged 13 commits into from
Sep 4, 2024

Conversation

MeniKadosh1
Copy link
Contributor

@MeniKadosh1 MeniKadosh1 commented Aug 26, 2024

Thank you for your contribution.
Before submitting this PR, please make sure:

  • PR description and commit message should describe the changes done in this PR
  • Verify the PR is pointing to correct branch i.e. Release or Beta branch if the code fix is for specific release , else point it to master
  • Latest Code from master or specific release branch is merged to your branch
  • No unwanted\commented\junk code is included
  • No new warning upon build solution
  • Code Summary\Comments are added to my code which explains what my code is doing
  • Existing unit test cases are passed
  • New Unit tests are added for your development
  • Sanity Tests are successfully executed for New and Existing Functionality
  • Verify that changes are compatible with all relevant browsers and platforms.
  • After creating pull request there should not be any conflicts
  • Resolve all Codacy comments
  • Builds and checks are passed before PR is sent for review
  • Resolve code review comments
  • Update the Help Library document to match any feature changes

Summary by CodeRabbit

  • New Features

    • Enhanced mobile device action capabilities with new input fields for application package, press duration, drag duration, swipe scale, and swipe duration.
    • Added support for new user interactions: double-tap and press actions.
    • Introduced a button for opening an external view of the device and zooming capabilities.
  • Improvements

    • Improved dynamic interface responsiveness based on selected mobile device actions.
    • Enhanced mouse interaction logic for more nuanced click and drag actions based on duration.
    • Adjusted window properties for better usability and user experience.
  • Bug Fixes

    • Updated error handling in mobile interaction methods for better feedback on action failures.

@MeniKadosh1 MeniKadosh1 changed the base branch from master to Releases/Beta August 26, 2024 12:16
Copy link
Contributor

coderabbitai bot commented Aug 26, 2024

Walkthrough

The changes introduced include modifications to the user interface for mobile device actions, enhancements to mouse interaction handling, and updates to method signatures for better control over mobile gestures. New properties and enumerations have been added, aiming to improve the functionality and responsiveness of the application in managing user interactions and mobile device actions.

Changes

Files Change Summary
.../Actions/ActionEditPages/ActMobileDeviceEditPage.xaml, .../Actions/ActionEditPages/ActMobileDeviceEditPage.xaml.cs Adjusted UI layout and added new input panels for mobile actions. Enhanced methods to bind and set control views based on user actions.
.../Drivers/DriversWindows/MobileDriverWindow.xaml, .../Drivers/DriversWindows/MobileDriverWindow.xaml.cs Modified UI properties and added new buttons for external view and zoom capabilities. Enhanced mouse interaction handling with updated method logic.
.../Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs Refactored to use new methods for touch actions, improving interaction handling with enhancements for swipes and taps.
.../Drivers/DriversWindow/IMobileDriverWindow.cs Updated method signatures to include new parameters for press duration, drag duration, and swipe dynamics, enhancing interaction flexibility.
.../GingerCoreNET/GingerCoreNET.csproj Updated Appium.WebDriver package from version 5.0.0-rc.6 to 5.1.0.
.../ActionsLib/UI/Legacy/ActGenElement.cs Added new enumerations DoubleTapElement and PressElement to eGenElementAction.
.../ActionsLib/UI/Mobile/ActMobileDevice.cs Introduced new properties for mobile actions and updated the eMobileDeviceAction enum to include DoubleTapXY.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant MobileDriver
    participant AppiumDriver

    User->>MobileDriver: PerformAction(actionType)
    MobileDriver->>AppiumDriver: ExecuteAction(actionType, parameters)
    AppiumDriver->>AppiumDriver: HandleAction(actionType)
    AppiumDriver-->>MobileDriver: ActionResult
    MobileDriver-->>User: DisplayActionResult
Loading

🐇 Hop, hop, a change we see,
New features bloom like flowers free!
With swipes and taps, we dance and play,
In the world of code, we leap all day!
Let's cheer for progress, bouncy and bright,
As our app takes flight, oh what a sight! 🌼


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai or @coderabbitai title anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between e7d1ad4 and 0a7a1df.

Files selected for processing (13)
  • Ginger/Ginger/Actions/ActionEditPages/ActMobileDeviceEditPage.xaml (2 hunks)
  • Ginger/Ginger/Actions/ActionEditPages/ActMobileDeviceEditPage.xaml.cs (3 hunks)
  • Ginger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.cs (10 hunks)
  • Ginger/Ginger/Ginger.csproj (1 hunks)
  • Ginger/GingerCore/GingerCore.csproj (1 hunks)
  • Ginger/GingerCoreCommon/GingerCoreCommon.csproj (1 hunks)
  • Ginger/GingerCoreNET/ActionsLib/UI/Legacy/ActGenElement.cs (1 hunks)
  • Ginger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.cs (2 hunks)
  • Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs (26 hunks)
  • Ginger/GingerCoreNET/Drivers/DriversWindow/IMobileDriverWindow.cs (1 hunks)
  • Ginger/GingerCoreNET/GingerCoreNET.csproj (2 hunks)
  • Ginger/GingerPluginCore/GingerPluginCore.csproj (1 hunks)
  • Ginger/GingerUtils/GingerUtils.csproj (1 hunks)
Files skipped from review due to trivial changes (4)
  • Ginger/Ginger/Ginger.csproj
  • Ginger/GingerCore/GingerCore.csproj
  • Ginger/GingerCoreCommon/GingerCoreCommon.csproj
  • Ginger/GingerUtils/GingerUtils.csproj
Additional context used
GitHub Check: Codacy Static Code Analysis
Ginger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.cs

[warning] 1507-1507: Ginger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.cs#L1507
Return 'Task' instead.


[warning] 1559-1559: Ginger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.cs#L1559
Return 'Task' instead.

Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs

[warning] 673-673: Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs#L673
Remove this commented out code.


[warning] 683-683: Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs#L683
Remove this commented out code.


[warning] 965-965: Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs#L965
Remove this commented out code.


[notice] 3287-3287: Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs#L3287
Remove this unnecessary cast to 'TimeSpan'.

Additional comments not posted (40)
Ginger/GingerPluginCore/GingerPluginCore.csproj (1)

19-19: LGTM! Verify compatibility with the new package version.

The update to System.Drawing.Common version 8.0.7 may include bug fixes, performance improvements, or new features. Ensure compatibility with the rest of the codebase.

The code changes are approved.

Run the following script to verify compatibility:

Ginger/GingerCoreNET/Drivers/DriversWindow/IMobileDriverWindow.cs (3)

50-50: LGTM! Verify implementations of the updated method.

The addition of the optional pressDuration parameter enhances flexibility. Ensure that all implementations of this interface are updated accordingly.

The code changes are approved.

Run the following script to verify implementations:

Verification successful

Implementation of PerformLongPress is consistent with the interface.

The method in GenericAppiumDriver.cs correctly implements the interface method PerformLongPress with the optional TimeSpan? parameter, despite a difference in parameter naming (clickDuration vs. pressDuration). Ensure consistency in parameter naming if necessary for clarity.

  • GenericAppiumDriver.cs: Implementation uses clickDuration instead of pressDuration.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify implementations of the updated method.

# Test: Search for the method implementations. Expect: Only occurrences of the new signature.
rg --type cs 'void PerformLongPress' -A 5

Length of output: 1436


54-54: LGTM! Verify implementations of the updated method.

The addition of the swipeScale and swipeDuration parameters allows for more nuanced swipe gestures. Ensure that all implementations of this interface are updated accordingly.

The code changes are approved.

Run the following script to verify implementations:

Verification successful

Implementations Updated Correctly

The PerformScreenSwipe method has been updated with the new parameters swipeScale and swipeDuration in all relevant implementations. The changes have been correctly propagated across the codebase.

  • GenericAppiumDriver.cs: Method implemented with the new signature.
  • MobileDriverWindow.xaml.cs: Method implemented with the new signature.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify implementations of the updated method.

# Test: Search for the method implementations. Expect: Only occurrences of the new signature.
rg --type cs 'void PerformScreenSwipe' -A 5

Length of output: 1916


52-52: LGTM! Verify implementations of the updated method.

The addition of the pressDuration and dragDuration parameters provides more control over the timing of the drag operation. Ensure that all implementations of this interface are updated accordingly.

The code changes are approved.

Run the following script to verify implementations:

Verification successful

All implementations of PerformDrag are updated correctly.

The PerformDrag method has been implemented in GenericAppiumDriver.cs with the updated signature, and its usage in MobileDriverWindow.xaml.cs is consistent with this update. No other implementations were found, confirming that the update is applied correctly throughout the codebase.

  • GenericAppiumDriver.cs: Implementation matches the updated signature.
  • MobileDriverWindow.xaml.cs: Usage is consistent with the updated method signature.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify implementations of the updated method.

# Test: Search for the method implementations. Expect: Only occurrences of the new signature.
rg --type cs 'void PerformDrag' -A 5

Length of output: 1415


Script:

#!/bin/bash
# Description: Search for any other implementations of the PerformDrag method in the codebase.

# Test: Look for any other occurrences of PerformDrag to ensure all implementations are updated.
rg 'PerformDrag' -A 5

Length of output: 2177

Ginger/Ginger/Actions/ActionEditPages/ActMobileDeviceEditPage.xaml (5)

15-15: LGTM!

The change to the Width property for xOperationNameComboBox likely aims to enhance usability by providing more space for the displayed options.

The code changes are approved.


59-62: LGTM!

The addition of the xAppPnl StackPanel enhances the functionality of the page by allowing users to input the application package.

The code changes are approved.


64-67: LGTM!

The addition of the xPressPnl StackPanel enhances the functionality of the page by allowing users to input the press duration.

The code changes are approved.


69-72: LGTM!

The addition of the xDragPnl StackPanel enhances the functionality of the page by allowing users to input the drag duration.

The code changes are approved.


74-83: LGTM!

The addition of the xSwipePnl StackPanel enhances the functionality of the page by allowing users to input the swipe scale and duration.

The code changes are approved.

Ginger/Ginger/Actions/ActionEditPages/ActMobileDeviceEditPage.xaml.cs (2)

69-74: LGTM!

The added initialization calls for xAppPackageVE, xPressDurationTxtBox, xDragDurationTxtBox, xSwipeScaleTxtBox, and xSwipeDurationTxtBox are correctly implemented.

The code changes are approved.


Line range hint 206-263: LGTM!

The added cases for DoubleTapXY and LongPressXY and the visibility adjustments for xAppPnl, xPressPnl, xDragPnl, and xSwipePnl are correctly implemented.

The code changes are approved.

Ginger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.cs (6)

170-181: LGTM!

The ActionAppPackage property is correctly implemented, enhancing the configurability of mobile actions.

The code changes are approved.


183-194: LGTM!

The PressDuration property is correctly implemented, enhancing the configurability of mobile actions.

The code changes are approved.


196-207: LGTM!

The DragDuration property is correctly implemented, enhancing the configurability of mobile actions.

The code changes are approved.


209-220: LGTM!

The SwipeScale property is correctly implemented, enhancing the configurability of mobile actions.

The code changes are approved.


222-233: LGTM!

The SwipeDuration property is correctly implemented, enhancing the configurability of mobile actions.

The code changes are approved.


305-307: LGTM!

The new enum value DoubleTapXY is correctly added, expanding the set of available mobile actions.

The code changes are approved.

Ginger/GingerCoreNET/ActionsLib/UI/Legacy/ActGenElement.cs (1)

618-621: LGTM!

The new enum values DoubleTapElement and PressElement are correctly added, expanding the set of available actions.

The code changes are approved.

Ginger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.cs (8)

661-661: LGTM!

The variable mMoveStartTime is correctly declared and initialized.

The code changes are approved.


681-681: LGTM!

The code correctly sets mMoveStartTime to the current time.

The code changes are approved.


725-741: LGTM!

The logic for calculating pressDuration and dragDuration is correctly implemented. The additional parameters enhance the functionality of the drag and click actions.

The code changes are approved.


755-759: LGTM!

The additional swipeDuration parameter enhances the functionality of the swipe actions.

The code changes are approved.


1151-1176: LGTM!

The additional swipeDuration parameter enhances the functionality of the swipe actions.

The code changes are approved.


Line range hint 1507-1523: LGTM!

The method signature update and the use of the clickDuration parameter in the PerformLongPress method enhance the functionality of the click actions.

The code changes are approved.

Tools
GitHub Check: Codacy Static Code Analysis

[warning] 1507-1507: Ginger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.cs#L1507
Return 'Task' instead.


Line range hint 1559-1571: LGTM!

The method signature update and the use of the pressDuration and dragDuration parameters in the PerformDrag method enhance the functionality of the drag actions.

The code changes are approved.

Tools
GitHub Check: Codacy Static Code Analysis

[warning] 1559-1559: Ginger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.cs#L1559
Return 'Task' instead.


1644-1648: LGTM!

The method signature update and the use of the swipeDuration parameter enhance the functionality of the swipe actions.

The code changes are approved.

Ginger/GingerCoreNET/GingerCoreNET.csproj (2)

239-239: LGTM!

The upgrade from Appium.WebDriver version 5.0.0-rc.6 to 5.1.0 is a good move as it transitions from a release candidate to a stable version, likely including bug fixes and new features.

The code changes are approved.


336-336: LGTM!

The upgrade from System.Drawing.Common version 8.0.1 to 8.0.7 is a good move as it likely includes enhancements or fixes that could improve compatibility or performance.

The code changes are approved.

Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs (12)

56-58: LGTM!

The new imports are necessary for the interaction methods implemented in the file.


1439-1447: LGTM!

The SwipeByXY method is correctly implemented using PointerInputDevice and ActionBuilder.


1338-1346: LGTM!

The TapXY method is correctly implemented using PointerInputDevice and ActionBuilder.


1358-1369: LGTM!

The DoubleTapXY method is correctly implemented using PointerInputDevice and ActionBuilder.


1385-1394: LGTM!

The PressXY method is correctly implemented using PointerInputDevice and ActionBuilder.


Line range hint 2894-2909: LGTM!

The PerformDrag method is correctly implemented with additional parameters for press and drag durations.


Line range hint 1645-1684: LGTM!

The SwipeScreen method is correctly implemented with additional parameters for swipe scale and duration.


3279-3287: LGTM!

The PerformScreenSwipe method is correctly implemented with additional parameters for swipe scale and duration.

Tools
GitHub Check: Codacy Static Code Analysis

[notice] 3287-3287: Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs#L3287
Remove this unnecessary cast to 'TimeSpan'.


Line range hint 965-1165: LGTM!

The MobileDeviceActionHandler method is correctly updated to call the new interaction methods.


1348-1356: LGTM!

The TapElement method is correctly implemented using PointerInputDevice and ActionBuilder.


1371-1382: LGTM!

The DoubleTapElement method is correctly implemented using PointerInputDevice and ActionBuilder.


1415-1424: LGTM!

The PressElement method is correctly implemented using PointerInputDevice and ActionBuilder.

@@ -1500,7 +1504,7 @@
}
}

private async void DeviceScreenshotImageMouseClickAsync(System.Windows.Point clickedPoint, bool performLongPress = false)
private async void DeviceScreenshotImageMouseClickAsync(System.Windows.Point clickedPoint, bool performLongPress = false, TimeSpan? clickDuration=null)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider returning Task instead of void for asynchronous methods.

Returning Task instead of void for asynchronous methods allows for better error handling and composition.

Apply this diff to change the return type:

-private async void DeviceScreenshotImageMouseClickAsync(System.Windows.Point clickedPoint, bool performLongPress = false, TimeSpan? clickDuration=null)
+private async Task DeviceScreenshotImageMouseClickAsync(System.Windows.Point clickedPoint, bool performLongPress = false, TimeSpan? clickDuration=null)

-private async void DeviceScreenshotImageMouseDragAsync(System.Windows.Point startPoint, System.Windows.Point endPoint, TimeSpan pressDuration, TimeSpan dragDuration)
+private async Task DeviceScreenshotImageMouseDragAsync(System.Windows.Point startPoint, System.Windows.Point endPoint, TimeSpan pressDuration, TimeSpan dragDuration)

Also applies to: 1559-1559

Tools
GitHub Check: Codacy Static Code Analysis

[warning] 1507-1507: Ginger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.cs#L1507
Return 'Task' instead.

- added UFT external link button
- changed behavior to stop mobile agent on close of mobile window
- add buttons to zoom in and out- part1
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 0a7a1df and 37bd706.

Files selected for processing (4)
  • Ginger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml (5 hunks)
  • Ginger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.cs (14 hunks)
  • Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs (48 hunks)
  • Ginger/GingerCoreNET/Drivers/DriversWindow/IMobileDriverWindow.cs (3 hunks)
Files skipped from review as they are similar to previous changes (2)
  • Ginger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.cs
  • Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs
Additional comments not posted (12)
Ginger/GingerCoreNET/Drivers/DriversWindow/IMobileDriverWindow.cs (5)

30-31: LGTM!

The addition of the IsUftLabDevice property is clear and useful for indicating whether the device is a UFT Lab device.

The code changes are approved.


52-52: LGTM!

The addition of the optional pressDuration parameter enhances the flexibility of the PerformLongPress method.

The code changes are approved.


54-54: LGTM!

The addition of the pressDuration and dragDuration parameters enhances control over the drag operation.

The code changes are approved.


56-56: LGTM!

The replacement of the impact parameter with swipeScale and the addition of the swipeDuration parameter provide more nuanced control over swipe gestures.

The code changes are approved.


68-69: LGTM!

The addition of the OpenDeviceExternalView method enhances the functionality for interacting with the device's external view.

The code changes are approved.

Ginger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml (7)

8-8: LGTM!

The changes to the WindowStyle and ResizeMode properties enhance the window's appearance and resizing behavior.

The code changes are approved.


99-99: LGTM!

The change to the Margin attribute of the xCordBtn button is acceptable and may improve its positioning within the grid layout.

The code changes are approved.


100-100: LGTM!

The change to the Margin attribute of the xSwipeBtn button is acceptable and may improve its positioning within the grid layout.

The code changes are approved.


101-101: LGTM!

The change to the Margin attribute of the xPortraiteBtn button is acceptable and may improve its positioning within the grid layout.

The code changes are approved.


102-102: LGTM!

The change to the Margin attribute of the xLandscapeBtn button is acceptable and may improve its positioning within the grid layout.

The code changes are approved.


112-112: LGTM!

The addition of the xExternalViewBtn button enhances the functionality for opening the device's external view.

The code changes are approved.


214-216: LGTM!

The addition of the xZoomInBtn and xZoomOutBtn buttons enhances the functionality by providing zooming capabilities.

The code changes are approved.

@MeniKadosh1 MeniKadosh1 changed the base branch from Releases/Beta to master September 3, 2024 20:34
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 37bd706 and 77d047f.

Files selected for processing (2)
  • Ginger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml (7 hunks)
  • Ginger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.cs (15 hunks)
Additional context used
GitHub Check: Codacy Static Code Analysis
Ginger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.cs

[warning] 1055-1055: Ginger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.cs#L1055
Remove this commented out code.


[warning] 1510-1510: Ginger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.cs#L1510
Return 'Task' instead.


[warning] 1562-1562: Ginger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.cs#L1562
Return 'Task' instead.

Additional comments not posted (17)
Ginger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml (5)

7-8: LGTM!

The changes to the Title and WindowStyle properties are approved.


11-11: LGTM!

The change to the Background property of the Grid named xWindowGrid is approved.


Line range hint 27-60: LGTM!

The reformatting of the TabControl and TabItem elements improves readability without altering the functionality. The changes are approved.


75-76: LGTM!

The adjustment to the StackPanel containing the TabControl is approved. The overall layout logic is preserved.


97-100: Verify the event handlers and logic for the new buttons.

Several new buttons have been added to the user interface:

  • xCordBtn: Show Mouse Coordinates
  • xSwipeBtn: Perform Swipe
  • xPortraiteBtn: Switch to Portrait
  • xLandscapeBtn: Switch to Landscape
  • xExternalViewBtn: Open Device External View
  • xZoomInBtn: Zoom In
  • xZoomOutBtn: Zoom Out

Please ensure that the corresponding event handlers (xCordBtn_Click, xSwipeBtn_Click, xOrientationBtn_Click, xExternalViewBtn_Click, xZoomInBtn_Click, xZoomOutBtn_Click) and the associated logic are implemented correctly in the code-behind file.

Also applies to: 110-110, 212-215

Ginger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.cs (12)

235-238: LGTM!

The code changes are approved.


665-665: LGTM!

The code changes are approved.


685-685: LGTM!

The code changes are approved.


729-730: LGTM!

The code changes are approved.


735-737: LGTM!

The code changes are approved.


741-745: LGTM!

The code changes are approved.


759-763: LGTM!

The code changes are approved.


1051-1067: LGTM!

The code changes are approved.

Tools
GitHub Check: Codacy Static Code Analysis

[warning] 1055-1055: Ginger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.cs#L1055
Remove this commented out code.


1153-1178: LGTM!

The code changes are approved.


1647-1651: LGTM!

The code changes are approved.


1737-1773: LGTM!

The code changes are approved.


1510-1510: Consider returning Task instead of void for asynchronous methods.

Returning Task instead of void for asynchronous methods allows for better error handling and composition.

Apply this diff to change the return type:

-private async void DeviceScreenshotImageMouseClickAsync(System.Windows.Point clickedPoint, bool performLongPress = false, TimeSpan? clickDuration=null)
+private async Task DeviceScreenshotImageMouseClickAsync(System.Windows.Point clickedPoint, bool performLongPress = false, TimeSpan? clickDuration=null)

Likely invalid or redundant comment.

Tools
GitHub Check: Codacy Static Code Analysis

[warning] 1510-1510: Ginger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.cs#L1510
Return 'Task' instead.

@@ -1552,7 +1559,7 @@
}
}

private async void DeviceScreenshotImageMouseDragAsync(System.Windows.Point startPoint, System.Windows.Point endPoint)
private async void DeviceScreenshotImageMouseDragAsync(System.Windows.Point startPoint, System.Windows.Point endPoint, TimeSpan pressDuration, TimeSpan dragDuration)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider returning Task instead of void for asynchronous methods.

Returning Task instead of void for asynchronous methods allows for better error handling and composition.

Apply this diff to change the return type:

-private async void DeviceScreenshotImageMouseDragAsync(System.Windows.Point startPoint, System.Windows.Point endPoint, TimeSpan pressDuration, TimeSpan dragDuration)
+private async Task DeviceScreenshotImageMouseDragAsync(System.Windows.Point startPoint, System.Windows.Point endPoint, TimeSpan pressDuration, TimeSpan dragDuration)
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private async void DeviceScreenshotImageMouseDragAsync(System.Windows.Point startPoint, System.Windows.Point endPoint, TimeSpan pressDuration, TimeSpan dragDuration)
private async Task DeviceScreenshotImageMouseDragAsync(System.Windows.Point startPoint, System.Windows.Point endPoint, TimeSpan pressDuration, TimeSpan dragDuration)
Tools
GitHub Check: Codacy Static Code Analysis

[warning] 1562-1562: Ginger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.cs#L1562
Return 'Task' instead.

{
if (DevicePlatformType == eDevicePlatformType.Android)
{
appPackage = AppiumCapabilities.Where(x => x.Parameter == "appPackage" || x.Parameter == "appium:appPackage").FirstOrDefault().Value;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use Find if its list or FirstOrDefault instead of where e.g. AppiumCapabilities.FirstOrDefault(x => x.Parameter == "appPackage" || x.Parameter == "appium:appPackage").Value;

}
else
{
appPackage = AppiumCapabilities.Where(x => x.Parameter == "bundleId" || x.Parameter == "appium:bundleId").FirstOrDefault().Value;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use FirstOrDefault or Find

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 77d047f and 4c30b8a.

Files selected for processing (2)
  • Ginger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.cs (15 hunks)
  • Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs (48 hunks)
Additional context used
GitHub Check: Codacy Static Code Analysis
Ginger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.cs

[warning] 1507-1507: Ginger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.cs#L1507
Return 'Task' instead.


[warning] 1559-1559: Ginger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.cs#L1559
Return 'Task' instead.

Additional comments not posted (28)
Ginger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.cs (19)

235-238: LGTM!

The code changes are approved.


665-666: LGTM!

The code changes are approved.


685-686: LGTM!

The code changes are approved.


729-737: LGTM!

The code changes are approved.


741-745: LGTM!

The code changes are approved.


759-760: LGTM!

The code changes are approved.


763-764: LGTM!

The code changes are approved.


1051-1064: LGTM!

The code changes are approved.


1150-1151: LGTM!

The code changes are approved.


1155-1156: LGTM!

The code changes are approved.


1160-1161: LGTM!

The code changes are approved.


1165-1166: LGTM!

The code changes are approved.


1170-1171: LGTM!

The code changes are approved.


1175-1176: LGTM!

The code changes are approved.


1214-1215: LGTM!

The code changes are approved.


1251-1252: LGTM!

The code changes are approved.


1523-1524: LGTM!

The code changes are approved.


1507-1508: Consider returning Task instead of void for asynchronous methods.

Returning Task instead of void for asynchronous methods allows for better error handling and composition.

Apply this diff to change the return type:

-private async void DeviceScreenshotImageMouseClickAsync(System.Windows.Point clickedPoint, bool performLongPress = false, TimeSpan? clickDuration=null)
+private async Task DeviceScreenshotImageMouseClickAsync(System.Windows.Point clickedPoint, bool performLongPress = false, TimeSpan? clickDuration=null)
Tools
GitHub Check: Codacy Static Code Analysis

[warning] 1507-1507: Ginger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.cs#L1507
Return 'Task' instead.


1559-1560: Consider returning Task instead of void for asynchronous methods.

Returning Task instead of void for asynchronous methods allows for better error handling and composition.

Apply this diff to change the return type:

-private async void DeviceScreenshotImageMouseDragAsync(System.Windows.Point startPoint, System.Windows.Point endPoint, TimeSpan pressDuration, TimeSpan dragDuration)
+private async Task DeviceScreenshotImageMouseDragAsync(System.Windows.Point startPoint, System.Windows.Point endPoint, TimeSpan pressDuration, TimeSpan dragDuration)
Tools
GitHub Check: Codacy Static Code Analysis

[warning] 1559-1559: Ginger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.cs#L1559
Return 'Task' instead.

Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs (9)

176-189: The new IsUftLabDevice property looks good!

It correctly returns true if the DeviceSource is MicroFoucsUFTMLab, false otherwise.


411-411: Handling "platformVersion" capability looks good.

The change allows setting the platform version capability via driverOptions.PlatformVersion which is important for mobile testing scenarios.


1010-1029: The new mobile device actions for swipe gestures look great!

The actions SwipeByCoordinates, SwipeDown, SwipeUp, SwipeLeft, SwipeRight enhance the mobile testing capabilities. They are properly implemented using the SwipeByXY and SwipeScreen methods under the hood.


Line range hint 1661-1698: The updates to the SwipeScreen method look great!

Adding the swipeScale and swipeDuration parameters enhances the flexibility of performing swipe actions. The swipe end point calculations based on the swipeScale factor appear to be correct.


1354-1410: The new TapXY, DoubleTapXY, and PressXY methods look great!

These methods provide a clean and efficient way to perform tap, double-tap, and press actions at specific coordinates. Using the PointerInputDevice and ActionBuilder classes aligns with Appium's recommended approach for touch actions. The implementations appear to be correct and follow best practices.


3378-3406: The new OpenDeviceExternalView method is a nice addition!

It provides a convenient way to open the device's external view in a browser. The URL construction logic, using the Appium server details and the device UDID, appears to be correct. Launching the browser with the constructed URL is properly implemented.


Line range hint 3295-3326: The updates to the PerformScreenSwipe method look good!

The method now utilizes the enhanced SwipeScreen method, passing the swipeScale and swipeDuration parameters for better control over the swipe action. The recording logic is properly implemented, creating an appropriate ActMobileDevice action based on the swipe side and capturing the relevant details.


Line range hint 3378-3538: The new device information retrieval methods are a valuable addition!

Methods like GetDeviceCPUInfo, GetDeviceMemoryInfo, GetDeviceNetworkInfo, GetDeviceBatteryInfo, and GetDeviceGeneralInfo provide useful capabilities to access various device details. Utilizing the Appium's ExecuteScript command is the correct approach to retrieve this information. The method implementations appear to be clean and efficient.


3378-3406: The new OpenDeviceSettings method is a handy addition!

It provides a convenient way to programmatically open the device settings. The implementation handles both Android and iOS devices appropriately. For Android, using the PressKeyCode method with the settings key code is the correct approach. For iOS, using the ActivateApp method to open the settings app is the right way. The implementation follows platform-specific best practices.

@Maheshkale447 Maheshkale447 merged commit 51f727f into master Sep 4, 2024
7 of 9 checks passed
@Maheshkale447 Maheshkale447 deleted the Upgrade-Appium-Solution branch September 4, 2024 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants