Monday, 4 February 2013

Synchronising Multithreaded Integration Tests

Testing threads is hard, very hard and this makes writing good integration tests for multithreaded systems under test... hard. This is because in JUnit there's no built in synchronisation between the test code, the object under test and any threads. This means that problems usually arise when you have to write a test for a method that creates and runs a thread. One of the most common scenarios in this domain is in making a call to a method under test, which starts a new thread running before returning. At some point in the future when the thread's job is done you need assert that everything went well. Examples of this scenario could include asynchronously reading data from a socket or carrying out a long and complex set of operations on a database.

For example, the ThreadWrapper class below contains a single public method: doWork(). Calling doWork() sets the ball rolling and at some point in the future, at the discretion of the JVM, a thread runs adding data to a database.

public class ThreadWrapper {

 
/**
   * Start the thread running so that it does some work.
   */
 
public void doWork() {

   
Thread thread = new Thread() {

     
/**
       * Run method adding data to a fictitious database
       */
     
@Override
     
public void run() {

       
System.out.println("Start of the thread");
        addDataToDB
();
        System.out.println
("End of the thread method");
     
}

     
private void addDataToDB() {
       
// Dummy Code...
       
try {
         
Thread.sleep(4000);
       
} catch (InterruptedException e) {
         
e.printStackTrace();
       
}
      }

    }
;

    thread.start
();
    System.out.println
("Off and running...");
 
}

}

A straightforward test for this code would be to call the doWork() method and then check the database for the result. The problem is that, owing to the use of a thread, there's no co-ordination between the object under test, the test and the thread. A common way of achieving some co-ordination when writing this kind of test is to put some kind of delay in between the call to the method under test and checking the results in the database as demonstrated below:

public class ThreadWrapperTest {

 
@Test
 
public void testDoWork() throws InterruptedException {

   
ThreadWrapper instance = new ThreadWrapper();

    instance.doWork
();

    Thread.sleep
(10000);

   
boolean result = getResultFromDatabase();
    assertTrue
(result);
 
}

 
/**
   * Dummy database method - just return true
   */
 
private boolean getResultFromDatabase() {
   
return true;
 
}
}

In the code above there is a simple Thread.sleep(10000) between two method calls. This technique has the benefit of being incredabile simple; however it's also very risky. This is because it introduces a race condition between the test and the worker thread as the JVM makes no guarantees about when threads will run. Often it'll work on a developer's machine only to fail consistently on the build machine. Even if it does work on the build machine it atificially lengthens the duration of the test; remember that quick builds are important.

The only sure way of getting this right is to synchronise the two different threads and one technique for doing this is to inject a simple CountDownLatch into the instance under test. In the example below I've modified the ThreadWrapper class's doWork() method adding the CountDownLatch as an argument.

public class ThreadWrapper {

 
/**
   * Start the thread running so that it does some work.
   */
 
public void doWork(final CountDownLatch latch) {

   
Thread thread = new Thread() {

     
/**
       * Run method adding data to a fictitious database
       */
     
@Override
     
public void run() {

       
System.out.println("Start of the thread");
        addDataToDB
();
        System.out.println
("End of the thread method");
        countDown
();
     
}

     
private void addDataToDB() {

       
try {
         
Thread.sleep(4000);
       
} catch (InterruptedException e) {
         
e.printStackTrace();
       
}
      }

     
private void countDown() {
       
if (isNotNull(latch)) {
         
latch.countDown();
       
}
      }

     
private boolean isNotNull(Object obj) {
       
return latch != null;
     
}

    }
;

    thread.start
();
    System.out.println
("Off and running...");
 
}
}

The Javadoc API describes a count down latch as:

A synchronization aid that allows one or more threads to wait until a set of operations being performed in other threads completes.
A CountDownLatch is initialized with a given count. The await methods block until the current count reaches zero due to invocations of the countDown() method, after which all waiting threads are released and any subsequent invocations of await return immediately. This is a one-shot phenomenon -- the count cannot be reset. If you need a version that resets the count, consider using a CyclicBarrier.

A CountDownLatch is a versatile synchronization tool and can be used for a number of purposes. A CountDownLatch initialized with a count of one serves as a simple on/off latch, or gate: all threads invoking await wait at the gate until it is opened by a thread invoking countDown(). A CountDownLatchinitialized to N can be used to make one thread wait until N threads have completed some action, or some action has been completed N times.

A useful property of a CountDownLatch is that it doesn't require that threads calling countDown wait for the count to reach zero before proceeding, it simply prevents any thread from proceeding past an await until all threads could pass.

The idea here is that the test code will never check the database for the results until the run() method of the worker thread has called latch.countdown(). This is because the test code thread is blocking at the call to latch.await(). latch.countdown() decrements latch's count and once this is zero the blocking call the latch.await() returns and the test code continues executing, safe in the knowledge that any results which should be in the database, are in the database. The test can then retrieve these results and make a valid assertion.

Obviously, the code above merely fakes the database connection and operations.

The thing is you may not want to, or need to, inject a CountDownLatch directly into your code; after all it's not used in production and it doesn't look particularly clean or elegant. One quick way around this is to simply make the doWork(CountDownLatch latch) method package private and expose it through a public doWork() method.

