ExampleTo illustrate my point I'll play around with the following Employee class from an imagined resource allocation system. The system handles work assignments and can only assign them to employees that
- do not have overlapping assignments
- have the necessary skills
- cost less per hour than the assignment pays
The implementation of Employee shown here is simplistic. Real business rules would no doubt be more involved. Nevertheless this class serves to illustrate the point.
If we follow the notion that the 'unit' of a unit test is a production class it is reasonable to assume that there is a test class corresponding to Employee and that it could look something like this:
Refactoring Production CodeEven though the employee class is simplistic it still has several reasons to change:
- Changes to the business rules around matching employee skill set and assignment requirements
- Changes to the business rules for employee availability, for instance introduction of part time assignments
- Changes to the business rules around employee cost ans assignment rate
As a first refactoring lets introduce a SkillSet class capable of comparing a set of skills held by an employee and a set skill needed by a task:
which would change the Employe class to this (hint: The changes are in line 12 and 19):
At this point the existing tests need just one follow one change in its constructor:
After that the tests will pass again and - importantly - they will still cover all our code including the SkillSet class.
Refactoring the Test Code?The Employee_should class now tests not only the Employee class but also the SkillSet class. If we assume that the 'unit' of a unit test is a production class Employee_should is no longer a unit test. Following this line of thought leads towards two changes to the test code:
- Mock out the SkillSet in the Employee_should. Typically this involves first introducing an interface on top of SkillSet and then using a mocked version of that in the Employee_should
- Create a new test class for the SkillSet - Skillset_should
I'm not going to write that code, but I'm sure you can imagine it.
Test Code SmellIf we went ahead and did the refactorings to our test code it would be becoming quite smelly:
- We would have lost all testing of the collaboration between Employee and SkillSet. This is pretty serious since that is in fact where part of the business rules are upheld. These parts of the rules would effectively become untested.
- The Employee_should test class would become more complicated by the extra setup of the mocked SkillSet - this might not seem too bad at this point, but quickly becomes unwieldy when dealing with more complicated scenarios
- The Skillset_should test would not add much - it almost just checks that the framework method IsSubsetOf works.
If we, on the other hand, follow the line of thought that the 'unit' of a unit test is a unit of behavior, we would just keep the unit tests as the are after the introduction of the SkillSet class. This is better because the tests are not hampering the freedom to refactor the production code.