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

Mobile enhancements and changes #4005

Merged
merged 4 commits into from
Nov 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@

<StackPanel x:Name="xSwipePnl" HorizontalAlignment="Left" Orientation="Vertical" DockPanel.Dock="Top" Margin="0,10,0,0" Visibility="Collapsed">
<StackPanel x:Name="xSwipeScalePnl" Orientation="Horizontal">
<Label Style="{StaticResource $LabelStyle}" Content="Swipe scale (0.1-1):" Margin="0 0 0 0"/>
<Label Style="{StaticResource $LabelStyle}" Content="Swipe scale (0.1-2):" Margin="0 0 0 0"/>
<Activities:UCValueExpression x:Name="xSwipeScaleTxtBox" HorizontalAlignment="Left" VerticalAlignment="Center" Width="160" Margin="57 0 0 0" />
</StackPanel>
<StackPanel Orientation="Horizontal" Margin="0 10 0 0">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
</Hyperlink>):
</TextBlock>
<Actions:UCValueExpression x:Name="xServerURLTextBox" HorizontalAlignment="Stretch" VerticalAlignment="Top" DockPanel.Dock="Top"/>
<CheckBox x:Name="xLoadDeviceWindow" Style="{StaticResource $CeckBoxStyle}" Content="Load device window" Margin="0,5,0,0" IsChecked="True" Checked="xLoadDeviceWindow_Checked" Unchecked="xLoadDeviceWindow_Checked"/>
<CheckBox x:Name="xLoadDeviceWindow" Style="{StaticResource $CeckBoxStyle}" Content="Load device window" Margin="0,15,0,0" IsChecked="True" Checked="xLoadDeviceWindow_Checked" Unchecked="xLoadDeviceWindow_Checked"/>
<StackPanel x:Name="xAutoRefreshModePnl" Orientation="Horizontal" Margin="0,5,0,0">
<Label Content="Auto refresh mode:" Style="{StaticResource $LabelStyle}" VerticalAlignment="Center" />
<RadioButton x:Name="xLiveRdBtn" Content="Live" Style="{StaticResource $InputRadioButtonStyle}" GroupName="AutoRefreshMode" VerticalAlignment="Center" IsChecked="True" />
Expand All @@ -53,11 +53,11 @@
<!--<CheckBox x:Name="xAllowUnahutCertChkBox" Content="Allow unauthorized certificates" Style="{StaticResource $CeckBoxStyle}" Margin="0,5,0,0"/>-->
<StackPanel Orientation="Horizontal" Margin="0,5,0,0">
<CheckBox x:Name="xUseProxyChkBox" Content="Use proxy" Style="{StaticResource $CeckBoxStyle}" VerticalAlignment="Center" Checked="xUseProxyChkBox_Checked" Unchecked="xUseProxyChkBox_Unchecked"/>
<Actions:UCValueExpression x:Name="xProxyTextBox" Width="180" Margin="10,0,0,0" IsEnabled="False"/>
<Actions:UCValueExpression x:Name="xProxyTextBox" Width="190" Margin="10,0,0,0" IsEnabled="False"/>
</StackPanel>
<StackPanel Orientation="Horizontal" Margin="0,10,0,10">
<StackPanel Orientation="Horizontal" Margin="0,15,0,10">
<Label Content="Load timeout (seconds):" Style="{StaticResource $LabelStyle}"/>
<Actions:UCValueExpression x:Name="xLoadTimeoutTxtbox" Width="120" Margin="10,0,0,0"/>
<Actions:UCValueExpression x:Name="xLoadTimeoutTxtbox" Width="122" Margin="17,0,0,0"/>
</StackPanel>
<!--<StackPanel Orientation="Horizontal" Margin="0,2,0,10">
<Label Content="Screen click correction X:" Style="{StaticResource $LabelStyle}"/>
Expand All @@ -67,6 +67,14 @@
<Label Content="Screen click correction Y:" Style="{StaticResource $LabelStyle}"/>
<Actions:UCValueExpression x:Name="xScreenScaleFactorCorrectionYTextBox" Width="120" Margin="5,0,0,0"/>
</StackPanel>-->
<StackPanel Orientation="Horizontal" Margin="0,10,0,10">
<Label Content="Screenshot height (pixels):" Style="{StaticResource $LabelStyle}"/>
<Actions:UCValueExpression x:Name="xScreenshotHeightTextBox" Width="120" Margin="5,0,0,0"/>
</StackPanel>
<StackPanel Orientation="Horizontal" Margin="0,10,0,10">
<Label Content="Screenshot width (pixels):" Style="{StaticResource $LabelStyle}"/>
<Actions:UCValueExpression x:Name="xScreenshotWidthTextBox" Width="120" Margin="7,0,0,0"/>
</StackPanel>
</StackPanel>
</Expander.Content>
</Expander>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ limitations under the License.
using Amdocs.Ginger.CoreNET;
using Amdocs.Ginger.CoreNET.Drivers.CoreDrivers.Mobile;
using Ginger.UserControls;
using Ginger.ValidationRules;
using GingerCore;
using GingerCore.GeneralLib;
using System;
Expand Down Expand Up @@ -76,6 +77,14 @@ private void BindConfigurationsFields()
//xScreenScaleFactorCorrectionYTextBox.Init(null, screenScaleFactorCorrectionY, nameof(DriverConfigParam.Value));
//BindingHandler.ObjFieldBinding(xScreenScaleFactorCorrectionYTextBox, TextBox.ToolTipProperty, mAgent.GetOrCreateParam(nameof(GenericAppiumDriver.ScreenScaleFactorCorrectionY)), nameof(DriverConfigParam.Description));

