Seperating Concurrency for your Class
Testing stuff concurrently is hard (tm)! GOOS amongst other people recommending separating the concurrency part from the parts that are doing some work. So, for example, if you had some Scheduler
which is supposed to schedule some task on one or more threads. You can pass in the part that is responsible for the threading to your scheduler and just test the scheduler collaborates with this object correctly. This is more in a classic unit testing style.
An example with a `Scheduler is here, this uses a mocking framework to help. If you're not familiar with those ideas, don't worry, they're probably not relevant for your test.
Having said that, you might actually want to run your class 'in context' in a multi-threaded way. This seems to be the kind of test you're writing above. The trick here is to keep the test deterministic. Well, I say that, theres a couple of choices.
Deterministic
If you can setup your test to progress in a deterministic way, waiting at key points for conditions to be met before moving forward, you can try to simulate a specific condition to test. This means understanding exactly what you want to test (for example, forcing the code into a deadlock) and stepping through deterministically (for example, using abstractions like CountdownLatches
etc to 'synchronise' the moving parts).
When you attempt to make some multi-threaded test syncrhonise its moving parts, you can use whatever concurrency abstraction is available to you but it's difficult because its concurrent; things could happen in an unexpected order. You're trying to mitegate this in your test by using the sleep
calls. We generally don't like to sleep in a test because it'll make the test run slower and when you've got thousands of tests to run, every ms counts. If you lower the sleep period too much the test become non-deterministic and ordering isn't guaranteed.
Some examples include
You've spotted one of the gotchas where the main test thread will finish before the newly spawned threads under test complete (using the join
). Another way is to wait for a condition, for example using WaitFor.
Soak / Load Testing
Another choice is to setup a test to setup, run and spam your classes in an attempt to overload them and force them to betray some subtle concurrency issue. Here, just as in the other style, you'll need to setup up specific assertion so that you can tell if and when the classes did betray themselves.
For you're test then, I'd suggest coming up with an assertion so that you can see both positive and negative runs against your class and replacing the sleep
(and system.out
calls. If you can, running your test from something like JUnit is more idiosyncratic.
For example, a basic test in the style you've started down might look like this
public class TestDriver {
private static final CyclicBarrier barrier = new CyclicBarrier(3);
private static final AtomicInteger counter = new AtomicInteger(0);
static class Runnable1 implements Runnable {
public void run() {
try {
barrier.await();
counter.getAndIncrement();
} catch (Exception ie) {
throw new RuntimeException();
}
}
}
@Test (timeout = 200)
public void shouldContinueAfterBarrier() throws InterruptedException {
Thread t1 = new Thread(new Runnable1());
Thread t2 = new Thread(new Runnable1());
Thread t3 = new Thread(new Runnable1());
t1.start();
t2.start();
t3.start();
t1.join();
t2.join();
t3.join();
assertThat(counter.get(), is(3));
}
}
If possible, adding a timeout to your Barrier is good practice and would help write a negative test like this
public class TestDriver {
private static final CyclicBarrier barrier = new CyclicBarrier(3);
private static final AtomicInteger counter = new AtomicInteger(0);
static class Runnable1 implements Runnable {
public void run() {
try {
barrier.await(10, MILLISECONDS);
counter.getAndIncrement();
} catch (Exception ie) {
throw new RuntimeException();
}
}
}
@Test (timeout = 200)
public void shouldTimeoutIfLastBarrierNotReached() throws InterruptedException {
Thread t1 = new Thread(new Runnable1());
Thread t2 = new Thread(new Runnable1());
t1.start();
t2.start();
t1.join();
t2.join();
assertThat(counter.get(), is(not((3))));
}
}
If you wanted to post your implementation, we might be able to suggest more alternatives. Hope that gives you some ideas though...
EDIT: Another choice is to reach into your barrier object for finer grained assertions, for example,
@Test (timeout = 200)
public void shouldContinueAfterBarrier() throws InterruptedException, TimeoutException {
Thread t1 = new Thread(new BarrierThread(barrier));
Thread t2 = new Thread(new BarrierThread(barrier));
Thread t3 = new Thread(new BarrierThread(barrier));
assertThat(barrier.getNumberWaiting(), is(0));
t1.start();
t2.start();
waitForBarrier(2);
t3.start();
waitForBarrier(0);
}
private static void waitForBarrier(final int barrierCount) throws InterruptedException, TimeoutException {
waitOrTimeout(new Condition() {
@Override
public boolean isSatisfied() {
return barrier.getNumberWaiting() == barrierCount;
}
}, timeout(millis(500)));
}
EDIT: I wrote some of this up at http://tempusfugitlibrary.org/recipes/2012/05/20/testing-concurrent-code/