Updated local screenshot testing configurations. (#11814)

* A separate screenshot module hasn't been in use for a while but local
testing configurations and instructions weren't updated to the new model
at the time. Could use some further cleanup but this covers the basics.
* Some tweaks to ScreenshotBrowser to get it functional again, although
the actions for automatically replacing or adding alternative
screenshots still require work and have thus been disabled.

README-TESTS.md

@@ -53,7 +53,7 @@ We're going to do the same as with the command-line approach - launch the `mvn j
 Before running any tests from your IDE you need to
 1. copy `uitest/eclipse-run-selected-test.properties` to `work/eclipse-run-selected-test.properties`
 2. edit `work/eclipse-run-selected-test.properties`
-    1. Define `com.vaadin.testbench.screenshot.directory` as the directory where you checked out the screenshots repository (this directory contains the “references” subdirectory)
+    1. Define `com.vaadin.testbench.screenshot.directory` as the `uitest` repository (this directory contains the “reference-screenshots” subdirectory)
     2. Set `com.vaadin.testbench.deployment.url=http://localhost:8888/`
     3. Set `com.vaadin.testbench.runLocally=chrome` to only run tests on Chrome. On Ubuntu you can then install Chrome driver easily:
        `sudo apt install chromium-chromedriver`

test/servlet-containers/generic/build.xml

@@ -39,10 +39,10 @@
     <target name="clean-testbench-errors"><!--todo remove when have got rid of screenshots-->
         <fail unless="com.vaadin.testbench.screenshot.directory"
             message="Define screenshot directory using -Dcom.vaadin.testbench.screenshot.directory" />
-        <mkdir dir="${com.vaadin.testbench.screenshot.directory}/errors" />
+        <mkdir dir="${com.vaadin.testbench.screenshot.directory}/error-screenshots" />
         <delete>
             <fileset
-                dir="${com.vaadin.testbench.screenshot.directory}/errors">
+                dir="${com.vaadin.testbench.screenshot.directory}/error-screenshots">
                 <include name="*" />
             </fileset>
         </delete>

uitest/eclipse-run-selected-test.properties

@@ -10,7 +10,7 @@
 # Location of the screenshot directory. This is mutually exclusive with the folder settings for XVFB testing.
 # This is the directory that contains the "references" directory
 
-com.vaadin.testbench.screenshot.directory=<enter the full path to the screenshots directory, parent of "references" directory>
+com.vaadin.testbench.screenshot.directory=<enter the full path to the "uitest" directory, parent of "reference-screenshots" directory>
 
 # Deployment url to use for testing. Context path must be /  
 

uitest/src/main/java/com/vaadin/screenshotbrowser/ScreenshotBrowser.java

@@ -9,6 +9,10 @@ import java.util.Properties;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import org.apache.commons.io.FileUtils;
+import org.apache.commons.io.filefilter.DirectoryFileFilter;
+import org.apache.commons.io.filefilter.SuffixFileFilter;
+
 import com.vaadin.event.ShortcutAction.KeyCode;
 import com.vaadin.event.ShortcutListener;
 import com.vaadin.server.ExternalResource;
@@ -35,11 +39,11 @@ public class ScreenshotBrowser extends UI {
      * 3 - platform
      * 4 - browser name
      * 5 - browser version
-     * 6 - additional qualifiers
-     * 7 - identifier
+     * 6 - identifier
+     * 7 - additional identifiers
      */
     private static final Pattern screenshotNamePattern = Pattern
-            .compile("(.+?)-(.+?)_(.+?)_(.+?)_(.+?)(_.+)?_(.+?)\\.png\\.html");
+            .compile("(.+?)-(.+?)_(.+?)_(.+?)_(.*?)_(.+?)(_.+)?\\.png\\.html");
 
     public static enum Action {
         ACCEPT {
@@ -97,7 +101,7 @@ public class ScreenshotBrowser extends UI {
         }
 
         private static File getReferenceDir() {
-            return new File(screenshotDir, "reference");
+            return new File(screenshotDir, "reference-screenshots");
         }
 
         private static File getAlternative(File baseFile,
@@ -146,12 +150,12 @@ public class ScreenshotBrowser extends UI {
             return matcher.group(4) + " " + matcher.group(5);
         }
 
-        public String getQualifiers() {
-            return matcher.group(6);
-        }
-
         public String getIdentifier() {
-            return matcher.group(7);
+            String additional = matcher.group(7);
+            if (additional != null) {
+                return matcher.group(6) + additional;
+            }
+            return matcher.group(6);
         }
 
         public void setAction(Action action) {
@@ -176,7 +180,8 @@ public class ScreenshotBrowser extends UI {
 
             left.setMargin(true);
             left.setSpacing(true);
-            left.setSizeFull();
+            left.setSizeUndefined();
+            left.setWidth("270px");
 
             left.addComponent(
                     createActionButton("Accept changes", 'j', Action.ACCEPT));
@@ -199,15 +204,17 @@ public class ScreenshotBrowser extends UI {
             left.addComponent(expandSpacer);
             left.setExpandRatio(expandSpacer, 1);
 
-            left.addComponent(new Label(
-                    "Press the j, k or l keys to quickly select an action for the selected item."));
+            Label instructions = new Label(
+                    "Press the j, k or l keys to quickly select an action for the selected item.");
+            instructions.setWidth("100%");
+            left.addComponent(instructions);
 
             root.setExpandRatio(left, 1);
             root.setSizeFull();
 
             setCompositionRoot(root);
             setHeight("850px");
-            setWidth("100%");
+            setWidth("1800px");
         }
 
         private Button createActionButton(String caption, char shortcut,
@@ -216,6 +223,11 @@ public class ScreenshotBrowser extends UI {
                     caption + " <strong>" + shortcut + "</strong>",
                     createSetActionListener(action));
             button.setCaptionAsHtml(true);
+            if (!Action.IGNORE.equals(action)) {
+                // other actions disabled for now since the functionality was
+                // designed for a different directory structure, needs reworking
+                button.setEnabled(false);
+            }
             return button;
         }
 
@@ -253,7 +265,7 @@ public class ScreenshotBrowser extends UI {
     @Override
     protected void init(VaadinRequest request) {
         table.setWidth("100%");
-        table.setHeight("100%");
+        table.setPageLength(10);
 
         table.setMultiSelect(true);
         table.addValueChangeListener(event -> {
@@ -274,9 +286,8 @@ public class ScreenshotBrowser extends UI {
 
         refreshTableContainer();
 
-        VerticalLayout mainLayout = new VerticalLayout(table, viewer);
-        mainLayout.setExpandRatio(table, 1);
-        mainLayout.setSizeFull();
+        VerticalLayout mainLayout = new VerticalLayout(viewer, table);
+        mainLayout.setSizeUndefined();
 
         setSizeFull();
         setContent(mainLayout);
@@ -339,10 +350,10 @@ public class ScreenshotBrowser extends UI {
     }
 
     private void refreshTableContainer() {
-        File errorsDir = new File(screenshotDir, "errors");
+        File errorsDir = new File(screenshotDir, "error-screenshots");
 
-        File[] failures = errorsDir
-                .listFiles((dir, name) -> name.endsWith(".html"));
+        Collection<File> failures = FileUtils.listFiles(errorsDir,
+                new SuffixFileFilter(".html"), DirectoryFileFilter.DIRECTORY);
 
         BeanItemContainer<ComparisonFailure> container = new BeanItemContainer<>(
                 ComparisonFailure.class);
@@ -352,7 +363,7 @@ public class ScreenshotBrowser extends UI {
 
         table.setContainerDataSource(container);
         table.setVisibleColumns("testClass", "testMethod", "browser",
-                "qualifiers", "identifier", "action");
+                "identifier", "action");
         if (container.size() > 0) {
             table.select(container.firstItemId());
         }

uitest/src/test/java/com/vaadin/tests/tb3/PrivateTB3Configuration.java

@@ -80,8 +80,9 @@ public abstract class PrivateTB3Configuration extends ScreenshotTB3Test {
         String dir = System.getProperty(SCREENSHOT_DIRECTORY,
                 properties.getProperty(SCREENSHOT_DIRECTORY));
         if (dir != null && !dir.isEmpty()) {
-            String reference = Paths.get(dir, "reference").toString();
-            String errors = Paths.get(dir, "errors").toString();
+            String reference = Paths.get(dir, "reference-screenshots")
+                    .toString();
+            String errors = Paths.get(dir, "error-screenshots").toString();
             Parameters.setScreenshotReferenceDirectory(reference);
             Parameters.setScreenshotErrorDirectory(errors);
         } else {

uitest/src/test/java/com/vaadin/tests/tb3/VaadinBrowserFactory.java

@@ -43,7 +43,7 @@ public class VaadinBrowserFactory extends DefaultBrowserFactory {
         case PHANTOMJS:
             return create(browser, "1", Platform.LINUX);
         case CHROME:
-            return create(browser, "40", Platform.VISTA);
+            return create(browser, "", Platform.ANY);
         case FIREFOX:
         default:
             DesiredCapabilities dc = create(Browser.FIREFOX, "45",