“Test Code and Production Code – two distinct beasties”. Better late than never, they say…

There is something problematic about Mocks. They are just too convenient. Developers are lured into employing them all over the place, often forgetting Mocks’ primary function. And that is not the convenience of satisfying the dependencies of the code under test.

Mocks’ main purpose is to test interaction between Objects. This is why Mock verification is performed upon expectations of method calls. One of the golden rules of Unit Testing is testing one thing at a time. In this context, if there is more than one Mock per test case, this rule is probably broken.

public class CustomerServiceTest extends TestCase {

  public void testShouldSaveCustomerDetailsWithRepository() {
    Customer customer = new Customer("Kirk", "Hammet");

    CustomerRepository repository =
        EasyMock.createMock(CustomerRepository.class);
    AuditLog log = EasyMock.createMock(AuditLog.class);

    repository.save(customer);
    log.logCustomerSaved(customer);

    EasyMock.replay(repository);
    EasyMock.replay(log);

    new CustomerService(repository, log).save(customer);

    EasyMock.verify(customer);
    EasyMock.verify(log);
  }

}

Looking at the name of the test, we should be able to safely assume that we expect the CustomerService to use the CustomerRepository for saving a new Customer. Going through the code of the test, it is apparent that we are testing one more scenario apart from the interaction between the CustomerService and the CustomerRepository: We expect the CustomerService to call the logCustomerSaved(customer) method on the AuditLog. In other words, we are also testing the interaction between the CustomerService and the AuditLog.

Let’s imagine that at some point the call to logCustomerSaved(customer) is removed from the actual CustomerService code. This will result to a breaking test telling us that CustomerService Should Save Customer Details With Repository is no longer happening. This is clearly not the case.

There is a sloppy way out of situations like the above and that is stubbing the code that needs to be there in order for the test to run. This can be achieved either by coding a Stub for the AuditLog or by employing the stubbing facilities offered by Mock Object Frameworks, such as EasyMock’s createNiceMock() method. This doesn’t significantly impove readability, neither does it solve the underlying problem which is the usual suspect behind dubious tests: The Application Code is probably not modeled correctly. In the CustomerService case, logging is probably not happening in the right place.

The benefits of code being responsible for performing one task are well documented and apply to both Application and Test Code. Breaking the one mock per test limit usually signifies that this rule is not sufficiently applied.

If you’d like to follow the discussion, feel free to grab The Testing Anti-Patterns Drafts feed.

Tests are often the mirror of application code in terms of elegance and correct distribution of responsibilities. Stuart explains…

If you’d like to follow the discussion, feel free to grab The Testing Anti-Patterns Drafts feed.

Stuart on unreadable tests with convoluted expectations and unclear purpose.

Among other interesting engineering marvels, cars tend to have an accelerator and a speed meter. It would obviously make sense to test both in order to verify whether they’re doing their job properly.

If a test consists of putting the pedal to the metal and watching the speed meter react, would that give us confidence that the accelerator works as intended? To an extend, it would, especially if the speed meter has had its own fair share of testing. However, if a couple of days down the line somebody worked on the speed meter to give it a stylish racing white panel and accidentally broke the way the meter displays speed, the accelerator test would erroneously indicate that the accelerator is broken.

This kind of danger is apparent when testing a class’s methods with other methods of the same class:

class Repository
  def save record
    OrmFramework.save record
  end

  def load id
    OrmFramework.load id
  end
end

class RepositoryTest < Test::Unit::TestCase
  def test_should_save_record
    record = Record.new
    repository = Repository.new
    repository.save record
    assert_equal record, Repository.load record.id
  end
end

If we changed the implementation of the load method, the test_should_save_record test would potentially break, although the save method still works as intended. A more sane test would look like:

def test_should_save_record
  record = Record.new
  Repository.new.save record
  assert_equal record, OrmFramework.load record.id
end

The important detail is that OrmFramework is an external import, whose implementation we know is not going to change and we can assume it has been tested to work as expected. The moral is to avoid testing code with code that has not been frozen and is still code in progress.

If you'd like to follow the discussion, feel free to grab The Testing Anti-Patterns Drafts feed.

Stuart Caborn just delivered draft 2 of The Testing Anti-Pattern Drafts.

“The Testing Anti-Patterns Drafts” is a collaborative effort between Stuart “Stubs are evil” Caborn and myself, which aims to identify cases of Testing gone bad.

It consists of a single document that will undergo constant enhancements and modifications, in a “pair-authoring” manner, utilizing our respective weblogs as the platform.

We hope to get input from anyone following the document, our goal being to produce an interesting resource for the TDD, or Testing Oriented in general community.

Design Pervasive Testing

Both Stuart and I consider ourselves fortunate enough to work as members of teams where TDD is practiced by default.

There are two reasons beyond the obvious as to why I find it relatively harder to code in a non TDD approach. Test Driving my code enhances my vision on how to design/model/instrument my application’s universe. Also, starting with a test means work begins and ends with coding, not meetings, discussions or modeling our vision in pictures bound to be proven unrealistic when the first coding bottlenecks arise.

Having said that, testing properly is not easy. Past the simplistic examples, proper tests are hard work, often much harder than the actual code.

Encapsulation and Behavior are two of the most important characteristics of Object Oriented design. Applying them properly is difficult, and after all the hard work, testing Classes that are very concise about their behavior and very strict about what their peers can do with them, writing meaningful, readable, fine grained tests becomes a mission.

It’s the extra effort required that sometimes phases developers who end up taking shortcuts in order to facilitate testing.

Let’s look at an example:

public void testShouldProduceComplexObject() {
	ComplexObject expected = ObjectMother.getComplexObject();
	ComplexObject actual = classUnderTest.getComplexObject();
	assertEquals(expected, actual);
}

Obviously, such an assertion would only be valid if ComplexObject overrides the hashCode() and equals() methods. Countless times have I witnessed Classes implementing these two methods only to accommodate tests similar to the aforementioned example.

This is terminally wrong and a potential bug. Equality is an integral attribute of a Class’s behavior and should be strictly meaningful under the context of the Class’s definition.

I’m not aware of an easy way out of this sticky situation. Specific fields of the Class need to be tested for equality in isolation.

Does this mean that we need to expose all the Class fields we need to test by giving them public, or package level access? Not necessarily. In fact, not at all. After all, changing access levels would further compromise our Class’s behavior.

More and more, we have discovered Reflection to be an invaluable tool in our quest for less intrusive to purist OO design testing:

public void testShouldProduceComplexObject() {
	ComplexObject expected = ObjectMother.getComplexObject();
	ComplexObject actual = classUnderTest.getComplexObject();
	assertEquals(getPrivateField(expected, "fieldOne"),
		getPrivateField(actual, "fieldOne");
	assertEquals(getPrivateField(expected, "fieldTwo"),
		getPrivateField(actual, "fieldTwo");
	//etc...
}

If this were a container-bean type of Class with fifteen fields, that’s a lot typing we’d have to deal with. This is not necessarily bad for a Unit Test, but we could all do with less sore fingertips.

public void testShouldProduceComplexObject() {
	ComplexObject expected = ObjectMother.getComplexObject();
	ComplexObject actual = classUnderTest.getComplexObject();
	String[] fields = new String[] { "fieldOne", [...], "fieldFifteen" };
	for (String field : fields) {
		assertEquals(getPrivateField(expected, field),
			getPrivateField(actual, field);
	}
}

TBC…

If you’d like to follow the discussion, feel free to grab The Testing Anti-Patterns Drafts feed.