From e0afb6fea66c98b86d2fe3c9927e833df4a54c36 Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Fri, 25 Oct 2024 16:00:51 -0700 Subject: [PATCH] Address PR review. Signed-off-by: Yury-Fridlyand --- .../src/test/java/glide/TestUtilities.java | 21 +++++-------------- .../test/java/glide/cluster/CommandTests.java | 12 +++++------ .../java/glide/standalone/CommandTests.java | 8 +++---- 3 files changed, 15 insertions(+), 26 deletions(-) diff --git a/java/integTest/src/test/java/glide/TestUtilities.java b/java/integTest/src/test/java/glide/TestUtilities.java index 7bf23757b2..c6f8ef7201 100644 --- 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> 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 index f06c149ea9..ee2682a259 100644 --- 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 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 index 483480f7f7..f7e4943e3a 100644 --- 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 promise = new CompletableFuture<>();