exception expectation

In my last post I created a new BalancePresenter class with a constructor requiring two parameters: IBalanceView and IRepository.
(Catch up on this series of posts: one, two, three)

Now I want to ensure that at the point of entry I can identify if either of these parameters are null. In the absence of Code Contracts I'll have to roll my own code.
First I write the tests and here I come across a bone of contention for many when dealing with exceptions - what should the observation name be?

My context is "when_creating_a_balance_presenter" which is very clear, but the observation name could be either "should_throw_an_argument_null_exception_if_balance_view_is_null" or a more generic "should_fail_if_balance_view_is_null".

In my previous projects using traditional unit testing I have used the first - being very explicit about what the observation is testing, but this exposes the internal implementation.
Do we really need to describe the exception type we expect when this is already self-described by the test code within the observation? I've recently had a change of heart and now want my observations to be descriptive and accurate but not exhaustive - they should not deal with implementation details.

Here is how I first wrote the observation code (and please note that I normally red/green/refactor one observation at a time but to save space on this post I've done two at once below).

[Observations] 
public class when_incorrectly_creating_a_balance_presenter : BalancePresenterSpecification 
{ 
    private static IBalanceView balanceView; 
    private Exception exceptionCaughtWhileCreatingThePresenter; 

    private context c = () => balanceView = an<IBalanceView>(); 

    [Observation] 
    public void should_fail_if_balance_view_is_null() 
    { 
        try 
        { 
            BalancePresenter invalidPresenter = new BalancePresenter(null, 
                          new MemoryRepository()); 
        } 
        catch (Exception ex) 
        { 
            exceptionCaughtWhileCreatingThePresenter = ex; 
        } 

        Assert.IsInstanceOfType(typeof(ArgumentNullException),
                      exceptionCaughtWhileCreatingThePresenter); 
    } 

    [Observation] 
    public void should_fail_if_repository_is_null() 
    { 
        try 
        { 
            BalancePresenter invalidPresenter = new BalancePresenter(balanceView, null); 
        } 
        catch (Exception ex) 
        { 
            exceptionCaughtWhileCreatingThePresenter = ex; 
        } 

        Assert.IsInstanceOfType(typeof(ArgumentNullException), 
                      exceptionCaughtWhileCreatingThePresenter); 
    } 
} 

I then easily saw the repeated code pattern and refactored this:

[Observations] 
    public class when_incorrectly_creating_a_balance_presenter : BalancePresenterSpecification 
    { 
        private static IBalanceView balanceView; 

    private context c = () => balanceView = an<IBalanceView>(); 

        [Observation] 
        public void should_fail_if_balance_view_is_null() 
        { 
            AssertThatWeCatchAnArgumentNullExceptionWhenCreatingAPresenterWith(null,
                                 new MemoryRepository()); 
        } 

        [Observation] 
        public void should_fail_if_repository_is_null() 
        { 
            AssertThatWeCatchAnArgumentNullExceptionWhenCreatingAPresenterWith(balanceView, 
                                 null); 
        } 

        private void AssertThatWeCatchAnArgumentNullExceptionWhenCreatingAPresenterWith(
                                 IBalanceView view, IRepository repository) 
        { 
                  Exception exceptionCaughtWhileCreatingThePresenter;  

            try 
            { 
                BalancePresenter invalidPresenter = new BalancePresenter(view, repository); 
            } 
            catch (Exception ex) 
            { 
                exceptionCaughtWhileCreatingThePresenter = ex; 
            } 

            Assert.IsInstanceOfType(typeof(ArgumentNullException), 
                                 exceptionCaughtWhileCreatingThePresenter); 
        } 
}

This looks much better than the first attempt but there are a few things wrong with this that I didn't like. So digging into the bdd library innards I came up with the following much better syntax which I will illustrate further in a moment. But first, the implementation needs putting in (again, one test passing at a time, but here, the code is shown once):

public BalancePresenter(IBalanceView balanceView, IRepository repository) 
{ 
    if(balanceView==null || repository==null) 
        throw new ArgumentNullException(); 

    this.balanceView = balanceView; 
    this.repository = repository; 
}

I have a better way of checking for null arguments that I aim to present in a future post, but for the purpose of this illustration the above code will suffice.

Onto the better test syntax... again, after looking at the code on the base classes of the bdd library I cleaned my test code into the following:

[Observations] 
public class when_incorrectly_creating_a_balance_presenter : BalancePresenterSpecification 
{ 
    private static IBalanceView balanceView; 

    private context c = () => balanceView = an<IBalanceView>(); 

    private static BalancePresenter invalidBalancePresenter; 

    [Observation] 
    public void should_fail_if_balance_view_is_null() 
    { 
        doing( () => invalidBalancePresenter = new BalancePresenter(null, 
                                 new MemoryRepository()) ); 
        exception_thrown_by_the_sut.should_be_an_instance_of<ArgumentNullException>(); 
    } 

    [Observation] 
    public void should_fail_if_repository_is_null() 
    { 
        doing( () => invalidBalancePresenter = new BalancePresenter(balanceView, null) ); 
        exception_thrown_by_the_sut.should_be_an_instance_of<ArgumentNullException>(); 
    } 
}

Here I am still setting the same context up where a mock is generated for my IBalanceView instance but I no longer need to catch the exception myself. A public property on the base class in the bdd library called "exception_thrown_by_the_sut" exposes any exception that occurred when processing the an Action called "behaviour_performed_in_because":

static public Exception exception_thrown_by_the_sut 
{ 
    get { return exception_thrown_while_the_sut_performed_its_work ?? 
              (exception_thrown_while_the_sut_performed_its_work = 
                 get_exception_throw_by(behaviour_performed_in_because)); } 
} 

static Exception get_exception_throw_by(Action because_behaviour) 
{ 
    try 
    { 
        because_behaviour(); 
        return null; 
    } 
    catch (Exception e) 
    { 
        return e; 
    } 
} 

static public void doing(Action because_behaviour) 
{ 
    behaviour_performed_in_because = because_behaviour; 
}

The code above is from the bdd library and note that the doing() method stores the action that is sent in on the "behaviour_performed_in_because" member.

Therefore within my observation I can set the action using the doing() method and then assert that an exception is raised when the action is performed.
I'm also using better assertion syntax from the bdd library with the extension method "should_be_an_instance_of<T>()".



5 comments:

  1. Jay May 21, 2009 at 8:32 PM

    Thanks, this was a big help to me today. I hadn't seen the doing() Action before, and was trying to use exception_thrown_by_the_sut with the because delegate.

     
  2. Justin Davies May 21, 2009 at 10:42 PM

    Hi Jay, glad it was some help. I've actually moved the doing() out of the [Observation] because I like my Asserts to be the only thing in that method.
    Moving out to the because is perhaps a bit clearer that it is the "Act":
    because b = () => doing( () => doingsomething );
    However, it is a bit wierd that the because delegate replaces the because action. I'm still in two minds about it!

     
  3. Jay May 22, 2009 at 3:18 AM

    That's how I ended up doing it, too:
    because b = () => doing(...

    That way I can use the "it" delegate instead of the observation:

    it should_throw_an_argument_exception = () => exception_thrown_by_the_sut
    .should_be_an_instance_of<ArgumentException>();

     
  4. Justin Davies May 22, 2009 at 7:38 AM

    That's great - this library is just so expressive that your line reads very well.
    Funnily enough I've got a post coming up about the option of using the it delegate instead of observations.
    The only reason I don't as a rule is that my resharper test runner for mbunit doesn't pick them up and can't display them in the test run GUI. Being able to press Alt-T and get instant visual feedback for a test I've just written beats a slimmer syntax for me at the moment.
    But I DO love the slimmer it delegate... it just reads so well.
    I keep planning to try and write my own resharper test plug in that picks it all up... the time, the time!

     
  5. Jay May 26, 2009 at 4:16 AM

    I've had the same problem with the ReSharper runner, and it is disappointing. I've switched over to running exclusively from TestDriven.NET and generating reports with Gallio.

    There is a thread on the mbUnit project page about the test runner issue here, and they in turn submitted a bug to JetBrains, so maybe this will get resolved at some point.