Rules of Unit Testing

There is hopefully no need for me to discuss the benefits of unit testing but with more and more shops adopting agile, “scrum-but” or other TDD-like processes, people start to realise that it’s not all plain sailing. One of the major headaches related to unit testing is the cost of maintaining the tests themselves. Unless you get smart about your testing approach, growing number of unit tests may become a drag over time seriously increasing the cost of any change, becoming a hindrance rather than help.

In order to be successful with any form of TDD it’s worthwhile to consider some basic rules. With notable exceptions of posts by Szczepan Faber and Michael Feathers, Google renders rather poor results on the subject so I decided to give it another go. What you are (hopefully) about to read is the result of the learning I have received with other #Fellows in the “school of hard knocks”. We have learned by our, sometimes grave, mistakes so be smart and do not repeat them.

Rob C. Martin in his Clean Code mentions the following FIRST rules when it comes to unit testing. According to the rules unit tests should be:

  • Fast, as otherwise no-one will want to run them,
  • Independent as otherwise they will affect each other’s results
  • Repeatable in any environment
  • Self-validating (either failing or passing)
  • written in a Timely manner (together with the code being tested, not in some yet undetermined “future”)

The rules are by all means valuable but when it comes to practice of programming we need to get a bit more detailed. So let’s start with the basics

Design for testing

The prerequisite to successful unit tests is the “testability” of the code, and for the code to be testable you need to be able to inject it with mock implementation of dependencies. In practice it means that database, network, file, UI etc access has to be hidden behind some form of abstraction (interface). This gives us the possibility to use mocking framework to construct those dependencies and cheaply test various combinations of test inputs. The other obvious benefit is the fact that your tests become independent of difficult to maintain external “variables” as there is no need to deploy files, databases etc. When it comes to UI dependencies (using UI controls etc)  avoid them in your testable code as they may introduce thread affinity which cannot always be guaranteed by the environment/test runner.

The testable architecture naturally gravitates towards SOLID design principles as individual classes tend to have single responsibility and have every chance to remain “solid” regardless of the project timescales and turns in direction it may take.

Do not repeat yourself

The primary sin of unit testing is repetition and it is easy to explain it: you start with one test of your class Foo. Then you  add another test scenario, so now both of them will have to get an instance of Foo from somewhere and they will obviously “new()” the Foo. Then you will have more methods to tests and more lines of new Foo(). And one day you will need to add a parameter to Foo’s constructor and suddenly all those pesky tests will need to be reworked. You may argue that it should be easy enough to add an overload, but sometimes fabrication of objects gets more complicated: you need to use properties, call some methods on them etc. For this reason my advice is: Do not Repeat Yourself (stay DRY), use factory methods, builders etc in order to avoid the costs of refactoring which will inevitably get you one day. Funnily enough John Rayner of this shire produced recently a very nice automocking container which helps in instantiating types with large number of dependencies.

I find it quite interesting that developers are often willing to accept substantially lower coding standards when it comes to unit tests, often copy-pasting entire test cases only to change one line within it. Before you realise it, your test code base becomes a tangled, repeatable mess and introducing any change becomes a serious cost and challenge. If you ever come across a situation when you need to test similar cases over and over again, re-factor a common method, use a template method pattern or row based testing as implemented in NUnit etc but please, do not repeat yourself!

Keep the intent clear

How many times have you come across a test methods called “Test1”? Not a very helpful name, isn’t it? The purpose of the tests should be crystal clear as otherwise it may be very difficult to decipher over time the original intent. And if the intent needs to be ever deciphered, it adds to the cost of change. I have worked once on a project where the cost of refactoring tests was substantially higher than the cost of changing the code itself and other than violation of the DRY rule, the primary reason was the difficulty of “rebuilding” the tests so that they tested the original behaviour exactly as intended. To make the intent of the test clear I try to follow these rules:

  • Have a consistent naming convention for tests and test methods: say Ctor_WIthInvalidArgs_ThrowsException(). This naming convention outlines what is being tested, how it is being tested and what is expected. Or better yet…
  • Consider using NBehave to specify your expectations and assumptions. Side effect of using NBehave is the fact that your expectations and assumptions become crystal clear
  • Follow “triple A” structure for test methods (Arrange, Act Assert) and make it obvious in the test code what are you doing
  • Test one thing (method, property, behaviour) per test method
  • Add messages to asserts. Asserting that (a == b)  won’t tell you much when the expectation gets violated.

Do not share static state