DriverConfigParam screenshotHeight = mAgent.GetOrCreateParam(nameof(GenericAppiumDriver.ScreenshotHeight));
xScreenshotHeightTextBox.Init(null, screenshotHeight, nameof(DriverConfigParam.Value));
BindingHandler.ObjFieldBinding(xScreenshotHeightTextBox, TextBox.ToolTipProperty, mAgent.GetOrCreateParam(nameof(GenericAppiumDriver.ScreenshotHeight)), nameof(DriverConfigParam.Description));

DriverConfigParam screenshotWidth = mAgent.GetOrCreateParam(nameof(GenericAppiumDriver.ScreenshotWidth));
xScreenshotWidthTextBox.Init(null, screenshotWidth, nameof(DriverConfigParam.Value));
BindingHandler.ObjFieldBinding(xScreenshotWidthTextBox, TextBox.ToolTipProperty, mAgent.GetOrCreateParam(nameof(GenericAppiumDriver.ScreenshotWidth)), nameof(DriverConfigParam.Description));
Comment on lines +80 to +86
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add validation for screenshot dimensions.

While the implementation follows the existing pattern, consider adding validation to ensure the screenshot dimensions are positive numbers greater than zero.

Add numeric validation rules:

 DriverConfigParam screenshotHeight = mAgent.GetOrCreateParam(nameof(GenericAppiumDriver.ScreenshotHeight));
 xScreenshotHeightTextBox.Init(null, screenshotHeight, nameof(DriverConfigParam.Value));
+xScreenshotHeightTextBox.ValueTextBox.AddValidationRule(new ValidatePositiveNumber("Height must be a positive number"));

 DriverConfigParam screenshotWidth = mAgent.GetOrCreateParam(nameof(GenericAppiumDriver.ScreenshotWidth));
 xScreenshotWidthTextBox.Init(null, screenshotWidth, nameof(DriverConfigParam.Value));
+xScreenshotWidthTextBox.ValueTextBox.AddValidationRule(new ValidatePositiveNumber("Width must be a positive number"));

Committable suggestion skipped: line range outside the PR's diff.


