Fixed concurrency bugs in tests: Using await(timeout,unit) without ch…

…ecking the return value is discouraged, because then the test may give false positives or false negatives. It's better to use await() and let the test freeze, because then you can take a thread dump and debug the reason of the freeze (maybe some other thread is in wrong state).
luontola · Nov 27, 2008 · 589ef34 · 589ef34
1 parent a01e061
commit 589ef34
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 23 deletions.
diff --git a/dimdwarf-core/src/test/java/net/orfjackal/dimdwarf/scheduler/TaskThreadPoolSpec.java b/dimdwarf-core/src/test/java/net/orfjackal/dimdwarf/scheduler/TaskThreadPoolSpec.java
@@ -51,8 +51,6 @@
 @Group({"fast"})
 public class TaskThreadPoolSpec extends Specification<Object> {
 
-    private static final int TEST_TIMEOUT = 50;
-
     private Context taskContext;
     private BlockingQueue<TaskBootstrap> taskQueue;
     private Logger logger;
@@ -130,7 +128,7 @@ public Runnable getTaskInsideTransaction() {
                     return task;
                 }
             });
-            end.await(TEST_TIMEOUT, TimeUnit.MILLISECONDS);
+            end.await();
         }
 
         public void theyAreExecuted() throws InterruptedException {
@@ -185,18 +183,21 @@ public void run() {
             runningTasks0 = pool.getRunningTasks();
             taskQueue.add(new SimpleTaskBootstrap(task1));
 
-            step1.await(TEST_TIMEOUT, TimeUnit.MILLISECONDS);
+            step1.await();
             taskQueue.add(new SimpleTaskBootstrap(task2));
 
-            step3.await(TEST_TIMEOUT, TimeUnit.MILLISECONDS);
+            step3.await();
             pool.awaitForCurrentTasksToFinish();
             runningTasksEnd = pool.getRunningTasks();
         }
 
         public void theyAreExecutedInParallel() throws InterruptedException {
-            specify(step1.await(TEST_TIMEOUT, TimeUnit.MILLISECONDS));
-            specify(step2.await(TEST_TIMEOUT, TimeUnit.MILLISECONDS));
-            specify(step3.await(TEST_TIMEOUT, TimeUnit.MILLISECONDS));
+            step1.await();
+            step2.await();
+            step3.await();
+            specify(step1.getCount(), should.equal(0));
+            specify(step2.getCount(), should.equal(0));
+            specify(step3.getCount(), should.equal(0));
         }
 
         public void thePoolKnowsTheNumberOfRunningTasks() {
@@ -242,7 +243,7 @@ public void run() {
                 }
             };
             taskQueue.add(new SimpleTaskBootstrap(task1));
-            firstTaskIsExecuting.await(TEST_TIMEOUT, TimeUnit.MILLISECONDS);
+            firstTaskIsExecuting.await();
 
             executeAfterCurrentThreadIsNotRunning(new Runnable() {
                 public void run() {
@@ -285,7 +286,7 @@ public void run() {
                 }
             };
             taskQueue.add(new SimpleTaskBootstrap(task));
-            end.await(TEST_TIMEOUT, TimeUnit.MILLISECONDS);
+            end.await();
             pool.awaitForCurrentTasksToFinish();
         }
 

diff --git a/dimdwarf-core/src/test/java/net/orfjackal/dimdwarf/scopes/TaskScopeSpec.java b/dimdwarf-core/src/test/java/net/orfjackal/dimdwarf/scopes/TaskScopeSpec.java
@@ -31,23 +31,15 @@
 
 package net.orfjackal.dimdwarf.scopes;
 
-import com.google.inject.AbstractModule;
-import com.google.inject.Guice;
-import com.google.inject.Injector;
-import com.google.inject.Provider;
-import jdave.Block;
-import jdave.Group;
-import jdave.Specification;
+import com.google.inject.*;
+import jdave.*;
 import jdave.junit4.JDaveRunner;
 import net.orfjackal.dimdwarf.context.Context;
-import net.orfjackal.dimdwarf.context.FakeContext;
-import net.orfjackal.dimdwarf.context.ThreadContext;
-import net.orfjackal.dimdwarf.modules.FakeEntityModule;
-import net.orfjackal.dimdwarf.modules.TaskContextModule;
+import net.orfjackal.dimdwarf.context.*;
+import net.orfjackal.dimdwarf.modules.*;
 import org.junit.runner.RunWith;
 
 import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
 
 /**
@@ -162,7 +154,7 @@ public void run() {
             t1.start();
             t2.setDaemon(true);
             t2.start();
-            gotInstance.await(100, TimeUnit.MILLISECONDS);
+            gotInstance.await();
 
             specify(s1.get(), should.not().equal(null));
             specify(s2.get(), should.not().equal(null));