Why ‘Should Attach View To Presenter’ Is An Invalid Unit Test / Observation.
I’ve written a lot of specification tests like this in the last three years, from a UI / Workflow perspective, with Model-View-Presenter as my core UI architecture:
[TestFixture]
public class When_starting_some_process()
{
IMyView view;
MyPresenter presenter;
[Setup]
public void Setup()
{
//...setup code and execute stuff for the test here
view = MockRepository.GenerateMock<IMyView>();
presenter = new MyPresenter(view);
presenter.StartSomeProcess();
}
[Test]
public void Should_attach_the_view_to_the_presenter()
{
presenter.View.ShouldNotBeNull();
}
[Test]
public void Should_show_something()
{
view.AssertWasCalled(v => v.ShowSomething(something));
}
}</p>
The Devil In These Details
I had one of those ‘aha!’ moments yesterday where several of my nagging suspicions and annoyances at writing specification tests like this example finally gelled into a coherent understanding. That understanding is easily stated by saying that the test, “Should_attach_the_view_to_the_presenter” is invalid and should never be written. There are a number of reasons for this.
- In practicing BDD, the technical jargon that is leaking into the test is irrelevant to the real value that is intended with this specification
- In practicing any form of TDD, the implementation detail of attaching a view to a presenter creates a brittle test – I care about the API and how the system really works, not a very technical, implementation detail like this.
- Exposing the “View” property of the Presenter object is a violation of encapsulation and an ‘over-intimate’ smell. My test has far too much fine detail, granular knowledge of what’s going on behind the scenes, to really be of any practical value
- Finally – and possibly outweighing all other reasons combined – it’s simply not necessary.
Look at the second test: “Should_show_something”. It’s actually using the view. Presumably, the “StartSomeProcess” method on the presenter is going to call a method on the view – that’s why we are asserting that the method on the view was called. If this is true, then it can be safely assumed that not having a view attached to the presenter would throw a null reference exception when trying to call that method on the view.
If not having the view attached to the presenter results in a null reference exception for the other test, then we have a valid reason for that test to fail. We certainly can’t say that the view’s method was called when the view is null. Therefore, testing to ensure that we have a view attached to the presenter is a duplication of effort. We’re only proving what we have already proved transitively, via another test.
Beauty And Meaning In Simplicity
Assuming that this is all true, we can remove that test entirely. Our specification now looks like this:
[TestFixture]
public class When_starting_some_process()
{
IMyView view;
MyPresenter presenter;
[Setup]
public void Setup()
{
//...setup code and execute stuff for the test here
view = MockRepository.GenerateMock<IMyView>();
presenter = new MyPresenter(view);
presenter.StartSomeProcess();
}
[Test]
public void Should_show_something()
{
view.AssertWasCalled(v => v.ShowSomething(something));
}
}</p>
It’s one less test to deal with, yet is as expressive and meaningful. In fact, I would say it is more meaningful because we have avoided all of the problems I listed. I’m reminded of a quote by Alan Cooper:
“No matter how beautiful, no matter how cool your interface, it would be better if there were less of it.”
Although I think he was specifically addressing User interfaces in this quote, it is applicable to all interfaces – programmatic, user, etc. Simplicity and subtleness is the key. I know I’m not the first person to talk about this problem, why it’s a problem, or the solution. I’m only claiming that I finally had that ‘aha!’ moment and realized why so many others have talked about this before.