How to avoid Thread.sleep in Unit tests?

问题:

Let's imagine I have the following method which should be tested:

@Autowired
private RoutingService routingservice;

public void methodToBeTested() {
    Object objectToRoute = initializeObjectToRoute();
    if (someConditions) {
         routingService.routeInOneWay(objectToRoute);
    } else {
         routingService.routeInAnotherWay(objectToRoute);
    }
}

In this case RoutingService is running in the separate thread, thus in it's constructor we have the following:

Thread thread = new Thread(this);
thread.setDaemon(true);
thread.start();

The problem is that RoutingService changes the state of objectToRoute and this is exactly what I want to check, but this doesn't happen straight away thus the test fails. However, if I add Thread.sleep() then it works, but this is bad practice as I know.

How can I avoid Thread.sleep() in this case?

回答1:

If you are testing methodToBeTested, you should simply mock routingservice. You shouldn't be testing any methods that methodToBeTested calls. However, it sounds like you want to test the RoutingService (you said "The problem is that RoutingService changes the state of objectToRoute and this is exactly what I want to check"). To test RoutingService methods, you should write separate unit tests for those methods.

回答2:

You could mock objectToRoute to set the value of a CompletableFuture and then call get on that in your assertion. This will wait until the value is set before continuing. Then set a timeout @Test(timeout=5000) in case the value is never set.

This has the advantage that the test won't wait longer than necessary and it's harder to fail because of too short a time because you can make the timeout much larger than normal.

回答3:

I suggest awaitility for synchronizing asynchronous tests. For example, assume you have an result object that get set after some thread operations and you want to test it. You write statement like this:

 await()
.atMost(100, TimeUnit.SECONDS)
.untilAsserted(() -> assertNotNull(resultObject.getResult()));

It waits maximum 100 seconds, or until the assertion is satisfied. For instance, if getResult() returns something that is not null in between 0-100 seconds, the execution continues unlike Thread.sleep which holds the execution for given time no matter result is there or not.

回答4:

It depends. As Ben Green said in his comment, sleep is dangerous in tests because it can hide a race condition. You know if the overall design contains such a race condition, where the service could be used before the route is ready. It it does, you should fix it in code, for example by testing a ready condition, and test it the same in your test class.

If you know that it cannot happen, you should document it in you main code and in your test class. That would then be a perfect justification for a sleep.

(I assume that this is for an integration test - for unit tests mocks should be enough as you were said in other answers)

回答5:

Instead of avoiding Thread.sleep(), You can pass value as zero in Junit Test Case.

回答6:

Use Object.wait()

We have worked with some asynchronous processes that obtain files from directories or .ZIP archives. We use the content of the files somewhere else while the asynchronous one keeps on reading.

The way we came around to test them was to wait() on the communication object --in our case it was a Queue--, so the asynchronous process would queue.notifyAll() whenever there was a new file ready to be used and kept working. On the other end the consumer would process one item at a time until the queue was empty and then queue.wait() to wait for more. We do use a special object passed through the queue to indicate that there are no more items to process.

In your case I suppose the object you want to check, objectToRoute, is not really created inside the method you want to test. You could wait() on it inside the test, and notifyAll() on it inside the method you want to test, methodToBeTested. I know this would introduce the extra line of code in your production code base, but with nobody waiting on it, it should result harmless. It would end up something like:

public void methodToBeTested(Object objectToRoute) {
    if (someConditions) {
         routingService.routeInOneWay(objectToRoute);
    } else {
         routingService.routeInAnotherWay(objectToRoute);
    }
    synchronized(objectToRoute) {
        objectToRoute.notifyAll();
    }
}

And in your test class there will be something like:

@Test
public void testMethodToBeTested() throws InterruptedException {
    Object objectToRoute = initializeObjectToRoute();
    methodToBeTested(objectToRoute);
    synchronized (objectToRoute) {
        objectToRoute.wait();
    }
    verifyConditionsAfterRouting(objectToRoute);
}

I do know that in this simple example does not make too much sense, since the sample system is not multithreaded. I assume the multithread twist is added in the routeInOneWay and routeInAnotherWay methods; thus, those are the ones to call notifyAll() method.

As sample code to point the direction of our solution, here are some code fragments.

In the asynchronous worker side, or producer:

while(files.hasNext(){
   queue.add(files.next());
   synchronized (outputQueue) {
       queue.notifyAll()
   }
}

And in the consumer side:

while(!finished){
    while(!queue.isEmpty()){
        nextFile = queue.poll();
        if (nextFile.equals(NO_MORE_FILES_SIGNAL)) {
            finished = true;
            break;
        }
        doYourThingWith(nextFile);
    }
    if (!finished) {
        synchronized (outputQueue) {
            outputQueue.wait();
        }
    }
}