Should we enforce constructor args?!

 Context

Say we have a Car object:

public class Car
{
    protected Engine engine;
    protected Door[] doors;
    protected Wheel[] wheels;

    public Car(Engine engine, Door[] doors, Wheel[] wheels)
    {
        Check.PreCondition(engine != null, "engine must not be null");
        Check.PreCondition(doors != null, "doors must not be null");
        Check.PreCondition(wheels != null, "wheels must not be null");

        this.engine = engine;
        this.doors = doors;
        this.wheels = wheels;
    }

    public void EnableCentralLocking()
    {
        doors.ForEach(door => door.Lock());
    }

    public void Start()
    {
        engine.TurnOver();
    }
}

The code above is used to demonstrate a context wereby we have an object that takes a number of other collabarators which are then delegated to when certain behaviour is required this is a common event in OO, You can see I use a custom assertion class to make sure the collabarators are not null.

One thing we pick up on is that different behaviours that the Car object is instructed to perform may effect only one collabarator or may effect them all for instance EnableCentralLocking effects the doors collabarating object but has no effect on the wheels or engine.

Unit testing the above

Ok so now we want to unit test the Car object:

[TestFixture]
public class when_central_locking_enabled
{
    [Test]
    public void should_tell_all_doors_to_lock()
    {
        var engine = new Engine();
        var wheels = new Wheel[4];
        var doors = createStubDoors();

        var sut = new Car(engine, doors, wheels);
        sut.EnableCentralLocking();
        doors.All(door => door.Locked);
    }
}

Here I use a state based way of testing the doors have all been locked. One thing is apparent in order to create the Car due to the assertion checks at the beginning I have to create all the collabarators now if I was testing a behaviour that required all of them this is a must however to test the central locking the doors is the only collabarator I need, the creation of the others is just a wasted overhead.

Now this is how I currently create my objects that require collabarators and I was ok with this however recently I was told by a friend to check out a blog by Misko Hevery I was astonished by how well written and the information surrounding good OO and unit testing was, anyhow I came across this post which made me think about whether we should be asserting the collabarators in the constructor or not.

If we remove the assertions our test becomes:

[TestFixture]
public class when_central_locking_enabled
{
    [Test]
    public void should_tell_all_doors_to_lock()
    {
        var doors = createStubDoors();
        var sut = new Car(null, doors, null);
        sut.EnableCentralLocking();
        doors.All(door => door.Locked);
    }
}

hmmm...
We removed the overhead of creating the collabarators not needed for the test however our code is open to NullReferenceExceptions being thrown if were passed a null collabarator, hmmm… a real head scratcher! 

What are the options?

  1. Make the responsibility of making sure objects are created be assigned to a factory object or on an IoC container
  2. Initialize the fields with NullObject values, and use a property to access the field so checks can be made (this would not be enforcable)
  3. Keep the assertions but allow your assertion library to be context aware, at runtime in your unit tests allow it to be in a fall through mode
  4. Keep the assertions and accept the overhead, use Test Data Builder pattern / Object Mother to cut down on repeated code
  5. Move the assertions into the methods, this could cause a lot of DRY violation and extra overhead.

As with most of these things it’s a balance between acheiving 100% defence and overhead required in our unit tests, after looking at the options I could come up with I’m inclined to favour 1 or 3.

What would you use? Do you have any different options about how to balance the 2 out?

Advertisements

Obstacles of unit testing & methods to side step them – Part 2: multi-threading

Introduction

Unit testing is good. Sure it’s no silver bullet but there is no such thing! Looking back I’m not sure how I managed to work without writing unit tests the number of time having tests has saved me from tracking down bugs isn’t worth thinking about :)

However everything isn’t all rosy once you start unit testing against your code, there are a number of items that can put a spanner in the works, and that’s why I have created this series of posts to try and help other people resolve some of these common issues you may find yourself up against.

  • Date & Time
  • Multi-threading
  • External Dependencies
  • Sharing Static items
Prerequisites
  • You have already been writing unit tests
  • You are familiar with state based testing and behaviour based testing & when best to use either
  • Are familiar with a mocking framework (these examples use Rhino Mocks).
Multi-threading

Let’s face it once we introduce multi-threading things usually get more complicated and trying to unit test multi-threaded code is no exception! Why? Well if we think about how our code is going to be executed it will be ran on the test runner thread by NUnit, MBUnit MSTest etc… And if we have code which is going to perform work on a separate thread this causes a problem because the test runner thread will not know that it should really be waiting for work on the other thread to finish before carrying on, let’s get into some code:

   1: [TestFixture]

   2: public class when_adding_two_ints_together

   3: {

   4:     [Test]

   5:     public void should_give_us_the_correct_result()

   6:     {
   8:         var processor = new Processor();

   7:         var sut = new AddTwoIntsWorkItem(1, 2);

   9:         processor.DoWork(sut);

  10:  

  11:         Assert.That(sut.Result, Iz.EqualTo(3));

  12:     }

  13: }

