-
Notifications
You must be signed in to change notification settings - Fork 193
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
Selenium 4 and Appium 2 Upgrade #360
Conversation
1. Updating to Appium Java client 8
...rc/com/testsigma/automator/actions/mobile/android/generic/ChangeScreenOrientationAction.java
Outdated
Show resolved
Hide resolved
server/src/main/java/com/testsigma/service/TestsigmaLabDriverSettingsService.java
Outdated
Show resolved
Hide resolved
this.mobileAutomationServerExecutablePath = new File(PathUtil.getInstance().getMobileAutomationServerPath(), "appium"); | ||
if (SystemUtils.IS_OS_WINDOWS) { | ||
this.mobileAutomationServerExecutablePath = new File(PathUtil.getInstance().getMobileAutomationServerPath(), "appium.exe"); | ||
} | ||
this.serverIpAddress = serverIP(); | ||
this.logFilePath = new File(PathUtil.getInstance().getLogsPath() + File.separator + "appium.log"); | ||
Integer serverPort = NetworkUtil.getFreePort(); | ||
this.serverURL = String.format("http://%s:%d/wd/hub", serverIpAddress, serverPort); |
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.
@vikramvr20 we need to have Appium server also need to upgraded. jfyi
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.
@vikramvr20 Please confirm and do necessary changes..
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.
@PratheepV - Yes that's already been done and discussed the approach with Rajesh and Vikram.
@@ -298,6 +310,12 @@ | |||
<artifactId>log4j-api</artifactId> | |||
<version>2.17.1</version> | |||
</dependency> | |||
<dependency> | |||
<groupId>org.springframework</groupId> |
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.
Why do we need this in automator?
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.
@@ -9,14 +9,20 @@ | |||
|
|||
package com.testsigma.automator.actions.mobile; | |||
|
|||
|
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.
Format this file, remove unwanted empty lines
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.
Addressed
@@ -29,7 +38,12 @@ public class SwipeAction extends MobileDriverAction { | |||
public void execute() throws Exception { | |||
PointOption startingPoint = PointOption.point(tapPoints[0].getX(), tapPoints[0].getY()); | |||
PointOption endingPoint = PointOption.point(tapPoints[1].getX(), tapPoints[1].getY()); | |||
new TouchAction<>(getDriver()).press(startingPoint) | |||
.waitAction(WaitOptions.waitOptions(Duration.ofSeconds(1))).moveTo(endingPoint).release().perform(); | |||
PointerInput FINGER = new PointerInput(TOUCH, "finger"); |
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.
Rename the variable FINGER, use camel case
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.
Addressed.
getDriver().switchTo().activeElement().sendKeys(Keys.RETURN); | ||
log.error("Hide keyboard by switching to active element and clicking enter"); | ||
} catch (Exception e) { | ||
log.error("Could not hide keyboard by switching to active element and clicking enter"); |
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.
log the exception as well. log.error("Could not hide keyboard.....",e);
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.
Addressed
try { | ||
getDriver().findElement(By.xpath("//*[contains(@name, '" + button + "')]")).click(); | ||
} catch (Exception e) { | ||
log.error(e, e); |
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.
never use log.error(e,e), u cannot search for exception globallky. add some mesningfiul message
log.error("message",e); also do not use e.getMessage() in place of message
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.
Addressed
@@ -145,4 +143,16 @@ public void setWebCapabilities(TestDevice testDevice, | |||
public Integrations getLabDetails() throws IntegrationNotFoundException { | |||
return this.integrationsService.findByApplication(Integration.TestsigmaLab); | |||
} | |||
|
|||
public String getExecutionName(TestPlanResult testPlanDetails) { |
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.
@vikramvr20 can we exclude runId so that we don't need to pollute methods with TestPlanResult as an argument and as its for debugging only I feel its not worth
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.
Addressed
@@ -1,6 +1,6 @@ | |||
server.port=${TESTSIGMA_SERVER_PORT:9090} | |||
server.url=${TESTSIGMA_SERVER_URL:https://local.testsigmaos.com} | |||
server.version=v2.9.1 | |||
server.version=v2.9.2 |
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.
@vikramvr20 can we have the version as 3.0.0 because its major release
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.
Addressed
Description
Checklist: