I am still fairly new to RhinoMocks but a question about this. It seems more logicical to me when you tell your mock to expect a call on a property/method and then give it a value to return. So do you still set it up with Expect.Call and then assert the values later or only do the assertion? If that is the case where do you setup what the mock is supposed to return to an object that is calling it?

I think for dynamic mock / stubs this looks like it should be the expected behaviour. Strict Mocks should remain explicit.

I like it.

How would it be a breaking change, though? It's a dynamic mock in the first place, so even if people specified the same expectation two or more times to satisfy the previous behavior, their test would still pass, no? Their extra expectations wouldn't be needed any longer, but it wouldn't hurt to have them there, either, especially if they're all returning the same values, expecting the same parameters, etc.

Unless they relied on the fact that the expectation could only be consumed once for their test to pass, but that seems like a smell and unnatural anyway. It would be a good incentive to fix those tests :)

I'm torn. On the one hand the syntax is cleaner/simpler. on the other hand explicitly requiring 3 calls (with an expectation to be called 3 times) is more explicit, albeit more fragile.

Demis makes a good point that the new behavior should only apply to dynamic mocks, not strict mocks.

enough thinking out loud... I like it.

Jason,

It is applicable to stubs and dynamics, not to strict

When I first saw this syntax I definitely thought that I was defining a rule (return 1 whenever this action occurs) rather than an explicit action (the next time in the recording phase that this action occurs, return 1).

That being said, I'm not sure that the syntax of

Expect.Call( bar.Foo ).Return(1);

actually indicates which behavior should be expected. Repeat.Once definitely does.

I like this change and it has been my main gripe with RM, (although in retrospect I was probably not using the right kind of mock- shame on me)

I like it..

+1 in favor.

I agree. I've always wished that Dynamic Mocks would work this way. Now they will.

It is really more natural this way.

Now it is less "surprising" !

I am definitely in favor of this change and agree with others who have said that this 'behavior' seems more 'natural' when defining a dynamic mock/stub than the current behavior.

In such a 'dynamic-XXX' context, the paradigm is sort of already 'let me say what I want to happen and if something else happens, just disregard it please'. In a sense we are really saying that the repeated call is something 'else' even though its technically the same as the first thing I was explicit about.

The implicit .Repeat().Any() that you are suggesting here that I can later get more specific with a .Repeat().Once() if I want to makes much more sense to me in the dynamic context.

Also +1 for leaving strict-XXX behavior...strict!

I like the change. That is the behavior I want most of the time anyway.

+1...seems to fall more in line with the intent of a stub.

I like it. With the old behavior, a replace-temp-with-query refactoring could easily break a test. I was often having to go back and explicitly indicate one or more calls after having broken a test this way. This is much better. Thank you.

I'm not sure if this is better. The other way made you think more about your code. Like why did it do that twice? But I guess I will just have to be more explicit. And it is all yours anyway. And as long as it works in VB ;-)

Expect.Call( bar.Foo ).Return(1).EveryTime();

Had been better in my opion.

Upgrading from 3.5 RC1 to 3.5 RTM is already going to break dozens of my tests, so what's a few more. ;) Not that I'm happy about there being breaking changes between a release candidate and an RTM, though. :(

In all seriousness, I don't think I want mocks changing to default to allowing multiple calls. If my code says "expect a call", I expect a call. I'll add repeat if I want it. I doubly-dislike that this would be a breaking change to a very highly used piece of Rhino.Mocks. :(

Now, for stubs, on the other hand, defaulting to the equivalent of Repeat.Any() makes perfect sense.

Addendum - I'll have to give more thought to the fact that we are talking about dynamic mocks here, not "classic" ones, but I suspect my opinion will probably still hold since dynamic mocks are the basis of the AAA mocks, no? If so, then I'm still against the change.

I like the change, I've been bit a couple of time by the previous behavior.

I actually prefer explicit distinction between expectation and stubbing. I.e., Expect.Call() IMO would be more natural to expect exactly 1 call; whereas Stub.Call() would merely stub a return value regardless of number of calls (including zero). This is also the same semantics used in many mockers e.g. jMock.

Nonetheless, I hope AAA on dynamic mock would still expect 1 call by default?

*Stub.Call()

Or rather SetupResult.For() in Rhino.Mocks

Repeat.Once is unclear. Repeat is repetition so therefore

"Perform once" means to perform an action only once, whereas "Repeat once" means to perform an action twice (one original time, and one repeated time).

Because of this I have always read Repeat.Once to mean 1+1 and not 1. I have always thought "Perform" would be more clear than "Repeat"

Perform().Once();

Perform().Times(3);

Perform().ManyTimes();

Back onto your subject.

To me Expect.Call() means two things

01: It is mandatory

02: It is singular

This is because you use the word Call and not the plural Calls. Whereas SetupResult.For() has no implications of how many times and is therefore easily assumed to mean "as many times as you need".

When testing I either need to set up a result for a method call (SetupResult) or test that something mandatory happens (Expect.Call). When I expect a call 99% of the time I expect it to be called only once, and the current scheme implies that is what I am doing. Therefore having to add Repeat().Once() is bad in two ways

01: 99% of the time I have to type more.

02: Due to the wording it implies 2 calls minimum, and is confusing for someone who is new.

I think it is always better to break code at compile time rather than execution time. With that in mind I would recommend changing the naming to something like

Expect.OneCall( a.B() ).Return(1);

and

Expect.CallsTo (a.B() ).Return(1);

If you implement "CallsTo" then it is important to rename Call because I think having "Call" and "CallsTo" are too similar.

maybe you should also rename Repeat to Exactly

Expect.CallsTo( ... ).Exactly(2).Return(1);

To summarise

Expect.OneCall(...).Return(1); - 1..1 calls (no Repeat option available)

Expect.CallsTo(...).Return(1); = 1..* calls

Expect.CallsTo(...).Return(1).Exactly(5); 5..5 calls

I hope this posts okay :-)