Reusing mocks in multiple run-verify sequences

[Pedro Pedruzzi]

Hi!

In unit testing often we run a lot of related tests sequentialy.

In my case I'm testing a component that receives its dependencies in
the constructor.

I create a mock, setup some stubbing, and then construct the component
I want to test passing the mock as a parameter. And I'd like doing
that just once for a whole set of related tests (all tests in my test
class, for instance).

If all I wanted to do was stubing, it would work nicely. But if I also
want to do verification I will have problems because the mock will
accumulate the interactions since its creation.

Is there any way to "clear" the interaction records on a mock? So in
each test method I could simply "clear-run-verify", without the need
to recreate the mock and (because of the constructor dependency
injection pattern) create other target component instance.

Regards,

--
Pedro Pedruzzi | V2COM

[Igor Czechowski]

Hi Pedro,

Do you have any code snippet, which actually requires clear/reset on the mock ?

You can create a mock and other target components in @Before method,
which is invoked before each test method, that way you will have fresh
mock for each test.

Beside, when using Mockito, you can verify what you have interest in
even if more invocations happen, unless you call
Mockito.verifyNoMoreInteractions() at the end of each test.

Hopes it helps.
Igor

[Pedro Pedruzzi]

Hi Igor.

Take the following snippet as an example (it uses TestNG):

public class TimedCacheTest {

private final Object KEY = new Object();
private final Object VALUE = new Object();
private final Object NEW_VALUE = new Object();

private ObjectLoader mockLoader;
private TimedCache cache;

@BeforeClass
public void setup() {

// ObjectLoader mock
mockLoader = mock(ObjectLoader.class);
// consecutively stubbing (mockito 1.4 or higher)
stub(mockLoader.load(KEY)).toReturn(VALUE).toReturn(NEW_VALUE);

// suppose this TimeCache construction is somehow a time expensive task
// in a way I want to do it only once
cache = new TimedCache(mockLoader, mockClock, mockReloadPolicy);

}

@BeforeMethod
public void setup() {
// if I could just
clear(mockLoader);

// I will do not have to reinstanciate the mock and the TimeCache
// nor redefining the "global" stubbings
}

@Test
public void testFirstLookUp() {
assertSame(VALUE, cache.lookUp(KEY));
verify(mockLoader).load(KEY);
}}

@Test
public void testCachedObject() {
assertSame(VALUE, cache.lookUp(KEY));
assertSame(VALUE, cache.lookUp(KEY));
verify(mockLoader).load(KEY);
}

@Test
public void testCacheReload() {
assertSame(VALUE, cache.lookUp(KEY));
delay(2000);
assertSame(NEW_VALUE, cache.lookUp(KEY));
verify(mockLoader, times(2)).load(KEY);
}

// ...
}

Pedro Pedruzzi

[szczepiq]

I agree that when object creation is expensive then the natural tweak
would be using @BeforeClass instead of just @BeforeTest. The class
under test is created once instead n times, where n is the number of
tests.

But is it a real scenario you have there?
Firstly, I don't think there is much to gain when using @BeforeClass
(miliseconds? did anyone measure it?). Secondly, one of the key points
of mocking is the ability to replace expensive logic with a cheap but
controllable substitute. If there is something expensive in TimedCache
creation then this is something that should be encapsulated in a
dependency and mocked out.

Here is my test:

@BeforeMethod
public void setup() {
mockLoader = mock(ObjectLoader.class);
cache = new TimedCache(mockLoader, otherStuff);
}

@Test
public void shouldReadValueFromCache() {
//I prefer to stub per test method:
stub(mockLoader.load("foo")).toReturn("bar");
assertSame("bar", cache.lookUp("foo"));
assertSame("bar", cache.lookUp("foo"));
verify(mockLoader, once()).load("foo");
}

