Every now and then a team will find itself with an architecture that isn't quite right or a system that isn't performing quite as everyone would have hoped so. In these cases the inevitable happens - The Big Refactoring or Optimization. Now there is nothing wrong with this, evolving architectures and code bases are a fundamental part of agile software projects. What I'm sharing today are some insights that I gained while recently undertaking this journey with my team:
- Having a high unit test coverage is a requirement but it isn't enough
- Solid acceptance and or functional tests are a godsend
The biggest learning from this exercise was that even though you might have a high unit level coverage (> 92%) things still manage to sneak through the cracks. While all those execution paths might be covered, they might not be tested that well. Furthermore, a good unit test is one that tests itself in isolation of others. The problem then arises that if those isolated unit tests aren't thoroughly testing things at their level, problems don't show themselves until a much higher level - once everything is integrated.
To put this point into context, we were doing optimization's around repository calls for entities from the database. The tests for the repository methods probably didn't cover as many scenarios as they should have. After the optimization the tests all passed, the repository methods were still returning the same information that the tests were expecting. The problem of course was that they weren't returning the exact information that they used too. This resulted in some hard to find bugs popping up on the UI layer that took some time to pin back down to those refactored repository methods.
This is where a good acceptance and or functional test suite can come in handy. I'm not saying that they should be some how testing the right data is coming in and out of the repository but a decent functional test suite will at least fail on one of the flows affected by the refactoring. An investigation into the failure would then lead to the discovery of the fudged refactorings. Unfortunately in our case, we weren't doing BDD or ATDD and this refactoring was occurring before those tests existed. It must also be noted that if a functional and or acceptance test does happen to discover one of these bugs its more of a bonus, these cases really need to be covered at the unit level.
Another important learning was that these optimization's and refactorings should be done by someone who has context on the code that is being worked on. This is because if those unit tests are not sufficient, at least you are pairing with someone who has a bit of an idea of what that code should be doing.