Did I ever post that unit testing is hard?
It's hard. I'd love to see it in action at other shops since we've got so much to learn. But let's tally the score of the moment:
It's about 10 minutes to midnight and I've been working my way through someone else's code. This code is a piece of a separate assembly that does validation of a whole bunch of business rules before a person can process a loan application. Each rule is fairly simple, along the lines of "if a person is not older than 18, they must have a cosigner" but there happen to be a lot of rules. I designed the rules to be in a separate assembly because the client changes these frequently, so it will be easy to update with a new DLL instead of reinstalling the application.
Our application is modeled as an object with a series of other objects and lists attached. An application has a borrower. A borrower has an employment history. And so it goes - which makes for a fluid, transactional save or validation by simply passing the application object around.
But I'm looking at a lot of validation at this late hour and finding a systemic issue. The person who wrote the code made an assumption with respect to Lists attached to objects that makes our validation code ineffectual. Here's a sample:
25 if (targetApplication.Coborrowers.Count > 0 && targetApplication.Coborrowers.Citizenship == String.Empty)
27 result.IsValid = false;
28 result.Message = "First Cosigner Citizenship is required for Approved Status.";
31 return result;
See the bug? If there are no coborrowers the rule (citizenship must be set) is never evaluated.
But even more so, here's the rub: I had the developer write unit tests for each and every validation rule they put out as a method. I thought a night like tonight: midnight, fatigue, and a crapload of code to fix for a promised build, would be avoided as a result.
The large picture here has to do with the way the unit test was designed to verify the functionality of the validation rule. Because the problem was approached as gingerly, I think the unit tests weren't liable to find a more subtle bug like this. Instead, the unit test probably attempted to validate a coborrower with a citizenship, asserting true, and a coborrower without citizenship, asserting false. No thought was given as to whether a coborrower may not exist.
Perhaps, in programming, we need to focus more on falsification in our unit tests - attempts to break the code or find ways it will assert what's expected under the wrong conditions. But that's all theory right now and I've got a decision to make: stay up late and try to keep my word or heading to bed and fixing the code under the right conditions... the purists will say that a rushed fix is bad, especially since I didn't write the first iteration and won't have time to update the unit tests (I know, I know: bad). But I have to live in a world of realisms where the pain of a broken promise is greater than the pain of a rushed, patchworked fix. Time to get the Mt. Dew out of the fridge...