fix: Fixed #227 - issue with resetImplicitTimeout

serenity-bdd · May 15, 2016 · 32b0850 · 32b0850
1 parent f679ad0
commit 32b0850
Show file tree

Hide file tree

Showing 7 changed files with 87 additions and 25 deletions.
diff --git a/serenity-core/src/main/java/net/serenitybdd/core/pages/PageObject.java b/serenity-core/src/main/java/net/serenitybdd/core/pages/PageObject.java
@@ -89,23 +89,22 @@ public void setImplicitTimeout(int duration, TimeUnit unit) {
         setDriverImplicitTimeout(waitForElementTimeout);
     }
 
-    public void resetImplicitTimeout() {
+    private void setDriverImplicitTimeout(Duration implicitTimeout) {
         if (driver instanceof ConfigurableTimeouts) {
-            ((ConfigurableTimeouts) driver).resetTimeouts();
+            ((ConfigurableTimeouts) driver).setImplicitTimeout(implicitTimeout);
         } else {
-            Duration defaultImplicitTimeout = getDefaultImplicitTimeout();
-            driver.manage().timeouts().implicitlyWait(defaultImplicitTimeout.in(MILLISECONDS), MILLISECONDS);
+            driver.manage().timeouts().implicitlyWait(implicitTimeout.in(MILLISECONDS), MILLISECONDS);
         }
     }
 
-    private void setDriverImplicitTimeout(Duration implicitTimeout) {
+    public void resetImplicitTimeout() {
         if (driver instanceof ConfigurableTimeouts) {
-            ((ConfigurableTimeouts) driver).setImplicitTimeout(implicitTimeout);
+            waitForElementTimeout = ((ConfigurableTimeouts) driver).resetTimeouts();
         } else {
-            driver.manage().timeouts().implicitlyWait(implicitTimeout.in(MILLISECONDS), MILLISECONDS);
+            waitForElementTimeout = getDefaultImplicitTimeout();
+            driver.manage().timeouts().implicitlyWait(waitForElementTimeout.in(MILLISECONDS), MILLISECONDS);
         }
     }
-
     private Duration getDefaultImplicitTimeout() {
         Integer configuredTimeout = ThucydidesSystemProperty.WEBDRIVER_TIMEOUTS_IMPLICITLYWAIT.integerFrom(environmentVariables);
         return new Duration(configuredTimeout, TimeUnit.MILLISECONDS);

diff --git a/serenity-core/src/main/java/net/serenitybdd/core/pages/WebElementFacadeImpl.java b/serenity-core/src/main/java/net/serenitybdd/core/pages/WebElementFacadeImpl.java
@@ -475,10 +475,11 @@ public Duration getCurrentImplicitTimeout() {
     }
 
     @Override
-    public void resetTimeouts() {
+    public Duration resetTimeouts() {
         if (driver instanceof ConfigurableTimeouts) {
-            ((ConfigurableTimeouts) driver).resetTimeouts();
+            return ((ConfigurableTimeouts) driver).resetTimeouts();
         }
+        return DefaultTimeouts.DEFAULT_IMPLICIT_WAIT_TIMEOUT;
     }
 
     public String getFoundBy() {

diff --git a/serenity-core/src/main/java/net/thucydides/core/webdriver/ConfigurableTimeouts.java b/serenity-core/src/main/java/net/thucydides/core/webdriver/ConfigurableTimeouts.java
@@ -11,5 +11,5 @@ public interface ConfigurableTimeouts {
 
     Duration getCurrentImplicitTimeout();
 
-    public void resetTimeouts();
+    public Duration resetTimeouts();
 }
diff --git a/serenity-core/src/main/java/net/thucydides/core/webdriver/WebDriverFacade.java b/serenity-core/src/main/java/net/thucydides/core/webdriver/WebDriverFacade.java
@@ -261,8 +261,8 @@ public Duration getCurrentImplicitTimeout() {
         return webDriverFactory.currentTimeoutFor(getProxiedDriver());
     }
 
-    public void resetTimeouts() {
-        webDriverFactory.resetTimeouts(getProxiedDriver());
+    public Duration resetTimeouts() {
+        return webDriverFactory.resetTimeouts(getProxiedDriver());
     }
 
 

diff --git a/serenity-core/src/main/java/net/thucydides/core/webdriver/WebDriverFactory.java b/serenity-core/src/main/java/net/thucydides/core/webdriver/WebDriverFactory.java
@@ -778,20 +778,23 @@ public Duration currentTimeoutFor(WebDriver proxiedDriver) {
         return storedTimeoutValue.or(getDefaultImplicitTimeout());
     }
 
-    public void resetTimeouts(WebDriver proxiedDriver) {
+    public Duration resetTimeouts(WebDriver proxiedDriver) {
         Duration currentTimeout = currentTimeoutFor(proxiedDriver);
-        if (timeoutStack.containsTimeoutFor(proxiedDriver)) {
-            timeoutStack.popTimeoutFor(proxiedDriver);
-            Duration previousTimeout = currentTimeoutFor(proxiedDriver);//timeoutStack.popTimeoutFor(proxiedDriver).or(getDefaultImplicitTimeout());
-            if ((currentTimeout != previousTimeout)  && isNotAMocked(proxiedDriver)) {
-                proxiedDriver.manage().timeouts().implicitlyWait(previousTimeout.in(TimeUnit.MILLISECONDS), TimeUnit.MILLISECONDS);
-            }
+        if (!timeoutStack.containsTimeoutFor(proxiedDriver)) {
+            return currentTimeout;
+        }
+
+        timeoutStack.popTimeoutFor(proxiedDriver);
+        Duration previousTimeout = currentTimeoutFor(proxiedDriver);//timeoutStack.popTimeoutFor(proxiedDriver).or(getDefaultImplicitTimeout());
+        if ((currentTimeout != previousTimeout)  && isNotAMocked(proxiedDriver)) {
+            proxiedDriver.manage().timeouts().implicitlyWait(previousTimeout.in(TimeUnit.MILLISECONDS), TimeUnit.MILLISECONDS);
         }
+        return previousTimeout;
     }
 
     public Duration getDefaultImplicitTimeout() {
-        Integer configuredTimeout = ThucydidesSystemProperty.WEBDRIVER_TIMEOUTS_IMPLICITLYWAIT.integerFrom(environmentVariables);
-        return (configuredTimeout != null) ? new Duration(configuredTimeout, TimeUnit.MILLISECONDS)
+        String configuredTimeoutValue = ThucydidesSystemProperty.WEBDRIVER_TIMEOUTS_IMPLICITLYWAIT.from(environmentVariables);
+        return (configuredTimeoutValue != null) ? new Duration(Integer.valueOf(configuredTimeoutValue), TimeUnit.MILLISECONDS)
                                            : DefaultTimeouts.DEFAULT_IMPLICIT_WAIT_TIMEOUT;
 
     }

diff --git a/serenity-core/src/main/java/net/thucydides/core/webdriver/stubs/WebElementFacadeStub.java b/serenity-core/src/main/java/net/thucydides/core/webdriver/stubs/WebElementFacadeStub.java
@@ -558,8 +558,8 @@ public Duration getCurrentImplicitTimeout() {
     }
 
     @Override
-    public void resetTimeouts() {
-
+    public Duration resetTimeouts() {
+        return null;
     }
 
     @Override

diff --git a/.../src/test/groovy/net/serenitybdd/core/webdriver/WhenKeepingTrackOfImplicitTimeouts.groovy b/.../src/test/groovy/net/serenitybdd/core/webdriver/WhenKeepingTrackOfImplicitTimeouts.groovy
@@ -1,13 +1,17 @@
 package net.serenitybdd.core.webdriver
 
 import com.google.common.base.Optional
+import net.serenitybdd.core.pages.PageObject
+import net.thucydides.core.annotations.DefaultUrl
 import net.thucydides.core.webdriver.TimeoutStack
+import net.thucydides.core.webdriver.WebDriverFacade
+import net.thucydides.core.webdriver.WebDriverFactory
 import org.openqa.selenium.WebDriver
+import org.openqa.selenium.htmlunit.HtmlUnitDriver
 import org.openqa.selenium.support.ui.Duration
 import spock.lang.Specification
 
 import java.util.concurrent.TimeUnit
-
 /**
  * Created by john on 12/03/15.
  */
@@ -106,5 +110,60 @@ class WhenKeepingTrackOfImplicitTimeouts extends Specification{
             storedDuration2.get() == timeout1
     }
 
+    @DefaultUrl("classpath:static-site/index.html")
+    static class PageObjectUsingImplicitTimeouts extends PageObject {
+
+        PageObjectUsingImplicitTimeouts(WebDriver driver) {
+            super(driver)
+        }
+
+        public void setImplicitTimeoutTo(int timeout) {
+            setImplicitTimeout(timeout, TimeUnit.SECONDS);
+        }
+
+        public long getImplicitTimoutMilliseconds() {
+            return implicitTimoutMilliseconds();
+        }
+    }
+
+    def "should be able to set the implicit timeout"() {
+        given:
+            def driver = new WebDriverFacade(HtmlUnitDriver, new WebDriverFactory());
+            def pageObject = new PageObjectUsingImplicitTimeouts(driver)
+        when:
+            pageObject.setImplicitTimeoutTo(3)
+        then:
+            pageObject.getImplicitTimoutMilliseconds() == 3000
+    }
+
+    def "should be able to reset the implicit timeout"() {
+        given:
+            def driver = new WebDriverFacade(HtmlUnitDriver, new WebDriverFactory());
+            def pageObject = new PageObjectUsingImplicitTimeouts(driver)
+            def originalTimeout = pageObject.getImplicitTimoutMilliseconds()
+        when:
+            pageObject.setImplicitTimeoutTo(12)
+        and:
+            pageObject.resetImplicitTimeout()
+        then:
+            pageObject.getImplicitTimoutMilliseconds() == originalTimeout
+    }
+
+    def "should be able to set and reset the implicit timeout using nested calls"() {
+        given:
+            def driver = new WebDriverFacade(HtmlUnitDriver, new WebDriverFactory());
+            def pageObject = new PageObjectUsingImplicitTimeouts(driver)
+            def originalTimeout = pageObject.getImplicitTimoutMilliseconds()
+        when:
+            pageObject.setImplicitTimeoutTo(6)
+            pageObject.setImplicitTimeoutTo(12)
+        then:
+            pageObject.getImplicitTimoutMilliseconds() == 12000
+            pageObject.resetImplicitTimeout()
+            pageObject.getImplicitTimoutMilliseconds() == 6000
+            pageObject.resetImplicitTimeout()
+            pageObject.getImplicitTimoutMilliseconds() == originalTimeout
+    }
+
 }