A quick look revealed that all the unit tests show a typical testing antipattern: they are optimized for 100% code coverage. In order to reach this, the SUT is modified to expose the internals of the code, not only the public interface of it. All methods' visibility were changed to public/protected, and tests covered not only the method calls and their results, but the very details of the implementation as well. This led to a very fragile environment, where even a tiny little change in the implementation could potentially break most of the unit tests. Moreover, the test were structured very poorly, effectively rendering the readability to the bare minimum.
After we finished the code review, it was clear that we need to refactor the whole test suite, not only some tests of it. First we should change the SUT, and revert all the visibility related changes. Then we dropped all the tests that verified the internals of the SUT. The remaining tests were refactored to increase the readability. We also needed to make changes to these tests to verify the right thing. Finally, we wrote some new tests against the public API of the SUT to reach the desired coverage level.
To sum up:
- Unit tests are part of the documentation. They should be readable and clearly structured (given, when, then).
- Unit tests are only useful if they increase the productivity of the developers by helping them catching bugs earlier. Failing to comply this requirement means that they do not add value to the development process.