Encapsulation Practices When Mocking Queries

As a developer and unit tester, I agree with the TDD goal to test only the interface of a class and ignore the private methods. Those private methods are implementation details of the class and can change with any refactoring without affecting the actual public method result and hence do not need tested if you test the public methods.

But what should a tester do when he needs a implementation details to set up mocks?

Say you have a class:

public class UserPermissions {

private final String CREATE_ORDERS_PERMISSION_QUERY = "SELECT create FROM permissions WHERE objType = 'Orders'";
  // ... other fields, constructors etc
 
  public boolean hasCreateOrdersPermission() {
 
  boolean createPermission = false; 
  try(PreparedStatement preparedStatement = connection.prepareStatement(query);
      ResultSet resultSet = statement.executeQuery()){
    while(resultSet != null && resultSet.next()){
      //... process results and set createPermission
    }
  } catch (SQLException sqlE) {
    // handle
  }
  return createPermission;
  }
}

If I want to test this code with a tool such as MockRunner, a tool to mock JDBC ResultSets, I need to mock the query the method will call, CREATE_ORDERS_PERMISSION_QUERY. However, the test cannot access the query because it is private. Should you use something like Jmockit’s Deencapsulation to make the private method visible to the test, or should you change “private final String” to “final String” which makes the String “package private” which is available to the Unit Test class in the same package.

Deencapsulation vs default/package private

In my experience, tools like Deencapsulation point to serious issues in your unit test strategy. If we agree that private methods are used for implementation details and that you should not be testing or relying on implementation details when writing tests then Deencapsulation violates multiple best practices.  Another issue I have with Deencapsulation is that using Deencapsulation.invoke() on a field deliberately invalidates the private modifier you put on the object, so why make it private at all? If it’s private, keep it private.  If it’s not private, just mark it package private and move on with your testing. Therefore, opening up that variable with Deencapsulation is a bad practice. However, I happen to agree that encapsulation is best with the String bring private than package private.  Package private suffers from the same fallacy as private deencapsulation, if it is really supposed to be private then opening it up with package private is incorrect.  Keeping it private allows for refactoring of the underlying fields or methods without breaking ‘public’ method unit tests.

Since private and package private both have cons and that testing both have cons then you could come to the conclusion that mocking mocking private components is bad.  This makes perfect sense to me. Don’t test or mock private components.  So then what do you do, skip unit tests and do integration tests? Integration tests need to know what tables and fields need set up for the code-under-test to run correctly.  These are most certainly implementation details.  If you will not change a method from private to package private then you would need to integration test and I would certainly rather mock objects than force a user into an integration test.

It’s too bad there isn’t a “test private” modifier in Java.  Some other languages have this feature.

There is an interesting option provided by Guava – @VisibleForTesting. It allows you to mark a method as package private, protected, or public but the build will fail if you use the method anywhere in the main source code.  It can however be used in tests.  This is an interesting option because it is a psuedo “test private” modifier.  According to you IDE you can use the method from other classes but according to the build, it will fail.  This is an interesting option.

Extract complex private methods to classes

Continuing on the idea that testing private methods is bad and package private is bad, then there is a solution: extract the private method to a public class.  This may seem a little odd at first, but it can make sense.  The idea is that if a private method needs tested or it needs used in testing, then it must be visible enough of be it’s own class.  Another way to look at it is “this code is not a refactor-able implementation detail, this is important logic that can’t change without repercussions.  Therefore, let’s unit test it and move it out of the UserPermissions class because this is not a part of the UserPermissions interface.”  This means that the field on UserPermissions still has private modifier and that UserPermissions unit test never needs to invoke that private method.  Note that the method will be tested via that new class’es own unit test class.

This keeps everyone happy except when taken to an extreme:  breaking a functional class into two or three sub classes that are basically utility classes and need each other to work.  This should be avoided at all costs.  This breaks encapsulation and causes even more problems than just changing the modifier or using Deencapsulation.  Therefore this can only be done ‘properly’ if breaking up the classes/private methods into new classes follows good OOP design.

Wait… I think we’ve made a huge mistake

I was really bothered by an integration testing comment I made earlier. If coding tests to implementation details is bad, then coding unit tests to implementation details is bad, then coding integration tests to implementation details is bad.  But isn’t this what automation engineers do every day?  They write tests and setup data on external systems to verify integration is correct.  Is there an exception for integration tests that allows writing tests around implementation details? ……

….NO! There is no exception. When implemented correctly Integration tests when verify REQUIREMENTS.  They verify external interfaces. Just like unit tests verify interfaces of classes.  The problem with this entire article is that I’m trying to mock the QUERY which is an implementation detail.  What I should be mocking is the thing I’m integrating with – the DB.  This is a shortcoming of the MockRunner tool. We should not be mocking the query, we should be mocking the tables and permissions and data in the DB.  If we do that, we don’t need this private field at all.  Maybe we just need to find a new JDBC testing tool.

Conclusions: Integration testing is bad. private should not be mocked. package private creates poor encapsulation. Deencapsulation deliberately breaks the private setting on the class. Deencapsulation allows for proper encapsulation in all other cases except for testing. @VisibleForTesting sounds interesting. If the private method is important or complex enough to be used in tests then make it it’s own class but don’t touch the private modifier in the UserPermissions class.

So what is the right answer?

  1. Avoid the entire question by not mocking implementation details.
  2. Use a language with “test private” (Can’t switch from java)
  3. Other answers in no particular order….
    • Keep the variable private and use Deencapsulation
    • Make the variable package private
      • And possibly use @VisibleForTesting
    • Create public class with the private implementation details.
  4. Skip unit tests and use integration tests. (lol)

What do you think?

Good references for this topic:

Don’t test private methods at all

– but in legacy code you may have to until you have refactored out complex public methods.  If you still have a private method left that should be tested, it should most likely be it’s own class.
http://www.caroli.org/testing-private-methods-tdd-and-test-driven-refactoring/

Create a new class if it’s not just “implementation detail”
http://softwareengineering.stackexchange.com/questions/135047/new-to-tdd-should-i-avoid-private-methods-now

Advocates changing private to package-protected (default)

https://github.com/mockito/mockito/wiki/Mockito-And-Private-Methods
https://dzone.com/articles/unit-testing-private-methods
Put the @VisibleForTesting annotation on ‘package private’ methods and use https://github.com/Monits/findbugs-plugin to fail the build if the method is used outside of unit tests.

Keep Private and Use Reflection / Deencapsulation

http://stackoverflow.com/questions/34571/how-do-i-test-a-class-that-has-private-methods-fields-or-inner-classes/34658#34658

Advertisements

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s