From 684f38c24669f824bbd2ea25de9c0f69156ac5ad Mon Sep 17 00:00:00 2001 From: Diego Fernando Molina Bocanegra Date: Thu, 17 Nov 2016 18:11:44 +0100 Subject: [PATCH 1/6] [BUA-720] No need to wait so long for startup locally since the node registers without delay. --- scripts/zalenium.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/zalenium.sh b/scripts/zalenium.sh index 895406f227..59171f78f8 100755 --- a/scripts/zalenium.sh +++ b/scripts/zalenium.sh @@ -110,7 +110,7 @@ StartUp() if [ "${IN_TRAVIS}" = "true" ]; then sleep 20 else - sleep 10 + sleep 2 fi echo "DockerSeleniumStarter node started!" From 422fa6e1b0d27e0abd5009377190af8c56718839 Mon Sep 17 00:00:00 2001 From: Diego Fernando Molina Bocanegra Date: Thu, 17 Nov 2016 18:14:34 +0100 Subject: [PATCH 2/6] [BUA-720] Creating extra nodes in a node to avoid blocking, also fixing the unit test. --- .../DockerSeleniumStarterRemoteProxy.java | 68 +++++++++++++++---- .../zalenium/proxy/SauceLabsRemoteProxy.java | 2 +- .../DockerSeleniumStarterRemoteProxyTest.java | 7 ++ 3 files changed, 62 insertions(+), 15 deletions(-) diff --git a/src/main/java/de/zalando/tip/zalenium/proxy/DockerSeleniumStarterRemoteProxy.java b/src/main/java/de/zalando/tip/zalenium/proxy/DockerSeleniumStarterRemoteProxy.java index 6b51112798..851bc38bef 100644 --- a/src/main/java/de/zalando/tip/zalenium/proxy/DockerSeleniumStarterRemoteProxy.java +++ b/src/main/java/de/zalando/tip/zalenium/proxy/DockerSeleniumStarterRemoteProxy.java @@ -60,6 +60,7 @@ public class DockerSeleniumStarterRemoteProxy extends DefaultRemoteProxy impleme private static CommonProxyUtilities commonProxyUtilities = defaultCommonProxyUtilities; private static final String LOGGING_PREFIX = "[DS] "; + private static boolean setupCompleted; private static int chromeContainersOnStartup; private static int firefoxContainersOnStartup; @@ -134,20 +135,42 @@ public TestSession getNewSession(Map requestedCapability) { /* Starting a few containers (Firefox, Chrome), so they are ready when the tests come. + Executed in a thread so we don't wait for the containers to be created and the node + registration is not delayed. */ @Override public void beforeRegistration() { readConfigurationFromEnvVariables(); - if (getChromeContainersOnStartup() > 0 || getFirefoxContainersOnStartup() > 0) { - LOGGER.log(Level.INFO, LOGGING_PREFIX + "Setting up {0} Firefox nodes and {1} Chrome nodes ready to use.", - new Object[]{getFirefoxContainersOnStartup(), getChromeContainersOnStartup()}); - for (int i = 0; i < getChromeContainersOnStartup(); i++) { - startDockerSeleniumContainer(BrowserType.CHROME); - } - for (int i = 0; i < getFirefoxContainersOnStartup(); i++) { - startDockerSeleniumContainer(BrowserType.FIREFOX); + setupCompleted = false; + new Thread() { + public void run() { + String message = String.format("%s Setting up %s Firefox nodes and %s Chrome nodes...", LOGGING_PREFIX, + getFirefoxContainersOnStartup(), getChromeContainersOnStartup()); + LOGGER.log(Level.INFO, message); + + int configuredContainers = getChromeContainersOnStartup() + getFirefoxContainersOnStartup(); + int containersToCreate = configuredContainers > getMaxDockerSeleniumContainers() ? + getMaxDockerSeleniumContainers() : configuredContainers; + int createdContainers = 0; + + while (createdContainers < containersToCreate && + getNumberOfRunningContainers() <= getMaxDockerSeleniumContainers()) { + + boolean wasContainerCreated; + if (createdContainers < getChromeContainersOnStartup()) { + wasContainerCreated = startDockerSeleniumContainer(BrowserType.CHROME); + } else { + wasContainerCreated = startDockerSeleniumContainer(BrowserType.FIREFOX); + } + + if (wasContainerCreated) { + createdContainers++; + } + } + LOGGER.log(Level.INFO, "Done setting up containers during startup."); + setupCompleted = true; } - } + }.start(); } /* @@ -216,7 +239,7 @@ public static List getCapabilities(String url) { } @VisibleForTesting - protected void startDockerSeleniumContainer(String browser) { + protected boolean startDockerSeleniumContainer(String browser) { if (validateAmountOfDockerSeleniumContainers()) { @@ -273,10 +296,12 @@ protected void startDockerSeleniumContainer(String browser) { final ContainerCreation dockerSeleniumContainer = dockerClient.createContainer(containerConfig, containerName); dockerClient.startContainer(dockerSeleniumContainer.id()); + return true; } catch (Exception e) { LOGGER.log(Level.SEVERE, LOGGING_PREFIX + e.toString(), e); } } + return false; } private String getLatestDownloadedImage(String imageName) throws DockerException, InterruptedException { @@ -369,6 +394,10 @@ public static void setScreenHeight(int screenHeight) { DockerSeleniumStarterRemoteProxy.screenHeight = screenHeight <= 0 ? DEFAULT_SCREEN_HEIGHT : screenHeight; } + public boolean isSetupCompleted() { + return setupCompleted; + } + @VisibleForTesting protected static void setEnv(final Environment env) { DockerSeleniumStarterRemoteProxy.env = env; @@ -416,7 +445,7 @@ protected static List getDockerSeleniumFallbackCapabilities return dsCapabilities; } - private boolean validateAmountOfDockerSeleniumContainers() { + private int getNumberOfRunningContainers() { try { List containerList = dockerClient.listContainers(DockerClient.ListContainersParam.allContainers()); int numberOfDockerSeleniumContainers = 0; @@ -426,6 +455,16 @@ private boolean validateAmountOfDockerSeleniumContainers() { numberOfDockerSeleniumContainers++; } } + return numberOfDockerSeleniumContainers; + } catch (Exception e) { + LOGGER.log(Level.SEVERE, LOGGING_PREFIX + e.toString(), e); + } + return 0; + } + + private boolean validateAmountOfDockerSeleniumContainers() { + try { + int numberOfDockerSeleniumContainers = getNumberOfRunningContainers(); /* Validation to avoid the situation where 20 containers are running and only 4 proxies are registered. @@ -441,10 +480,11 @@ private boolean validateAmountOfDockerSeleniumContainers() { return false; } - LOGGER.log(Level.FINE, LOGGING_PREFIX + "{0} docker-selenium containers running", containerList.size()); + LOGGER.log(Level.FINE, String.format("%s %s docker-selenium containers running", LOGGING_PREFIX, + numberOfDockerSeleniumContainers)); if (numberOfDockerSeleniumContainers >= getMaxDockerSeleniumContainers()) { - LOGGER.log(Level.FINE, LOGGING_PREFIX + "Max. number of docker-selenium containers has been reached, no more " + - "will be created until the number decreases below {0}.", getMaxDockerSeleniumContainers()); + LOGGER.log(Level.FINE, LOGGING_PREFIX + "Max. number of docker-selenium containers has been reached, " + + "no more will be created until the number decreases below {0}.", getMaxDockerSeleniumContainers()); return false; } return true; diff --git a/src/main/java/de/zalando/tip/zalenium/proxy/SauceLabsRemoteProxy.java b/src/main/java/de/zalando/tip/zalenium/proxy/SauceLabsRemoteProxy.java index 62a2606370..fa1e5467d2 100644 --- a/src/main/java/de/zalando/tip/zalenium/proxy/SauceLabsRemoteProxy.java +++ b/src/main/java/de/zalando/tip/zalenium/proxy/SauceLabsRemoteProxy.java @@ -70,7 +70,7 @@ private static RegistrationRequest addCapabilitiesToRegistrationRequest(Registra for (JsonElement cap : slCapabilities.getAsJsonArray()) { JsonObject capAsJsonObject = cap.getAsJsonObject(); DesiredCapabilities desiredCapabilities = new DesiredCapabilities(); - desiredCapabilities.setCapability(RegistrationRequest.MAX_INSTANCES, 10); + desiredCapabilities.setCapability(RegistrationRequest.MAX_INSTANCES, 1); desiredCapabilities.setBrowserName(capAsJsonObject.get("api_name").getAsString()); desiredCapabilities.setPlatform(getPlatform(capAsJsonObject.get("os").getAsString())); desiredCapabilities.setVersion(capAsJsonObject.get("long_version").getAsString()); diff --git a/src/test/java/de/zalando/tip/zalenium/proxy/DockerSeleniumStarterRemoteProxyTest.java b/src/test/java/de/zalando/tip/zalenium/proxy/DockerSeleniumStarterRemoteProxyTest.java index b55f538e4a..a540e09748 100644 --- a/src/test/java/de/zalando/tip/zalenium/proxy/DockerSeleniumStarterRemoteProxyTest.java +++ b/src/test/java/de/zalando/tip/zalenium/proxy/DockerSeleniumStarterRemoteProxyTest.java @@ -22,7 +22,11 @@ import java.util.HashMap; import java.util.Map; +import java.util.concurrent.Callable; +import java.util.concurrent.TimeUnit; +import static java.util.concurrent.TimeUnit.MILLISECONDS; +import static java.util.concurrent.TimeUnit.SECONDS; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyString; import static org.mockito.Mockito.verify; @@ -32,6 +36,7 @@ import static org.mockito.Mockito.spy; import static org.mockito.Mockito.never; import static org.mockito.Mockito.withSettings; +import static org.awaitility.Awaitility.await; public class DockerSeleniumStarterRemoteProxyTest { @@ -286,6 +291,8 @@ public void amountOfCreatedContainersIsTheConfiguredOne() { registry.add(spyProxy); + Callable callable = () -> spyProxy.isSetupCompleted(); + await().atMost(1, SECONDS).pollInterval(100, MILLISECONDS).until(callable); verify(spyProxy, times(amountOfChromeContainers)).startDockerSeleniumContainer(BrowserType.CHROME); verify(spyProxy, times(amountOfFirefoxContainers)).startDockerSeleniumContainer(BrowserType.FIREFOX); Assert.assertEquals(amountOfChromeContainers, DockerSeleniumStarterRemoteProxy.getChromeContainersOnStartup()); From 98b5182b4db19328e25db83f649e27c19722a9bc Mon Sep 17 00:00:00 2001 From: Diego Fernando Molina Bocanegra Date: Thu, 17 Nov 2016 18:44:33 +0100 Subject: [PATCH 3/6] [BUA-720] Fixing SQ issues. --- .../DockerSeleniumStarterRemoteProxy.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/main/java/de/zalando/tip/zalenium/proxy/DockerSeleniumStarterRemoteProxy.java b/src/main/java/de/zalando/tip/zalenium/proxy/DockerSeleniumStarterRemoteProxy.java index 851bc38bef..35b47d1227 100644 --- a/src/main/java/de/zalando/tip/zalenium/proxy/DockerSeleniumStarterRemoteProxy.java +++ b/src/main/java/de/zalando/tip/zalenium/proxy/DockerSeleniumStarterRemoteProxy.java @@ -60,7 +60,7 @@ public class DockerSeleniumStarterRemoteProxy extends DefaultRemoteProxy impleme private static CommonProxyUtilities commonProxyUtilities = defaultCommonProxyUtilities; private static final String LOGGING_PREFIX = "[DS] "; - private static boolean setupCompleted; + private boolean setupCompleted; private static int chromeContainersOnStartup; private static int firefoxContainersOnStartup; @@ -142,27 +142,26 @@ Starting a few containers (Firefox, Chrome), so they are ready when the tests co public void beforeRegistration() { readConfigurationFromEnvVariables(); setupCompleted = false; - new Thread() { - public void run() { - String message = String.format("%s Setting up %s Firefox nodes and %s Chrome nodes...", LOGGING_PREFIX, - getFirefoxContainersOnStartup(), getChromeContainersOnStartup()); - LOGGER.log(Level.INFO, message); + createStartupContainers(); + } + private void createStartupContainers() { + Thread thread = new Thread() { + public void run() { + LOGGER.log(Level.INFO, String.format("%s Setting up %s Firefox nodes and %s Chrome nodes...", + LOGGING_PREFIX, getFirefoxContainersOnStartup(), getChromeContainersOnStartup())); int configuredContainers = getChromeContainersOnStartup() + getFirefoxContainersOnStartup(); int containersToCreate = configuredContainers > getMaxDockerSeleniumContainers() ? getMaxDockerSeleniumContainers() : configuredContainers; int createdContainers = 0; - while (createdContainers < containersToCreate && getNumberOfRunningContainers() <= getMaxDockerSeleniumContainers()) { - boolean wasContainerCreated; if (createdContainers < getChromeContainersOnStartup()) { wasContainerCreated = startDockerSeleniumContainer(BrowserType.CHROME); } else { wasContainerCreated = startDockerSeleniumContainer(BrowserType.FIREFOX); } - if (wasContainerCreated) { createdContainers++; } @@ -170,7 +169,8 @@ public void run() { LOGGER.log(Level.INFO, "Done setting up containers during startup."); setupCompleted = true; } - }.start(); + }; + thread.start(); } /* From f9e7170d0d6d343e9292286159fd681852cab3f0 Mon Sep 17 00:00:00 2001 From: Diego Fernando Molina Bocanegra Date: Fri, 18 Nov 2016 13:34:07 +0100 Subject: [PATCH 4/6] [BUA-720] Fixing SQ issues and improving coverage. --- .../DockerSeleniumStarterRemoteProxy.java | 53 +++++++++---------- .../proxy/DockerSeleniumRemoteProxyTest.java | 6 +++ .../DockerSeleniumStarterRemoteProxyTest.java | 11 ++++ 3 files changed, 42 insertions(+), 28 deletions(-) diff --git a/src/main/java/de/zalando/tip/zalenium/proxy/DockerSeleniumStarterRemoteProxy.java b/src/main/java/de/zalando/tip/zalenium/proxy/DockerSeleniumStarterRemoteProxy.java index 35b47d1227..e6d6a36f47 100644 --- a/src/main/java/de/zalando/tip/zalenium/proxy/DockerSeleniumStarterRemoteProxy.java +++ b/src/main/java/de/zalando/tip/zalenium/proxy/DockerSeleniumStarterRemoteProxy.java @@ -145,34 +145,6 @@ public void beforeRegistration() { createStartupContainers(); } - private void createStartupContainers() { - Thread thread = new Thread() { - public void run() { - LOGGER.log(Level.INFO, String.format("%s Setting up %s Firefox nodes and %s Chrome nodes...", - LOGGING_PREFIX, getFirefoxContainersOnStartup(), getChromeContainersOnStartup())); - int configuredContainers = getChromeContainersOnStartup() + getFirefoxContainersOnStartup(); - int containersToCreate = configuredContainers > getMaxDockerSeleniumContainers() ? - getMaxDockerSeleniumContainers() : configuredContainers; - int createdContainers = 0; - while (createdContainers < containersToCreate && - getNumberOfRunningContainers() <= getMaxDockerSeleniumContainers()) { - boolean wasContainerCreated; - if (createdContainers < getChromeContainersOnStartup()) { - wasContainerCreated = startDockerSeleniumContainer(BrowserType.CHROME); - } else { - wasContainerCreated = startDockerSeleniumContainer(BrowserType.FIREFOX); - } - if (wasContainerCreated) { - createdContainers++; - } - } - LOGGER.log(Level.INFO, "Done setting up containers during startup."); - setupCompleted = true; - } - }; - thread.start(); - } - /* Making the node seem as heavily used, in order to get it listed after the 'docker-selenium' nodes. 98% used. @@ -445,6 +417,31 @@ protected static List getDockerSeleniumFallbackCapabilities return dsCapabilities; } + private void createStartupContainers() { + int configuredContainers = getChromeContainersOnStartup() + getFirefoxContainersOnStartup(); + int containersToCreate = configuredContainers > getMaxDockerSeleniumContainers() ? + getMaxDockerSeleniumContainers() : configuredContainers; + new Thread() { + @Override + public void run() { + LOGGER.log(Level.INFO, String.format("%s Setting up %s nodes...", LOGGING_PREFIX, configuredContainers)); + int createdContainers = 0; + while (createdContainers < containersToCreate && + getNumberOfRunningContainers() <= getMaxDockerSeleniumContainers()) { + boolean wasContainerCreated; + if (createdContainers < getChromeContainersOnStartup()) { + wasContainerCreated = startDockerSeleniumContainer(BrowserType.CHROME); + } else { + wasContainerCreated = startDockerSeleniumContainer(BrowserType.FIREFOX); + } + createdContainers = wasContainerCreated ? createdContainers + 1 : createdContainers; + } + LOGGER.log(Level.INFO, String.format("%s containers were created.", createdContainers)); + setupCompleted = true; + } + }.start(); + } + private int getNumberOfRunningContainers() { try { List containerList = dockerClient.listContainers(DockerClient.ListContainersParam.allContainers()); diff --git a/src/test/java/de/zalando/tip/zalenium/proxy/DockerSeleniumRemoteProxyTest.java b/src/test/java/de/zalando/tip/zalenium/proxy/DockerSeleniumRemoteProxyTest.java index a83375f63c..3b8dfb9f17 100644 --- a/src/test/java/de/zalando/tip/zalenium/proxy/DockerSeleniumRemoteProxyTest.java +++ b/src/test/java/de/zalando/tip/zalenium/proxy/DockerSeleniumRemoteProxyTest.java @@ -188,6 +188,9 @@ public void videoRecordingIsStartedAndStopped() throws DockerException, Interrup DockerSeleniumStarterRemoteProxy.class.getCanonicalName()); DockerSeleniumStarterRemoteProxy dsProxy = new DockerSeleniumStarterRemoteProxy(request, registry); DockerSeleniumStarterRemoteProxy.setMaxDockerSeleniumContainers(1); + DockerSeleniumStarterRemoteProxy.setScreenHeight(DockerSeleniumStarterRemoteProxy.DEFAULT_SCREEN_HEIGHT); + DockerSeleniumStarterRemoteProxy.setScreenWidth(DockerSeleniumStarterRemoteProxy.DEFAULT_SCREEN_WIDTH); + DockerSeleniumStarterRemoteProxy.setTimeZone(DockerSeleniumStarterRemoteProxy.DEFAULT_TZ); dsProxy.getNewSession(getCapabilitySupportedByDockerSelenium()); // Creating a spy proxy to verify the invoked methods @@ -243,6 +246,9 @@ public void videoRecordingIsDisabled() throws DockerException, InterruptedExcept DockerSeleniumStarterRemoteProxy.class.getCanonicalName()); DockerSeleniumStarterRemoteProxy dsProxy = new DockerSeleniumStarterRemoteProxy(request, registry); DockerSeleniumStarterRemoteProxy.setMaxDockerSeleniumContainers(1); + DockerSeleniumStarterRemoteProxy.setScreenHeight(DockerSeleniumStarterRemoteProxy.DEFAULT_SCREEN_HEIGHT); + DockerSeleniumStarterRemoteProxy.setScreenWidth(DockerSeleniumStarterRemoteProxy.DEFAULT_SCREEN_WIDTH); + DockerSeleniumStarterRemoteProxy.setTimeZone(DockerSeleniumStarterRemoteProxy.DEFAULT_TZ); dsProxy.getNewSession(getCapabilitySupportedByDockerSelenium()); // Mocking the environment variable to return false for video recording enabled diff --git a/src/test/java/de/zalando/tip/zalenium/proxy/DockerSeleniumStarterRemoteProxyTest.java b/src/test/java/de/zalando/tip/zalenium/proxy/DockerSeleniumStarterRemoteProxyTest.java index a540e09748..d1e0d00bda 100644 --- a/src/test/java/de/zalando/tip/zalenium/proxy/DockerSeleniumStarterRemoteProxyTest.java +++ b/src/test/java/de/zalando/tip/zalenium/proxy/DockerSeleniumStarterRemoteProxyTest.java @@ -130,6 +130,17 @@ public void containerIsStartedWhenFirefoxCapabilitiesAreSupported() { verify(spyProxy, times(1)).startDockerSeleniumContainer(BrowserType.FIREFOX); } + @Test + public void noContainerIsStartedWhenBrowserCapabilityIsAbsent() { + // Browser is absent + Map nonSupportedCapability = new HashMap<>(); + nonSupportedCapability.put(CapabilityType.PLATFORM, Platform.WINDOWS); + TestSession testSession = spyProxy.getNewSession(nonSupportedCapability); + + Assert.assertNull(testSession); + verify(spyProxy, never()).startDockerSeleniumContainer(anyString()); + } + /* The following tests check that if for any reason the capabilities from DockerSelenium cannot be fetched, it should fallback to the default ones. From 7e38e6336f636b994e3c95db272a22c527487812 Mon Sep 17 00:00:00 2001 From: Diego Fernando Molina Bocanegra Date: Fri, 18 Nov 2016 13:41:21 +0100 Subject: [PATCH 5/6] [BUA-780] Checking for docker errors before starting. --- scripts/zalenium.sh | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/scripts/zalenium.sh b/scripts/zalenium.sh index 59171f78f8..cab7f10a8f 100755 --- a/scripts/zalenium.sh +++ b/scripts/zalenium.sh @@ -24,6 +24,14 @@ EnsureCleanEnv() fi } +EnsureDockerWorks() +{ + if ! docker images elgalu/selenium >/dev/null; then + echo "Docker seems to be not working properly, check the above error." + exit 1 + fi +} + DockerTerminate() { echo "Trapped SIGTERM/SIGINT so shutting down Zalenium gracefully..." @@ -37,6 +45,7 @@ trap DockerTerminate SIGTERM SIGINT SIGKILL StartUp() { + EnsureDockerWorks EnsureCleanEnv DOCKER_SELENIUM_IMAGE_COUNT=$(docker images | grep "elgalu/selenium" | wc -l) From bd160cde3bd7fe485aca44643a529f710a8870eb Mon Sep 17 00:00:00 2001 From: Diego Fernando Molina Bocanegra Date: Fri, 18 Nov 2016 13:57:12 +0100 Subject: [PATCH 6/6] [BUA-780] Fixing unit test. --- .../zalenium/proxy/DockerSeleniumStarterRemoteProxyTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/de/zalando/tip/zalenium/proxy/DockerSeleniumStarterRemoteProxyTest.java b/src/test/java/de/zalando/tip/zalenium/proxy/DockerSeleniumStarterRemoteProxyTest.java index d1e0d00bda..8704b5ec53 100644 --- a/src/test/java/de/zalando/tip/zalenium/proxy/DockerSeleniumStarterRemoteProxyTest.java +++ b/src/test/java/de/zalando/tip/zalenium/proxy/DockerSeleniumStarterRemoteProxyTest.java @@ -134,7 +134,7 @@ public void containerIsStartedWhenFirefoxCapabilitiesAreSupported() { public void noContainerIsStartedWhenBrowserCapabilityIsAbsent() { // Browser is absent Map nonSupportedCapability = new HashMap<>(); - nonSupportedCapability.put(CapabilityType.PLATFORM, Platform.WINDOWS); + nonSupportedCapability.put(CapabilityType.PLATFORM, Platform.LINUX); TestSession testSession = spyProxy.getNewSession(nonSupportedCapability); Assert.assertNull(testSession);