So for the example I created a simple Processor object and as simple command based interface and a concrete object:

   1: public class Processor

   2: {

   3:     public void DoWork(IWorkItem workItem)

   4:     {

   5:         ThreadPool.QueueUserWorkItem((state) =>

   6:             {

   7:                 workItem.Work();

   8:                 Thread.Sleep(2000);

   9:             });

  10:     }

  11: }

  12:  

  13: public interface IWorkItem

  14: {

  15:     void Work();

  16: }

  17:  

  18: public class AddTwoIntsWorkItem : IWorkItem

  19: {

  20:     protected int first;

  21:     protected int second;

  22:     protected int result;        

  23:  

  24:     public AddTwoIntsWorkItem(int first, int second)

  25:     {

  26:         this.first = first;

  27:         this.second = second;

  28:     }

  29:  

  30:     public void Work()

  31:     {

  32:         result = first + second;

  33:     }

  34:  

  35:     public int Result

  36:     {

  37:         get { return result; }

  38:     }

  39: }

You could imagine that this could represent something more real such as a scheduling object that hold a list of IWorkItem’s and can make calls to them in a async manner, so now we have everything setup we can run the unit test:

image

Hmm… were expecting 3 but getting 0 strange, well not really let’s try to breakdown what’s happening:

image

So our assertion Assert.That(sut.Result, Iz.EqualTo(3)); is being called before the work is done hence we get zero.

So how can we tell the test runner thread to wait till the work is done? Well there’s thread.Join() however we A don’t want to expose the thread object outside of the Processor object as this breaks encapsulation and B In this case we use the threadpool so don’t actually have a new thread created.

With thread.Join() out of the question we need to look for an alternative, luckily the Threading namespace gives a good way of handling this scenario via the AutoResetEvent/ManualResetEvent (the difference being that AutoResetEvent will set it’s state to false after being signalled automatically) that’s great we now have a mechanism for telling the thread to wait however how do we know when to signal the event and allow the test runner thread to carry on? at the moment we have no way of knowing when the Processor has processed the IWorkItem.

I don’t think it’s unreasonable to provide someway of knowing when the DoWork is complete, we can provide a WorkItemComplete event that can be subscribed to:

   1: public class Processor

   2: {

   3:     public void DoWork(IWorkItem workItem)

   4:     {

   5:         ThreadPool.QueueUserWorkItem((state) =>

   6:             {

   7:                 workItem.Work();

   8:                 Thread.Sleep(2000);

   9:                 RaiseWorkComplete();

  10:             });

  11:     }

  12:  

  13:     protected void RaiseWorkComplete()

  14:     {

  15:         if (WorkComplete != null)

  16:             WorkComplete(this, EventArgs.Empty);

  17:     }

  18:  

  19:     public EventHandler WorkComplete;

  20: }

I have introduced the event the next step is to amend our unit test to take advantage of this event and to synchronize the test runner thread with the threadpool thread doing the work:

   1: [Test]

   2: public void should_give_us_the_correct_result()

   3: {

   4:     // this is used to synchronize between this current thread &

   5:     // the thread that performs the work in Processor

   6:     var evt = new ManualResetEvent(false); 

   7:     var sut = new AddTwoIntsWorkItem(1, 2);

   8:     var processor = new Processor();

   9:     

  10:     processor.WorkComplete += (sender, e) => evt.Set(); // signal that work is done

  11:     processor.DoWork(sut);

  12:     // tells current thread to wait till work has been done in Processor

  13:     evt.WaitOne(3000); 

  14:  

  15:     Assert.That(sut.Result, Iz.EqualTo(3));

  16: }

It shouldn’t be too hard to see what’s going on here, I create a new ManualResetEvent and set it’s state to unsignalled, I subscribe to the WorkComplete event via a lambda which simply sets the ManualResetEvent to signalled, after the call the DoWork is made we wait for the ManualResetEvent to be signalled. So let’s give the unit test another go:

image 

Great! that worked a treat one thing to bear in mind is when waiting for the event to be signalled is what timeout to use, in the case above I used 3000 milliseconds you typically want to use something around this figure and should be used as a failover in case the event/method that your expecting to be called isn’t and you don’t get to signal the event, this will prevent the test runner thread from getting stuck.

You should now be more comfortable when having to unit test code that performs work in a separate thread, if you don’t want to expose an event to know when a particular task is finished you could use subclass and override to only provide the event on the test subclass object.

Obstacles of unit testing & methods to side step them – Part 1: date and time

Introduction

Unit testing is good. Sure it’s no silver bullet but there is no such thing! Looking back I’m not sure how I managed to work without writing unit tests the number of time having tests has saved me from tracking down bugs isn’t worth thinking about 🙂

