Rhino 3.5: an example where dynamic mock lead to flawed tests

[(luKa)]

Ciao from Stockholm,

I'm reading this
http://ayende.com/wiki/Rhino+Mocks+3.5.ashx#CreateMockisdeprecated,replacedbyStrictMockTheuseofStrictMockisdiscouraged
and I've noted that dynamic mocks can lead to flawed tests.

Going on with the example in the link, suppose that with the evolution
of the sw the method RevertUserChanges() is added to the
IUserRepository interface.
Then a dev introduce a bug in the LoginController.ForgotMyPassword()
so that RevertUserChanges() get called by mistake.
As I understood the Dynamic Mock now wont raise an error because of
this call to RevertUserChanges().

I feel that brittle tests are a smell for responsibilities that must
be decoupled instead of a need for object dynamically mocked. What you
think ?
when I think about exception I see a similarity, I prefer to get the
exception instead of running the risk of catching and ignoring an
important problem.

The agile principle here is feedback and fail fast



Luca Minudel
Stockholm, Sweden GMT+1
http://www.linkedin.com/in/lucaminudel en-GB
http://blogs.ugidotnet.org/luKa/ it-IT

[Tim Barcz]

If I may summarize...I believe you are saying that the dynamic mocks create more brittle tests than tests where strict mocks are used (CreateMock in previous versions of RhinoMocks).

I resonate with what you are saying.  I held this view for some time as I felt that strict mocks were more "correct".  As someone who has used RhinoMocks for a few years I can say that I understand the move to deprecate CreateMock.  I found through our experience that our tests were far more brittle using strict mocks.  You mention agile and feedback loops and failing fast.  What I found is that our use of strict mocks did now allow for easy refactoring, another highly held agile principle.  The issue I see with "failing fast" and strict mocks is that often the failing tests are really just false positives.

If you're concerned something might be called that shouldn't be...you can always test for specific scenarios...ie. say AssertWasNotCalled or Expect().Times.Never

Very interested in hearing your thoughts and others.

Tim

http://www.twitter.com/timbarcz

[Shane Courtrille]

Like Tim I understand the concern you have with missed calls as I started using Rhino Mocks with the same concern.  With that said I agree with everything Tim has said.  Experience has shown over years of using Mocking that strict mocks actually leads to very strong test smells.  If we look at the bible of testing 'xUnit Test Patterns' the closest test smell which I think matches when we say brittle tests is Fragile Tests.  Fragile tests are defined as 'A test fails to compile or run when the SUT is changed in ways that do not affect the part the test is exercising'.  As Tim mentions this behavior actually occurs as a result of StrictMocks.  When using StrictMocks every call that occurs must be recorded.  At first this doesn't seem so bad until you take into account the practice of asserting one thing per test.  This includes mock usage.  But because you are using a StrictMock every test must include expectations for every single call that occurs.  This means that if you are asserting a few different mock calls every single test fails when you change a single line of code rather then the one test that should fail.  This is a textbook example of Fragile Tests.

I would also mention that having switched to DynamicMocks awhile ago I can happily say that I have never ran into the problem we all started out being concerned with.  The use of Dynamic Mocks has proven itself to be quite safe and lead to tests which are much more trustable (a key requirement of good tests).

Regards,

Shane

[Tim Van Wassenhove]

In my experience strick mocks lead to overspecification.

Here is a real life example:

-While developping a web interface we first focussed on implementing
the basic edit functionality.This meant that our strict view moq knew
a registration to the EditClick event would happen.

- Then we moved on to the next feature, which included subscribing to
the SaveClick event. Because this extra subscription all our tests of
the first feature failed because they were not expecting that extra
event subscription. Is this extra subscription a concern in our tests
for the first feature? I have the feeling that it shouldn't matter.

That was the point were i started to choose dynamick mocks by default,
and strict mocks when required.

Greetz,
Tim

[isaiah perumalla]

This is an interesting topic i had many discussions recently.

>>>While developping a web interface we first focussed on implementing
>>>the basic edit functionality.This meant that our strict view moq knew
>>>a registration to the EditClick event would happen.

>>>- Then we moved on to the next feature, which included subscribing to
>>>the SaveClick event. Because this extra subscription all our tests of
>>>the first feature failed because they were not expecting that extra
>>>event subscription. Is this extra subscription a concern in our tests
>>>for the first feature? I have the feeling that it shouldn't matter.

