Add suspend answer; Fix #346

[neworld]

This code was inspired by:

• Java wrapper part: https://github.com/square/retrofit/blob/8c93b59dbc57841959f5237cb141ce0b3c18b778/retrofit/src/main/java/retrofit2/HttpServiceMethod.java#L168
• Kotlin wrapper part: https://github.com/square/retrofit/blob/8c93b59dbc57841959f5237cb141ce0b3c18b778/retrofit/src/main/java/retrofit2/KotlinExtensions.kt#L83-L98

I used Java code because kotlin does not allow call suspend functions or lambdas by giving own Continuation. But Kotlin does it under the hood, so Java could do it as well. For example:

fun foo(continuation: Continuation) {
  bar(continuation) //compilation error
}

suspend fun bar() {

}  

[nhaarman]

Thanks! Looks like a useful feature :raised_hands:

I'm not that familiar with coroutines yet, does this solution rely on any undocumented Kotlin internals? I.e. is there any chance this may break in upcoming Kotlin versions, and if so why?

[neworld]

Yes, it is. I am using it for some tests; otherwise, I was not able to write at all.

does this solution rely on any undocumented Kotlin internals?

I would say yes. Implementation details are not documented well.

is there any chance this may break in upcoming Kotlin versions

I believe this feature is quite safe:

1. The way suspend functions are working is compiler feature to add Continuation argument to all suspend functions. Kotlin devs should ensure backward compatibility. Otherwise, at some point, you could not upgrade kotlinc until all dependencies are rebuilt with a new compiler.
2. Retrofit is using the same approach. They don't need weird java wrapper only because their code is in java already.

I suppose there is a little chance the riskiest line could not compile with future version.

[neworld]

Created an issue: https://youtrack.jetbrains.com/issue/KT-33766

[neworld]

@nhaarman, friendly ping. Any news for this solution? At Vinted we are using this solution for multiple cases and they are working well.

[neworld]

Thanks to Roman Elizarov. He provided me with missing details and I was able to find a solution without Java.

Also added some tests to make sure the cases I am facing in my personal code will be covered as well.

[JustinBis]

Ping @nhaarman, I'd love to use this in my projects as well as an official feature. Thanks @neworld for the awesome PR.

[connected-rmcleod]

@nhaarman Adding to the friendly pinging. Code like this is fundamentally difficult to test without a suspending answer:

class ViewModel(private val api: API) {
  var isLoading: Boolean = false

  suspend loadData(): Data {
    isLoading = true
    val data = api.fetchData() // suspending call
    isLoading = false
    return data
  }
}

If I can't stub fetchData in such a way that it doesn't immediately return, it's pretty much impossible for me to write a passing test asserting that isLoading is ever true. In fact, the only way I could figure out how to do it was by copying and pasting the code from this PR into my code (or to use something other than Mockito.)

I will add that while this PR uses low-level code, it is not marked experimental (it was briefly in an experimental package at one point but quickly moved back) and it is in the documentation: https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.coroutines.intrinsics/start-coroutine-unintercepted-or-return.html

It was also the approach suggested by the team lead for Kotlin libraries in the YT issue: https://youtrack.jetbrains.com/issue/KT-33766

Would be happy to help if there's any way I can assist in getting @neworld's excellent work into a mergeable state.

[mhernand40]

FWIW, in case anyone feels blocked because of this limitation, I have actually begun testing without Mockito whenever possible by leveraging interfaces and this "fake" constructor pattern I have seen being used in many well known Kotlin libraries including the Kotlinx Coroutines library. Using the fetchData() example mentioned above by @connected-rmcleod:

interface Api {
    suspend fetchData(): Data

    companion object {
        operator fun invoke(... /* dependencies */): Api {
            return ApiImpl(...)
        }
    }
}

private class ApiImpl(... /* dependencies */) {
    override suspend fetchData(): Data {
        // Implementation details here
    }
}

Or if you don't like the companion object allocation:

@Suppress("FunctionName")
fun Api(... /* dependencies */): Api {
    return ApiImpl(...)
}

interface Api {
    suspend fetchData(): Data
}

private class ApiImpl(... /* dependencies */) {
    override suspend fetchData(): Data {
        // Implementation details here
    }
}

In code, I can still "instantiate" Api using val api = Api(...) and in tests I can create an inner TestApi and back it with a CompletableDeferred like so:

    private val deferredData = CompletableDeferred<Data>()
    private val api = TestApi()

    ...

    private inner class TestApi() : Api {
        override suspend fetchData(): Data {
            return deferredData.await()
        }
    }

