Use WebDriverWait on some flaky tests

Author: duanemay

uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/SessionControllerIntegrationTests.java

@@ -43,8 +43,11 @@ void sessionPageHasTheFunction() {
                 "/session?clientId=admin&messageOrigin=http://localhost:8080");
 
         WebDriverWait wait = webDriver.createWebDriverWait();
-        Object type = wait.until(driver -> webDriver.getJavascriptExecutor().executeScript(
-                "return typeof(handleMessage);"));
+        Object type = wait.until(driver -> {
+            Object t = webDriver.getJavascriptExecutor().executeScript(
+                    "return typeof(handleMessage);");
+            return "function".equals(String.valueOf(t)) ? t : null;
+        });
 
         assertThat(type).hasToString("function");
     }

uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/HomeIT.java

@@ -89,6 +89,8 @@ void profilePage() {
         } catch (TimeoutException e) {
             webDriver.get(baseUrl + "/profile");
         }
+        WebDriverWait wait = webDriver.createWebDriverWait();
+        wait.until(ExpectedConditions.visibilityOfElementLocated(By.cssSelector("h1")));
         assertThat(webDriver.findElement(By.cssSelector("h1")).getText()).contains("Account Settings");
     }
 

uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/LoginIT.java

@@ -30,6 +30,8 @@
 import org.openqa.selenium.By;
 import org.openqa.selenium.JavascriptExecutor;
 import org.openqa.selenium.WebElement;
+import org.openqa.selenium.support.ui.ExpectedConditions;
+import org.openqa.selenium.support.ui.WebDriverWait;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.http.HttpEntity;
@@ -252,14 +254,15 @@ void passcodeRedirect() {
 
         attemptLogin(testAccounts.getUserName(), testAccounts.getPassword());
 
+        WebDriverWait wait = webDriver.createWebDriverWait();
+        wait.until(ExpectedConditions.visibilityOfElementLocated(By.cssSelector("h1")));
         assertThat(webDriver.findElement(By.cssSelector("h1")).getText()).contains("Temporary Authentication Code");
 
-        // Verify that the CopyToClipboard function can be executed
+        wait.until(ExpectedConditions.visibilityOfElementLocated(By.id("passcode")));
         String passcode = webDriver.findElement(By.id("passcode")).getText();
         (webDriver.getJavascriptExecutor()).executeScript("CopyToClipboard",
                 passcode);
-        // Verify that the copybutton can be clicked
-        webDriver.findElement(By.id("copybutton")).click();
+        wait.until(ExpectedConditions.elementToBeClickable(By.id("copybutton"))).click();
     }
 
     @Test
@@ -335,6 +338,8 @@ void redirectAfterUnsuccessfulLogin() {
     @Test
     void loginPageReloadBasedOnCsrf() {
         webDriver.get(baseUrl + "/login");
+        webDriver.createWebDriverWait()
+                .until(driver -> driver.getPageSource().contains("http-equiv=\"refresh\""));
         assertThat(webDriver.getPageSource()).contains("http-equiv=\"refresh\"");
     }
 

uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/RateLimitingIT.java

@@ -68,28 +68,33 @@ void logout_and_clear_cookies() {
     @Test
     void infoEndpointRateLimited() throws InterruptedException {
         RestOperations restTemplate = serverRunning.getRestTemplate();
-        //One Request should pass
+
+        // Wait for a fresh rate-limit window to avoid interference from prior requests
+        TimeUnit.SECONDS.sleep(2);
+
         ResponseEntity<String> response = restTemplate.getForEntity(baseUrl + "/info", String.class);
         assertThat(response.getStatusCode()).isNotEqualTo(HttpStatus.TOO_MANY_REQUESTS);
-        boolean rateLimited = false;
+
         int infoLimit = 20;
         int requestCount = 50;
-        //Limit on /info is set to 20
+        int tolerance = 5;
+
         List<ResponseEntity> responses = new ArrayList<>(requestCount);
-        //Many Requests should hit the RL
         IntStream.range(0, requestCount).forEach(x -> responses.add(restTemplate.getForEntity(baseUrl + "/info", String.class)));
-        //Check numbers
+
         long limits = responses.stream().filter(s -> HttpStatus.TOO_MANY_REQUESTS.equals(s.getStatusCode())).count();
         long oKs = responses.stream().filter(s -> HttpStatus.OK.equals(s.getStatusCode())).count();
         assertThat(limits + oKs).isEqualTo(requestCount);
-        //Expect limited count around expected ones, more limited then with OK and check with tolerance of 2 that only expected limits are done
-        if (limits > oKs && limits > (infoLimit - 2) && limits < (requestCount - infoLimit + 2)) {
-            rateLimited = true;
-        }
-        assertThat(rateLimited).as("Rate limit counters are not as expected. Request: " + requestCount + ", Limit: " + infoLimit + ", blocked: " + limits
-                + ", allowed: " + oKs).isTrue();
-        //After 1s, New Limit should be available
-        TimeUnit.SECONDS.sleep(1);
+
+        assertThat(limits)
+                .as("Rate limit counters are not as expected. Request: %d, Limit: %d, blocked: %d, allowed: %d",
+                        requestCount, infoLimit, limits, oKs)
+                .isGreaterThan(oKs)
+                .isGreaterThanOrEqualTo(infoLimit - tolerance)
+                .isLessThanOrEqualTo(requestCount - infoLimit + tolerance);
+
+        // After the window resets, a new request should pass
+        TimeUnit.SECONDS.sleep(2);
         response = restTemplate.getForEntity(baseUrl + "/info", String.class);
         assertThat(response.getStatusCode()).isNotEqualTo(HttpStatus.TOO_MANY_REQUESTS);
     }