-
Notifications
You must be signed in to change notification settings - Fork 62
Add Android TV support and enhance platform handling #4391
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
base: Releases/Official-Release
Are you sure you want to change the base?
Conversation
Enhanced platform support by introducing Android TV as a new platform type (`ePlatformType.AndroidTV`) alongside Mobile and iOS. Updated platform descriptions to include "Mobile/TV" for better clarity and added new image types for visual representation. UI components now display friendly platform descriptions and improved tooltips. Added ComboBox support for enum descriptions in grids. Extended the Appium driver to support Android TV with specific capabilities, configurations. Improved error handling and fallback mechanisms for driver initialization. Updated `ActMobileDevice` to include Android TV-specific actions and defaulted action descriptions to "Mobile/Tv Device Action." Enhanced `DeviceViewPage` to handle Android TV resolutions and scaling. Added dynamic screen size calculations with fallbacks for Android TV. Updated agent configurations to include Android TV as a selectable driver type and set default configurations for Android TV agents. Refactored code for better maintainability, improved exception handling, and consolidated driver configuration logic. Added new image resources for Android TV. Improved `GenericAppiumDriver` to handle Android TV-specific scenarios, including focused element detection and screenshot scaling.
WalkthroughThis pull request introduces Android TV support throughout the Ginger mobile test automation framework, refactors platform display to show descriptive enum text instead of raw values, adds Android TV-specific image assets and UI controls, extends the Appium driver with focused element detection for TV devices, and implements new coordinate mapping logic for improved device interaction handling. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Key areas requiring extra attention:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
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: 28
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
Ginger/Ginger/Drivers/DriversWindows/ADBDriverWindow/DeviceViewPage.xaml.cs (1)
521-552: Guard clauses and positioning logic are correct; consider renaming local scale variable for clarity.The added validation checks appropriately guard against null references and invalid state. Recalculating the scale factor from
DeviceImage.Source(rather than reusing the memberscaleFactorwhich is based onDeviceScreenShotImageBK.Source) is correct because:
DeviceScreenShotImageBKis the device frame background imageDeviceImageis the actual screenshot overlaid on the frame- These may have different dimensions, requiring distinct scale factors
However, reusing the variable name
scaleFactorfor a different purpose (overwriting the member variable in this local scope) could be confusing to future maintainers.💡 Optional refactor: Use a distinct local variable name for clarity
- double hscale = (double)image.Width / DeviceImage.ActualWidth; - double vscale = (double)image.Height / DeviceImage.ActualHeight; - scaleFactor = Math.Max(hscale, vscale); - if (scaleFactor <= 0) scaleFactor = 1; + double hscale = (double)image.Width / DeviceImage.ActualWidth; + double vscale = (double)image.Height / DeviceImage.ActualHeight; + double screenScaleFactor = Math.Max(hscale, vscale); + if (screenScaleFactor <= 0) screenScaleFactor = 1; - rect.Width = Width / scaleFactor; - rect.Height = Height / scaleFactor; + rect.Width = Width / screenScaleFactor; + rect.Height = Height / screenScaleFactor; double canvasLeft = DeviceScreenCanvas.Margin.Left; double canvasTop = DeviceScreenCanvas.Margin.Top; - double left = canvasLeft + X / scaleFactor; - double top = canvasTop + Y / scaleFactor; + double left = canvasLeft + X / screenScaleFactor; + double top = canvasTop + Y / screenScaleFactor; rect.Margin = new Thickness(left, top, 0, 0);This makes it clear that the highlighter uses a screen-specific scale factor distinct from the member
scaleFactor(which is frame-based).Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs (3)
1158-1202: AndroidTv is currently treated as iOS in GetAppPackage – adjust to reuse Android logic
GetAppPackagebranches only onDevicePlatformType == eDevicePlatformType.Android; any other value (including the newAndroidTv) goes down the iOS/bundleId capability path. For Android TV sessions this will ignore the usualappPackage/appium:appPackagecapabilities and likely setact.Erroreven though the app package is correctly configured.You should treat AndroidTv like Android here, for example:
Suggested fix for AndroidTv in GetAppPackage
- if (DevicePlatformType == eDevicePlatformType.Android) - { - param = AppiumCapabilities.FirstOrDefault(x => x.Parameter is "appPackage" or "appium:appPackage"); - } - else - { - param = AppiumCapabilities.FirstOrDefault(x => x.Parameter is "bundleId" or "appium:bundleId"); - } + if (DevicePlatformType == eDevicePlatformType.Android || + DevicePlatformType == eDevicePlatformType.AndroidTv) + { + param = AppiumCapabilities.FirstOrDefault(x => x.Parameter is "appPackage" or "appium:appPackage"); + } + else + { + param = AppiumCapabilities.FirstOrDefault(x => x.Parameter is "bundleId" or "appium:bundleId"); + }
1950-2071: AndroidTv screenshot size fallback double‑scales existing window sizeIn
GetUserCustomeScreenshotSize, when only one dimension is configured for Android TV, the other dimension falls back to0.7 * mWindowWidth/Height. SinceCalculateMobileDeviceScreenSizesalready setsmWindowWidth/mWindowHeightto ~70% of the device resolution for AndroidTv, this branch produces ~49% (0.7 * 0.7) of the real size instead of the intended “70% of device resolution”.Consider using
mWindowWidth/mWindowHeightdirectly (or reusing the samedeviceInfo-based 70% logic) instead of multiplying them by 0.7 again, e.g.:Suggested adjustment to avoid double scaling
- if (customeWidth == null) customeWidth = mWindowWidth > 0 ? (int?)Math.Round(mWindowWidth * 0.7) : (int?)(int)Math.Round(1920 * 0.7); - if (customeHeight == null) customeHeight = mWindowHeight > 0 ? (int?)Math.Round(mWindowHeight * 0.7) : (int?)(int)Math.Round(1080 * 0.7); + if (customeWidth == null) customeWidth = mWindowWidth > 0 ? mWindowWidth : (int?)(int)Math.Round(1920 * 0.7); + if (customeHeight == null) customeHeight = mWindowHeight > 0 ? mWindowHeight : (int?)(int)Math.Round(1080 * 0.7);
4891-4910: GetDeviceActivityAndPackage will throw for AndroidTv because it casts AndroidDriver to IOSDriver
GetDeviceActivityAndPackagechecks onlyDevicePlatformType == eDevicePlatformType.Android; any other platform (includingAndroidTv) goes into theelsebranch, whereDriveris cast toIOSDriver. For Android TV sessionsDriveris still anAndroidDriver, so this will throw anInvalidCastExceptionas soon as the method is used.You should treat
AndroidTvthe same asAndroidhere, e.g.:Suggested fix for AndroidTv in GetDeviceActivityAndPackage
- public Dictionary<string, string> GetDeviceActivityAndPackage() - { - if (DevicePlatformType == eDevicePlatformType.Android) + public Dictionary<string, string> GetDeviceActivityAndPackage() + { + if (DevicePlatformType == eDevicePlatformType.Android || + DevicePlatformType == eDevicePlatformType.AndroidTv) { Dictionary<string, string> dict = new Dictionary<string, string> { { "Activity",string.Concat(((AndroidDriver)Driver).CurrentPackage, ((AndroidDriver)Driver).CurrentActivity) }, { "Package", ((AndroidDriver)Driver).CurrentPackage } }; return dict; } else { Dictionary<string, string> dict2 = new Dictionary<string, string> { { "Bundle ID", (string)((IOSDriver)Driver).ExecuteScript("mobile: currentApp") } }; return dict2; } }Ginger/Ginger/SolutionWindows/AddSolutionPage.xaml.cs (2)
161-178: Platform string→enum mapping should support both “Mobile” and “Android TV”, not only “Mobile/TV”
ConvertStringToPlatformTypenow maps"Mobile/TV"toePlatformType.Mobile, but:
- Existing callers that still pass
"Mobile"will now fall through toNA.- New descriptions like
"Android TV"are not mapped at all, so any text-based flow using that label will also getNA.To keep things robust and backward compatible, consider:
Suggested expansion of ConvertStringToPlatformType
- return text switch - { - "Windows" => ePlatformType.Windows, - "Unix" => ePlatformType.Unix, - "Mobile/TV" => ePlatformType.Mobile, + return text switch + { + "Windows" => ePlatformType.Windows, + "Unix" => ePlatformType.Unix, + "Mobile/TV" => ePlatformType.Mobile, + "Mobile" => ePlatformType.Mobile, // backward compatibility + "Android TV" => ePlatformType.AndroidTV, "Web" => ePlatformType.Web, "DOS" => ePlatformType.DOS, "Java" => ePlatformType.Java, "WebServices" => ePlatformType.WebServices, "ASCF" => ePlatformType.ASCF, "MainFrame" => ePlatformType.MainFrame, "PowerBuilder" => ePlatformType.PowerBuilder, "Service" => ePlatformType.Service, _ => ePlatformType.NA, };
193-235: AddFirstAgentForSolutionForApplicationPlatfrom lacks a default driver for AndroidTVThe initial agent creation switch only handles
ePlatformType.Mobile(mapping toAgent.eDriverType.Appium) and doesn’t have a case forePlatformType.AndroidTV. If the main application platform is Android TV, the new solution’s first agent will either have an unset driver type or hit the “No default driver set for first agent” warning, which contradicts the PR goal of providing Android TV as a selectable driver type with defaults.Assuming
Agent.eDriverType.AppiumAndroidTvexists (per theGetDriverPlatformTypemapping in GingerCoreCommon/RunLib/Agent.cs), you likely want:Suggested AndroidTV mapping for the default agent
- case ePlatformType.Mobile: - agent.DriverType = Agent.eDriverType.Appium; - break; + case ePlatformType.Mobile: + agent.DriverType = Agent.eDriverType.Appium; + break; + case ePlatformType.AndroidTV: + agent.DriverType = Agent.eDriverType.AppiumAndroidTv; + break;
📜 Review details
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (3)
Ginger/Ginger/UserControlsLib/ImageMakerLib/Images/multiple_screens.pngis excluded by!**/*.png,!**/*.pngGinger/Ginger/UserControlsLib/ImageMakerLib/Images/radio-androidtv.pngis excluded by!**/*.png,!**/*.pngGinger/Ginger/UserControlsLib/ImageMakerLib/Images/whiteTv.pngis excluded by!**/*.png,!**/*.png
📒 Files selected for processing (31)
Ginger/Ginger/Actions/ActionsListViewHelper.csGinger/Ginger/Activities/ActivityPage.xaml.csGinger/Ginger/Agents/AgentEditPage.xaml.csGinger/Ginger/Agents/ApplicationAgentsMapPage.xamlGinger/Ginger/BusinessFlowWindows/EditBusinessFlowAppsPage.xaml.csGinger/Ginger/Drivers/DriversConfigsEditPages/AppiumDriverEditPage.xamlGinger/Ginger/Drivers/DriversConfigsEditPages/AppiumDriverEditPage.xaml.csGinger/Ginger/Drivers/DriversWindows/ADBDriverWindow/DeviceViewPage.xaml.csGinger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.csGinger/Ginger/Environments/AppsListPage.xaml.csGinger/Ginger/Ginger.csprojGinger/Ginger/SolutionWindows/AddApplicationPage.xaml.csGinger/Ginger/SolutionWindows/AddSolutionPage.xaml.csGinger/Ginger/SolutionWindows/TargetApplicationsPage.xaml.csGinger/Ginger/SolutionWindows/TreeViewItems/EnvironmentsTreeItems/EnvironmentApplicationList.xaml.csGinger/Ginger/UserControlsLib/ImageMakerLib/ImageMakerControl.xaml.csGinger/Ginger/UserControlsLib/UCListView/UcListViewItem.xaml.csGinger/Ginger/UserControlsLib/ucGridView/ComboEnumItemValueConverter.csGinger/Ginger/UserControlsLib/ucGridView/ucGrid.xaml.csGinger/GingerCoreCommon/Actions/Act.csGinger/GingerCoreCommon/Drivers/CoreDrivers/Mobile/MobileDriverCommon.csGinger/GingerCoreCommon/EnumsLib/eImageType.csGinger/GingerCoreCommon/EnvironmentLib/EnvApplication.csGinger/GingerCoreCommon/Repository/PlatformsLib/ApplicationPlatform.csGinger/GingerCoreCommon/Repository/PlatformsLib/Platform.csGinger/GingerCoreCommon/Run/ApplicationAgent.csGinger/GingerCoreCommon/RunLib/Agent.csGinger/GingerCoreNET/ActionsLib/ActValidation.csGinger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.csGinger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.csGinger/GingerCoreNET/RunLib/AgentOperations.cs
🧰 Additional context used
📓 Path-based instructions (6)
**/{Ginger,GingerCore}/**/{*Page,*Window}.xaml.cs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Name pages and windows following the pattern:
{Feature}Pageor{Feature}Window
Files:
Ginger/Ginger/Agents/AgentEditPage.xaml.csGinger/Ginger/Environments/AppsListPage.xaml.csGinger/Ginger/BusinessFlowWindows/EditBusinessFlowAppsPage.xaml.csGinger/Ginger/SolutionWindows/AddSolutionPage.xaml.csGinger/Ginger/SolutionWindows/TargetApplicationsPage.xaml.csGinger/Ginger/Drivers/DriversConfigsEditPages/AppiumDriverEditPage.xaml.csGinger/Ginger/SolutionWindows/AddApplicationPage.xaml.csGinger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.csGinger/Ginger/Drivers/DriversWindows/ADBDriverWindow/DeviceViewPage.xaml.csGinger/Ginger/Activities/ActivityPage.xaml.cs
**/*.cs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
Reporter.ToLog(eLogLevel.ERROR, message)for logging errors
Files:
Ginger/Ginger/Agents/AgentEditPage.xaml.csGinger/Ginger/Actions/ActionsListViewHelper.csGinger/Ginger/SolutionWindows/TreeViewItems/EnvironmentsTreeItems/EnvironmentApplicationList.xaml.csGinger/GingerCoreCommon/Run/ApplicationAgent.csGinger/Ginger/Environments/AppsListPage.xaml.csGinger/Ginger/BusinessFlowWindows/EditBusinessFlowAppsPage.xaml.csGinger/Ginger/SolutionWindows/AddSolutionPage.xaml.csGinger/Ginger/UserControlsLib/ucGridView/ComboEnumItemValueConverter.csGinger/GingerCoreCommon/Actions/Act.csGinger/Ginger/SolutionWindows/TargetApplicationsPage.xaml.csGinger/GingerCoreCommon/RunLib/Agent.csGinger/GingerCoreCommon/EnvironmentLib/EnvApplication.csGinger/GingerCoreCommon/EnumsLib/eImageType.csGinger/Ginger/Drivers/DriversConfigsEditPages/AppiumDriverEditPage.xaml.csGinger/GingerCoreCommon/Repository/PlatformsLib/ApplicationPlatform.csGinger/GingerCoreCommon/Drivers/CoreDrivers/Mobile/MobileDriverCommon.csGinger/GingerCoreNET/ActionsLib/ActValidation.csGinger/Ginger/SolutionWindows/AddApplicationPage.xaml.csGinger/GingerCoreNET/RunLib/AgentOperations.csGinger/Ginger/UserControlsLib/UCListView/UcListViewItem.xaml.csGinger/GingerCoreCommon/Repository/PlatformsLib/Platform.csGinger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.csGinger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.csGinger/Ginger/Drivers/DriversWindows/ADBDriverWindow/DeviceViewPage.xaml.csGinger/Ginger/Activities/ActivityPage.xaml.csGinger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.csGinger/Ginger/UserControlsLib/ImageMakerLib/ImageMakerControl.xaml.csGinger/Ginger/UserControlsLib/ucGridView/ucGrid.xaml.cs
**/{PlugInsLib,Agents}/*.cs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
New plugins should use
Agent.eAgentType.ServicewithPluginId/ServiceIdinstead of legacy DriverBase implementation
Files:
Ginger/Ginger/Agents/AgentEditPage.xaml.cs
**/{GingerCore,GingerCoreNET,GingerCoreCommon}/Actions/Act*.cs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/{GingerCore,GingerCoreNET,GingerCoreCommon}/Actions/Act*.cs: Actions should inherit fromActclass and implement platform-specific logic
Name actions following the pattern:Act{PlatformType}{ActionType}(e.g.,ActBrowserElement,ActConsoleCommand)
On action failure, setact.Errorandact.Status = eRunStatus.Failed
Files:
Ginger/GingerCoreCommon/Actions/Act.cs
**/*.csproj
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Target framework should be .NET 8.0 with Windows 10.0.17763.0 compatibility
Files:
Ginger/Ginger/Ginger.csproj
**/Repository/**/*.cs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Repository objects should use
[IsSerializedForLocalRepository]attribute for persistence
Files:
Ginger/GingerCoreCommon/Repository/PlatformsLib/ApplicationPlatform.csGinger/GingerCoreCommon/Repository/PlatformsLib/Platform.cs
🧠 Learnings (41)
📓 Common learnings
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 4222
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs:2645-2656
Timestamp: 2025-06-19T05:08:45.192Z
Learning: In the Ginger mobile automation framework (GenericAppiumDriver.cs), XPath checks for "android" and "XCUIElement" should remain case-sensitive as currently implemented. The team prefers not to make these checks case-insensitive.
📚 Learning: 2025-08-28T09:29:59.151Z
Learnt from: AmanPrasad43
Repo: Ginger-Automation/Ginger PR: 4286
File: Ginger/Ginger/Drivers/DriversConfigsEditPages/WebServicesDriverEditPage.xaml.cs:149-152
Timestamp: 2025-08-28T09:29:59.151Z
Learning: In Ginger's WebServicesDriverEditPage.xaml.cs, the FillComboItemsFromEnumType method should be used instead of FillComboFromEnumType when sorting by enum order is required. FillComboItemsFromEnumType preserves EnumValueDescription display through WPF's automatic ToString() calling on enum Content, and provides built-in sorting functionality via SortDescriptions.Add("text", Ascending). This was confirmed by AmanPrasad43 in PR #4286 for the ZAP vulnerability type dropdown.
Applied to files:
Ginger/Ginger/Agents/AgentEditPage.xaml.csGinger/Ginger/UserControlsLib/ucGridView/ComboEnumItemValueConverter.csGinger/Ginger/SolutionWindows/TargetApplicationsPage.xaml.csGinger/Ginger/SolutionWindows/AddApplicationPage.xaml.csGinger/Ginger/UserControlsLib/UCListView/UcListViewItem.xaml.csGinger/Ginger/UserControlsLib/ucGridView/ucGrid.xaml.cs
📚 Learning: 2025-09-04T09:34:04.299Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 4294
File: Ginger/GingerCoreCommon/UIElement/ElementInfo.cs:555-569
Timestamp: 2025-09-04T09:34:04.299Z
Learning: In Ginger/GingerCoreCommon/UIElement/ElementInfo.cs, the eLearnedType.AI enum value should be retained as it's required for future AI development features, as confirmed by prashelke.
Applied to files:
Ginger/Ginger/Agents/AgentEditPage.xaml.csGinger/GingerCoreCommon/RunLib/Agent.csGinger/GingerCoreCommon/EnumsLib/eImageType.csGinger/GingerCoreCommon/Drivers/CoreDrivers/Mobile/MobileDriverCommon.csGinger/Ginger/SolutionWindows/AddApplicationPage.xaml.csGinger/GingerCoreCommon/Repository/PlatformsLib/Platform.cs
📚 Learning: 2025-11-28T05:32:43.529Z
Learnt from: CR
Repo: Ginger-Automation/Ginger PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T05:32:43.529Z
Learning: Applies to **/{PlugInsLib,Agents}/*.cs : New plugins should use `Agent.eAgentType.Service` with `PluginId`/`ServiceId` instead of legacy DriverBase implementation
Applied to files:
Ginger/Ginger/Agents/AgentEditPage.xaml.csGinger/GingerCoreCommon/RunLib/Agent.csGinger/Ginger/SolutionWindows/AddApplicationPage.xaml.csGinger/GingerCoreNET/RunLib/AgentOperations.cs
📚 Learning: 2025-06-13T13:47:39.193Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 4228
File: Ginger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.cs:384-394
Timestamp: 2025-06-13T13:47:39.193Z
Learning: For the `eUnlockTypes` enum in `Ginger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.cs`, the identifiers must remain lowercase (`pin`, `password`, `pattern`, `none`) per project requirements.
Applied to files:
Ginger/Ginger/Agents/AgentEditPage.xaml.csGinger/GingerCoreCommon/Run/ApplicationAgent.csGinger/Ginger/SolutionWindows/AddSolutionPage.xaml.csGinger/GingerCoreCommon/RunLib/Agent.csGinger/GingerCoreCommon/EnumsLib/eImageType.csGinger/GingerCoreCommon/Drivers/CoreDrivers/Mobile/MobileDriverCommon.csGinger/GingerCoreCommon/Repository/PlatformsLib/Platform.csGinger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.cs
📚 Learning: 2025-09-05T09:30:10.074Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:5942-5960
Timestamp: 2025-09-05T09:30:10.074Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (around SVG helpers, approx. lines 5942–5960 and 5983–5984), maintainer prefers keeping ToLower() for tag/name normalization; do not suggest replacing with ToLowerInvariant() in future reviews for this area.
Applied to files:
Ginger/Ginger/Agents/AgentEditPage.xaml.csGinger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.csGinger/Ginger/Drivers/DriversWindows/ADBDriverWindow/DeviceViewPage.xaml.cs
📚 Learning: 2025-06-13T12:50:36.132Z
Learnt from: GokulBothe99
Repo: Ginger-Automation/Ginger PR: 4226
File: Ginger/GingerCoreCommon/Repository/BusinessFlowLib/BusinessFlow.cs:234-238
Timestamp: 2025-06-13T12:50:36.132Z
Learning: In the Ginger codebase, the `[AllowUserToEdit("<Label>")]` attribute is intentionally supplied with a string that represents the display label (e.g., `"Description"`, `"Active"`), not a default value. Suggestions to remove or change this parameter should be avoided unless the label itself is incorrect.
Applied to files:
Ginger/Ginger/Agents/AgentEditPage.xaml.cs
📚 Learning: 2025-11-28T05:32:43.529Z
Learnt from: CR
Repo: Ginger-Automation/Ginger PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T05:32:43.529Z
Learning: Applies to **/{GingerCore,GingerCoreNET,GingerCoreCommon}/Actions/Act*.cs : Name actions following the pattern: `Act{PlatformType}{ActionType}` (e.g., `ActBrowserElement`, `ActConsoleCommand`)
Applied to files:
Ginger/Ginger/Agents/AgentEditPage.xaml.csGinger/Ginger/Actions/ActionsListViewHelper.csGinger/GingerCoreCommon/Run/ApplicationAgent.csGinger/Ginger/SolutionWindows/AddSolutionPage.xaml.csGinger/GingerCoreCommon/Actions/Act.csGinger/GingerCoreNET/ActionsLib/ActValidation.csGinger/Ginger/UserControlsLib/UCListView/UcListViewItem.xaml.csGinger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.csGinger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.cs
📚 Learning: 2025-06-19T05:08:45.192Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 4222
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs:2645-2656
Timestamp: 2025-06-19T05:08:45.192Z
Learning: In the Ginger mobile automation framework (GenericAppiumDriver.cs), XPath checks for "android" and "XCUIElement" should remain case-sensitive as currently implemented. The team prefers not to make these checks case-insensitive.
Applied to files:
Ginger/Ginger/Agents/AgentEditPage.xaml.csGinger/Ginger/Drivers/DriversConfigsEditPages/AppiumDriverEditPage.xamlGinger/Ginger/SolutionWindows/AddSolutionPage.xaml.csGinger/GingerCoreCommon/RunLib/Agent.csGinger/Ginger/Drivers/DriversConfigsEditPages/AppiumDriverEditPage.xaml.csGinger/GingerCoreCommon/Drivers/CoreDrivers/Mobile/MobileDriverCommon.csGinger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.csGinger/Ginger/Drivers/DriversWindows/ADBDriverWindow/DeviceViewPage.xaml.csGinger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs
📚 Learning: 2025-06-16T10:36:01.993Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 4228
File: Ginger/Ginger/Actions/ActionEditPages/ActMobileDeviceEditPage.xaml.cs:83-84
Timestamp: 2025-06-16T10:36:01.993Z
Learning: For the `UnLockType` property in `Ginger/Ginger/Actions/ActionEditPages/ActMobileDeviceEditPage.xaml.cs`, the current naming convention with capital 'L' in the middle should be maintained per project preferences.
Applied to files:
Ginger/Ginger/Agents/AgentEditPage.xaml.csGinger/Ginger/SolutionWindows/AddSolutionPage.xaml.csGinger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.cs
📚 Learning: 2025-08-22T16:22:20.870Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 4280
File: Ginger/GingerCoreNET/GeneralLib/ElementWrapperInfo.cs:41-41
Timestamp: 2025-08-22T16:22:20.870Z
Learning: In Ginger/GingerCoreNET/GeneralLib/ElementWrapperInfo.cs, the user prashelke prefers to keep the current lowercase naming for properties like `elementGuid` rather than changing them to PascalCase, likely for consistency within the ElementWrapperInfo data model classes.
Applied to files:
Ginger/Ginger/Agents/AgentEditPage.xaml.cs
📚 Learning: 2025-12-18T05:29:42.896Z
Learnt from: AmanPrasad43
Repo: Ginger-Automation/Ginger PR: 4385
File: Ginger/GingerCoreNET/Run/Excels/ExcelNPOIOperations.cs:468-475
Timestamp: 2025-12-18T05:29:42.896Z
Learning: In the Ginger codebase, assume file extensions are lowercase (e.g., .xlsx, .xls). Do not perform case-insensitive extension checks (such as StringComparison.OrdinalIgnoreCase) when using EndsWith() or similar methods to verify file extensions; rely on lowercase comparisons or convert to lowercase first. This ensures consistency and avoids unnecessary case-insensitive comparisons across C# source files.
Applied to files:
Ginger/Ginger/Agents/AgentEditPage.xaml.csGinger/Ginger/Actions/ActionsListViewHelper.csGinger/Ginger/SolutionWindows/TreeViewItems/EnvironmentsTreeItems/EnvironmentApplicationList.xaml.csGinger/GingerCoreCommon/Run/ApplicationAgent.csGinger/Ginger/Environments/AppsListPage.xaml.csGinger/Ginger/BusinessFlowWindows/EditBusinessFlowAppsPage.xaml.csGinger/Ginger/SolutionWindows/AddSolutionPage.xaml.csGinger/Ginger/UserControlsLib/ucGridView/ComboEnumItemValueConverter.csGinger/GingerCoreCommon/Actions/Act.csGinger/Ginger/SolutionWindows/TargetApplicationsPage.xaml.csGinger/GingerCoreCommon/RunLib/Agent.csGinger/GingerCoreCommon/EnvironmentLib/EnvApplication.csGinger/GingerCoreCommon/EnumsLib/eImageType.csGinger/Ginger/Drivers/DriversConfigsEditPages/AppiumDriverEditPage.xaml.csGinger/GingerCoreCommon/Repository/PlatformsLib/ApplicationPlatform.csGinger/GingerCoreCommon/Drivers/CoreDrivers/Mobile/MobileDriverCommon.csGinger/GingerCoreNET/ActionsLib/ActValidation.csGinger/Ginger/SolutionWindows/AddApplicationPage.xaml.csGinger/GingerCoreNET/RunLib/AgentOperations.csGinger/Ginger/UserControlsLib/UCListView/UcListViewItem.xaml.csGinger/GingerCoreCommon/Repository/PlatformsLib/Platform.csGinger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.csGinger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.csGinger/Ginger/Drivers/DriversWindows/ADBDriverWindow/DeviceViewPage.xaml.csGinger/Ginger/Activities/ActivityPage.xaml.csGinger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.csGinger/Ginger/UserControlsLib/ImageMakerLib/ImageMakerControl.xaml.csGinger/Ginger/UserControlsLib/ucGridView/ucGrid.xaml.cs
📚 Learning: 2025-11-28T05:32:43.529Z
Learnt from: CR
Repo: Ginger-Automation/Ginger PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T05:32:43.529Z
Learning: Applies to **/{GingerCore,GingerCoreNET,GingerCoreCommon}/Actions/Act*.cs : Actions should inherit from `Act` class and implement platform-specific logic
Applied to files:
Ginger/Ginger/Actions/ActionsListViewHelper.csGinger/GingerCoreCommon/Actions/Act.csGinger/GingerCoreNET/ActionsLib/ActValidation.csGinger/Ginger/UserControlsLib/UCListView/UcListViewItem.xaml.csGinger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.cs
📚 Learning: 2024-07-26T22:04:12.930Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 3429
File: Ginger/Ginger/AutomatePageLib/AddActionMenu/WindowExplorer/Common/WindowExplorerPage.xaml.cs:1123-1126
Timestamp: 2024-07-26T22:04:12.930Z
Learning: The user has removed an unnecessary empty line at 1123 in `WindowExplorerPage.xaml.cs` for better code compactness.
Applied to files:
Ginger/Ginger/Environments/AppsListPage.xaml.csGinger/Ginger/Drivers/DriversWindows/ADBDriverWindow/DeviceViewPage.xaml.cs
📚 Learning: 2025-04-25T13:29:45.059Z
Learnt from: GokulBothe99
Repo: Ginger-Automation/Ginger PR: 4188
File: Ginger/GingerCoreNET/RunLib/CLILib/DoOptionsHanlder.cs:22-23
Timestamp: 2025-04-25T13:29:45.059Z
Learning: The `using Amdocs.Ginger.Repository;` statement in Ginger/GingerCoreNET/RunLib/CLILib/DoOptionsHanlder.cs is necessary as it provides access to the `ObservableList<>` class which is used throughout the file for collections of RunSetConfig, AnalyzerItemBase, and ApplicationPOMModel objects.
Applied to files:
Ginger/Ginger/Environments/AppsListPage.xaml.csGinger/GingerCoreCommon/Repository/PlatformsLib/ApplicationPlatform.csGinger/Ginger/UserControlsLib/UCListView/UcListViewItem.xaml.csGinger/Ginger/UserControlsLib/ucGridView/ucGrid.xaml.cs
📚 Learning: 2025-04-25T13:29:45.059Z
Learnt from: GokulBothe99
Repo: Ginger-Automation/Ginger PR: 4188
File: Ginger/GingerCoreNET/RunLib/CLILib/DoOptionsHanlder.cs:22-23
Timestamp: 2025-04-25T13:29:45.059Z
Learning: The `using Amdocs.Ginger.Repository;` statement in Ginger/GingerCoreNET/RunLib/CLILib/DoOptionsHanlder.cs is necessary as it imports the ObservableList<> class which is used throughout the file.
Applied to files:
Ginger/Ginger/Environments/AppsListPage.xaml.csGinger/GingerCoreCommon/Repository/PlatformsLib/ApplicationPlatform.csGinger/Ginger/UserControlsLib/UCListView/UcListViewItem.xaml.csGinger/Ginger/UserControlsLib/ucGridView/ucGrid.xaml.cs
📚 Learning: 2025-11-28T05:32:43.529Z
Learnt from: CR
Repo: Ginger-Automation/Ginger PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T05:32:43.529Z
Learning: WPF UI components should be in the Ginger/ directory; console components should be cross-platform compatible
Applied to files:
Ginger/Ginger/Environments/AppsListPage.xaml.csGinger/Ginger/SolutionWindows/AddApplicationPage.xaml.csGinger/Ginger/UserControlsLib/UCListView/UcListViewItem.xaml.csGinger/Ginger/UserControlsLib/ucGridView/ucGrid.xaml.cs
📚 Learning: 2025-09-04T15:43:57.789Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:9146-9291
Timestamp: 2025-09-04T15:43:57.789Z
Learning: In SeleniumDriver.cs, prefer using the C# fallback GenerateEnhancedSvgXPath(IWebElement) for SVG XPath generation instead of injecting/validating a JavaScript-based XPath builder. This was accepted in PR Ginger-Automation/Ginger#4294.
Applied to files:
Ginger/Ginger/Drivers/DriversConfigsEditPages/AppiumDriverEditPage.xamlGinger/Ginger/Drivers/DriversConfigsEditPages/AppiumDriverEditPage.xaml.csGinger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.csGinger/Ginger/Drivers/DriversWindows/ADBDriverWindow/DeviceViewPage.xaml.csGinger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs
📚 Learning: 2025-12-18T05:19:56.687Z
Learnt from: AmanPrasad43
Repo: Ginger-Automation/Ginger PR: 4385
File: Ginger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml:0-0
Timestamp: 2025-12-18T05:19:56.687Z
Learning: In XAML DataTriggers, enum properties can be compared to string values because WPF automatically converts enums to strings during evaluation. This pattern is valid for XAML files like ActExcelEditPage.xaml where eExcelActionType is compared to string literals such as "ReadCellByIndex" or "GetSheetDetails". Apply this guideline broadly to XAML files that bind to enum properties; verify that such comparisons behave as expected in the specific UI components and avoid relying on string literals if the enum values change.
Applied to files:
Ginger/Ginger/Drivers/DriversConfigsEditPages/AppiumDriverEditPage.xamlGinger/Ginger/Agents/ApplicationAgentsMapPage.xaml
📚 Learning: 2025-11-28T05:32:43.529Z
Learnt from: CR
Repo: Ginger-Automation/Ginger PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T05:32:43.529Z
Learning: Applies to **/{Ginger,GingerCore}/**/{*Page,*Window}.xaml.cs : Name pages and windows following the pattern: `{Feature}Page` or `{Feature}Window`
Applied to files:
Ginger/Ginger/SolutionWindows/AddSolutionPage.xaml.cs
📚 Learning: 2025-12-18T05:20:03.536Z
Learnt from: AmanPrasad43
Repo: Ginger-Automation/Ginger PR: 4385
File: Ginger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml:0-0
Timestamp: 2025-12-18T05:20:03.536Z
Learning: In the Ginger codebase, WPF DataTriggers can use string values (e.g., "ReadCellByIndex", "GetSheetDetails") to compare against enum properties (e.g., eExcelActionType) because WPF automatically handles enum-to-string conversion during DataTrigger evaluation. This pattern is confirmed to work correctly and is acceptable in XAML files.
Applied to files:
Ginger/Ginger/UserControlsLib/ucGridView/ComboEnumItemValueConverter.csGinger/Ginger/UserControlsLib/UCListView/UcListViewItem.xaml.csGinger/Ginger/UserControlsLib/ucGridView/ucGrid.xaml.cs
📚 Learning: 2025-11-28T05:32:43.529Z
Learnt from: CR
Repo: Ginger-Automation/Ginger PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T05:32:43.529Z
Learning: Applies to **/{GingerCore,GingerCoreNET,GingerCoreCommon}/Actions/Act*.cs : On action failure, set `act.Error` and `act.Status = eRunStatus.Failed`
Applied to files:
Ginger/GingerCoreCommon/Actions/Act.csGinger/GingerCoreNET/ActionsLib/ActValidation.csGinger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.cs
📚 Learning: 2024-07-26T22:04:12.930Z
Learnt from: AmanPrasad43
Repo: Ginger-Automation/Ginger PR: 0
File: :0-0
Timestamp: 2024-07-26T22:04:12.930Z
Learning: The `WorkSpace.Instance.Solution.GetSolutionTargetApplications()` method in the Ginger project is designed to always return a value, making additional error handling unnecessary according to the project's contributors.
Applied to files:
Ginger/Ginger/SolutionWindows/TargetApplicationsPage.xaml.cs
📚 Learning: 2025-11-28T05:32:43.529Z
Learnt from: CR
Repo: Ginger-Automation/Ginger PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T05:32:43.529Z
Learning: Applies to **/Drivers/*Driver.cs : Name drivers following the pattern: `{Platform}Driver` (e.g., `SeleniumDriver`, `JavaDriver`)
Applied to files:
Ginger/GingerCoreCommon/RunLib/Agent.csGinger/Ginger/Drivers/DriversConfigsEditPages/AppiumDriverEditPage.xaml.csGinger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs
📚 Learning: 2025-03-28T13:45:33.635Z
Learnt from: GokulBothe99
Repo: Ginger-Automation/Ginger PR: 4149
File: Ginger/GingerCoreNET/RunLib/DynamicExecutionLib/AgentConfigOperations.cs:136-142
Timestamp: 2025-03-28T13:45:33.635Z
Learning: In AgentConfigOperations.UpdateExistingAgentDetails method, the design is intentionally set to only update existing driver parameters and not add new ones that don't exist in the configuration.
Applied to files:
Ginger/GingerCoreCommon/RunLib/Agent.csGinger/GingerCoreNET/RunLib/AgentOperations.cs
📚 Learning: 2025-11-28T05:32:43.529Z
Learnt from: CR
Repo: Ginger-Automation/Ginger PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T05:32:43.529Z
Learning: Applies to **/*.csproj : Target framework should be .NET 8.0 with Windows 10.0.17763.0 compatibility
Applied to files:
Ginger/Ginger/Ginger.csproj
📚 Learning: 2025-07-09T13:47:31.577Z
Learnt from: GokulBothe99
Repo: Ginger-Automation/Ginger PR: 4245
File: Ginger/Ginger/Actions/ActionEditPages/ValidationDBPage.xaml.cs:164-170
Timestamp: 2025-07-09T13:47:31.577Z
Learning: In ValidationDBPage.xaml.cs, the RadioButtonsSection_IsVisibleChanged event handler that immediately collapses the UI element when it becomes visible is intentional and working as expected, as confirmed by the user.
Applied to files:
Ginger/Ginger/Drivers/DriversConfigsEditPages/AppiumDriverEditPage.xaml.cs
📚 Learning: 2025-07-09T13:46:53.701Z
Learnt from: GokulBothe99
Repo: Ginger-Automation/Ginger PR: 4245
File: Ginger/Ginger/Actions/ActionEditPages/ValidationDBPage.xaml.cs:142-143
Timestamp: 2025-07-09T13:46:53.701Z
Learning: In ValidationDBPage.xaml.cs, the new constructor overload that accepts SelectedContentArgs and ActDBValidation intentionally removes the SelectionChanged event handlers for AppNameComboBox and DBNameComboBox without restoring them. This is expected behavior since this constructor sets up the combo boxes with fixed values and makes them non-editable, so the normal selection changed behavior is not needed.
Applied to files:
Ginger/Ginger/Drivers/DriversConfigsEditPages/AppiumDriverEditPage.xaml.cs
📚 Learning: 2025-03-20T11:10:30.816Z
Learnt from: GokulBothe99
Repo: Ginger-Automation/Ginger PR: 4137
File: Ginger/Ginger/SourceControl/SourceControlProjectsPage.xaml.cs:21-21
Timestamp: 2025-03-20T11:10:30.816Z
Learning: The `Amdocs.Ginger.Common.SourceControlLib` namespace is required in files that reference the `GingerSolution` class for source control operations in the Ginger automation framework.
Applied to files:
Ginger/GingerCoreCommon/Repository/PlatformsLib/ApplicationPlatform.csGinger/Ginger/UserControlsLib/UCListView/UcListViewItem.xaml.csGinger/Ginger/UserControlsLib/ucGridView/ucGrid.xaml.cs
📚 Learning: 2025-08-26T07:40:08.345Z
Learnt from: AmanPrasad43
Repo: Ginger-Automation/Ginger PR: 4285
File: Ginger/Ginger/Actions/ActionEditPages/ActSecurityTestingEditPage.xaml:23-23
Timestamp: 2025-08-26T07:40:08.345Z
Learning: In the Ginger codebase ActSecurityTestingEditPage.xaml file, the large bottom margin of 530 on the "Acceptable Alerts" label (Margin="10,10,0,530") is intentional design choice, not an error.
Applied to files:
Ginger/Ginger/Agents/ApplicationAgentsMapPage.xamlGinger/Ginger/Drivers/DriversWindows/ADBDriverWindow/DeviceViewPage.xaml.cs
📚 Learning: 2025-07-16T14:42:32.229Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 4254
File: Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs:70-73
Timestamp: 2025-07-16T14:42:32.229Z
Learning: In Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs, prashelke prefers to keep the current `!WorkSpace.Instance.BetaFeatures.ShowPOMForAI` logic for binding and showing AI POM controls, rather than changing it to positive logic.
Applied to files:
Ginger/Ginger/Agents/ApplicationAgentsMapPage.xamlGinger/Ginger/SolutionWindows/AddApplicationPage.xaml.cs
📚 Learning: 2025-06-16T10:37:13.073Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 4232
File: Ginger/GingerCoreNET/ActionsLib/UI/VisualTesting/VRTAnalyzer.cs:19-22
Timestamp: 2025-06-16T10:37:13.073Z
Learning: In Ginger codebase, both `using amdocs.ginger.GingerCoreNET;` and `using Amdocs.Ginger.CoreNET;` are valid and serve different purposes. The first (lowercase) contains the WorkSpace class and related workspace functionality, while the second (proper case) contains drivers like GenericAppiumDriver and other core functionality. Both may be required in files that use types from both namespaces.
Applied to files:
Ginger/Ginger/UserControlsLib/UCListView/UcListViewItem.xaml.csGinger/Ginger/UserControlsLib/ucGridView/ucGrid.xaml.cs
📚 Learning: 2025-08-25T08:48:11.915Z
Learnt from: AmanPrasad43
Repo: Ginger-Automation/Ginger PR: 4281
File: Ginger/GingerCore/GingerCore.csproj:538-538
Timestamp: 2025-08-25T08:48:11.915Z
Learning: In the Ginger codebase, the OWASPZAPDotNetAPI package reference in GingerCore.csproj is required for architectural reasons, even when direct usage is not immediately visible in the codebase. This was confirmed by AmanPrasad43 during the security testing feature implementation.
Applied to files:
Ginger/Ginger/UserControlsLib/UCListView/UcListViewItem.xaml.cs
📚 Learning: 2025-06-16T09:52:40.873Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 4232
File: Ginger/GingerCoreNET/ActionsLib/UI/VisualTesting/VRTAnalyzer.cs:19-22
Timestamp: 2025-06-16T09:52:40.873Z
Learning: The `GenericAppiumDriver` class is located in the `Amdocs.Ginger.CoreNET` namespace, not in a namespace that matches its file path structure. File paths don't always correspond to namespace declarations in this codebase.
Applied to files:
Ginger/Ginger/UserControlsLib/UCListView/UcListViewItem.xaml.csGinger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs
📚 Learning: 2025-09-04T09:55:20.543Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:5789-5796
Timestamp: 2025-09-04T09:55:20.543Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (around line 5786), maintainer prashelke prefers to keep the helper method `IsSvgElement(eElementType)` as-is (no inlining or renaming), even though it currently checks only for `eElementType.Svg`. Future reviews should not suggest removing or renaming this helper.
Applied to files:
Ginger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.csGinger/Ginger/Drivers/DriversWindows/ADBDriverWindow/DeviceViewPage.xaml.csGinger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs
📚 Learning: 2025-06-16T10:13:01.346Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 4232
File: Ginger/GingerCoreNET/ActionsLib/UI/VisualTesting/VRTAnalyzer.cs:219-226
Timestamp: 2025-06-16T10:13:01.346Z
Learning: In VRTAnalyzer.cs for the GenericAppiumDriver class, the mAct.IsFullPageScreenshot flag is not applicable and should not be used to control screenshot behavior. GenericAppiumDriver always uses CaptureFullPageCroppedScreenshot() method regardless of the IsFullPageScreenshot flag setting.
Applied to files:
Ginger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.csGinger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs
📚 Learning: 2025-09-04T11:19:44.719Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:10651-10658
Timestamp: 2025-09-04T11:19:44.719Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs, maintainer prefers not to keep the helper IsSvgElementByWebElement(IWebElement); rely on the existing enum-based IsSvgElement(eElementType) helper instead. Future reviews should not reintroduce IsSvgElementByWebElement.
Applied to files:
Ginger/Ginger/Drivers/DriversWindows/ADBDriverWindow/DeviceViewPage.xaml.csGinger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs
📚 Learning: 2025-09-04T16:40:09.511Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:6019-6061
Timestamp: 2025-09-04T16:40:09.511Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (AddSvgSpecificLocators, around lines ~6000–6065) for PR Ginger-Automation/Ginger#4294, maintainer prashelke indicated that adjusting XPath to remove outer quotes around EscapeXPathString(...) is not required. Do not suggest changing the quoting around EscapeXPathString for SVG-specific locators in future reviews unless maintainers request it.
Applied to files:
Ginger/Ginger/Drivers/DriversWindows/ADBDriverWindow/DeviceViewPage.xaml.cs
📚 Learning: 2025-09-04T15:42:00.330Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:8334-8350
Timestamp: 2025-09-04T15:42:00.330Z
Learning: In PR Ginger-Automation/Ginger#4294, SeleniumDriver.cs: maintainer requested to keep the WebElement-based helper IsSvgElementByWebElement(IWebElement) and its current usages for now, deferring the replacement with GetElementTypeEnum(..) + IsSvgElement(eElementType) to a later PR.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs
📚 Learning: 2025-08-25T09:27:30.249Z
Learnt from: AmanPrasad43
Repo: Ginger-Automation/Ginger PR: 4281
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:1433-1441
Timestamp: 2025-08-25T09:27:30.249Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs, SetProxy(dynamic options) should not force options.AcceptInsecureCertificates = false. Only set AcceptInsecureCertificates = true when UseZAP is enabled to avoid breaking setups that permit self-signed/dev certificates.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs
📚 Learning: 2024-07-26T22:04:12.930Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 3429
File: Ginger/Ginger/AutomatePageLib/AddActionMenu/WindowExplorer/Common/WindowExplorerPage.xaml.cs:1152-1153
Timestamp: 2024-07-26T22:04:12.930Z
Learning: User feedback for exceptions in the screenshot functionality is handled using the `Reporter.ToLog` method within a `catch` block in the `ShowScreenShot` method of `WindowExplorerPage.xaml.cs`.
Applied to files:
Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs
🧬 Code graph analysis (15)
Ginger/Ginger/Agents/AgentEditPage.xaml.cs (4)
Ginger/Ginger/Environments/AppsListPage.xaml.cs (1)
ePlatformType(338-357)Ginger/Ginger/SolutionWindows/AddSolutionPage.xaml.cs (1)
ePlatformType(161-178)Ginger/GingerCoreCommon/RunLib/Agent.cs (1)
ePlatformType(360-380)Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs (1)
ePlatformType(4734-4737)
Ginger/Ginger/SolutionWindows/TreeViewItems/EnvironmentsTreeItems/EnvironmentApplicationList.xaml.cs (1)
Ginger/GingerCoreCommon/Repository/PlatformsLib/ApplicationPlatform.cs (1)
ApplicationPlatform(25-241)
Ginger/GingerCoreCommon/Run/ApplicationAgent.cs (4)
Ginger/Ginger/Environments/AppsListPage.xaml.cs (1)
ePlatformType(338-357)Ginger/Ginger/SolutionWindows/AddSolutionPage.xaml.cs (1)
ePlatformType(161-178)Ginger/GingerCoreCommon/RunLib/Agent.cs (1)
ePlatformType(360-380)Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs (1)
ePlatformType(4734-4737)
Ginger/Ginger/Environments/AppsListPage.xaml.cs (1)
Ginger/GingerCoreCommon/EnvironmentLib/EnvApplication.cs (1)
EnvApplication(29-293)
Ginger/Ginger/BusinessFlowWindows/EditBusinessFlowAppsPage.xaml.cs (1)
Ginger/GingerCoreCommon/Repository/PlatformsLib/ApplicationPlatform.cs (1)
ApplicationPlatform(25-241)
Ginger/Ginger/SolutionWindows/AddSolutionPage.xaml.cs (3)
Ginger/Ginger/Environments/AppsListPage.xaml.cs (1)
ePlatformType(338-357)Ginger/GingerCoreCommon/RunLib/Agent.cs (1)
ePlatformType(360-380)Ginger/GingerCoreCommon/WorkSpaceLib/Solution.cs (2)
ePlatformType(387-402)ePlatformType(409-417)
Ginger/Ginger/UserControlsLib/ucGridView/ComboEnumItemValueConverter.cs (1)
Ginger/Ginger/UserControlsLib/UCListView/UcListViewItem.xaml.cs (2)
Convert(100-121)Convert(1033-1043)
Ginger/Ginger/SolutionWindows/TargetApplicationsPage.xaml.cs (1)
Ginger/GingerCoreCommon/Repository/PlatformsLib/ApplicationPlatform.cs (1)
ApplicationPlatform(25-241)
Ginger/GingerCoreCommon/EnvironmentLib/EnvApplication.cs (2)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs (1)
OnPropertyChanged(277-280)Ginger/Ginger/UserControlsLib/UCListView/UcListViewItem.xaml.cs (1)
OnPropertyChanged(88-95)
Ginger/GingerCoreCommon/Repository/PlatformsLib/ApplicationPlatform.cs (3)
Ginger/GingerCoreCommon/Actions/Act.cs (1)
ToString(574-591)Ginger/Ginger/Environments/AppsListPage.xaml.cs (1)
ePlatformType(338-357)Ginger/GingerCoreCommon/RunLib/Agent.cs (1)
ePlatformType(360-380)
Ginger/Ginger/UserControlsLib/UCListView/UcListViewItem.xaml.cs (2)
Ginger/Ginger/Actions/ActionsListViewHelper.cs (2)
Convert(1071-1081)Convert(1091-1101)Ginger/Ginger/UserControlsLib/ImageMakerLib/ImageMakerControl.xaml.cs (2)
ImageMakerControl(34-1385)ImageMakerControl(149-152)
Ginger/Ginger/Drivers/DriversWindows/ADBDriverWindow/DeviceViewPage.xaml.cs (4)
Ginger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.cs (1)
System(1647-1661)Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs (1)
Point(4383-4490)Ginger/Ginger/GeneralLib/General.cs (1)
BitmapSource(464-480)Ginger/GingerCoreNET/Resources/JavaScripts/GingerLiveSpy.js (2)
X(123-123)Y(124-124)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs (2)
Ginger/GingerCoreCommon/RunLib/Agent.cs (1)
ePlatformType(360-380)Ginger/GingerCoreNET/Run/Platforms/PlatformsInfo/MobilePlatform.cs (1)
ePlatformType(32-35)
Ginger/Ginger/UserControlsLib/ImageMakerLib/ImageMakerControl.xaml.cs (2)
Ginger/GingerCoreCommon/Repository/PlatformsLib/ApplicationPlatform.cs (1)
eImageType(205-224)Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (1)
Size(12585-12588)
Ginger/Ginger/UserControlsLib/ucGridView/ucGrid.xaml.cs (1)
Ginger/Ginger/UserControlsLib/ucGridView/ComboEnumItemValueConverter.cs (3)
ComboEnumItemValueConverter(16-80)ComboEnumItemValueConverter(20-23)Convert(26-47)
| // 4) If driver is running, apply the config change at runtime | ||
| if (mAgent?.AgentOperations != null) | ||
| { | ||
| if (mAgent.AgentOperations is GingerCore.AgentOperations ops && ops.Driver != null) | ||
| { | ||
| System.Threading.Tasks.Task.Run(() => | ||
| { | ||
| try | ||
| { | ||
| ops.SetDriverConfiguration(); | ||
| ops.WaitForAgentToBeReady(); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| Reporter.ToLog(eLogLevel.ERROR, "Failed to apply agent driver configuration after platform change", ex); | ||
| } | ||
| }); | ||
| } | ||
| } |
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.
🧹 Nitpick | 🔵 Trivial
Runtime driver reconfiguration runs on background thread with silent failure.
The async driver reconfiguration at lines 596-607 runs on a background thread which is good for UI responsiveness. However, failures are only logged without user feedback. Consider showing a user notification on failure since this is a configuration change initiated by user action.
Also, WaitForAgentToBeReady() may block indefinitely if the agent fails to start - verify this has appropriate timeout handling.
| private void Page_Loaded(object sender, RoutedEventArgs e) | ||
| { | ||
| if ((xAndroidRdBtn.IsChecked == null || xAndroidRdBtn.IsChecked == false) && (xIOSRdBtn.IsChecked == null || xIOSRdBtn.IsChecked == false)) | ||
| if ((xAndroidRdBtn.IsChecked == null || xAndroidRdBtn.IsChecked == false) && | ||
| (xIOSRdBtn.IsChecked == null || xIOSRdBtn.IsChecked == false) && | ||
| (xAndroidTvRdBtn.IsChecked == null || xAndroidTvRdBtn.IsChecked == false)) | ||
| { | ||
| //TODO: binding lost from some reason- need to find out why | ||
| // binding may have been lost - rebind radio buttons | ||
| BindRadioButtons(); | ||
| } | ||
|
|
||
| // Ensure config DriverConfigParam instances are available | ||
| if (mDevicePlatformType == null) | ||
| { | ||
| mDevicePlatformType = mAgent.GetOrCreateParam(nameof(GenericAppiumDriver.DevicePlatformType)); | ||
| // re-bind UI to param if needed | ||
| BindingHandler.ObjFieldBinding(xAndroidRdBtn, RadioButton.IsCheckedProperty, mDevicePlatformType, nameof(DriverConfigParam.Value), bindingConvertor: new RadioBtnEnumConfigConverter(), converterParameter: nameof(eDevicePlatformType.Android)); | ||
| BindingHandler.ObjFieldBinding(xAndroidTvRdBtn, RadioButton.IsCheckedProperty, mDevicePlatformType, nameof(DriverConfigParam.Value), bindingConvertor: new RadioBtnEnumConfigConverter(), converterParameter: nameof(eDevicePlatformType.AndroidTv)); | ||
| BindingHandler.ObjFieldBinding(xIOSRdBtn, RadioButton.IsCheckedProperty, mDevicePlatformType, nameof(DriverConfigParam.Value), bindingConvertor: new RadioBtnEnumConfigConverter(), converterParameter: nameof(eDevicePlatformType.iOS)); | ||
| } | ||
|
|
||
| if (mDeviceSource == null) | ||
| { | ||
| mDeviceSource = mAgent.GetOrCreateParam(nameof(GenericAppiumDriver.DeviceSource)); | ||
| BindingHandler.ObjFieldBinding(xLocalAppiumRdBtn, RadioButton.IsCheckedProperty, mDeviceSource, nameof(DriverConfigParam.Value), bindingConvertor: new RadioBtnEnumConfigConverter(), converterParameter: nameof(eDeviceSource.LocalAppium)); | ||
| BindingHandler.ObjFieldBinding(xUFTRdBtn, RadioButton.IsCheckedProperty, mDeviceSource, nameof(DriverConfigParam.Value), bindingConvertor: new RadioBtnEnumConfigConverter(), converterParameter: nameof(eDeviceSource.MicroFoucsUFTMLab)); | ||
| BindingHandler.ObjFieldBinding(xKobitonRdBtn, RadioButton.IsCheckedProperty, mDeviceSource, nameof(DriverConfigParam.Value), bindingConvertor: new RadioBtnEnumConfigConverter(), converterParameter: nameof(eDeviceSource.Kobiton)); | ||
| } | ||
|
|
||
| if (mAppType == null) | ||
| { | ||
| mAppType = mAgent.GetOrCreateParam(nameof(GenericAppiumDriver.AppType)); | ||
| BindingHandler.ObjFieldBinding(xNativeHybRdBtn, RadioButton.IsCheckedProperty, mAppType, nameof(DriverConfigParam.Value), bindingConvertor: new RadioBtnEnumConfigConverter(), converterParameter: nameof(eAppType.NativeHybride)); | ||
| BindingHandler.ObjFieldBinding(xWebRdBtn, RadioButton.IsCheckedProperty, mAppType, nameof(DriverConfigParam.Value), bindingConvertor: new RadioBtnEnumConfigConverter(), converterParameter: nameof(eAppType.Web)); | ||
| } | ||
|
|
||
| // Default values (keep Mobile as default platform) | ||
| if (string.IsNullOrEmpty(mDevicePlatformType.Value)) | ||
| { | ||
| mDevicePlatformType.Value = nameof(eDevicePlatformType.Android); | ||
| mAgent.OnPropertyChanged(nameof(GenericAppiumDriver.DevicePlatformType)); | ||
| } | ||
|
|
||
| // Default device source if not set | ||
| if (string.IsNullOrEmpty(mDeviceSource.Value)) | ||
| { | ||
| mDeviceSource.Value = nameof(eDeviceSource.LocalAppium); | ||
| mAgent.OnPropertyChanged(nameof(GenericAppiumDriver.DeviceSource)); | ||
| } | ||
|
|
||
| // Default app type if not set | ||
| if (string.IsNullOrEmpty(mAppType.Value)) | ||
| { | ||
| mAppType.Value = nameof(eAppType.NativeHybride); | ||
| mAgent.OnPropertyChanged(nameof(GenericAppiumDriver.AppType)); | ||
| } | ||
|
|
||
| // Ensure radio buttons reflect the DriverConfigParam values and then apply visibility rules | ||
| try | ||
| { | ||
| // Platform radios | ||
| if (mDevicePlatformType.Value == nameof(eDevicePlatformType.AndroidTv)) | ||
| { | ||
| xTvRdBtn.IsChecked = true; | ||
| xAndroidTvRdBtn.IsChecked = true; | ||
| } | ||
| else if (mDevicePlatformType.Value == nameof(eDevicePlatformType.iOS)) | ||
| { | ||
| xMobileRdBtn.IsChecked = true; | ||
| xIOSRdBtn.IsChecked = true; | ||
| } | ||
| else | ||
| { | ||
| // default / Android | ||
| xMobileRdBtn.IsChecked = true; | ||
| xAndroidRdBtn.IsChecked = true; | ||
| } | ||
|
|
||
| // Device source radios | ||
| if (mDeviceSource.Value == nameof(eDeviceSource.MicroFoucsUFTMLab)) | ||
| { | ||
| xUFTRdBtn.IsChecked = true; | ||
| } | ||
| else if (mDeviceSource.Value == nameof(eDeviceSource.Kobiton)) | ||
| { | ||
| xKobitonRdBtn.IsChecked = true; | ||
| } | ||
| else | ||
| { | ||
| xLocalAppiumRdBtn.IsChecked = true; | ||
| } | ||
|
|
||
| // App type radios | ||
| if (mAppType.Value == nameof(eAppType.Web)) | ||
| { | ||
| xWebRdBtn.IsChecked = true; | ||
| } | ||
| else | ||
| { | ||
| xNativeHybRdBtn.IsChecked = true; | ||
| } | ||
| } | ||
| catch | ||
| { | ||
| // ignore individual control issues during load | ||
| } | ||
|
|
||
| // Apply visibility + capabilities according to current platform selection | ||
| PlatformSelectionChanged(null, null); | ||
| } |
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.
🧹 Nitpick | 🔵 Trivial
Consider refactoring Page_Loaded for maintainability.
This method handles multiple responsibilities: binding restoration, default value initialization, radio button state synchronization, and visibility rules. Consider extracting logical sections into smaller helper methods like RestoreBindingsIfNeeded(), ApplyDefaultValues(), and SyncRadioButtonState() to improve readability.
🤖 Prompt for AI Agents
Ginger/Ginger/Drivers/DriversConfigsEditPages/AppiumDriverEditPage.xaml.cs lines
654-762: The Page_Loaded method is doing too many things (restoring bindings,
initializing defaults, syncing radio button states, applying visibility) —
extract those responsibilities into three private helper methods
RestoreBindingsIfNeeded(), ApplyDefaultValues(), and SyncRadioButtonState() and
replace the inlined blocks: move the initial radio-button null-check and
BindRadioButtons() call into RestoreBindingsIfNeeded(); move the
GetOrCreateParam + binding setup and default Value assignment +
OnPropertyChanged calls into ApplyDefaultValues(); move the try/catch block that
sets IsChecked on the various radio buttons into SyncRadioButtonState(); finally
call these helpers from Page_Loaded in the same order and keep the final
PlatformSelectionChanged call, preserving existing behavior and exception
handling (e.g., keep the try/catch around SyncRadioButtonState if needed).
| } | ||
| catch | ||
| { | ||
| // ignore individual control issues during load | ||
| } |
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.
Empty catch block swallows exceptions without logging.
Silent exception handling makes debugging difficult. Per coding guidelines, use Reporter.ToLog to log errors, even if the intent is to continue gracefully.
🔎 Proposed fix
catch
{
- // ignore individual control issues during load
+ // Log but continue - individual control issues during load shouldn't block page
+ Reporter.ToLog(eLogLevel.DEBUG, "Minor control initialization issue during AppiumDriverEditPage load");
}📝 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.
| } | |
| catch | |
| { | |
| // ignore individual control issues during load | |
| } | |
| } | |
| catch | |
| { | |
| // Log but continue - individual control issues during load shouldn't block page | |
| Reporter.ToLog(eLogLevel.DEBUG, "Minor control initialization issue during AppiumDriverEditPage load"); | |
| } |
🤖 Prompt for AI Agents
In Ginger/Ginger/Drivers/DriversConfigsEditPages/AppiumDriverEditPage.xaml.cs
around lines 754-758 there is an empty catch that swallows exceptions; change it
to catch (Exception ex) and call Reporter.ToLog with a clear message and the
exception (e.g., Reporter.ToLog(eLogLevel.ERROR, "Error loading Appium driver
control", ex)) so the exception is recorded while preserving the current
graceful continue behavior.
| public (int x, int y) GetMousePositionOnDevice() | ||
| { | ||
| try | ||
| { | ||
| var img = this.FindName("xDeviceImage") as Image; | ||
| if (img == null) throw new InvalidOperationException("Device image control 'xDeviceImage' not found in MobileDriverWindow."); | ||
|
|
||
| Point relative = Mouse.GetPosition(img); | ||
|
|
||
| if (img.Source is BitmapSource bmp && img.ActualWidth > 0 && img.ActualHeight > 0) | ||
| { | ||
| double sx = bmp.PixelWidth / img.ActualWidth; | ||
| double sy = bmp.PixelHeight / img.ActualHeight; | ||
| int dx = Math.Max(0, (int)Math.Round(relative.X * sx)); | ||
| int dy = Math.Max(0, (int)Math.Round(relative.Y * sy)); | ||
| return (dx, dy); | ||
| } | ||
|
|
||
| return ((int)relative.X, (int)relative.Y); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| throw new InvalidOperationException("GetMousePositionOnDevice failed: " + ex.Message, ex); | ||
| } | ||
| } |
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.
🧹 Nitpick | 🔵 Trivial
Consider returning an error indicator instead of throwing exceptions.
This public method throws InvalidOperationException on error, which may be unexpected for callers treating it as a simple getter. For a helper method that other components might call frequently, consider returning a nullable tuple (int x, int y)? or a sentinel value like (-1, -1) to indicate failure, allowing callers to gracefully handle the error without try-catch blocks.
🔎 Proposed alternative signature
-public (int x, int y) GetMousePositionOnDevice()
+public (int x, int y)? GetMousePositionOnDevice()
{
try
{
var img = this.FindName("xDeviceImage") as Image;
- if (img == null) throw new InvalidOperationException("Device image control 'xDeviceImage' not found in MobileDriverWindow.");
+ if (img == null) return null;
Point relative = Mouse.GetPosition(img);
if (img.Source is BitmapSource bmp && img.ActualWidth > 0 && img.ActualHeight > 0)
{
double sx = bmp.PixelWidth / img.ActualWidth;
double sy = bmp.PixelHeight / img.ActualHeight;
int dx = Math.Max(0, (int)Math.Round(relative.X * sx));
int dy = Math.Max(0, (int)Math.Round(relative.Y * sy));
return (dx, dy);
}
return ((int)relative.X, (int)relative.Y);
}
catch (Exception ex)
{
- throw new InvalidOperationException("GetMousePositionOnDevice failed: " + ex.Message, ex);
+ Reporter.ToLog(eLogLevel.ERROR, "GetMousePositionOnDevice failed: " + ex.Message, ex);
+ return null;
}
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
Ginger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.cs around lines
69-93: the current public GetMousePositionOnDevice method throws
InvalidOperationException on failures; change its contract to return a failure
indicator instead (either change signature to nullable tuple (int x,int y)? and
return null on any error/invalid state, or keep signature and return sentinel
(-1,-1)), remove/rework the try/catch so exceptions are swallowed/handled and
mapped to the chosen failure value, ensure you handle null img and invalid bmp
cases by returning the failure indicator rather than throwing, and update all
callers to check for null or (-1,-1) accordingly.
| public void HighlightElementOnDevice(Amdocs.Ginger.Common.UIElement.ElementInfo element) | ||
| { | ||
| try | ||
| { | ||
| var canvas = this.FindName("xHighlightCanvas") as System.Windows.Controls.Canvas; | ||
| if (canvas == null) return; | ||
|
|
||
| canvas.Dispatcher.Invoke(() => | ||
| { | ||
| canvas.Children.Clear(); | ||
| var rect = new System.Windows.Shapes.Rectangle | ||
| { | ||
| Stroke = Brushes.Red, | ||
| StrokeThickness = 3, | ||
| Fill = new SolidColorBrush(Color.FromArgb(40, 255, 0, 0)) | ||
| }; | ||
|
|
||
| // element.X/Y/Width/Height are device pixels; map to image control size | ||
| var img = this.FindName("xDeviceImage") as Image; | ||
| if (img != null && img.Source is BitmapSource bmp && img.ActualWidth > 0 && img.ActualHeight > 0) | ||
| { | ||
| double sx = img.ActualWidth / bmp.PixelWidth; | ||
| double sy = img.ActualHeight / bmp.PixelHeight; | ||
| Canvas.SetLeft(rect, element.X * sx); | ||
| Canvas.SetTop(rect, element.Y * sy); | ||
| rect.Width = Math.Max(1, element.Width * sx); | ||
| rect.Height = Math.Max(1, element.Height * sy); | ||
| canvas.Children.Add(rect); | ||
| canvas.Visibility = Visibility.Visible; | ||
| } | ||
| }); | ||
| } | ||
| catch { /* non-fatal */ } | ||
| } |
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.
Add error logging to exception handler.
The method silently swallows all exceptions with a comment indicating they are "non-fatal." However, per coding guidelines, errors should be logged using Reporter.ToLog(eLogLevel.ERROR, message). This would help diagnose issues like missing UI controls, dispatcher failures, or incorrect element coordinates during development and debugging.
🔎 Proposed fix
catch { /* non-fatal */ }
+ catch (Exception ex)
+ {
+ Reporter.ToLog(eLogLevel.ERROR, "Failed to highlight element on device", ex);
+ }
}📝 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.
| public void HighlightElementOnDevice(Amdocs.Ginger.Common.UIElement.ElementInfo element) | |
| { | |
| try | |
| { | |
| var canvas = this.FindName("xHighlightCanvas") as System.Windows.Controls.Canvas; | |
| if (canvas == null) return; | |
| canvas.Dispatcher.Invoke(() => | |
| { | |
| canvas.Children.Clear(); | |
| var rect = new System.Windows.Shapes.Rectangle | |
| { | |
| Stroke = Brushes.Red, | |
| StrokeThickness = 3, | |
| Fill = new SolidColorBrush(Color.FromArgb(40, 255, 0, 0)) | |
| }; | |
| // element.X/Y/Width/Height are device pixels; map to image control size | |
| var img = this.FindName("xDeviceImage") as Image; | |
| if (img != null && img.Source is BitmapSource bmp && img.ActualWidth > 0 && img.ActualHeight > 0) | |
| { | |
| double sx = img.ActualWidth / bmp.PixelWidth; | |
| double sy = img.ActualHeight / bmp.PixelHeight; | |
| Canvas.SetLeft(rect, element.X * sx); | |
| Canvas.SetTop(rect, element.Y * sy); | |
| rect.Width = Math.Max(1, element.Width * sx); | |
| rect.Height = Math.Max(1, element.Height * sy); | |
| canvas.Children.Add(rect); | |
| canvas.Visibility = Visibility.Visible; | |
| } | |
| }); | |
| } | |
| catch { /* non-fatal */ } | |
| } | |
| public void HighlightElementOnDevice(Amdocs.Ginger.Common.UIElement.ElementInfo element) | |
| { | |
| try | |
| { | |
| var canvas = this.FindName("xHighlightCanvas") as System.Windows.Controls.Canvas; | |
| if (canvas == null) return; | |
| canvas.Dispatcher.Invoke(() => | |
| { | |
| canvas.Children.Clear(); | |
| var rect = new System.Windows.Shapes.Rectangle | |
| { | |
| Stroke = Brushes.Red, | |
| StrokeThickness = 3, | |
| Fill = new SolidColorBrush(Color.FromArgb(40, 255, 0, 0)) | |
| }; | |
| // element.X/Y/Width/Height are device pixels; map to image control size | |
| var img = this.FindName("xDeviceImage") as Image; | |
| if (img != null && img.Source is BitmapSource bmp && img.ActualWidth > 0 && img.ActualHeight > 0) | |
| { | |
| double sx = img.ActualWidth / bmp.PixelWidth; | |
| double sy = img.ActualHeight / bmp.PixelHeight; | |
| Canvas.SetLeft(rect, element.X * sx); | |
| Canvas.SetTop(rect, element.Y * sy); | |
| rect.Width = Math.Max(1, element.Width * sx); | |
| rect.Height = Math.Max(1, element.Height * sy); | |
| canvas.Children.Add(rect); | |
| canvas.Visibility = Visibility.Visible; | |
| } | |
| }); | |
| } | |
| catch (Exception ex) | |
| { | |
| Reporter.ToLog(eLogLevel.ERROR, "Failed to highlight element on device", ex); | |
| } | |
| } |
🤖 Prompt for AI Agents
Ginger/Ginger/Drivers/DriversWindows/MobileDriverWindow.xaml.cs around lines
95-128: the catch block currently swallows all exceptions silently; change it to
catch (Exception e) and call Reporter.ToLog(eLogLevel.ERROR,
$"HighlightElementOnDevice failed: {e.Message} | Stack: {e.StackTrace}") (or
similar descriptive message) so the error is logged, then preserve the non-fatal
behavior by not rethrowing; ensure you include necessary using/namespace for
Reporter and eLogLevel if not already present.
| // Add constructor to initialize Description when creating new instance | ||
| public ActMobileDevice() | ||
| { | ||
| // If Description not set, default it to the canonical action-level description | ||
| if (string.IsNullOrEmpty(this.Description) && !string.IsNullOrEmpty(this.ActionDescription)) | ||
| { | ||
| this.Description = this.ActionDescription; | ||
| } | ||
| } |
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.
🧹 Nitpick | 🔵 Trivial
Consolidate duplicated Description initialization logic.
The same logic for setting Description from ActionDescription is repeated in three places: constructor (lines 40-43), DoNewActionSetup (lines 84-87), and PostDeserialization (lines 161-164). Consider extracting to a private helper method.
🔎 Proposed fix
+ private void EnsureDescriptionSet()
+ {
+ if (string.IsNullOrEmpty(this.Description) && !string.IsNullOrEmpty(this.ActionDescription))
+ {
+ this.Description = this.ActionDescription;
+ }
+ }
+
public ActMobileDevice()
{
- // If Description not set, default it to the canonical action-level description
- if (string.IsNullOrEmpty(this.Description) && !string.IsNullOrEmpty(this.ActionDescription))
- {
- this.Description = this.ActionDescription;
- }
+ EnsureDescriptionSet();
}Then update DoNewActionSetup and PostDeserialization to call EnsureDescriptionSet() instead of duplicating the logic.
Also applies to: 81-88, 156-170
🤖 Prompt for AI Agents
In Ginger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.cs around lines
36-44 (and duplicated at 81-88 and 156-170), the logic that sets Description
from ActionDescription is duplicated; extract that logic into a private helper
method EnsureDescriptionSet() that checks if Description is null or empty and
ActionDescription is not null or empty and then sets Description =
ActionDescription, make the method private void and place it with other helper
methods in the class, and replace the duplicated blocks in the constructor,
DoNewActionSetup, and PostDeserialization with a call to EnsureDescriptionSet().
| public static List<eMobileDeviceAction> GetSupportedActionsByPlatform(ePlatformType platform) | ||
| { | ||
| // Define the limited set of actions for Android TV. Adjust as required. | ||
| if (platform == ePlatformType.AndroidTV) | ||
| { | ||
| return new List<eMobileDeviceAction> | ||
| { | ||
| // typical remote/TV related operations - change to match product requirements | ||
| eMobileDeviceAction.PressBackButton, | ||
| eMobileDeviceAction.PressHomeButton, | ||
| eMobileDeviceAction.PressMenuButton, | ||
| eMobileDeviceAction.PressVolumeUp, | ||
| eMobileDeviceAction.PressVolumeDown, | ||
| eMobileDeviceAction.PressKey, | ||
| eMobileDeviceAction.OpenApp, | ||
| eMobileDeviceAction.CloseApp, | ||
| eMobileDeviceAction.IsAppInstalled, | ||
| eMobileDeviceAction.GetAppPackage, | ||
| eMobileDeviceAction.GetDeviceBattery, | ||
| eMobileDeviceAction.GetDeviceGeneralInfo, | ||
| eMobileDeviceAction.GetCurrentActivityDetails, | ||
| eMobileDeviceAction.GetDeviceOSType | ||
| }; | ||
| } | ||
|
|
||
| // default: full mobile actions list | ||
| return Enum.GetValues(typeof(eMobileDeviceAction)).Cast<eMobileDeviceAction>().ToList(); | ||
| } |
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.
🧹 Nitpick | 🔵 Trivial
LGTM! Consider improving formatting consistency.
The GetSupportedActionsByPlatform method correctly filters actions for Android TV. However, the return list has inconsistent indentation (lines 53-70 use 4 spaces instead of 8).
🔎 Proposed fix for formatting
public static List<eMobileDeviceAction> GetSupportedActionsByPlatform(ePlatformType platform)
{
// Define the limited set of actions for Android TV. Adjust as required.
if (platform == ePlatformType.AndroidTV)
{
- return new List<eMobileDeviceAction>
- {
- // typical remote/TV related operations - change to match product requirements
- eMobileDeviceAction.PressBackButton,
- eMobileDeviceAction.PressHomeButton,
- ...
- };
+ return new List<eMobileDeviceAction>
+ {
+ // typical remote/TV related operations - change to match product requirements
+ eMobileDeviceAction.PressBackButton,
+ eMobileDeviceAction.PressHomeButton,
+ ...
+ };
}📝 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.
| public static List<eMobileDeviceAction> GetSupportedActionsByPlatform(ePlatformType platform) | |
| { | |
| // Define the limited set of actions for Android TV. Adjust as required. | |
| if (platform == ePlatformType.AndroidTV) | |
| { | |
| return new List<eMobileDeviceAction> | |
| { | |
| // typical remote/TV related operations - change to match product requirements | |
| eMobileDeviceAction.PressBackButton, | |
| eMobileDeviceAction.PressHomeButton, | |
| eMobileDeviceAction.PressMenuButton, | |
| eMobileDeviceAction.PressVolumeUp, | |
| eMobileDeviceAction.PressVolumeDown, | |
| eMobileDeviceAction.PressKey, | |
| eMobileDeviceAction.OpenApp, | |
| eMobileDeviceAction.CloseApp, | |
| eMobileDeviceAction.IsAppInstalled, | |
| eMobileDeviceAction.GetAppPackage, | |
| eMobileDeviceAction.GetDeviceBattery, | |
| eMobileDeviceAction.GetDeviceGeneralInfo, | |
| eMobileDeviceAction.GetCurrentActivityDetails, | |
| eMobileDeviceAction.GetDeviceOSType | |
| }; | |
| } | |
| // default: full mobile actions list | |
| return Enum.GetValues(typeof(eMobileDeviceAction)).Cast<eMobileDeviceAction>().ToList(); | |
| } | |
| public static List<eMobileDeviceAction> GetSupportedActionsByPlatform(ePlatformType platform) | |
| { | |
| // Define the limited set of actions for Android TV. Adjust as required. | |
| if (platform == ePlatformType.AndroidTV) | |
| { | |
| return new List<eMobileDeviceAction> | |
| { | |
| // typical remote/TV related operations - change to match product requirements | |
| eMobileDeviceAction.PressBackButton, | |
| eMobileDeviceAction.PressHomeButton, | |
| eMobileDeviceAction.PressMenuButton, | |
| eMobileDeviceAction.PressVolumeUp, | |
| eMobileDeviceAction.PressVolumeDown, | |
| eMobileDeviceAction.PressKey, | |
| eMobileDeviceAction.OpenApp, | |
| eMobileDeviceAction.CloseApp, | |
| eMobileDeviceAction.IsAppInstalled, | |
| eMobileDeviceAction.GetAppPackage, | |
| eMobileDeviceAction.GetDeviceBattery, | |
| eMobileDeviceAction.GetDeviceGeneralInfo, | |
| eMobileDeviceAction.GetCurrentActivityDetails, | |
| eMobileDeviceAction.GetDeviceOSType | |
| }; | |
| } | |
| // default: full mobile actions list | |
| return Enum.GetValues(typeof(eMobileDeviceAction)).Cast<eMobileDeviceAction>().ToList(); | |
| } |
🤖 Prompt for AI Agents
In Ginger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.cs around lines
48-75, the returned list for AndroidTV has inconsistent indentation for the enum
entries (lines ~53-70 are indented 4 spaces instead of matching the surrounding
block). Reformat those enum entries and the closing brace to match the method's
existing indentation style (align the list items with the other code inside the
if-block—use the same indent level as the return statement and surrounding
braces), so all lines in the returned List initialization are consistently
indented.
| catch | ||
| { | ||
| // be defensive - do not throw during deserialization | ||
| } |
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.
🧹 Nitpick | 🔵 Trivial
Avoid bare catch clause; consider logging the exception.
The bare catch silently swallows exceptions. As per coding guidelines, use Reporter.ToLog(eLogLevel.ERROR, message) for logging errors.
🔎 Proposed fix
- catch
+ catch (Exception ex)
{
- // be defensive - do not throw during deserialization
+ // be defensive - do not throw during deserialization
+ Reporter.ToLog(eLogLevel.DEBUG, "Error during ActMobileDevice PostDeserialization", ex);
}📝 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.
| catch | |
| { | |
| // be defensive - do not throw during deserialization | |
| } | |
| catch (Exception ex) | |
| { | |
| // be defensive - do not throw during deserialization | |
| Reporter.ToLog(eLogLevel.DEBUG, "Error during ActMobileDevice PostDeserialization", ex); | |
| } |
🤖 Prompt for AI Agents
In Ginger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.cs around lines
166-169, replace the bare catch with a catch(Exception e) that logs the error
using Reporter.ToLog(eLogLevel.ERROR, "...") so exceptions are not silently
swallowed; include a concise contextual message (e.g., "Failed deserializing
mobile device config" plus exception.Message and optionally e.StackTrace) and
keep the method defensive (do not rethrow).
| foreach (DriverConfigParam DCP in Agent.DriverConfiguration) | ||
| { | ||
| if (DCP == null) continue; | ||
|
|
||
| string value = null; | ||
| ObservableList<DriverConfigParam> multiValues = null; | ||
|
|
||
| // process VE for this param | ||
| if (DCP.MultiValues != null && DCP.MultiValues.Count > 0) | ||
| { | ||
| multiValues = new ObservableList<DriverConfigParam>(); | ||
| foreach (DriverConfigParam subValue in DCP.MultiValues) | ||
| { | ||
| ve.Value = DCP.Value; | ||
| value = ve.ValueCalculated; | ||
| ve.Value = subValue.Value; | ||
| multiValues.Add(new DriverConfigParam() { Parameter = subValue.Parameter, Value = ve.ValueCalculated }); | ||
| } | ||
| } | ||
| else | ||
| { | ||
| ve.Value = DCP.Value; | ||
| value = ve.ValueCalculated; | ||
| } |
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.
🛠️ Refactor suggestion | 🟠 Major
Redundant Value Expression evaluation in runtime application phase.
The first loop (lines 388-405) already evaluates VE and stores results in DCP.Value. The second loop (lines 423-436) evaluates VE again on these already-evaluated values. This is redundant since DCP.Value now contains the calculated value, not the original expression.
You can simplify by directly using DCP.Value and DCP.MultiValues without re-evaluation:
🔎 Proposed simplification
// Runtime driver exists - apply evaluated values to the driver via reflection
foreach (DriverConfigParam DCP in Agent.DriverConfiguration)
{
if (DCP == null) continue;
string value = null;
ObservableList<DriverConfigParam> multiValues = null;
- // process VE for this param
if (DCP.MultiValues != null && DCP.MultiValues.Count > 0)
{
- multiValues = new ObservableList<DriverConfigParam>();
- foreach (DriverConfigParam subValue in DCP.MultiValues)
- {
- ve.Value = subValue.Value;
- multiValues.Add(new DriverConfigParam() { Parameter = subValue.Parameter, Value = ve.ValueCalculated });
- }
+ // MultiValues already have evaluated values from first loop
+ multiValues = DCP.MultiValues;
}
else
{
- ve.Value = DCP.Value;
- value = ve.ValueCalculated;
+ // DCP.Value already contains evaluated value from first loop
+ value = DCP.Value;
}📝 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.
| foreach (DriverConfigParam DCP in Agent.DriverConfiguration) | |
| { | |
| if (DCP == null) continue; | |
| string value = null; | |
| ObservableList<DriverConfigParam> multiValues = null; | |
| // process VE for this param | |
| if (DCP.MultiValues != null && DCP.MultiValues.Count > 0) | |
| { | |
| multiValues = new ObservableList<DriverConfigParam>(); | |
| foreach (DriverConfigParam subValue in DCP.MultiValues) | |
| { | |
| ve.Value = DCP.Value; | |
| value = ve.ValueCalculated; | |
| ve.Value = subValue.Value; | |
| multiValues.Add(new DriverConfigParam() { Parameter = subValue.Parameter, Value = ve.ValueCalculated }); | |
| } | |
| } | |
| else | |
| { | |
| ve.Value = DCP.Value; | |
| value = ve.ValueCalculated; | |
| } | |
| foreach (DriverConfigParam DCP in Agent.DriverConfiguration) | |
| { | |
| if (DCP == null) continue; | |
| string value = null; | |
| ObservableList<DriverConfigParam> multiValues = null; | |
| if (DCP.MultiValues != null && DCP.MultiValues.Count > 0) | |
| { | |
| // MultiValues already have evaluated values from first loop | |
| multiValues = DCP.MultiValues; | |
| } | |
| else | |
| { | |
| // DCP.Value already contains evaluated value from first loop | |
| value = DCP.Value; | |
| } |
🤖 Prompt for AI Agents
In Ginger/GingerCoreNET/RunLib/AgentOperations.cs around lines 415-436 the code
re-evaluates the Value Expression (VE) a second time for DriverConfigParam
entries that were already evaluated earlier; remove the redundant VE usage and
use the already-calculated values instead. For multi-values, iterate
DCP.MultiValues and add new DriverConfigParam items using subValue.Value (which
already contains the evaluated value) rather than setting ve.Value and using
ve.ValueCalculated; for single-value params set value = DCP.Value directly (no
ve assignment). Keep existing null/Count checks and preserve creation of the new
DriverConfigParam objects but source their Value from the pre-evaluated fields.
| case "Int32": | ||
| if (!string.IsNullOrEmpty(value)) | ||
| { | ||
| driverProp.SetValue(Driver, int.Parse(value)); | ||
| } | ||
| break; |
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.
Add error handling for Int32 parsing to match Boolean pattern.
int.Parse can throw FormatException or OverflowException if the value is invalid. The Boolean case (lines 476-483) has resilient error handling with a fallback, but Int32 does not.
🔎 Proposed fix
case "Int32":
- if (!string.IsNullOrEmpty(value))
+ if (!string.IsNullOrEmpty(value))
{
- driverProp.SetValue(Driver, int.Parse(value));
+ if (int.TryParse(value, out int intValue))
+ {
+ driverProp.SetValue(Driver, intValue);
+ }
+ else
+ {
+ Reporter.ToLog(eLogLevel.WARN, $"Failed to parse Int32 value '{value}' for driver configuration '{DCP.Parameter}'");
+ }
}
break;📝 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.
| case "Int32": | |
| if (!string.IsNullOrEmpty(value)) | |
| { | |
| driverProp.SetValue(Driver, int.Parse(value)); | |
| } | |
| break; | |
| case "Int32": | |
| if (!string.IsNullOrEmpty(value)) | |
| { | |
| if (int.TryParse(value, out int intValue)) | |
| { | |
| driverProp.SetValue(Driver, intValue); | |
| } | |
| else | |
| { | |
| Reporter.ToLog(eLogLevel.WARN, $"Failed to parse Int32 value '{value}' for driver configuration '{DCP.Parameter}'"); | |
| } | |
| } | |
| break; |
🤖 Prompt for AI Agents
In Ginger/GingerCoreNET/RunLib/AgentOperations.cs around lines 485-490, the
Int32 case uses int.Parse without error handling which can throw FormatException
or OverflowException; change this to robust parsing like the Boolean branch:
attempt to parse using int.TryParse (or wrap int.Parse in a try/catch for
FormatException/OverflowException) and on failure set a safe default (e.g., 0)
via driverProp.SetValue(Driver, 0), ensuring the code does not throw for invalid
or out-of-range values.
Enhanced platform support by introducing Android TV as a new platform type (
ePlatformType.AndroidTV) alongside Mobile and iOS. Updated platform descriptions to include "Mobile/TV" for better clarity and added new image types for visual representation.UI components now display friendly platform descriptions and improved tooltips. Added ComboBox support for enum descriptions in grids.
Extended the Appium driver to support Android TV with specific capabilities, configurations. Improved error handling and fallback mechanisms for driver initialization.
Updated
ActMobileDeviceto include Android TV-specific actions and defaulted action descriptions to "Mobile/Tv Device Action."Enhanced
DeviceViewPageto handle Android TV resolutions and scaling. Added dynamic screen size calculations with fallbacks for Android TV.Updated agent configurations to include Android TV as a selectable driver type and set default configurations for Android TV agents.
Refactored code for better maintainability, improved exception handling, and consolidated driver configuration logic. Added new image resources for Android TV.
Improved
GenericAppiumDriverto handle Android TV-specific scenarios, including focused element detection and screenshot scaling.Description
Type of Change
Checklist
[IsSerializedForLocalRepository]where neededReporter.ToLog()patternSummary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.