-2

I want to create a unit test for the following class:

@Service
public class XService{
    
    public String getSomething(String inputField) {
        final SomeEntity someEntity1 = new SomeEntity();
        final AtomicReference<Throwable> throwable = new AtomicReference<>();

        BiConsumer<Response, Throwable> consumer = (response, error) -> {
        if (error != null) {
            throwable.set(error);
        } else {
            SomeEntity someEntity2 = response.readEntity(SomeEntity.class);
            someEntity1.setSomeField(someEntity2.getSomeField());
            //does some stuff with the response
        }
        };
        
        WebTarget target = client.target("api_url"+inputField);
        target.queryParam("param", param)
            .request(MediaType.APPLICATION_JSON)
            .acceptLanguage(Locale.ENGLISH)
            .header("Authorization", token)
            .rx()
            .get()
            .whenCompleteAsync(consumer);

        return someEntity1.getSomeField();
    }
}

I have mocked everything until .whenCompleteAsync(consumer) using something like this:

when(mockWebTarget.queryParam(any(),any())).thenReturn(mockWebTarget);
CompletionStageRxInvoker completionStageRxInvoker = mock(CompletionStageRxInvoker.class);
when(mockBuilder.rx()).thenReturn(completionStageRxInvoker);
CompletionStage<Response> mockResp = mock(CompletionStage.class);
when(completionStageRxInvoker.get()).thenReturn(mockResp);

I cannot currently change the design of the class, only make tests for it.

How can I mock the consumer object to make the code run inside the lambda? Is this even possible?

  • In my opinion, You need to change `design` of this class. If You have to create plenty of mocks, there is a sign that design needs to be improved. Try to add parameters to function signature and treat them as input, return should be output. This is the easiest way to test something. if You can't pass the parameters then add some mock. – Łukasz Olszewski Aug 19 '21 at 14:06
  • @ŁukaszOlszewski I agree, but I cannot currently change the design of the class only make tests for it. The method already has input and output as you said. I modified the question to reflect that. My only issue here is getting the lambda to execute when running the test. – blackorange Aug 19 '21 at 15:08
  • The code in `XService` doesn't actually compile, since the lambda is trying to modify the local variable `someEntity` (and is missing a semicolon). The return statement at the end refers to an undefined variable `entity`. Presumably, this should be `someEntity`, but this entire approach won't work, because the consumer is called asynchronously. It would be better to make the entire method asynchronous (have it return `CompletionStage`). I would avoid trying to mock `CompletionStage` and instead return a pre-completed one using `CompletableFuture.completedFuture`. – Tim Moore Aug 20 '21 at 00:31
  • @TimMoore The code does work, I just made some mistakes when typing it here. Thanks for pointing that out, I updated the question code. On what method should I return `CompletableFuture` ? There are no methods in that chain that support this. – blackorange Aug 20 '21 at 11:05
  • @blackorange thanks for the update. It can compile now, but it has a race condition, so the code in `XService` will have to change to work correctly. I'll elaborate in a full answer. – Tim Moore Aug 20 '21 at 12:13

1 Answers1

0

The getSomething method has a race condition. It isn't possible to reliably test it, because it has non-deterministic behavior.

The problem is that consumer is invoked asynchronously, after the request completes. Nothing in getSomething ensures that will happen before return someEntity1.getSomeField() occurs. This means that it might return the field that is copied from the read entity, or it might return the default value of that field. Most likely, it will return before consumer is invoked (since the request is relatively slow). Once the request completes, it will set the field in someEntity1, but by this point, getSomething has already returned the incorrect value to the caller, and the object referenced by someEntity1 won't be read again.

The correct way to handle this is to make getSomething also return a CompletionStage:

public CompletionStage<String> getSomething(String inputField) {
    WebTarget target = client.target("api_url"+inputField);
    return target.queryParam("param", param)
        .request(MediaType.APPLICATION_JSON)
        .acceptLanguage(Locale.ENGLISH)
        .header("Authorization", token)
        .rx()
        .get()
        .thenApply(response -> response.readEntity(SomeEntity.class).getSomeField());
}

Then, to unit test this, you can create mocks for WebTarget, Invocation.Builder, CompletionStageRxInvoker, and Response as you have. Rather than mocking CompletionStage, it will be simpler to have the mocked completionStageRxInvoker.get() method return CompletableFuture.completedFuture(mockResponse). Note that CompletableFuture is a concrete implementation of CompletionStage that is part of JavaSE.

Even better, to reduce the proliferation of mocks, you can refactor this to separate out the request logic from the response-handling logic. Something like this:

public CompletionStage<String> getSomething(String inputField) {
    return apiClient
            .get(inputField, param, token)
            .thenApply(SomeEntity::getSomeField);
}

Where apiClient is an injected instance of a custom class or interface that you can mock, with a method declared like this:

public CompletionStage<SomeEntity> get(String inputField, Object param, String token) {
    WebTarget target = client.target("api_url"+inputField);
    return target.queryParam("param", param)
            .request(MediaType.APPLICATION_JSON)
            .acceptLanguage(Locale.ENGLISH)
            .header("Authorization", token)
            .rx()
            .get()
            .thenApply(response -> response.readEntity(SomeEntity.class));
}
Tim Moore
  • 8,958
  • 2
  • 23
  • 34
  • Thanks a lot for the elaborate answer, that really clears things up! The race condition that you mentioned is handled by an await, that I omitted. That class is bound to be refactored in the future. – blackorange Aug 20 '21 at 14:07
  • Glad it helps! Awaiting the response makes a big difference. That does solve the race condition, but it also negates the benefit of using the reactive API in the first place, since it blocks the calling thread. It might as well invoke the request synchronously on that thread in the first place, and simplify the execution flow. I'd ask in the future that you try to use the actual code that your question is about. In any case, using the `completedFuture` will ensure the consumer is invoked. – Tim Moore Aug 20 '21 at 14:51
  • I'll be more mindful in the future. I left that part out because I only cared about the lambda handling. Thanks again. – blackorange Aug 20 '21 at 16:17