@Test
public void shouldReloadValue() {
stub(mockLoader.load("foo")
.toReturn("bar")
.toReturn("baz");

assertSame("bar", cache.lookUp("foo"));
delay(2000);
assertSame("baz", cache.lookUp("foo"));
//don't really need verify() here
}

I prefer using just @BeforeMethod. That makes me happier because there
is no state left over after a test method. I guess it makes the test
methods truly independent and maintainable. Actually I don't remember
ever using @BeforeClass. I don't even know what is (and if there is)
the junit4 equivalent (and I'm a junit user :) ...

What do you think?

Cheers,
Szczepan Faber

[Pedro Pedruzzi]

Hi Szczepan!

Thanks for your response.

On Wed, Jul 2, 2008 at 8:32 PM, szczepiq <szcz...@gmail.com> wrote:
>
> I agree that when object creation is expensive then the natural tweak
> would be using @BeforeClass instead of just @BeforeTest. The class
> under test is created once instead n times, where n is the number of
> tests.

That is my point.

> But is it a real scenario you have there?

It is not. But I think it is completely possible.

In fact I think I was making a wrong assumption about testing methods.
Your point that tests methods should be independent made sense to me.
I don't see it as a general rule. TestNG for example has a mechanism
of test dependencies. But I think this is just about prerequisites
(skip the dependent test if "parent" test failed avoid failing a whole
set of tests - it is a very handy feature of TestHG by the way).

I was trying to make a statefull test class. I don't know if this
makes sense or is completely wrong. I just feel that it is not the
best practice.

I will make my test classes stateless as you suggest. Let us see if I
get more happy with that! :-)

Thanks for your time and for the excellent mocking framework.

Regards,

--
Pedro Pedruzzi | V2COM

PS: I am using mockito 1.4, and I'm waiting to see it on maven official repo!

[ErikEngerd]

On 2 Jul, 14:51, "Pedro Pedruzzi" <pedro.pedru...@gmail.com> wrote:
> Hi!
>
> In unit testing often we run a lot of related tests sequentialy.
>
> In my case I'm testing a component that receives its dependencies in
> the constructor.
>
> I create a mock, setup some stubbing, and then construct the component
> I want to test passing the mock as a parameter. And I'd like doing
> that just once for a whole set of related tests (all tests in my test
> class, for instance).
>
> If all I wanted to do was stubing, it would work nicely. But if I also
> want to do verification I will have problems because the mock will
> accumulate the interactions since its creation.

Can't you use in order verification? For instance:

final Observer mockObserver = mock(Observer.class);
InOrder order = inOrder(mockObserver);

long subscription = _observable.subscribe(mockObserver);

assertEquals(1, _observable.getObserverCount());

final String message = "hallo";
_observable.send(message);

order.verify(mockObserver).send(_observed, message);
verifyNoMoreInteractions(mockObserver);
order.verify(mockObserver).send(_observed,
message); // will fail because inOrder does not accumulate.

This works because without inOrder, writing
'verify(obj).method(args)' simply checks the same condition twice (was
method called once with args), whereas writing
inOrder.verify(obj).method(args) twice checks for two consecutive
calls.

Cheers
Erik

[ErikEngerd]

Hi all,


I am rewriting some tests from EasyMock to Mockito and I have several
uses for reset functionality that, I think are legitimate.
The example is that of a directory poller with a poll() method that
notifies a listener of files being created, deleted, or changed.

Here I want to test that the deletion of a file is detected. To do
this, I first setup the directory monitor with the listener. Then
establish an initial state by creating a file and calling poll().
After that initial setup, I delete the file and call poll() again. I
now want to verify that fileDeleted() is called on the listener,
ignoring whatever happened before. In my design, I am passing in the
listener at construction time of the directory monitor which is good
practice since I do not need multiple listeners and it makes my
directory monitor immutable. Implementing a subscription mechanism
would add even more tests to test the subscription mechanism so that
is not an option.

To deal with this situation, I made a simple utility by which the
reset functionality is implemented. It consists of a utility class
that creates a proxy object that delegates to a Mockito mock and
provides the reset functionality which simply creates a new mock under
the hood. The utiilty provides one method getProxy() to obtain the
proxy to pass to your tested classed and one method getMock() to pass
to mockito. I tried creating a proxy that would act as a mockito mock
by itself but that didn't work because mockito checks whether a given
object is a mockito mock or not.

The test code is as follows for the file deletion example:

protected void setUp() throws Exception {
super.setUp();
_data = new TestData(this);
_data.clean();
_listener = new ResettableMock<DirectoryMonitor.Listener>(
DirectoryMonitor.Listener.class);
_monitor = new DirectoryMonitor(_data.getRoot(), new
AwkFilenameFilter(
REGEX), _listener.getProxy());
}

and the test case is

public void testFileDeleted() {
_data.createFile(FILE1, "hello");
_monitor.poll();
_listener.reset();

_data.deleteFile(FILE1);
_monitor.poll();

verify(_listener.getMock()).fileDeleted(_data.getFile(FILE1));
verifyNoMoreInteractions(_listener.getMock());
}

The ResettableMock class is as follows:

/**
*
*/
package org.wamblee.test;

import static org.mockito.Mockito.*;

import java.lang.reflect.InvocationHandler;
import java.lang.reflect.Method;
import java.lang.reflect.Proxy;

/**
* Resettable mock is a utility to support reset functionality for
mockito.
* @author Erik Brakkee
*
* @param <T>
*/
public class ResettableMock<T> {

private static class MockitoInvocationHandler<T> implements
InvocationHandler {

private Class<T> _class;
private T _mock;

public MockitoInvocationHandler(Class<T> aClass) {
_class = aClass;
_mock = mock(aClass);
}

public void reset() {
_mock = mock(_class);
}

public T getMock() {
return _mock;
}

@Override
public Object invoke(Object aProxy, Method aMethod, Object[] aArgs)
throws Throwable {
return aMethod.invoke(_mock, aArgs);
}
}

/**
* Invocation handler that delegates to the current mockito mock and
allows
* creation of a new mock.
*/
private ResettableMock.MockitoInvocationHandler<T> _handler;

/**
* Proxy object to be passed to tested classes.
*/
private T _proxy;

/**
* Constructs the resettable mock.
* @param aType Type to mock. Must be an interface type.
*/
public ResettableMock(Class<T> aType) {
if ( !aType.isInterface()) {
throw new IllegalArgumentException("Class '" + aType.getName() + "'
must be an interface");
}
_handler = new MockitoInvocationHandler(aType);
_proxy = (T) Proxy.newProxyInstance(aType.getClassLoader(),
new Class[] { aType }, _handler);
}

/**
* Resets the mock effectively forgetting all previous interactions
and verifications.
*/
public void reset() {
_handler.reset();
}

/**
* Gets the current mock object to pass to mockito calls.
*
* @return Mock object.
*/
public T getMock() {
return _handler.getMock();
}

/**
* Returns the proxy that your tested classes will used.
*
* @return Proxy object.
*/
public T getProxy() {
return _proxy;
}

}

Cheers
Erik

[szczepiq]

>This works because without inOrder, writing
>'verify(obj).method(args)' simply checks the same condition twice (was
>method called once with args), whereas writing
>inOrder.verify(obj).method(args) twice checks for two consecutive
>calls.

This is true. For example:

verify(foo, times(2)).bar();

is equal to:

inOrder.verify(foo).bar();
inOrder.verify(foo).bar();

Obviously I recommend using inOrder only if it is relevant to test
interactions in order :)

Cheers,
Szczepan Faber

When you do this:

_observable.send(message);

there is only single interaction:

observer.send(observed, message);

So in the test:

...

order.verify(mockObserver).send(_observed, message);
verifyNoMoreInteractions(mockObserver);
order.verify(mockObserver).send(_observed, message); // will fail
because inOrder does not accumulate.

[szczepiq]

Hi,

Now the response for the other question about reset()

>now want to verify that fileDeleted() is called on the listener,
>ignoring whatever happened before.

If you're ignoring whatever happened before then why do you use
verifyNoMoreInteractions() ? verifyNoMoreInteractions() is rather
recommended for those situations where you actually don't want to
ignore anything.

There is a common misuse of verifyNoMoreInteractions() that I often
see. Devs who did a lot of mocking with expect-run-verify libraries
tend to write verifyNoMoreInteractions() very often or even in every
test method. verifyNoMoreInteractions() is not a part of the mockito
contract that should be called per test method. See my post here:
http://groups.google.com/group/mockito/t/a7b6c7632c728e76

The very point of Mockito library is to provide painless, noninvasive
testing. In your example for instance, you "want to verify that

fileDeleted() is called on the listener, ignoring whatever happened

before". Mockito helps you to test precisely what you want: verify
that fileDeleted() is called on the listener, ignoring other
interactions that are irrelevant in your test.

So, let's get back to your example. I think there are 2 options here:

1. I could write pseudo-functional, scenario-like test where I do
multiple operations (createFile(), deleteFile(), etc.). In that kind
of test I could verify all interactions. At the end, I could put
verifyNoMoreInteractions() (though, I probably wouldn't - it depends
if it's relevant).
2. I could write classic unit test with small test methods focused
around behaviour. In that kind of test I would verify only the
interaction that is relevant to the test method (e.g.
shouldNotifyDeleteMessage(), shouldNotifyCreateMessage()).

Considering what I said above would you still use reset() method in
your example?

Hope that helps,
Szczepan Faber.

[ErikEngerd]

On Jul 10, 4:35 pm, szczepiq <szcze...@gmail.com> wrote:
> Hi,
>
> Now the response for the other question about reset()
>
> >now want to verify that fileDeleted() is called on the listener,
> >ignoring whatever happened before.
>
> If you're ignoring whatever happened before then why do you use
> verifyNoMoreInteractions() ? verifyNoMoreInteractions() is rather
> recommended for those situations where you actually don't want to
> ignore anything.
>
> There is a common misuse of verifyNoMoreInteractions() that I often
> see. Devs who did a lot of mocking with expect-run-verify libraries
> tend to write verifyNoMoreInteractions() very often or even in every
> test method. verifyNoMoreInteractions() is not a part of the mockito
> contract that should be called per test method. See my post here:http://groups.google.com/group/mockito/t/a7b6c7632c728e76
>
> The very point of Mockito library is to provide painless, noninvasive
> testing. In your example for instance, you "want to verify that
> fileDeleted() is called on the listener, ignoring whatever happened
> before". Mockito helps you to test precisely what you want: verify
> that fileDeleted() is called on the listener, ignoring other
> interactions that are irrelevant in your test.

This is indeed something I could do and what I also considered.
The problem is basically that the part up to and including the first
poll() call is part of the setup for my specific test. Therefore, to
be precise I want to start with a clean slate after that call. That
allows me to
verify that 'only fileDeleted() is called since the file was deleted
and no other methods'.

>
> So, let's get back to your example. I think there are 2 options here:
>
> 1. I could write pseudo-functional, scenario-like test where I do
> multiple operations (createFile(), deleteFile(), etc.). In that kind
> of test I could verify all interactions. At the end, I could put
> verifyNoMoreInteractions() (though, I probably wouldn't - it depends
> if it's relevant).
> 2. I could write classic unit test with small test methods focused
> around behaviour. In that kind of test I would verify only the
> interaction that is relevant to the test method (e.g.
> shouldNotifyDeleteMessage(), shouldNotifyCreateMessage()).

That is possible but less precise in this example because I wanted to
exclude the interactions that were part of the setup for the test
case.

In fact, I did something like you suggest where I added functionality
to the code that led to interactions that were not relevant for the
test at hand. Mockito saved me here since it allowed me to only test
the interactions I was interested in. In that particular case, the
interactions occurred after the setup for the test case so reset()
couldn't have helped there.

>
> Considering what I said above would you still use reset() method in
> your example?

I think I would have because during the setup of my test case I have
interactions that are not relevant (have been tested before) and want
to ignore those. It just allows me to be much more precise in the
verification.

Cheers
Erik

>
> Hope that helps,
> Szczepan Faber.
>

[szczepiq]

Hi,

Interesting idea of using reset() functionality. I guess I can
generalize the pattern:

test {
unit.exercise();
mock.forgetInteractions();

unit.exerciseAgain();
mock.verifyInteractions();
mock.verifyNoMoreInteractions();
}

The problem is that I don't know if it is a good pattern... Cannot
really say it's bad at all times, though. Generally in unit tests, I
don't exercise the unit more than once per test method. In functional
tests I don't generally use mocks. In most tests I don't generally use
verifyNoMoreInteractions() and so on.

Your scenario could be a legitimate use of reset(). OTOH, introducing
reset() functionality opens the framework for bad usage with lengthy
tests with exercise->verify->reset->exercise->... sequences (I'm 100%
sure some devs will do that). Also, verifyNoMoreInteractions() is a
message telling that all interactions are specified in *this* test.
That's why I'd rather not enable mock resetting (unless I'm voted out
by users. Any votes now?). But I will keep your idea in mind.

Also, I'm not entirely convinced why you cannot just do the following
(show me the code under test to prove that verifyNoMoreInteractions()
is required :)

test {
unit.exercise();
unit.exerciseAgain();
mock.verifyInteractions();
}

Cheers,
Szczepan Faber

[ErikEngerd]

On Jul 12, 10:16 pm, szczepiq <szcze...@gmail.com> wrote:
> Hi,
>

> Interesting idea of using reset() functionality. I guess I can
> generalize the pattern:
>
> test {
> unit.exercise();
> mock.forgetInteractions();
>
> unit.exerciseAgain();
> mock.verifyInteractions();
> mock.verifyNoMoreInteractions();
>
> }

In the example I gave it is more like unit.setup() instead of
unit.exercise();

>
> The problem is that I don't know if it is a good pattern... Cannot
> really say it's bad at all times, though. Generally in unit tests, I
> don't exercise the unit more than once per test method. In functional
> tests I don't generally use mocks. In most tests I don't generally use
> verifyNoMoreInteractions() and so on.

In functional test cases I definitely use mocks. Someone once gave me
an example of the definition of a functional test, which was running
the tests in a simulated environment, as opposed to system test where
all the actual components are in place. In particular, I am thinking
here about tests that involve a third-party system that is very
difficult to control directly from java code. Consequently, it is
useful to write mocks for them.

I think in general it is good practice to be precise in
verifications.
So,

unit.exercise();
verifyInteractions();
clear();
unit.exerciseSomeMore();
verifyAdditionalInteractions();

In my particular example, I first setup the test case by creating a
file and polling the directorymonitor. Then the testcase starts and I
want to verify that fileDeleted is called.

Here is what this would look like without the reset() functionality.


File testfile = testdata.createFile(file);
monitor.poll();
testfile.delete();
monitor.poll();
verify(listener).fileDeleted(testfile);

Now the above test would be imprecise since fileCreated() was also
called on the file. How do I know that only fileDeleted was called
after the file was deleted and no other method? Well, I would have to
resort to this:

File testfile = testdata.createFile(file);
monitor.poll();
verify(listener).fileCreated(testfile);
testfile.delete();
monitor.poll();
verify(listener).fileCreated(testfile);
verify(listener).fileDeleted(testfile);
verifyNoMoreInterfactions(listener);

Now the above would work, but looks extremely awkward. Why would I be
testing that fileCreated() was called after the deletion of a file?
Well, only to test that no further calls were made but it does not
look natural. Having a reset makes this code much more natural. Of
course, in simple cases, in order verification also works, but that
could be too strict sometimes.

And I don't see why a reset() in test code would be bad. I want to use
a mocking framework also to simulate more complex scenario's where I
want to ignore all interactions prior to a certain point. Just to
verify that it is really the last exercise of the unit that I am
testing.

>
> Your scenario could be a legitimate use of reset(). OTOH, introducing
> reset() functionality opens the framework for bad usage with lengthy
> tests with exercise->verify->reset->exercise->... sequences (I'm 100%
> sure some devs will do that).

It is not clear to me why you call this bad usage.

> Also, verifyNoMoreInteractions() is a
> message telling that all interactions are specified in *this* test.

Sometimes it is easier to use verifyNoMoreInteractions() then
explicitly verifying zero interactions on specific methods of the
mock. If a test case can use verifyNoMoreInteractions() than that is a
more strict assertion than not having it or verifying only a subset of
the interactions. Of course, it is a huge advantage of mockito not
having to verify all interactions but I don't see anything bad with
verifyNoMoreInteractions() unless of course the test case verifies
interactions that are unrelated to the test.

In other words, there should not be a rule to avoid
verifyNoMoreInteractions() but there really *must* be a rule to avoid
verifying interactions that are irrelevant for the test case at hand.

For instance, in my directory poller example the following would be
wrong:

verify(listener).makeSomeCoffee(); // IRRELEVANT VERIFICATION
verifyNoMoreInteractions(); // By itself a really strong assertion
so should be ok.

Instead, I would have to focus on the interactions that are of
interest:

verify(listener, never()).fileCreated((File)anyObject());
verify(listener, never()).fileModified((File)anyObject());
verify(listener).fileCreated(testfile);

In the above case, verifyNoMoreInteractions() is not the way to go
because of the makeSomeCoffee() call. But, in my example, all methods
are relevant so there I can use verifyNoMoreInteractions() without
having to verify irrelevant interactions.

> That's why I'd rather not enable mock resetting (unless I'm voted out
> by users. Any votes now?). But I will keep your idea in mind.

As you'd guess I am in favor of reset() functionality. Also,
verifyNoMoreInteractions() is not the cause of the problem. People
just have to step out of their easymock/jmock way of working and
verify only relevant interactions. The use of
verifyNoMoreInteractions() is not bad, but verifying irrelevant
interactions is.

>
> Also, I'm not entirely convinced why you cannot just do the following
> (show me the code under test to prove that verifyNoMoreInteractions()
> is required :)

Given some, hopefully convincing, arguments above.

Cheers
Erik

>
> test {
> unit.exercise();
> unit.exerciseAgain();
> mock.verifyInteractions();
>
> }
>
> Cheers,
> Szczepan Faber
>

> ...
>
> read more »

[Igor Czechowski]

Hi,

It maybe a little bit of topic, but I have a strong feeling that
Monitor object keeps the state from the moment it's created, whereas
the test tries to verify only part of the state.

Maybe the following code modification would solve your problem,
without a need for reset functionality.

protected void setUp() throws Exception {
super.setUp();
_data = new TestData(this);
_data.clean();

_listener = mock(DirectoryMonitor.Listener.class);
_monitor = new DirectoryMonitor(new
AwkFilenameFilter(REGEX), _listener);
}

and the test case is

public void testFileDeleted() {
_data.createFile(FILE1, "hello");

_monitor.observe(_data.getRoot());
_data.deleteFile(FILE1);
_monitor.poll();

verify(_listener.getMock()).fileDeleted(_data.getFile(FILE1));
verifyNoMoreInteractions(_listener.getMock());
}

Cheers,
Igor

[ErikEngerd]

On Jul 14, 9:42 pm, "Igor Czechowski" <iczechow...@gmail.com> wrote:
> Hi,
>
> It maybe a little bit of topic, but I have a strong feeling that
> Monitor object keeps the state from the moment it's created, whereas
> the test tries to verify only part of the state.

It indeed keeps the state of the directory it is monitoring. It does
that at every poll() call. That way it can determine if something has
changed.

>
> Maybe the following code modification would solve your problem,
> without a need for reset functionality.
>
> protected void setUp() throws Exception {
> super.setUp();
> _data = new TestData(this);
> _data.clean();
> _listener = mock(DirectoryMonitor.Listener.class);
> _monitor = new DirectoryMonitor(new
> AwkFilenameFilter(REGEX), _listener);
> }
>
> and the test case is
>
> public void testFileDeleted() {
> _data.createFile(FILE1, "hello");
>
> _monitor.observe(_data.getRoot());
> _data.deleteFile(FILE1);
> _monitor.poll();
>
> verify(_listener.getMock()).fileDeleted(_data.getFile(FILE1));
> verifyNoMoreInteractions(_listener.getMock());
> }

Now, the monitor does not know which directory it is monitoring so it
cannot detect any changes. In the above example, poll() would have to
be called before the file is deleted. Otherwise, it does not know the
previous state of the observed directory.

Also, extending the design with an observe(File) method to add a
directory (or file) to the list of observed file entries would make
the design much more complicated, giving rise to even more test cases
to test the observe() method. If I would have to implement
functionality like that, I would probably create a new directory
monitor object that delegates to the one I have now (sort of a
composite pattern).

Cheers
Erik

>
> Cheers,
> Igor

> ...
>
> read more »

[szczepiq]

>Someone once gave me an example of the definition of a functional test

Let's leave the discussion what's a functional test :) I just don't
remember using mocks in functional tests - I'm more interested in
system behaviour rather than which method was called on what object.

> I think in general it is good practice to be precise in
> verifications.

or:

>How do I know that only fileDeleted was called
> after the file was deleted and no other method?

By "being precise" you mean being extra-defensive and check for
unexpected methods, right? If that's how you define precision than I
don't think it's a good practice:
http://monkeyisland.pl/2008/07/12/should-i-worry-about-the-unexpected

> In other words, there should not be a rule to avoid
> verifyNoMoreInteractions() but there really *must* be a rule to avoid
> verifying interactions that are irrelevant for the test case at hand.

I really like what you said about avoiding irrelevant interactions.
But I believe that verifyNoMoreInteractions() in most cases is
irrelevant (unless the implementation really does call different
methods that I want to exclude specifically). For example,
verifyNoMoreInteractions() can be relevant when it describes the
conditional logic
(http://groups.google.com/group/mockito/t/a7b6c7632c728e76)

> And I don't see why a reset() in test code would be bad.

Here is why. Instead of this one:

{

File testfile = testdata.createFile(file);
monitor.poll();
verify(listener).fileCreated(testfile);
testfile.delete();

reset(listener);
monitor.poll();
verify(listener).fileDeleted(testfile);
verifyNoMoreInteractions();
}

you could write:

{

File testfile = testdata.createFile(file);
monitor.poll();

verify(listener).fileCreated(testFile);
}

{

File testfile = testdata.createFile(file);
monitor.poll();
testfile.delete();
monitor.poll();
verify(listener).fileDeleted(testfile);
}

Morphing a bigger test into 2 focused tests make your suite *not worse
in quality* but better in readability and maintainability.

I have a feeling that you'd like write the test that:
- checks that delete() is called when file is deleted
- checks that only delete() is called when file is deleted
- checks that delete() is called when: the file is created and then deleted.

What's the true focus of this test? Does checking those 3 things
together make the implementation expose some exceptional scenario? Why
not just splitting it up?

For the sake of the argument, let's say it's truly important to test
all that together. So, what I would do is a handy resetMonitor()
method:

{
File testfile = testdata.createFile(file);
//file creation makes the monitor do nasty things that are irrelevant, so:
resetMonitor();
testfile.delete();
monitor.poll();
verify(listener).fileDeleted(testfile);
verifyNoMoreInteractions();
}

Hope that helps
Cheers,
Szczepan Faber

[ErikEngerd]

On Jul 15, 1:17 pm, szczepiq <szcze...@gmail.com> wrote:
> >Someone once gave me an example of the definition of a functional test
>
> Let's leave the discussion what's a functional test :) I just don't
> remember using mocks in functional tests - I'm more interested in
> system behaviour rather than which method was called on what object.
>
> > I think in general it is good practice to be precise in
> > verifications.
>
> or:
>
> >How do I know that only fileDeleted was called
> > after the file was deleted and no other method?
>
> By "being precise" you mean being extra-defensive and check for
> unexpected methods, right? If that's how you define precision than I
> don't think it's a good practice:http://monkeyisland.pl/2008/07/12/should-i-worry-about-the-unexpected

I don't want to be extra defensive but I just want to verify that the
correct callback method is called on the listener. The link you give
is a good motivation of why other mocking frameworks are not that good
because they force the tester to specify all interactions even if they
are not relevant. Of course, everything is in the eye of the beholder.
What one person thinks is "relevant" to test is not that "relevant" to
sometone else.

>
> > In other words, there should not be a rule to avoid
> > verifyNoMoreInteractions() but there really *must* be a rule to avoid
> > verifying interactions that are irrelevant for the test case at hand.
>
> I really like what you said about avoiding irrelevant interactions.
> But I believe that verifyNoMoreInteractions() in most cases is
> irrelevant (unless the implementation really does call different
> methods that I want to exclude specifically). For example,
> verifyNoMoreInteractions() can be relevant when it describes the
> conditional logic
> (http://groups.google.com/group/mockito/t/a7b6c7632c728e76)

The example I have is an example of conditional logic. In the monitor
code it calls fileCreated() if a file was created, fileDeleted() for
deleted files,and fileModified() for modified files.

>
> > And I don't see why a reset() in test code would be bad.
>
> Here is why. Instead of this one:
>
> {
> File testfile = testdata.createFile(file);
> monitor.poll();
> verify(listener).fileCreated(testfile);
> testfile.delete();
> reset(listener);
> monitor.poll();
> verify(listener).fileDeleted(testfile);
> verifyNoMoreInteractions();
>
> }
>
> you could write:
>
> {
> File testfile = testdata.createFile(file);
> monitor.poll();
> verify(listener).fileCreated(testFile);
>
> }
>
> {
> File testfile = testdata.createFile(file);
> monitor.poll();
> testfile.delete();
> monitor.poll();
> verify(listener).fileDeleted(testfile);
>
> }

Actually, I didn't show you the test code but I already had these two
test cases. Still, in the deletion testcase, I want to verify only the
interactions as a result of the file deletion and I am not interested
in the interactions that occurred before the file was deleted. As I
also said earlier I don't want to modify the design to add a listener
at runtime (subcribing just before deleting the file). This is
because I don't need it and it would required additional testing of
the subscription functionality.

>
> Morphing a bigger test into 2 focused tests make your suite *not worse
> in quality* but better in readability and maintainability.

I already have this.

>
> I have a feeling that you'd like write the test that:
> - checks that delete() is called when file is deleted

Yes

> - checks that only delete() is called when file is deleted

Yes

> - checks that delete() is called when: the file is created and then deleted.

Not really but this comes automatically.

>
> What's the true focus of this test? Does checking those 3 things
> together make the implementation expose some exceptional scenario? Why
> not just splitting it up?

It's already split up.

>
> For the sake of the argument, let's say it's truly important to test
> all that together. So, what I would do is a handy resetMonitor()
> method:
>
> {
> File testfile = testdata.createFile(file);
> //file creation makes the monitor do nasty things that are irrelevant, so:
> resetMonitor();
> testfile.delete();
> monitor.poll();
> verify(listener).fileDeleted(testfile);
> verifyNoMoreInteractions();
>
> }

I don't see what reset monitor would do. The monitor notifies
listeners of file creation, deletion, and modification each time it is
polled. Surely you don't mean implemented a special method for
disabling notifications as that would also require more tests to test
that functionality.

Cheers
Erik

>
> Hope that helps
> Cheers,
> Szczepan Faber
>

> ...
>
> read more »

[szczepiq]

Hi,

Ok I see now why you really want to verifyNoMoreInteractions(). Here
are the two solutions I would do in your scenario (both I find better
than resetting mocks).

#1. addListener()/setListener() method on the Monitor.

>I don't want to modify the design to add a listener
>at runtime (subcribing just before deleting the file). This is
>because I don't need it and it would required additional testing of
>the subscription functionality.

I don't think there is any need for additional testing. addListener()
is like any other setter - it doesn't make sense to test it. It is
already tested implicitly every time you use the Monitor in other
tests. If you want to keep your design then just delegate to
setListener() from the Monitor() constructor.

#2. resetMonitor() (using your code):

resetMonitor() {

_monitor = new DirectoryMonitor(_data.getRoot(), new
AwkFilenameFilter(REGEX), _listener.getProxy());
}

Why I think both solutions are better than resetting mocks? They are
just more natural. They less depend on the mock framework (don't tell
me it does not matter - you did rewrite existing EM test :). They more
depend on the testable design. Look at #1 - this is the classic way we
do things in unit tests - we let the test to set/replace dependencies
(e.g. a listener) to facilitate testing. Actually #2 is quite ugly I'd
go for #1 :)

I see that you did quite a big effort in implementing a resettable
mock. Don't you think that adding setListener() is so much easier and
cleaner way of solving the problem? It's a design for testability.

Let me know what you think about above. I guess I'm just unable
explain why #1 is better than reset(). In this case our view on how we
should test is different and this is perfectly fine. I'm still -1 on
the subject but I will keep your vote in case there are others asking.

Cheers,
Szczepan Faber

[ErikEngerd]

On Jul 16, 12:57 pm, szczepiq <szcze...@gmail.com> wrote:
> Hi,
>

> Ok I see now why you really want to verifyNoMoreInteractions(). Here
> are the two solutions I would do in your scenario (both I find better
> than resetting mocks).
>
> #1. addListener()/setListener() method on the Monitor.
>
> >I don't want to modify the design to add a listener
> >at runtime (subcribing just before deleting the file). This is
> >because I don't need it and it would required additional testing of
> >the subscription functionality.
>
> I don't think there is any need for additional testing. addListener()
> is like any other setter - it doesn't make sense to test it. It is
> already tested implicitly every time you use the Monitor in other
> tests. If you want to keep your design then just delegate to
> setListener() from the Monitor() constructor.

That is a consideration but it still does not solve the general
problem of ignoring interactions that are part of the setup of the
test case. As I mentioned before I don't want to add functionality for
dynamically adding listeners because I don't need it. The design as is
is the simplest design that could work.

>
> #2. resetMonitor() (using your code):
>
> resetMonitor() {
> _monitor = new DirectoryMonitor(_data.getRoot(), new
> AwkFilenameFilter(REGEX), _listener.getProxy());
>
> }

This will not work because it has to poll once for changes and then it
will call the listener's fileCreated() method for every file in the
directory.

>
> Why I think both solutions are better than resetting mocks? They are
> just more natural. They less depend on the mock framework (don't tell
> me it does not matter - you did rewrite existing EM test :). They more
> depend on the testable design. Look at #1 - this is the classic way we
> do things in unit tests - we let the test to set/replace dependencies
> (e.g. a listener) to facilitate testing. Actually #2 is quite ugly I'd
> go for #1 :)

I disagree with this. The design I have is really testabie since I am
passing in the listener at construction and the listener is an
interface. It doesn't have any additional functionality like
dynamically adding listeners because I don't need it. Also, resetting
a mock is natural as well, perhaps we shouldn't call it reset() but
forgetPreviousInteractions(). Reset functionality is useful for
ignoring interactions in the setup of test case that are irrelevant,
and for verifying more complex scenario's.

>
> I see that you did quite a big effort in implementing a resettable
> mock. Don't you think that adding setListener() is so much easier and
> cleaner way of solving the problem? It's a design for testability.

I have another design consideration which is to use immutable objects
as much as possible and as I mentioned I don't need to support
changing the listener at runtime so I am going for the simplest
possible design and it is really well testable as well. There is
nothing in the design that prevents it from being tested to the last
level of detail. It is the lack of reset functionality in mockito
which is getting in the way. Adding setListener() just to make sure
the mockito filosophy of not resetting mocks keeps on working just
sounds like the world turned upside down to me. In concrete, can you
name anything in the design that prevents it from being tested apart
from restrictions in mockito?

The entire implementation from idea to implementing the resettable
mock and using it in the test code cost me about 1 hour, and that
includes reading the javadocs of java.lang.Proxy to understand again
how that worked. I wouldn't call that a lot of work.

>
> Let me know what you think about above. I guess I'm just unable
> explain why #1 is better than reset(). In this case our view on how we
> should test is different and this is perfectly fine. I'm still -1 on
> the subject but I will keep your vote in case there are others asking.

#1 is a valid option but I am not going for that for the reasons I
mentioned.

Perhaps there is another way out of this discussion as well. In an
initial attempt I created a static method mock() in a utility class
that created a new proxy object that wrapped the mockito mock in the
same way that the resettable mock is now wrapping a proxy and a mock.
The advantage of that design was that I only had to deal with a single
object. One to pass to the tested objects, and another to pass to
mockito for verification. Unfortunately, that design did not work
because mockito refused the object because it was not a mock. How
about extensibility of mockito? Wouldn't it be good to be able to
create new mock objects with additional behavior that are still
recognized by mockito?

Cheers
Erik

>
> Cheers,
> Szczepan Faber

> ...
>
> read more »

[szczepiq]

My view on the reset() is still -1. I think your case is very
interesting but there is something I find hard to agree on. You can
solve your problem easily by providing a setter. However, in the name
of "Immaculate Design" you decide not to. For me, design for
testability is fundamental. I'm very happy to change the design if it
lets me write simple, maintainable tests.

You have to understand that it's hard for me to add yet another
feature to the framework because you want to keep your design
pristine.

Can I give you a hint? Try TDD - I guarantee you would not have
similar problems at all. Your design will be discovered by your tests.

Now, let me refer to your post:

>As I mentioned before I don't want to add functionality for
>dynamically adding listeners because I don't need it.

Well... you need it - your test needs it. The test is one of the most
important clients of the code.

>Also, resetting
>a mock is natural as well, perhaps we shouldn't call it reset() but
>forgetPreviousInteractions()

Hmm, interesting. I'll keep this in mind.

>it is really well testable as well. There is
>nothing in the design that prevents it from being tested to the last
>level of detail

We're having this conversation so clearly it's not that testable ;)

>It is the lack of reset functionality in mockito
>which is getting in the way.

My no. 1 rule in unit testing is: "hard to test? never blame testing
tools." Decent code is *always* easily testable using any mocking
framework.

>In concrete, can you
>name anything in the design that prevents it from being tested apart
>from restrictions in mockito?

Hard to answer - for me, it's the code that has restrictions
preventing to test it easily...

>Wouldn't it be good to be able to
>create new mock objects with additional behavior that are still
>recognized by mockito?

I don't think it's easy. Mockito framework obviously works only with
Mockito mocks. Have a go a try to hack it.

Cheers,
Szczepan Faber

[ErikEngerd]

On Jul 19, 1:40 am, szczepiq <szcze...@gmail.com> wrote:
> My view on the reset() is still -1. I think your case is very
> interesting but there is something I find hard to agree on. You can
> solve your problem easily by providing a setter. However, in the name
> of "Immaculate Design" you decide not to. For me, design for
> testability is fundamental. I'm very happy to change the design if it
> lets me write simple, maintainable tests.
>
> You have to understand that it's hard for me to add yet another
> feature to the framework because you want to keep your design
> pristine.

Testability is a major concern for me as well from the start of every
design. Yet simplicity of the design is as well a major concern.

> >it is really well testable as well. There is
> >nothing in the design that prevents it from being tested to the last
> >level of detail
>
> We're having this conversation so clearly it's not that testable ;)

Please give a real argument instead of referring to the fact that we
are having the discussion.

It is very well testable, it is only the migration of the test code to
mockito that gave rise to a few issues that you do not want to solve
to keep your test filosphy and mockito pristine.

>
> >It is the lack of reset functionality in mockito
> >which is getting in the way.
>
> My no. 1 rule in unit testing is: "hard to test? never blame testing
> tools." Decent code is *always* easily testable using any mocking
> framework.

I think testing tools can be blamed as well. No software should be
exempt from criticism.
In your case, the issue is to keep mockito as pristine as possible
And perhaps there are flaws in mockito but, of course, mockito cannot
be blamed because it is a mocking framework.

In my opinion, it is just as bad for a mocking framework to blame test
issues on the code it is testing. It should be a hint to you about
functionality in mockito that is missing if the discussion about
resetting keeps on popping up. Reset functionality is definitely
needed, if only to allow mockito to be used for testing more complex
scenario's and for cases where existing code must be tested that
cannot easily be refactored.

By the way, do we already know what other users think about reset
functionality?

>
> >In concrete, can you
> >name anything in the design that prevents it from being tested apart
> >from restrictions in mockito?
>
> Hard to answer - for me, it's the code that has restrictions
> preventing to test it easily...

But the code does not have any such restrictions!

Cheers
Erik

>
> >Wouldn't it be good to be able to
> >create new mock objects with additional behavior that are still
> >recognized by mockito?
>
> I don't think it's easy. Mockito framework obviously works only with
> Mockito mocks. Have a go a try to hack it.
>
> Cheers,
> Szczepan Faber
>

> ...
>
> read more »

[matte]

I had a look at your example and I think the obvious way to test
"delete file" is to monitor a directory that has a file, delete the
file and verify that the listener was called.

So this code:

> protected void setUp() throws Exception {
> super.setUp();
> _data = new TestData(this);
> _data.clean();
> _listener = new ResettableMock<DirectoryMonitor.Listener>(
> DirectoryMonitor.Listener.class);
> _monitor = new DirectoryMonitor(_data.getRoot(), new
> AwkFilenameFilter(
> REGEX), _listener.getProxy());
> }
>
> and the test case is
>
> public void testFileDeleted() {
> _data.createFile(FILE1, "hello");
> _monitor.poll();
> _listener.reset();
>
> _data.deleteFile(FILE1);
> _monitor.poll();
>
> verify(_listener.getMock()).fileDeleted(_data.getFile(FILE1));
> verifyNoMoreInteractions(_listener.getMock());
> }

Turns in to:

protected void setUp() throws Exception {
super.setUp();
_data = new TestData(this);
_data.clean();

_data.createFile(FILE1, "hello");
_listener = mock(DirectoryMonitor.Listener.class);

_monitor = new DirectoryMonitor(_data.getRoot(), new
AwkFilenameFilter(

REGEX), _listener);
}

public void testFileDeleted() {
_data.deleteFile(FILE1);
_monitor.poll();

verify(_listener).fileDeleted(_data.getFile(FILE1));
verifyNoMoreInteractions(_listener.getMock());
}

No need for reset and no need to change the implementation.

The interface between the DirectoryMonitor and TestData aren't shown
but I would probably have used a mock instead of the TestData class
also.

/Mattias

[szczepiq]

>Please give a real argument instead of referring to the fact that we
>are having the discussion.

What is a 'real argument'? Do you want me to look at your code and
decide whether it is testable or not? The only way to decide if
something is *testable* is to write a test for it. Clearly you cannot
write the test as you like so I consider it a 'real argument' for
untestability. I'd rather not evaluate testability by looking at the
code - I prefer to test it.

>It is very well testable, it is only the migration of the test code to
>mockito that gave rise to a few issues that you do not want to solve
>to keep your test filosphy and mockito pristine.

Very true, I'm really keen on keeping mockito as simple as possible
and this is the philosophy I defend eagerly. However, there were
features I added because we thoroughly investigated user's scenarios.
For example this one:
http://wuetender-junger-mann.de/wordpress/?p=568. So yes, I want to
keep mockito pristine but I'm not that zealous to keep the framework
closed.

>I think testing tools can be blamed as well. No software should be
>exempt from criticism.

I agree to some extent. I wrote mockito because I felt there were
issues with other mocking frameworks (readability, aggressiveness,
etc.). So I was in fact criticizing existing tools. But I wouldn't say
that something is difficult to test *because of* mocking tools. I
still strongly believe that when we discuss untestability it's the
code that is guilty.

>In your case, the issue is to keep mockito as pristine as possible

So it's your pristine design VS my pristine mockito. It's a simple
change of your design VS new controversial feature in the framework
that many people can use and abuse.

>In my opinion, it is just as bad for a mocking framework to blame test
>issues on the code it is testing.

This is where our view on the software writing differs. Untestable? I
blame the code.

>It should be a hint to you about
>functionality in mockito that is missing if the discussion about
>resetting keeps on popping up

It is a hint and I take it seriously. For example, I thoroughly
discussed your example offline with my friends.

>And perhaps there are flaws in mockito but, of course, mockito cannot
>be blamed because it is a mocking framework.

Of course there are flaws in mockito and I wonder what will the
interaction testing look like say in 2 years. Let's get back to your
scenario. You showed me the sources, I analyzed them and I dare to
blame your code, not mockito, for having problems with testing it the
way you want it to test.

>By the way, do we already know what other users think about reset
>functionality?

Go ahead and browse the mailing list. As far as I remember, all of the
users so far agreed that they could refactor the test(or code) and
achieve simpler/more maintainable test without reset()

Probably there is a valid case for reset() out there. IMHO your
scenario is not it. There is nothing wrong in the fact we cannot
agree. Best frameworks, though, are written when people leave off the
features they cannot agree on.

Cheers,
Szczepan Faber

[ErikEngerd]

That is very true, were it not for the fact that the listener is
already registered when the directory monitor is constructed and that
a first poll() has to be done to let the directory monitor notice the
file in the directory. When it notices the file, it calls
fileCreated() on the listener, which is a call I want to ignore. Also,
I really want these fileCreated() callbacks in the first poll() in the
application that I wrote this for.

I could of course extend the design by allowing a subscription
mechanism after construction of the directory monitor. In that case, I
could subscribe the listener after the first poll() and then the test
case would be simple. The only thing is that I do not want to add such
subscription functionality since I don't need it. Having a single
listener subscribed at construction is sufficient.

Given the nature of this simple example, the discussion is of course a
bit academic. However, the general issue is that:
1. sometimes I need to do some initialization for a test case that
leads to interactions that I want to ignore
2. sometimes I want to verify more complex scenario's consisting of
several steps where in each step I want to ignore all interactions of
the previous step.
3. sometimes I want to migrate from another mocking framework to
mockito and I don't want to redesign all my tests just because of
this.

As you can tell from the discussion with Szczepan, it is clear that he
is against supporting this 'reset' functionality in mockito. The
problem with this discussion is that it focuses on a very simple
example which can easily be refactored. However, that is not always
the case. Sometimes, you just want to use a mocking framework to help
you in implementing/improving existing tests and redesign of the code
is not always possible (this usually occurs with commercial software).

>
> The interface between the DirectoryMonitor and TestData aren't shown
> but I would probably have used a mock instead of the TestData class
> also.

The TestData class is nothing more than a utility for manipulating
files in a testcase specific output directory. I need to create files
in a directory for the directory monitor class. The TestData class
just provides some convenient methods for working with these files.

Cheers
Erik

>
> /Mattias

[szczepiq]

>1. sometimes I need to do some initialization for a test case that
>leads to interactions that I want to ignore
>2. sometimes I want to verify more complex scenario's consisting of
>several steps where in each step I want to ignore all interactions of
>the previous step.
>3. sometimes I want to migrate from another mocking framework to
>mockito and I don't want to redesign all my tests just because of
>this.

And sometimes all of those scenarios lead to less maintainable tests
that test too much.

There is no other way than looking at those on the case-by-case basis.
And this is what this mailing list stands for.

>As you can tell from the discussion with Szczepan, it is clear that he
>is against supporting this 'reset' functionality in mockito.

Hopefully you can tell from the discussion that I *just didn't find a
valid case* for reset(). So yes, I'm against controversial features
that don't really bring value (quite the opposite, actually).

>Sometimes, you just want to use a mocking framework to help
>you in implementing/improving existing tests and redesign of the code
>is not always possible (this usually occurs with commercial software).

Maybe. OTOH the willingness to refactor is a virtue I'd like to promote.

Cheers,
Szczepan Faber

>is against supporting this 'reset' functionality in mockito. The
>problem with this discussion is that it focuses on a very simple
>example which can easily be refactored. However, that is not always
>the case. Sometimes, you just want to use a mocking framework to help
>you in implementing/improving existing tests and redesign of the code
>is not always possible (this usually occurs with commercial software).

I'm not against it. I just didn't find a valid case for it.

>The only thing is that I do not want to add such
>subscription functionality since I don't need it.

You need it because your test needs it...

[ErikEngerd]

On Jul 21, 12:27 am, szczepiq <szcze...@gmail.com> wrote:
> >1. sometimes I need to do some initialization for a test case that
> >leads to interactions that I want to ignore
> >2. sometimes I want to verify more complex scenario's consisting of
> >several steps where in each step I want to ignore all interactions of
> >the previous step.
> >3. sometimes I want to migrate from another mocking framework to
> >mockito and I don't want to redesign all my tests just because of
> >this.
>
> And sometimes all of those scenarios lead to less maintainable tests
> that test too much.
>
> There is no other way than looking at those on the case-by-case basis.
> And this is what this mailing list stands for.
>
> >As you can tell from the discussion with Szczepan, it is clear that he
> >is against supporting this 'reset' functionality in mockito.
>
> Hopefully you can tell from the discussion that I *just didn't find a
> valid case* for reset(). So yes, I'm against controversial features
> that don't really bring value (quite the opposite, actually).

At least, there is also the workaround with the resettable mock.

>
> >Sometimes, you just want to use a mocking framework to help
> >you in implementing/improving existing tests and redesign of the code
> >is not always possible (this usually occurs with commercial software).
>
> Maybe. OTOH the willingness to refactor is a virtue I'd like to promote.

Refactoring is also something I am promoting. Note that I just used
this example to illustrate a point. I refactor a lot for testability
but in this particular case I think testability was already good so
wanted to solve this problem in the test code.
Our fundamental difference of opinion lies in the weight we give to
design and test. In my opinion the design should be well testable but
it is not the aim of the design to make testing as easy as possible
because there are also design concerns that are really important.

I once heard the following analogy of software development to a car.
If design/coding is the engine of the car then test is the headlights
of the car. Test is only a measuring device for quality. The final
stability of the system that is being developed also depends a lot on
general design principles. That is why I sometime go a little bit out
of my way in the test so that I can keep enforcing good design
principles.

Let's just rest the discussion for now. Enough has been said about
this.

>
> Cheers,
> Szczepan Faber
>
> >is against supporting this 'reset' functionality in mockito. The
> >problem with this discussion is that it focuses on a very simple
> >example which can easily be refactored. However, that is not always
> >the case. Sometimes, you just want to use a mocking framework to help
> >you in implementing/improving existing tests and redesign of the code
> >is not always possible (this usually occurs with commercial software).
>
> I'm not against it. I just didn't find a valid case for it.
>
> >The only thing is that I do not want to add such
> >subscription functionality since I don't need it.
>
> You need it because your test needs it...
>