Allow specifying the timeout for runTest
#3603
Conversation
Deprecate `dispatchTimeoutMs`, as this is a confusing implementation detail that made it to the final API. We use the fact that the `runTest(Duration)` overload was never published, so we can reuse it to have the `Duration` mean the whole-test timeout in a backward-compatible manner.
|
Regarding the two changes (timeout + experimentality markers) for the price of one: I'll split the PR in two if it makes it easier to review this. |
Please squash commits into two prior to merge and mention the corresponding |
dkhalanskyjb
force-pushed
the
coroutines-test-timeout
branch
from
5bca4b5 to
9fd86bb
Compare
| ) | ||
| ): TestResult { | ||
| if (context[RunningInRunTest] != null) | ||
| throw IllegalStateException("Calls to `runTest` can't be nested. Please read the docs on `TestResult` for details.") |
There was a problem hiding this comment.
Out of curiosity: is it better/more readable than check(context[RunningInRunTest] != null)) { ..message.. }?
There was a problem hiding this comment.
Not really. I think I wrote it this way to be able to place a breakpoint on throw, but one can do it with check also:
check(...) {
"message" // breakpoint here
}There was a problem hiding this comment.
(for the future) IDEA allows you to place a breakpoint into a specific part of such expressions:
There was a problem hiding this comment.
In theory, yes, I just struggle to make it work sometimes. Line breakpoints are much more reliable.
| var timeoutError: Throwable? = null | ||
| var cancellationException: CancellationException? = null | ||
| try { | ||
| withTimeout(timeout) { |
There was a problem hiding this comment.
Is withTimeout guaranteed to only throw after the invokeOnCompletion handler has finished its work?
There was a problem hiding this comment.
Yes, all intermediate handlers are invoked synchronously, thus making it impossible for coroutine to finalize its state (-> throw or return to the caller)
dkhalanskyjb
force-pushed
the
coroutines-test-timeout
branch
from
9da1fcf to
4fe30ec
Compare
Deprecate
dispatchTimeoutMs, as this is a confusing implementation detail that made it to the final API.We use the fact that the
runTest(Duration)overload was never published, so we can reuse it to have theDurationmean the whole-test timeout in a backward-compatible manner.Fixes #3270