2

I am using Mockito to mock some objects and test my WebSocket message sender service. The send method takes a org.springframework.web.socket.WebSocketSession and a message, and returns a CompletableFuture.

In the lambda passed to the thenAccept() method of the CompletableFuture, I verify that the session.sendMessage() method has been called with the expected value :

WebSocketSession session = mockWebSocketSession();
TextMessage expectedMessage = new TextMessage("test text message");

sender.sendStringMessage(session, "test text message").thenAccept(nil -> {
    try{ // this is what I am talking about
        verify(session).sendMessage(expectedMessage);
    }catch(IOException e){}
});

Since the sendMessage() method throws an IOException I am forced to add a useless try/catch block around the call on the inside the lambda. It is needlessly verbose.

How could I avoid it ?

Pierre Henry
  • 16,658
  • 22
  • 85
  • 105
  • I am not sure if this is the good practice to get rid of the ugly exception part, I'll wrap the code part in charge of the exception in a method and then use it in my lambda expression. See my answer for details. – MadJlzz Sep 16 '16 at 14:50
  • Yes, that would be a possibility. – Pierre Henry Sep 16 '16 at 14:53
  • *I am forced to add a useless try/catch block around the call on the inside the lambda.* I would argue that it is not useless if it is required for the code to function correctly. It is more *you just don't like it*. And to this *It is needlessly verbose.*, I say **Because Java**. –  Sep 16 '16 at 14:58
  • @Jarrod Well, I would say that it is actually useless because of the way mockito works. The `sendMessage` method on the mock created by the `verify` method will actually never throw am `IOException`, so the try catch is useless because it will never catch anything. Of course it is needed for the code to compile. That doesn't mean it is useful. – Pierre Henry Sep 20 '16 at 08:11

4 Answers4

3

You can try using Durian library 

foodOnPlate.forEach(Errors.suppress().wrap(this::eat));
list.forEach(Errors.rethrow().wrap(c -> somethingThatThrows(c)));

or extend Cosumer yorself

@FunctionalInterface
public interface ThrowingConsumer<T> extends Consumer<T> {

    @Override
    default void accept(final T elem) {
        try {
            acceptThrows(elem);
        } catch (final Exception e) {
            /* Do whatever here ... */
            System.out.println("handling an exception...");
            throw new RuntimeException(e);
        }
    }

    void acceptThrows(T elem) throws Exception;
}
//and then pass
thenAccept((ThrowingConsumer<String>) aps -> {
  // maybe some other code here...
throw new Exception("asda");
})
Dennis R
  • 1,769
  • 12
  • 13
0

I would rework you test in this way

final String testMessage = "test text message";
WebSocketSession session = mockWebSocketSession();

sender.sendStringMessage(session, testMessage).get(); // will wait here till operation completion

verify(session).sendMessage(new TextMessage(testMessage));

and add IOException to the test method signature.

This solution solves 2 issues:

  1. you test code is cleaner and all you assertions and verifications are at the end of your test method in one place;
  2. the solution solves race condition when you test may finish silently and green but your vitrification in CompletableFuture lambda even been executed
Andriy Kryvtsun
  • 3,220
  • 3
  • 27
  • 41
  • But the `sender.sendString` method does things asynchronously using a thread pool, so it is good to do the check in the `then...` lambda. Otherwise I would have to add some `Thread.sleep(...)` in order to be sure the operation has finishd, and that is imo worse than the ugly try/catch. – Pierre Henry Sep 16 '16 at 14:58
  • @PierreHenry I don't know details of `sender.sendString` impl but in this case just add needed results for checking into some `BlockingQueue` and takes expected result outside lambda with needed waiting on the test method level again. – Andriy Kryvtsun Sep 16 '16 at 15:10
  • Excellent idea. The blocking queue also solves the problem that the test process might finish before the promise is complete. 2 birds with one stone. Please edit your answer with some code and I will accept it. Btw, not my downvote. – Pierre Henry Sep 16 '16 at 15:49
  • @PierreHenry Done. Pls, note on `Feature#get()` call. In your case with `CompletebleFuture` any `BlockingQueue` doesn't needed. – Andriy Kryvtsun Sep 16 '16 at 16:11
  • @Anriy : Yes, I figured that out too. Currently refactoring my tests. Thanks. – Pierre Henry Sep 16 '16 at 16:19
  • Note : In case I need to verify some exception raised in `.exceptional()`, I created a simple holder class and I set them inside the lambda. I can then verify the excpetion after the call to `get()` returned. – Pierre Henry Sep 16 '16 at 16:37
  • 1
    @PierreHenry the specific impl depends on your code under the test. But the idea is the same: concentrate assertions at the test end to make sure there are executed and there are true; put your data to check in some holder assertions can access; wait in assertions the data appearance. Also it's good to add @Test(timeout = 1_000) or so to make sure your test will not hang on if smth is wrong in your threads and data never will be put in holder. – Andriy Kryvtsun Sep 16 '16 at 16:55
0

Follow on from my comment, I'll do something like this :

public void myMethod() {
  try{ // this is what I am talking about
    verify(session).sendMessage(expectedMessage);
  }catch(IOException e) {}
}

And then :

sender.sendStringMessage(session, "test text message").thenAccept(nil -> myMethod());
MadJlzz
  • 767
  • 2
  • 13
  • 35
  • Not perfect but the best answer fo far. Since there are several test cases with the same logic, I could do something like this. – Pierre Henry Sep 16 '16 at 15:00
  • 1
    It's not good idea to put assertions outside test method at all (you don't see your expectations in one place) and It's possible you'll have race condition when your test method is silently finished but you lambda verification even been called. As a result, you test is green but no actually checks were executed. – Andriy Kryvtsun Sep 16 '16 at 15:13
  • Of course you have to check that in your tests... I just answered the question – MadJlzz Sep 16 '16 at 16:38
-2

duplicate of this ? Java 8: Mandatory checked exceptions handling in lambda expressions. Why mandatory, not optional?

you are in test class, so just add throws IOException to your method. that way if this method raised IOException in which case it means your test will fail.

or you can also say that your method is expected to throw IOException,

something like :

@Test(expected = IOException.class)
public void yourTestCaser(){
   //...

with that , it could look like this : 

sender.sendStringMessage(session, "test text message").thenAccept(nil -> 
{ verify(session).sendMessage(expectedMessage); });
Community
  • 1
  • 1
user828107
  • 1
  • 1