0

The following method calls the method serveThis() of a service synchronously and the method serveThat() in a separate thread i.e. asynchronously: 

public void doSomething() {
    service.serveThis();
    new Thread(() -> service.serveThat()).start();
}

I want to verify in a unit test that service.serveThat() will be executed asynchronously as according to the specification it must not be executed synchronously. So, I want to prevent that later on someone just removes starting a new thread like this:

public void doSomething() {
    service.serveThis();
    // This synchronous execution must cause the test to fail
    service.serveThat();
}
Andras Hatvani
  • 4,346
  • 4
  • 29
  • 45
  • Rather than deal with primitive Threads, why not use the abstraction `Future`? Then you could have your method contract return a `Future` and write tests against that. – David Rawson Nov 01 '17 at 00:18
  • @DavidRawson In this case this really is just a fire'n'forget action and dealing with a Future would contradict this fact. – Andras Hatvani Nov 01 '17 at 14:52

3 Answers3

1

I want to verify that service.serveThat() will be executed asynchronously.

It will be. The syntax says so. Don't test the platform.

user207421
  • 305,947
  • 44
  • 307
  • 483
  • Thanks, I've improved the naming – Andras Hatvani Oct 31 '17 at 10:15
  • 1
    Nothing essential has changed. You are still asking how to check that `service.serveThat()` runs in a separate thread when executed via `new Thread(() -> service.serveThat()).start();`. It does. The Javadoc and the Java Language Specification guarantee it. Don't test the platform. – user207421 Oct 31 '17 at 10:21
  • 1
    @AndrasHatvani This *question* is about unit-testing the platform, which is not a proper use for unit-testing. If there was something wrong with `new Thread(...). start()` it would be a proper occasion for a Java bug report, and prior to that would make it almost impossible for the Java release to be released. You don't test the platform in an application unit test. Otherwise we would have to test `if`, `else`, `while`, ... and we don't. We couldn't. Because we would be relying in the unit test on exactly the behaviour we are trying to test. You have to stop somewhere. – user207421 Oct 31 '17 at 10:29
  • I'll try to further extend the description. – Andras Hatvani Oct 31 '17 at 10:32
  • 2
    There is nothing in any of your numerous edits that addresses the underlying issue. Java already guarantees this behaviour, and it is already subject to hundreds or thousands of its own unit tests prior to being released. You need to worry about whether *your application* satisfies its contracts: not about whether Java does. You are just wasting your time. – user207421 Oct 31 '17 at 10:36
  • 2
    @EJP, OP's `doSomething` function is the method to test, not the test method. It is subject to change, so it seems natural to protect its behaviour with a test to make sure a change won't break its asynchronous behaviour. It's not testing the platform. – Joel Oct 31 '17 at 10:41
  • @Joel Rubbish. There is nothing the OP could do to his `doSomething()` function that could *prevent* it from running asynchronously, given the syntax he has employed to start it. *Ergo* there is no point in testing that aspect of its operation, which is defined 100% by Java and 0% by the application. He is testing the platform,not his own code. It is a waste of his time. And yours. And mine. And there is nothing about it that is subject to change, *except* the syntax he has employed to start it. Your comment does not make sense. – user207421 Oct 31 '17 at 11:44
0

In order to achieve that I use Mockito:

Thread threadServeThatRunsOn;

@Test
public void serveThatWillBeExecutedAsynchronously() throws Exception {
    doAnswer(invocation -> {
        threadServeThatRunsOn = Thread.currentThread();
        return null;
    }).when(mockService).serveThat();

    testObject.doSomething();

    verify(mockService, timeout(200)).serveThat();
    assertNotEquals(threadServeThatRunsOn, Thread.currentThread());
}

Now, if someone would modify doSomething()so that service.serveThat() would run synchronously, then this test would fail.

Andras Hatvani
  • 4,346
  • 4
  • 29
  • 45
  • 1
    Using thread.sleep in unit tests is a very bad practice. Unit tests should be super fast, and with an arbitrary waiting of millisecs the test will be inherently flaky. – pcjuzer Oct 31 '17 at 10:10
  • @pcjuzer Feel free to improve this approach – Andras Hatvani Oct 31 '17 at 10:12
  • Due to the nature of multi-threading in Java the fact that something is executed asynchronuously isn't a guarantee in itself that it will be completed later. There might be exceptional cases when the asynchronuously started thread completes earlier. Multithreading behaviour isn't the thing which can be (unit) tested very well. In your case, I'd probably choose an implementation, where serveThat() can't be called directly, just through a wrapper which always starts a new thread. – pcjuzer Oct 31 '17 at 11:44
  • @pcjuzer The test does guarantee that `serveThat()` will complete later, because it instruments that call with Mockito's `doAnswer()` to enforce that. – Andras Hatvani Oct 31 '17 at 12:40
  • Sleep for 500msec indeed guarantees the order very well, however, in 500msec ~200 normal unit tests could run instead of this one. Writing unit tests which last for half a second isn't scalable. It's also possible to adjust 500msec to a much lower value but in this case you loose the guarantee about timing and it becomes a find-out-the-number game. – pcjuzer Oct 31 '17 at 13:42
  • @pcjuzer @Joel I absolutely agree that `Thread.sleep()` isn't good enough. Based on your input I adapted the test. – Andras Hatvani Oct 31 '17 at 14:39
0

It's possible to get Thread.currentThread() anywhere in the code, so you can write something like this and make asserts based on it (you can achieve this with Mockito in a different way if you don't have YourInterface):

public class ThreadChecker implements YourInterface {

    volatile Thread serveThisThread;
    volatile Thread serveThatThread;

    public void serveThis() {
        serveThisThread = Thread.currentThread();
    }

    public void serveThat() {
        serveThatThread = Thread.currentThread();
    }
}

A unit test can be something like this, but additional asserts might be needed based on the use case:

ThreadChecker mockService = new ThreadChecker(); 

@Test
public void serveThatWillBeExecutedAsynchronously() throws Exception {
    doSomething();
    TestCase.assertFalse(mockService.serveThatThread == mockService.serveThisThread);
}
pcjuzer
  • 2,724
  • 4
  • 25
  • 34
  • My q&a is not about testing the platform, but about preventing that serveThat() will be executed synchronously. Furthermore, your code snippet is not even a test. – Andras Hatvani Oct 31 '17 at 10:43
  • This suggestion adds unnecessary complexity to production code for testing purposes and exposes technical details. Furthermore, it doesn't wait for the initialization of the thread which `serveThat()` will run on. – Andras Hatvani Oct 31 '17 at 12:44
  • I didn't suggest to modify any production code here. You totally misunderstand things. – pcjuzer Oct 31 '17 at 13:44
  • I think this is actually a good answer. There's no change on production code, as it is said the mocking interface can be done with Mockito. Checking Thread objects is better/safer than your current solution based on arbitrary sleep time. – Joel Oct 31 '17 at 13:45
  • 1
    PS: with mockito, you could define first an `AtomicReference`, then: ` Mockito.doAnswer(invocation -> { threadRef.set(Thread.currentThread()); return null; }).when(mockService).serveThat();` and after awaiting execution, check `assertNotEquals(Thread.currentThread(), threadRef.get())` – Joel Oct 31 '17 at 13:51