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)

Winn

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? – Atworth

@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? – Winn

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. – Atworth

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. – Winn

Answer 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(() ->
            {
                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.

Brechtel

"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. – Liveried

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. – Winn

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

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

Answer 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
}

Communize

Answer 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 my 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
    }
}

Incertitude

Answer 4

JUnit throws AssertionError that extends of Throwable, it has the same parent of Exception. You can catch the fail assert of the thread, then save in a static field and finally check in the main thread if the other thread has failed some assert.

First, create the static field

private volatile static Throwable excepcionTE = null;

Second, wrap the asserts in a try/catch and catch AssertionError

        try
    {
      assertTrue("", mensaje.contains("1234"));
    }
    catch (AssertionError e)
    {
      excepcionTE = e;
      throw e;
    }

And finally, check in the main thread that field

 if (excepcionTE != null)
{
  excepcionTE.printStackTrace();
  fail("Se ha producido una excepcion en el servidor TE: "
      + excepcionTE.getMessage());
}

Ozenfant

Answer 5

I ended up using this pattern it work with both Runnables and Threads. It is largely inspired from the answer of @Eyal Schneider:

private final class ThreadUnderTestWrapper extends ThreadUnderTest {
    private Exception ex;

    @Override
    public void run() {
        try {
            super.run();
        } catch ( Exception ex ) {
            this.ex = ex;
        }
    }

    public Exception getException() throws InterruptedException {
        super.join(); // use runner.join here if you use a runnable. 
        return ex;
    }
}

Codel

Answer 6

I was looking for a simple and readable solution. Inspired by Eyal Schneider and Riki Gomez's answer, I've come up with this:

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;


public class ThreadExceptionTest {
    private static Throwable failedThreadException;

    @BeforeEach
    public void setup() {
        failedThreadException = null;
    }

    @Test
    public void threadTest() {
        final Thread thread = new Thread(() -> codeThatMayFail(...));

        thread.start();

        // We have to join before we check for exceptions, 
        //   otherwise we might check before the Thread even finished.
        thread.join();

        if (failedThreadException != null) {
            fail("The thread failed with an exception", failedThreadException);
        }
    }

    private void codeThatMayFail(...) {
        try {
            // Code that may throw the exception
            // ...
        } catch (Exception e) {
            failedThreadException = e;
        }
    }
}

So, you can achieve the desired result with the help of a static variable. The thread runs as usual, and all you have to do is store the exception you are interested in on the static variable. Just don't forget to reset its value to null before every test, or you may run into trouble on subsequent tests on the same Class.

Final note: If you are planning on running multiple Threads on the same test, and it is expected for them to run the same blocks of code simultaneously, I would advice to make the static variable volatile so that updates to the variable propagate predictably to other threads:

private volatile static Throwable failedThreadException;

Moyra