pivotal-jbarrett commented on code in PR #7608:
URL: https://github.com/apache/geode/pull/7608#discussion_r855451606
##########
geode-core/src/distributedTest/java/org/apache/geode/cache/client/ClientServerRegisterInterestsDUnitTest.java:
##########
@@ -19,10 +19,9 @@
import static org.apache.geode.distributed.ConfigurationProperties.LOG_FILE;
import static org.apache.geode.distributed.ConfigurationProperties.LOG_LEVEL;
import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
-import static org.junit.Assert.assertEquals;
+import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertFalse;
Review Comment:
Since you are in here can we constantly replace all these with AssertJ.
There is a handle plug-in for IntelliJ for this.
##########
geode-core/src/distributedTest/java/org/apache/geode/cache/client/ClientServerRegisterInterestsDUnitTest.java:
##########
@@ -83,74 +79,62 @@ public final void postSetUp() throws Exception {
public final void preTearDown() throws Exception {
serverPort.set(0);
entryEvents.clear();
- gemfireServerVm.invoke(new SerializableRunnable() {
- @Override
- public void run() {
- CacheFactory.getAnyInstance().close();
- }
- });
+ gemfireServerVm.invoke(() -> CacheFactory.getAnyInstance().close());
gemfireServerVm = null;
}
private void setupGemFireCacheServer() {
- Host localhost = Host.getHost(0);
- gemfireServerVm = localhost.getVM(0);
+
+ gemfireServerVm = VM.getVM(0);
serverPort.set(AvailablePortHelper.getRandomAvailableTCPPort());
- gemfireServerVm.invoke(new SerializableRunnable() {
- @Override
- public void run() {
- try {
- Cache cache = new CacheFactory()
- .set("name",
"ClientServerRegisterInterestsTestGemFireServer").set(MCAST_PORT, "0")
- .set(LOG_FILE,
"clientServerRegisterInterestsTest.log").set(LOG_LEVEL, "config")
- // .set("jmx-manager", "true")
- // .set("jmx-manager-http-port", "0")
- // .set("jmx-manager-port", "1199")
- // .set("jmx-manager-start", "true")
- .create();
+ gemfireServerVm.invoke(() -> {
+ try {
+ Cache cache = new CacheFactory()
+ .set("name",
"ClientServerRegisterInterestsTestGemFireServer").set(MCAST_PORT, "0")
+ .set(LOG_FILE,
"clientServerRegisterInterestsTest.log").set(LOG_LEVEL, "config")
+ .create();
- RegionFactory<String, String> regionFactory =
cache.createRegionFactory();
+ RegionFactory<String, String> regionFactory =
cache.createRegionFactory();
- regionFactory.setDataPolicy(DataPolicy.REPLICATE);
- regionFactory.setKeyConstraint(String.class);
- regionFactory.setValueConstraint(String.class);
+ regionFactory.setDataPolicy(DataPolicy.REPLICATE);
+ regionFactory.setKeyConstraint(String.class);
+ regionFactory.setValueConstraint(String.class);
- Region<String, String> example = regionFactory.create("Example");
+ Region<String, String> example = regionFactory.create("Example");
- assertNotNull("The 'Example' Region was not properly configured and
initialized!",
- example);
- assertEquals(SEPARATOR + "Example", example.getFullPath());
- assertEquals("Example", example.getName());
- assertTrue(example.isEmpty());
+ assertNotNull("The 'Example' Region was not properly configured and
initialized!",
+ example);
+ assertThat(example.getFullPath()).isEqualTo(SEPARATOR + "Example");
+ assertThat(example.getName()).isEqualTo("Example");
+ assertTrue(example.isEmpty());
- example.put("1", "ONE");
+ example.put("1", "ONE");
- assertFalse(example.isEmpty());
- assertEquals(1, example.size());
+ assertFalse(example.isEmpty());
Review Comment:
For example `assertThat(example).isEmpty();`.
##########
geode-cq/src/distributedTest/java/org/apache/geode/internal/cache/tier/sockets/DurableClientTestBase.java:
##########
@@ -687,41 +675,37 @@ public void run2() throws CacheException {
*/
void checkCqListenerEvents(VM vm, final String cqName, final int numEvents,
final int secondsToWait) {
- vm.invoke(() -> {
- checkCqListenerEvents(cqName, numEvents, secondsToWait);
- });
+ vm.invoke(() -> checkCqListenerEvents(cqName, numEvents, secondsToWait));
}
- void checkCqListenerEvents(final String cqName, final int numEvents,
- final int secondsToWait) {
- QueryService qs = CacheServerTestUtil.getCache().getQueryService();
+ void checkCqListenerEvents(final String cqName, final int numEvents, final
int secondsToWait) {
+ QueryService qs = getCache().getQueryService();
CqQuery cq = qs.getCq(cqName);
// Get the listener and wait for the appropriate number of events
- CacheServerTestUtil.ControlCqListener listener =
- (CacheServerTestUtil.ControlCqListener)
cq.getCqAttributes().getCqListener();
- listener.waitWhileNotEnoughEvents(secondsToWait * 1000, numEvents);
+ ControlCqListener listener =
+ (ControlCqListener) cq.getCqAttributes().getCqListener();
+ listener.waitWhileNotEnoughEvents(secondsToWait * 1000L, numEvents);
Review Comment:
Using `TimeUnits.SECONDS.toMillis(secondsToWait)` makes it more readable.
##########
geode-cq/src/distributedTest/java/org/apache/geode/internal/cache/tier/sockets/DurableClientTestBase.java:
##########
@@ -639,44 +627,44 @@ public void run2() throws CacheException {
*/
void checkHAQueueSize(VM server, final String durableClientId, final int
expectedNumber,
final int remaining) {
- server.invoke(new CacheSerializableRunnable(
- "Check ha queued size for durable client " + durableClientId) {
- @Override
- public void run2() throws CacheException {
+ server.invoke("Check ha queued size for durable client " + durableClientId,
+ new CacheSerializableRunnable() {
+ @Override
+ public void run2() throws CacheException {
- final CacheClientNotifier ccnInstance =
CacheClientNotifier.getInstance();
- final CacheClientProxy clientProxy =
ccnInstance.getClientProxy(durableClientId);
+ final CacheClientNotifier ccnInstance =
CacheClientNotifier.getInstance();
+ final CacheClientProxy clientProxy =
ccnInstance.getClientProxy(durableClientId);
- // Wait until we get the expected number of events or until 10 seconds
are up
- await()
- .until(() -> clientProxy.getQueueSizeStat() == expectedNumber
- || clientProxy.getQueueSizeStat() == remaining);
+ // Wait until we get the expected number of events or until 10
seconds are up
+ await()
+ .until(() -> clientProxy.getQueueSizeStat() == expectedNumber
+ || clientProxy.getQueueSizeStat() == remaining);
Review Comment:
Using AssertJ here will give us useful failure messages.
##########
geode-cq/src/distributedTest/java/org/apache/geode/internal/cache/tier/sockets/DurableClientSimpleDUnitTest.java:
##########
@@ -1014,26 +1010,35 @@ public void testSimpleDurableClientMultipleServers() {
public void testDurableClientReceivedClientSessionInitialValue() {
// Start server 1
server1Port = server1VM
- .invoke(() -> CacheServerTestUtil.createCacheServer(regionName,
Boolean.TRUE));
+ .invoke(() -> CacheServerTestUtil.createCacheServer(regionName, true));
// Start server 2
int server2Port = server2VM
- .invoke(() -> CacheServerTestUtil.createCacheServer(regionName,
Boolean.TRUE));
+ .invoke(() -> CacheServerTestUtil.createCacheServer(regionName, true));
// Start normal publisher client
publisherClientVM.invoke(() -> CacheServerTestUtil
.createCacheClient(getClientPool(getServerHostName(),
server1Port, server2Port, false), regionName));
// Create an entry
- publishEntries(0, 1);
+ publishEntries(publisherClientVM, 0, 1);
// Start a durable client with the ControlListener
durableClientId = getName() + "_client";
- startupDurableClient(VERY_LONG_DURABLE_TIMEOUT_SECONDS,
- getClientPool(getServerHostName(), server1Port, server2Port,
- true),
- Boolean.TRUE);
+ durableClientVM.invoke(() -> createCacheClient(
+ getClientPool(getServerHostName(), server1Port, server2Port, true),
regionName,
+ getClientDistributedSystemProperties(durableClientId,
VERY_LONG_DURABLE_TIMEOUT_SECONDS),
+ true));
+
+ durableClientVM.invoke(() -> {
+ await().atMost(1 * HEAVY_TEST_LOAD_DELAY_SUPPORT_MULTIPLIER, MINUTES)
+ .pollInterval(100, MILLISECONDS)
+ .until(CacheServerTestUtil::getCache, notNullValue());
Review Comment:
Maybe stick with AssertJ:
```java
...
.utilAsserted(() -> assertThat(CacheServerTestUtil.getCache()).isNotNull());
```
##########
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/tier/sockets/DurableClientStatsDUnitTest.java:
##########
@@ -340,13 +320,13 @@ private static void putValue(String key, String value) {
Region r = CacheServerTestUtil.getCache()
.getRegion(DurableClientStatsDUnitTest.class.getName() + "_region");
// Region r = CacheServerTestUtil.getCache().getRegion(regionName);
- assertNotNull(r);
+ assertThat(r).isNotNull();
if (r.getEntry(key) != null) {
r.put(key, value);
} else {
r.create(key, value);
}
- assertEquals(value, r.getEntry(key).getValue());
+ assertThat(r.getEntry(key).getValue()).isEqualTo(value);
Review Comment:
```java
assertThat(r).contains(entry(key, value));
```
##########
geode-core/src/distributedTest/java/org/apache/geode/cache/client/ClientServerRegisterInterestsDUnitTest.java:
##########
@@ -224,27 +205,27 @@ public void testClientRegisterInterests() {
Region<String, String> example = clientCache.getRegion(SEPARATOR +
"Example");
assertNotNull("'Example' Region in Client Cache was not found!",
example);
- assertEquals(1, example.size());
+ assertThat(example.size()).isEqualTo(1);
Review Comment:
```java
assertThat(example).hasSize(1);
```
##########
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/tier/sockets/DurableClientBug39997DUnitTest.java:
##########
@@ -50,81 +46,61 @@ public class DurableClientBug39997DUnitTest extends
JUnit4CacheTestCase {
@Override
public final void postTearDownCacheTestCase() {
-
Host.getHost(0).getVM(0).invoke(JUnit4DistributedTestCase::disconnectFromDS);
+ VM.getVM(0).invoke(JUnit4DistributedTestCase::disconnectFromDS);
}
@Test
public void testNoServerAvailableOnStartup() {
Host host = Host.getHost(0);
- VM vm0 = host.getVM(0);
- VM vm1 = host.getVM(1);
+ VM vm0 = VM.getVM(0);
+ VM vm1 = VM.getVM(1);
- final String hostName = NetworkUtils.getServerHostName(host);
+ final String hostName = VM.getHostName();
final int port = AvailablePortHelper.getRandomAvailableTCPPort();
- vm0.invoke(new SerializableRunnable("create cache") {
- @Override
- public void run() {
- getSystem(getClientProperties());
- PoolImpl p = (PoolImpl)
PoolManager.createFactory().addServer(hostName, port)
- .setSubscriptionEnabled(true).setSubscriptionRedundancy(0)
- .create("DurableClientReconnectDUnitTestPool");
- AttributesFactory factory = new AttributesFactory();
- factory.setScope(Scope.LOCAL);
- factory.setPoolName(p.getName());
- Cache cache = getCache();
- Region region1 = cache.createRegion("region", factory.create());
- cache.readyForEvents();
+ vm0.invoke("create cache", () -> {
+ getSystem(getClientProperties());
+ PoolImpl p = (PoolImpl) PoolManager.createFactory().addServer(hostName,
port)
+ .setSubscriptionEnabled(true).setSubscriptionRedundancy(0)
+ .create("DurableClientReconnectDUnitTestPool");
+ Cache cache = getCache();
+ RegionFactory regionFactory = cache.createRegionFactory();
+ regionFactory.setScope(Scope.LOCAL);
+ regionFactory.setPoolName(p.getName());
+ Region region1 = regionFactory.create("region");
- try {
- region1.registerInterest("ALL_KEYS");
- fail("Should have received an exception trying to register
interest");
- } catch (NoSubscriptionServersAvailableException expected) {
- // this is expected
- }
+ try {
+ region1.registerInterestForAllKeys();
+ fail("Should have received an exception trying to register interest");
+ } catch (NoSubscriptionServersAvailableException expected) {
+ // this is expected
}
});
- vm1.invoke(new SerializableRunnable() {
- @Override
- public void run() {
- Cache cache = getCache();
- AttributesFactory factory = new AttributesFactory();
- factory.setScope(Scope.DISTRIBUTED_ACK);
- cache.createRegion("region", factory.create());
- CacheServer server = cache.addCacheServer();
- server.setPort(port);
- try {
- server.start();
- } catch (IOException e) {
- Assert.fail("couldn't start server", e);
- }
+ vm1.invoke(() -> {
+ Cache cache = getCache();
+ RegionFactory regionFactory = cache.createRegionFactory();
+ regionFactory.setScope(Scope.DISTRIBUTED_ACK);
+ regionFactory.create("region");
+ CacheServer server = cache.addCacheServer();
+ server.setPort(port);
+ try {
+ server.start();
+ } catch (IOException e) {
+ fail("couldn't start server", e);
Review Comment:
Alternatively there is:
```java
assertDoesNotThrow(() -> server.start());
```
##########
geode-cq/src/distributedTest/java/org/apache/geode/internal/cache/tier/sockets/DurableClientTestBase.java:
##########
@@ -216,33 +208,35 @@ void verifyDurableClientPresence(int
durableClientTimeout, String durableClientI
});
}
+ void waitUntilHARegionQueueSizeIsZero(VM serverVM) {
+ serverVM.invoke(() -> await().atMost(60, SECONDS)
+ .until(() -> getClientProxy().getHARegionQueue().size() == 0));
+ }
+
public void closeDurableClient() {
durableClientVM.invoke((SerializableRunnableIF)
CacheServerTestUtil::closeCache);
}
public void disconnectDurableClient(boolean keepAlive) {
- printClientProxyState("Before");
+ // printClientProxyState("Before");
durableClientVM.invoke("close durable client cache",
() -> CacheServerTestUtil.closeCache(keepAlive));
- await()
- .until(CacheServerTestUtil::getCache, nullValue());
- printClientProxyState("after");
+ await().until(CacheServerTestUtil::getCache, nullValue());
+ // printClientProxyState("after");
Review Comment:
commented out code here and above.
##########
geode-core/src/distributedTest/java/org/apache/geode/cache/client/ClientServerRegisterInterestsDUnitTest.java:
##########
@@ -224,27 +205,27 @@ public void testClientRegisterInterests() {
Region<String, String> example = clientCache.getRegion(SEPARATOR +
"Example");
assertNotNull("'Example' Region in Client Cache was not found!",
example);
- assertEquals(1, example.size());
+ assertThat(example.size()).isEqualTo(1);
assertTrue(example.containsKey("1"));
- assertEquals("ONE", example.get("1"));
+ assertThat(example.get("1")).isEqualTo("ONE");
Review Comment:
This line and the 2 above it could be collapsed into:
```java
assertThat(example).containsOnly(entry("1", "ONE"));
```
##########
geode-cq/src/distributedTest/java/org/apache/geode/internal/cache/tier/sockets/DurableClientTestBase.java:
##########
@@ -687,41 +675,37 @@ public void run2() throws CacheException {
*/
void checkCqListenerEvents(VM vm, final String cqName, final int numEvents,
final int secondsToWait) {
- vm.invoke(() -> {
- checkCqListenerEvents(cqName, numEvents, secondsToWait);
- });
+ vm.invoke(() -> checkCqListenerEvents(cqName, numEvents, secondsToWait));
}
- void checkCqListenerEvents(final String cqName, final int numEvents,
- final int secondsToWait) {
- QueryService qs = CacheServerTestUtil.getCache().getQueryService();
+ void checkCqListenerEvents(final String cqName, final int numEvents, final
int secondsToWait) {
+ QueryService qs = getCache().getQueryService();
CqQuery cq = qs.getCq(cqName);
// Get the listener and wait for the appropriate number of events
- CacheServerTestUtil.ControlCqListener listener =
- (CacheServerTestUtil.ControlCqListener)
cq.getCqAttributes().getCqListener();
- listener.waitWhileNotEnoughEvents(secondsToWait * 1000, numEvents);
+ ControlCqListener listener =
+ (ControlCqListener) cq.getCqAttributes().getCqListener();
+ listener.waitWhileNotEnoughEvents(secondsToWait * 1000L, numEvents);
assertThat(numEvents).isEqualTo(listener.events.size());
}
void checkListenerEvents(int numberOfEntries, final int sleepMinutes, final
int eventType,
final VM vm) {
vm.invoke(() -> {
// Get the region
- Region<Object, Object> region =
CacheServerTestUtil.getCache().getRegion(regionName);
+ Region<Object, Object> region = getCache().getRegion(regionName);
assertThat(region).isNotNull();
// Get the listener and wait for the appropriate number of events
- CacheServerTestUtil.ControlListener controlListener =
- (CacheServerTestUtil.ControlListener)
region.getAttributes().getCacheListeners()[0];
-
- controlListener.waitWhileNotEnoughEvents(sleepMinutes * 60 * 1000,
numberOfEntries,
+ ControlListener controlListener =
+ (ControlListener) region.getAttributes().getCacheListeners()[0];
+ controlListener.waitWhileNotEnoughEvents((long) sleepMinutes * 60 *
1000, numberOfEntries,
Review Comment:
More time math.
##########
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/tier/sockets/DurableClientBug39997DUnitTest.java:
##########
@@ -50,81 +46,61 @@ public class DurableClientBug39997DUnitTest extends
JUnit4CacheTestCase {
@Override
public final void postTearDownCacheTestCase() {
-
Host.getHost(0).getVM(0).invoke(JUnit4DistributedTestCase::disconnectFromDS);
+ VM.getVM(0).invoke(JUnit4DistributedTestCase::disconnectFromDS);
}
@Test
public void testNoServerAvailableOnStartup() {
Host host = Host.getHost(0);
- VM vm0 = host.getVM(0);
- VM vm1 = host.getVM(1);
+ VM vm0 = VM.getVM(0);
+ VM vm1 = VM.getVM(1);
- final String hostName = NetworkUtils.getServerHostName(host);
+ final String hostName = VM.getHostName();
final int port = AvailablePortHelper.getRandomAvailableTCPPort();
- vm0.invoke(new SerializableRunnable("create cache") {
- @Override
- public void run() {
- getSystem(getClientProperties());
- PoolImpl p = (PoolImpl)
PoolManager.createFactory().addServer(hostName, port)
- .setSubscriptionEnabled(true).setSubscriptionRedundancy(0)
- .create("DurableClientReconnectDUnitTestPool");
- AttributesFactory factory = new AttributesFactory();
- factory.setScope(Scope.LOCAL);
- factory.setPoolName(p.getName());
- Cache cache = getCache();
- Region region1 = cache.createRegion("region", factory.create());
- cache.readyForEvents();
+ vm0.invoke("create cache", () -> {
+ getSystem(getClientProperties());
+ PoolImpl p = (PoolImpl) PoolManager.createFactory().addServer(hostName,
port)
+ .setSubscriptionEnabled(true).setSubscriptionRedundancy(0)
+ .create("DurableClientReconnectDUnitTestPool");
+ Cache cache = getCache();
+ RegionFactory regionFactory = cache.createRegionFactory();
+ regionFactory.setScope(Scope.LOCAL);
+ regionFactory.setPoolName(p.getName());
+ Region region1 = regionFactory.create("region");
- try {
- region1.registerInterest("ALL_KEYS");
- fail("Should have received an exception trying to register
interest");
- } catch (NoSubscriptionServersAvailableException expected) {
- // this is expected
- }
+ try {
+ region1.registerInterestForAllKeys();
+ fail("Should have received an exception trying to register interest");
+ } catch (NoSubscriptionServersAvailableException expected) {
+ // this is expected
}
});
- vm1.invoke(new SerializableRunnable() {
- @Override
- public void run() {
- Cache cache = getCache();
- AttributesFactory factory = new AttributesFactory();
- factory.setScope(Scope.DISTRIBUTED_ACK);
- cache.createRegion("region", factory.create());
- CacheServer server = cache.addCacheServer();
- server.setPort(port);
- try {
- server.start();
- } catch (IOException e) {
- Assert.fail("couldn't start server", e);
- }
+ vm1.invoke(() -> {
+ Cache cache = getCache();
+ RegionFactory regionFactory = cache.createRegionFactory();
+ regionFactory.setScope(Scope.DISTRIBUTED_ACK);
+ regionFactory.create("region");
+ CacheServer server = cache.addCacheServer();
+ server.setPort(port);
+ try {
+ server.start();
+ } catch (IOException e) {
+ fail("couldn't start server", e);
}
});
- vm0.invoke(new SerializableRunnable() {
- @Override
- public void run() {
- Cache cache = getCache();
- final Region region = cache.getRegion("region");
- GeodeAwaitility.await().untilAsserted(new WaitCriterion() {
-
- @Override
- public String description() {
- return "Wait for register interest to succeed";
- }
-
- @Override
- public boolean done() {
- try {
- region.registerInterest("ALL_KEYS");
- } catch (NoSubscriptionServersAvailableException e) {
- return false;
- }
- return true;
- }
-
- });
- }
+ vm0.invoke(() -> {
+ Cache cache = getCache();
+ final Region region = cache.getRegion("region");
+ GeodeAwaitility.await("Wait for register interest to succeed").until(()
-> {
+ try {
+ region.registerInterestForAllKeys();
Review Comment:
alternatively:
```java
await("Wait for register interest to succeed")
.untilAsserted(() -> assertDoesNotThrow(() ->
region.registerInterestForAllKeys()));
```
##########
geode-cq/src/distributedTest/java/org/apache/geode/internal/cache/tier/sockets/DurableClientTestBase.java:
##########
@@ -216,33 +208,35 @@ void verifyDurableClientPresence(int
durableClientTimeout, String durableClientI
});
}
+ void waitUntilHARegionQueueSizeIsZero(VM serverVM) {
+ serverVM.invoke(() -> await().atMost(60, SECONDS)
+ .until(() -> getClientProxy().getHARegionQueue().size() == 0));
+ }
+
public void closeDurableClient() {
durableClientVM.invoke((SerializableRunnableIF)
CacheServerTestUtil::closeCache);
}
public void disconnectDurableClient(boolean keepAlive) {
- printClientProxyState("Before");
+ // printClientProxyState("Before");
durableClientVM.invoke("close durable client cache",
() -> CacheServerTestUtil.closeCache(keepAlive));
- await()
- .until(CacheServerTestUtil::getCache, nullValue());
- printClientProxyState("after");
+ await().until(CacheServerTestUtil::getCache, nullValue());
+ // printClientProxyState("after");
}
private void printClientProxyState(String st) {
- CacheSerializableRunnable s =
- new CacheSerializableRunnable("Logging CCCP and ServerConnection
state") {
- @Override
- public void run2() throws CacheException {
- // TODO Auto-generated method stub
- CacheServerTestUtil.getCache().getLogger()
- .info(st + " CCP states: " + getAllClientProxyState());
- CacheServerTestUtil.getCache().getLogger().info(st + " CHM states:
"
- + printMap(
-
ClientHealthMonitor.getInstance().getConnectedClients(null)));
- }
- };
- server1VM.invoke(s);
+ server1VM.invoke("Logging CCCP and ServerConnection state", new
CacheSerializableRunnable() {
+ @Override
+ public void run2() throws CacheException {
+ // TODO Auto-generated method stub
Review Comment:
Useless comment
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
