Address PR review.

Signed-off-by: Yury-Fridlyand <[email protected]>
valkey-io · Oct 27, 2024 · c36d5a4 · c36d5a4
1 parent 8410910
commit c36d5a4
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 26 deletions.
diff --git a/java/integTest/src/test/java/glide/TestUtilities.java b/java/integTest/src/test/java/glide/TestUtilities.java
@@ -27,6 +27,8 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
+import java.util.concurrent.CompletableFuture;
+import java.util.function.Supplier;
 import java.util.stream.Collectors;
 import lombok.NonNull;
 import lombok.SneakyThrows;
@@ -397,32 +399,19 @@ public static String createLongRunningLuaScript(int timeout, boolean readOnly) {
     /**
      * Lock test until server completes a script/function execution.
      *
-     * @param function true if need to kill a function, false to kill a script.
+     * @param lambda Client api reference to use for checking the server.
      */
-    @SneakyThrows
-    public static void waitForNotBusy(BaseClient client, boolean function) {
+    public static void waitForNotBusy(Supplier<CompletableFuture<?>> lambda) {
         // If function wasn't killed, and it didn't time out - it blocks the server and cause rest
         // test to fail.
         boolean isBusy = true;
-        int timeout = 10000; // 10 sec - to avoid infinite locking
         do {
-            if (timeout <= 0) fail();
             try {
-                if (client instanceof GlideClusterClient) {
-                    if (function) ((GlideClusterClient) client).functionKill().get();
-                    else ((GlideClusterClient) client).scriptKill().get();
-                } else if (client instanceof GlideClient) {
-                    if (function) ((GlideClient) client).functionKill().get();
-                    else ((GlideClient) client).scriptKill().get();
-                }
-                timeout -= 100;
+                lambda.get().get();
             } catch (Exception busy) {
                 // should throw `notbusy` error, because the function should be killed before
                 if (busy.getMessage().toLowerCase().contains("notbusy")) {
                     isBusy = false;
-                } else {
-                    Thread.sleep(100);
-                    timeout -= 100;
                 }
             }
         } while (isBusy);

diff --git a/java/integTest/src/test/java/glide/cluster/CommandTests.java b/java/integTest/src/test/java/glide/cluster/CommandTests.java
@@ -1808,7 +1808,7 @@ public void functionKill_no_write_without_route() {
 
                 assertTrue(functionKilled);
             } finally {
-                waitForNotBusy(clusterClient, true);
+                waitForNotBusy(clusterClient::functionKill);
             }
         }
     }
@@ -1863,7 +1863,7 @@ public void functionKillBinary_no_write_without_route() {
 
                 assertTrue(functionKilled);
             } finally {
-                waitForNotBusy(clusterClient, true);
+                waitForNotBusy(clusterClient::functionKill);
             }
         }
     }
@@ -1915,7 +1915,7 @@ public void functionKill_no_write_with_route(boolean singleNodeRoute) {
 
                 assertTrue(functionKilled);
             } finally {
-                waitForNotBusy(clusterClient, true);
+                waitForNotBusy(clusterClient::functionKill);
             }
         }
     }
@@ -1969,7 +1969,7 @@ public void functionKillBinary_no_write_with_route(boolean singleNodeRoute) {
 
                 assertTrue(functionKilled);
             } finally {
-                waitForNotBusy(clusterClient, true);
+                waitForNotBusy(clusterClient::functionKill);
             }
         }
     }
@@ -3276,7 +3276,7 @@ public void scriptKill_with_route() {
 
                 assertTrue(scriptKilled);
             } finally {
-                waitForNotBusy(clusterClient, false);
+                waitForNotBusy(clusterClient::scriptKill);
             }
         }
 
@@ -3297,7 +3297,7 @@ public void scriptKill_unkillable() {
         String key = UUID.randomUUID().toString();
         RequestRoutingConfiguration.Route route =
                 new RequestRoutingConfiguration.SlotKeyRoute(key, PRIMARY);
-        String code = createLongRunningLuaScript(5, false);
+        String code = createLongRunningLuaScript(6, false);
         Script script = new Script(code, false);
 
         CompletableFuture<Object> promise = new CompletableFuture<>();

diff --git a/java/integTest/src/test/java/glide/standalone/CommandTests.java b/java/integTest/src/test/java/glide/standalone/CommandTests.java
@@ -784,7 +784,7 @@ public void functionKill_no_write() {
 
                 assertTrue(functionKilled);
             } finally {
-                waitForNotBusy(regularClient, true);
+                waitForNotBusy(regularClient::functionKill);
             }
         }
     }
@@ -835,7 +835,7 @@ public void functionKillBinary_no_write() {
 
                 assertTrue(functionKilled);
             } finally {
-                waitForNotBusy(regularClient, true);
+                waitForNotBusy(regularClient::functionKill);
             }
         }
     }
@@ -1681,7 +1681,7 @@ public void scriptKill() {
 
                 assertTrue(scriptKilled);
             } finally {
-                waitForNotBusy(regularClient, false);
+                waitForNotBusy(regularClient::scriptKill);
             }
         }
 
@@ -1700,7 +1700,7 @@ public void scriptKill() {
     @Test
     public void scriptKill_unkillable() {
         String key = UUID.randomUUID().toString();
-        String code = createLongRunningLuaScript(5, false);
+        String code = createLongRunningLuaScript(6, false);
         Script script = new Script(code, false);
 
         CompletableFuture<Object> promise = new CompletableFuture<>();