However everything isn’t all rosy once you start unit testing against your code, there are a number of items that can put a spanner in the works, and that’s why I have created this series of posts to try and help other people resolve some of these common issues you may find yourself up against.

  • Date & Time
  • Multi-threading
  • External Dependencies
  • Sharing Static items

Prerequisites

  • You have already been writing unit tests
  • You are familar with state based testing and behaviour based testing & when best to use either
  • Are familar with a mocking framework (these examples use Rhino Mocks).

Date & Time

This first post covers testing against date and time, it is quite common to want to use the current date/time in your objects, say for example you have an audit object that keeps track of any changes made to a user of a system by an admin, it’s not unreasonable to expect a method along the way performing something like this:

public void PasswordChanged(string newPassword, IAdministrator changedBy)
{
    //.. set audit values
    passwordAudit.ChangedAt = DateTime.Now;
    auditService.Save(passwordAudit);
}

Now we have a problem how can we test that the audit service object got called with the correct values? Lets write a test for this:

[TestFixture]
public class when_changing_password_for_user : ContextSpecification
{
 //.. [snip]
    [Test]
    public void should_record_correct_values_against_audit_record()
    {
        mockAuditService.Expect(x => x.Save(null), o => o.IgnoreArguments())
                .Do(new Action(delegate(IAuditRecord audit))
                {
                    changedby.Username.ShouldEqual(audit.ChangedbyUsername);
                    audit.ChangedAt.ShouldEqual(DateTime.Now); // this aint gonna work!
                }
    }

    IPasswordAudit sut = createSUT();
    sut.PasswordChanged(newPwd, changedBy);
    }
 //..[snip]
}

I provided the textfixture to give some context to the testing taking place, this is the style I choose to write my tests it is based around BDD (Behaviour Driven Design).

As we can see audit.ChangedAt.ShouldEqual(DateTime.Now)); this bit of code is not going to do the trick, given that stopping time is out of the question, we have some other options a simpler way and one that works in this scenario is to push the date & time into the method call:

public void PasswordChanged(string newPassword, DateTime changedAt, IAdministrator changedBy)
{
    //.. set audit values
    passwordAudit.ChangedAt = changedAt;
    auditService.Save(passwordAudit);
}

We could now update the unit test to take this into account and check the DateTime values match (i’m not going to show this), there’s a lot to be said about KISS however sometimes this push approach can simple not be applied.

Take for example a scheduler object that is tracking the current date & time the push approach is unworkable, we need to some way to provide the scheduler object with a DateTime value but for it to be under our control.

There are 2 different styles that I know of to get date and time under our control, the first is by using an abstraction type that provides us with the current date and time:

public interface ICurrentDateTimeProvider
{
    DateTime Current {get;}
}

We can then use in our class that needs to access the current date and time, this would be a good argument for using property injection as it’s only a seam for testing purposes:

public class Scheduler
{
    protected ICurrentDateTimeProvider currentDateTimeProvider = new RealDateTimeProvider();

    public ICurrentDateTimeProvider DateTimeProvider
    {
        set {currentDateTimeProvider = value;}
    }
}

RealDateTimeProvider simply returns DateTime.Now.

We can now use this abstraction to control date and time:

[TextFixture]
public void when_scheduler_is_started : ContextSpecification
{
    [Test]
    public void given_schedule_item_next_run_is_in_the_past_should_execute_it()
    {
        currentDateTimeProvider.Stub(x => x.Current).Return(scheduleItemNextRun.AddMinutes(-1));
        var sut = createSUT();
        sut.CurrentDateTimeProvider = currentDateTimeProvider;
        sut.Start();
        //...[SNIP]
    }
}

I have kept the code to a minimum to show the important part where we stub out the ICurrentDateTimeProvider and inject it via the property on Scheduler.

The problem with this approach is that it’s a little long-winded, we have to introduce a new interface, create the real implementation and also find a way to inject the dependency into the class were testing against.

There is a really nice way I came across recently that gets round these issues it was while reading ayende’s blog, the code needed is incredibly small but gives us a lot:

public static class SystemTime
{
    public static Func Now = () => DateTime.Now;
}

This gives us one place to go to for date and time so no injection needed, it also provides us with the real date and time and allows us to control what should be returned. Code wanting to get the current date and time would use this:

var currentDateTime = SystemTime.Now();

Code wanting to change what is returned i.e. out test code would do so like this:

SystemTime.Now = () => new DateTime(2009,1,1);

Very neat and requires less work than the former way. In the next post we will look out dealing with testing multi-threading code.

Registering a mock/stub with Windsor

