The Inquisitive Coder – Davy Brion's Blog

I recently posted the first draft of the caching implementation of Agatha.  You might want to take another look at the piece of code that deals with processing requests in the Request Processor.  I thought it was alright.  And if only 10% of my readers actually read that code, then it means that about 200 people thought it was alright too because nobody mentioned a possible problem with that part.  Today i was adding some cached requests/responses to the RequestProcessor’s QuickNet exception handling test.  And lo and behold, 2 bugs showed up that i hadn’t anticipated.

Before i discuss the two bugs, i’d like to explain what the exception handling quicknet test does, and how it works.  It basically calls the request processing code a bunch of times, each time with a set of requests where either none, one or more of the request handlers will throw an exception during the handling of the requests.  The order of the exceptions or which handlers will throw an exception is completely randomized (thanks to QuickNet) and thus it varies from test to test.  This particular QuickNet test will perform 50 testruns, where each testrun consists of 50 transitions that will be executed.  Each transition is a piece of functionality that will be executed (in this case, the processing of requests) with randomized input (in this case, there is a fixed set of requests but which ones will fail will differ with each run of the transition) and after a transition is executed, the relevant specs are verified (in this case, a bunch of checks to make sure that the whole error handling functionality always does what it needs to do) to make sure that the piece of code that you’re testing (your transition, in this case the processing of requests) actually works.

Instead of testing with a mocked caching layer, i used the real thing in the tests.  I also added 2 cached request types to the test request types, and gave one of them an expiration of 1 second, the other an expiration of 2 seconds.  This means that during the entire QuickNet testrun, some requests will return cached responses, some won’t, some cached responses will expire and subsequent requests of that particular type need to be executed and have their responses cached again until they expire.   In the meantime, QuickNet is constantly firing requests at the RequestProcessor, and some of them will randomly fail.  Our specs need to verify that the returned responses always contain the correct exception information for the batch of requests that was processed.

And here’s the beauty of the whole thing.  I hadn’t even thought about how exception handling also influences how you deal with cached responses.  I didn’t take exception handling into account when adding the caching code, and without even adding specific tests to see whether the caching worked correctly, my QuickNet specs that intended to cover something that doesn’t really have anything to do with caching uncovered 2 issues with my caching code.   I was suddenly getting error messages from QuickNet that in some cases, the exception information wasn’t correct.   And it frequently took over 30 executions of the transition, or sometimes multiple testruns before some of those specs would fail.  The longer it takes for a bug to show up in a QuickNet test, the better the bug is at hiding and the harder it is to find it manually because it almost certainly is an edge-case that you hadn’t considered at all and that only occurs in certain situations.

And those are the type of bugs that you typically don’t find with traditional unit tests.  When you write traditional tests that are aimed at asserting correct behavior, you typically only write tests for the problems that you thought about.  Sometimes you’ll get lucky and traditional unit tests will inform you of incorrect behavior that you didn’t think of,  but it rarely happens for the edge-cases.  And those are the bugs that can be really hard and painful to find and fix.

By now you’re probably wondering what the two bugs are.  The first one is that if an exception occurs during the handling of a request which is eligible for caching, the response with the exception information would be stored in the cache, and the next time that that particular request would be processed while the cached response hadn’t expired yet, you’d simply get the original response with the exception information again instead of actually handling the request again (which might have been handled successfully this time).  The second bug was that if an earlier request in the batch of requests already failed, it could possibly return a valid response later on in the same batch for one of the cached requests if its response hadn’t expired yet.   Pretty stupid huh?  But they were there nevertheless, and sooner or later somebody would’ve tripped over it and it would’ve caused frustration for the user who would run into it, and pain for me because i’d have to go looking for it long after i’d written the code.

Again, i seriously doubt that traditional unit tests would’ve prevented the existence of these 2 problems since it never occurred to me while i was writing it.  The QuickNet tests did find it, even before i added specific tests for the caching functionality to them, which i found pretty impressive and a huge benefit to this sort of testing.  Granted, the learning curve for writing valuable QuickNet tests is steep (i’m not even halfway there IMO) and it takes a lot of effort when you’re starting out with it.  But if it helps me prevent the sort of bugs that are otherwise easily missed and could negatively influence the perception of my project, then i definitely consider it worth it.