java - Designing a Test class for a custom Barrier

Question

I had to do an implementation of a custom barrier class using locks as part of my course work. To test my LockBarrier class, I came up with a the following test code. It is working correctly, but I am concerned whether that is the proper way to do. Could you please suggest improvements I can do, especially structuring the classes. I think my way of coding is not the correct one. Any suggestions are welcome.

public class TestDriver 
{
        private static LockBarrier barrier;

        static class Runnable1 implements Runnable
        {
            public Runnable1()
            { }

            public void run()
            {
                try
                {
                    System.out.println(Thread.currentThread().getId()+" lazy arrived at barrier");
                    Thread.sleep(10000);
                    barrier.await();
                    System.out.println(Thread.currentThread().getId()+" passed barrier");           

                }
                catch (InterruptedException ie)
                {
                    System.out.println(ie);
                }
            }     

        }

        static class Runnable2 implements Runnable
        {       

            public Runnable2()
            { } 

            public void run()
            {
                try
                {
                    System.out.println(Thread.currentThread().getId()+" quick arrived at barrier");

                    //barrier.await(1,TimeUnit.SECONDS);
                    barrier.await();
                    System.out.println(Thread.currentThread().getId()+" passed barrier");
                }               
                catch (InterruptedException ie)
                {
                    System.out.println(ie);
                }
            }
        }

        static class Runnable3 implements Runnable
        {
            public Runnable3()
            { }

            public void run()
            {
                try
                {
                    System.out.println(Thread.currentThread().getId()+" very lazy arrived at barrier");
                    Thread.sleep(20000);
                    barrier.await();
                    System.out.println(Thread.currentThread().getId()+" passed barrier");
                }               
                catch (InterruptedException ie)
                { 
                    System.out.println(ie);
                }
            }
        }


        public static void main(String[] args) throws InterruptedException
        {
            barrier = new LockBarrier(3);           
            Thread t1 = new Thread(new TestDriver.Runnable1());
            Thread t2 = new Thread(new TestDriver.Runnable2());
            Thread t3 = new Thread(new TestDriver.Runnable3());         
            t1.start();
            t2.start();
            t3.start();

            t1.join();
            t2.join();
            t3.join();
        }   
} 

Answer 1

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

• Forcing a deadlock using CountdownLatch
• Setting up a thread to be interuptted

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/