tags:

views:

81

answers:

3

Should we aim for DRY, in the sense that changes to the functionality affect as little code as possible, our predictability, in the sense that operation of the code is trivial, when writing unit tests? Basically I'm asking about the trade-off between creating helper methods that are very general and that can be used by multiple unit tests versus only ever constraining the testing code to a single unit test. As an example take the case of a factory which has the following method signature:

public Node buildNode(String path, String name, Map<String, Object> attributes);

Depending on the parameters provided the resultant Node object will be different, and as such we need to test the different possibilities. If we aim for predictability we might write two self-contained unit tests as given in the first example, but if we aim for DRY we'd rather add a common helper method such as in the second example:

EXAMPLE1:
@Test
public void testBuildBasicNode() {
  Node node = testee.buildNode("/home/user", "Node", null);
  assertEquals("/home/user/Node", node.getAbsolutePath());
  assertEquals(false, node.isFolder());
}

@Test
public void testBuildAdvancedNode() {
  Map<String, Object> attributes = new HashMap<String, Object>();
  attributes.put("type", NodeType.FOLDER);
  Node node = testee.buildNode("/home/user", "Node", attributes);
  assertEquals("/home/user/Node", node.getAbsolutePath());
  assertEquals(true, node.isFolder());
}

EXAMPLE2:
@Test
public void testBuildBasicNode() {
  Node node = testee.buildNode("/home/user", "Node", null);
  Node comparisonNode = buildComparisonNode("/home/user", "Node", null);
  assertEquals(comparisonNode, node);
}

@Test
public void testBuildAdvancedNode() {
  Map<String, Object> attributes = new HashMap<String, Object>();
  attributes.put("type", NodeType.FOLDER);
  Node node = testee.buildNode("/home/user", "Node", attributes);
  Node comparisonNode = buildComparisonNode("/home/user", "Node", attributes);
  assertEquals(comparisonNode, node);
}

private Node buildComparisonNode(String path, String name, Map<String, Object> attributes) {
  // Not just a duplicate of the buildNode method,
  // can be more trivial if we only limit it to unit tests that share some common attributes
  ...
}

My problem with the first example (predictability) is that if any functionality changes (like say how the AbsolutePath should be formatted), it requires changes across all my unit tests. My problem with the second example is that buildComparisonNode feels like something which should be tested as well, and I certainly don't want to start writing tests for tests.

Also, as a closing thought, would you declare final variables for the literal Strings used in the example unit tests, or are they fine as they are?

+2  A: 

While DRY is applicable to production code it isn't always applicable to unit tests. You really want each test to be independent of each other and often that means repeating yourself. On the other hand, I do find it useful to group certain things into helper methods that are used by all tests as long as it doesn't couple the tests together then it should be fine. One place I usually reduce duplication is by using test data builders to construct objects that exist in a particular state in my test.

My rule of thumb is to keep my tests as small and as readable as possible. If using DRY can help achieve that then I use it. If not, then I don't. :-)

Hope that helps. I'm not a world expert at unit testing so I could be very wrong. :-)

mezoid
+2  A: 
  1. Good question. I've heard before "unit tests can be wet, but not soaking wet..." For maintainability of the test, focus (or predictability) is key. The larger your team is, the more important I think that should be to you. Another thing to consider is that if there is specific test helper code, that can become an API in itself, so it may not be a bad idea if everyone knows how to utilize it. My rule of thumb is I will remove duplication if I can do it with an IDE in an automated refactoring and I can give it a good name.

  2. A suggestion... check out the Test Data Builder pattern writeup by Nat Pryce about a more maintainable/extensible approach.

cwash
Thanks, that Test Data Builder pattern is ingenious!
Zecrates
+1: Don't obsess about DRY unit tests, that defeats the purpose. Tests must be super clear (to be trustworthy) and do not have to be efficient at all. Any decent unit test library allows inheritance, so you can easily put some common code in a superclass.
S.Lott
A: 

Another thing to think about - often when you are removing duplication in tests it's telling you that something is production code or design that is waiting for a legit reason (a use in the production codebase) to move it into production code. I can't remember exactly where I got that ah hah moment from... but I think it was related to TDD as if you mean it.

cwash