mockito / mockito-kotlin

Using Mockito with Kotlin
MIT License
3.11k stars 202 forks source link

What about improving doReturn -> when implementation? #453

Closed htafoyag closed 1 year ago

htafoyag commented 2 years ago

I believe there's a missed opportunity on the doReturn.when implementation. Mockito kotlin is only changing reseved word 'when'with whenever, but the implementation is not on par with the rest of the library.

When using on{ myMethod() } doReturn "something", myMethod is inferred from the mock scope. However the doReturn is not doing the same and we need to resend the mock to whenever method:

val myMock = mock<MyClass> {
    doReturn("something").whenever(myMock).myMethod()
}

A better format would be:

val myMock = mock<MyClass> {
    doReturn("something").whenever(myMethod())
}

or even a format similar to the ones already there:

val myMock = mock<MyClass> {
    doReturn "something" whenever { myMethod() }
}

The same for doNothing and other similar methods

Or by a fact, i would've preferred that on - doReturn called doReturn.when instead, I see no purpose on invoking the real methods and the need to use a different method for spies.

thecodewarrior commented 1 year ago

The problem with on { myMethod() } doReturn "something" using the doReturn("something").when(mock).myMethod() internally is that there's no way to tell when the mock is "done"

At no point in the chain do we know that we're "done" and can wrap it up.

val myMock = mock<MyClass> {
    on { myMethod() }  doReturn "first" doReturn "second" doThrow IllegalStateException()
}

Technically we could store the current stub in the KStubbing and close the stubbing the next time on is called, and again at the end of the block, but that seems a bit iffy to me.

I would propose a different syntax, which provides symmetry with the existing implementation. It would be extremely easy to implement as well.

val myMock = mock<MyClass> {
    doReturn("first").on { myMethod() }
}

I went with doReturn(...).on { ... } instead of doReturn(...) on { ... } because of the difference in how they're chained. With the on { ... } doReturn syntax, everything is chained as infix functions, however with this syntax you have to chain with dot syntax.

val myMock = mock<MyClass> {
    // consistent dot notation
    doReturn("first").doReturn("second").doThrow(IllegalStateException()).on { myMethod() }
    // no dot notation is inconsistent and awkward after chaining
    doReturn("first").doReturn("second").doThrow(IllegalStateException()) on { myMethod() }
}

Implementing it would just require one extra method in KStubbing:

fun Stubber.on(methodCall: T.() -> Unit) {
    this.`when`(mock).methodCall()
}

That method wouldn't provide type safety, but we could create a KStubber<T> class that does offer that. However, while that will provide type safety, it won't provide type inference, since you do the .on { myMethod() } at the end.

We could provide type safety and type inference given a different syntax, but it's awkward.

val myMock = mock<MyClass> {
    on({ myMethod() }) {
        doReturn("first")
        doReturn("second")
        doThrow(IllegalStateException())
    }
}

There are probably other possible syntaxes, but these are the ones I can think of