In order to stick with the Isolation rule, it’s good practice to give up on shared static state as it is very easy to introduce frustrating dependencies between tests. Other than this, static variables are either difficult or impossible to mock, or lead to seriously hacked code (a singleton with a setter etc). For this reasons any statics, singletons etc have to be avoided for the sake of testability. If you think of this restriction as harsh, think again: there is always a way to replace your singleton with another pattern (say a factory returning a shared instance) which at the end of the day produces the same results, yet is far easier to test and more flexible in the long term.

Avoid mega-tests

I sometimes come across a phenomenon which I call mega-test: this usually involves running a process end-to-end and producing some output file (usually in 10s of MBs) which is then compared to a “master” file with the expectation that it has to be identical. This is probably the worst ever way to test software and the only circumstances where it could be acceptable practice is when refactoring a system which has no unit tests whatsoever. The problem with tests like this is the fact that when they fail, investigating the cause of failure is very expensive as you will have to trawl through tons of code to get to the bottom of the problem. Test failure should make you smarter and you should be able to say what went wrong pretty much immediately. Do not get me wrong, I am not saying here that you should not have “integration tests”. But the fact remains that majority of the testing work should be done through unit testing of individual components rather than slow and laborious “end to end” testing which often requires difficult preparation, setup and deployment.

Avoid strict mocks

This is another constant source of grief as strict mocks do not allow your code to deviate from pre determined expectations: so strict mock will accept the calls you set it to, but anything other than this will fail. If the code under test changes a little and does a bit less or more with the mock, the test will fail and you will need to revisit it adding extra expectations. Do not get me wrong here, strict mocks are valuable tool, but be very careful how you use them and ask yourself what exactly are you testing?

Maintain symmetry

Symmetry between code being tested and unit tests is all about 1:1 relationship between the code and it’s unit tests: so you have one test fixture per class, one test project per assembly etc. The reason for this symmetry is the fact that if you do change a class, there is (hopefully) only one test fixture which you need to execute to get a good idea if the code still works. This setup makes life easier as if you run your tests constantly you do not have to run all of them all the time. It also becomes easier to package code together with tests in cases you want to share it with someone.

Szczepan Faber mentions that testing class hierarchies is difficult and I agree with him, but my approach to resolving this problem is slightly different. If you maintain symmetry, every type (including base and derived types) will have a corresponding test fixture, which means that you will not get into a situation where you test the base through tests designed for the derived class. This is a rather uncomfortable as you will potentially have the same functionality tested multiple times, or worse yet, when the derived type gets scraped, your base class will be left “uncovered”. Maintaining the symmetry will help you alleviate this issue.

Maintaining the symmetry may be tricky in case of abstract types which cannot be instantiated, but in this case you can either derive a special “test only” type or use one of the already derived types hidden behind a factory of sorts.

Get clever

Sometimes in large projects it becomes necessary to follow certain convention in your unit tests, e.g. it sometimes makes sense to run the tests within transaction scope, or follow certain Setup/Teardown standards. In such cases you may want to derive all test fixtures from a common base, and to enforce the rule, write a test inspecting all test fixtures through reflection making sure that the test classes do indeed use common base class. I used this approach in the past to isolate all integration tests from each other through transaction scope.

Do not get too clever!

The most frustrating test failures are related to someone being unusually clever and writing a test which for example sets value held in a private field using reflection as the code cannot be tested otherwise. Then you happen to rename this field in good faith and everything builds, you do not see any reason to run the tests, check in your code and the integration build bombs out. Not a pretty sight but it’s an evil practice so please do not try to get overly smart with your testing.

June 24 2010

WPF Wait Indicator (aka Spinner)

UPDATE: New version of the spinner is available here:

Yet another leftover form Nym’s fancy downloader project is the “spinning doughnut of wait”. The downloader uses BackgroundWorker to get the initial list of videos available for download as doing so on the UI thread could cause the application to freeze for way too long. When we first put the BackgroundWorker in place, there was an “uneasy” period of time when the user would be staring at an empty screen with no visual clues as to what was going on. So I came up with a concept of a “spinner” show below (not very original, I know):

image 

The idea here is to display a spinning “doughnut” which gives the user a clue that something is indeed going on. As this is quite a common issue with background processing, I though that  wrapping the spinner in a reusable user control may be useful. The logic behind the control is simple: as soon as it becomes visible it starts spinning and stops when it’s being hidden. Parent control may then control visibility of the spinner (through data trigger etc) and that’s all that is required to use it. This behaviour is implemented through a bit of code behind as changes in visibility are advertised through IsVIsibleChanged event which is a standard non-routed .NET event. This sadly means that there is no way to react to it from the XAML event triggers and a piece of code was required. It is important to note here that it is necessary to stop the animation when it is no longer required. If you leave it running, it will keep spinning in the background, despite of the fact that it is not visible, consuming processor resources in the process. Get a couple of those running off screen and you will soon notice the difference in your computer’s performance.

