TDD can force the creation of "fake" dependencies

Question

I'm using a boilerplate implementation of Model-View-Presenter in an ASP.NET WebForms application. My View has two events of consequence, one that signals that the user has filled out enough fields on the domain model to initiate a duplication check, and the other is a regular Save event. My pseudo code looks like this:

public class ItemNewPresenter : PresenterBase<IItemNewView>
{
public IItemService Service { get; private set; }
public IItemNewView View { get; private set; }

public ItemNewPresenter(IItemService service, IItemNewView view)
{
    Service = service;
    View = view;
    View.OnSave += DoItemSave;
    View.OnItemIsDuplicateCheck+= DoItemIsDuplicateCheck;
}


private void DoItemIsDuplicateCheck(object sender, CheckItemDuplicateEventArgs e)
{
    CheckForItemDuplication(e.Item);
}

private void CheckForItemDuplication(Item item){

if (Service.IsDuplicateItem(item))
    {
        View.RedirectWithNotification(BuildItemUrl(item), "This item already exists");
    }
}
private void DoItemSave(object sender, SaveItemEventArgs e)
{
    DoItemIsDuplicateCheck(this, e.ToItemDuplicateEventArgs());
    Service.Save(e.Item);
}

}

Here's my test for ensuring that my presenter behaves properly when OnItemIsDuplicateCheck is raised from the view:

[Test]
public void presenter_checking_for_existing_item_should_call_redirect_if_found()
{
    var service = new Mock<IItemService>();
    var view = new Mock<IItemNewView>();
    var presenter = new ItemNewPresenter (service.Object, view.Object);

    var onCheckExistingHandler = view.CreateEventHandler <CheckItemDuplicateEventArgs>();
    view.Object.OnExistingDenominatorCheck += onCheckExistingHandler;
    var eventArgs = new CheckItemDuplicateEventArgs();

    service.Setup(s => s.IsDuplicate(It.Is<CheckItemDuplicateEventArgs>(c => c.Equals(eventArgs)))).Returns(true);

    onCheckExistingHandler.Raise(eventArgs);

    view.Verify(v => v.RedirectWithNotification(It.IsAny<String>(), It.IsAny<string>()), Times.Once());
    service.Verify();
}

For consistency, I would like to have the same duplicate check fired when the View raises the OnSave event. My question is around how I am supposed to write my test when one of the methods I want to verify (CheckForItemDuplication) is declared on the class under test. The alternative to verifying the method invocation on the SUT (bad) would be to write my save test with lots of duplicated code (setup and assertion of all my mocks would be copied from the above test) and it also makes the unit test less focused.

   [Test]
    public void presenter_saving_item_should_check_for_dupe_and_save_if_not_one()    {
         //duplicate mocks/setups/asserts from duplicate check fixture
         //additional mocks/setups/asserts to test save logic
    }

I think TDD would suggest pulling this private method out into a separate class that collaborates with my Presenter and would be injected via DI. But adding another dependency to my Presenter for functionality that doesn't seem worthy of being a freestanding abstraction *and*represents an internal implementation detail of my Presenter seems...well...crazy. Am I way off base here? There must be some design pattern or refactoring I can apply that would avoid the need to turn a private method into a dependency.

Answer 1

I would go with testing the class as is by adding the duplicate setup code. Once that test is passing and you are confident all test cases are covered you can then refactor your test code to remove duplication.

You can move the dependencies (service and view) to private fields, then add a method to create the SUT:

private Mock<IItemService> _service;
private Mock<IItemNewView> _view;

private PresenterBase<IItemNewView> CreateSUT()
{
    _service = new Mock<IItemService>();
    _view = new Mock<IItemNewView>();
    return new ItemNewPresenter (service.Object, view.Object);
}

(I think most people would prefer to initialize the Mock objects in the Setup method.)

Call the CreateSUT from your tests and now there is a little less duplication. Then you may want to add private method(s) for creating the event handler / raising the event as long as it is something that is being done the same or similar in more than one tests case.

Having this CreateSUT method cuts down on the amount of test code that is calling your constructor making it easier in the future if you were to add / remove / change dependencies. If you treat your test code like any other code and use the DRY principle when you see duplication it can result in more explicit, easier to read, maintainable test code. Dealing with very similar setup and test context is a common issue with unit testing and should not always change how the class being tested is/was designed.