From 96526d2ef8bb8bee3448ec2fd9fe9b83f998eb70 Mon Sep 17 00:00:00 2001 From: atakavci Date: Fri, 27 Sep 2024 16:05:32 +0300 Subject: [PATCH 1/7] fix flaky test - dumy set-get to gain time --- .../jedis/csc/UnifiedJedisClientSideCacheTestBase.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/test/java/redis/clients/jedis/csc/UnifiedJedisClientSideCacheTestBase.java b/src/test/java/redis/clients/jedis/csc/UnifiedJedisClientSideCacheTestBase.java index 388113307b..566f04f9b5 100644 --- a/src/test/java/redis/clients/jedis/csc/UnifiedJedisClientSideCacheTestBase.java +++ b/src/test/java/redis/clients/jedis/csc/UnifiedJedisClientSideCacheTestBase.java @@ -52,6 +52,9 @@ public void simpleWithSimpleMap() { assertEquals("bar", jedis.get("foo")); assertEquals(1, cache.getSize()); control.del("foo"); + control.set("dummyKey", "dummyValue"); + control.get("dummyKey"); + control.del("dummyKey"); assertEquals(1, cache.getSize()); assertNull(jedis.get("foo")); assertEquals(1, cache.getSize()); @@ -80,6 +83,9 @@ public void flushAllWithSimpleMap() { assertEquals("bar", jedis.get("foo")); assertEquals(1, cache.getSize()); control.flushAll(); + control.set("dummyKey", "dummyValue"); + control.get("dummyKey"); + control.del("dummyKey"); assertEquals(1, cache.getSize()); assertNull(jedis.get("foo")); assertEquals(1, cache.getSize()); From b702cee4072c3c7aa7a56cef1d7ad7b673b1f311 Mon Sep 17 00:00:00 2001 From: atakavci <58048133+atakavci@users.noreply.github.com> Date: Wed, 2 Oct 2024 15:48:51 +0300 Subject: [PATCH 2/7] adding same commands for 'simple' --- .../jedis/csc/UnifiedJedisClientSideCacheTestBase.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/test/java/redis/clients/jedis/csc/UnifiedJedisClientSideCacheTestBase.java b/src/test/java/redis/clients/jedis/csc/UnifiedJedisClientSideCacheTestBase.java index 566f04f9b5..9e56005559 100644 --- a/src/test/java/redis/clients/jedis/csc/UnifiedJedisClientSideCacheTestBase.java +++ b/src/test/java/redis/clients/jedis/csc/UnifiedJedisClientSideCacheTestBase.java @@ -39,6 +39,10 @@ public void simple() { control.set("foo", "bar"); assertEquals("bar", jedis.get("foo")); control.del("foo"); + // These dummyKey operations just to gain some time for arrival of invalidation message on connection + control.set("dummyKey", "dummyValue"); + control.get("dummyKey"); + control.del("dummyKey"); assertNull(jedis.get("foo")); } } @@ -52,6 +56,7 @@ public void simpleWithSimpleMap() { assertEquals("bar", jedis.get("foo")); assertEquals(1, cache.getSize()); control.del("foo"); + // These dummyKey operations just to gain some time for arrival of invalidation message on connection control.set("dummyKey", "dummyValue"); control.get("dummyKey"); control.del("dummyKey"); @@ -83,6 +88,7 @@ public void flushAllWithSimpleMap() { assertEquals("bar", jedis.get("foo")); assertEquals(1, cache.getSize()); control.flushAll(); + // These dummyKey operations just to gain some time for arrival of invalidation message on connection control.set("dummyKey", "dummyValue"); control.get("dummyKey"); control.del("dummyKey"); From 8b8dc700d64bcd743821b3d5bde2b4688f7f2db6 Mon Sep 17 00:00:00 2001 From: atakavci Date: Thu, 10 Oct 2024 19:12:40 +0300 Subject: [PATCH 3/7] introdue tryAssert in CSC tests --- .../UnifiedJedisClientSideCacheTestBase.java | 26 ++++------------- .../redis/clients/jedis/util/AssertUtil.java | 29 +++++++++++++++++-- 2 files changed, 33 insertions(+), 22 deletions(-) diff --git a/src/test/java/redis/clients/jedis/csc/UnifiedJedisClientSideCacheTestBase.java b/src/test/java/redis/clients/jedis/csc/UnifiedJedisClientSideCacheTestBase.java index 9e56005559..88b8c244c9 100644 --- a/src/test/java/redis/clients/jedis/csc/UnifiedJedisClientSideCacheTestBase.java +++ b/src/test/java/redis/clients/jedis/csc/UnifiedJedisClientSideCacheTestBase.java @@ -2,16 +2,14 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotSame; -import static org.junit.Assert.assertNull; - import java.util.Arrays; import java.util.List; - import org.junit.After; import org.junit.Before; import org.junit.Test; import redis.clients.jedis.UnifiedJedis; +import redis.clients.jedis.util.AssertUtil; public abstract class UnifiedJedisClientSideCacheTestBase { @@ -39,11 +37,7 @@ public void simple() { control.set("foo", "bar"); assertEquals("bar", jedis.get("foo")); control.del("foo"); - // These dummyKey operations just to gain some time for arrival of invalidation message on connection - control.set("dummyKey", "dummyValue"); - control.get("dummyKey"); - control.del("dummyKey"); - assertNull(jedis.get("foo")); + AssertUtil.tryAssert(() -> jedis.get("foo"), null, 50, 100); } } @@ -56,14 +50,8 @@ public void simpleWithSimpleMap() { assertEquals("bar", jedis.get("foo")); assertEquals(1, cache.getSize()); control.del("foo"); - // These dummyKey operations just to gain some time for arrival of invalidation message on connection - control.set("dummyKey", "dummyValue"); - control.get("dummyKey"); - control.del("dummyKey"); assertEquals(1, cache.getSize()); - assertNull(jedis.get("foo")); - assertEquals(1, cache.getSize()); - assertNull(jedis.get("foo")); + AssertUtil.tryAssert(() -> jedis.get("foo"), null, 50, 100); assertEquals(1, cache.getSize()); } } @@ -75,7 +63,7 @@ public void flushAll() { control.set("foo", "bar"); assertEquals("bar", jedis.get("foo")); control.flushAll(); - assertNull(jedis.get("foo")); + AssertUtil.tryAssert(() -> jedis.get("foo"), null, 50, 100); } } @@ -93,9 +81,7 @@ public void flushAllWithSimpleMap() { control.get("dummyKey"); control.del("dummyKey"); assertEquals(1, cache.getSize()); - assertNull(jedis.get("foo")); - assertEquals(1, cache.getSize()); - assertNull(jedis.get("foo")); + AssertUtil.tryAssert(() -> jedis.get("foo"), null, 50, 100); assertEquals(1, cache.getSize()); } } @@ -106,7 +92,7 @@ public void cacheNotEmptyTest() { Cache cache = jedis.getCache(); control.set("foo", "bar"); assertEquals(0, cache.getSize()); - assertEquals("bar", jedis.get("foo")); + AssertUtil.tryAssert(() -> jedis.get("foo"), "bar", 50, 100); assertEquals(1, cache.getSize()); } } diff --git a/src/test/java/redis/clients/jedis/util/AssertUtil.java b/src/test/java/redis/clients/jedis/util/AssertUtil.java index 110be7e48e..69cb9da1f5 100644 --- a/src/test/java/redis/clients/jedis/util/AssertUtil.java +++ b/src/test/java/redis/clients/jedis/util/AssertUtil.java @@ -7,10 +7,10 @@ import java.util.Collection; import java.util.Iterator; import java.util.List; -import java.util.Map; import java.util.Objects; import java.util.Set; - +import java.util.concurrent.Callable; +import java.util.concurrent.TimeUnit; import org.junit.ComparisonFailure; import redis.clients.jedis.RedisProtocol; @@ -149,4 +149,29 @@ private static void assertPipelineSyncAllSet(Set expected, Set actual) { assertEquals(expected, actual); } } + + public static void tryAssert(Callable callable, T expected, long interval, int iterations) { + int attempts = 0; + + while (attempts++ < iterations) { + + T result; + try { + result = callable.call(); + } catch (Exception e) { + throw new AssertionError("Callable threw an exception", e); + } + + if (Objects.equals(expected, result)) { + return; + } + + try { + TimeUnit.MILLISECONDS.sleep(interval); + } catch (InterruptedException e) { + throw new AssertionError("Thread was interrupted", e); + } + } + throw new AssertionError("tryAssert failed after " + iterations + " attempts"); + } } From d3704411c25df3a4f1a29906fca01edaa6de4b96 Mon Sep 17 00:00:00 2001 From: atakavci Date: Thu, 10 Oct 2024 20:12:45 +0300 Subject: [PATCH 4/7] remove leftovers --- .../jedis/csc/UnifiedJedisClientSideCacheTestBase.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/test/java/redis/clients/jedis/csc/UnifiedJedisClientSideCacheTestBase.java b/src/test/java/redis/clients/jedis/csc/UnifiedJedisClientSideCacheTestBase.java index 88b8c244c9..a6692309ef 100644 --- a/src/test/java/redis/clients/jedis/csc/UnifiedJedisClientSideCacheTestBase.java +++ b/src/test/java/redis/clients/jedis/csc/UnifiedJedisClientSideCacheTestBase.java @@ -76,10 +76,6 @@ public void flushAllWithSimpleMap() { assertEquals("bar", jedis.get("foo")); assertEquals(1, cache.getSize()); control.flushAll(); - // These dummyKey operations just to gain some time for arrival of invalidation message on connection - control.set("dummyKey", "dummyValue"); - control.get("dummyKey"); - control.del("dummyKey"); assertEquals(1, cache.getSize()); AssertUtil.tryAssert(() -> jedis.get("foo"), null, 50, 100); assertEquals(1, cache.getSize()); From d39cc0b83bdc9f5a60a5b39a530a3998c609ee08 Mon Sep 17 00:00:00 2001 From: atakavci Date: Mon, 14 Oct 2024 11:19:43 +0300 Subject: [PATCH 5/7] introduce awaitility for polling --- pom.xml | 7 +++++ .../UnifiedJedisClientSideCacheTestBase.java | 19 ++++++++----- .../redis/clients/jedis/util/AssertUtil.java | 27 ------------------- 3 files changed, 20 insertions(+), 33 deletions(-) diff --git a/pom.xml b/pom.xml index 2b8b56f20b..4b839b8b9b 100644 --- a/pom.xml +++ b/pom.xml @@ -143,6 +143,13 @@ test + + org.awaitility + awaitility + 4.2.2 + test + + io.github.resilience4j diff --git a/src/test/java/redis/clients/jedis/csc/UnifiedJedisClientSideCacheTestBase.java b/src/test/java/redis/clients/jedis/csc/UnifiedJedisClientSideCacheTestBase.java index a6692309ef..ba8358242c 100644 --- a/src/test/java/redis/clients/jedis/csc/UnifiedJedisClientSideCacheTestBase.java +++ b/src/test/java/redis/clients/jedis/csc/UnifiedJedisClientSideCacheTestBase.java @@ -2,14 +2,17 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotSame; +import static org.awaitility.Awaitility.await; + import java.util.Arrays; import java.util.List; +import java.util.concurrent.TimeUnit; + import org.junit.After; import org.junit.Before; import org.junit.Test; import redis.clients.jedis.UnifiedJedis; -import redis.clients.jedis.util.AssertUtil; public abstract class UnifiedJedisClientSideCacheTestBase { @@ -37,7 +40,8 @@ public void simple() { control.set("foo", "bar"); assertEquals("bar", jedis.get("foo")); control.del("foo"); - AssertUtil.tryAssert(() -> jedis.get("foo"), null, 50, 100); + await().atMost(5, TimeUnit.SECONDS).pollInterval(50, TimeUnit.MILLISECONDS) + .until(() -> jedis.get("foo") == null); } } @@ -51,7 +55,8 @@ public void simpleWithSimpleMap() { assertEquals(1, cache.getSize()); control.del("foo"); assertEquals(1, cache.getSize()); - AssertUtil.tryAssert(() -> jedis.get("foo"), null, 50, 100); + await().atMost(5, TimeUnit.SECONDS).pollInterval(50, TimeUnit.MILLISECONDS) + .until(() -> jedis.get("foo") == null); assertEquals(1, cache.getSize()); } } @@ -63,7 +68,8 @@ public void flushAll() { control.set("foo", "bar"); assertEquals("bar", jedis.get("foo")); control.flushAll(); - AssertUtil.tryAssert(() -> jedis.get("foo"), null, 50, 100); + await().atMost(5, TimeUnit.SECONDS).pollInterval(50, TimeUnit.MILLISECONDS) + .until(() -> jedis.get("foo") == null); } } @@ -77,7 +83,8 @@ public void flushAllWithSimpleMap() { assertEquals(1, cache.getSize()); control.flushAll(); assertEquals(1, cache.getSize()); - AssertUtil.tryAssert(() -> jedis.get("foo"), null, 50, 100); + await().atMost(5, TimeUnit.SECONDS).pollInterval(50, TimeUnit.MILLISECONDS) + .until(() -> jedis.get("foo") == null); assertEquals(1, cache.getSize()); } } @@ -88,7 +95,7 @@ public void cacheNotEmptyTest() { Cache cache = jedis.getCache(); control.set("foo", "bar"); assertEquals(0, cache.getSize()); - AssertUtil.tryAssert(() -> jedis.get("foo"), "bar", 50, 100); + assertEquals("bar", (jedis.get("foo"))); assertEquals(1, cache.getSize()); } } diff --git a/src/test/java/redis/clients/jedis/util/AssertUtil.java b/src/test/java/redis/clients/jedis/util/AssertUtil.java index 69cb9da1f5..474b34e74b 100644 --- a/src/test/java/redis/clients/jedis/util/AssertUtil.java +++ b/src/test/java/redis/clients/jedis/util/AssertUtil.java @@ -9,8 +9,6 @@ import java.util.List; import java.util.Objects; import java.util.Set; -import java.util.concurrent.Callable; -import java.util.concurrent.TimeUnit; import org.junit.ComparisonFailure; import redis.clients.jedis.RedisProtocol; @@ -149,29 +147,4 @@ private static void assertPipelineSyncAllSet(Set expected, Set actual) { assertEquals(expected, actual); } } - - public static void tryAssert(Callable callable, T expected, long interval, int iterations) { - int attempts = 0; - - while (attempts++ < iterations) { - - T result; - try { - result = callable.call(); - } catch (Exception e) { - throw new AssertionError("Callable threw an exception", e); - } - - if (Objects.equals(expected, result)) { - return; - } - - try { - TimeUnit.MILLISECONDS.sleep(interval); - } catch (InterruptedException e) { - throw new AssertionError("Thread was interrupted", e); - } - } - throw new AssertionError("tryAssert failed after " + iterations + " attempts"); - } } From ef8d4a0134953522f07e5f80ef4bb617c9826317 Mon Sep 17 00:00:00 2001 From: atakavci Date: Mon, 14 Oct 2024 11:27:16 +0300 Subject: [PATCH 6/7] nit --- .../clients/jedis/csc/UnifiedJedisClientSideCacheTestBase.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/redis/clients/jedis/csc/UnifiedJedisClientSideCacheTestBase.java b/src/test/java/redis/clients/jedis/csc/UnifiedJedisClientSideCacheTestBase.java index ba8358242c..5346db3809 100644 --- a/src/test/java/redis/clients/jedis/csc/UnifiedJedisClientSideCacheTestBase.java +++ b/src/test/java/redis/clients/jedis/csc/UnifiedJedisClientSideCacheTestBase.java @@ -95,7 +95,7 @@ public void cacheNotEmptyTest() { Cache cache = jedis.getCache(); control.set("foo", "bar"); assertEquals(0, cache.getSize()); - assertEquals("bar", (jedis.get("foo"))); + assertEquals("bar", jedis.get("foo")); assertEquals(1, cache.getSize()); } } From 5cb7b30b5cdbbb2dc5478ca9f4898bb546d62188 Mon Sep 17 00:00:00 2001 From: atakavci Date: Mon, 14 Oct 2024 13:51:13 +0300 Subject: [PATCH 7/7] fix pipelining test untilasserted --- src/test/java/redis/clients/jedis/PipeliningTest.java | 5 ++++- .../jedis/csc/UnifiedJedisClientSideCacheTestBase.java | 9 +++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/test/java/redis/clients/jedis/PipeliningTest.java b/src/test/java/redis/clients/jedis/PipeliningTest.java index 7df000c7a3..fede809686 100644 --- a/src/test/java/redis/clients/jedis/PipeliningTest.java +++ b/src/test/java/redis/clients/jedis/PipeliningTest.java @@ -20,12 +20,14 @@ import java.util.Map; import java.util.Set; import java.util.UUID; +import java.util.concurrent.TimeUnit; import org.hamcrest.Matcher; import org.hamcrest.Matchers; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; +import org.awaitility.Awaitility; import redis.clients.jedis.commands.ProtocolCommand; import redis.clients.jedis.commands.jedis.JedisCommandsTestBase; @@ -342,7 +344,8 @@ public void waitAof() { try (Jedis j = new Jedis(endpoint.getHostAndPort())) { j.auth(endpoint.getPassword()); - assertEquals("aof", j.get("wait")); + Awaitility.await().atMost(5, TimeUnit.SECONDS).pollInterval(50, TimeUnit.MILLISECONDS) + .untilAsserted(() -> assertEquals("aof", j.get("wait"))); } } diff --git a/src/test/java/redis/clients/jedis/csc/UnifiedJedisClientSideCacheTestBase.java b/src/test/java/redis/clients/jedis/csc/UnifiedJedisClientSideCacheTestBase.java index 5346db3809..e07bf578ee 100644 --- a/src/test/java/redis/clients/jedis/csc/UnifiedJedisClientSideCacheTestBase.java +++ b/src/test/java/redis/clients/jedis/csc/UnifiedJedisClientSideCacheTestBase.java @@ -2,6 +2,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertNull; import static org.awaitility.Awaitility.await; import java.util.Arrays; @@ -41,7 +42,7 @@ public void simple() { assertEquals("bar", jedis.get("foo")); control.del("foo"); await().atMost(5, TimeUnit.SECONDS).pollInterval(50, TimeUnit.MILLISECONDS) - .until(() -> jedis.get("foo") == null); + .untilAsserted(() -> assertNull(jedis.get("foo"))); } } @@ -56,7 +57,7 @@ public void simpleWithSimpleMap() { control.del("foo"); assertEquals(1, cache.getSize()); await().atMost(5, TimeUnit.SECONDS).pollInterval(50, TimeUnit.MILLISECONDS) - .until(() -> jedis.get("foo") == null); + .untilAsserted(() -> assertNull(jedis.get("foo"))); assertEquals(1, cache.getSize()); } } @@ -69,7 +70,7 @@ public void flushAll() { assertEquals("bar", jedis.get("foo")); control.flushAll(); await().atMost(5, TimeUnit.SECONDS).pollInterval(50, TimeUnit.MILLISECONDS) - .until(() -> jedis.get("foo") == null); + .untilAsserted(() -> assertNull(jedis.get("foo"))); } } @@ -84,7 +85,7 @@ public void flushAllWithSimpleMap() { control.flushAll(); assertEquals(1, cache.getSize()); await().atMost(5, TimeUnit.SECONDS).pollInterval(50, TimeUnit.MILLISECONDS) - .until(() -> jedis.get("foo") == null); + .untilAsserted(() -> assertNull(jedis.get("foo"))); assertEquals(1, cache.getSize()); } }