If mocks are used for that then i would agree strict mock will cause you problems.
The argument for dynamic mock is that Strict mocks is often a design smell, because tests to express knowledge of an operation's internals, and does not allow for easy refactoring.
To me this is a design issue with the code undertest, the brittleness of the test is a symptom of a weakness in the design. To me it means that the object is exposing its internal details, its not encapsulating its internal interactions. Test should be treated like any client code and should never know the internal details of the object under test. When i have a brittle test first thing i do is check to see if i'm specifying interactions concerned with implementation.
I mainly use mocks to discover roles in a system so i can create and design role interfaces, which consist mainly of command methods.
eg receiptReceiver.Expect(x => x.ReceiveTotalDue(3.Dollars));
I rarely use dynamic mocks, since i mainly use mock to design interactions between objects and very the indirect output.
Normally i start with an end-to end failing test and work my way in.
In your case i guess you using MVP with webforms and you dont have a higher level end to end test to start with so in this case you need something like a dynamic mock, when you testing  that  something subscribes to some event.

--
Isaiah Perumalla

[Tim Barcz]

I would disagree with "Test should be treated like any client code and should never know the internal details of the object under test".  If you don't know the internal how can you ever do

• Expect.Call.Time.Never
• Assert.WasNotCalled
• Assert.WasCalled
• etc

you HAVE to know the internals of the SUT in order to do some types of testing.  Remeber, there are two types of testing which mocking helps with

1. State based tests
2. Interaction based

Stubs shine with state based tests providing canned results, but to really DO interaction based tests you need mocks and knowledge of the internals of the method.

Agree?

[isaiah perumalla]

Ok i should clarify that a little, when i design an object, i distinguish between an objects internals and its peers. Peer objects are things either passed into the object through the constructor if it is a dependency or via a method if only need for that method call. Internals on the other hand are objects created inside the object undertest, the interactions with this should be hidden. What a lot of people do is create an interface and push what should have be an internal object into the constructor just so it can be mocked and tested, this where you end up getting brittle tests. Given this distinction i would never want my test to know the internal interactions

Hope that made sense

--
Isaiah Perumalla

[(luKa)]

Tim yes that what I'm saying

I see 2 thread-off here

1) having interaction-based tests with mocks that are sensitive enough
to catch errors/bugs and insensitive enough not to break at every
small refactoring
2) having mock framework powerful enough to easily mocks objects and
basic enough to let the developer fill the code design smells

from the TDD point of view , the #2 is by far the most important.
I would say that 8 times out of 10 when an interaction-based test is
fragile it is a smell that
- in the object under test there is inappropriate responsibility
coupling , or
- the object under test is interacting with too low-level objects or
it is exposing internal low-level implementation details
the other 2 times, once mocks where used for input or state testing
instead of stabs and the other a hand coded mocks will do the job
(better then complicated mock tool features)

for #1, there are objects (like simulation objects or math/statistic
objects) that change their global state from every single methods,
because of this a dynamic mock will be too insensitive of bugs. since
you cannot predict the evolution of your objects I feel a little
dangerous the use of dynamic mocks

On Mar 23, 1:36 am, Tim Barcz <timba...@gmail.com> wrote:
> If I may summarize...I believe you are saying that the dynamic mocks create
> more brittle tests than tests where strict mocks are used (CreateMock in
> previous versions of RhinoMocks).
> I resonate with what you are saying.  I held this view for some time as I
> felt that strict mocks were more "correct".  As someone who has used
> RhinoMocks for a few years I can say that I understand the move to deprecate
> CreateMock.  I found through our experience that our tests were far more
> brittle using strict mocks.  You mention agile and feedback loops and
> failing fast.  What I found is that our use of strict mocks did now allow
> for easy refactoring, another highly held agile principle.  The issue I see
> with "failing fast" and strict mocks is that often the failing tests are
> really just false positives.
>
> If you're concerned something might be called that shouldn't be...you can
> always test for specific scenarios...ie. say AssertWasNotCalled or
> Expect().Times.Never
>
> Very interested in hearing your thoughts and others.
>

> Timhttp://www.twitter.com/timbarcz

>
> On Sun, Mar 22, 2009 at 6:52 PM, (luKa) <luca.minu...@gmail.com> wrote:
>
> > Ciao from Stockholm,
>
> >     I'm reading this
>

> >http://ayende.com/wiki/Rhino+Mocks+3.5.ashx#CreateMockisdeprecated,re...

[Tim Barcz]

Thanks for sharing your thoughts...I'll have to chew on this a bit more.  I'm am by no means an authority on testing and really adjust what I do based on friction and feedback I'm internalizing as I'm developing my applications.

I'll think/stew on this a bit....

Tim

[(luKa)]

