-
Notifications
You must be signed in to change notification settings - Fork 20
Conversation
#path to page objects | ||
page.package = ru.sbtqa.tag.mobilefactory.entries | ||
#default wait timeout | ||
page.load.timeout = 60000 |
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.
milliseconds
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.
What I need to fix?
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.
comment that it is in ms
@@ -0,0 +1,35 @@ | |||
#mobile or web | |||
driver.environment = mobile | |||
driver.url = http://127.0.0.1:4723/wd/hub |
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.
In which cases is this important?
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.
This is actual both the mobile and the web.
private static final String APPIUM_DEVICE_PLATFORM = Props.get("appium.device.platform"); | ||
private static final String APPIUM_APP_PACKAGE = Props.get("appium.app.package"); | ||
private static final String APPIUM_APP_ACTIVITY = Props.get("appium.app.activity"); | ||
private static final String VIDEO_ENABLE = Props.get("video.enable", "false"); |
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.
ENABLED
} catch (MalformedURLException e) { | ||
throw new FactoryRuntimeException("Failed to connect to appium on url " + PageFactory.getInitialUrl(), e); | ||
throw new FactoryRuntimeException("Appium url is malformed", 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.
("Could not parse appium.url "" + APPIUM_URL+""", e)
} catch (MalformedURLException e) { | ||
log.error("Can not parse remote url. Check webdriver.remote.host property"); | ||
log.error("Can not parse remote url. Check webdriver.url property"); |
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.
wrap in quotes all of properties in text
screenshot.strategy = driver | ||
|
||
#tasks to kill before test | ||
tasks.to.kill = iexplorer,chrome |
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.
Is it OS independent?
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.
It is not. check this issue #26
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 in commet please whait is for windows only
@@ -81,7 +81,7 @@ private static void createDriver() throws UnsupportedBrowserException { | |||
if (Props.get("webdriver.remote.host").isEmpty()) { | |||
|
|||
//Local proxy available on local webdriver instances only | |||
if (!Props.get("proxy.enable").isEmpty()) { | |||
if (!Props.get(".webdriver.proxy.enable").isEmpty()) { |
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.
Dot as a prefix? Are you sure?
@@ -142,7 +141,7 @@ public Page changeUrlByTitle(String packageName, String title) throws PageInitia | |||
} else { | |||
try { | |||
URL currentUrl = new URL(PageFactory.getWebDriver().getCurrentUrl()); | |||
String finalUrl = currentUrl.getProtocol() + "://" + currentUrl.getAuthority() + getUrlPrefix() + ((PageEntry) annotation).url(); | |||
String finalUrl = currentUrl.getProtocol() + "://" + currentUrl.getAuthority() + ((PageEntry) annotation).url(); |
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.
Can you add "://" to somewhere or add it as constant?
@@ -25,6 +24,7 @@ | |||
private Page currentPage; | |||
|
|||
private final String pagesPackage; | |||
private static final String COLON_TWO_SLASHES = "://"; |
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.
SCHEME_SEPARATOR on SCHEME_DELIMITER
@@ -142,7 +142,7 @@ public Page changeUrlByTitle(String packageName, String title) throws PageInitia | |||
} else { | |||
try { | |||
URL currentUrl = new URL(PageFactory.getWebDriver().getCurrentUrl()); |
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.
Current url is fetched as a URL instance, why not build it with the same class, instead of string concatenations and COLON_TWO_SLASHES variables? Or use https://docs.oracle.com/javaee/7/api/javax/ws/rs/core/UriBuilder.html or https://github.com/mikaelhg/urlbuilder for URL buildings
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.
And no, this is not a legacy. When one touches legacy shit and changes "//" string to a const, he gets involved. So one must make a good code from legacy shit, not a less shitty legacy shit.
private static BrowserMobProxy proxy; | ||
private static final int WEBDRIVER_CREATE_ATTEMPTS = Integer.parseInt(Props.get("webdriver.create.attempts", "3")); | ||
private static final String WEBDRIVER_PATH = "src/test/resources/webdrivers/"; |
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.
Move it to config and by default get it from system PATH
#IN CASE OF WEB | ||
#parameters for web driver create | ||
webdriver.browser.name = Chrome | ||
webdriver.drivers.path = src/test/resources/webdrivers/ |
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 comment what is optional and if in case it not pointed webdriver will searched via system PATH
fixes #24