Default unit test for doesn't test anything

[jdneo]

by @hf-kklein in microsoft/vscode-azurefunctions#491 (comment):

Steps to reproduce:

• create a new Azure Function Project using the Azure Function VS Code PlugIn . Choose language Java and default values.
• The path src\test\java\com\function\FunctionTest.java contains an autogenerated unit test for the Function class in src\main\java\com\function\Function.java.
• One would expect the test to fail, if the HTTP status code of the repsonse generated by the Azure Function inside the Function class returns something is different from 200 -- OK.
• Run original test => passed;
• Modify the Azure Function HttpTriggerJava in Function´.java:
  replace this part (lines 26-30)

if (name == null) {
     return request.createResponse(400, "Please pass a name on the query string or in the request body");
} else {
     return request.createResponse(200, "Hello, " + name);
}

with this:

return request.createResponse(500, "This unit test is pointless");

• Run test again => still passes.

By default you're mocking all characteristics of the response. Except for the case when the function returns null, the test can never fail.

[jdneo]

by @hf-kklein in microsoft/vscode-azurefunctions#491 (comment):

Here's an alternative approach.
Create a class that implements HttpResponseMessage:

package com.your.package;

import java.util.HashMap;
import java.util.Map;

import com.microsoft.azure.functions.HttpResponseMessage;

public class MockingHttpResponseMessage implements HttpResponseMessage<String> {

    private int status;
    private Map<String, String> headers;
    private String body;

    public MockingHttpResponseMessage() {
        this(0, null);
    }

    public MockingHttpResponseMessage(int status, String body) {
        this.status = status;
        this.body = body;
        headers = new HashMap<String, String>();
    }

    @Override
    public int getStatus() {
        return status;
    }

    @Override
    public void setStatus(int status) {
        this.status = status;
    }

    @Override
    public void addHeader(String key, String value) {
        headers.put(key, value);
    }

    @Override
    public String getHeader(String key) {
        return headers.get(key);
    }

    @Override
    public String getBody() {
        return body;
    }

    @Override
    public void setBody(String body) {
        this.body = body;
    }
}

Replace these lines in the unit test:

final HttpResponseMessage res = mock(HttpResponseMessage.class);
doReturn(res).when(req).createResponse(anyInt(), anyString());

with this:

doAnswer(new Answer<MockingHttpResponseMessage>() {
            @Override
            public MockingHttpResponseMessage answer(InvocationOnMock invocation) throws Throwable {
                Integer status = (Integer) invocation.getArguments()[0];
                String body = (String) invocation.getArguments()[1];
                return new MockingHttpResponseMessage(status, body);
            }
        }).when(req).createResponse(anyInt(), anyString());

Don't forget to

import static org.mockito.Mockito.doAnswer;

Now you can use assertions to test the result of your function.

See this StackOverflow thread for mockito details.

[hf-kklein]

Note: Since the breaking changes introduced recently , my example code from above doesn't work anymore. You have to mock HttpResponseMessage.Builder now.

[jdneo]

@hf-kklein Thanks.

This is a worth discussing topic about what should be tested in our unit test.

Looks like doAnswer() is to test whether the response is the same as it has been generated.

While another thought you provided is to test whether the response is 500 or not.

I'll think about this. Please also feel free to provide your thoughts on this topic if you want.

[hf-kklein]

Hi,

the reason I originally opened the issue was because I used to test an Azure Function App with different input parameters to trigger different response codes. In my use case the the actual logic is encapsulated and tested in separate classes. The Azure Function App just acts as a wrapper to expose the function logic to HTTP API requests. In consequence the sole purpose of the function app unit test is to test if well formed inputs lead to a positive return code and if malformed requests with missing or illegal parameters result in a negative response code.

That's why I found the default unit test generated by the Azure Function App PlugIn for VS Code to be pointless. The test just passed no matter what input I provided as long as no exception was thrown. In case of a simple "Hello World" function there's barely any advantage over just avoiding compile time errors. In fact there was no possibility to test if e.g. a missing name resulted in a HTTP status code 400 or if a given name returns the expected result 200 + "Hello, <name>". Since the test just mocks the interface of the HttpResponse object you could not even determine the HTTP response from outside the function because all setters inside the function app and getters of response body and status code in the unit test had no effect at all. The latter is the reson why I created the MockingHttpResponseMessage class in the code above.

In my opinion a good unit test should allow for testing different paths and different outcomes of a function. The default test doesn't meet this requirement.

I appreciate your time and feedback.

[jdneo]

@hf-kklein My apologize for the late reply.

Yes, I think that by adopting the doAnswer() here can make the unit tests better. Will work on a PR for this.

[jdneo]

@hf-kklein I submitted a PR in #45. Please let me know if this addressed your advice. 😄