Skip to content

Commit cc6bde6

Browse files
mp911dechristophstrobl
mp911de
authored and
christophstrobl
committed
DATAREDIS-463 - Improve test synchronization.
Several tests rely heavily on time to perform synchronization. Therfore we remove Thread.sleep from tests which do not require a sleep at all, use a condition where possible and increase timeouts on tests known to fail very likely due to a short Thread.sleep. Original Pull Request: spring-projects#171
1 parent bdea5f6 commit cc6bde6

File tree

5 files changed

+74
-37
lines changed

5 files changed

+74
-37
lines changed

‎src/test/java/org/springframework/data/redis/connection/AbstractConnectionIntegrationTests.java‎

Lines changed: 27 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import java.util.Set;
3636
import java.util.UUID;
3737
import java.util.concurrent.BlockingDeque;
38+
import java.util.concurrent.CountDownLatch;
3839
import java.util.concurrent.LinkedBlockingDeque;
3940
import java.util.concurrent.TimeUnit;
4041
import java.util.concurrent.atomic.AtomicBoolean;
@@ -80,6 +81,7 @@
8081
* @author Jennifer Hickey
8182
* @author Christoph Strobl
8283
* @author Thomas Darimont
84+
* @author Mark Paluch
8385
*/
8486
@ProfileValueSourceConfiguration(RedisTestProfileValueSource.class)
8587
public abstract class AbstractConnectionIntegrationTests {
@@ -343,10 +345,12 @@ public void testScriptKill() throws Exception {
343345
assumeTrue(RedisVersionUtils.atLeast("2.6", byteConnection));
344346
initConnection();
345347
final AtomicBoolean scriptDead = new AtomicBoolean(false);
348+
final CountDownLatch sync = new CountDownLatch(1);
346349
Thread th = new Thread(new Runnable() {
347350
public void run() {
348351
DefaultStringRedisConnection conn2 = new DefaultStringRedisConnection(connectionFactory.getConnection());
349352
try {
353+
sync.countDown();
350354
conn2.eval("local time=1 while time < 10000000000 do time=time+1 end", ReturnType.BOOLEAN, 0);
351355
} catch (DataAccessException e) {
352356
scriptDead.set(true);
@@ -355,7 +359,8 @@ public void run() {
355359
}
356360
});
357361
th.start();
358-
Thread.sleep(1000);
362+
sync.await(2, TimeUnit.SECONDS);
363+
Thread.sleep(200);
359364
connection.scriptKill();
360365
getResults();
361366
assertTrue(waitFor(new TestCondition() {
@@ -380,11 +385,10 @@ public void testScriptFlush() {
380385
@IfProfileValue(name = "runLongTests", value = "true")
381386
public void testPersist() throws Exception {
382387
connection.set("exp3", "true");
383-
actual.add(connection.expire("exp3", 1));
388+
actual.add(connection.expire("exp3", 30));
384389
actual.add(connection.persist("exp3"));
385-
Thread.sleep(1500);
386-
actual.add(connection.exists("exp3"));
387-
verifyResults(Arrays.asList(new Object[] { true, true, true }));
390+
actual.add(connection.ttl("exp3"));
391+
verifyResults(Arrays.asList(new Object[] { true, true, -1L }));
388392
}
389393

390394
@Test
@@ -414,23 +418,20 @@ public void testPsetEx() throws Exception {
414418
@IfProfileValue(name = "runLongTests", value = "true")
415419
public void testBRPopTimeout() throws Exception {
416420
actual.add(connection.bRPop(1, "alist"));
417-
Thread.sleep(1500l);
418421
verifyResults(Arrays.asList(new Object[] { null }));
419422
}
420423

421424
@Test
422425
@IfProfileValue(name = "runLongTests", value = "true")
423426
public void testBLPopTimeout() throws Exception {
424427
actual.add(connection.bLPop(1, "alist"));
425-
Thread.sleep(1500l);
426428
verifyResults(Arrays.asList(new Object[] { null }));
427429
}
428430

429431
@Test
430432
@IfProfileValue(name = "runLongTests", value = "true")
431433
public void testBRPopLPushTimeout() throws Exception {
432434
actual.add(connection.bRPopLPush(1, "alist", "foo"));
433-
Thread.sleep(1500l);
434435
verifyResults(Arrays.asList(new Object[] { null }));
435436
}
436437

@@ -633,12 +634,16 @@ public void onMessage(Message message, byte[] pattern) {
633634

634635
Thread th = new Thread(new Runnable() {
635636
public void run() {
636-
// sleep 1/2 second to let the registration happen
637+
// sync to let the registration happen
638+
waitFor(new TestCondition() {
639+
@Override
640+
public boolean passes() {
641+
return connection.isSubscribed();
642+
}
643+
}, 2000);
637644
try {
638645
Thread.sleep(500);
639-
} catch (InterruptedException ex) {
640-
throw new RuntimeException(ex);
641-
}
646+
} catch (InterruptedException o_O) {}
642647

643648
// open a new connection
644649
RedisConnection connection2 = connectionFactory.getConnection();
@@ -681,12 +686,17 @@ public void onMessage(Message message, byte[] pattern) {
681686

682687
Thread th = new Thread(new Runnable() {
683688
public void run() {
684-
// sleep 1/2 second to let the registration happen
689+
// sync to let the registration happen
690+
waitFor(new TestCondition() {
691+
@Override
692+
public boolean passes() {
693+
return connection.isSubscribed();
694+
}
695+
}, 2000);
696+
685697
try {
686698
Thread.sleep(500);
687-
} catch (InterruptedException ex) {
688-
throw new RuntimeException(ex);
689-
}
699+
} catch (InterruptedException o_O) {}
690700

691701
// open a new connection
692702
RedisConnection connection2 = connectionFactory.getConnection();
@@ -1007,7 +1017,7 @@ public void testRestoreTtl() {
10071017
actual.add(connection.get("testing"));
10081018
connection.restore("testing".getBytes(), 100l, (byte[]) results.get(0));
10091019
verifyResults(Arrays.asList(new Object[] { 1l, null }));
1010-
assertTrue(waitFor(new KeyExpired("testing"), 300l));
1020+
assertTrue(waitFor(new KeyExpired("testing"), 400l));
10111021
}
10121022

10131023
@Test

‎src/test/java/org/springframework/data/redis/connection/lettuce/LettuceClusterConnectionTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ public class LettuceClusterConnectionTests implements ClusterConnectionTests {
8686
public void setUp() {
8787

8888
client = new RedisClusterClient(Builder.redis(CLUSTER_HOST, MASTER_NODE_1_PORT)
89-
.withTimeout(100, TimeUnit.MILLISECONDS).build());
89+
.withTimeout(500, TimeUnit.MILLISECONDS).build());
9090
nativeConnection = client.connectCluster();
9191
clusterConnection = new LettuceClusterConnection(client);
9292
}

‎src/test/java/org/springframework/data/redis/core/RedisTemplateTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -443,7 +443,7 @@ public void testExpireAndGetExpireMillis() {
443443
final K key1 = keyFactory.instance();
444444
V value1 = valueFactory.instance();
445445
redisTemplate.boundValueOps(key1).set(value1);
446-
redisTemplate.expire(key1, 250, TimeUnit.MILLISECONDS);
446+
redisTemplate.expire(key1, 500, TimeUnit.MILLISECONDS);
447447

448448
assertTrue(redisTemplate.getExpire(key1, TimeUnit.MILLISECONDS) > 0l);
449449
// Timeout is longer because expire will be 1 sec if pExpire not supported

‎src/test/java/org/springframework/data/redis/listener/PubSubResubscribeTests.java‎

Lines changed: 38 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2011-2014 the original author or authors.
2+
* Copyright 2011-2016 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -19,6 +19,7 @@
1919
import static org.hamcrest.core.Is.*;
2020
import static org.junit.Assert.*;
2121
import static org.junit.Assume.*;
22+
import static org.springframework.data.redis.SpinBarrier.*;
2223

2324
import java.util.ArrayList;
2425
import java.util.Arrays;
@@ -44,6 +45,7 @@
4445
import org.springframework.data.redis.ConnectionFactoryTracker;
4546
import org.springframework.data.redis.RedisTestProfileValueSource;
4647
import org.springframework.data.redis.SettingsUtils;
48+
import org.springframework.data.redis.TestCondition;
4749
import org.springframework.data.redis.connection.ConnectionUtils;
4850
import org.springframework.data.redis.connection.RedisConnectionFactory;
4951
import org.springframework.data.redis.connection.jedis.JedisConnectionFactory;
@@ -58,6 +60,7 @@
5860
* @author Costin Leau
5961
* @author Jennifer Hickey
6062
* @author Christoph Strobl
63+
* @author Mark Paluch
6164
*/
6265
@RunWith(Parameterized.class)
6366
public class PubSubResubscribeTests {
@@ -149,7 +152,12 @@ public void setUp() throws Exception {
149152
container.afterPropertiesSet();
150153
container.start();
151154

152-
Thread.sleep(1000);
155+
waitFor(new TestCondition() {
156+
@Override
157+
public boolean passes() {
158+
return container.getConnectionFactory().getConnection().isSubscribed();
159+
}
160+
}, 1000);
153161
}
154162

155163
@After
@@ -175,36 +183,47 @@ public void testContainerPatternResubscribe() throws Exception {
175183
container.addMessageListener(anotherListener, new PatternTopic(PATTERN));
176184
container.removeMessageListener(adapter);
177185

186+
// Wait for async subscription tasks to setup
187+
Thread.sleep(400);
188+
178189
// test no messages are sent just to patterns
179190
template.convertAndSend(CHANNEL, payload1);
180191
template.convertAndSend(ANOTHER_CHANNEL, payload2);
181192

182193
// anotherListener receives both messages
183194
List<String> msgs = new ArrayList<String>();
184-
msgs.add(bag2.poll(1, TimeUnit.SECONDS));
185-
msgs.add(bag2.poll(1, TimeUnit.SECONDS));
195+
msgs.add(bag2.poll(500, TimeUnit.MILLISECONDS));
196+
msgs.add(bag2.poll(500, TimeUnit.MILLISECONDS));
186197

187198
assertEquals(2, msgs.size());
188199
assertTrue(msgs.contains(payload1));
189200
assertTrue(msgs.contains(payload2));
190201
msgs.clear();
191202

192203
// unsubscribed adapter did not receive message
193-
assertNull(bag.poll(1, TimeUnit.SECONDS));
204+
assertNull(bag.poll(500, TimeUnit.MILLISECONDS));
194205

195206
// bind original listener on another channel
196207
container.addMessageListener(adapter, new ChannelTopic(ANOTHER_CHANNEL));
197208

209+
// Wait for async subscription tasks to setup
210+
Thread.sleep(400);
211+
198212
template.convertAndSend(CHANNEL, payload1);
199213
template.convertAndSend(ANOTHER_CHANNEL, payload2);
200214

201215
// original listener received only one message on another channel
202-
assertEquals(payload2, bag.poll(1, TimeUnit.SECONDS));
203-
assertNull(bag.poll(1, TimeUnit.SECONDS));
216+
msgs.clear();
217+
msgs.add(bag.poll(500, TimeUnit.MILLISECONDS));
218+
msgs.add(bag.poll(500, TimeUnit.MILLISECONDS));
219+
220+
assertTrue(msgs.contains(payload2));
221+
assertTrue(msgs.contains(null));
204222

205223
// another listener receives messages on both channels
206-
msgs.add(bag2.poll(1, TimeUnit.SECONDS));
207-
msgs.add(bag2.poll(1, TimeUnit.SECONDS));
224+
msgs.clear();
225+
msgs.add(bag2.poll(500, TimeUnit.MILLISECONDS));
226+
msgs.add(bag2.poll(500, TimeUnit.MILLISECONDS));
208227
assertEquals(2, msgs.size());
209228
assertTrue(msgs.contains(payload1));
210229
assertTrue(msgs.contains(payload2));
@@ -225,6 +244,10 @@ public void testContainerChannelResubscribe() throws Exception {
225244
container.addMessageListener(adapter, new ChannelTopic(ANOTHER_CHANNEL));
226245
container.removeMessageListener(null, new ChannelTopic(CHANNEL));
227246

247+
// timing: There's currently no other way to synchronize
248+
// than to hope the subscribe/unsubscribe are executed within the time.
249+
Thread.sleep(400);
250+
228251
// Listener removed from channel
229252
template.convertAndSend(CHANNEL, payload1);
230253
template.convertAndSend(CHANNEL, payload2);
@@ -234,8 +257,8 @@ public void testContainerChannelResubscribe() throws Exception {
234257
template.convertAndSend(ANOTHER_CHANNEL, anotherPayload2);
235258

236259
Set<String> set = new LinkedHashSet<String>();
237-
set.add(bag.poll(1, TimeUnit.SECONDS));
238-
set.add(bag.poll(1, TimeUnit.SECONDS));
260+
set.add(bag.poll(500, TimeUnit.MILLISECONDS));
261+
set.add(bag.poll(500, TimeUnit.MILLISECONDS));
239262

240263
assertFalse(set.contains(payload1));
241264
assertFalse(set.contains(payload2));
@@ -259,15 +282,16 @@ public void testInitializeContainerWithMultipleTopicsIncludingPattern() throws E
259282
Arrays.asList(new Topic[] { new ChannelTopic(CHANNEL), new PatternTopic("s*") }));
260283
container.start();
261284

262-
// Wait for async subscription tasks to setup
285+
// timing: There's currently no other way to synchronize
286+
// than to hope the subscribe/unsubscribe are executed within the time.
263287
Thread.sleep(1000);
264288

265289
template.convertAndSend("somechannel", "HELLO");
266290
template.convertAndSend(CHANNEL, "WORLD");
267291

268292
Set<String> set = new LinkedHashSet<String>();
269-
set.add(bag.poll(1, TimeUnit.SECONDS));
270-
set.add(bag.poll(1, TimeUnit.SECONDS));
293+
set.add(bag.poll(500, TimeUnit.MILLISECONDS));
294+
set.add(bag.poll(500, TimeUnit.MILLISECONDS));
271295

272296
assertEquals(new HashSet<String>(Arrays.asList(new String[] { "HELLO", "WORLD" })), set);
273297
}

‎src/test/java/org/springframework/data/redis/listener/SubscriptionConnectionTests.java‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2011-2014 the original author or authors.
2+
* Copyright 2011-2016 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -47,6 +47,7 @@
4747
* @author Jennifer Hickey
4848
* @author Thomas Darimont
4949
* @author Christoph Strobl
50+
* @author Mark Paluch
5051
*/
5152
@RunWith(Parameterized.class)
5253
public class SubscriptionConnectionTests {
@@ -135,9 +136,11 @@ public void testStopMessageListenerContainers() throws Exception {
135136
container.afterPropertiesSet();
136137
container.start();
137138

138-
// Need to sleep shortly as jedis cannot deal propery with multiple repsonses within one connection
139-
// @see https://github.com/xetorthio/jedis/issues/186
140-
Thread.sleep(1000);
139+
if (connectionFactory instanceof JedisConnectionFactory) {
140+
// Need to sleep shortly as jedis cannot deal propery with multiple repsonses within one connection
141+
// @see https://github.com/xetorthio/jedis/issues/186
142+
Thread.sleep(100);
143+
}
141144

142145
container.stop();
143146
containers.add(container);

0 commit comments

Comments
 (0)