-
Notifications
You must be signed in to change notification settings - Fork 49
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 upgraded to 3.8.1, guava upgraded to 23.6-jre #209
Conversation
This pull request require update for AET cookbook (folder for Firefox log) |
@@ -35,6 +35,11 @@ public boolean apply(DBKey dbKey) { | |||
return companyMatches(dbKey) && projectMatches(dbKey); | |||
} | |||
|
|||
@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.
WE could use default implementation from the Predicate
interface providing we change the JAVA level to 1.8 in root pom XML file: dbecf17
What do you think?
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.
good point, fixed
@@ -129,23 +135,17 @@ private FirefoxProfile getFirefoxProfile() throws IOException { | |||
return firefoxProfile; | |||
} | |||
|
|||
private AetFirefoxDriver getFirefoxDriver(FirefoxProfile fp, DesiredCapabilities capabilities) | |||
private RemoteWebDriver getFirefoxDriver(DesiredCapabilities capabilities) |
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.
We are declaring RemoteWebDriver
, but are returning FirefoxWebDriver
.
Could we return WebDriver
interface here?
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.
ok
requestExecutorFactory | ||
.createInstance()); | ||
} catch (Exception e) { | ||
throw new WorkerException(e.getMessage(), e); | ||
} | ||
} | ||
|
||
private void setCommonCapabilities(DesiredCapabilities capabilities, FirefoxProfile fp) { |
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 we move this private method down after the two public methods that use it?
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.
moved
import org.openqa.selenium.firefox.FirefoxProfile; | ||
import org.openqa.selenium.remote.CapabilityType; | ||
import org.openqa.selenium.remote.DesiredCapabilities; | ||
import org.openqa.selenium.remote.RemoteWebDriver; |
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 looks like unused import.
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.
Indeed, removed.
@@ -67,7 +67,7 @@ | |||
<dependency> | |||
<groupId>org.seleniumhq.selenium</groupId> | |||
<artifactId>selenium-htmlunit-driver</artifactId> | |||
<version>${selenium.version}</version> | |||
<version>2.52.0</version> |
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.
I've seen that selenium htmlunit package was separated from the selenium itself some time ago. And I believe this version has connection with all ignored unit tests?
Is there any way to make those ignored unit tests working? If not, maybe we should just remove them?
Or maybe we should add ignore comment like:
@Ignore("Not working because of selenium-htmlunit-driver broken compatibility. Consider to remove in the future.")
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.
We are irrespectively working on fixing or even rewriting failing tests. Probably soon there will be additional pull request for junit changes.
Could you please update PR description and/or remove unnecessary sections? |
@@ -61,7 +61,7 @@ | |||
<dependency> | |||
<groupId>com.google.guava</groupId> | |||
<artifactId>guava</artifactId> | |||
<version>15.0</version> | |||
<version>23.6-jre</version> |
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.
Let's upgrade guava also in aet-features.xml
file.
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.
/osgi-dependencies/aet-features.xml is recently added to gitignore. Is this expected? That's why my change is not in this PR.
4f4b7cb
to
4e9df39
Compare
Description
I've upgraded Selenium version to 3.8.1 and additionally Guava to 23.6-jre.
Motivation and Context
This upgrade is required for further AET's migration to newest versions of Chrome (and potentially Firefox) browsers.
Checklist:
I hereby agree to the terms of the AET Contributor License Agreement.