> - the object under test is interacting with too low-level objects or

I've run into this while testing the presenter of an MVC aspx page.
If it can be of any use I can post a code snippet of this case

[Tim Barcz]

I would be interested in seeing...

[(luKa)]

I have a a web page to execute some batch electronic payment.
Present() is a Presenter method called when the page is first loaded.
For it I wrote the first test.

A new feature that check that paymentId really is an Electronic
payment required a second test ad a change to the first one:

///THIS IS THE FIRST TEST FOR THE PRESENTER
[Test]
public void
PrensentMethodGetElectronicPaymentInfosFromModelAndShowsInTheView()
{
var mockModel =
mocks.StrictMock<IPanelPaymentCreateBatchModel>();
var mockView =
mocks.StrictMock<IPanelPaymentCreateBatchView>();

using(mocks.Record())
{
///
///THIS EXPECTATION HAD TO BE ADDED FOR THE NEW
FEATURE (CHECK ON PaymentId)
///
mockModel.Expect(m => m.IsElectronicPayment
(12345)).Return(true);

mockModel.Expect(m => m.GetPaymentInfos
(Arg<int>.Is.Equal(12345), Arg<int>.Is.Equal(53),
out
Arg<string>.Out("paymentDesc").Dummy));

mockView.Expect(v => v.ShowListInput
("transBatchTypeDesc", "PanelName","paymentDesc"));
}

using(mocks.Playback())
{
var presenter = new PanelPaymentCreateBatchPresenter
(mockView, mockModel);
presenter.Present(12345,
53);
}
}


///THIS IS THE TEST ADDED FOR THE NEW FEATURE (CHECK ON
PaymentId)
[Test]
public void PrensentMethodShowErrorForNonEletronicPayments()
{
var mockModel =
mocks.StrictMock<IPanelPaymentCreateBatchModel>();
var mockView =
mocks.StrictMock<IPanelPaymentCreateBatchView>();

using (mocks.Record())
{
mockModel.Expect(m => m.IsElectronicPayment
(12345)).Return(false);

mockView.Expect(v => v.ShowError(null)).IgnoreArguments
();
}

using (mocks.Playback())
{
var presenter = new PanelPaymentCreateBatchPresenter
(mockView, mockModel);
presenter.Present(12345, 53);
}
}


having to change also the 2nd test for the new feature was a smell to
me.
it pointed out that this logic (test PannelId before getting
Electronic payment infos) should be pushed down in the model. so the 2
tests would result to be decoupled then.

[(luKa)]

And here are the tests after the coupled responsibility has been moved
down into the model. the tests are decoupled too now :

[Test]
public void
PrensentMethodGetElectronicPaymentInfosFromModelAndShowsInTheView()
{
____var mockModel = mocks.StrictMock<IPanelPaymentCreateBatchModel>();
____var mockView = mocks.StrictMock<IPanelPaymentCreateBatchView>();

____using(mocks.Record())
____{
________mockModel.Expect(m => m.GetPaymentInfos(
_______________________Arg<int>.Is.Equal(12345), Arg<int>.Is.Equal
(53),
_______________________out Arg<string>.Out
("transBatchTypeDesc").Dummy,
_______________________out Arg<string>.Out("PanelName").Dummy,
_______________________out Arg<string>.Out("paymentDesc").Dummy));

________mockView.Expect(v => v.ShowListInput("transBatchTypeDesc",
"PanelName","paymentDesc"));
____}

____using(mocks.Playback())
____{
________var presenter = new PanelPaymentCreateBatchPresenter(mockView,
mockModel, null);
________presenter.Present(12345, 53);
____}
}


[Test]
public void PrensentMethodShowErrorForNonEletronicPayments()
{
____var mockModel = mocks.StrictMock<IPanelPaymentCreateBatchModel>();
____var mockView = mocks.StrictMock<IPanelPaymentCreateBatchView>();

____using (mocks.Record())
____{
________mockModel.Expect(m => m.GetPaymentInfos(
_______________________Arg<int>.Is.Anything, Arg<int>.Is.Anything,
_______________________out Arg<string>.Out(null).Dummy,
_______________________out Arg<string>.Out(null).Dummy,
_______________________out Arg<string>.Out(null).Dummy))
_________________.Throw(new ArgumentOutOfRangeException());

________mockView.Expect(v => v.ShowError(null)).IgnoreArguments();
____}

____using (mocks.Playback())
____{
________var presenter = new PanelPaymentCreateBatchPresenter(mockView,
mockModel, null);
________presenter.Present(12345, 53);
____}

}