{"id":1477,"date":"2011-02-21T12:22:49","date_gmt":"2011-02-21T02:22:49","guid":{"rendered":"http:\/\/www.somethinkodd.com\/oddthinking\/?p=1477"},"modified":"2011-02-21T12:22:49","modified_gmt":"2011-02-21T02:22:49","slug":"unit-tests-reconsidered","status":"publish","type":"post","link":"https:\/\/www.somethinkodd.com\/oddthinking\/2011\/02\/21\/unit-tests-reconsidered\/","title":{"rendered":"Unit-tests reconsidered&#8230;"},"content":{"rendered":"<p>Dear Youse Bastards,<\/p>\n<p>I <a href=\"http:\/\/www.somethinkodd.com\/oddthinking\/2011\/02\/09\/unit-tests-considered\/\">recently grumbled<\/a> about letting myself get stuck in a unit-testing quandary.<\/p>\n<p>You responded in the comments with a calvalcade of recommended best-practices, which I proceeded to both agree with in principle in many situations, but also stridently defend that I shouldn&#8217;t use them in my situation &#8211; the payback wasn&#8217;t warranted.<\/p>\n<p>I then proceeded to get back to work on changing a mid-level unit.  Suddenly, the Refactoring Fairy came and whispered in my ear a suggested way of abstracting away one part of the ugly, third-party I\/O layer.<\/p>\n<p>It meant abandoning some of the progress I had made, but it was The Right Thing&trade;, so I added a new class. Of course, I had your silly ideas still ringing in my head, so the new layer was written to support Dependency Injection (DI) to replace the layers below, and I also subclassed a Mock version so I could test its clients. When I unit-tested the new layer, I was careful not to over-test.<\/p>\n<p>Then I went back to the mid-level unit and refactored to use the new layer. The code became 43.0% more beautiful.<\/p>\n<div class=\"aside\">The old code had lots of Pylint (a Python style-checker) pragmas telling it to stop warning me about the code. &#8220;Dear Pylint, I know this function has too many parameters and the class has too many attributes, but I need them all to deal with the lower layers.&#8221; and &#8220;Dear Pylint, I know this class calls methods that don&#8217;t follow the coding standard, but that is because the third-party library doesn&#8217;t follow them.&#8221;  I was able to remove all those pragmas, which made the Refactoring Fairy very happy.<\/div>\n<p>While I was updating the mid-level unit&#8217;s unit-tests, I could use the new mock objects, and it went very smoothly.<\/p>\n<p>Now, I could work with the exactly same API from the high-level unit to the mid-level unit, but I could now simplify the handles to I\/O objects, so I went ahead and did that&#8230; and while I did, I added a few more interface features to allow mock objects to be more easily used.<\/p>\n<p>While changing the high-level unit &#8211; the one that starts &#8220;# TO DO: Refactor into smaller parts, that can be tested faster.&#8221; &#8211; to use the new mid-level API, I got the Refactoring Fairy to keep look-out to make sure no Project Manager was watching, and I hacked out the complete chunk that dealt with the lower-level unit, wrapped it in a pretty &#8211; and mockable &#8211; API, and tested it separately.<\/p>\n<p>Finally, I stitched together the wounds of the top-level object, and looked at its unit-tests&#8230; the horrible ones that had hard-dependencies on the lower-level objects and were the cause of the original complaint.  With your fancy-pants concepts at top of mind, I could now see several problems with them:<\/p>\n<ul>\n<li>Of course, they had horrible dependency problems, which is why I originally complained.<\/li>\n<li>They were slow (dealing with timeouts, and the like, meant they were filled with sleep statements), which triggered the To Do comment.<\/li>\n<li>They were over-testing &#8211; spending effort on checking that the sub-layers were working correctly rather than focussing on the current unit being examined.<\/li>\n<li>Finally, they also tested items that were no longer part of the unit, because they had been cut away.<\/li>\n<\/ul>\n<p>I substantially rewrote these tests, using the mock layers I now had waiting. The results were much shorter and much easier to understand. The number of test-cases requiring sleep statements was substantially reduced, and I also made the timeouts overrideable, so I could force timeouts by sleeping for 50 milliseconds rather than 5 seconds.<\/p>\n<p>So, to summarise.  Because youse bastards put your ideas about better testability in my head, and got both my internal Unit Test Advocate andthe Refactoring Fairy all excited, I went ahead and wasted several days on refactoring &#8211; removing over-testing, adding two more layers of abstraction, adding mocks to pretend to be those new abstraction layers. That was development time that I swore would not be worth it.<\/p>\n<h3>Benefits<\/h3>\n<p>The result is:<\/p>\n<ul>\n<li>cleaner code, on the whole. The increased complexity of DI was outweighed by the other simplifications.<\/li>\n<li>cleaner &#8211; and less fragile &#8211; tests. While the amount of code in each test-cases was generally much shorter, that was outweighed by the new Mock implementations. There is more test code now, but it is easier to understand.<\/li>\n<li>a unit-test suite that runs about 2 minutes faster. I run the suite at least half-a-dozen times a day &#8211; often more,  but I rarely sit around waiting for the result.<\/li>\n<\/ul>\n<h3>Limitations<\/h3>\n<h4>Improved Bug Detection<\/h4>\n<p>I would have liked to say &#8220;And I found 5 serious bugs in my original code I had never caught before!&#8221;<\/p>\n<p>However, that wasn&#8217;t the case. I only discovered one bug:<\/p>\n<p><code><\/p>\n<pre>if number_of_unprocessed_items > 0:\r\n    slowly_prepare_system_for_processing()\r\n    process_remaining_items()<\/pre>\n<pre><\/pre>\n<p><\/code><\/p>\n<p>The idea of the condition here was to avoid the slow system preparation if there was nothing to do.<\/p>\n<p>However, <code>number_of_unprocessed_items<\/code> was a function, not a number. I omitted the parenthesis. It should have been <code>number_of_unprocessed_items<em>()<\/em> > 0<\/code>. Python should know better than to define a greater-than operation on function-pointers. That is the kind of mistake C makes, not Python.<\/p>\n<p>The result was the slow system preparation was always run &#8211; a small unnecessary performance hit that my unit-tests could never detect&#8230; until I mocked out the <code>slowly_prepare_system_for_processing<\/code> and noticed it was being called more often than expected.<\/p>\n<h4>No Dependency Injection Framework<\/h4>\n<p>I haven&#8217;t taken Sunny&#8217;s suggestion and found a DI Framework for Python. I suspect the demand for such a beast is lower in a language like Python where classes can be passed as parameters. I did take a look at a major one, and particularly its advertised benefits. There was a large list, and all but one of them (testability) was of zero interest to me on this project.<\/p>\n<h4>Poorer Integration Tests<\/h4>\n<p>If Class A depends on B depends on C depends on D, then I am now testing:<\/p>\n<ul>\n<li>C works against mock D.<\/li>\n<li>B works against mock C.<\/li>\n<li>A works against mock B.<\/li>\n<\/ul>\n<p>That&#8217;s good unit-test style, right?<\/p>\n<p>Before I was testing:<\/p>\n<ul>\n<li>C works against mock D.<\/li>\n<li>B works against C running against mock D.<\/li>\n<li>A works against B running against C running against mock D.<\/li>\n<\/ul>\n<p>I was encountering problems that A&#8217;s tests were affected when C changed.<\/p>\n<p>But now I have opened myself to another risk. What if one of the many mocks doesn&#8217;t match the real classes very well. I am never performing integration tests that make sure my units fit together. Before, I was getting at least some reassurance that most of my code was working together.<\/p>\n<p>I guess I now need to write some systems tests that exercise the entire system (or at least, as much as I can manage without causing the transfer of real money) with an understanding that these tests will be pretty fragile.<\/p>\n<p>More bloody best practice that I would insist upon in any enterprise project, but have avoided on a single-developer, I-am-the-customer-so-no-acceptance-tests-required project.<\/p>\n<p>I blame you.<\/p>\n","protected":false},"excerpt":{"rendered":"<p>I blame you for the time I just spent refactoring my test code.<\/p>\n","protected":false},"author":1,"featured_media":0,"comment_status":"open","ping_status":"open","sticky":false,"template":"","format":"standard","meta":{"_s2mail":"yes","footnotes":""},"categories":[1],"tags":[],"class_list":["post-1477","post","type-post","status-publish","format-standard","hentry","category-uncategorized"],"_links":{"self":[{"href":"https:\/\/www.somethinkodd.com\/oddthinking\/wp-json\/wp\/v2\/posts\/1477","targetHints":{"allow":["GET"]}}],"collection":[{"href":"https:\/\/www.somethinkodd.com\/oddthinking\/wp-json\/wp\/v2\/posts"}],"about":[{"href":"https:\/\/www.somethinkodd.com\/oddthinking\/wp-json\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"https:\/\/www.somethinkodd.com\/oddthinking\/wp-json\/wp\/v2\/users\/1"}],"replies":[{"embeddable":true,"href":"https:\/\/www.somethinkodd.com\/oddthinking\/wp-json\/wp\/v2\/comments?post=1477"}],"version-history":[{"count":6,"href":"https:\/\/www.somethinkodd.com\/oddthinking\/wp-json\/wp\/v2\/posts\/1477\/revisions"}],"predecessor-version":[{"id":1483,"href":"https:\/\/www.somethinkodd.com\/oddthinking\/wp-json\/wp\/v2\/posts\/1477\/revisions\/1483"}],"wp:attachment":[{"href":"https:\/\/www.somethinkodd.com\/oddthinking\/wp-json\/wp\/v2\/media?parent=1477"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/www.somethinkodd.com\/oddthinking\/wp-json\/wp\/v2\/categories?post=1477"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/www.somethinkodd.com\/oddthinking\/wp-json\/wp\/v2\/tags?post=1477"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}