CURATOR-559 test used InterProcessReadWriteLock - the problem is that…

… the retry can end up in background loop which has its own thread thereby spoiling the test. Instead use a foreground operation for consistent tests
apache · Apr 19, 2020 · 62cd345 · 62cd345
1 parent e73b034
commit 62cd345
Showing 1 changed file with 5 additions and 7 deletions.
diff --git a/curator-recipes/src/test/java/org/apache/curator/connection/TestThreadLocalRetryLoop.java b/curator-recipes/src/test/java/org/apache/curator/connection/TestThreadLocalRetryLoop.java
@@ -22,7 +22,6 @@
 import org.apache.curator.RetrySleeper;
 import org.apache.curator.framework.CuratorFramework;
 import org.apache.curator.framework.CuratorFrameworkFactory;
-import org.apache.curator.framework.recipes.locks.InterProcessReadWriteLock;
 import org.apache.curator.framework.state.ConnectionState;
 import org.apache.curator.retry.RetryNTimes;
 import org.apache.curator.test.compatibility.CuratorTestBase;
@@ -46,7 +45,7 @@ public void testRecursingRetry() throws Exception
         try (CuratorFramework client = newClient(count))
         {
             prep(client);
-            doLock(client);
+            doOperation(client);
             Assert.assertEquals(count.get(), retryCount + 1);    // Curator's retry policy has been off by 1 since inception - we might consider fixing it someday
         }
     }
@@ -62,10 +61,10 @@ public void testThreadedRecursingRetry() throws Exception
             prep(client);
             for ( int i = 0; i < threadQty; ++i )
             {
-                executorService.submit(() -> doLock(client));
+                executorService.submit(() -> doOperation(client));
             }
             executorService.shutdown();
-            executorService.awaitTermination(timing.milliseconds(), TimeUnit.MILLISECONDS);
+            Assert.assertTrue(executorService.awaitTermination(timing.milliseconds(), TimeUnit.MILLISECONDS));
             Assert.assertEquals(count.get(), threadQty * (retryCount + 1));    // Curator's retry policy has been off by 1 since inception - we might consider fixing it someday
         }
     }
@@ -97,12 +96,11 @@ private void prep(CuratorFramework client) throws Exception
         Assert.assertTrue(timing.awaitLatch(lostLatch));
     }
 
-    private Void doLock(CuratorFramework client) throws Exception
+    private Void doOperation(CuratorFramework client) throws Exception
     {
-        InterProcessReadWriteLock lock = new InterProcessReadWriteLock(client, "/test/lock");
         try
         {
-            lock.readLock().acquire();
+            client.checkExists().forPath("/hey");
             Assert.fail("Should have thrown an exception");
         }
         catch ( KeeperException ignore )