xLoadTimeoutTxtbox.Init(null, mAgent.GetOrCreateParam(nameof(GenericAppiumDriver.DriverLoadWaitingTime)), nameof(DriverConfigParam.Value));
BindingHandler.ObjFieldBinding(xLoadTimeoutTxtbox, TextBox.ToolTipProperty, mAgent.GetOrCreateParam(nameof(GenericAppiumDriver.DriverLoadWaitingTime)), nameof(DriverConfigParam.Description));

Expand All @@ -86,6 +95,8 @@ private void BindConfigurationsFields()

BindRadioButtons();

ApplyValidationRules();

if (mAppiumCapabilities.MultiValues == null || mAppiumCapabilities.MultiValues.Count == 0)
{
mAppiumCapabilities.MultiValues = [];
Expand All @@ -94,6 +105,25 @@ private void BindConfigurationsFields()
SetCapabilitiesGridView();
}

private void ApplyValidationRules()
{
xServerURLTextBox.ValueTextBox.AddValidationRule(new ValidateEmptyValue("URL cannot be empty"));
xLoadTimeoutTxtbox.ValueTextBox.AddValidationRule(new ValidateEmptyValue("Load Timeout cannot be empty"));
xScreenshotHeightTextBox.ValueTextBox.AddValidationRule(new ValidateEmptyValue("Height cannot be empty"));
xScreenshotWidthTextBox.ValueTextBox.AddValidationRule(new ValidateEmptyValue("Width cannot be empty"));

CallConfigPropertyChange();
}

private void CallConfigPropertyChange()
{
// need in order to trigger the validation's rules
mAgent.OnPropertyChanged(nameof(GenericAppiumDriver.AppiumServer));
mAgent.OnPropertyChanged(nameof(GenericAppiumDriver.LoadDeviceWindow));
mAgent.OnPropertyChanged(nameof(GenericAppiumDriver.ScreenshotHeight));
mAgent.OnPropertyChanged(nameof(GenericAppiumDriver.ScreenshotWidth));
}

