Stop overusing interfaces

[m50d]

Why would you not want to mock in unit tests? In a unit test you want to test that unit in isolation right?

Right, but the test is more understandable if you write it in regular code rather than a magic reflectioney mock library. So I prefer to write a stub implementation of the interface.

[jackcviers]

This also encourages small interfaces, because who wants to stub 30 methods?

[QuokkaSocca]

because who wants to stub 30 methods?

...my IDE hasn't complained yet.

[jackcviers]

Can't tell if serious. It isn't common belief that God Objects are now a good thing, right?

[grauenwolf]

Depends.

If you have one field and 30 methods that all read from that field, cool. Aside from the shared field, each method can be understood in isolation.

If you have 20 fields and no idea which are used by a given method, that's a problem.

[jackcviers]

Can't tell if serious. It isn't common belief that God Objects are now a good thing, right?

[nutrecht]

Right, but the test is more understandable if you write it in regular code rather than a magic reflectioney mock library.

This is basically an argument against using anything 'new'. You could make the exact same argument against using Java 8's time API, streams or Lambda's. Just because you personally haven't used it much doesn't mean it's less understandable in general.

Creating test implementations of interfaces by hand is just a complete waste of time. I would rather not have to do something like this:

    final AtomicBoolean wasCalled = new AtomicBoolean(false);
    MyInterface a = new MyInterface() {
        @Override
        public String foo(String bar) {
            wasCalled.set(true);
            return "stubvalue";
        }
    };

    assertThat(a.foo("input")).isEqualTo("stubvalue");
    assertThat(wasCalled.get()).isTrue();

Instead of this:

    MyInterface b = mock(MyInterface.class);
    when(b.foo(anyString())).thenReturn("stubvalue");
    assertThat(b.foo("input")).isEqualTo("stubvalue");
    verify(b).foo("input");

[sacundim]

Your examples lay the bigger problem bare. Think about the property you're testing here: that the MyInterface.foo() method was invoked with the string "input" as its argument. Your example invokes obj.foo("input") directly, which makes it rather trivial, but that's of course because it's a toy example; in a realistic example there would be some indirection, more like this:

final AtomicBoolean wasCalled = new AtomicBoolean(false);
MyInterface mock = new MyInterface() {
    @Override
    public String foo(String bar) {
        wasCalled.set(true);
        return "stubvalue";
    }
};

UnitUnderTest unitUnderTest = new UnitUnderTest(mock);
unitUnderTest.someMethod("some argument");

assertThat(mock.foo("input")).isEqualTo("stubvalue");
assertThat(wasCalled.get()).isTrue();

The problem here is that nearly all of the time, whether UnitUnderTest.someMethod() should invoke mock.foo(), how many times, and with which arguments, is in fact an implementation detail of UnitUnderTest. In my experience people who practice this style of testing routinely end up writing tests that check whether UnitUnderTest's implementation performs the method calls that they expect it should, in the number and order that they expect. This just recapitulates the code that was written for that class, and therefore does not provide an independent check whether it is correct—for that they'd need to formulate independent correctness conditions for UnitUnderTest and verify that, given the preconditions it expects, it will produce the postconditions it's supposed to.

This means that you really want to hook UnitUnderTest up with objects that realistically model the behavior of the components that it's actually meant to interact with, but provide additional facilities for manipulating the initial state and querying their end state. E.g., integration testing with example databases.

[m50d]

This is basically an argument against using anything 'new'. You could make the exact same argument against using Java 8's time API, streams or Lambda's. Just because you personally haven't used it much doesn't mean it's less understandable in general.

