Yet another simplification of Prism’s EventAggregator syntax part II–mocking extension methods

After my last post, I had quite a healthy discussion with Alex Hung (@alexhung) on you guessed it, testability and mocking. Alex’s observation was although my extension methods were easy to use, they were difficult to test. He had gone and implemented a similar user experience but through using a custom wrapper which added Publish and Subscribe methods but called to EA under the covers. His wrapper implements an interface that is easy to mock.

The question then was how can you mock the new extension methods? Before getting into the how lets first answer the question of why?

Here are unit tests that should illustrate what I want to do.

[TestFixture]
public class When_SelectOrder_is_invoked {
  [Test]
  public void then_controller_publishes_OrderSelected() 
  {
    //mock out the event aggregator;  
    var mock = new Mock<IEventAggregator>() 
    var controller = new OrderController(mock.Object);
    var order=new Order();
    controller.SelectOrder(order);
    mock.Verify(ea=>ea.Publish(order));
  }
}

[TestFixture]
public class When_OrderViewModel_is_created {
  [Test]
  public void then_subscribes_to_OrderSelected() {
    //mock out the event aggregator;  
    var mock = new Mock<IEventAggregator>() 
    var vm = new OrderViewModel(mock.Object);
    mock.Verify(ea=>ea.Subscribe(vm.OnOrderSelected));
  }
}

Looks simple enough right? I am creating a mock EventAggregator, passing it in and then verifying that the Publish and Subscribe methods are called. Both Alex and I originally though that would work. Winking smile Though Alex was a bit skeptical.

However the reality is that it cannot work. The reason is because I am creating a mock of IEventAggregator but the methods I want to mock are not on the event aggregator itself, they are extension methods! Yet another proof that what looks like it will work in the blackboard of your mind, often doesn’t hold up any water in the real world.

There is hope though. Extension methods CAN be mocked, but it takes a bit of refactoring. Daniel Cazzulino (@kzu) has a great post on this here. The approach I came up with is based on his. For my extension methods here are the steps I went through in that refactoring.

  1. Created an IEventAggregatorExtensionsProvider interface. This interface has all the methods from my EventAggregatorExtensions class however, the methods are all non-static (obviously) and I removed this from the first param. Other than that the signatures are the same.
  2. Created EventAggregatorExtensionsProvider which implements IEventAggregatorExtensionsProvider. I ripped the implementation of each method out of EventAggregatorExtenions and then moved it into this provider.
  3. Added a private static variable _Provider of type IEventAggregatorExtensionsProvider to EventAggregatorExtensions. The variable initializes itself to a new instance of EventAggregatorExtensionsProvider.
  4. Changed all public methods in EventAggregatorExtensions to delegate directly to the _provider instance.
  5. Added a static SetProvider method which accepts an IEventAggregatorExtensionsProvider which it then overrides _provider with.

Here’s what the resulting code now looks like (which has grown a bit from my first Codepaste): http://codepaste.net/woqq1d

With these changes I can now pass in a mock of IEventAggregatorExtensionsProvider to do exactly what I want. Now my unit tests read like this:

[TestFixture]
public class When_SelectOrder_is_invoked {
  [Test]
  public void then_controller_publishes_OrderSelected()
  {
    //mock out the provider
    var mock = new Mock<IEventAggregatorExtensionsProvider>();
    var ea = new EventAggregator();
    EventAggregatorExtensions.SetProvider(mock.Object);
    var controller = new OrderController(ea);
    var order = new Order();
    controller.SelectOrder(order);
    mock.Verify(p=>p.Publish(ea, order));
}

[TestFixture]
public class When_OrderViewModel_is_created {
  [Test]
  public void then_subscribes_to_OrderSelected()
  {
    //mock out the provider
    var mock = new Mock<IEventAggregatorExtensionsProvider>();
    var ea = new EventAggregator();
    EventAggregatorExtensions.SetProvider(mock.Object);
    var vm = new OrderViewModel(ea);
    mock.Verify(p=>p.Subscribe(ea, vm.OnOrderSelected));
    
  }
}

The main difference is that I am now creating a mock of the extensions provider rather than the event aggregator itself. That means that my mock is the thing that will get invoked by the classes under test.

Actually the EA instance I create in the unit tests is just a dummy and could even be null as extensions methods can be invoked on null instances. That’s because my classes under test are only accessing the extension methods. However I passed it in because it is entirely conceivable one might want to access the actual instance methods.

To be fair there is more ceremony here than the approach that Alex was suggesting as you have to mock the provider and pass in an event aggregator. However the difference I think is marginal, and it works nicely and is completely testable.

What do you think?

This entry was posted in EventAggregator, prism. Bookmark the permalink. Follow any comments here with the RSS feed for this post.
  • http://codebetter.com/members/gblock/default.aspx Glenn Block

