4

I have a class A, that looks like the following:

public class A {

  // there is a public API that uses both
  // method1 and method2, but that's not important
  // in this question.
  public Integer publicApi(Integer a, Integer b) {
    // a bunch of other stuff is done that is 
    // not related to method1 or method2 here.
    // However, method1 IS used somewhere in here.
    // For now, I don't want to test other
    // implementation details for this API, I only
    // am interested in testing method1.

    return method1(a, b);
  }

  private static Integer method1(Integer a, Integer b) {
    if(method2(a, b, 10) == null) {
      return -1;
    }
    return 10 + method2(a, b, 10);
  }

  private static Integer method2(Integer a, Integer b, Integer c) {
    // some complicated logic with multiple values 
    // to return (based on various conditions).

    // however, I want to simply test what happens
    // when this method returns null (which is a possibility).
    return a+b+c;
  }
}

As is given, my class A has a public API that's (sort of) complicated logic, that I don't want to test (for the purpose of this question). Say I am only interested in testing method1.

Here is my test code (I'm using PowerMockito):

import org.junit.Test;
import org.junit.runner.RunWith;
import org.powermock.core.classloader.annotations.PrepareForTest;
import org.powermock.modules.junit4.PowerMockRunner;

import java.lang.reflect.Method;
import static org.junit.Assert.assertNull;
import static org.powermock.api.mockito.PowerMockito.spy;
import static org.powermock.api.mockito.PowerMockito.when;
import static org.powermock.api.support.membermodification.MemberMatcher.method;

@RunWith(PowerMockRunner.class)
@PrepareForTest(A.class)
public class SimpleTest {

  @Test
  public void testMethod1Invocation() throws Exception {

    spy(A.class);

    when(A.class, method(A.class,
        "method2", Integer.class,
        Integer.class, Integer.class))
        .withArguments(1, 2, 10)
        .thenReturn(null);

    // use reflection to call method1
    Method method1 = A.class.getDeclaredMethod(
        "method1",
        Integer.class, Integer.class
    );

    method1.setAccessible(true);
    Object ret = method1.invoke(1, 2);
    assertEquals(-1, ret);
  }
}

But, I get the following error:

com.apple.geo.tart.A.method1(java.lang.Integer, java.lang.Integer, java.lang.Integer)
java.lang.NoSuchMethodException: com.apple.geo.tart.A.method1(java.lang.Integer, java.lang.Integer, java.lang.Integer)
    at java.lang.Class.getDeclaredMethod(Class.java:2130)
    at com.apple.geo.tart.SimpleTest.testMethod1Invocation(SimpleTest.java:32)

I don't see what I'm doing wrong here. As it seems to me, there are 3 things that I need here:

  1. Spy on A so that I can mock when/thenReturn on the static methods.
  2. Mock the private static method that I'm interested in (method2 in this case).
  3. Use reflection to access method1, and then invoke it.

How do I then mock a private static method (method2), just so that I can test another private static method(method1) of the same class that depends on the first?

Makoto
  • 104,088
  • 27
  • 192
  • 230
Nishant Kelkar
  • 412
  • 1
  • 4
  • 20
  • 5
    Is it absolutely necessary to test the private methods? In general you should only be writing tests for public methods. You can put `verify` to see if any of your mocks are being called in the private methods. – user2004685 Mar 13 '16 at 08:22
  • exception says you look for `method1` with 3 parameters while your code shows 2 parameters call. Try to recompile your project -- it looks like code doesn't refer to binaries under test. – Sergey Grinev Mar 13 '16 at 08:22
  • @SergeyGrinev I actually did try that, wiped out the build directory, and re-assembled the code (using Gradle). But I still get the same error. – Nishant Kelkar Mar 13 '16 at 08:42
  • @all, found the problem. The issue is ``method1.invoke(...)``. The first argument it takes is the *object* to invoke the method on, not the actual args! When testing a static method of a class, this is usually ``null``. Changing that method call to ``method1.invoke(null, 1, 2)`` gives the expected result. – Nishant Kelkar Mar 13 '16 at 08:48

1 Answers1

5

I've got some issues with this code before we talk about the testability of it:

  • method2 will never return null in actuality. If any of the inputs to it are null, you will get a NullPointerException due to Java attempting to unbox null. Therefore, the condition you're attempting to test for is fairly pointless.
  • There's really no value in having these methods hidden. They appear to do business-level calculations, and thus have value in being tested.

I will say this much for your initial case: I don't get the same error that you do. From what I see with Java 8, you're missing a parameter to your invoke method call, with which you need to pass method1 in.

Object ret = method1.invoke(method1, 1, 2);

However, I do not recommend this at all. Let's break this apart into a few tests.

First, let's loosen the restrictions on those methods. They're likely private because we don't to expose their behavior elsewhere, but there's likely no harm if we weaken that to package-private. Remove the private marker from those methods.

Second, actually test those methods! Those methods can be influenced by a null and you should test against that scenario.

Third, and only after have you tested the other two methods, should you come back to your publicApi method. At this point, all you care about is that the method was simply invoked with the right parameters.

Makoto
  • 104,088
  • 27
  • 192
  • 230
  • thanks for the answer. Regarding your first bullet point in the beginning, as I mentioned in the comment of ``method2``, a ``null`` is a possibility in my use-case. For example: ``if(b == (a+c)/2) return null;`` Regarding your other points, I think I agree. Confining them to being package-private is one way to go. – Nishant Kelkar Mar 13 '16 at 09:10
  • I would recommend against returning `null` in that scenario. Surely there's a better thing - like a number - like zero - to return? – Makoto Mar 13 '16 at 09:11
  • so as you've probably guessed, I've masked a lot of stuff in the above example. I'm not using ``Integer`` params in my app, but I just used them here since I thought they make for a good example. In my app, I'm actually passing in/out custom Java objects, and using null as a marker that something went wrong in ``method2``. – Nishant Kelkar Mar 13 '16 at 09:15
  • If something goes wrong, raise an exception. That's what exceptions are for. But that'd be my advice. – Makoto Mar 13 '16 at 09:20
  • Thanks for your comments! I refactored some of the code per your suggestions in the answer (removed ``private`` modifiers, now I can call ``A.method1()`` and ``A.method2()`` in the tests, etc.) and it seems to be a reasonable layout for my class ``A``. Marking this as the accepted answer. – Nishant Kelkar Mar 13 '16 at 19:25