-
Notifications
You must be signed in to change notification settings - Fork 4
Added restart driver capability #14
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: develop
Are you sure you want to change the base?
Added restart driver capability #14
Conversation
@Bean | ||
public WebController getWebController(@Autowired AnaxDriver anaxDriver, @Value("${anax.defaultWaitSeconds:5}") Integer defaultWaitSeconds) throws Exception { | ||
return new WebDriverWebController(anaxDriver.getWebDriver(), defaultWaitSeconds); | ||
return new WebDriverWebController(anaxDriver.getWebDriver(), anaxDriver, defaultWaitSeconds); |
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.
@dimitrianos pls simplify the signature to new WebDriverWebController(anaxDriver, defaultWaitSeconds)
since you can receive the WebDriver from that object - no need to have both of them.
*/ | ||
String getPageSource() ; | ||
|
||
boolean restart(); |
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.
please add documentation - why restart exists and what it does, how
public AppiumDriverWebController(WebDriver webDriver, long defaultWaitSeconds) { | ||
public AppiumDriverWebController(WebDriver webDriver, AnaxDriver anaxDriver, long defaultWaitSeconds) { | ||
this.driver = webDriver; | ||
this.anaxDriver = anaxDriver; |
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.
replace this.driver = webDriver
with this.driver = anaxDriver.getWebDriver()
- much cleaner after you simplify the signature (see first comment)
return null; | ||
} | ||
|
||
@Override |
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.
Instead of adding restart()
as implementation to all drivers, consider adding a default
on the interface (that returns false) and only implement in WebController
s that can possibly do it.
Restart browser is sometimes essential to clean up the environment