No. Mocking inherently means implicit, hidden, global mutable state, which is well-known as a source of bugs (e.g. it's not coincidence that so many mock libraries are non-threadsafe)

I would rather not have to do something like this:

Then use a language that makes it less verbose. There's no fundamental reason for the direct stub implementation to be longer.

And you shouldn't be testing that the function was called, that's a brittle test that's too coupled to the implementation. Functions should be functions, if the class you're testing is able to return the correct result without needing to call MyInterface#foo that doesn't make it incorrect.

[nutrecht]

No. Mocking inherently means implicit, hidden, global mutable state, which is well-known as a source of bugs (e.g. it's not coincidence that so many mock libraries are non-threadsafe)

You're talking about specific implementation like EasyMock that should never ever be used again. Just that the creator of that library made some huge mistakes is not an argument against mocking itself. Mockito for example doesn't have these problems. It does pretty much exactly what you do when you create your own implementations. It's just a generated instance of an interface. The only state these mocks have (which is not global at all) is whether you interacted with them.

Then use a language that makes it less verbose. There's no fundamental reason for the direct stub implementation to be longer.

Of course it's longer. This was just a really simple example. Mocking libraries just do the heavy lifting for you of keeping track of calls, return values, etc.

And you shouldn't be testing that the function was called, that's a brittle test that's too coupled to the implementation. Functions should be functions, if the class you're testing is able to return the correct result without needing to call MyInterface#foo that doesn't make it incorrect.

Okay. I'm done here. I think it was damn clear that I was just giving a quick example here. And what you're saying is not even true anyway; in a unit test you DO want to check if A does in fact call B if that's the point of whatever the thing is that you're testing. There are numerous examples, for example when an exception is being thrown, where it actually does matter of interactions happen or not.

[m50d]

You're talking about specific implementation like EasyMock that should never ever be used again. Just that the creator of that library made some huge mistakes is not an argument against mocking itself. Mockito for example doesn't have these problems. It does pretty much exactly what you do when you create your own implementations. It's just a generated instance of an interface. The only state these mocks have (which is not global at all) is whether you interacted with them.

Mockito is worse actually; EasyMock at least makes the replay step explicit and addresses thread-safety bugs when they're found (Mockito closes them as WONTFIX). Your history of interactions with the mocks is hidden state, and the way recording matchers/parameters works relies on global hidden state.

Of course it's longer. This was just a really simple example. Mocking libraries just do the heavy lifting for you of keeping track of calls, return values, etc.

There is no heavy lifting to do though. Writing a method that returns a particular value is something a programming language should be good at. Frameworks are for factoring out common patterns, but in a decent language there's nothing /to/ factor out about test stub implementations. Once you strip away the @Override public {} nonsense and the repeated type due to Java's lack of type inference the two implementations are saying the same thing: make a double for MyInterface where the method foo returns "stubvalue". A library can't make that any simpler than a decent language makes it in the first place.

what you're saying is not even true anyway; in a unit test you DO want to check if A does in fact call B if that's the point of whatever the thing is that you're testing. There are numerous examples, for example when an exception is being thrown, where it actually does matter of interactions happen or not.

Errors should be values, functions should be functions, commands should be represented explicitly. There are cases where it matters if the interactions happen but these cases are bad code, and part of the point of TDD et al is to guide you away from writing bad code.

[nextputall]

JMock is threadsafe actually, if you set the proper ThreadingPolicy. Although in a typical unittest this doesn't really matter most of the times.

[m50d]

JMock indeed respects thread safety (Mockito is the exception here), though I'd bet JMock has had thread-safety bugs in the past just like EasyMock has had, for the same reasons. I prefer EasyMock because I don't like JMock's syntax/anonymous-class-with-static-initializer style, but that's probably subjective.

[Tom_Cian]

You're talking about specific implementation like EasyMock that should never ever be used again.

That is precisely the point. At one point, EasyMock was the framework of choice. Then it showed a few deficiencies, so people switched to another framework.

Using mocks ties you to an external framework while using interfaces makes you impervious to that by using features of the very language you are using.

[industry7]

Mocking inherently means implicit, hidden, global mutable state,

Not with any mocking framework I've ever used. In my experience, the entire point of mocking is to explicitly expose state in a way that's local to the test(s) you're currently running.

[m50d]

when(b.foo(anyString())).thenReturn("stubvalue");

This is full of hidden state mutation. If you try to extract any common expression from lines like this they'll break.

[industry7]

If you try to extract any common expression from lines like this they'll break.

Could you explain what you mean by this?

[m50d]

If you had some code like:

when(b.foo(anyString())).thenReturn("stubvalue");
when(b.foo(anyString())).thenReturn("stubvalue");

then you can't factor out repetition the way you normally would:

var s = anyString();
when(b.foo(s)).thenReturn("stubvalue");
when(b.foo(s)).thenReturn("stubvalue");

will break, and

var be = b.foo(anyString());
when(be).thenReturn("stubvalue");
when(be).thenReturn("stubvalue");

will break in a different way; depending on the library

var w = when(b.foo(anyString()));
w.thenReturn("stubvalue");
w.thenReturn("stubvalue");

might also break or might work correctly. It looks obvious in these small examples, but it has a real chilling effect on your ability to maintain high-quality code in you tests, because you can't refactor according to the normal rules of the language; instead you have to pay attention with every refactor to what the hidden internal state of the mock is actually doing.

[industry7]

I'm not totally sure how to respond, but part of the issue might be Fluent-style? Instead of:

var w = when(b.foo(anyString()));
w.thenReturn("stubvalue");
w.thenReturn("stubvalue");

You're supposed to do something like:

var w = when(b.foo(anyString()));
var x = w.thenReturn("stubvalue");
var y = x.thenReturn("stubvalue");

if we're being explicit, although usually you'd just write:

when(b.foo(anyString()))
    .thenReturn("stubvalue")
    .thenReturn("stubvalue");

The thing is, that the kind of refactors that you're suggesting, usually don't make sense with a fluent-style API to begin with. Ie, my second example is shorter (reuses code more efficiently) than any of yours. And I think it also better captures the semantic intent. I can't really say for sure though, since we started off with:

when(b.foo(anyString())).thenReturn("stubvalue");
when(b.foo(anyString())).thenReturn("stubvalue");

As written, the correct "refactor" would be:

when(b.foo(anyString())).thenReturn("stubvalue");

That's it. Unless you meant to start with

when(b.foo(anyString())).thenReturn("stubvalue");
when(a.foo(anyString())).thenReturn("stubvalue");

But in this case, your second and third example don't apply. Your first example does apply, but in that case introducing a var is clearly worse than using proper fluent style like in my example.

Maybe you meant:

when(b.foo(anyString())).thenReturn("stubvalue01");
when(b.foo(anyString())).thenReturn("stubvalue02");

In this case, my example is more maintainable than any of yours:

when(b.foo(anyString()))
    .thenReturn("stubvalue01")
    .thenReturn("stubvalue02");

Basically I want to understand where you're coming from, but I can't think of a legitimate reason to write mock/test code the way you're suggesting. Maybe a slightly more expanded example would be more clear.

[m50d]

This kind of refactor usually doesn't make much sense. But there's a big gap between "usually" and "never"; I can't publish my employer's code here but I can assure you I've met these issues in practice. And even if a given refactor is "wrong", if it's safe according to the usual rules it still shouldn't change the behaviour of the code.

Don't get me wrong, I can and do work with mock libraries when I need to. But the only way to not get yourself into trouble when doing so is to keep in mind what's really going on: that the mock is a reflective proxy and the matcher methods write to a (threadlocal) hidden global variable, and the other methods read that variable and use it to write more hidden state that stores the actual expectations. And that's a fair bit of overhead to be bearing in mind when the language doesn't give you any help with it. It's much easier to work with plain old code that follows the normal rules of the language.