If I want to test the scenario where the fetch suspends indefinitely, I simply avoid calling deferredData.complete(...). If I want the fetch to return a result immediately, I simply call deferredData.complete(expectedData).

I get that this doesn't actually address the need for supporting suspend Answers in Mockito Kotlin but I figured I'd share how I have adapted my testing practices to some of the limitations. 🙂

Examples of this "fake" constructor pattern: https://kotlin.github.io/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines/-coroutine-scope.html https://kotlin.github.io/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines/-job.html https://kotlin.github.io/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines/-supervisor-job.html https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.collections/-list.html

Notice how these are actually funs.

[neworld]

Actually you do not need to wait for the release. Thanks to kotlin extensions you could just copy-paste into your project:

/**
 * This should be replaced after the release of https://github.com/nhaarman/mockito-kotlin/pull/357
 */
@Suppress("UNCHECKED_CAST")
infix fun <T> OngoingStubbing<T>.willAnswer(answer: suspend (InvocationOnMock) -> T?): OngoingStubbing<T> {
    return thenAnswer {
        //all suspend functions/lambdas has Continuation as the last argument.
        //InvocationOnMock does not see last argument
        val rawInvocation = it as InterceptedInvocation
        val continuation = rawInvocation.rawArguments.last() as Continuation<T?>

        answer.startCoroutineUninterceptedOrReturn(it, continuation)
    }
}

I am using the same snippet for a year and works perfectly.

[connected-rmcleod]

@mhernand40 @neworld All good advice! I'm aware that I can just roll my own test doubles or copy in the extension function from this PR. (In fact, in a lot of ways I prefer not using a mocking framework at all, but I've never found a team that agrees with me. xD) I'm just trying to emphasize that if someone is trying to write tests using mockito-kotlin and runs into this issue that it's a substantial stumbling block for them without having to resort to a different approach entirely.

Also, as an aside I had never seen that pattern of essentially adding a default constructor to an interface before. That's pretty neat.

I came across this mostly because I was going to give a small presentation to the Android devs at my company about the differences between using Mockito + mockito-kotlin vs MockK and this stuck out as a substantial thing that MockK can do much more easily.

[neworld]

To be honest, recently I am trying to avoid mocking my own code at all. The topic was discussed many times and there is plenty of good arguments. The exception is for 3rd party libraries I have no control.

[connected-rmcleod]

@neworld Without getting too much into the weeds, I think the correct position is between those two extremes, but I agree the far more common mistake is over-mocking. Over-mocking is a cancer.

[connected-rmcleod]

This PR has been open for a long time. The approach also seems quite stable, and would be a pretty useful addition. Is there a substantial reason not to bring it in?

[neworld]

@nhaarman, friendly ping. More than a year has passed. Is any blocker for this PR? It seems quite useful for the community and the same code is working like a charm in our production environment.

[TimvdLippe]

The mockito-kotlin artifact recently moved to the GitHub Mockito organization and we have been busy at work with publishing to Maven Central. Now that the dust mostly has settled, we can take a look at community PRs 😄 I will put this on my review list for this week, but I will have to get accustomed to the code a bit to understand what is going on here. From a first glance this looks fine, but I just want to make sure I understand what the overall logic is.

[TimvdLippe]

@neworld Do you mind rebasing this PR so that I can review a clean integration? I think the only blockers are import updates.

[neworld]

Finished rebasing.

I just want to make sure I understand what the overall logic is

Suspend function under the hood returns Any object which could be COROUTINE_SUSPENDED in case of suspension. AFAIK all coroutines are started via startCoroutineUninterceptedOrReturn or similar primitives, which properly handles the return of the suspendable body. This block will be executed by another coroutine so it is important to return proper result as coroutines are expected:

        //all suspend functions/lambdas has Continuation as the last argument.
        //InvocationOnMock does not see last argument
        val rawInvocation = it as InterceptedInvocation
        val continuation = rawInvocation.rawArguments.last() as Continuation<T?>

        answer.startCoroutineUninterceptedOrReturn(it, continuation)

More professional explanation is here: https://youtrack.jetbrains.com/issue/KT-33766#focus=Comments-27-3707299.0-0

[TimvdLippe]

This feature is available per version 3.1.0: https://repo1.maven.org/maven2/org/mockito/kotlin/mockito-kotlin/3.1.0/