0

I have an Controller action method that regardless of what happens always returns null, because I want it to reload the same page. (JSF 2.2).

I have successfully used mockito to orchestrate execution down every possible path.

However every path mostly calls an abstract method to add a message or calls a 3rd party library.

So i can assert that it returns null, but that's the case regardless. I see this pattern repeating as development continues.

Problem: I'm always testing for null despite the execution path.

Possible Solutions as I see them:

  1. I'm very green to mockito, so there might be something else I can do to verify that the 3rd party and abstract methods are called.
  2. I feel that something that could be useful but hacky is a status flag of some sort to know what kind of message was just added to the stack. Hacky because its only real use is that it will be used for testing, as I see it currently.

  3. Reevaluate my methods, in that if I'm in this situation because my code design is wrong.

  4. I don't have a problem. Leave it as is, and be satisfied that I'm running every execution path and verifying its outcome even thought its the same.

Question: Given what is known, which direction would you consider first to try and solve the problem of verifying internal execution paths externally in a unit test? Or is there a better solution?

Thanks in advance.

Updating with example code to explain verification troubles, if that's the route i should take:

try {
   account.save();  //<-- third party object i don't own, & returns void
   addInfoMessage("All Updated!");  //<-- abstract method
  } catch (final ResourceException e) {  //<-- third party exception
   addErrorMessage("Sorry your account could not be updated. ");//<-- abstract method
   LOG.error("error msg");
  }
  ...
  return null;
Pompey Magnus
  • 2,191
  • 5
  • 27
  • 40

2 Answers2

0

The solution is the first one. Your method does nothing except side-effects on dependencies, and possibly change the state of the object under test. That's those side effects, and the new state of the object that you want to test.

Testing the new state is simple. Testing the side effects on the dependencies is done using the verify() method of Mockito, as explained in the very first item of the official documentation:

//Let's import Mockito statically so that the code looks clearer
import static org.mockito.Mockito.*;

//mock creation
List mockedList = mock(List.class);

//using mock object
mockedList.add("one");
mockedList.clear();

//verification
verify(mockedList).add("one");
verify(mockedList).clear();
JB Nizet
  • 678,734
  • 91
  • 1,224
  • 1,255
  • I should have stated that I verified in other tests and I'm aware of it. But in this case I'm not sure it holds true, i will update my question with an example. – Pompey Magnus Nov 23 '14 at 18:00
0

It looks like there are at least two outcomes, so covering them is important. You can slightly redesign the class to make testing easy.

public abstract class YourClass {
  protected abstract void addInfoMessage(String message);
  protected abstract void addErrorMessage(String message);

  public void closeTransaction() {
    try {
      saveAccountInternal();                                                /* ! */
      addInfoMessage("All Updated!");
    } catch (final ResourceException e) {
      addErrorMessage("Sorry your account could not be updated.");
    }
    return null;
  }

  /** Package-private. Overridable for testing. */
  void saveAccountInternal() throws ResourceException {
    account.save();
  }
}

You're designing a class for subclassing, so test it using a subclass:

public class YourClassTest {
  private static class TestYourClass extends YourClass {
    boolean saveCalled = false;
    boolean shouldThrow = false;
    List<String> infoMessages = new ArrayList<>();
    List<String> errorMessages = new ArrayList<>();
    protected void addInfoMessage(String message) { infoMessages.add(message); }
    protected void addErrorMessage(String message) { errorMessages.add(message); }

    @Override void saveAccountInternal() throws ResourceException {
      saveCalled = true;
      if (shouldThrow) throw new ResourceException();
    }
  }

  @Test public void closeTransactionShouldSave() {
    TestYourClass testYourClass = new TestYourClass();
    assertNull(testYourClass.closeTransaction());
    assertTrue(testYourClass.saveCalled);
    assertEquals(1, testYourClass.infoMessages.size());
    assertEquals(0, testYourClass.errorMessages.size());
  }

  @Test public void closeTransactionShouldSave() {
    TestYourClass testYourClass = new TestYourClass();
    testYourClass.shouldThrow = true;
    assertNull(testYourClass.closeTransaction());
    assertTrue(testYourClass.saveCalled);
    assertEquals(1, testYourClass.infoMessages.size());
    assertEquals(0, testYourClass.errorMessages.size());
  }
}

Note that the solution above doesn't involve Mockito. Once you're comfortable with that test redesign, you can consider using Mockito to automate the creation of the subclass as in this SO answer.

public class YourClassTest {
  private YourClass stubYourClass() {
    YourClass yourClass = Mockito.mock(YourClass.test, Mockito.CALLS_REAL_METHODS);
    doNothing().when(yourClass).addInfoMessage(anyString());
    doNothing().when(yourClass).addErrorMessage(anyString());
    doNothing().when(yourClass).saveAccountInternal();
    return yourClass;
  }

  @Test public void closeTransactionShouldSave() {
    YourClass yourClass = stubYourClass();
    assertNull(yourClass.closeTransaction());
    verify(yourClass).saveAccountInternal();
    verify(yourClass).addInfoMessage(anyString());
    verify(yourClass, never()).addErrorMessage(anyString());
  }

  @Test public void closeTransactionShouldSave() {
    YourClass yourClass = stubYourClass();
    doThrow(new ResourceException()).when(yourClass).saveAccountInternal();
    assertNull(yourClass.closeTransaction());
    verify(yourClass).saveAccountInternal();
    verify(yourClass).addErrorMessage(anyString());
    verify(yourClass, never()).addInfoMessage(anyString());
  }
}

Of course, asserting "never" on info/error message calls may make your test more brittle than you'd want; this is just to show that testing using a manual or Mockito-generated subclass can give you all the articulation you need for that sort of test.

Community
  • 1
  • 1
Jeff Bowman
  • 90,959
  • 16
  • 217
  • 251