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

Update BrowserWebDriverContainer to add Selenium 4 support, drop Selenium 2 support #4914

Merged
merged 7 commits into from Jan 13, 2022
Merged

Update BrowserWebDriverContainer to add Selenium 4 support, drop Selenium 2 support #4914

Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4fcc24e
Option to fix #4593 in the context of #4609
GannaChernyshova Jan 6, 2022
5ceaac7
Deleted RemoteWebDriver creation from containerIsStarted
GannaChernyshova Jan 11, 2022
0d74044
Added RemoteWebDriver creation to getWebDriver() method
GannaChernyshova Jan 12, 2022
5c8df20
Updated default DockerImage strategy
GannaChernyshova Jan 12, 2022
cb1432e
Updated lambdas and added synchronized to getWebDriver
GannaChernyshova Jan 12, 2022
68d530a
Removed statement of Selenium 2 support in docs
GannaChernyshova Jan 13, 2022
94fce73
Renamed variables in getStandardImageForCapabilities
GannaChernyshova Jan 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 10 additions & 21 deletions modules/selenium/src/main/java/org/testcontainers/containers/BrowserWebDriverContainer.java
View file
Open in desktop
Expand Up @@ -156,23 +156,6 @@ protected void configure() {

String seleniumVersion = SeleniumUtils.determineClasspathSeleniumVersion();

if (capabilities == null) {
if (seleniumVersion.startsWith("2.")) {
logger().info("No capabilities provided, falling back to DesiredCapabilities.chrome()");
capabilities = DesiredCapabilities.chrome();
} else {
logger().info("No capabilities provided, falling back to ChromeOptions");
capabilities = new ChromeOptions();
}
}

// Hack for new selenium-chrome image that contains Chrome 92.
// If not disabled, container startup will fail in most cases and consume excessive amounts of CPU.
if (capabilities instanceof ChromeOptions) {
ChromeOptions options = (ChromeOptions) this.capabilities;
options.addArguments("--disable-gpu");
}

if (recordingMode != VncRecordingMode.SKIP) {

if (vncRecordingDirectory == null) {
Expand Down Expand Up @@ -280,10 +263,6 @@ public int getPort() {

@Override
protected void containerIsStarted(InspectContainerResponse containerInfo) {
driver = Unreliables.retryUntilSuccess(30, TimeUnit.SECONDS,
() -> Timeouts.getWithTimeout(10, TimeUnit.SECONDS,
() -> new RemoteWebDriver(getSeleniumAddress(), capabilities)));

if (vncRecordingContainer != null) {
LOGGER.debug("Starting VNC recording");
vncRecordingContainer.start();
Expand All @@ -299,6 +278,16 @@ protected void containerIsStarted(InspectContainerResponse containerInfo) {
* @return a new Remote Web Driver instance
*/
public RemoteWebDriver getWebDriver() {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public RemoteWebDriver getWebDriver() {
public synchronized RemoteWebDriver getWebDriver() {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

agree, updated

if (driver == null) {
if (capabilities == null) {
logger().warn("No capabilities provided - this will cause an exception in future versions. Falling back to ChromeOptions");
capabilities = new ChromeOptions();
}

driver = Unreliables.retryUntilSuccess(30, TimeUnit.SECONDS,
() -> Timeouts.getWithTimeout(10, TimeUnit.SECONDS,
() -> new RemoteWebDriver(getSeleniumAddress(), capabilities)));
Copy link
Member

Choose a reason for hiding this comment

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

nit: while we are on it, for the sake of readability, could you please use long lambdas here? :)

Something like:

driver = Unreliables.retryUntilSuccess(30, TimeUnit.SECONDS, () -> {
    return Timeouts.getWithTimeout(10, TimeUnit.SECONDS, () -> {
        return new RemoteWebDriver(getSeleniumAddress(), capabilities);
    });
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done :)

}
return driver;
}

Expand Down