Code Snippet
  1. public partial class Spinner : UserControl
  2. {
  3.     private Storyboard _storyboard;
  4.  
  5.     public Spinner()
  6.     {
  7.         InitializeComponent();
  8.  
  9.         this.IsVisibleChanged += OnVisibleChanged;
  10.     }
  11.  
  12.     private void OnVisibleChanged(object sender, DependencyPropertyChangedEventArgs e)
  13.     {
  14.         if ( IsVisible )
  15.         {
  16.             StartAnimation();
  17.         }
  18.         else
  19.         {
  20.             StopAnimation();
  21.         }
  22.     }
  23.  
  24.     private void StartAnimation()
  25.     {
  26.         _storyboard = (Storyboard)FindResource("canvasAnimation");
  27.         _storyboard.Begin(canvas, true);
  28.     }
  29.  
  30.     private void StopAnimation()
  31.     {
  32.         _storyboard.Remove(canvas);
  33.     }
  34. }

 

I have tested a couple of approaches to animate the spinner but settled on a version which simply rotates the canvas on which the rectangles are drawn. This means that I have just one thing to animate and when compared with phase-shift-animating opacities of sixteen individual rectangles it substantially easier :) It is also substantially cheaper (processor wise) when compared to animating opacities. The XAML of the spinner is shown below and I hope it is self explanatory. In order to achieve on/off effect for each rectangle I settled on key frame animation which is “jerky” by design, but it means that rectangles maintain their position but become lighter/darker as the doughnut spins thus creating the impression of them being momentarily turned on and then dimming slowly down. Think cheap 80’s disco effect :)

