junit assert in thread throws exception

Question

What am I doing wrong that an exception is thrown instead of showing a failure, or should I not have assertions inside threads?

 @Test
 public void testComplex() throws InterruptedException {
  int loops = 10;
  for (int i = 0; i < loops; i++) {
   final int j = i;
   new Thread() {
    @Override
    public void run() {
     ApiProxy.setEnvironmentForCurrentThread(env);//ignore this
     new CounterFactory().getCounter("test").increment();//ignore this too
     int count2 = new CounterFactory().getCounter("test").getCount();//ignore
     assertEquals(j, count2);//here be exceptions thrown. this is line 75
    }
   }.start();
  }
  Thread.sleep(5 * 1000);
  assertEquals(loops, new CounterFactory().getCounter("test").getCount());
}

StackTrace

Exception in thread "Thread-26" junit.framework.AssertionFailedError: expected:<5> but was:<6>
    at junit.framework.Assert.fail(Assert.java:47)
    at junit.framework.Assert.failNotEquals(Assert.java:277)
    at junit.framework.Assert.assertEquals(Assert.java:64)
    at junit.framework.Assert.assertEquals(Assert.java:195)
    at junit.framework.Assert.assertEquals(Assert.java:201)
    at com.bitdual.server.dao.ShardedCounterTest$3.run(ShardedCounterTest.java:77)

Solution 1

The JUnit framework captures only assertion errors in the main thread running the test. It is not aware of exceptions from within new spawn threads. In order to do it right, you should communicate the thread's termination state to the main thread. You should synchronize the threads correctly, and use some kind of shared variable to indicate the nested thread's outcome.

EDIT:

Here is a generic solution that can help:

class AsynchTester{
    private Thread thread;
    private AssertionError exc; 

    public AsynchTester(final Runnable runnable){
        thread = new Thread(new Runnable(){
            public void run(){
                try{            
                    runnable.run();
                }catch(AssertionError e){
                    exc = e;
                }
            }
        });
    }

    public void start(){
        thread.start();
    }

    public void test() throws InterruptedException{
        thread.join();
        if (exc != null)
            throw exc;
    }
}

You should pass it the runnable in the constructor, and then you simply call start() to activate, and test() to validate. The test method will wait if necessary, and will throw the assertion error in the main thread's context.

Solution 2

A small improvement to Eyal Schneider's answer:
The ExecutorService allows to submit a Callable and any thrown Exceptions or Errors are rethrown by the returned Future.
Consequently, the test can be written as:

@Test
public void test() throws Exception {
  ExecutorService es = Executors.newSingleThreadExecutor();
  Future<?> future = es.submit(() -> {
    testSomethingThatMightThrowAssertionErrors();
    return null;
  });

  future.get(); // This will rethrow Exceptions and Errors as ExecutionException
}

Solution 3

Where multiple worker threads are concerned, such as in the original question, simply joining one of them is not sufficient. Ideally, you'll want to wait for all worker threads to complete while still reporting assertion failures back to the main thread, such as in Eyal's answer.

Here's a simple example of how to do this using ConcurrentUnit:

public class MyTest extends ConcurrentTestCase {
    @Test
    public void testComplex() throws Throwable {
        int loops = 10;
        for (int i = 0; i < loops; i++) {
            new Thread(new Runnable() {
                public void run() {
                    threadAssertEquals(1, 1);
                    resume();
                }
            }).start();
        }

        threadWait(100, loops); // Wait for 10 resume calls
    }
}

Comments

[Matenia Rossides]

Why are you creating a new Thread in this test? I mean, why the h@$! would you want to create Threads in a unit test?

[antony.trupe]

@Cem I have a set of (initial) tests I'm developing off of and one of them (attempts) to detect a race condition(the 3 lines I say to ignore become relevant for this discussion). Is there a better way to do race condition testing? Do I need to move to another tool for this kind of test?

[Matenia Rossides]

You can't really test for race conditions with unit tests, especially by creating threads to simulate situations. Even on this example you gave, you're checking that the counter better be 2 when the 2nd thread is running. Even though you create the threads in order they're not necessarily going to run at the same order. Also, the threads can get preempted between the time you call increment and get so there is already a race condition in your test. Every once in a blue moon it will pass or fail. Unit tests should be more deterministic that this.

[antony.trupe]

Granted the specific assertions were a little naive, but the unit tests are incredibly effective in catching a significant portion of race/contention issues for my specific situation.

[Stephen C]

"You should synchronize the threads correctly ..." in this example, the simple way is for the main thread to call join() on the child thread ... and get rid of the sleep(5000) call.

[antony.trupe]

The sleep call smelled a little, but I didn't dwell on it since it was unit test code, but I will most certainly use the correct way now that I know.

[eregon]

FWIW exc doesn't need to be volatile, since Thread.join() synchronizes the state of the joined thread with the joining thread.

[Eyal Schneider]

@eregon: You are right. The happens-before relationship is well established with this data member. Fixed.