Add times(n) to consecutive stubbing

[GoogleCodeExporter]

Add the ability to state how many times you would like a specific answer to be 
returned from a stub. For example:

when(mock.method()).thenReturn(answer, times(n));

If n = 2, the above would be the equivalent of:
when(mock.method()).thenReturn(answer).thenReturn(answer);

Original issue reported on code.google.com by gemcfad...@hotmail.co.uk on 28 Mar 2014 at 12:59

[GoogleCodeExporter]

Pull request submitted: https://github.com/mockito/mockito/pull/45

Original comment by gemcfad...@hotmail.co.uk on 28 Mar 2014 at 1:10

[GoogleCodeExporter]

Hey,

What's the use case behind this change? I'm worried it does not add great deal 
value on top of what we have?

Thanks for the pull request!

Original comment by szcze...@gmail.com on 12 Apr 2014 at 2:55

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

The use case is for when you want to return the same value several times from a 
given method call on a mock, then return a different value. 

For example, I came up with the idea when I was implementing a gaming algorithm 
where a player could take up to 9 'goes' in tic-tac-toe. To mock this 
functionality out, I wanted to return true 9 times, then return false to break 
out of a loop (signifying they had taken all their 'goes'). To achieve this 
with the current code, I had to list out true nine times eg:


    when(grid.hasFreeSlot()).thenReturn(true).thenReturn(true)
        .thenReturn(true).thenReturn(true).thenReturn(true)
        .thenReturn(true).thenReturn(true).thenReturn(true)
        .thenReturn(true).thenReturn(false);


Where as using the code in the pull request, the same can be achieved by doing:

when(grid.hasFreeSlot()).thenReturn(true, times(9)).thenReturn(false);  

I feel using the 'times' the code reads better, and is clearer. It is 
functionality I wanted to use when I was implementing the game, but found it 
wasn't available, hence the pull request.

Thanks.

Original comment by gemcfad...@hotmail.co.uk on 12 Apr 2014 at 6:55

[GoogleCodeExporter]

>For example, I came up with the idea when I was implementing a gaming 
algorithm where a player could take up to 9 'goes' in tic-tac-toe.

How about you provide a constructor parameter to 'GamingAlgorithm' class (SUT). 
By default, it would be configured to '9'. For tests, you'd configure it to '2' 
or '3'.

Original comment by szcze...@gmail.com on 12 Apr 2014 at 8:19

[GoogleCodeExporter]

Hey,

The suggestion of changing the SUT contract solely for the purpose of the test 
is not so appealing as it would result in parts of the production code that 
would only exist for testing purposes which is not ideal.

Also, even if the number of times is small eg: 

when(mock.method()).thenReturn(value, times(2)).thenReturn(anotherValue); 

There is still the advantage of no code duplication.

Thanks.

Original comment by gemcfad...@hotmail.co.uk on 15 Apr 2014 at 11:03

[GoogleCodeExporter]

>The suggestion of changing the SUT contract solely for the purpose of the test 
is not so appealing as it would result in parts of the production code that 
would only exist for testing purposes which is not ideal.

"changing the SUT contract solely for the purpose of the test" - this is 
precisely my religion. I do it every day for the past 8 years or so :D

I'm not convinced. In your example, I could do this:

when(mock.method()).thenReturn(value, value, anotherValue);
//above is simpler and shorter than below:
when(mock.method()).thenReturn(value, times(2)).thenReturn(anotherValue); 

One part of my mind likes your API suggestion, it looks good. However, I'd 
prefer if the test code was harder to silence when it calls for changing the 
SUT to get it easier to test.

For now, I'll close this issue. Feel free to comment / discuss / argue your 
POV. I can be convinced and I think your ideas pretty cool, it's just I'm not 
convinced about this particular API.

Thanks!

Original comment by szcze...@gmail.com on 20 Apr 2014 at 2:43

• Changed state: WontFix

[GoogleCodeExporter]

Hi, thanks for your input.

I agree that the example:

when(mock.method()).thenReturn(value, value, anotherValue);

is better than: 

when(mock.method()).thenReturn(value, times(2)).thenReturn(anotherValue);


however,  

when(mock.method()).thenReturn(value, value, value, value, value, value, value, 
value, anotherValue); 

is not better than

when(mock.method()).thenReturn(value, times(8)).thenReturn(anotherValue); 

The latter case is much clearer and more explicit on what kind of behaviour we 
are looking for from the mock.

The API is also symmetric with the verify times functionality, and keeps the 
api and usage consistent between stubbing and verification [ 
Mockito.verify(mock, Mockito.times(1)).method(); ]

Thanks.

Original comment by gemcfad...@hotmail.co.uk on 22 Apr 2014 at 11:12

[GoogleCodeExporter]

Yes, it looks better. However, the use case is weak because the production code 
can be changed to allow simpler testing. So unless you have some other, real 
and compelling use case, this issue will remain closed ;)

Original comment by szcze...@gmail.com on 22 Apr 2014 at 9:23

[GoogleCodeExporter]