Code Snippet
  1. <UserControl x:Class="FancyPDCDownload2009.Modules.Main.Views.Spinner"
  2.    xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
  3.    xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml">
  4.   <UserControl.Resources>
  5.         <SolidColorBrush x:Key="SpinnerRectangleBrush" Color="Blue"/>
  6.         <Style TargetType="Rectangle">
  7.             <Setter Property="RadiusX" Value="5"/>
  8.             <Setter Property="RadiusY" Value="5"/>
  9.             <Setter Property="Width" Value="50"/>
  10.             <Setter Property="Height" Value="20"/>
  11.             <Setter Property="Fill" Value="{StaticResource SpinnerRectangleBrush}"/>
  12.             <Setter Property="Canvas.Left" Value="220"/>
  13.             <Setter Property="Canvas.Top" Value="140"/>
  14.             <Setter Property="Opacity" Value="0.1"/>
  15.         </Style>
  16.  
  17.         <Storyboard x:Key="canvasAnimation">
  18.             <DoubleAnimationUsingKeyFrames
  19.                RepeatBehavior="Forever"
  20.                SpeedRatio="16"
  21.                Storyboard.TargetProperty="RenderTransform.(RotateTransform.Angle)">
  22.                 <DiscreteDoubleKeyFrame KeyTime="0:0:1" Value="22.5"/>
  23.                 <DiscreteDoubleKeyFrame KeyTime="0:0:2" Value="45"/>
  24.                 <DiscreteDoubleKeyFrame KeyTime="0:0:3" Value="67.5"/>
  25.                 <DiscreteDoubleKeyFrame KeyTime="0:0:4" Value="90"/>
  26.                 <DiscreteDoubleKeyFrame KeyTime="0:0:5" Value="112.5"/>
  27.                 <DiscreteDoubleKeyFrame KeyTime="0:0:6" Value="135"/>
  28.                 <DiscreteDoubleKeyFrame KeyTime="0:0:7" Value="157.5"/>
  29.                 <DiscreteDoubleKeyFrame KeyTime="0:0:8" Value="180"/>
  30.                 <DiscreteDoubleKeyFrame KeyTime="0:0:9" Value="202.5"/>
  31.                 <DiscreteDoubleKeyFrame KeyTime="0:0:10" Value="225"/>
  32.                 <DiscreteDoubleKeyFrame KeyTime="0:0:11" Value="247.5"/>
  33.                 <DiscreteDoubleKeyFrame KeyTime="0:0:12" Value="270"/>
  34.                 <DiscreteDoubleKeyFrame KeyTime="0:0:13" Value="292.5"/>
  35.                 <DiscreteDoubleKeyFrame KeyTime="0:0:14" Value="315"/>
  36.                 <DiscreteDoubleKeyFrame KeyTime="0:0:15" Value="337.5"/>
  37.                 <DiscreteDoubleKeyFrame KeyTime="0:0:16" Value="360"/>
  38.             </DoubleAnimationUsingKeyFrames>
  39.         </Storyboard>
  40.     </UserControl.Resources>
  41.     <Grid>
  42.       <TextBlock
  43.          HorizontalAlignment="Center"
  44.          VerticalAlignment="Center"
  45.          Text="Loading..."/>
  46.         <Canvas Height="300" Width="300"
  47.                 Background="Transparent"
  48.                 Name="canvas">
  49.         <!-- First quadrant -->
  50.           <Rectangle Opacity="1"/>
  51.             <Rectangle>
  52.           <Rectangle.RenderTransform>
  53.             <RotateTransform Angle="22.5" CenterX="-70" CenterY="10"/>
  54.           </Rectangle.RenderTransform>
  55.         </Rectangle>
  56.         <Rectangle >
  57.           <Rectangle.RenderTransform>
  58.             <RotateTransform Angle="45" CenterX="-70" CenterY="10"/>
  59.           </Rectangle.RenderTransform>
  60.         </Rectangle>
  61.         <Rectangle >
  62.           <Rectangle.RenderTransform>
  63.             <RotateTransform Angle="67.5" CenterX="-70" CenterY="10"/>
  64.           </Rectangle.RenderTransform>
  65.         </Rectangle>
  66.        
  67.         <!-- Second quadrant -->
  68.         <Rectangle >
  69.           <Rectangle.RenderTransform>
  70.             <RotateTransform Angle="90" CenterX="-70" CenterY="10"/>
  71.           </Rectangle.RenderTransform>
  72.         </Rectangle>
  73.         <Rectangle >
  74.           <Rectangle.RenderTransform>
  75.             <RotateTransform Angle="112.5" CenterX="-70" CenterY="10"/>
  76.           </Rectangle.RenderTransform>
  77.         </Rectangle>
  78.         <Rectangle Name="r7">
  79.           <Rectangle.RenderTransform>
  80.             <RotateTransform Angle="135" CenterX="-70" CenterY="10"/>
  81.           </Rectangle.RenderTransform>
  82.         </Rectangle>
  83.         <Rectangle >
  84.           <Rectangle.RenderTransform>
  85.             <RotateTransform Angle="157.5" CenterX="-70" CenterY="10"/>
  86.           </Rectangle.RenderTransform>
  87.         </Rectangle>
  88.        
  89.         <!-- Third quadrant -->
  90.         <Rectangle Opacity="0.2">
  91.           <Rectangle.RenderTransform>
  92.             <RotateTransform Angle="180" CenterX="-70" CenterY="10"/>
  93.           </Rectangle.RenderTransform>
  94.         </Rectangle>
  95.             <Rectangle Opacity="0.3">
  96.           <Rectangle.RenderTransform>
  97.             <RotateTransform Angle="202.5" CenterX="-70" CenterY="10"/>
  98.           </Rectangle.RenderTransform>
  99.         </Rectangle>
  100.             <Rectangle Opacity="0.4" >
  101.           <Rectangle.RenderTransform>
  102.             <RotateTransform Angle="225" CenterX="-70" CenterY="10"/>
  103.           </Rectangle.RenderTransform>
  104.         </Rectangle>
  105.             <Rectangle Opacity="0.5">
  106.           <Rectangle.RenderTransform>
  107.             <RotateTransform Angle="247.5" CenterX="-70" CenterY="10"/>
  108.           </Rectangle.RenderTransform>
  109.         </Rectangle>
  110.        
  111.         <!-- Fourth quadrant -->
  112.             <Rectangle Opacity="0.6">
  113.           <Rectangle.RenderTransform>
  114.             <RotateTransform Angle="270" CenterX="-70" CenterY="10"/>
  115.           </Rectangle.RenderTransform>
  116.         </Rectangle>
  117.             <Rectangle Opacity="0.7">
  118.           <Rectangle.RenderTransform>
  119.             <RotateTransform Angle="292.5" CenterX="-70" CenterY="10"/>
  120.           </Rectangle.RenderTransform>
  121.         </Rectangle>
  122.             <Rectangle Opacity="0.8">
  123.           <Rectangle.RenderTransform>
  124.             <RotateTransform Angle="315" CenterX="-70" CenterY="10"/>
  125.           </Rectangle.RenderTransform>
  126.         </Rectangle>
  127.             <Rectangle Opacity="0.9">
  128.           <Rectangle.RenderTransform>
  129.             <RotateTransform Angle="337.5" CenterX="-70" CenterY="10"/>
  130.           </Rectangle.RenderTransform>
  131.         </Rectangle>
  132.         <Canvas.RenderTransform>
  133.             <RotateTransform Angle="0" CenterX="150" CenterY="150"/>
  134.         </Canvas.RenderTransform>
  135.                     </Canvas>
  136.     </Grid>
  137. </UserControl>
       

