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.

Sign up for GitHub

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

Jump to bottom

Add new WebDriver support for Edge - JAVA #7164

Merged
merged 4 commits into from
May 31, 2019
Merged

Add new WebDriver support for Edge - JAVA #7164

merged 4 commits into from
May 31, 2019

Conversation

loly89
Copy link
Contributor

@loly89 loly89 commented May 2, 2019 •
edited
Loading

Following coding pattern from FirefoxDriver to spin up MicrosoftWebDriver.exe or MSEdgeDriver.exe depending on System Property "webdriver.edge.edgehtml".

By default, MicrosoftWebDriver.exe will be picked and launched if the system property does not exist or set to "true". If it is "false", MSEdgeDriver.exe will be used (available for download under Microsoft Edge Insider: https://developer.microsoft.com/en-us/microsoft-edge/tools/webdriver/)

I will probably follow up with a Wiki page if necessary when the PR is completed.

This change is Reviewable

loly89 added 2 commits May 2, 2019 12:22
@loly89
Moving edgehtml to its own package. Add StandardSeleniumTests suite.
ebfef8f 
Moving edgehtml into its own package since the new Chromium Edge will
be used primarily. Adding StandardSeleniumTests suite for EdgeDriver
so that we can test Edge by running
'go //java/client/test/org/openqa/selenium/edge:edgehtml:run'.
@loly89
Integrate ChromiumEdge under EdgeDriver and add StandardSeleniumTest.
6e86b64 
Integrate ChromiumEdge under EdgeDriver and add StandardSeleneiumTest
suite for ChromiumEdge. We can run the test by calling
'go test_edge'. Also, all tests skipped by CHROME will also be
skipped by CHROMIUMEDGE.
shs96c
shs96c requested changes May 3, 2019
View reviewed changes
Copy link
Member

@shs96c shs96c left a comment

Choose a reason for hiding this comment

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

This looks a lot better. Thank you for the update.

I do worry that this diff clearly states that Edge is a Chrome derivative, and not a Chromium derivative, and I'm not sure that's the intent. I'd be fine extracting an abstract ChromiumDriver in a separate PR. If you'd prefer to land that after this, please LMK.

java/client/src/org/openqa/selenium/BUCK Show resolved Hide resolved
java/client/src/com/thoughtworks/selenium/BUCK Show resolved Hide resolved
java/client/src/org/openqa/selenium/chrome/ChromeDriver.java Outdated Show resolved Hide resolved
java/client/src/org/openqa/selenium/chrome/ChromeDriverCommandExecutor.java Outdated Show resolved Hide resolved
java/client/src/org/openqa/selenium/edge/ChromiumEdgeDriverService.java Outdated Show resolved Hide resolved
java/client/src/org/openqa/selenium/edge/edgehtml/EdgeHTMLDriverService.java Outdated Show resolved Hide resolved
java/client/src/org/openqa/selenium/edge/edgehtml/module-info.txt Show resolved Hide resolved
java/client/test/org/openqa/selenium/edge/EdgeOptionsFunctionalTest.java Outdated Show resolved Hide resolved
java/client/test/org/openqa/selenium/edge/EdgeOptionsFunctionalTest.java Show resolved Hide resolved
java/client/test/org/openqa/selenium/edge/EdgeOptionsFunctionalTest.java Outdated Show resolved Hide resolved
810 added a commit to 810/application that referenced this pull request May 8, 2019
@810
Use official browsername
a2f21b3 
See: SeleniumHQ/selenium#7164
@810 810 mentioned this pull request May 8, 2019
Closed
@diemol diemol mentioned this pull request May 8, 2019
Closed
@loly89
Copy link
Contributor Author

loly89 commented May 14, 2019

This looks a lot better. Thank you for the update.

I do worry that this diff clearly states that Edge is a Chrome derivative, and not a Chromium derivative, and I'm not sure that's the intent. I'd be fine extracting an abstract ChromiumDriver in a separate PR. If you'd prefer to land that after this, please LMK.

Thanks @shs96c. I'll be extracting 'ChromiumDriver' in this PR.

@loly89
Extract chromium package from chrome
b4b9ac4 
Extracting Chromium package from Chrome and have both Edge and Chrome
inherited from Chromium package. Also, address some PR feedbacks.
shs96c
shs96c approved these changes May 29, 2019
View reviewed changes
Copy link
Member

@shs96c shs96c left a comment

Choose a reason for hiding this comment

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

This is ready to merge once the naming is more like EdgeHtmlDriver rather than EdgeHTMLDriver. Everything else can be addressed in a later PR.

java/client/src/org/openqa/selenium/chromium/ChromiumDriverInfo.java Outdated Show resolved Hide resolved
java/client/src/org/openqa/selenium/chromium/ChromiumOptions.java Outdated Show resolved Hide resolved
java/client/src/org/openqa/selenium/edge/ChromiumEdgeDriverInfo.java Outdated Show resolved Hide resolved
java/client/src/org/openqa/selenium/edge/ChromiumEdgeDriverInfo.java Outdated Show resolved Hide resolved
java/client/src/org/openqa/selenium/edge/ChromiumEdgeDriverService.java
* @return A self reference.
*/
@Override
public EdgeDriverService.Builder withVerbose(boolean verbose) {
Copy link
Member

Choose a reason for hiding this comment

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

I'm curious to know if you can be both verbose and silent....

Side story: when I was working in radio, my fellow engineers and I once had to find "noisy silence". You can't have silence on the air, but it had to be clear it was meant to be silent.

java/client/src/org/openqa/selenium/edge/edgehtml/EdgeHTMLDriverInfo.java Outdated Show resolved Hide resolved
java/client/src/org/openqa/selenium/edge/edgehtml/EdgeHTMLDriverService.java Outdated Show resolved Hide resolved
java/client/src/org/openqa/selenium/edge/edgehtml/module-info.txt Show resolved Hide resolved
java/client/src/org/openqa/selenium/edge/edgehtml/module-info.txt Show resolved Hide resolved
java/client/src/org/openqa/selenium/edge/edgehtml/module-info.txt Outdated Show resolved Hide resolved
@loly89
Rename EdgeHTML to EdgeHtml
1023060 
Rename EdgeHTML to EdgeHtml and add generics return type for
ChromeOptions.
@loly89
Copy link
Contributor Author

loly89 commented May 30, 2019

This is ready to merge once the naming is more like EdgeHtmlDriver rather than EdgeHTMLDriver. Everything else can be addressed in a later PR.

@shs96c All done :)

@shs96c
Copy link
Member

shs96c commented May 31, 2019

Woohoo! Landing this now :) Congratulations!

@shs96c shs96c merged commit aa546c5 into SeleniumHQ:master May 31, 2019
@loly89 loly89 deleted the add_new_driver_support_for_edge_java branch May 31, 2019 16:37
@bhaidar
Copy link

bhaidar commented Sep 14, 2019

What about JavaScript client for Selenium/Edge Chromium?

@loly89
Copy link
Contributor Author

loly89 commented Sep 16, 2019

What about JavaScript client for Selenium/Edge Chromium?

Javascript is not our top priority for now, but there will definitely be more languages support in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shs96c shs96c shs96c approved these changes

Assignees
No one assigned
Labels
C-java D-edge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@loly89 @shs96c @bhaidar @p0deje