Is a `suspend` version of `shouldThrow` missing fo...

# kotest

[Oliver.O]

Is a suspend version of shouldThrow missing for a reason or should I file an issue?

[sam]

Is shouldThrow inline ?

[LeoColman]

I think it is, yes

[LeoColman]

We had some issues with that a lot of time ago, in which x = y couldn't be inlined by our function, and then we added shouldThrowUnit. I believe all of this is inline

[sam]

If it's inline then it will work for suspend too

[Oliver.O]

Indeed it is, I'll investigate further.

[Oliver.O]

Still investigating... Had to deal with this one first:
https://github.com/kotest/kotest/issues/2447

[sam]

Nice find. I'd like to introduce coroutine based versions of these for 5.0

[Jim]

Ah, y'all are in here

[Jim]

I'm thinking TestExecutionFilter that installs the necessary stuff for withClue. Then deprecate the old ones and make withClue a suspending extension function on TestContext

[Oliver.O]

Hmm, in that case withClue would no longer be usable with the standalone assertions-package, (i.e. without the Kotest framework), right?

[Jim]

Hmmm, we could have a version that is an extension on CoroutineContext too that just hooks up the necessary state if it hasn’t been set up in the TestExecutionFilter

[Oliver.O]

I had just envisioned something like this:

private val _errorCollector = ThreadLocal<ThreadLocalErrorCollector>()
val errorCollector: ErrorCollector get() = _errorCollector.get()
    
suspend fun withErrorCollector(thunk: suspend () -> R) = withContext(_errorCollector.asContextElement()) { thunk() }

suspend fun <R> withClue(clue: Any?, thunk: suspend () -> R): R {
    try {
        errorCollector.pushClue { clue.toString() }
        return withErrorCollector { thunk() }
    } finally {
        errorCollector.popClue()
    }
}

[Jim]

well that is simpler 🤣

[Jim]

I’ll have to think about how that works across multiple withClue, any, all, and one

[Oliver.O]

Yes, the above is pretty preliminary stuff. It also needs some additional thinking on how to bridge coroutines and non-coroutines land.

[Jim]

The current implementation seems fine for non-coroutines land I think, right? So probably dual implementations

[Oliver.O]

Yes, in an ideal universe, the same invocation would work in both worlds. However, the compiler does not like type inference based on suspend differences alone:

suspend fun <R> withClue(clue: Any?, thunk: suspend () -> R): R

fun <R> withClue(clue: Any?, thunk: () -> R): R

In this case, it would complain on the non-suspending call site:

Overload resolution ambiguity

This is https://youtrack.jetbrains.com/issue/KT-23610.

Another point: You start in non-coroutine land, invoke withClue (or assertSoftly) there, then start coroutines which might switch threads. You'd probably have to make the ThreadLocalErrorCollector from non-coroutine land available to coroutines.