Is this a poor design?

Question

I'm trying my hand at behavior driven development and I'm finding myself second guessing my design as I'm writing it. This is my first greenfield project and it may just be my lack of experience. Anyway, here's a simple spec for the class(s) I'm writing. It's written in NUnit in a BDD style instead of using a dedicated behavior driven framework. This is because the project targets .NET 2.0 and all of the BDD frameworks seem to have embraced .NET 3.5.

[TestFixture]
public class WhenUserAddsAccount
{
    private DynamicMock _mockMainView;
    private IMainView _mainView;

    private DynamicMock _mockAccountService;
    private IAccountService _accountService;

    private DynamicMock _mockAccount;
    private IAccount _account;

    [SetUp]
    public void Setup()
    {
        _mockMainView = new DynamicMock(typeof(IMainView));
        _mainView = (IMainView) _mockMainView.MockInstance;

        _mockAccountService = new DynamicMock(typeof(IAccountService));
        _accountService = (IAccountService) _mockAccountService.MockInstance;

        _mockAccount = new DynamicMock(typeof(IAccount));
        _account = (IAccount)_mockAccount.MockInstance;
    }

    [Test]
    public void ShouldCreateNewAccount()
    {
        _mockAccountService.ExpectAndReturn("Create", _account);
        MainPresenter mainPresenter = new MainPresenter(_mainView, _accountService);
        mainPresenter.AddAccount();
        _mockAccountService.Verify();
    }
}

None of the interfaces used by MainPresenter have any real implementations yet. AccountService will be responsible for creating new accounts. There can be multiple implementations of IAccount defined as separate plugins. At runtime, if there is more than one then the user will be prompted to choose which account type to create. Otherwise AccountService will simply create an account.

One of the things that has me uneasy is how many mocks are required just to write a single spec/test. Is this just a side effect of using BDD or am I going about this thing the wrong way?

[Update]

Here's the current implementation of MainPresenter.AddAccount

    public void AddAccount()
    {
        IAccount account;
        if (AccountService.AccountTypes.Count == 1)
        {
            account = AccountService.Create();
        }
        _view.Accounts.Add(account);
    }

Any tips, suggestions or alternatives welcome.

Answer 1

That seems like the correct number of mocks for a presenter with a service which is supposed to hand back an account.

This seems more like an acceptance test rather than a unit test, though - perhaps if you reduced your assertion complexity you would find a smaller set of concerns being mocked.

Answer 2

When doing top to down development it's quite common to find yourself using a lot of mocks. The pieces you need aren't there so naturally you need to mock them. With that said this does feel like an acceptance level test. In my experience BDD or Context/Specification starts to get a bit weird at the unit test level. At the unit test level I'd probably be doing something more along the lines of...

when_adding_an_account
   should_use_account_service_to_create_new_account
   should_update_screen_with_new_account_details

You may want to reconsider your usage of an interface for IAccount. I personally stick with keeping interfaces for services over domain entities. But that's more of a personal preference.

A few other small suggestions...

• You may want to consider using a Mocking framework such as Rhino Mocks (or Moq) which allow you to avoid using strings for your assertions.

  _mockAccountService.Expect(mock => mock.Create())
     .Return(_account);

• If you are doing BDD style one common pattern I've seen is using chained classes for test setup. In your example...

public class MainPresenterSpec 
{
    // Protected variables for Mocks 

    [SetUp]
    public void Setup()
    {
       // Setup Mocks
    }

}

[TestFixture]
public class WhenUserAddsAccount : MainPresenterSpec
{
    [Test]
    public void ShouldCreateNewAccount()
    {
    }
}

• Also I'd recommend changing your code to use a guard clause..

     public void AddAccount()
     {
        if (AccountService.AccountTypes.Count != 1)
        {
            // Do whatever you want here.  throw a message?
        return;
        }

    IAccount account = AccountService.Create();

        _view.Accounts.Add(account);
     }

Answer 3

You might want to use MockContainers in order to get rid of all the mock management, while creating the presenter. It simplifies unit tests a lot.

Answer 4

This is okay, but I would expect an IoC automocking container in there somewhere. The code hints at the test writer manually (explicitly) switching between mocked and real objects in tests which should not be the case because if we are talking about a unit test (with unit being just one class), it's simpler to just auto-mock all other classes and use the mocks.

What I'm trying to say is that if you have a test class that uses both mainView and mockMainView, you don't have a unit test in the strict sense of the word -- more like an integration test.

Answer 5

The test life support is a lot simpler if you use an auto mocking container such as RhinoAutoMocker (part of StructureMap) . You use the auto mocking container to create the class under test and ask it for the dependencies you need for the test(s). The container might need to inject 20 things in the constructor but if you only need to test one you only have to ask for that one.

using StructureMap.AutoMocking;

namespace Foo.Business.UnitTests
{
    public class MainPresenterTests
    {
     public class When_asked_to_add_an_account
     {
      private IAccountService _accountService;
      private IAccount _account;
      private MainPresenter _mainPresenter;

      [SetUp]
      public void BeforeEachTest()
      {
       var mocker = new RhinoAutoMocker<MainPresenter>();
       _mainPresenter = mocker.ClassUnderTest;
       _accountService = mocker.Get<IAccountService>();
       _account = MockRepository.GenerateStub<IAccount>();
      }

      [TearDown]
      public void AfterEachTest()
      {
       _accountService.VerifyAllExpectations();
      }

      [Test]
      public void Should_use_the_AccountService_to_create_an_account()
      {
       _accountService.Expect(x => x.Create()).Return(_account);
       _mainPresenter.AddAccount();
      }
     }
    }
}

Structurally I prefer to use underscores between words instead of RunningThemAllTogether as I find it easier to scan. I also create an outer class named for the class under test and multiple inner classes named for the method under test. The test methods then allow you to specify the behaviors of the method under test. When run in NUnit this gives you a context like:

Foo.Business.UnitTests.MainPresenterTest
  When_asked_to_add_an_account
    Should_use_the_AccountService_to_create_an_account
    Should_add_the_Account_to_the_View

Answer 6

It is my opinion that if you find yourself needing mocks, your design is incorrect.

Components should be layered. You build and test components A in isolation. Then you build and test B+A. Once happy, you build layer C and test C+B+A.

In your case you shouldn't need a "_mockAccountService". If your real AccountService has been tested, then just use it. That way you know any bugs are in MainPresentor and not in the mock itself.

If your real AccountService hasn't been tested, stop. Go back and do what you need to ensure it is working correctly. Get it to the point where you can really depend on it, then you won't need the mock.

Answer 7

Yes, your design is flawed. You are using mocks :)

More seriously, I agree with the previous poster who suggests your design should be layered, so that each layer can be tested separately. I think it is wrong in principle that testing code should alter the actual production code -- unless this can be done automatically and transparently the way code can be compiled for debug or release.

It's like the Heisenberg uncertainty principle - once you have the mocks in there, your code is so altered it becomes a maintenance headache and the mocks themselves have the potential to introduce or mask bugs.

If you have clean interfaces, I have no quarrel with implementing a simple interface that simulates (or mocks) an unimplemented interface to another module. This simulation could be used in the same way mocking is, for unit testing etc.