kirklund commented on code in PR #7697:
URL: https://github.com/apache/geode/pull/7697#discussion_r890433012


##########
geode-core/src/distributedTest/java/org/apache/geode/cache30/ReconnectDUnitTest.java:
##########
@@ -320,143 +304,128 @@ public void doTestReconnectOnForcedDisconnect(final 
boolean createInAppToo) thro
     Invoke
         .invokeInEveryVM(() -> 
DistributedTestUtils.deleteLocatorStateFile(locPort, secondLocPort));
 
-
     final String xmlFileLoc = (new File(".")).getAbsolutePath();
 
-    SerializableCallable create1 =
-        new SerializableCallable("Create Cache and Regions from cache.xml") {
-          @Override
-          public Object call() throws CacheException {
-            locatorPort = locPort;
-            Properties props = getDistributedSystemProperties();
-            props.put(CACHE_XML_FILE, xmlFileLoc + fileSeparator + 
"MyDisconnect-cache.xml");
-            props.put(MAX_WAIT_TIME_RECONNECT, "1000");
-            cache = (InternalCache) new CacheFactory(props).create();
-            Region myRegion = cache.getRegion("root" + SEPARATOR + "myRegion");
-            ReconnectDUnitTest.savedSystem = cache.getDistributedSystem();
-            myRegion.put("MyKey1", "MyValue1");
-            return savedSystem.getDistributedMember();
-          }
-        };
+    SerializableCallableIF<DistributedMember> create1 = () -> {
+      locatorPort = locPort;
+      Properties props = getDistributedSystemProperties();
+      props.put(CACHE_XML_FILE, xmlFileLoc + fileSeparator + 
"MyDisconnect-cache.xml");
+      props.put(MAX_WAIT_TIME_RECONNECT, "1000");
+      cache = (InternalCache) new CacheFactory(props).create();
+      Region myRegion = cache.getRegion("root" + SEPARATOR + "myRegion");
+      ReconnectDUnitTest.savedSystem = cache.getDistributedSystem();
+      myRegion.put("MyKey1", "MyValue1");
+      return savedSystem.getDistributedMember();
+    };
 
-    SerializableCallable create2 =
-        new SerializableCallable("Create Cache and Regions from cache.xml") {
+    SerializableCallableIF<DistributedMember> create2 = () -> {
+      locatorPort = locPort;
+      final Properties props = getDistributedSystemProperties();
+      props.put(CACHE_XML_FILE, xmlFileLoc + fileSeparator + 
"MyDisconnect-cache.xml");
+      props.put(MAX_WAIT_TIME_RECONNECT, "5000");
+      props.put(START_LOCATOR, "localhost[" + secondLocPort + "]");
+      props.put(LOCATORS, props.get(LOCATORS) + ",localhost[" + secondLocPort 
+ "]");
+      getSystem(props);
+      cache = getCache();
+      ReconnectDUnitTest.savedSystem = cache.getDistributedSystem();
+      Region myRegion = cache.getRegion("root" + SEPARATOR + "myRegion");
+      myRegion.put("Mykey2", "MyValue2");
+      assertNotNull(myRegion.get("MyKey1"));
+      if (createInAppToo) {
+        Thread recreateCacheThread = new 
Thread("ReconnectDUnitTest.createInAppThread") {
           @Override
-          public Object call() throws CacheException {
-            locatorPort = locPort;
-            final Properties props = getDistributedSystemProperties();
-            props.put(CACHE_XML_FILE, xmlFileLoc + fileSeparator + 
"MyDisconnect-cache.xml");
-            props.put(MAX_WAIT_TIME_RECONNECT, "5000");
-            props.put(START_LOCATOR, "localhost[" + secondLocPort + "]");
-            props.put(LOCATORS, props.get(LOCATORS) + ",localhost[" + 
secondLocPort + "]");
-            getSystem(props);
-            cache = getCache();
-            ReconnectDUnitTest.savedSystem = cache.getDistributedSystem();
-            Region myRegion = cache.getRegion("root" + SEPARATOR + "myRegion");
-            myRegion.put("Mykey2", "MyValue2");
-            assertNotNull(myRegion.get("MyKey1"));
-            if (createInAppToo) {
-              Thread recreateCacheThread = new 
Thread("ReconnectDUnitTest.createInAppThread") {
-                @Override
-                public void run() {
-                  while (!cache.isClosed()) {
-                    Wait.pause(100);
-                  }
-                  try {
-                    cache = (InternalCache) new CacheFactory(props).create();
-                    System.err.println(
-                        "testReconnectCollidesWithApplication failed - 
application thread was able to create a cache");
-                  } catch (IllegalStateException cacheExists) {
-                    // expected
-                  }
-                }
-              };
-              recreateCacheThread.setDaemon(true);
-              recreateCacheThread.start();
+          public void run() {
+            while (!cache.isClosed()) {
+              Wait.pause(100);
+            }
+            try {
+              cache = (InternalCache) new CacheFactory(props).create();
+              System.err.println(
+                  "testReconnectCollidesWithApplication failed - application 
thread was able to create a cache");
+            } catch (IllegalStateException cacheExists) {
+              // expected

Review Comment:
   I know it's hassle but this sort of thing is probably important. But I'm 
also worried that it may have been dumbed down to a println because it might 
intermittently fail (due to timing). I'd probably look at the history of those 
lines to see if that happened. If not, then consider changing it to an 
assertion.



-- 
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]

Reply via email to