Switch browser containers to use LogMessageWaitStrategy (#328)

* Resolve #326, making browser container startup timeout configurable. Reduces default browser container startup timeout to 15s, down from an unnecessarily high 120s. This also replaces inheritance-based `waitUntilContainerStarted` with composition-based `WaitStrategy`. The new `LogMessageWaitStrategy` allows us to wait based upon the Selenium container log output, rather than the relatively expensive process of repeatedly trying to create and connect a `RemoteWebDriver` instance. * Resolve shading-related inheritance issues with BrowserWebDriverContainer This change makes docker-java NO LONGER SHADED into testcontainers core JAR * Trim WaitingConsumer output frame text to eliminate double-newline effect
testcontainers · May 1, 2017 · c03fba3 · c03fba3
1 parent b369ab3
commit c03fba3
Show file tree

Hide file tree

Showing 7 changed files with 58 additions and 28 deletions.
diff --git a/core/pom.xml b/core/pom.xml
@@ -18,7 +18,8 @@
         <dependency>
             <groupId>com.github.docker-java</groupId>
             <artifactId>docker-java</artifactId>
-            <version>3.0.9</version>
+            <version>${docker-java.version}</version>
+            <scope>compile</scope>
             <exclusions>
                 <!-- replace with junixsocket -->
                 <exclusion>
@@ -151,10 +152,6 @@
                             <pattern>com.fasterxml.jackson</pattern>
                             <shadedPattern>org.testcontainers.shaded.com.fasterxml.jackson</shadedPattern>
                         </relocation>
-                        <relocation>
-                            <pattern>com.github.dockerjava</pattern>
-                            <shadedPattern>org.testcontainers.shaded.com.github.dockerjava</shadedPattern>
-                        </relocation>
                         <relocation>
                             <pattern>jersey.repackaged</pattern>
                             <shadedPattern>org.testcontainers.shaded.jersey.repackaged</shadedPattern>

diff --git a/core/src/main/java/org/testcontainers/containers/GenericContainer.java b/core/src/main/java/org/testcontainers/containers/GenericContainer.java
@@ -867,8 +867,9 @@ public ExecResult execInContainer(Charset outputCharset, String... command)
      *
      * @param attempts number of attempts
      */
-    public void withStartupAttempts(int attempts) {
+    public SELF withStartupAttempts(int attempts) {
         this.startupAttempts = attempts;
+        return self();
     }
 
     @Override

diff --git a/core/src/main/java/org/testcontainers/containers/output/WaitingConsumer.java b/core/src/main/java/org/testcontainers/containers/output/WaitingConsumer.java
@@ -86,7 +86,8 @@ private void waitUntil(Predicate<OutputFrame> predicate, long expiry, int times)
                 OutputFrame frame = frames.pollLast(100, TimeUnit.MILLISECONDS);
 
                 if (frame != null) {
-                    LOGGER.debug("{}: {}", frame.getType(), frame.getUtf8String());
+                    final String trimmedFrameText = frame.getUtf8String().replaceFirst("\n$", "");
+                    LOGGER.debug("{}: {}", frame.getType(), trimmedFrameText);
 
                     if (predicate.test(frame)) {
                         numberOfMatches++;

diff --git a/modules/selenium/pom.xml b/modules/selenium/pom.xml
@@ -1,5 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
     <modelVersion>4.0.0</modelVersion>
 
     <parent>
@@ -18,6 +19,12 @@
             <artifactId>testcontainers</artifactId>
             <version>${project.version}</version>
         </dependency>
+        <dependency>
+            <groupId>com.github.docker-java</groupId>
+            <artifactId>docker-java</artifactId>
+            <version>${docker-java.version}</version>
+            <scope>provided</scope>
+        </dependency>
 
         <dependency>
             <groupId>com.google.guava</groupId>
@@ -39,6 +46,7 @@
             <groupId>org.mortbay.jetty</groupId>
             <artifactId>jetty</artifactId>
             <version>6.1.25</version>
+            <scope>test</scope>
         </dependency>
     </dependencies>
 

diff --git a/modules/selenium/src/main/java/org/testcontainers/containers/BrowserWebDriverContainer.java b/modules/selenium/src/main/java/org/testcontainers/containers/BrowserWebDriverContainer.java
@@ -1,28 +1,28 @@
 package org.testcontainers.containers;
 
-import com.google.common.util.concurrent.Uninterruptibles;
+import com.github.dockerjava.api.command.InspectContainerResponse;
 import org.jetbrains.annotations.Nullable;
 import org.junit.runner.Description;
 import org.openqa.selenium.remote.BrowserType;
 import org.openqa.selenium.remote.DesiredCapabilities;
 import org.openqa.selenium.remote.RemoteWebDriver;
-import org.rnorth.ducttape.unreliables.Unreliables;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.testcontainers.containers.traits.LinkableContainer;
 import org.testcontainers.containers.traits.VncService;
+import org.testcontainers.containers.wait.LogMessageWaitStrategy;
 
 import java.io.File;
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.text.SimpleDateFormat;
+import java.time.Duration;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Date;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicInteger;
 
 import static com.google.common.base.Preconditions.checkState;
+import static java.time.temporal.ChronoUnit.SECONDS;
 
 /**
  * A chrome/firefox/custom container based on SeleniumHQ's standalone container sets.
@@ -55,14 +55,17 @@ public class BrowserWebDriverContainer<SELF extends BrowserWebDriverContainer<SE
     /**
      */
     public BrowserWebDriverContainer() {
-
+        this.waitStrategy = new LogMessageWaitStrategy()
+                .withRegEx(".*RemoteWebDriver instances should connect to.*\n")
+                .withStartupTimeout(Duration.of(15, SECONDS));
     }
 
     /**
      * Constructor taking a specific webdriver container name and tag
      * @param dockerImageName
      */
     public BrowserWebDriverContainer(String dockerImageName) {
+        this();
         super.setDockerImageName(dockerImageName);
         this.customImageNameIsSet = true;
     }
@@ -143,26 +146,18 @@ public int getPort() {
     }
 
     @Override
-    protected void waitUntilContainerStarted() {
-        // Repeatedly try and open a webdriver session
-
-        AtomicInteger backoff = new AtomicInteger(1000);
-
-        driver = Unreliables.retryUntilSuccess(120, TimeUnit.SECONDS, () -> {
-            Uninterruptibles.sleepUninterruptibly(backoff.getAndUpdate(current -> (int)(current * 1.5)), TimeUnit.MILLISECONDS);
-            RemoteWebDriver driver = new RemoteWebDriver(getSeleniumAddress(), desiredCapabilities);
-            driver.getCurrentUrl();
-
-            logger().info("Obtained a connection to container ({})", BrowserWebDriverContainer.this.getSeleniumAddress());
-            return driver;
-        });
-
+    protected void containerIsStarted(InspectContainerResponse containerInfo) {
         if (recordingMode != VncRecordingMode.SKIP) {
             LOGGER.debug("Starting VNC recording");
-            VncRecordingSidekickContainer recordingSidekickContainer = new VncRecordingSidekickContainer<>(this);
+
+            // Use multiple startup attempts due to race condition between Selenium being available and VNC being available
+            VncRecordingSidekickContainer recordingSidekickContainer = new VncRecordingSidekickContainer<>(this)
+                    .withStartupAttempts(3);
+
             recordingSidekickContainer.start();
             currentVncRecordings.add(recordingSidekickContainer);
         }
+        this.driver = new RemoteWebDriver(getSeleniumAddress(), desiredCapabilities);
     }
 
     /**

diff --git a/...enium/src/test/java/org/testcontainers/junit/CustomWaitTimeoutWebDriverContainerTest.java b/...enium/src/test/java/org/testcontainers/junit/CustomWaitTimeoutWebDriverContainerTest.java
@@ -0,0 +1,27 @@
+package org.testcontainers.junit;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.openqa.selenium.remote.DesiredCapabilities;
+import org.testcontainers.containers.BrowserWebDriverContainer;
+
+import java.io.IOException;
+import java.time.Duration;
+
+import static java.time.temporal.ChronoUnit.SECONDS;
+
+/**
+ *
+ */
+public class CustomWaitTimeoutWebDriverContainerTest extends BaseWebDriverContainerTest {
+
+    @Rule
+    public BrowserWebDriverContainer chromeWithCustomTimeout = new BrowserWebDriverContainer<>()
+            .withDesiredCapabilities(DesiredCapabilities.chrome())
+            .withStartupTimeout(Duration.of(30, SECONDS));
+
+    @Test
+    public void simpleTest() throws IOException {
+        doSimpleWebdriverTest(chromeWithCustomTimeout);
+    }
+}
diff --git a/pom.xml b/pom.xml
@@ -157,6 +157,7 @@
 
     <properties>
         <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+        <docker-java.version>3.0.9</docker-java.version>
     </properties>
 
     <modules>