private void BindRadioButtons()
{
mDeviceAutoScreenshotRefreshMode = mAgent.GetOrCreateParam(nameof(GenericAppiumDriver.DeviceAutoScreenshotRefreshMode), nameof(eAutoScreenshotRefreshMode.Live));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,16 @@ public string GetDriverWindowName(Agent.eDriverType driverSubType = Agent.eDrive
[UserConfiguredDescription("Screen Scale Factor Correction for Y coordinate, needed for fixing screen mouse click/point accuracy (decimal number)")]
public String ScreenScaleFactorCorrectionY { get; set; }

[UserConfigured]
[UserConfiguredDefault("Auto")]
[UserConfiguredDescription("Define the Height to set for the mobile device screenshot")]
public String ScreenshotHeight { get; set;}

[UserConfigured]
[UserConfiguredDefault("Auto")]
[UserConfiguredDescription("Define the Width to set for the mobile device screenshot")]
public String ScreenshotWidth { get; set; }

[UserConfigured]
[UserConfiguredEnumType(typeof(eDevicePlatformType))]
[UserConfiguredDefault("Android")]
Expand Down Expand Up @@ -1336,7 +1346,8 @@ private void ActScreenShotHandler(Act act)
{
try
{
act.AddScreenShot(GetScreenshotImageFromDriver(), "Device Screenshot");
Tuple<int?, int?> customSize = GetUserCustomeScreenshotSize();
act.AddScreenShot(GetScreenshotImageFromDriver(customSize.Item1, customSize.Item2), "Device Screenshot");
}
catch (Exception ex)
{
Expand All @@ -1345,6 +1356,58 @@ private void ActScreenShotHandler(Act act)
}
}

private Tuple<int?,int?> GetUserCustomeScreenshotSize()
{
int? customeWidth = null;
int? customeHeight = null;

//override with user configured sizes
if (ScreenshotWidth.ToLower().Trim() != "auto")
{
//convert from int
int userConfiguredWidth;
if (int.TryParse(ScreenshotWidth, out userConfiguredWidth))
{
//validate user configured width is at least 300 pixels, if not than use the calculated width and write Warn message
if (userConfiguredWidth < 200)
{
Reporter.ToUser(eUserMsgKey.StaticWarnMessage, "The user configured screenshot width is less than 200 pixels, using the calculated width '" + mWindowWidth.ToString() + "' instead.");
}
else
{
customeWidth = userConfiguredWidth;
}
}
else
{
Reporter.ToUser(eUserMsgKey.StaticWarnMessage, "The user configured screenshot width is not valid, using the calculated width.");
}
}
if (ScreenshotHeight.ToLower().Trim() != "auto")
{
//convert from int
int userConfiguredHeight;
if (int.TryParse(ScreenshotHeight, out userConfiguredHeight))
{
//validate user configured height is at least 300 pixels, if not than use the calculated height and write Warn message
if (userConfiguredHeight < 200)
{
Reporter.ToUser(eUserMsgKey.StaticWarnMessage, "The user configured screenshot height is less than 200 pixels, using the calculated height '" + mWindowHeight.ToString() + "' instead.");
}
else
{
customeHeight = userConfiguredHeight;
}
}
else
{
Reporter.ToUser(eUserMsgKey.StaticWarnMessage, "The user configured screenshot height is not valid, using the calculated height");
}
}

return new Tuple<int?, int?>(customeWidth, customeHeight);
}
Comment on lines +1359 to +1409
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Refactor screenshot dimension validation for better maintainability.

The current implementation has duplicate code and magic numbers. Consider these improvements:

  1. Extract the dimension validation to a method
  2. Define constants for magic numbers
  3. Use consistent string formatting
+        private const int MIN_SCREENSHOT_DIMENSION = 200;
+        private const string AUTO_VALUE = "auto";
+
+        private int? ValidateScreenshotDimension(string configuredValue, string dimensionName)
+        {
+            if (configuredValue.ToLower().Trim() == AUTO_VALUE)
+            {
+                return null;
+            }
+
+            if (!int.TryParse(configuredValue, out int userConfigured))
+            {
+                Reporter.ToUser(eUserMsgKey.StaticWarnMessage, 
+                    string.Format("The user configured screenshot {0} is not valid, using the calculated {0}", dimensionName));
+                return null;
+            }
+
+            if (userConfigured < MIN_SCREENSHOT_DIMENSION)
+            {
+                Reporter.ToUser(eUserMsgKey.StaticWarnMessage, 
+                    string.Format("The user configured screenshot {0} is less than {1} pixels, using the calculated {0}", 
+                    dimensionName, MIN_SCREENSHOT_DIMENSION));
+                return null;
+            }
+
+            return userConfigured;
+        }
+
         private Tuple<int?,int?> GetUserCustomeScreenshotSize()
         {
-            int? customeWidth = null;
-            int? customeHeight = null;
-
-            //override with user configured sizes
-            if (ScreenshotWidth.ToLower().Trim() != "auto")
-            {
-                //convert from int
-                int userConfiguredWidth;
-                if (int.TryParse(ScreenshotWidth, out userConfiguredWidth))
-                {
-                    //validate user configured width is at least 300 pixels, if not than use the calculated width and write Warn message
-                    if (userConfiguredWidth < 200)
-                    {
-                        Reporter.ToUser(eUserMsgKey.StaticWarnMessage, "The user configured screenshot width is less than 200 pixels, using the calculated width '" + mWindowWidth.ToString() + "' instead.");
-                    }
-                    else
-                    {
-                        customeWidth = userConfiguredWidth;
-                    }
-                }
-                else
-                {
-                    Reporter.ToUser(eUserMsgKey.StaticWarnMessage, "The user configured screenshot width is not valid, using the calculated width.");
-                }
-            }
-            if (ScreenshotHeight.ToLower().Trim() != "auto")
-            {
-                //convert from int
-                int userConfiguredHeight;
-                if (int.TryParse(ScreenshotHeight, out userConfiguredHeight))
-                {
-                    //validate user configured height is at least 300 pixels, if not than use the calculated height and write Warn message
-                    if (userConfiguredHeight < 200)
-                    {
-                        Reporter.ToUser(eUserMsgKey.StaticWarnMessage, "The user configured screenshot height is less than 200 pixels, using the calculated height '" + mWindowHeight.ToString() + "' instead.");
-                    }
-                    else
-                    {
-                        customeHeight = userConfiguredHeight;
-                    }
-                }
-                else
-                {
-                    Reporter.ToUser(eUserMsgKey.StaticWarnMessage, "The user configured screenshot height is not valid, using the calculated height");
-                }
-            }
-
-            return new Tuple<int?, int?>(customeWidth, customeHeight);
+            return new Tuple<int?, int?>(
+                ValidateScreenshotDimension(ScreenshotWidth, "width"),
+                ValidateScreenshotDimension(ScreenshotHeight, "height")
+            );
         }
📝 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 Tuple<int?,int?> GetUserCustomeScreenshotSize()
{
int? customeWidth = null;
int? customeHeight = null;
//override with user configured sizes
if (ScreenshotWidth.ToLower().Trim() != "auto")
{
//convert from int
int userConfiguredWidth;
if (int.TryParse(ScreenshotWidth, out userConfiguredWidth))
{
//validate user configured width is at least 300 pixels, if not than use the calculated width and write Warn message
if (userConfiguredWidth < 200)
{
Reporter.ToUser(eUserMsgKey.StaticWarnMessage, "The user configured screenshot width is less than 200 pixels, using the calculated width '" + mWindowWidth.ToString() + "' instead.");
}
else
{
customeWidth = userConfiguredWidth;
}
}
else
{
Reporter.ToUser(eUserMsgKey.StaticWarnMessage, "The user configured screenshot width is not valid, using the calculated width.");
}
}
if (ScreenshotHeight.ToLower().Trim() != "auto")
{
//convert from int
int userConfiguredHeight;
if (int.TryParse(ScreenshotHeight, out userConfiguredHeight))
{
//validate user configured height is at least 300 pixels, if not than use the calculated height and write Warn message
if (userConfiguredHeight < 200)
{
Reporter.ToUser(eUserMsgKey.StaticWarnMessage, "The user configured screenshot height is less than 200 pixels, using the calculated height '" + mWindowHeight.ToString() + "' instead.");
}
else
{
customeHeight = userConfiguredHeight;
}
}
else
{
Reporter.ToUser(eUserMsgKey.StaticWarnMessage, "The user configured screenshot height is not valid, using the calculated height");
}
}
return new Tuple<int?, int?>(customeWidth, customeHeight);
}
private const int MIN_SCREENSHOT_DIMENSION = 200;
private const string AUTO_VALUE = "auto";
private int? ValidateScreenshotDimension(string configuredValue, string dimensionName)
{
if (configuredValue.ToLower().Trim() == AUTO_VALUE)
{
return null;
}
if (!int.TryParse(configuredValue, out int userConfigured))
{
Reporter.ToUser(eUserMsgKey.StaticWarnMessage,
string.Format("The user configured screenshot {0} is not valid, using the calculated {0}", dimensionName));
return null;
}
if (userConfigured < MIN_SCREENSHOT_DIMENSION)
{
Reporter.ToUser(eUserMsgKey.StaticWarnMessage,
string.Format("The user configured screenshot {0} is less than {1} pixels, using the calculated {0}",
dimensionName, MIN_SCREENSHOT_DIMENSION));
return null;
}
return userConfigured;
}
private Tuple<int?,int?> GetUserCustomeScreenshotSize()
{
return new Tuple<int?, int?>(
ValidateScreenshotDimension(ScreenshotWidth, "width"),
ValidateScreenshotDimension(ScreenshotHeight, "height")
);
}


public ICollection<IWebElement> GetAllElements()
{
return (ICollection<IWebElement>)Driver.FindElements(By.XPath(".//*"));
Expand Down Expand Up @@ -1666,6 +1729,18 @@ public void SwipeScreen(eSwipeSide side, double swipeScale, TimeSpan swipeDurati
double startY;
double endX;
double endY;
//valide swipeScale is between 0.1 and 2 and if not set it to 1 and write warn messaage
if (swipeScale < 0.1 || swipeScale > 2)
{
Reporter.ToLog(eLogLevel.WARN, "Mobile action swipeScreen(): swipeScale: '" + swipeScale + "' is out of range (0.1-2), setting it to 1");
swipeScale = 1;
}
//validate swipeDuration is bigger than 0 and if not set it to 200 and write warn messaage
if (swipeDuration.TotalMilliseconds < 0)
{
Reporter.ToLog(eLogLevel.WARN, "Mobile action swipeScreen(): swipeDuration: '" + swipeDuration + "' is out of range, setting it to 200");
swipeDuration = TimeSpan.FromMilliseconds(200);
}
switch (side)
{
case eSwipeSide.Down: // center of footer
Expand Down Expand Up @@ -2793,27 +2868,43 @@ public Byte[] GetScreenshotImage()
return null;
}

private Byte[] GetScreenshotImageFromDriver()
private Byte[] GetScreenshotImageFromDriver(int? width = null, int ? height = null)
{
int screenshotWidth = 0;
int screenshotHeight = 0;

//Take screen shot
var screenshot = Driver.GetScreenshot();

//Update screen size for iOS as it changed per app
if (DevicePlatformType == eDevicePlatformType.iOS)
{
CalculateMobileDeviceScreenSizes();
}
screenshotWidth = mWindowWidth;
screenshotHeight = mWindowHeight;

//ovveride with user configured width/height if such
if (width != null)
{
screenshotWidth = width.Value;
}
if (height != null)
{
screenshotHeight = height.Value;
}

// Convert screenshot to Image for resizing
using (var stream = new MemoryStream(screenshot.AsByteArray))
using (var image = Image.FromStream(stream))
{
// Create a new bitmap with the native device size
using (var resizedImage = new Bitmap(mWindowWidth, mWindowHeight))
using (var resizedImage = new Bitmap(screenshotWidth, screenshotHeight))
{
// Draw the original image onto the new bitmap
using (var graphics = Graphics.FromImage(resizedImage))
{
graphics.DrawImage(image, 0, 0, mWindowWidth, mWindowHeight);
graphics.DrawImage(image, 0, 0, screenshotWidth, screenshotHeight);
}
// Convert the resized image to byte array
using (var ms = new MemoryStream())
Expand All @@ -2823,7 +2914,6 @@ private Byte[] GetScreenshotImageFromDriver()
}
}
}

}

public void PerformTap(long x, long y)
Expand Down Expand Up @@ -2930,7 +3020,16 @@ public Bitmap GetScreenShot(Tuple<int, int> setScreenSize = null, bool IsFullPag
try
{
// Get the screenshot as a byte array
byte[] screenshotBytes = GetScreenshotImageFromDriver();
Tuple<int?, int?> customSize = null;
if (setScreenSize != null)
{
customSize = new Tuple<int?, int?>(setScreenSize.Item1, setScreenSize.Item2);
}
else
{
customSize = GetUserCustomeScreenshotSize();
}
byte[] screenshotBytes = GetScreenshotImageFromDriver(customSize.Item1, customSize.Item2);

if (screenshotBytes == null || screenshotBytes.Length == 0)
{
Expand Down Expand Up @@ -3176,7 +3275,7 @@ private void CalculateMobileDeviceScreenSizes()
mWindowScaleFactor = 1;
}
mWindowWidth = (int)(windowSize.Width * mWindowScaleFactor);
mWindowHeight = (int)(windowSize.Height * mWindowScaleFactor);
mWindowHeight = (int)(windowSize.Height * mWindowScaleFactor);
}
catch (Exception ex)
{
Expand Down
Loading