From 5db8cbc994b081cee955bf6001b3d4030bf309f6 Mon Sep 17 00:00:00 2001 From: Weimin Yu Date: Mon, 31 Aug 2020 15:09:54 -0400 Subject: [PATCH] Fix flaky web driver tests (#784) * Fix flaky web driver tests Identified two flaky tests in RegistrarConsoleScreenshotTest through local testing and fixed them by waiting for specific web elements instead of using fixed delays. Refactored the wait methods to support different test scenarios, and removed unnecessary delays. Extensively tested locally. Also ran multiple presubmits on Kokoro. --- .../OteSetupConsoleScreenshotTest.java | 10 +- .../RegistrarConsoleScreenshotTest.java | 124 +++++++++--------- .../webdriver/RegistrarConsoleWebTest.java | 8 +- .../RegistrarCreateConsoleScreenshotTest.java | 10 +- .../WebDriverPlusScreenDifferExtension.java | 57 +++++--- 5 files changed, 111 insertions(+), 98 deletions(-) diff --git a/core/src/test/java/google/registry/webdriver/OteSetupConsoleScreenshotTest.java b/core/src/test/java/google/registry/webdriver/OteSetupConsoleScreenshotTest.java index d69ef1f2c..6f03c0ac0 100644 --- a/core/src/test/java/google/registry/webdriver/OteSetupConsoleScreenshotTest.java +++ b/core/src/test/java/google/registry/webdriver/OteSetupConsoleScreenshotTest.java @@ -41,7 +41,7 @@ public class OteSetupConsoleScreenshotTest extends WebDriverTestCase { @RetryingTest(3) void get_owner_fails() throws Throwable { driver.get(server.getUrl("/registrar-ote-setup")); - driver.waitForElement(By.tagName("h1")); + driver.waitForDisplayedElement(By.tagName("h1")); driver.diffPage("unauthorized"); } @@ -49,14 +49,14 @@ public class OteSetupConsoleScreenshotTest extends WebDriverTestCase { void get_admin_succeeds() throws Throwable { server.setIsAdmin(true); driver.get(server.getUrl("/registrar-ote-setup")); - driver.waitForElement(By.tagName("h1")); + driver.waitForDisplayedElement(By.tagName("h1")); driver.diffPage("formEmpty"); driver.findElement(By.id("clientId")).sendKeys("acmereg"); driver.findElement(By.id("email")).sendKeys("acmereg@registry.example"); driver.findElement(By.id("password")).sendKeys("StRoNgPaSsWoRd"); driver.diffPage("formFilled"); driver.findElement(By.id("submit-button")).click(); - driver.waitForElement(By.tagName("h1")); + driver.waitForDisplayedElement(By.tagName("h1")); driver.diffPage("oteResult"); } @@ -64,11 +64,11 @@ public class OteSetupConsoleScreenshotTest extends WebDriverTestCase { void get_admin_fails_badEmail() throws Throwable { server.setIsAdmin(true); driver.get(server.getUrl("/registrar-ote-setup")); - driver.waitForElement(By.tagName("h1")); + driver.waitForDisplayedElement(By.tagName("h1")); driver.findElement(By.id("clientId")).sendKeys("acmereg"); driver.findElement(By.id("email")).sendKeys("bad email"); driver.findElement(By.id("submit-button")).click(); - driver.waitForElement(By.tagName("h1")); + driver.waitForDisplayedElement(By.tagName("h1")); driver.diffPage("oteResultFailed"); } } diff --git a/core/src/test/java/google/registry/webdriver/RegistrarConsoleScreenshotTest.java b/core/src/test/java/google/registry/webdriver/RegistrarConsoleScreenshotTest.java index e85cf7f65..37cffa369 100644 --- a/core/src/test/java/google/registry/webdriver/RegistrarConsoleScreenshotTest.java +++ b/core/src/test/java/google/registry/webdriver/RegistrarConsoleScreenshotTest.java @@ -68,7 +68,7 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase { @RetryingTest(3) void index_owner() throws Throwable { driver.get(server.getUrl("/registrar")); - driver.waitForElement(By.tagName("h1")); + driver.waitForDisplayedElement(By.tagName("h1")); driver.diffPage("page"); } @@ -77,7 +77,7 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase { void index_adminAndOwner() throws Throwable { server.setIsAdmin(true); driver.get(server.getUrl("/registrar")); - driver.waitForElement(By.tagName("h1")); + driver.waitForDisplayedElement(By.tagName("h1")); driver.diffPage("page"); } @@ -88,22 +88,21 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase { // To make sure we're only ADMIN (and not also "OWNER"), we switch to the NewRegistrar we // aren't in the contacts of driver.get(server.getUrl("/registrar?clientId=NewRegistrar")); - driver.waitForElement(By.tagName("h1")); + driver.waitForDisplayedElement(By.tagName("h1")); driver.diffPage("page"); } @RetryingTest(3) void contactUs() throws Throwable { driver.get(server.getUrl("/registrar#contact-us")); - driver.waitForElement(By.tagName("h1")); + driver.waitForDisplayedElement(By.tagName("h1")); driver.diffPage("page"); } @RetryingTest(3) void settingsContact() throws Throwable { driver.get(server.getUrl("/registrar#contact-settings")); - Thread.sleep(1000); - driver.waitForElement(By.tagName("h1")); + driver.waitForDisplayedElement(By.tagName("h1")); driver.diffPage("page"); } @@ -112,16 +111,14 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase { void settingsContact_asAdmin() throws Throwable { server.setIsAdmin(true); driver.get(server.getUrl("/registrar?clientId=NewRegistrar#contact-settings")); - Thread.sleep(1000); - driver.waitForElement(By.tagName("h1")); + driver.waitForDisplayedElement(By.tagName("h1")); driver.diffPage("page"); } @RetryingTest(3) void settingsContactItem() throws Throwable { driver.get(server.getUrl("/registrar#contact-settings/johndoe@theregistrar.com")); - Thread.sleep(1000); - driver.waitForElement(By.tagName("h1")); + driver.waitForDisplayedElement(By.tagName("h1")); driver.diffPage("page"); } @@ -132,8 +129,7 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase { driver.get( server.getUrl( "/registrar?clientId=NewRegistrar#contact-settings/janedoe@theregistrar.com")); - Thread.sleep(1000); - driver.waitForElement(By.tagName("h1")); + driver.waitForDisplayedElement(By.tagName("h1")); driver.diffPage("page"); } @@ -141,9 +137,8 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase { void settingsContactEdit() throws Throwable { driver.manage().window().setSize(new Dimension(1050, 2000)); driver.get(server.getUrl("/registrar#contact-settings/johndoe@theregistrar.com")); - Thread.sleep(1000); - driver.waitForElement(By.tagName("h1")); - driver.waitForElement(By.id("reg-app-btn-edit")).click(); + driver.waitForDisplayedElement(By.tagName("h1")); + driver.waitForDisplayedElement(By.id("reg-app-btn-edit")).click(); driver.diffPage("page"); } @@ -162,9 +157,8 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase { }); driver.manage().window().setSize(new Dimension(1050, 2000)); driver.get(server.getUrl("/registrar#contact-settings/johndoe@theregistrar.com")); - Thread.sleep(1000); - driver.waitForElement(By.tagName("h1")); - driver.waitForElement(By.id("reg-app-btn-edit")).click(); + driver.waitForDisplayedElement(By.tagName("h1")); + driver.waitForDisplayedElement(By.id("reg-app-btn-edit")).click(); // The password should show as dots when the user types it in driver.findElement(By.id("contacts[1].registryLockPassword")).sendKeys("password"); driver.diffPage("page_with_password"); @@ -179,9 +173,10 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase { Thread.sleep(5); driver.diffPage("page_with_password_after_hide"); - // now actually set the password - driver.waitForElement(By.id("reg-app-btn-save")).click(); - Thread.sleep(500); + // Now click the Save button and wait for another Edit button to show up + driver.waitForRefreshedElementAfterAction( + () -> driver.waitForDisplayedElement(By.id("reg-app-btn-save")).click(), + By.id("reg-app-btn-edit")); driver.diffPage("contact_view"); server.runInAppEngineEnvironment( @@ -213,9 +208,8 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase { }); driver.manage().window().setSize(new Dimension(1050, 2000)); driver.get(server.getUrl("/registrar#contact-settings/johndoe@theregistrar.com")); - Thread.sleep(1000); - driver.waitForElement(By.tagName("h1")); - driver.waitForElement(By.id("reg-app-btn-edit")).click(); + driver.waitForDisplayedElement(By.tagName("h1")); + driver.waitForDisplayedElement(By.id("reg-app-btn-edit")).click(); driver.diffPage("page"); } @@ -225,9 +219,8 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase { () -> persistResource(makeRegistrar2().asBuilder().setRegistryLockAllowed(true).build())); driver.manage().window().setSize(new Dimension(1050, 2000)); driver.get(server.getUrl("/registrar#contact-settings/johndoe@theregistrar.com")); - Thread.sleep(1000); - driver.waitForElement(By.tagName("h1")); - driver.waitForElement(By.id("reg-app-btn-edit")).click(); + driver.waitForDisplayedElement(By.tagName("h1")); + driver.waitForDisplayedElement(By.id("reg-app-btn-edit")).click(); driver.diffPage("page"); } @@ -235,9 +228,8 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase { void settingsContactAdd() throws Throwable { driver.manage().window().setSize(new Dimension(1050, 2000)); driver.get(server.getUrl("/registrar#contact-settings")); - Thread.sleep(1000); - driver.waitForElement(By.tagName("h1")); - driver.waitForElement(By.id("reg-app-btn-add")).click(); + driver.waitForDisplayedElement(By.tagName("h1")); + driver.waitForDisplayedElement(By.id("reg-app-btn-add")).click(); // Attempt to fix flaky tests. The going theory is that the click button CSS animation needs to // finish before the screenshot is captured. Thread.sleep(250); @@ -251,10 +243,10 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase { // To make sure we're only ADMIN (and not also "OWNER"), we switch to the NewRegistrar we // aren't in the contacts of driver.get(server.getUrl("/registrar?clientId=NewRegistrar#admin-settings")); - driver.waitForElement(By.tagName("h1")); + driver.waitForDisplayedElement(By.tagName("h1")); driver.diffPage("view"); - driver.waitForElement(By.id("reg-app-btn-edit")).click(); - driver.waitForElement(By.tagName("h1")); + driver.waitForDisplayedElement(By.id("reg-app-btn-edit")).click(); + driver.waitForDisplayedElement(By.tagName("h1")); driver.diffPage("edit"); } @@ -272,7 +264,7 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase { void settingsAdmin_whenNotAdmin_showsHome() throws Throwable { driver.manage().window().setSize(new Dimension(1050, 2000)); driver.get(server.getUrl("/registrar#admin-settings")); - driver.waitForElement(By.tagName("h1")); + driver.waitForDisplayedElement(By.tagName("h1")); driver.diffPage("view"); } @@ -280,7 +272,7 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase { void getOteStatus_noButtonWhenReal() throws Exception { server.setIsAdmin(true); driver.get(server.getUrl("/registrar#admin-settings")); - driver.waitForElement(By.tagName("h1")); + driver.waitForDisplayedElement(By.tagName("h1")); driver.diffPage("result"); } @@ -299,7 +291,7 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase { void getOteStatus_completed() throws Exception { server.setIsAdmin(true); driver.get(server.getUrl("/registrar?clientId=otefinished-1#admin-settings")); - driver.waitForElement(By.id("btn-ote-status")); + driver.waitForDisplayedElement(By.id("btn-ote-status")); driver.diffPage("before_click"); driver.findElement(By.id("btn-ote-status")).click(); driver.findElement(By.id("ote-results-table")).click(); @@ -312,10 +304,10 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase { void settingsSecurity() throws Throwable { driver.manage().window().setSize(new Dimension(1050, 2000)); driver.get(server.getUrl("/registrar#security-settings")); - driver.waitForElement(By.tagName("h1")); + driver.waitForDisplayedElement(By.tagName("h1")); driver.diffPage("view"); - driver.waitForElement(By.id("reg-app-btn-edit")).click(); - driver.waitForElement(By.tagName("h1")); + driver.waitForDisplayedElement(By.id("reg-app-btn-edit")).click(); + driver.waitForDisplayedElement(By.tagName("h1")); driver.diffPage("edit"); } @@ -325,7 +317,7 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase { server.setIsAdmin(true); driver.manage().window().setSize(new Dimension(1050, 2000)); driver.get(server.getUrl("/registrar?clientId=NewRegistrar#security-settings")); - driver.waitForElement(By.tagName("h1")); + driver.waitForDisplayedElement(By.tagName("h1")); driver.diffPage("view"); } @@ -343,10 +335,10 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase { }); driver.manage().window().setSize(new Dimension(1050, 2000)); driver.get(server.getUrl("/registrar#security-settings")); - driver.waitForElement(By.tagName("h1")); + driver.waitForDisplayedElement(By.tagName("h1")); driver.diffPage("view"); - driver.waitForElement(By.id("reg-app-btn-edit")).click(); - driver.waitForElement(By.tagName("h1")); + driver.waitForDisplayedElement(By.id("reg-app-btn-edit")).click(); + driver.waitForDisplayedElement(By.tagName("h1")); driver.diffPage("edit"); } @@ -363,10 +355,10 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase { }); driver.manage().window().setSize(new Dimension(1050, 2000)); driver.get(server.getUrl("/registrar#security-settings")); - driver.waitForElement(By.tagName("h1")); + driver.waitForDisplayedElement(By.tagName("h1")); driver.diffPage("view"); - driver.waitForElement(By.id("reg-app-btn-edit")).click(); - driver.waitForElement(By.tagName("h1")); + driver.waitForDisplayedElement(By.id("reg-app-btn-edit")).click(); + driver.waitForDisplayedElement(By.tagName("h1")); driver.diffPage("edit"); } @@ -377,14 +369,14 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase { persistResource( loadRegistrar("TheRegistrar").asBuilder().setState(State.DISABLED).build())); driver.get(server.getUrl("/registrar")); - driver.waitForElement(By.tagName("h1")); + driver.waitForDisplayedElement(By.tagName("h1")); driver.diffPage("view"); } @RetryingTest(3) void settingsWhois() throws Throwable { driver.get(server.getUrl("/registrar#whois-settings")); - driver.waitForElement(By.tagName("h1")); + driver.waitForDisplayedElement(By.tagName("h1")); driver.diffPage("page"); } @@ -392,8 +384,8 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase { void settingsWhoisEdit() throws Throwable { driver.manage().window().setSize(new Dimension(1050, 2000)); driver.get(server.getUrl("/registrar#whois-settings")); - driver.waitForElement(By.id("reg-app-btn-edit")).click(); - Thread.sleep(1000); + driver.waitForDisplayedElement(By.id("reg-app-btn-edit")).click(); + driver.waitForDisplayedElement(By.id("reg-app-btn-save")); driver.diffPage("page"); } @@ -401,10 +393,12 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase { void settingsWhoisEditError() throws Throwable { driver.manage().window().setSize(new Dimension(1050, 2000)); driver.get(server.getUrl("/registrar#whois-settings")); - driver.waitForElement(By.id("reg-app-btn-edit")).click(); + driver.waitForDisplayedElement(By.id("reg-app-btn-edit")).click(); driver.setFormFieldsById(ImmutableMap.of("faxNumber", "cat")); - driver.waitForElement(By.id("reg-app-btn-save")).click(); - Thread.sleep(1000); + driver.waitForDisplayedElement(By.id("reg-app-btn-save")).click(); + // After the click, a div element without id would show up with an error message. + driver.waitForElementWithCondition( + By.tagName("div"), e -> e.getText().startsWith("Must be a valid +E.164 phone number,")); driver.diffPage("page"); } @@ -412,7 +406,7 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase { void indexPage_smallScrolledDown() throws Throwable { driver.manage().window().setSize(new Dimension(600, 300)); driver.get(server.getUrl("/registrar")); - driver.waitForElement(By.tagName("h1")); + driver.waitForDisplayedElement(By.tagName("h1")); driver.executeScript("document.getElementById('reg-content-and-footer').scrollTop = 200"); Thread.sleep(500); driver.diffPage("page"); @@ -439,21 +433,21 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase { driver.get( server.getUrl( "/registry-lock-verify?isLock=true&lockVerificationCode=" + lockVerificationCode)); - driver.waitForElement(By.id("reg-content")); + driver.waitForDisplayedElement(By.id("reg-content")); driver.diffPage("page"); } @RetryingTest(3) void registryLockVerify_unknownLock() throws Throwable { driver.get(server.getUrl("/registry-lock-verify?isLock=true&lockVerificationCode=asdfasdf")); - driver.waitForElement(By.id("reg-content")); + driver.waitForDisplayedElement(By.id("reg-content")); driver.diffPage("page"); } @RetryingTest(3) void registryLock_empty() throws Throwable { driver.get(server.getUrl("/registrar?clientId=TheRegistrar#registry-lock")); - driver.waitForElement(By.tagName("h2")); + driver.waitForDisplayedElement(By.tagName("h2")); driver.diffPage("page"); } @@ -465,7 +459,7 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase { return null; }); driver.get(server.getUrl("/registrar?clientId=TheRegistrar#registry-lock")); - driver.waitForElement(By.tagName("h2")); + driver.waitForDisplayedElement(By.tagName("h2")); driver.diffPage("page"); } @@ -477,7 +471,7 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase { return null; }); driver.get(server.getUrl("/registrar#registry-lock")); - driver.waitForElement(By.tagName("h2")); + driver.waitForDisplayedElement(By.tagName("h2")); driver.diffPage("page"); } @@ -530,7 +524,7 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase { return null; }); driver.get(server.getUrl("/registrar#registry-lock")); - driver.waitForElement(By.tagName("h2")); + driver.waitForDisplayedElement(By.tagName("h2")); driver.diffPage("page"); } @@ -542,9 +536,9 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase { return null; }); driver.get(server.getUrl("/registrar#registry-lock")); - driver.waitForElement(By.tagName("h2")); + driver.waitForDisplayedElement(By.tagName("h2")); driver.findElement(By.id("button-unlock-example.tld")).click(); - driver.waitForElement(By.className("modal-content")); + driver.waitForDisplayedElement(By.className("modal-content")); driver.findElement(By.id("domain-lock-password")).sendKeys("password"); driver.diffPage("page"); } @@ -559,9 +553,9 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase { return null; }); driver.get(server.getUrl("/registrar#registry-lock")); - driver.waitForElement(By.tagName("h2")); + driver.waitForDisplayedElement(By.tagName("h2")); driver.findElement(By.id("button-lock-domain")).click(); - driver.waitForElement(By.className("modal-content")); + driver.waitForDisplayedElement(By.className("modal-content")); driver.findElement(By.id("domain-lock-input-value")).sendKeys("somedomain.tld"); driver.diffPage("page"); } @@ -578,7 +572,7 @@ class RegistrarConsoleScreenshotTest extends WebDriverTestCase { return null; }); driver.get(server.getUrl("/registrar?clientId=TheRegistrar#registry-lock")); - driver.waitForElement(By.tagName("h2")); + driver.waitForDisplayedElement(By.tagName("h2")); driver.diffPage("page"); } diff --git a/core/src/test/java/google/registry/webdriver/RegistrarConsoleWebTest.java b/core/src/test/java/google/registry/webdriver/RegistrarConsoleWebTest.java index 23bba0f1e..7e7267158 100644 --- a/core/src/test/java/google/registry/webdriver/RegistrarConsoleWebTest.java +++ b/core/src/test/java/google/registry/webdriver/RegistrarConsoleWebTest.java @@ -55,7 +55,7 @@ public class RegistrarConsoleWebTest extends WebDriverTestCase { /** Checks that an element is visible. */ void assertEltVisible(String eltId) throws Throwable { - assertThat(driver.waitForElement(By.id(eltId)).isDisplayed()).isTrue(); + assertThat(driver.waitForDisplayedElement(By.id(eltId)).isDisplayed()).isTrue(); } /** Checks that an element is invisible. */ @@ -141,7 +141,7 @@ public class RegistrarConsoleWebTest extends WebDriverTestCase { @RetryingTest(3) void testWhoisSettingsEdit() throws Throwable { driver.get(server.getUrl("/registrar#whois-settings")); - driver.waitForElement(By.id("reg-app-btn-edit")).click(); + driver.waitForDisplayedElement(By.id("reg-app-btn-edit")).click(); driver.setFormFieldsById( new ImmutableMap.Builder() .put("emailAddress", "test1@example.com") @@ -178,7 +178,7 @@ public class RegistrarConsoleWebTest extends WebDriverTestCase { @RetryingTest(3) void testContactSettingsView() throws Throwable { driver.get(server.getUrl("/registrar#contact-settings")); - driver.waitForElement(By.id("reg-app-btn-add")); + driver.waitForDisplayedElement(By.id("reg-app-btn-add")); ImmutableList contacts = server.runInAppEngineEnvironment( () -> loadRegistrar("TheRegistrar").getContacts().asList()); @@ -192,7 +192,7 @@ public class RegistrarConsoleWebTest extends WebDriverTestCase { @RetryingTest(3) void testSecuritySettingsView() throws Throwable { driver.get(server.getUrl("/registrar#security-settings")); - driver.waitForElement(By.id("reg-app-btn-edit")); + driver.waitForDisplayedElement(By.id("reg-app-btn-edit")); Registrar registrar = server.runInAppEngineEnvironment(() -> loadRegistrar("TheRegistrar")); assertThat(driver.findElement(By.id("phonePasscode")).getAttribute("value")) .isEqualTo(registrar.getPhonePasscode()); diff --git a/core/src/test/java/google/registry/webdriver/RegistrarCreateConsoleScreenshotTest.java b/core/src/test/java/google/registry/webdriver/RegistrarCreateConsoleScreenshotTest.java index ae9dd1a61..1c59cff01 100644 --- a/core/src/test/java/google/registry/webdriver/RegistrarCreateConsoleScreenshotTest.java +++ b/core/src/test/java/google/registry/webdriver/RegistrarCreateConsoleScreenshotTest.java @@ -41,7 +41,7 @@ class RegistrarCreateConsoleScreenshotTest extends WebDriverTestCase { @RetryingTest(3) void get_owner_fails() throws Throwable { driver.get(server.getUrl("/registrar-create")); - driver.waitForElement(By.tagName("h1")); + driver.waitForDisplayedElement(By.tagName("h1")); driver.diffPage("unauthorized"); } @@ -49,7 +49,7 @@ class RegistrarCreateConsoleScreenshotTest extends WebDriverTestCase { void get_admin_succeeds() throws Throwable { server.setIsAdmin(true); driver.get(server.getUrl("/registrar-create")); - driver.waitForElement(By.tagName("h1")); + driver.waitForDisplayedElement(By.tagName("h1")); driver.diffPage("formEmpty"); driver.findElement(By.id("clientId")).sendKeys("my-name"); driver.findElement(By.id("name")).sendKeys("registrar name"); @@ -69,7 +69,7 @@ class RegistrarCreateConsoleScreenshotTest extends WebDriverTestCase { driver.findElement(By.id("passcode")).sendKeys("01234"); driver.diffPage("formFilled"); driver.findElement(By.id("submit-button")).click(); - driver.waitForElement(By.tagName("h1")); + driver.waitForDisplayedElement(By.tagName("h1")); driver.diffPage("createResult"); } @@ -77,7 +77,7 @@ class RegistrarCreateConsoleScreenshotTest extends WebDriverTestCase { void get_admin_fails_badEmail() throws Throwable { server.setIsAdmin(true); driver.get(server.getUrl("/registrar-create")); - driver.waitForElement(By.tagName("h1")); + driver.waitForDisplayedElement(By.tagName("h1")); driver.findElement(By.id("clientId")).sendKeys("my-name"); driver.findElement(By.id("name")).sendKeys("registrar name"); driver @@ -93,7 +93,7 @@ class RegistrarCreateConsoleScreenshotTest extends WebDriverTestCase { driver.findElement(By.id("city")).sendKeys("Citysville"); driver.findElement(By.id("countryCode")).sendKeys("fr"); driver.findElement(By.id("submit-button")).click(); - driver.waitForElement(By.tagName("h1")); + driver.waitForDisplayedElement(By.tagName("h1")); driver.diffPage("createResultFailed"); } } diff --git a/core/src/test/java/google/registry/webdriver/WebDriverPlusScreenDifferExtension.java b/core/src/test/java/google/registry/webdriver/WebDriverPlusScreenDifferExtension.java index 1f1636f97..289902139 100644 --- a/core/src/test/java/google/registry/webdriver/WebDriverPlusScreenDifferExtension.java +++ b/core/src/test/java/google/registry/webdriver/WebDriverPlusScreenDifferExtension.java @@ -19,9 +19,11 @@ import static java.util.stream.Collectors.joining; import static org.apache.commons.text.StringEscapeUtils.escapeEcmaScript; import com.google.common.base.Preconditions; +import com.google.common.base.Predicates; import java.net.URL; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.function.Predicate; @@ -34,6 +36,7 @@ import org.openqa.selenium.Dimension; import org.openqa.selenium.HasCapabilities; import org.openqa.selenium.JavascriptExecutor; import org.openqa.selenium.OutputType; +import org.openqa.selenium.StaleElementReferenceException; import org.openqa.selenium.TakesScreenshot; import org.openqa.selenium.WebDriver; import org.openqa.selenium.WebDriverException; @@ -55,7 +58,6 @@ public final class WebDriverPlusScreenDifferExtension HasCapabilities { private static final int WAIT_FOR_ELEMENTS_POLLING_INTERVAL_MS = 10; - private static final int WAIT_FOR_ELEMENTS_BONUS_DELAY_MS = 150; // The maximum difference between pixels that would be considered as "identical". Calculated as // the sum of the absolute difference between the values of the RGB channels. So a 120,30,200 @@ -121,28 +123,40 @@ public final class WebDriverPlusScreenDifferExtension driver.get(url.toString()); } - /** Waits indefinitely for an element to appear on the page, then returns it. */ - WebElement waitForElement(By by) throws InterruptedException { - while (true) { - List elements = findElements(by); - if (!elements.isEmpty()) { - Thread.sleep(WAIT_FOR_ELEMENTS_BONUS_DELAY_MS); - return elements.get(0); - } - Thread.sleep(WAIT_FOR_ELEMENTS_POLLING_INTERVAL_MS); - } + /** Waits indefinitely for a visible element matching {@code by}, then returns it. */ + WebElement waitForDisplayedElement(By by) throws InterruptedException { + return waitForElementWithCondition(by, WebElement::isDisplayed); } - /** Waits for element matching {@code by} whose {@code attribute} satisfies {@code predicate}. */ - public WebElement waitForAttribute( - By by, String attribute, Predicate predicate) + /** Waits indefinitely for an element matching {@code by}, then returns it. */ + WebElement waitForElement(By by) throws InterruptedException { + return waitForElementWithCondition(by, Predicates.alwaysTrue()); + } + + /** + * Executes an action and waits indefinitely for the replacement of an existing element. + */ + WebElement waitForRefreshedElementAfterAction(ThrowingRunnable action, By by) throws Exception { + WebElement element = findElement(by); + action.run(); + while (true) { + try { + element.isDisplayed(); // Eventually triggers StaleElementReferenceException + Thread.sleep(WAIT_FOR_ELEMENTS_POLLING_INTERVAL_MS); + } catch (StaleElementReferenceException e) { + break; + } + } + return waitForDisplayedElement(by); + } + + /** Waits for an element matching {@code by} and satisfying {@code predicate}, then returns it. */ + WebElement waitForElementWithCondition(By by, Predicate predicate) throws InterruptedException { while (true) { - for (WebElement element : findElements(by)) { - if (predicate.test(element.getAttribute(attribute))) { - Thread.sleep(WAIT_FOR_ELEMENTS_BONUS_DELAY_MS); - return element; - } + Optional firstMatch = findElements(by).stream().filter(predicate).findFirst(); + if (firstMatch.isPresent()) { + return firstMatch.get(); } Thread.sleep(WAIT_FOR_ELEMENTS_POLLING_INTERVAL_MS); } @@ -303,4 +317,9 @@ public final class WebDriverPlusScreenDifferExtension Preconditions.checkNotNull(imageNamePrefix); return imageNamePrefix + "_" + imageKey; } + + public interface ThrowingRunnable { + + void run() throws Exception; + } }