GEODE-5424: Changing all awaitility calls to use consistent timings

We have a lot of Awaitility calls in our tests. Each test was picking
its own timeout. That lead to some tests picking too small of a timeout
and failing spuriously.

With this change, all tests will use a new factory,
GeodeAwaility.await(), rather than Awaitility.await(). This new factory
sets a default timeout of 5 minutes. It also sets a sensible pollDelay
and pollInterval.

The custom timeouts used in all tests have been removed, in favor of
this new factory, except for a couple of tests that had waits greater
than 5 minutes.

geode-assembly/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/StopLocatorCommandDUnitTest.java

@@ -15,7 +15,6 @@
 package org.apache.geode.management.internal.cli.commands;
 
 import static org.apache.geode.distributed.internal.DistributionConfig.GEMFIRE_PREFIX;
-import static org.apache.geode.management.MXBeanAwaitility.await;
 import static org.apache.geode.management.cli.Result.Status.ERROR;
 import static org.apache.geode.management.cli.Result.Status.OK;
 import static org.apache.geode.management.internal.cli.i18n.CliStrings.GROUP;
@@ -35,7 +34,6 @@
 import java.io.IOException;
 import java.util.Properties;
 import java.util.Set;
-import java.util.concurrent.TimeUnit;
 
 import javax.management.MBeanServerConnection;
 import javax.management.ObjectName;
@@ -60,6 +58,7 @@
 import org.apache.geode.management.internal.cli.i18n.CliStrings;
 import org.apache.geode.management.internal.cli.result.CommandResult;
 import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
+import org.apache.geode.test.awaitility.GeodeAwaitility;
 import org.apache.geode.test.dunit.rules.ClusterStartupRule;
 import org.apache.geode.test.dunit.rules.MemberVM;
 import org.apache.geode.test.junit.rules.GfshCommandRule;
