[GitHub] [geode] mhansonp commented on a change in pull request #4924: GEODE-7964: Upgrade Mockito to 3.3.3

[GitBox <gi...@apache.org>]

mhansonp commented on a change in pull request #4924:
URL: https://github.com/apache/geode/pull/4924#discussion_r413138633



##########
File path: geode-core/src/integrationTest/java/org/apache/geode/cache/client/internal/pooling/ConnectionManagerJUnitTest.java
##########
@@ -122,278 +138,271 @@ public void tearDown() throws InterruptedException {
 
   @Test
   public void testAddVarianceToInterval() {
-    assertThat(ConnectionManagerImpl.addVarianceToInterval(0)).as("Zero gets zero variance")
+    assertThat(ConnectionManagerImpl.addVarianceToInterval(0))
+        .as("Zero gets zero variance")
         .isEqualTo(0);
+
     assertThat(ConnectionManagerImpl.addVarianceToInterval(300000))
-        .as("Large value gets +/-10% variance").isNotEqualTo(300000).isGreaterThanOrEqualTo(270000)
+        .as("Large value gets +/-10% variance")
+        .isNotEqualTo(300000)
+        .isGreaterThanOrEqualTo(270000)
         .isLessThanOrEqualTo(330000);
-    assertThat(ConnectionManagerImpl.addVarianceToInterval(9)).as("Small value gets +/-1 variance")
-        .isNotEqualTo(9).isGreaterThanOrEqualTo(8).isLessThanOrEqualTo(10);
+
+    assertThat(ConnectionManagerImpl.addVarianceToInterval(9))
+        .as("Small value gets +/-1 variance")
+        .isNotEqualTo(9)
+        .isGreaterThanOrEqualTo(8)
+        .isLessThanOrEqualTo(10);
   }
 
   @Test
-  public void testGet()
-      throws InterruptedException, AllConnectionsInUseException, NoAvailableServersException {
+  public void testGet() {
     manager = new ConnectionManagerImpl("pool", factory, endpointManager, 3, 0, -1, -1, logger,
         60 * 1000, cancelCriterion, poolStats);
     manager.start(background);
 
-    Connection conn[] = new Connection[4];
-
-    conn[0] = manager.borrowConnection(0);
-    Assert.assertEquals(1, factory.creates);
-
-    manager.returnConnection(conn[0]);
-    conn[0] = manager.borrowConnection(0);
-    Assert.assertEquals(1, factory.creates);
-    conn[1] = manager.borrowConnection(0);
-    manager.returnConnection(conn[0]);
-    manager.returnConnection(conn[1]);
-    Assert.assertEquals(2, factory.creates);
-
-    conn[0] = manager.borrowConnection(0);
-    conn[1] = manager.borrowConnection(0);
-    conn[2] = manager.borrowConnection(0);
-    Assert.assertEquals(3, factory.creates);
-
-    try {
-      conn[4] = manager.borrowConnection(10);
-      fail("Should have received an all connections in use exception");
-    } catch (AllConnectionsInUseException e) {
-      // expected exception
-    }
+    Connection connection1 = manager.borrowConnection(0);
+    assertThat(factory.creates.get()).isEqualTo(1);
+
+    manager.returnConnection(connection1);
+    connection1 = manager.borrowConnection(0);
+
+    assertThat(factory.creates.get()).isEqualTo(1);
+
+    Connection connection2 = manager.borrowConnection(0);
+    manager.returnConnection(connection1);
+    manager.returnConnection(connection2);
+
+    assertThat(factory.creates.get()).isEqualTo(2);
+
+    manager.borrowConnection(0);
+    manager.borrowConnection(0);
+    manager.borrowConnection(0);
+
+    assertThat(factory.creates.get()).isEqualTo(3);
+
+    Throwable thrown = catchThrowable(() -> {
+      manager.borrowConnection(10);
+    });
+    assertThat(thrown).isInstanceOf(AllConnectionsInUseException.class);
   }
 
   @Test
-  public void testPrefill() throws InterruptedException {
+  public void testPrefill() {
     manager = new ConnectionManagerImpl("pool", factory, endpointManager, 10, 2, -1, -1, logger,
         60 * 1000, cancelCriterion, poolStats);
     manager.start(background);
-    final String descrip = manager.toString();
-    WaitCriterion ev = new WaitCriterion() {
-      @Override
-      public boolean done() {
-        return factory.creates == 2 && factory.destroys == 0;
-      }
 
-      @Override
-      public String description() {
-        return "waiting for manager " + descrip;
-      }
-    };
-    GeodeAwaitility.await().untilAsserted(ev);
+    await("waiting for manager " + manager).untilAsserted(() -> {
+      assertThat(factory.creates.get()).isEqualTo(2);
+      assertThat(factory.destroys.get()).isEqualTo(0);
+    });
   }
 
   @Test
-  public void testInvalidateConnection()
-      throws InterruptedException, AllConnectionsInUseException, NoAvailableServersException {
+  public void testInvalidateConnection() {
     manager = new ConnectionManagerImpl("pool", factory, endpointManager, 10, 0, 0L, -1, logger,
         60 * 1000, cancelCriterion, poolStats);
     manager.start(background);
 
-    Connection conn = manager.borrowConnection(0);
-    Assert.assertEquals(1, factory.creates);
-    Assert.assertEquals(0, factory.destroys);
-    conn.destroy();
-    manager.returnConnection(conn);
-    Assert.assertEquals(1, factory.creates);
-    Assert.assertEquals(1, factory.destroys);
-    conn = manager.borrowConnection(0);
-    Assert.assertEquals(2, factory.creates);
-    Assert.assertEquals(1, factory.destroys);
+    Connection connection = manager.borrowConnection(0);
+
+    assertThat(factory.creates.get()).isEqualTo(1);
+    assertThat(factory.destroys.get()).isEqualTo(0);
+
+    connection.destroy();
+    manager.returnConnection(connection);
+
+    assertThat(factory.creates.get()).isEqualTo(1);
+    assertThat(factory.destroys.get()).isEqualTo(1);
+
+    manager.borrowConnection(0);
+
+    assertThat(factory.creates.get()).isEqualTo(2);
+    assertThat(factory.destroys.get()).isEqualTo(1);
   }
 
   @Test
-  public void testInvalidateServer()
-      throws InterruptedException, AllConnectionsInUseException, NoAvailableServersException {
+  public void testInvalidateServer() {
     manager = new ConnectionManagerImpl("pool", factory, endpointManager, 10, 0, -1, -1, logger,
         60 * 1000, cancelCriterion, poolStats);
     manager.start(background);
 
     ServerLocation server1 = new ServerLocation("localhost", 1);
     ServerLocation server2 = new ServerLocation("localhost", 2);
-    factory.nextServer = server1;
-    Connection conn1 = manager.borrowConnection(0);
-    Connection conn2 = manager.borrowConnection(0);
-    Connection conn3 = manager.borrowConnection(0);
-    factory.nextServer = server2;
-    Connection conn4 = manager.borrowConnection(0);
-
-    Assert.assertEquals(4, factory.creates);
-    Assert.assertEquals(0, factory.destroys);
-
-    manager.returnConnection(conn2);
-    endpointManager.serverCrashed(conn2.getEndpoint());
-    Assert.assertEquals(3, factory.destroys);
-    conn1.destroy();
-    manager.returnConnection(conn1);
-    Assert.assertEquals(3, factory.destroys);
-    manager.returnConnection(conn3);
-    manager.returnConnection(conn4);
-    Assert.assertEquals(3, factory.destroys);
+    factory.nextServer.set(server1);
+    Connection connection1 = manager.borrowConnection(0);
+    Connection connection2 = manager.borrowConnection(0);
+    Connection connection3 = manager.borrowConnection(0);
+    factory.nextServer.set(server2);
+    Connection connection4 = manager.borrowConnection(0);
+
+    assertThat(factory.creates.get()).isEqualTo(4);
+    assertThat(factory.destroys.get()).isEqualTo(0);
+
+    manager.returnConnection(connection2);
+    endpointManager.serverCrashed(connection2.getEndpoint());
+
+    assertThat(factory.destroys.get()).isEqualTo(3);
+
+    connection1.destroy();
+    manager.returnConnection(connection1);
+
+    assertThat(factory.destroys.get()).isEqualTo(3);
+
+    manager.returnConnection(connection3);
+    manager.returnConnection(connection4);
+
+    assertThat(factory.destroys.get()).isEqualTo(3);
 
     manager.borrowConnection(0);
-    Assert.assertEquals(4, factory.creates);
-    Assert.assertEquals(3, factory.destroys);
+    assertThat(factory.creates.get()).isEqualTo(4);
+    assertThat(factory.destroys.get()).isEqualTo(3);
   }
 
   @Test
-  public void testIdleExpiration()
-      throws InterruptedException, AllConnectionsInUseException, NoAvailableServersException {
-    final long idleTimeoutMillis = 300;
+  public void testIdleExpiration() throws Exception {
+    long idleTimeoutMillis = 300;
     manager =
         new ConnectionManagerImpl("pool", factory, endpointManager, 5, 2, idleTimeoutMillis, -1,
             logger, 60 * 1000, cancelCriterion, poolStats);
     manager.start(background);
 
-    {
-      factory.waitWhile(() -> factory.creates < 2);
-      Assert.assertEquals(2, factory.creates);
-      Assert.assertEquals(0, factory.destroys);
-      Assert.assertEquals(0, factory.closes);
-      Assert.assertEquals(0, poolStats.getIdleExpire());
-      // no need to wait; dangerous because it gives connections a chance to expire
-      // //wait for prefill task to finish.
-      // Thread.sleep(100);
-    }
+    await().untilAsserted(() -> {
+      assertThat(factory.creates.get()).isEqualTo(2);
+      assertThat(factory.destroys.get()).isEqualTo(0);
+      assertThat(factory.closes.get()).isEqualTo(0);
+      assertThat(poolStats.getIdleExpire()).isEqualTo(0);
+    });
+
+    // no need to wait; dangerous because it gives connections a chance to expire
 
-    Connection conn1 = manager.borrowConnection(500);
-    Connection conn2 = manager.borrowConnection(500);
-    Connection conn3 = manager.borrowConnection(500);
-    Connection conn4 = manager.borrowConnection(500);
-    Connection conn5 = manager.borrowConnection(500);
+    Connection connection1 = manager.borrowConnection(500);
+    Connection connection2 = manager.borrowConnection(500);
+    Connection connection3 = manager.borrowConnection(500);
+    Connection connection4 = manager.borrowConnection(500);
+    Connection connection5 = manager.borrowConnection(500);
 
     // wait to make sure checked out connections aren't timed out
     Thread.sleep(idleTimeoutMillis * 2);
-    Assert.assertEquals(5, factory.creates);
-    Assert.assertEquals(0, factory.destroys);
-    Assert.assertEquals(0, factory.closes);
-    Assert.assertEquals(0, poolStats.getIdleExpire());
-
-    {
-      // make sure a connection that has been passivated can idle-expire
-      conn1.passivate(true);
-
-      long elapsedMillis = factory.waitWhile(() -> factory.destroys < 1);
-      Assert.assertEquals(5, factory.creates);
-      Assert.assertEquals(1, factory.destroys);
-      Assert.assertEquals(1, factory.closes);
-      Assert.assertEquals(1, poolStats.getIdleExpire());
-      checkIdleTimeout(idleTimeoutMillis, elapsedMillis);
-    }
+    assertThat(factory.creates.get()).isEqualTo(5);
+    assertThat(factory.destroys.get()).isEqualTo(0);
+    assertThat(factory.closes.get()).isEqualTo(0);
+    assertThat(poolStats.getIdleExpire()).isEqualTo(0);
+
+    // make sure a connection that has been passivated can idle-expire
+    connection1.passivate(true);
+
+    long elapsedMillis = Timer.measure(() -> {
+      await().untilAsserted(() -> {
+        assertThat(factory.creates.get()).isEqualTo(5);
+        assertThat(factory.destroys.get()).isEqualTo(1);
+        assertThat(factory.closes.get()).isEqualTo(1);
+        assertThat(poolStats.getIdleExpire()).isEqualTo(1);
+      });
+    });
+    checkIdleTimeout(idleTimeoutMillis, elapsedMillis);
 
     // now return all other connections to pool and verify that just 2 expire
-    manager.returnConnection(conn2);
-    manager.returnConnection(conn3);
-    manager.returnConnection(conn4);
-    manager.returnConnection(conn5);
-
-    {
-      long elapsedMillis = factory.waitWhile(() -> factory.destroys < 3);
-      Assert.assertEquals(5, factory.creates);
-      Assert.assertEquals(3, factory.destroys);
-      Assert.assertEquals(3, factory.closes);
-      Assert.assertEquals(3, poolStats.getIdleExpire());
-      checkIdleTimeout(idleTimeoutMillis, elapsedMillis);
-    }
+    manager.returnConnection(connection2);
+    manager.returnConnection(connection3);
+    manager.returnConnection(connection4);
+    manager.returnConnection(connection5);
+
+    elapsedMillis = Timer.measure(() -> {
+      await().untilAsserted(() -> {
+        assertThat(factory.creates.get()).isEqualTo(5);
+        assertThat(factory.destroys.get()).isEqualTo(3);
+        assertThat(factory.closes.get()).isEqualTo(3);
+        assertThat(poolStats.getIdleExpire()).isEqualTo(3);
+      });
+    });
+    checkIdleTimeout(idleTimeoutMillis, elapsedMillis);
 
     // wait to make sure min-connections don't time out
     Thread.sleep(idleTimeoutMillis * 2);
-    Assert.assertEquals(5, factory.creates);
-    Assert.assertEquals(3, factory.destroys);
-    Assert.assertEquals(3, factory.closes);
-    Assert.assertEquals(3, poolStats.getIdleExpire());
-  }
-
-  private void checkIdleTimeout(final long idleTimeoutMillis, long elapsedMillis) {
-    Assert.assertTrue(
-        "Elapsed " + elapsedMillis + " is less than idle timeout " + idleTimeoutMillis,
-        elapsedMillis >= (idleTimeoutMillis - ALLOWABLE_ERROR_IN_MILLIS));
-    Assert.assertTrue(
-        "Elapsed " + elapsedMillis + " is greater than idle timeout " + idleTimeoutMillis,
-        elapsedMillis <= (idleTimeoutMillis + ALLOWABLE_ERROR_IN_MILLIS));
+    assertThat(factory.creates.get()).isEqualTo(5);
+    assertThat(factory.destroys.get()).isEqualTo(3);
+    assertThat(factory.closes.get()).isEqualTo(3);
+    assertThat(poolStats.getIdleExpire()).isEqualTo(3);
   }

Review comment:
       This does not look like it is asserting the same thing about timeouts any more..




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org