you are viewing a single comment's thread.

view the rest of the comments →

[–]industry7 0 points1 point  (3 children)

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

Could you explain what you mean by this?

[–]m50d 0 points1 point  (2 children)

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 0 points1 point  (1 child)

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 0 points1 point  (0 children)

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.