I have some code that uses Castle Windsor to retrieve instances this helps to manage dependencies that I don’t have to worry about, the side effect of this is that when performing unit tests against the code that needs to get an instance from Windsor I want to provide a way to inject a mock/stub object is can be accomplished with the following:

container.Register(Component.For()
                                        .Instance(mockFoo));

To help reduce the amount of code in my test setups I usually have this method in my Unit Testing base class

protected void RegisterDependencyAgainst(TInterface instance)
{
    container.Register(Component.For()
                                .Instance(instance));
}

Which is then called like this:

// in setup method
mockFoo = MockRepository.CreateMock();
RegisterDependencyAgainst(mockFoo);

Enforcing Conventions

In my last post I demonstrated adding AOP to cut down on cross cutting code, and at the end mentioned that it would be nice to enforce a convention throughout the system, the example being each public method in the task layer being decorated with a certain attribute.

I was unsure about how to do this until recently seeing a post by Ayende http://www.ayende.com/Blog/archive/2008/05/05/Actively-enforce-your-conventions.aspx in it he references an article by Glenn Block whereby both of them came up with a unit test called PrismShouldNotReferenceUnity in the test they use reflection to check that there are indeed no references from the Prism assembly to the Unity assembly.

This is a great idea! You now have a repeatable test that can be run to make sure the conventions for your system are met, so armed with this technique I created the following test:

[Test]
public void task_class_methods_should_be_marked_with_wrap_exception_attributes()
{
    try
    {
        Assembly asm = Assembly.LoadFrom( "MCromwell.StaffIntranet.Task.dll" );
        var wrapExceptionType = asm.GetType( "MCromwell.StaffIntranet.Task.Infrastructure.WrapExceptionWithAttribute" );
        Assert.IsNotNull(wrapExceptionType);

        foreach (Type current in asm.GetTypes())
        {
            if (current.FullName.StartsWith( "MCromwell.StaffIntranet.Task.Tasks." ) && (!current.IsInterface) && (!current.IsAbstract))
            {
                foreach (var method in current.GetMethods())
                {
                    if ((method.IsPublic) && (method.DeclaringType.Name != "Object"))
                    {
                        if (method.GetCustomAttributes(wrapExceptionType, false).Length <= 0)
                            Assert.Fail("no wrap exception attribute found on type '{0}', method '{1}'", current.FullName,method.Name);
                    }
                }
             }
        }
    }
    catch (ReflectionTypeLoadException rex)
    {
        foreach (var current in rex.LoaderExceptions)
            Console.WriteLine(current.ToString());
        throw;
    }
    catch (Exception ex)
    {
        Console.Error.Write(ex.Message);
        throw;
    }
}

In here you can see by leveraging reflection I can browse all the public methods for my task layer classes and make sure they do indeed have a WrapExceptionWithAttribute the other cool thing is that by doing it this way I can freely add new classes and they will need to comply with the conventions set out or the testing will fail, cool eh!

One thing to point out is that if you start increasing the number of conventions and want a better way to control and report on, you probably want to look into something like FXCop or NDepend’s CQL.

Staff Intranet on Codeplex

I have not posted here for a little the main reason being I have been putting the final touches to another project I have been working on in my spare time, it’s a staff intranet application what I have tried to do with this project as with the previous projects is try and use it as a learning exercise, so I have incorporated some best practices and design patterns and used them for a real world example, in there you will find:

  • Inversion of control – This has been decoupled from a particular IoC container but I have utilised Castle Windsor under the hood
  • TDD/BDD – Around 98% of the behaviour of the application was designed test first, the reason I have put BDD as well is because I like to think of my tests as defining the behaviour of the application rather than of just testing assertions in my code, this comes out through the naming of my test cases
  • NHibernate – This was my first use of NHibernate and overall I was very pleased with what this very powerful ORM gives you and also keeping the domain clean from database artifacts (persistence ignorance)
  • Model View Presenter – This is the first time I have emplyed model view presenter for web and it acts as a nice interim between moving logic from the codebehind and going the whole hog and using an MVC framework such as monorail or ASP.NET MVC, I’m completely sold on having tests against the UI logic
  • SQL Server 2005 – This was probably the most dissapointing aspect of developing this application, I was hoping that the new client tools would be great and easy to get stuff done with however I found it the opposite things that used to be easy using the SQL server 2000 enterprise manager were not intuituve at all with the Management Studio, examples:
    • Wanting to remove a database that already exists
    • Setting permissions from the users perspective

Anyway after that intro you can find more details over at my project homepage on codeplex.

My hope is that this project will help others who are learning about the above but would like to see them used in a real world context rather than just in hello worls context.

First codeproject article posted

I just finished posting my first article to codeproject (only taken 3 years since I joined!), it covers unit testing against a database and also provides a handy little testing library that can be used to make it a little easier you can check it out at http://www.codeproject.com/KB/cs/unittestingdblib.aspx