Cache elementScreenshotSupport per browser (#1264)

* Cache elementScreenshotSupport per browser * Make browsersWithoutElementScreenshot threadsafe Fixes #1261
vaadin · Jun 8, 2020 · 0f3f20e · 0f3f20e
1 parent d905b64
commit 0f3f20e
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 9 deletions.
diff --git a/vaadin-testbench-core/src/main/java/com/vaadin/testbench/commands/ScreenshotComparator.java b/vaadin-testbench-core/src/main/java/com/vaadin/testbench/commands/ScreenshotComparator.java
@@ -4,6 +4,9 @@
 import java.io.ByteArrayInputStream;
 import java.io.File;
 import java.io.IOException;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
 import java.util.logging.Logger;
 
 import javax.imageio.IIOException;
@@ -26,7 +29,7 @@
 
 public class ScreenshotComparator {
 
-    private static Boolean supportsElementScreenshots = null;
+    private static Set<String> browsersWithoutElementScreenshot = Collections.synchronizedSet(new HashSet<>());
 
     public static boolean compareScreen(String referenceId,
             ReferenceNameGenerator referenceNameGenerator,
@@ -39,7 +42,8 @@ public static boolean compareScreen(String referenceId,
         for (int times = 0; times < Parameters
                 .getMaxScreenshotRetries(); times++) {
             boolean equal = imageComparison.imageEqualToReference(
-                    getScreenshot((TakesScreenshot) driver, takesScreenshot),
+                    getScreenshot((TakesScreenshot) driver, takesScreenshot,
+                            capabilities),
                     referenceName,
                     Parameters.getScreenshotComparisonTolerance(),
                     capabilities);
@@ -60,32 +64,35 @@ public static boolean compareScreen(String referenceId,
      * @param screenshotContext
      *            The context of the screenshot, either a driver for a full page
      *            screenshot or an element for a screenshot of only that element
+     * @param capabilities
+     *            Browser capabilities
      * @return a captured image
      * @throws IOException
      */
     private static BufferedImage getScreenshot(TakesScreenshot driver,
-            TakesScreenshot screenshotContext) throws IOException {
+            TakesScreenshot screenshotContext, Capabilities capabilities)
+            throws IOException {
         boolean elementScreenshot = (screenshotContext instanceof WebElement);
+        String browserName = capabilities.getBrowserName();
 
-        if (elementScreenshot && supportsElementScreenshots == null) {
+        if (elementScreenshot && !browsersWithoutElementScreenshot.contains(browserName)) {
             // Detect if the driver supports element screenshots or not
             try {
                 byte[] screenshotBytes = screenshotContext
                         .getScreenshotAs(OutputType.BYTES);
-                supportsElementScreenshots = true;
                 return ImageIO.read(new ByteArrayInputStream(screenshotBytes));
             } catch (UnsupportedCommandException e) {
-                supportsElementScreenshots = false;
+                browsersWithoutElementScreenshot.add(browserName);
             } catch (WebDriverException e) {
                 if (e.getCause() instanceof UnsupportedCommandException) {
-                    supportsElementScreenshots = false;
+                    browsersWithoutElementScreenshot.add(browserName);
                 } else {
                     throw e;
                 }
             }
         }
 
-        if (elementScreenshot && !supportsElementScreenshots) {
+        if (elementScreenshot && browsersWithoutElementScreenshot.contains(browserName)) {
             // Driver does not support element screenshots, get whole screen
             // and crop
             BufferedImage image = ImageIO.read(new ByteArrayInputStream(

diff --git a/...tbench-core/src/test/java/com/vaadin/testbench/commands/TestBenchCommandExecutorTest.java b/...tbench-core/src/test/java/com/vaadin/testbench/commands/TestBenchCommandExecutorTest.java
@@ -240,8 +240,11 @@ private WebDriver mockScreenshotDriver(int nrScreenshotsGrabbed,
         expect(driver.executeScript(contains("window.Vaadin.Flow")))
                 .andReturn(Boolean.TRUE).anyTimes();
         if (expectGetCapabilities) {
+            Capabilities mockedCapabilities = createNiceMock(Capabilities.class);
+            expect(mockedCapabilities.getBrowserName()).andReturn("Firefox");
+            replay(mockedCapabilities);
             expect(driver.getCapabilities())
-                    .andReturn(createNiceMock(Capabilities.class)).once();
+                    .andReturn(mockedCapabilities).once();
         }
         return driver;
     }