The source code for the downloader including the spinner can be downloaded from here.

March 30 2010

A pattern for “cancellable” background task

While working on Nym’s fancy downloader I stumbled upon a common problem: a task (the download to be precise) going on in a background which needs to be cancelled. Usually this sort of task is implemented by means of a loop executed on a background thread which periodically checks a “cancelled” flag. Exactly as illustrated below:

Code Snippet
  1. public void DoWork()
  2. {
  3.     while ( !Cancelled )
  4.     {
  5.         // Problem if DoSomeStuff() takes long time to complete...
  6.         DoSomeStuff();
  7.     }
  8. }

 

This is pretty much how the BackgroundWorker goes about it’s business. The only difference is that Cancelled and DoSomeStuf() are replaced by event’s. The major problem with this approach is the fact that when you want to cancel such a loop and wait for the thread to actually finish it’s job, you may be in for a very long wait. The method being executed on the background thread prevents it from checking Cancelled flag and thus terminating. In the context of the “fancy downloader” the problem was the method which actually reads the network stream and saves it to the file on your hard drive: if  it blocks due to poor network performance, busy site, etc the user would be facing a “hang”  in the app. The alternative solution is presented below:

Code Snippet
  1. protected virtual void CopyStream()
  2. {
  3.     var buffer = new byte[BlockSize];
  4.  
  5.     IAsyncResult result = Source.BeginRead(buffer, 0, BlockSize, null, null);
  6.  
  7.     while (WaitHandle.WaitAny(new[] { result.AsyncWaitHandle, _haltEvent }) == 0)
  8.     {
  9.         int bytesRead = Source.EndRead(result);
  10.  
  11.         if (bytesRead == 0)
  12.             return;
  13.  
  14.         Destination.Write(buffer, 0, bytesRead);
  15.  
  16.         result = Source.BeginRead(buffer, 0, BlockSize, null, null);
  17.     }
  18. }
  19.  
  20. public virtual void Cancel()
  21. {
  22.     _haltEvent.Set();
  23.     Thread.Join();
  24. }

In this case the loop simply continues as long as one of the two wait handles are signalled. If the first one gets signalled as the result of async I/O being completed, the program writes the data to the file. If the second handle gets signalled (due to Cancel method being called) the thread terminates. This allows for a Cancel() method to be pretty much instantaneous. The same approach can be used as well with any other operation but delegates may need to be used to execute method in question. What we also need to remember is the fact that the background operation does not simply “go away” because of our task being cancelled, so it would be wise to test the program and ensure that it does not crash because of some resources being disposed etc.

March 27 2010

A user’s story (and a short one at that)

I’ve been working professionally in software development for the last 15 yeas or so and a lot of this time has been spent “negotiating” with users scope and functionality of various software products. Being a developer I will always try to get the software designed in such a way that it is actually technically feasible, simple and (hopefully) cheap to develop. More often than not I have to push back on features which I feel do not provide much of a business value but are technically complex: think price/performance sort of analysis. For these and and many other reasons negotiating scope with users is sometimes tricky as both sides need to establish who is responsible for what: sort of “you tell me your problems, I will propose solutions” type of agreement. Sometimes however you will come across a user who most developers hate: the one who gives you solutions instead of problems, and poor solutions at that. It takes time and patience to explain to them why some of those solutions or features are not most desirable, etc. Such negotiations may be sometimes very testing, to say the least…

Anyway my last conversation with “the user” was going along those very lines: she was pushing for features which I felt were of little business value and she seemed to be very keen on getting them. When I pulled cheap “used-car-salesman-type-of-negotiating-trick” out of my sleeve (I simply went silent) she said something which changed my point of view completely: “you see we are always promised this, that and the other and then it takes months to deliver and it is still sh##t when we get it. Or things get postponed and they never arrive. So I try to put down as many requirement as I can, hoping that one day I may at least get some of them…”.

I have to admit here that I felt genuinely sorry for her. She had to endure years of using crap software, developed by internal IT with no hope of getting anything remotely better. And for the  first time in those 15 yeas I realised that we as developers can really affect other peoples lives: if we deliver poorly, their work will become miserable chore and quite possibly this misery may affect their personal lives. How long could you put up with a development environment which crashes, hangs and corrupts files all the time?

So the next time you write a piece of code think about people who will be using it. If we do our job well, then who knows they may actually like us… :)

March 23 2010
Newer Posts Older Posts