public class ThreadWrapper {

 
/**
   * Start the thread running so that it does some work.
   */
 
public void doWork() {
   
doWork(null);
 
}

 
@VisibleForTesting
 
void doWork(final CountDownLatch latch) {

   
Thread thread = new Thread() {

     
/**
       * Run method adding data to a fictitious database
       */
     
@Override
     
public void run() {

       
System.out.println("Start of the thread");
        addDataToDB
();
        System.out.println
("End of the thread method");
        countDown
();
     
}

     
private void addDataToDB() {

       
try {
         
Thread.sleep(4000);
       
} catch (InterruptedException e) {
         
e.printStackTrace();
       
}
      }

     
private void countDown() {
       
if (isNotNull(latch)) {
         
latch.countDown();
       
}
      }

     
private boolean isNotNull(Object obj) {
       
return latch != null;
     
}

    }
;

    thread.start
();
    System.out.println
("Off and running...");
 
}
}

The code above uses Google's Guava @VisibleForTesting annotation to tell us that the doWork(CountDownLatch latch) method visibility has been relaxed slightly for testing purposes.

Now I realise that making a method call package private for testing purposes in highly controversial; some people hate the idea, whilst others include it everywhere. I could write a whole blog on this subject (and may do one day), but for me it should be used judiciously, when there's no other choice, for example when you're writing characterisation tests for legacy code. If possible it should be avoided, but never ruled out. After all tested code is better than untested code.

With this in mind the next iteration of ThreadWrapper designs out the need for a method marked as @VisibleForTesting together with the need to inject a CountDownLatch into your production code. The idea here is to use the Strategy Pattern and separate the Runnable implementation from the Thread. Hence, we have a very simple ThreadWrapper

public class ThreadWrapper {

 
/**
   * Start the thread running so that it does some work.
   */
 
public void doWork(Runnable job) {

   
Thread thread = new Thread(job);
    thread.start
();
    System.out.println
("Off and running...");
 
}
}

and a separate job:

public class DatabaseJob implements Runnable {

 
/**
   * Run method adding data to a fictitious database
   */
 
@Override
 
public void run() {

   
System.out.println("Start of the thread");
    addDataToDB
();
    System.out.println
("End of the thread method");
 
}

 
private void addDataToDB() {

   
try {
     
Thread.sleep(4000);
   
} catch (InterruptedException e) {
     
e.printStackTrace();
   
}
  }
}

You'll notice that the DatabaseJob class doesn't use a CountDownLatch. How is it synchronised? The answer lies in the test code below...

public class ThreadWrapperTest {

 
@Test
 
public void testDoWork() throws InterruptedException {

   
ThreadWrapper instance = new ThreadWrapper();

    CountDownLatch latch =
new CountDownLatch(1);

    DatabaseJobTester tester =
new DatabaseJobTester(latch);
    instance.doWork
(tester);
    latch.await
();

   
boolean result = getResultFromDatabase();
    assertTrue
(result);
 
}

 
/**
   * Dummy database method - just return true
   */
 
private boolean getResultFromDatabase() {
   
return true;
 
}

 
private class DatabaseJobTester extends DatabaseJob {

   
private final CountDownLatch latch;

   
public DatabaseJobTester(CountDownLatch latch) {
     
super();
     
this.latch = latch;
   
}

   
@Override
   
public void run() {
     
super.run();
      latch.countDown
();
   
}
  }
}

The test code above contains an inner class DatabaseJobTester, which extends DatabaseJob. In this class the run() method has been overridden to include a call to latch.countDown() after our fake database has been updated via the call to super.run(). This works because the test passes a DatabaseJobTester instance to the doWork(Runnable job) method adding in the required thread testing capability.

The idea of sub-classing objects under test is something I've mentioned before in one of my blogs on testing techniques and is a really powerful technique.

So, to conclude:

  • Testing threads is hard.
  • Testing anonymous inner classes is almost impossible.
  • Using Thead.sleep(...) is a risky idea and should be avoided.
  • You can refactor out these problems using the Strategy Pattern.
  • Programming is the Art of Making the Right Decision

...and that relaxing a method's visibility for testing may or may not be a good idea, but more on that later...


The code above is available on Github in the captain debug repository (git://github.com/roghughe/captaindebug.git) under the unit-testing-threads project.




5 comments:

Chris said...

Why not simply using an ExecutorService and observing the Future(s)?

Roger Hughes said...

Chris,
a good point. There are several ways to crack this problem; however, one of the main points I wanted to make is that you need to synchronise the Junit test thread with any worker threads and do it without using any sleep calls. I did consider using an Executor, but wanted the code that demonstrates these points to be as simple as possible.

Tomasz Nurkiewicz said...

Thanks for your article, it inspired me to write about few other techniques based on the same example: http://nurkiewicz.blogspot.com/2013/05/synchronising-multithreaded-integration.html

Roger Hughes said...

Tomasz,
A nice article. I also did a follow up to this blog a few weeks later. See: http://www.captaindebug.com/2013/04/five-ways-of-synchronising.html#.UYn5nMu9KK0

Tomasz Nurkiewicz said...

Ups, I somehow missed your follow-up. Too bad, you already addressed some of my concerns. I would've think twice before posting my article.

At least I'll link to your second part. Thanks or pointing this out!