Search code examples
javaunit-testingmockitoaspects

Can I ignore aspect of a method while mocking it using Mockito?


I have a class with a few methods advised through an input validation aspect (validates whether all input parameters are not-null/non-empty strings).

I am facing an issue while writing test case for them and want to verify if this is indeed a bad design issue.

Here's a very simplified version of my class:

public class A {

public String one(String word) {
// Some actions
String val = two(word2);
// Some more actions
}

protected String two(String word) {
// Some actions
}

}

Now while writing test cases for one() I use Mockito and want to mock calls to two(). So I use:

@Spy
A a;

@Test
void test() {
doReturn("Bye").when(A).two(Mockito.anyString());

a.one("hello");
// Some validations
}

This test fails as the: doReturn() line fails with input being empty for two().

Should I not mock two() or can I make this work somehow?

Edit:
Adding a more specific example related to the two methods being present in two different classes as requested:

Create a page through a WebService. This builds a putRequest, executes it and returns a response.

public class AUtility implements BaseUtility {
    public Response create(Params params) {

        try {
            PutMethod putRequest = buildPUTRequest(params.getAttr1(), params.getAttr2());
            return Utils.buildResponse(client.executeMethod(putRequest),
                                                     params.getAttr3(),
                                                     params.getAttr4());
        } catch (Exception e) {
            throw new AppException(e);
        }
    }
}

The put request marshals the data into a file to write it through the HttpClient

private PutMethod buildPUTRequest(final String url, final Object obj) throws IOException, JAXBException {

    // Create a temp file to store the stream
    File tempFile = File.createTempFile(APPLICATION_LABEL, XML_LABEL);
    decoder.marshal(obj, tempFile);

    // Build the put method

    return putMethod;
}

XMLMarshaller

public interface XMLDecoder implement Decoder {

    public void marshal(Object obj, File tempFile) throws IOException, JAXBException {
    // Perform marshalling operations 
    }
}

The test fails on line2 with the inputs being null.

@Test
public void createPageParamsHttpException() throws HttpException, IOException, JAXBException {
    expectedException.expect(AppException.class);
    doNothing().when(decoder).marshal(Mockito.anyString(), Mockito.any(File.class));
    doThrow(HttpException.class).when(client).executeMethod(Mockito.any(HttpMethod.class));

    Params params = new Params(new Application(), 
                           APPLICATION_URL_LABEL, 
                                 SITE_NAME_LABEL,
                         URL_WITHOUT_HTTP_N_HTML);

    utility.createPage(params);
}

Any idea how should I proceed for the same?


Solution

  • You don't want to do this.

    You are inherently changing the behavior of the class. If you change what two() does, how do you know that one() will do what it's supposed to do in production?

    If you truly want to do this, you should extract the behavior of two() into another top level class, and then inject the dependency into A. Then you can mock this dependency and you don't have to worry about going to the trouble of creating a partial mock for A.

    In a similar vein, if you must keep two in the same class (because it's behavior is part of the same responsibility that is assigned to A - see the Single Responsibility Principle - why is it public?


    The reason you are having trouble is because you are violating the SRP, see my note above. You said this:

    This builds a putRequest, executes it and returns a response.

    You should not be trying to test the behavior of all three of those things at the same time. Ultimately, this method does not really do anything. The buildPUTRequest method does, and shouldn't be in a class called AUtility, it should be in a class RequestFactory. Then, you would want to test the Utils.buildResponse method, except that shouldn't be in a class called Utils, it should be in a class called Responder or something... and this method ABSOLUTELY should not be static.

    Work on naming your classes better things, and if you can't come up with a good name, that means the class probably does too much and should be refactored. And a method that wraps the work in two other methods doesn't need to be unit tested. Integration tested, perhaps, but that's another story.