    Dave

    Thanks. I think your points are completely valid. This is not optimal and I don’t mean to imply that it is. I think it does provide a workable solution. Actually my reason for pushing on it was to see how and what could be achieved.

    As I started down the mockability path, I realized there was value in just sharing the approach I went through to make it more testable so that others could benefit.

    Yes RX is awesome. A bunch of folks have done posts on using RX with EA, or even as an EA.

    Regards

  • Dave Stevens

    I concede that “stateful” does not really apply to this code as nothing is modified. It does depend on the state of the field containing the IEventAggregatorExtensionsProvider though. I’m personally not a fan of using extension methods unless they are self contained and deterministic (based purely on input) utility functions which have no side effects.

    You have successfully allowed code which uses these extension methods to be tested. I do very much like the pattern you’ve described here for when it is absolutely necessary. If it helps you be more productive then it probably helps other people be more productive as well too and this is all that matters.

    On an unrelated note, I just started playing around with Reactive Extensions and I can definitely see these working together.

  • http://codebetter.com/members/gblock/default.aspx Glenn Block

    Dave

    What do you mean by the extension methods depend on state? These don’t depend on any state within the extension method. They are not stateful.

  • Dave Stevens

    I know very little of EA and am simply commenting on the use of extension methods (and by extension, static methods) which depend on outside code and/or contain state. This is a common mistake and something I am forced to live with on a daily basis because my company’s internally developed uber-library makes similar use of side effecting, state-full static methods. These methods, similar to the problem you created for yourself, make life extremely difficult when attempting to do TDD.

    I think I’d completely agree with Richard, but when that’s not possible, Alex’s approach that you describe sounds very similar to what I would do.

    A little googling just now and I found this: http://www.chadmyers.com/blog/archive/2007/11/19/a-public-call-for-c-extension-method-best-practices.aspx.

  • http://codebetter.com/members/gblock/default.aspx Glenn Block

    Richard

    I am doing this as a consumer of the existing EA. Now you could do what Alex did and create an EA wrapper with it’s own interface and then register that in the container. Arugably that would make testing simpler. In this case I was trying to use the existing EA without introducing a new wrapper.

  • http://codebetter.com/members/gblock/default.aspx Glenn Block

    Dave these methods are performing syntactic sugar as they simply delegate to the instance they they extend and call existing methods. If you check the CodePaste there is almost zero logic in the extensions.

    You don’t “need’ to mock the extensions, but it makes it easier to test in this case because otherwise you need to mock 2 classes instead of one. ie you would need a mock event aggregator and a mock event. Wheras in this case I just mock the provider.

  • http://richarddingwall.name Richard Dingwall

    Appreciate what you’re trying to do Glenn, but why don’t you just fix the IEventAggregator directly?

  • Dave Stevens

    Is it just me or does the need to mock an extension method raise huge red flags? I think of extension methods as simply static utility classes with some syntactic sugar to allow for inferred typing. As such, I would argue that extension methods must be purely functional and execute deterministically based solely on their input.

    I’m a huge proponent of mocking and TDD, but I feel that if you’re attempting to mock an extension method than you’re doing something wrong.

  • Mark Green

    I’m with Jeroen. All that’s happened is that you’ve moved the complexity from the GetEvent call to the ViewModel, the extension methods and the controller, while gaining no additional functionality and no additional testability as compared to the original GetEvent syntax.

    GetEvent isn’t great but it’s not so complex that it needs an over-engineered “simplification”.

    Nice work on the testing of extension methods, though. I’ll take that from this post.

  • http://codebetter.com/members/gblock/default.aspx Glenn Block

    @JeroenH The observation that there is more code is fair, though the additional code is really a set of delegation calls and one additional public method.

    I am not sure your thought on using the real EA actually will work I started mentally down that path but here is the issue. Prism’s EA creates the event instance the first time you call GetEvent<> (i.e. it calls new). That means that in order to have the mock event, you also need a mock/fake event aggregator which returns the mock event that you are trying to test against.

    That sounded messy(ier) in my head. But I may have to give it a try and use it for a third post. <g>

  • JeroenH

    I really don’t see the point… This approach makes the code more complicated and doubles the size.

    I prefer your original code. It can be tested much simpler tested with a dummy event and the actual event aggregator.