[GitHub] [geode] kamilla1201 commented on a change in pull request #7348: GEODE-8705: Introduce classloader isolation for deployments using JBoss Modules.

[GitBox]

kamilla1201 commented on a change in pull request #7348:
URL: https://github.com/apache/geode/pull/7348#discussion_r816109913



##########
File path: 
geode-core/src/main/java/org/apache/geode/internal/net/BufferPool.java
##########
@@ -261,37 +261,37 @@ ByteBuffer expandWriteBufferIfNeeded(BufferType type, 
ByteBuffer existing,
   }
 
   ByteBuffer acquireDirectBuffer(BufferPool.BufferType type, int capacity) {
-    switch (type) {
-      case UNTRACKED:
+    switch (type.toString()) {
+      case "UNTRACKED":
         return ByteBuffer.allocate(capacity);
-      case TRACKED_SENDER:
+      case "TRACKED_SENDER":
         return acquireDirectSenderBuffer(capacity);
-      case TRACKED_RECEIVER:
+      case "TRACKED_RECEIVER":
         return acquireDirectReceiveBuffer(capacity);
     }
     throw new IllegalArgumentException("Unexpected buffer type " + type);
   }
 
   ByteBuffer acquireNonDirectBuffer(BufferPool.BufferType type, int capacity) {
-    switch (type) {
-      case UNTRACKED:
+    switch (type.toString()) {

Review comment:
       What's the benefit of using enum values as strings in switch blocks?

##########
File path: 
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/tier/sockets/InstantiatorPropagationDUnitTest.java
##########
@@ -424,23 +241,24 @@ public static void startServer() {
    * instantiators get propagated to client when client gets connected.
    */
   @Test
-  public void testServerUpFirstClientLater() throws Exception {
+  public void testServerUpFirstClientLater() {
     PORT1 = initServerCache(server1);
 
     unregisterInstantiatorsInAllVMs();
 
-    Wait.pause(3000);
+    GeodeAwaitility.await().atMost(3, TimeUnit.SECONDS)
+        
.untilAsserted(this::assertAllInstantiatorsHaveBeenUnregisteredInAllVMs);
 
-    server1.invoke(InstantiatorPropagationDUnitTest::registerTestObject1);
-    server1.invoke(InstantiatorPropagationDUnitTest::registerTestObject2);
+    server1
+        .invoke(() -> 
InstantiatorPropagationDUnitTest.registerTestObject("TestObject1", -100123));
+    server1
+        .invoke(() -> 
InstantiatorPropagationDUnitTest.registerTestObject("TestObject2", -100122));
 
     server1.invoke(() -> 
InstantiatorPropagationDUnitTest.verifyInstantiators(2));
 
     client1.invoke(() -> InstantiatorPropagationDUnitTest
         .createClientCache(NetworkUtils.getServerHostName(server1.getHost()), 
PORT1));
 
-    // // wait for client2 to come online
-    Wait.pause(3000);

Review comment:
       Since you removed most of the deprecated Wait.pause() calls from this 
file, maybe the last one (on line 295) could be removed too




-- 
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: notifications-unsubscr...@geode.apache.org

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