When asked to do a code review for a colleague, I noticed a couple of classes not tested. One, was a controller class with simple methods which each executed some code in a try/catch block which would re-throw on an error. The other class, had a few methods, one of which had a behemoth 270-line method. My colleague came to talk to me, pleading that the controller was worthless to test. “It’s so simple, I think a real test would be better suited for it,” she said. I tried explaining that to properly Unit Test, we needed to isolate that controller class of it’s collaborators. We’re not really going to execute the code that it’s calling, we just need to make sure that you only throw when needed. Sure, it’s a little bit of a stretch. They aren’t the most valuable Unit Tests, but if that code was done in a correct fashion, the tests would have been done before the code had even started, and it would have been a nice “oh wow, the code came together just as I’d hoped!” moment. It’s really just a safety net though, there to make sure I don’t come along later and break something without knowing it. The arguing went on for about ten minutes, which really surprised me considering how quickly she churns out code! But in the end, she realized the standard applies to everyone: everything checked in has to be covered in quality Unit Tests.
Then we came to the part of the 270-line untested method. At one point it had seven nested branches. She argued that Unit Testing was too difficult, and would take too much time. Again, she mentioned a “Functional Test” would be more valuable. I again argued that if a Test Driven Development methodology had been followed from the beginning, it would not only would have saved time but that massive method would have never happened in the first place. I mentioned the single-responsibly rule, about how this one class probably should have been three different ones, each with their own simple and few Unit Tests. That when we’re Unit Testing a method and not the class, we’re already doing too much. That Unit Tests only check that the code is right, and the Acceptance Tests ensure it’s the right code. It seemed to have worked, or so I thought. An hour later she comes back and asks if the Unit Tests can be in a separate code review, because adding two files would make the code review “too big.” Back to rule number one: no untested code is to ever be checked in.
What troubled me, is that I’ve had this same conversation before with new grads. They had never been exposed to this before, and didn’t understand the importance. I suppose everyone needs to have that moment where they break something which could have been easily caught by a test run at build time to really understand the importance. The new grads all learned, and I’m very proud to say they’re my biggest supporters on enforcing code quality. The other engineers of all levels equally feel this way about the importance. So I was really surprised when this senior well-seasoned engineer exerted so much energy to get out of doing the right thing.