Useless unit tests
I’m currently in the process of restructuring an ASP MVC project. The system was built around 9 months ago and unfortunately there are a few issues around the architecture of the system and as a result there is business logic that has creeped into the ‘data’ project. The current architecture of the system is something like this:
It is obvious there is something missing, a business project. As a result of this the business logic mainly resides within the repository objects with some creeping into the web project as well. Not great!
Before the next phase of development begins I need to restructure the application so the logical separation between the data, business logic and web tier exists. The architecture will then be much closer to the 3-tier architecture that is common in this kind of application.
The TDD methodology was loosely followed when this application was originally written and as a result there is a suite of unit tests providing a decent amount of coverage over the existing code base. This is where my problems restructuring the project start!
Even though dependency injection was used to decouple the elements of the application the majority of the unit tests ended up failing when the references to the data project were removed from the web project. I believe a lot of the unit tests that were written on this project are useless! If they weren’t before I started restructuring this project, they are now!
Most of the tests in this project were testing the implementation of the code opposed to the behaviour. The tests tended to be along the lines of:
[TestMethod]
public void TestThatOpenByIdCallsFeeRepository()
{
//arrange
_mockFeeRepository.Setup(x => x.GetById(1)).Returns(_stubFee).Verifiable();
var sut = new FeeController(_mockFeeRepository.Object, _mockFeeRecordRepository.Object, _mockUserRepository.Object);
//act
var result = sut.OpenById(1);
//assert
_mockFeeRepository.VerifyAll();
}
In this scenario, this test is completely irrelevant now as I won’t be calling a repository from the OpenById method, I will be calling the fee business object. I might as well delete all the unit tests like this one. These kinds of tests offer almost no value in hindsight, but they have provided a valuable learning opportunity. My understanding of the purpose of unit tests has been corrected and I have come to realise how beneficial it would be to have a suite of integration tests. This would give me complete confidence when restructuring a project the systems behaviour hasn’t changed, regardless of what architectural changes I have made behind the scenes.
There are some useful tests within the project. For instance I have an object called Month. This supports that fees are created for a specific month however the .NET DateTime object uses the whole date including day etc. which aren’t relevant. I have a collection of tests for this object which are testing the behaviour of the class. The Month object has methods such as GetNumberOfWorkingDays and ContainsDate(DateTime date) and each of these methods can be thoroughly tested including any edge cases that can be identified. This is where the true value of unit tests lie. One observation I have made is that the unit tests that provide the most value, seem to be the simplest. Not one test for the Month object has any need for mocking dependencies whereas the tests for the repository or the controller methods had numerous mocked dependencies that were injected.
I’m almost finished integrating the UI with the new business layer and once this has been done I’m going to have a big job on my hands to thoroughly test the application. (Fortunately I know how the system should work and I hope that there won’t be too many problems). As part of this testing I’m going to start writing integration tests to cover any future work. This should provide a good foundation for some upcoming development work on the project and with my new understanding of what to test and how, I believe I can produce much higher quality code that is thoroughly tested and therefore more maintainable.