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.

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

Use ChromeOptions and FirefoxOptions instead of DesiredCapabilities #995

Merged
merged 12 commits into from
Dec 4, 2018

Conversation

pavelpp
Copy link
Contributor

@pavelpp pavelpp commented Nov 27, 2018

The latest and recommended way to request a RemoteWebDriver is by passing ChromeOptions / FirefoxOptions to the constructor instead of DesiredCapabilities. Both of these implement Capabilities interface so the change was quite straightforward, deprecating the old method and changing the field type to Capabilities.

Another improvement is that we will not throw IllegalException when capabilities are not provided, but fall back to starting a chrome container. So now you could just write:

@Rule
public BrowserWebDriverContainer chrome = new BrowserWebDriverContainer();

omitting withCapabilities(new ChromeOptions()) and it will work out of the box.

Pavel Ponomarjov added 5 commits November 27, 2018 11:35
kiview and others added 6 commits November 27, 2018 12:48
…rowserWebDriverContainer.java

Co-Authored-By: pavelpp <pavelponomaryov@gmail.com>
…to selenium-update-capabilities

# Conflicts:
#	modules/selenium/src/main/java/org/testcontainers/containers/BrowserWebDriverContainer.java
…ncies to testCompile only, as they are only needed for tests
@rnorth
Copy link
Member

rnorth commented Dec 2, 2018

I'd just like to clarify whether this is a 'breaking change' for our users or not. I fear that it could be.

Because our Selenium dependency is provided scope, we let the user control the version. The API looks like it's backwards compatible, but will our code work with Selenium 2.x.x libraries? Has anybody tested this?

@pavelpp
Copy link
Contributor Author

pavelpp commented Dec 3, 2018

@rnorth Good point, I will test this locally in a dummy project when I find some more time

@pavelpp
Copy link
Contributor Author

pavelpp commented Dec 3, 2018

Updated fallback to Chrome when selenium version is 2.x. Now if detected Selenium version is 2.x, we will use DesiredCapabilities.chrome() and with newer Selenium versions - ChromeOptions() as a fallback with unspecified capabilities. That was the only issue I found. Verified locally with a dummy project and Selenium 2.53.1 dependency - works like a charm.

Copy link
Member

@kiview kiview left a comment

Choose a reason for hiding this comment

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

PR LGTM.

Just for clarification, using DesiredCapabilities is deprecated?
Will the class get removed in the future?

We probably also have to update the docs.

@rnorth
Copy link
Member

rnorth commented Dec 4, 2018

Thanks for addressing my query @pavelpp - LGTM

@kiview kiview merged commit 3ad8d80 into testcontainers:master Dec 4, 2018
@kiview kiview added this to the next milestone Dec 4, 2018
@kiview
Copy link
Member

kiview commented Dec 4, 2018

Thanks for your contribution @pavelpp, merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants