From b235d9c4e166197f3369694feec8d8f12a93c49d Mon Sep 17 00:00:00 2001
From: Artur Signell <artur@vaadin.com>
Date: Fri, 27 Sep 2013 13:14:36 +0300
Subject: [PATCH] Refactor how TB3 tests are written (#12572)

* openTestUrl() must be called in the beginning of each test. Before that setPush and setDebug can be used to determine whether /run-push or ?debug should be used in the URL

Change-Id: Ie70996fcbdb2769fcd4ec56cb878e20198e02bf6
---
 .../vaadin/tests/VerifyBrowserVersion.java    |  1 +
 .../tests/components/label/LabelModes.java    |  1 +
 .../vaadin/tests/components/ui/UIAccess.java  |  7 +-
 .../src/com/vaadin/tests/push/BarInUIDL.java  |  1 +
 .../src/com/vaadin/tests/push/BasicPush.java  |  2 +
 .../vaadin/tests/push/BasicPushStreaming.java |  8 --
 .../tests/push/PushConfigurationTest.java     |  7 +-
 .../com/vaadin/tests/push/PushFromInit.java   |  2 +
 .../tests/push/PushReattachedComponent.java   |  8 +-
 .../src/com/vaadin/tests/push/TogglePush.java | 62 ++------------
 .../tests/push/TrackMessageSizeUnitTests.java |  1 +
 .../com/vaadin/tests/tb3/AbstractTB3Test.java | 85 ++++++++++++++-----
 12 files changed, 87 insertions(+), 98 deletions(-)

diff --git a/uitest/src/com/vaadin/tests/VerifyBrowserVersion.java b/uitest/src/com/vaadin/tests/VerifyBrowserVersion.java
index ae6f0afa4d..47c09bdfd7 100644
--- a/uitest/src/com/vaadin/tests/VerifyBrowserVersion.java
+++ b/uitest/src/com/vaadin/tests/VerifyBrowserVersion.java
@@ -45,6 +45,7 @@ public class VerifyBrowserVersion extends TestBase {
 
         @Test
         public void verifyUserAgent() {
+            openTestURL();
             Assert.assertEquals(
                     expectedUserAgent.get(getDesiredCapabilities()),
                     vaadinElementById("userAgent").getText());
diff --git a/uitest/src/com/vaadin/tests/components/label/LabelModes.java b/uitest/src/com/vaadin/tests/components/label/LabelModes.java
index 1959447a4b..9e51978743 100644
--- a/uitest/src/com/vaadin/tests/components/label/LabelModes.java
+++ b/uitest/src/com/vaadin/tests/components/label/LabelModes.java
@@ -10,6 +10,7 @@ public class LabelModes extends ComponentTestCase<Label> {
     public static class LabelModesTest extends SimpleMultiBrowserTest {
         @Override
         public void test() throws Exception {
+            openTestURL();
             compareScreen("labelmodes");
         }
 
diff --git a/uitest/src/com/vaadin/tests/components/ui/UIAccess.java b/uitest/src/com/vaadin/tests/components/ui/UIAccess.java
index 057dcaa917..7515b3ede8 100644
--- a/uitest/src/com/vaadin/tests/components/ui/UIAccess.java
+++ b/uitest/src/com/vaadin/tests/components/ui/UIAccess.java
@@ -40,13 +40,10 @@ import com.vaadin.util.CurrentInstance;
 public class UIAccess extends AbstractTestUIWithLog {
 
     public static class UIAccessTest extends MultiBrowserTest {
-        @Override
-        protected boolean isPushEnabled() {
-            return true;
-        }
-
         @Test
         public void testThreadLocals() {
+            setPush(true);
+            openTestURL();
             getCurrentInstanceWhenPushingButton().click();
             waitUntil(ExpectedConditions.textToBePresentInElement(
                     vaadinLocatorById("Log_row_0"), "1."));
diff --git a/uitest/src/com/vaadin/tests/push/BarInUIDL.java b/uitest/src/com/vaadin/tests/push/BarInUIDL.java
index ef2568bebe..ebf349683d 100644
--- a/uitest/src/com/vaadin/tests/push/BarInUIDL.java
+++ b/uitest/src/com/vaadin/tests/push/BarInUIDL.java
@@ -35,6 +35,7 @@ public class BarInUIDL extends AbstractTestUI {
     public static class BarInUIDLTest extends MultiBrowserTest {
         @Test
         public void sendBarInUIDL() {
+            openTestURL();
             getButton().click();
             Assert.assertEquals(
                     "Thank you for clicking | bar",
diff --git a/uitest/src/com/vaadin/tests/push/BasicPush.java b/uitest/src/com/vaadin/tests/push/BasicPush.java
index 1f184863f7..d1a9fb575f 100644
--- a/uitest/src/com/vaadin/tests/push/BasicPush.java
+++ b/uitest/src/com/vaadin/tests/push/BasicPush.java
@@ -39,6 +39,8 @@ public class BasicPush extends AbstractTestUI {
 
         @Test
         public void testPush() {
+            openTestURL();
+
             // Test client initiated push
             Assert.assertEquals(0, getClientCounter());
             getIncrementButton().click();
diff --git a/uitest/src/com/vaadin/tests/push/BasicPushStreaming.java b/uitest/src/com/vaadin/tests/push/BasicPushStreaming.java
index 29534e1f5c..b63f782cc9 100644
--- a/uitest/src/com/vaadin/tests/push/BasicPushStreaming.java
+++ b/uitest/src/com/vaadin/tests/push/BasicPushStreaming.java
@@ -15,8 +15,6 @@
  */
 package com.vaadin.tests.push;
 
-import org.junit.Test;
-
 import com.vaadin.annotations.Push;
 import com.vaadin.server.VaadinRequest;
 import com.vaadin.shared.ui.ui.Transport;
@@ -32,11 +30,5 @@ public class BasicPushStreaming extends BasicPush {
     }
 
     public static class BasicPushStreamingTest extends BasicPushTest {
-        @Override
-        @Test
-        public void testPush() {
-            super.testPush();
-        }
     }
-
 }
diff --git a/uitest/src/com/vaadin/tests/push/PushConfigurationTest.java b/uitest/src/com/vaadin/tests/push/PushConfigurationTest.java
index f4a537392d..5100e8a4ea 100644
--- a/uitest/src/com/vaadin/tests/push/PushConfigurationTest.java
+++ b/uitest/src/com/vaadin/tests/push/PushConfigurationTest.java
@@ -37,13 +37,10 @@ public class PushConfigurationTest extends AbstractTestUI {
 
     public static class PushConfigurationWebsocket extends WebsocketTest {
 
-        @Override
-        protected boolean isDebug() {
-            return true;
-        }
-
         @Test
         public void testWebsocketAndStreaming() {
+            setDebug(true);
+            openTestURL();
             // Websocket
             Assert.assertEquals(1, getServerCounter());
             new Select(getTransportSelect()).selectByVisibleText("WEBSOCKET");
diff --git a/uitest/src/com/vaadin/tests/push/PushFromInit.java b/uitest/src/com/vaadin/tests/push/PushFromInit.java
index 000d5c0bce..de3334f707 100644
--- a/uitest/src/com/vaadin/tests/push/PushFromInit.java
+++ b/uitest/src/com/vaadin/tests/push/PushFromInit.java
@@ -28,6 +28,8 @@ public class PushFromInit extends AbstractTestUIWithLog {
     public static class PushFromInitTB3 extends MultiBrowserTest {
         @Test
         public void testPushFromInit() {
+            openTestURL();
+
             for (int second = 0;; second++) {
                 if (second >= 30) {
                     Assert.fail("timeout");
diff --git a/uitest/src/com/vaadin/tests/push/PushReattachedComponent.java b/uitest/src/com/vaadin/tests/push/PushReattachedComponent.java
index 35934273a4..0bc796e0ee 100644
--- a/uitest/src/com/vaadin/tests/push/PushReattachedComponent.java
+++ b/uitest/src/com/vaadin/tests/push/PushReattachedComponent.java
@@ -27,11 +27,6 @@ import com.vaadin.tests.tb3.MultiBrowserTest;
 
 public class PushReattachedComponent extends MultiBrowserTest {
 
-    @Override
-    protected boolean isPushEnabled() {
-        return true;
-    }
-
     @Override
     protected Class<?> getUIClass() {
         return PanelChangeContents.class;
@@ -39,6 +34,9 @@ public class PushReattachedComponent extends MultiBrowserTest {
 
     @Test
     public void testReattachComponentUsingPush() {
+        setPush(true);
+        openTestURL();
+
         Assert.assertEquals(
                 "stats",
                 vaadinElement(
diff --git a/uitest/src/com/vaadin/tests/push/TogglePush.java b/uitest/src/com/vaadin/tests/push/TogglePush.java
index f59ef83fe7..3ef369b408 100644
--- a/uitest/src/com/vaadin/tests/push/TogglePush.java
+++ b/uitest/src/com/vaadin/tests/push/TogglePush.java
@@ -34,30 +34,12 @@ import com.vaadin.ui.CheckBox;
 import com.vaadin.ui.Label;
 
 public class TogglePush extends AbstractTestUI {
-    public static class TogglePushInInitTB3 extends MultiBrowserTest {
-        @Override
-        protected boolean isPushEnabled() {
-            return true;
-        }
-
-        /*
-         * (non-Javadoc)
-         * 
-         * @see com.vaadin.tests.tb3.AbstractTB3Test#getTestUrl()
-         */
-        @Override
-        protected String getTestUrl() {
-            return null;
-        }
+    public static class TogglePushTB3 extends MultiBrowserTest {
 
         @Test
         public void togglePushInInit() {
-            String baseUrl = getBaseURL();
-            if (baseUrl.endsWith("/")) {
-                baseUrl = baseUrl.substring(0, baseUrl.length() - 1);
-            }
-
-            String url = baseUrl + getDeploymentPath();
+            setPush(true);
+            String url = getTestUrl();
 
             // Open with push disabled
             driver.get(addParameter(url, "push=disabled"));
@@ -80,12 +62,6 @@ public class TogglePush extends AbstractTestUI {
 
         }
 
-        /**
-         * @since
-         * @param url
-         * @param string
-         * @return
-         */
         private String addParameter(String url, String queryParameter) {
             if (url.contains("?")) {
                 return url + "&" + queryParameter;
@@ -94,31 +70,10 @@ public class TogglePush extends AbstractTestUI {
             }
         }
 
-        private String getCounterText() {
-            return vaadinElement(
-                    "/VVerticalLayout[0]/Slot[1]/VVerticalLayout[0]/Slot[0]/VLabel[0]")
-                    .getText();
-        }
-
-        private WebElement getPushToggle() {
-            return vaadinElement("/VVerticalLayout[0]/Slot[1]/VVerticalLayout[0]/Slot[1]/VCheckBox[0]/domChild[0]");
-        }
-
-        private WebElement getDelayedCounterUpdateButton() {
-            return vaadinElement("/VVerticalLayout[0]/Slot[1]/VVerticalLayout[0]/Slot[3]/VButton[0]/domChild[0]/domChild[0]");
-        }
-
-    }
-
-    public static class TogglePushTB3 extends MultiBrowserTest {
-
-        @Override
-        protected boolean isPushEnabled() {
-            return true;
-        }
-
         @Test
         public void togglePush() {
+            setPush(true);
+            openTestURL();
             getDelayedCounterUpdateButton().click();
             sleep(2000);
 
@@ -157,15 +112,16 @@ public class TogglePush extends AbstractTestUI {
             return vaadinElement("/VVerticalLayout[0]/Slot[1]/VVerticalLayout[0]/Slot[1]/VCheckBox[0]/domChild[0]");
         }
 
+        private WebElement getDelayedCounterUpdateButton() {
+            return vaadinElement("/VVerticalLayout[0]/Slot[1]/VVerticalLayout[0]/Slot[3]/VButton[0]/domChild[0]/domChild[0]");
+        }
+
         private String getCounterText() {
             return vaadinElement(
                     "/VVerticalLayout[0]/Slot[1]/VVerticalLayout[0]/Slot[0]/VLabel[0]")
                     .getText();
         }
 
-        private WebElement getDelayedCounterUpdateButton() {
-            return vaadinElement("/VVerticalLayout[0]/Slot[1]/VVerticalLayout[0]/Slot[3]/VButton[0]/domChild[0]/domChild[0]");
-        }
     }
 
     private final Label counterLabel = new Label();
diff --git a/uitest/src/com/vaadin/tests/push/TrackMessageSizeUnitTests.java b/uitest/src/com/vaadin/tests/push/TrackMessageSizeUnitTests.java
index d7459f83ab..cb3033aa58 100644
--- a/uitest/src/com/vaadin/tests/push/TrackMessageSizeUnitTests.java
+++ b/uitest/src/com/vaadin/tests/push/TrackMessageSizeUnitTests.java
@@ -43,6 +43,7 @@ public class TrackMessageSizeUnitTests extends AbstractTestUIWithLog {
     public static class TrackMessageSizeUnitTestsTB3 extends MultiBrowserTest {
         @Test
         public void runTests() {
+            openTestURL();
             Assert.assertEquals("1. All tests run",
                     vaadinElementById("Log_row_0").getText());
         }
diff --git a/uitest/src/com/vaadin/tests/tb3/AbstractTB3Test.java b/uitest/src/com/vaadin/tests/tb3/AbstractTB3Test.java
index 1897728366..054492444d 100644
--- a/uitest/src/com/vaadin/tests/tb3/AbstractTB3Test.java
+++ b/uitest/src/com/vaadin/tests/tb3/AbstractTB3Test.java
@@ -47,11 +47,10 @@ import com.vaadin.ui.UI;
  * <ul>
  * <li>Helpers for browser selection</li>
  * <li>Hub connection setup and teardown</li>
- * <li>Automatic opening of a given test on the development server using
- * {@link #getUIClass()} or by automatically finding an enclosing UI class</li>
+ * <li>Automatic generation of URL for a given test on the development server
+ * using {@link #getUIClass()} or by automatically finding an enclosing UI class
+ * and based on requested features, e.g. {@link #isDebug()}, {@link #isPush()}</li>
  * <li>Generic helpers for creating TB3+ tests</li>
- * <li>Automatic URL generation based on needed features, e.g.
- * {@link #isDebug()}, {@link #isPushEnabled()}</li>
  * </ul>
  * 
  * @author Vaadin Ltd
@@ -69,6 +68,10 @@ public abstract class AbstractTB3Test extends TestBenchTestCase {
     private static final int SCREENSHOT_WIDTH = 1500;
 
     private DesiredCapabilities desiredCapabilities;
+
+    private boolean debug = false;
+
+    private boolean push = false;
     {
         // Default browser to run on unless setDesiredCapabilities is called
         desiredCapabilities = BrowserUtil.firefox(24);
@@ -83,11 +86,6 @@ public abstract class AbstractTB3Test extends TestBenchTestCase {
     @Before
     public void setup() throws Exception {
         setupDriver();
-
-        String testUrl = getTestUrl();
-        if (testUrl != null) {
-            driver.get(testUrl);
-        }
     }
 
     /**
@@ -125,9 +123,20 @@ public abstract class AbstractTB3Test extends TestBenchTestCase {
     }
 
     /**
-     * Returns the full URL to be opened when the test starts.
+     * Opens the given test (defined by {@link #getTestUrl(boolean, boolean)},
+     * optionally with debug window and/or push
+     * 
+     * @param debug
+     * @param push
+     */
+    protected void openTestURL() {
+        driver.get(getTestUrl());
+    }
+
+    /**
+     * Returns the full URL to be used for the test
      * 
-     * @return the full URL to open or null to not open any URL automatically
+     * @return the full URL for the test
      */
     protected String getTestUrl() {
         String baseUrl = getBaseURL();
@@ -292,7 +301,12 @@ public abstract class AbstractTB3Test extends TestBenchTestCase {
      * Returns the path that should be used for the test. The path contains the
      * full path (appended to hostname+port) and must start with a slash.
      * 
-     * @return The path to open automatically when the test starts
+     * @param push
+     *            true if "?debug" should be added
+     * @param debug
+     *            true if /run-push should be used instead of /run
+     * 
+     * @return The URL path to the UI class to test
      */
     protected String getDeploymentPath() {
         Class<?> uiClass = getUIClass();
@@ -325,24 +339,47 @@ public abstract class AbstractTB3Test extends TestBenchTestCase {
     }
 
     /**
-     * Determines whether to run the test in debug mode (with the debug console
+     * Returns whether to run the test in debug mode (with the debug console
      * open) or not
      * 
      * @return true to run with the debug window open, false by default
      */
-    protected boolean isDebug() {
-        return false;
+    protected final boolean isDebug() {
+        return debug;
     }
 
     /**
-     * Determines whether to run the test with push enabled (using /run-push) or
+     * Sets whether to run the test in debug mode (with the debug console open)
+     * or not.
+     * 
+     * @param debug
+     *            true to open debug window, false otherwise
+     */
+    protected final void setDebug(boolean debug) {
+        this.debug = debug;
+    }
+
+    /**
+     * Returns whether to run the test with push enabled (using /run-push) or
      * not. Note that push tests can and should typically be created using @Push
      * on the UI instead of overriding this method
      * 
-     * @return true to use push in the test, false to use whatever UI specifies
+     * @return true if /run-push is used, false otherwise
+     */
+    protected final boolean isPush() {
+        return push;
+    }
+
+    /**
+     * Sets whether to run the test with push enabled (using /run-push) or not.
+     * Note that push tests can and should typically be created using @Push on
+     * the UI instead of overriding this method
+     * 
+     * @param push
+     *            true to use /run-push in the test, false otherwise
      */
-    protected boolean isPushEnabled() {
-        return false;
+    protected final void setPush(boolean push) {
+        this.push = push;
     }
 
     /**
@@ -350,15 +387,19 @@ public abstract class AbstractTB3Test extends TestBenchTestCase {
      * The path contains the full path (appended to hostname+port) and must
      * start with a slash.
      * 
-     * This method takes into account {@link #isPushEnabled()} and
-     * {@link #isDebug()} when the path is generated.
+     * This method takes into account {@link #isPush()} and {@link #isDebug()}
+     * when the path is generated.
      * 
      * @param uiClass
+     * @param push
+     *            true if "?debug" should be added
+     * @param debug
+     *            true if /run-push should be used instead of /run
      * @return The path to the given UI class
      */
     private String getDeploymentPath(Class<?> uiClass) {
         String runPath = "/run";
-        if (isPushEnabled()) {
+        if (isPush()) {
             runPath = "/run-push";
         }
 
-- 
2.39.5