@@ -183,7 +182,7 @@ public void testWithMemberID() {
   }
 
   private void waitForCommandToSucceed(String command) {
-    await().atMost(5, TimeUnit.MINUTES).untilAsserted(() -> {
+    GeodeAwaitility.await().untilAsserted(() -> {
       CommandResult result = gfsh.executeCommand(command);
       assertThat(result.getStatus()).isEqualTo(OK);
     });
@@ -260,7 +259,7 @@ protected static String getMemberId(int port) throws Exception {
       final MBeanServerConnection connection = conn.getMBeanServerConnection();
       assertThat(connection).isInstanceOf(MBeanServerConnection.class);
 
-      await().untilAsserted(() -> {
+      GeodeAwaitility.await().untilAsserted(() -> {
         final Set<ObjectName> objectNames = connection.queryNames(objectNamePattern, query);
         assertThat(objectNames).isNotNull().isNotEmpty().hasSize(1);
       });

geode-assembly/src/distributedTest/java/org/apache/geode/session/tests/GenericAppServerClientServerTest.java

@@ -14,15 +14,14 @@
  */
 package org.apache.geode.session.tests;
 
+import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
 import static org.junit.Assert.assertEquals;
 
 import java.io.IOException;
 import java.net.URISyntaxException;
-import java.util.concurrent.TimeUnit;
 
 import javax.servlet.http.HttpSession;
 
-import org.awaitility.Awaitility;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -95,7 +94,7 @@ protected void verifySessionIsRemoved(String key) throws IOException, URISyntaxE
     serverVM.invoke(() -> {
       Cache cache = getCache();
       Region region = cache.getRegion("gemfire_modules_sessions");
-      Awaitility.await().atMost(1, TimeUnit.MINUTES)
+      await()
           .untilAsserted(() -> assertEquals(0, region.size()));
     });
     super.verifySessionIsRemoved(key);

geode-assembly/src/integrationTest/java/org/apache/geode/tools/pulse/PulseSecurityIntegrationTest.java

@@ -15,9 +15,8 @@
 
 package org.apache.geode.tools.pulse;
 
-import static java.util.concurrent.TimeUnit.MINUTES;
+import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.awaitility.Awaitility.await;
 
 import org.junit.Rule;
 import org.junit.Test;
@@ -48,7 +47,7 @@ public void getAttributesWithSecurityManager() throws Exception {
     ManagementService service =
         ManagementService.getExistingManagementService(locator.getLocator().getCache());
 
-    await().atMost(2, MINUTES)
+    await()
         .untilAsserted(() -> assertThat(service.getMemberMXBean()).isNotNull());
 
     Cluster cluster = pulse.getRepository().getCluster("cluster", "cluster");

geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcAsyncWriterIntegrationTest.java

@@ -15,15 +15,14 @@
 package org.apache.geode.connectors.jdbc;
 
 import static org.apache.geode.cache.RegionShortcut.REPLICATE;
+import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
 import static org.assertj.core.api.Assertions.assertThat;
 
 import java.sql.Connection;
 import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.sql.Statement;
-import java.util.concurrent.TimeUnit;
 
-import org.awaitility.Awaitility;
 import org.awaitility.core.ThrowingRunnable;
 import org.junit.After;
 import org.junit.Before;
@@ -209,7 +208,7 @@ public void canInsertBecomeUpdate() throws Exception {
   }
 
   private void awaitUntil(final ThrowingRunnable supplier) {
-    Awaitility.await().atMost(30, TimeUnit.SECONDS).untilAsserted(supplier);
+    await().untilAsserted(supplier);
   }
 
   private void assertRecordMatchesEmployee(ResultSet resultSet, String key, Employee employee)

geode-connectors/src/acceptanceTest/java/org/apache/geode/connectors/jdbc/JdbcDistributedTest.java

@@ -14,6 +14,7 @@
  */
 package org.apache.geode.connectors.jdbc;
 
+import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
@@ -28,9 +29,7 @@
 import java.sql.Statement;
 import java.util.Date;
 import java.util.Properties;
-import java.util.concurrent.TimeUnit;
 
-import org.awaitility.Awaitility;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Rule;
@@ -210,7 +209,7 @@ public void throwsExceptionWhenNoMappingExistsUsingAsyncWriter() throws Exceptio
 
       JdbcAsyncWriter asyncWriter = (JdbcAsyncWriter) ClusterStartupRule.getCache()
           .getAsyncEventQueue("JAW").getAsyncEventListener();
-      Awaitility.await().atMost(30, TimeUnit.SECONDS).untilAsserted(() -> {
+      await().untilAsserted(() -> {
         assertThat(asyncWriter.getFailedEvents()).isEqualTo(1);
       });
 
@@ -456,7 +455,7 @@ public void getReadsFromDBWithAsyncWriter() throws Exception {
           .getAsyncEventQueue("JAW").getAsyncEventListener();
 
       region.put(key, pdxEmployee1);
-      Awaitility.await().atMost(30, TimeUnit.SECONDS).untilAsserted(() -> {
+      await().untilAsserted(() -> {
         assertThat(asyncWriter.getSuccessfulEvents()).isEqualTo(1);
       });
       region.invalidate(key);
@@ -465,7 +464,7 @@ public void getReadsFromDBWithAsyncWriter() throws Exception {
       assertThat(result.getField("id")).isEqualTo(pdxEmployee1.getField("id"));
       assertThat(result.getField("name")).isEqualTo(pdxEmployee1.getField("name"));
       assertThat(result.getField("age")).isEqualTo(pdxEmployee1.getField("age"));
-      Awaitility.await().atMost(30, TimeUnit.SECONDS).untilAsserted(() -> {
+      await().untilAsserted(() -> {
         assertThat(asyncWriter.getIgnoredEvents()).isEqualTo(1);
       });
     });
@@ -657,7 +656,7 @@ private void assertTableHasEmployeeData(int size, PdxInstance employee, String k
       throws SQLException {
     Connection connection = DriverManager.getConnection(connectionUrl);
     Statement statement = connection.createStatement();
-    Awaitility.await().atMost(30, TimeUnit.SECONDS).untilAsserted(() -> {
+    await().untilAsserted(() -> {
       assertThat(getRowCount(statement, TABLE_NAME)).isEqualTo(size);
     });
 

geode-connectors/src/acceptanceTest/java/org/apache/geode/test/junit/rules/MySqlConnectionRule.java

@@ -14,15 +14,14 @@
  */
 package org.apache.geode.test.junit.rules;
 
+import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
 import static org.assertj.core.api.Assertions.assertThat;
 
 import java.sql.Connection;
 import java.sql.DriverManager;
 import java.sql.SQLException;
-import java.util.concurrent.TimeUnit;
 
 import com.palantir.docker.compose.DockerComposeRule;
-import org.awaitility.Awaitility;
 
 public class MySqlConnectionRule extends SqlDatabaseConnectionRule {
   private static final String CREATE_DB_CONNECTION_STRING =
@@ -38,7 +37,7 @@ protected MySqlConnectionRule(DockerComposeRule dockerRule, String serviceName,
 
   @Override
   public Connection getConnection() throws SQLException {
-    Awaitility.await().ignoreExceptions().atMost(10, TimeUnit.SECONDS)
+    await().ignoreExceptions()
         .untilAsserted(
             () -> assertThat(DriverManager.getConnection(getCreateDbConnectionUrl())).isNotNull());
     String dbName = getDbName();

geode-connectors/src/acceptanceTest/java/org/apache/geode/test/junit/rules/SqlDatabaseConnectionRule.java

@@ -14,18 +14,17 @@
  */
 package org.apache.geode.test.junit.rules;
 
+import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
 import static org.assertj.core.api.Assertions.assertThat;
 
 import java.io.IOException;
 import java.sql.Connection;
 import java.sql.DriverManager;
 import java.sql.SQLException;
-import java.util.concurrent.TimeUnit;
 
 import com.palantir.docker.compose.DockerComposeRule;
 import com.palantir.docker.compose.connection.DockerPort;
 import com.palantir.docker.compose.connection.waiting.HealthChecks;
-import org.awaitility.Awaitility;
 import org.junit.rules.ExternalResource;
 
 public abstract class SqlDatabaseConnectionRule extends ExternalResource
@@ -65,7 +64,7 @@ protected String getDbName() {
   @Override
   public Connection getConnection() throws SQLException {
     String connectionUrl = getConnectionUrl();
-    Awaitility.await().ignoreExceptions().atMost(10, TimeUnit.SECONDS)
+    await().ignoreExceptions()
         .untilAsserted(() -> assertThat(DriverManager.getConnection(connectionUrl)).isNotNull());
     Connection connection = DriverManager.getConnection(connectionUrl);
     return connection;

geode-core/src/distributedTest/java/org/apache/geode/cache/RegionExpirationDistributedTest.java

@@ -16,9 +16,9 @@
 
 import static java.util.concurrent.TimeUnit.MINUTES;
 import static org.apache.geode.cache.RegionShortcut.REPLICATE;
+import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
 import static org.apache.geode.test.dunit.VM.getVM;
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.awaitility.Awaitility.await;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.timeout;
@@ -162,32 +162,32 @@ void assertThatExpirationOccurredInLocalMember(final Cache cache, final String r
       if (expirationAction().isInvalidate() || expirationAction().isLocalInvalidate()) {
         // verify region was invalidated
         Region<String, String> region = cache.getRegion(regionName);
-        await().atMost(1, MINUTES)
+        await()
             .untilAsserted(() -> assertThat(region.containsValueForKey(KEY)).isFalse());
 
       } else {
         // verify region was destroyed (or is in process of being destroyed)
-        await().atMost(1, MINUTES).until(() -> isDestroyed(cache.getRegion(regionName)));
+        await().until(() -> isDestroyed(cache.getRegion(regionName)));
       }
     }
 
     void assertRegionStateInRemoteMember(final Cache cache, final String regionName) {
       if (expirationAction().isInvalidate()) {
         // distributed invalidate region
         Region<String, String> region = cache.getRegion(regionName);
-        await().atMost(1, MINUTES).untilAsserted(() -> {
+        await().untilAsserted(() -> {
           assertThat(region.containsKey(KEY)).isTrue();
           assertThat(region.containsValueForKey(KEY)).isFalse();
         });
 
       } else if (expirationAction().isDestroy()) {
         // verify region was destroyed (or is in process of being destroyed)
-        await().atMost(1, MINUTES).until(() -> isDestroyed(cache.getRegion(regionName)));
+        await().until(() -> isDestroyed(cache.getRegion(regionName)));
 
       } else {
         // for LOCAL_DESTROY or LOCAL_INVALIDATE, the value should be present
         Region<String, String> region = cache.getRegion(regionName);
-        await().atMost(1, MINUTES).untilAsserted(() -> {
+        await().untilAsserted(() -> {
           assertThat(region.containsValueForKey(KEY)).isTrue();
           assertThat(region.get(KEY)).isEqualTo(VALUE);
         });

geode-core/src/distributedTest/java/org/apache/geode/cache/client/internal/AutoConnectionSourceDUnitTest.java

@@ -14,8 +14,7 @@
  */
 package org.apache.geode.cache.client.internal;
 
-import static java.util.concurrent.TimeUnit.MILLISECONDS;
-import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
 import static org.apache.geode.test.dunit.NetworkUtils.getServerHostName;
 import static org.assertj.core.api.Assertions.assertThatCode;
 import static org.junit.Assert.assertEquals;
@@ -29,7 +28,6 @@
 import java.util.Iterator;
 import java.util.List;
 
-import org.awaitility.Awaitility;
 import org.awaitility.core.ConditionTimeoutException;
 import org.junit.Assert;
 import org.junit.Test;
@@ -436,7 +434,7 @@ private Object getInVM(VM vm, final String regionName, final Serializable key) {
 
   private void putAndWaitForSuccess(VM vm, final String regionName, final Serializable key,
       final Serializable value) {
-    Awaitility.await().atMost(MAX_WAIT, MILLISECONDS).untilAsserted(() -> {
+    await().untilAsserted(() -> {
       assertThatCode(() -> putInVM(vm, regionName, key, value)).doesNotThrowAnyException();
     });
   }
@@ -464,7 +462,7 @@ protected void checkEndpoints(VM vm, final int... expectedPorts) {
       for (int i = 0; i < expectedPorts.length; i++) {
         expectedEndpointPorts.add(new Integer(expectedPorts[i]));
       }
-      Awaitility.await().atMost(5, SECONDS).untilAsserted(() -> {
+      await().untilAsserted(() -> {
         List<ServerLocation> endpoints;
         HashSet actualEndpointPorts;
         endpoints = pool.getCurrentServers();
@@ -528,7 +526,7 @@ private void waitForJoin(VM vm) {
     vm.invoke("wait for join", () -> {
       MyListener listener = (MyListener) remoteObjects.get(BRIDGE_LISTENER);
       try {
-        Awaitility.await().atMost(10, SECONDS).until(() -> listener.getJoins() > 0);
+        await().until(() -> listener.getJoins() > 0);
       } catch (ConditionTimeoutException e) {
         // do nothing here - caller will perform validations
       }
@@ -539,7 +537,7 @@ private void waitForCrash(VM vm) {
     vm.invoke("wait for crash", () -> {
       MyListener listener = (MyListener) remoteObjects.get(BRIDGE_LISTENER);
       try {
-        Awaitility.await().atMost(10, SECONDS).until(() -> listener.getCrashes() > 0);
+        await().until(() -> listener.getCrashes() > 0);
       } catch (ConditionTimeoutException e) {
         // do nothing here - caller will perform validations
       }
@@ -550,7 +548,7 @@ private void waitForDeparture(VM vm) {
     vm.invoke("wait for departure", () -> {
       MyListener listener = (MyListener) remoteObjects.get(BRIDGE_LISTENER);
       try {
-        Awaitility.await().atMost(10, SECONDS).until(() -> listener.getDepartures() > 0);
+        await().until(() -> listener.getDepartures() > 0);
       } catch (ConditionTimeoutException e) {
         // do nothing here - caller will perform validations
       }

geode-core/src/distributedTest/java/org/apache/geode/cache/client/internal/LocatorLoadBalancingDUnitTest.java

@@ -14,6 +14,8 @@
  */
 package org.apache.geode.cache.client.internal;
 
+import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
+
 import java.io.IOException;
 import java.io.Serializable;
 import java.net.InetAddress;
@@ -24,7 +26,6 @@
 import java.util.Properties;
 import java.util.concurrent.TimeUnit;
 
-import org.awaitility.Awaitility;
 import org.junit.Assert;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
@@ -254,15 +255,14 @@ public void testBalancing() throws Exception {
   private void checkConnectionCount(final int count) {
     Cache cache = (Cache) remoteObjects.get(CACHE_KEY);
     final CacheServerImpl server = (CacheServerImpl) cache.getCacheServers().get(0);
-    Awaitility.await().pollDelay(100, TimeUnit.MILLISECONDS)
-        .pollInterval(100, TimeUnit.MILLISECONDS).timeout(300, TimeUnit.SECONDS).until(() -> {
-          int sz = server.getAcceptor().getStats().getCurrentClientConnections();
-          if (Math.abs(sz - count) <= ALLOWABLE_ERROR_IN_COUNT) {
-            return true;
-          }
-          System.out.println("Found " + sz + " connections, expected " + count);
-          return false;
-        });
+    await().timeout(300, TimeUnit.SECONDS).until(() -> {
+      int sz = server.getAcceptor().getStats().getCurrentClientConnections();
+      if (Math.abs(sz - count) <= ALLOWABLE_ERROR_IN_COUNT) {
+        return true;
+      }
+      System.out.println("Found " + sz + " connections, expected " + count);
+      return false;
+    });
   }
 
   private void waitForPrefilledConnections(final int count) throws Exception {
@@ -272,8 +272,7 @@ private void waitForPrefilledConnections(final int count) throws Exception {
   private void waitForPrefilledConnections(final int count, final String poolName)
       throws Exception {
     final PoolImpl pool = (PoolImpl) PoolManager.getAll().get(poolName);
-    Awaitility.await().pollDelay(100, TimeUnit.MILLISECONDS)
-        .pollInterval(100, TimeUnit.MILLISECONDS).timeout(300, TimeUnit.SECONDS)
+    await().timeout(300, TimeUnit.SECONDS)
         .until(() -> pool.getConnectionCount() >= count);
   }
 
@@ -429,8 +428,7 @@ public void checkLocatorLoad(final Map expected) {
     final ServerLocator sl = locator.getServerLocatorAdvisee();
     InternalLogWriter log = new LocalLogWriter(InternalLogWriter.FINEST_LEVEL, System.out);
     sl.getDistributionAdvisor().dumpProfiles("PROFILES= ");
-    Awaitility.await().pollDelay(100, TimeUnit.MILLISECONDS)
-        .pollInterval(100, TimeUnit.MILLISECONDS).timeout(300, TimeUnit.SECONDS)
+    await().timeout(300, TimeUnit.SECONDS)
         .until(() -> expected.equals(sl.getLoadMap()));
   }