Not to mention it is easy to create an array of certain type (if there's no 
library that already do that in the classpath).

when(mock.method()).thenReturn(value, value, value, value, value, value, value, 
value, anotherValue);

would then look like with some custom factory code:

when(mock.method()).thenReturn(a_sequence_of(9, 
value)).thenReturn(anotherValue);
when(mock.method()).thenReturn(a_sequence_of(9, value, 1, anotherValue));
// whatever works for you

Which is in my opinion looking just as good if not better and doesn't require 
to change the API.

Original comment by brice.du...@gmail.com on 22 Apr 2014 at 10:24

[GoogleCodeExporter]

Hi, thanks for the suggestions. 

Here’s a description for why SUT should not be changed purely for the reason 
of testing. You’ll find a lot of material on why this is not a good idea:
http://xunitpatterns.com/Test%20Logic%20in%20Production.html

Also I feel that:
when(mock.method()).thenReturn(a_sequence_of(8, 
value)).thenReturn(anotherValue);

does not read better than: 
when(mock.method()).thenReturn(value, times(8)).thenReturn(anotherValue);

because the former is reading as, "return a sequence of values with a length of 
8, then another value". The later is reading as: “return a value 8 times then 
another value”. The latter describes the desired behaviour - the former is 
incorrectly stating that a sequence is to be returned.

Thanks.

Original comment by gemcfad...@hotmail.co.uk on 23 Apr 2014 at 10:50

[GoogleCodeExporter]

Thanks for linking to the XUnit book, nice catch.

Did you read the whole XUnit book? What other books did you read? :) Go ahead 
and describe this exact use case to the xunit patterns mailing list. Gerard 
used to be pretty responsive. 

There are other testing patterns violated when test contains magic numbers 
('8') or when it is hardcoded to default values encoded in production code 
('8'), etc. There's also difference between test logic in production code (e.g. 
production code behaves differently for purposes of testing) and making objects 
extensible and configurable for purposes of testing.

Of course, you're welcome to have a different opinion on this matter. I'm happy 
that you read XUnit book and you care for quality of your tests!

Original comment by szcze...@gmail.com on 24 Apr 2014 at 7:26

[GoogleCodeExporter]

First of all thanks for the prompt replies. Regardless of the outcome I find 
this conversation very informative.

As to your very valid concerns about magic numbers - the line is an example for 
illustrative purposes. The name of the test and the context around this line 
describes exactly why a particular value needs to be returned a certain number 
of times.

I think we are possibly talking cross purposes. Tests should certainly inform 
how the production code looks like in order to come up with a more suitable 
design. However changing the signature of a class solely for the purpose of the 
tests has drawbacks that are well documented in any good literature about unit 
testing and object oriented design.

Lets take this concrete example: https://gist.github.com/gemcfadyen/11285891. 
Can you please illustrate how you would change this according to your 
recommendations?

Original comment by gemcfad...@hotmail.co.uk on 25 Apr 2014 at 11:26

[GoogleCodeExporter]

>However changing the signature of a class solely for the purpose of the tests 
has drawbacks

We don't change the signature of a class for testing. We write tests first and 
the signatures are 'generated' from the test code. We treat tests as 
first-class clients of the production code and we do whatever it takes to be 
able to write simple, clean and highly maintainable tests. We == TDD devs.

>that are well documented in any good literature about unit testing and object 
oriented design.

Feel free to recommend books on testing and OOD that you _read_ and considered 
_good_ and the books that _read_ and considered _not good_

Regarding the code sample, can you gist/link to the Game class implementation? 
This code might be a good use case for adding times(x).

Cheers!

Original comment by szcze...@gmail.com on 26 Apr 2014 at 2:13

[GoogleCodeExporter]

I totally agree with TDD generating the class signatures which is why I am not 
keen on the earlier suggestion of:

"How about you provide a constructor parameter to 'GamingAlgorithm' class 
(SUT). By default, it would be configured to '9'. For tests, you'd configure it 
to '2' or '3'"

which would involve changing the production class signature for testing 
purposes only.

The gist of the Game class can be viewed at:
https://gist.github.com/gemcfadyen/11371102

Thanks.

Original comment by gemcfad...@hotmail.co.uk on 28 Apr 2014 at 12:59

[GoogleCodeExporter]

That's may be a question of test, but definately me and the team at my work 
place don't like this API :

when(mock.method()).thenReturn(value, times(8)).thenReturn(anotherValue);

If we were in scala or other languages `thenReturn(values, 8 times)` would work 
but not in java with the designed API of mockito. < That's the consensus of my 
colleagues.

About the signature I agree with you. Though building an object is a bit 
special, it is the place where you configure the object. I would mind to have a 
no arg constructor for production and a parametered constructor for tests. (And 
better have a builder API).

In our code we are extremely happy with builders, they helps us a lot to 
configure object at creation time. And those test builders could be even expose 
preconfiguration.

Cheers,
brice

Original comment by brice.du...@gmail.com on 27 Jun 2014 at 5:06