From 14680d638ef1936e77990bc0572f476036e919fd Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 19 Dec 2017 12:14:53 +0100 Subject: [PATCH] Polishing Improve SLOWLOG test resiliency, add assumption in case the build is slower to not impact our test. Improve test synchronization to reduce test failures. --- src/test/java/com/lambdaworks/SslTest.java | 29 ++++---- .../redis/cluster/AbstractClusterTest.java | 4 +- .../redis/cluster/RedisClusterClientTest.java | 67 ++++++++++--------- .../redis/commands/ServerCommandTest.java | 35 +++++----- .../redis/protocol/CommandArgsTest.java | 48 ++++++++----- 5 files changed, 104 insertions(+), 79 deletions(-) diff --git a/src/test/java/com/lambdaworks/SslTest.java b/src/test/java/com/lambdaworks/SslTest.java index 8391907656..2c2e6ac955 100644 --- a/src/test/java/com/lambdaworks/SslTest.java +++ b/src/test/java/com/lambdaworks/SslTest.java @@ -23,6 +23,7 @@ import java.io.File; import java.security.cert.CertificateException; +import java.util.Arrays; import java.util.List; import java.util.concurrent.ExecutionException; @@ -30,6 +31,7 @@ import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; +import org.springframework.test.util.ReflectionTestUtils; import com.lambdaworks.redis.*; import com.lambdaworks.redis.api.StatefulRedisConnection; @@ -245,22 +247,25 @@ public void pubSubSslAndBreakConnection() throws Exception { redisClient.setOptions(ClientOptions.builder().suspendReconnectOnProtocolFailure(true).build()); RedisPubSubAsyncCommands connection = redisClient.connectPubSub(redisURI).async(); - connection.subscribe("c1").get(); - connection.subscribe("c2").get(); - Thread.sleep(200); - RedisPubSubAsyncCommands connection2 = redisClient.connectPubSub(redisURI).async(); - assertThat(connection2.pubsubChannels().get()).contains("c1", "c2"); - redisURI.setVerifyPeer(true); + connection.subscribe("c1"); + connection.subscribe("c2"); - connection.quit(); - Thread.sleep(500); + Wait.untilTrue(() -> connection2.pubsubChannels().get().containsAll(Arrays.asList("c1", "c2"))).waitOrTimeout(); + + try { + connection.quit().get(); + } catch (Exception e) { + } + + List future = connection2.pubsubChannels().get(); + assertThat(future).doesNotContain("c1", "c2"); - RedisFuture> future = connection2.pubsubChannels(); - assertThat(future.get()).doesNotContain("c1", "c2"); - assertThat(future.isDone()).isEqualTo(true); + RedisChannelHandler channelHandler = (RedisChannelHandler) connection.getStatefulConnection(); + RedisChannelWriter channelWriter = channelHandler.getChannelWriter(); + Wait.untilNotEquals(null, () -> ReflectionTestUtils.getField(channelWriter, "connectionError")).waitOrTimeout(); RedisFuture> defectFuture = connection.pubsubChannels(); @@ -274,7 +279,7 @@ public void pubSubSslAndBreakConnection() throws Exception { assertThat(e).hasRootCauseInstanceOf(CertificateException.class); } - assertThat(defectFuture.isDone()).isEqualTo(true); + assertThat(defectFuture).isDone(); connection.close(); connection2.close(); diff --git a/src/test/java/com/lambdaworks/redis/cluster/AbstractClusterTest.java b/src/test/java/com/lambdaworks/redis/cluster/AbstractClusterTest.java index b5836709f6..1ad8c7ce4b 100644 --- a/src/test/java/com/lambdaworks/redis/cluster/AbstractClusterTest.java +++ b/src/test/java/com/lambdaworks/redis/cluster/AbstractClusterTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2016 the original author or authors. + * Copyright 2011-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -59,7 +59,7 @@ public class AbstractClusterTest extends AbstractTest { public ClusterRule clusterRule = new ClusterRule(clusterClient, port1, port2, port3, port4); @BeforeClass - public static void setupClusterClient() throws Exception { + public static void setupClusterClient() { clusterClient = RedisClusterClient.create(LettuceLists.unmodifiableList(RedisURI.Builder.redis(host, port1).build())); } diff --git a/src/test/java/com/lambdaworks/redis/cluster/RedisClusterClientTest.java b/src/test/java/com/lambdaworks/redis/cluster/RedisClusterClientTest.java index 99fd89597e..5a3741779b 100644 --- a/src/test/java/com/lambdaworks/redis/cluster/RedisClusterClientTest.java +++ b/src/test/java/com/lambdaworks/redis/cluster/RedisClusterClientTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2016 the original author or authors. + * Copyright 2011-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -31,6 +31,7 @@ import org.springframework.test.util.ReflectionTestUtils; import com.lambdaworks.TestClientResources; +import com.lambdaworks.Wait; import com.lambdaworks.redis.*; import com.lambdaworks.redis.api.StatefulConnection; import com.lambdaworks.redis.api.StatefulRedisConnection; @@ -63,8 +64,10 @@ public class RedisClusterClientTest extends AbstractClusterTest { protected RedisAdvancedClusterCommands sync; @BeforeClass - public static void setupClient() throws Exception { + public static void setupClient() { + setupClusterClient(); + client = RedisClient.create(TestClientResources.get(), RedisURI.Builder.redis(host, port1).build()); clusterClient = RedisClusterClient.create(TestClientResources.get(), Collections.singletonList(RedisURI.Builder.redis(host, port1) @@ -79,7 +82,7 @@ public static void shutdownClient() { } @Before - public void before() throws Exception { + public void before() { clusterClient.setOptions(ClusterClientOptions.create()); clusterRule.getClusterClient().reloadPartitions(); @@ -99,7 +102,7 @@ public void before() throws Exception { } @After - public void after() throws Exception { + public void after() { sync.close(); redis1.close(); @@ -110,21 +113,21 @@ public void after() throws Exception { } @Test - public void statefulConnectionFromSync() throws Exception { + public void statefulConnectionFromSync() { RedisAdvancedClusterConnection sync = clusterClient.connectCluster(); assertThat(sync.getStatefulConnection().sync()).isSameAs(sync); sync.close(); } @Test - public void statefulConnectionFromAsync() throws Exception { + public void statefulConnectionFromAsync() { RedisAsyncConnection async = client.connectAsync(); assertThat(async.getStatefulConnection().async()).isSameAs(async); async.close(); } @Test - public void shouldApplyTimeoutOnRegularConnection() throws Exception { + public void shouldApplyTimeoutOnRegularConnection() { clusterClient.setDefaultTimeout(1, TimeUnit.MINUTES); @@ -137,7 +140,7 @@ public void shouldApplyTimeoutOnRegularConnection() throws Exception { } @Test - public void shouldApplyTimeoutOnRegularConnectionUsingCodec() throws Exception { + public void shouldApplyTimeoutOnRegularConnectionUsingCodec() { clusterClient.setDefaultTimeout(1, TimeUnit.MINUTES); @@ -150,7 +153,7 @@ public void shouldApplyTimeoutOnRegularConnectionUsingCodec() throws Exception { } @Test - public void shouldApplyTimeoutOnPubSubConnection() throws Exception { + public void shouldApplyTimeoutOnPubSubConnection() { clusterClient.setDefaultTimeout(1, TimeUnit.MINUTES); @@ -161,7 +164,7 @@ public void shouldApplyTimeoutOnPubSubConnection() throws Exception { } @Test - public void shouldApplyTimeoutOnPubSubConnectionUsingCodec() throws Exception { + public void shouldApplyTimeoutOnPubSubConnectionUsingCodec() { clusterClient.setDefaultTimeout(1, TimeUnit.MINUTES); @@ -177,38 +180,39 @@ public void clusterConnectionShouldSetClientName() throws Exception { StatefulRedisClusterConnection connection = clusterClient.connect(); assertThat(connection.sync().clientGetname()).isEqualTo("my-client"); + Thread.sleep(10); connection.sync().quit(); + Wait.untilTrue(connection::isOpen).waitOrTimeout(); assertThat(connection.sync().clientGetname()).isEqualTo("my-client"); StatefulRedisConnection nodeConnection = connection.getConnection(connection.getPartitions() .getPartition(0).getNodeId()); assertThat(nodeConnection.sync().clientGetname()).isEqualTo("my-client"); - nodeConnection.sync().quit(); - assertThat(nodeConnection.sync().clientGetname()).isEqualTo("my-client"); connection.close(); } @Test - public void pubSubclusterConnectionShouldSetClientName() throws Exception { + public void pubSubclusterConnectionShouldSetClientName() throws InterruptedException { StatefulRedisClusterPubSubConnection connection = clusterClient.connectPubSub(); assertThat(connection.sync().clientGetname()).isEqualTo("my-client"); + Thread.sleep(10); connection.sync().quit(); + Wait.untilTrue(connection::isOpen).waitOrTimeout(); + assertThat(connection.sync().clientGetname()).isEqualTo("my-client"); StatefulRedisConnection nodeConnection = connection.getConnection(connection.getPartitions() .getPartition(0).getNodeId()); assertThat(nodeConnection.sync().clientGetname()).isEqualTo("my-client"); - nodeConnection.sync().quit(); - assertThat(nodeConnection.sync().clientGetname()).isEqualTo("my-client"); connection.close(); } @Test - public void reloadPartitions() throws Exception { + public void reloadPartitions() { assertThat(clusterClient.getPartitions()).hasSize(4); assertThat(clusterClient.getPartitions().getPartition(0).getUri()); @@ -257,7 +261,6 @@ public void testClusteredOperations() throws Exception { assertThat(setD.get()).isEqualTo("OK"); connection.close(); - } @Test @@ -370,7 +373,7 @@ public void testClusterRedirectionLimit() throws Exception { } @Test(expected = RedisException.class) - public void closeConnection() throws Exception { + public void closeConnection() { try (RedisAdvancedClusterCommands connection = clusterClient.connect().sync()) { @@ -431,7 +434,7 @@ public void clusterAuthPingBeforeConnect() throws Exception { } @Test(expected = RedisException.class) - public void clusterNeedsAuthButNotSupplied() throws Exception { + public void clusterNeedsAuthButNotSupplied() { RedisClusterClient clusterClient = RedisClusterClient.create(TestClientResources.get(), RedisURI.Builder.redis(TestSettings.host(), port7).build()); @@ -446,7 +449,7 @@ public void clusterNeedsAuthButNotSupplied() throws Exception { } @Test - public void noClusterNodeAvailable() throws Exception { + public void noClusterNodeAvailable() { RedisClusterClient clusterClient = RedisClusterClient.create(TestClientResources.get(), RedisURI.Builder.redis(host, 40400).build()); @@ -459,7 +462,7 @@ public void noClusterNodeAvailable() throws Exception { } @Test - public void getClusterNodeConnection() throws Exception { + public void getClusterNodeConnection() { RedisClusterNode redis1Node = getOwnPartition(redissync2); @@ -471,7 +474,7 @@ public void getClusterNodeConnection() throws Exception { } @Test - public void operateOnNodeConnection() throws Exception { + public void operateOnNodeConnection() { sync.set(KEY_A, value); sync.set(KEY_B, "d"); @@ -498,7 +501,7 @@ public void testStatefulConnection() throws Exception { } @Test(expected = RedisException.class) - public void getButNoPartitionForSlothash() throws Exception { + public void getButNoPartitionForSlothash() { for (RedisClusterNode redisClusterNode : clusterClient.getPartitions()) { redisClusterNode.setSlots(new ArrayList<>()); @@ -537,7 +540,7 @@ public void readOnlyOnCluster() throws Exception { } @Test - public void getKeysInSlot() throws Exception { + public void getKeysInSlot() { sync.set(KEY_A, value); sync.set(KEY_B, value); @@ -551,7 +554,7 @@ public void getKeysInSlot() throws Exception { } @Test - public void countKeysInSlot() throws Exception { + public void countKeysInSlot() { sync.set(KEY_A, value); sync.set(KEY_B, value); @@ -569,24 +572,24 @@ public void countKeysInSlot() throws Exception { } @Test - public void testClusterCountFailureReports() throws Exception { + public void testClusterCountFailureReports() { RedisClusterNode ownPartition = getOwnPartition(redissync1); assertThat(redissync1.clusterCountFailureReports(ownPartition.getNodeId())).isGreaterThanOrEqualTo(0); } @Test - public void testClusterKeyslot() throws Exception { + public void testClusterKeyslot() { assertThat(redissync1.clusterKeyslot(KEY_A)).isEqualTo(SLOT_A); assertThat(SlotHash.getSlot(KEY_A)).isEqualTo(SLOT_A); } @Test - public void testClusterSaveconfig() throws Exception { + public void testClusterSaveconfig() { assertThat(redissync1.clusterSaveconfig()).isEqualTo("OK"); } @Test - public void testClusterSetConfigEpoch() throws Exception { + public void testClusterSetConfigEpoch() { try { redissync1.clusterSetConfigEpoch(1L); } catch (RedisException e) { @@ -595,7 +598,7 @@ public void testClusterSetConfigEpoch() throws Exception { } @Test - public void testReadFrom() throws Exception { + public void testReadFrom() { StatefulRedisClusterConnection statefulConnection = sync.getStatefulConnection(); assertThat(statefulConnection.getReadFrom()).isEqualTo(ReadFrom.MASTER); @@ -605,12 +608,12 @@ public void testReadFrom() throws Exception { } @Test(expected = IllegalArgumentException.class) - public void testReadFromNull() throws Exception { + public void testReadFromNull() { sync.getStatefulConnection().setReadFrom(null); } @Test - public void testPfmerge() throws Exception { + public void testPfmerge() { RedisAdvancedClusterConnection connection = clusterClient.connectCluster(); assertThat(SlotHash.getSlot("key2660")).isEqualTo(SlotHash.getSlot("key7112")).isEqualTo(SlotHash.getSlot("key8885")); diff --git a/src/test/java/com/lambdaworks/redis/commands/ServerCommandTest.java b/src/test/java/com/lambdaworks/redis/commands/ServerCommandTest.java index 444f30cb78..fdaf30ed93 100644 --- a/src/test/java/com/lambdaworks/redis/commands/ServerCommandTest.java +++ b/src/test/java/com/lambdaworks/redis/commands/ServerCommandTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2016 the original author or authors. + * Copyright 2011-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -31,6 +31,7 @@ import org.junit.runners.MethodSorters; import com.lambdaworks.RedisConditions; +import com.lambdaworks.Wait; import com.lambdaworks.redis.AbstractRedisClientTest; import com.lambdaworks.redis.KillArgs; import com.lambdaworks.redis.RedisConnection; @@ -54,10 +55,10 @@ public void bgrewriteaof() { } @Test - public void bgsave() throws Exception { - while (redis.info().contains("aof_rewrite_in_progress:1")) { - Thread.sleep(100); - } + public void bgsave() { + + Wait.untilTrue(this::noSaveInProgress).waitOrTimeout(); + String msg = "Background saving started"; assertThat(redis.bgsave()).isEqualTo(msg); } @@ -287,9 +288,8 @@ public void lastsave() { @Test public void save() throws Exception { - while (redis.info().contains("aof_rewrite_in_progress:1")) { - Thread.sleep(100); - } + Wait.untilTrue(this::noSaveInProgress).waitOrTimeout(); + assertThat(redis.save()).isEqualTo("OK"); } @@ -325,6 +325,7 @@ public void slaveofNoOne() { @Test @SuppressWarnings("unchecked") public void slowlog() { + long start = System.currentTimeMillis() / 1000; assertThat(redis.configSet("slowlog-log-slower-than", "0")).isEqualTo("OK"); @@ -332,7 +333,7 @@ public void slowlog() { redis.set(key, value); List log = redis.slowlogGet(); - assertThat(log).hasSize(2); + assumeTrue(!log.isEmpty()); List entry = (List) log.get(0); assertThat(entry.size()).isGreaterThanOrEqualTo(4); @@ -341,15 +342,8 @@ public void slowlog() { assertThat(entry.get(2) instanceof Long).isTrue(); assertThat(entry.get(3)).isEqualTo(list("SET", key, value)); - entry = (List) log.get(1); - assertThat(entry.size()).isGreaterThanOrEqualTo(4); - assertThat(entry.get(0) instanceof Long).isTrue(); - assertThat((Long) entry.get(1) >= start).isTrue(); - assertThat(entry.get(2) instanceof Long).isTrue(); - assertThat(entry.get(3)).isEqualTo(list("SLOWLOG", "RESET")); - assertThat(redis.slowlogGet(1)).hasSize(1); - assertThat((long) redis.slowlogLen()).isGreaterThanOrEqualTo(4); + assertThat((long) redis.slowlogLen()).isGreaterThanOrEqualTo(1); redis.configSet("slowlog-log-slower-than", "10000"); } @@ -373,4 +367,11 @@ public void swapdb() { redis.select(2); assertThat(redis.get(key)).isEqualTo("value1"); } + + private boolean noSaveInProgress() { + + String info = redis.info(); + + return !info.contains("aof_rewrite_in_progress:1") && !info.contains("rdb_bgsave_in_progress:1"); + } } diff --git a/src/test/java/com/lambdaworks/redis/protocol/CommandArgsTest.java b/src/test/java/com/lambdaworks/redis/protocol/CommandArgsTest.java index 68bf11b711..1de42e7d90 100644 --- a/src/test/java/com/lambdaworks/redis/protocol/CommandArgsTest.java +++ b/src/test/java/com/lambdaworks/redis/protocol/CommandArgsTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2016 the original author or authors. + * Copyright 2011-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -36,15 +36,15 @@ public class CommandArgsTest { private Utf8StringCodec codec = new Utf8StringCodec(); @Test - public void getFirstIntegerShouldReturnNull() throws Exception { + public void getFirstIntegerShouldReturnNull() { - CommandArgs args = new CommandArgs<>(codec); + CommandArgs args = new CommandArgs<>(codec).add("foo"); assertThat(CommandArgsAccessor.getFirstInteger(args)).isNull(); } @Test - public void getFirstIntegerShouldReturnFirstInteger() throws Exception { + public void getFirstIntegerShouldReturnFirstInteger() { CommandArgs args = new CommandArgs<>(codec).add(1L).add(127).add(128).add(129).add(0).add(-1); @@ -52,7 +52,7 @@ public void getFirstIntegerShouldReturnFirstInteger() throws Exception { } @Test - public void getFirstIntegerShouldReturnFirstNegativeInteger() throws Exception { + public void getFirstIntegerShouldReturnFirstNegativeInteger() { CommandArgs args = new CommandArgs<>(codec).add(-1L).add(-127).add(-128).add(-129); @@ -60,15 +60,15 @@ public void getFirstIntegerShouldReturnFirstNegativeInteger() throws Exception { } @Test - public void getFirstStringShouldReturnNull() throws Exception { + public void getFirstStringShouldReturnNull() { - CommandArgs args = new CommandArgs<>(codec); + CommandArgs args = new CommandArgs<>(codec).add(1); assertThat(CommandArgsAccessor.getFirstString(args)).isNull(); } @Test - public void getFirstStringShouldReturnFirstString() throws Exception { + public void getFirstStringShouldReturnFirstString() { CommandArgs args = new CommandArgs<>(codec).add("one").add("two"); @@ -76,15 +76,31 @@ public void getFirstStringShouldReturnFirstString() throws Exception { } @Test - public void getFirstEncodedKeyShouldReturnNull() throws Exception { + public void getFirstCharArrayShouldReturnCharArray() { - CommandArgs args = new CommandArgs<>(codec); + CommandArgs args = new CommandArgs<>(codec).add(1L).add("two".toCharArray()); + + assertThat(CommandArgsAccessor.getFirstCharArray(args)).isEqualTo("two".toCharArray()); + } + + @Test + public void getFirstCharArrayShouldReturnNull() { + + CommandArgs args = new CommandArgs<>(codec).add(1L); + + assertThat(CommandArgsAccessor.getFirstCharArray(args)).isNull(); + } + + @Test + public void getFirstEncodedKeyShouldReturnNull() { + + CommandArgs args = new CommandArgs<>(codec).add(1L); assertThat(CommandArgsAccessor.getFirstString(args)).isNull(); } @Test - public void getFirstEncodedKeyShouldReturnFirstKey() throws Exception { + public void getFirstEncodedKeyShouldReturnFirstKey() { CommandArgs args = new CommandArgs<>(codec).addKey("one").addKey("two"); @@ -92,7 +108,7 @@ public void getFirstEncodedKeyShouldReturnFirstKey() throws Exception { } @Test - public void addValues() throws Exception { + public void addValues() { CommandArgs args = new CommandArgs<>(codec).addValues(Arrays.asList("1", "2")); @@ -106,7 +122,7 @@ public void addValues() throws Exception { } @Test - public void addByte() throws Exception { + public void addByte() { CommandArgs args = new CommandArgs<>(codec).add("one".getBytes()); @@ -165,7 +181,7 @@ public void addKeyUsingDirectByteCodec() throws Exception { } @Test - public void addByteUsingByteCodec() throws Exception { + public void addByteUsingByteCodec() { CommandArgs args = new CommandArgs<>(ByteArrayCodec.INSTANCE) .add("one".getBytes()); @@ -180,7 +196,7 @@ public void addByteUsingByteCodec() throws Exception { } @Test - public void addValueUsingByteCodec() throws Exception { + public void addValueUsingByteCodec() { CommandArgs args = new CommandArgs<>(ByteArrayCodec.INSTANCE) .addValue("one".getBytes()); @@ -195,7 +211,7 @@ public void addValueUsingByteCodec() throws Exception { } @Test - public void addKeyUsingByteCodec() throws Exception { + public void addKeyUsingByteCodec() { CommandArgs args = new CommandArgs<>(ByteArrayCodec.INSTANCE) .addValue("one".getBytes());