Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: safer expectations in idiomatic syntax #198

Closed
einholen opened this issue Feb 5, 2020 · 6 comments
Closed

Proposal: safer expectations in idiomatic syntax #198

einholen opened this issue Feb 5, 2020 · 6 comments

Comments

@einholen
Copy link
Contributor

einholen commented Feb 5, 2020

Hi!

I've grown a bit frustrated by expectations at the end of the line, methodCall wasCalled once, because when method signature is long the reader can lose context before they read until wasCalled once, and worse, in the case of long signatures, it's so easy to overlook adding (or drop) the verification completely. When verification is missing, the test will happily pass without complaining about anything.
Additionally, when multiple verifications follow each another, they don't line up as well as they used to, because actual verification clauses are no longer positioned one after another.

It wasn't the case with vanilla mockito, where expectation comes first (verify(mock).foo(any)).

Strict mode solves the problem partially, but it isn't always an option when migrating large projects from older versions of mockito: it's physically impossible to bring thousands of mocks that have been stubbed for multiple purposes into strict mode.

To help keep verifications safe even in lenient mode, I propose replacing single-step verifications with a macro-based two-step verification.

Instead of

val myMock1 = mock[Service1]
val myMock2 = mock[Service2]

// call tested method

myMock.foo(*, *, *) wasCalled once
myMock2.bar("hello", *, *) wasCalled twice

we'd do this

val myMock1 = mock[Service1]
val myMock2 = mock[Service2]

// call tested method

expect {
  myMock.foo(*, *, *) wasCalled once
  myMock2.bar("hello", *, *) wasCalled twice
}

The entire verification section of the test should consist of expect block that would use a macro to fail compilation for every line within the block that is not a verification.

An alternative approach would be to add idiomatic syntax resembling specs2-mockito:

there was one(myMock).foo(*, *, *)
there were two(myMock2).bar("hello", *, *)

or scalamock-style

(myMock.foo _).expect(*, *, *)

What do you think?

@ultrasecreth
Copy link
Member

Hey, thanks for the feedback

I think is a valid concern, I'd lean towards a syntax similar to the specs2-mockito, but a bit more fluent if possible (I don't like wrapping the mock and calling the method on the result)

Thing is, it would have to go with a new stubbing syntax, one of the reasons of having the verification in the end is that you can easily copy/paste stbbings and turn them on verifications and vice versa, which is something that is harder on the original syntax and used to annoy me quite a bit.

So what I'd say is that a new IdiomaticSyntax2 (will find a better name) should be created and it should support all the same features the current one does, but with some way to specify the mock call at the end rather than at the beggining.

Thoughts?

@einholen
Copy link
Contributor Author

einholen commented Feb 5, 2020

IdiomaticSyntax2 sounds good to me 😄

Very good point about the ease of copying mocks to expectations. I guess we could get the best of both worlds by going for something like

expect one myMock.foo(*, *, *)
expect.two(myMock2.bar("hello", *, *))

@ultrasecreth
Copy link
Member

What about stubbing?

@einholen
Copy link
Contributor Author

einholen commented Feb 6, 2020

Well, personally I think the way stubbing works now is cool. It's harder to screw up in general because if you lose a postfix for a stub, your test is more likely to fail than if you lose a verification statement

@ultrasecreth
Copy link
Member

Ok, we can do verification as a first step.
I'm quite busy at the moment, but will take a look as soon as I finish off some other stuff.

Feel free to raise a PR if you feel like doing it yourself :)

einholen added a commit to einholen/mockito-scala that referenced this issue Feb 29, 2020
…o#198

With existing postfix verifications, it is easy to overlook a missing verification when method signatures are long, so "was called" statements can be hard to spot - and unfortunately, in a lot of cases it means that a test that should be failing starts to pass.
New DSL helps with this situation by sort of going back to the roots of vanilla Mockito, where "verify" word is defined before the verified method signature. Macros help us solve the inconvenience of having to wrap the mock variable with parentheses.

Better naming wanted for IdiomaticMockito2 :)
@einholen
Copy link
Contributor Author

Okay, I gave it a stab - please take a look when you have time!

ultrasecreth added a commit that referenced this issue Mar 16, 2020
New "expect" DSL promoting safer prefixed verifications, fixes #198
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants