Make UncompletedTestCoroutines more verbose #3071
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
qwwdfsad
reviewed
Dec 8, 2021
| @@ -40,6 +40,7 @@ internal expect suspend inline fun recoverAndThrow(exception: Throwable): Nothin | |||
| * The opposite of [recoverStackTrace]. | |||
| * It is guaranteed that `unwrap(recoverStackTrace(e)) === e` | |||
| */ | |||
| @PublishedApi | |||
There was a problem hiding this comment.
Please leave a comment that it is only used from other module tests, so when the time comes we know it can be safely deleted/reverted
| val completionCause = try { | ||
| coroutine.tryGetCompletionCause().also { completing = true } | ||
| } catch (e: Throwable) { | ||
| completing = false |
There was a problem hiding this comment.
instead of try-catch, it should be enough to check coroutine.isCancelled.
The same goes for completing
| throw UncompletedCoroutinesError("The test coroutine was not completed after waiting for $dispatchTimeoutMs ms") | ||
| var completing: Boolean | ||
| val completionCause = try { | ||
| coroutine.tryGetCompletionCause().also { completing = true } |
There was a problem hiding this comment.
Suggested change
| coroutine.tryGetCompletionCause().also { completing = true } | |
| // Protected JobSupport.completionCause lifted in TestCoroutineScope and TestScope | |
| coroutine.tryGetCompletionCause().also { completing = true } |
| completing = false | ||
| null | ||
| } | ||
| var message = "After waiting for $dispatchTimeoutMs ms" |
There was a problem hiding this comment.
It would be nice to follow our overall approach and extract slow-path to a separate method
There was a problem hiding this comment.
But this whole code fragment is the slow path, don't you agree?
There was a problem hiding this comment.
Agree, then maybe it's worth extracting the whole onTimeout body?
qwwdfsad
approved these changes
Dec 9, 2021
dkhalanskyjb
deleted the
coroutines-test-verbose-uncompleted-coroutines
branch
woainikk
pushed a commit
that referenced
this pull request
Dec 14, 2021
* The error is more verbose: it lists the active children. * If the test coroutine is already finishing with an error, that error is added to the timeout notification as suppressed. * The timeout is considered more important, so now, if there were any uncaught exceptions, they don't take precedence over it but are suppressed by it. Fixes #3066 Fixes #3069
yorickhenning
pushed a commit
to yorickhenning/kotlinx.coroutines
that referenced
this pull request
Jan 28, 2022
* The error is more verbose: it lists the active children. * If the test coroutine is already finishing with an error, that error is added to the timeout notification as suppressed. * The timeout is considered more important, so now, if there were any uncaught exceptions, they don't take precedence over it but are suppressed by it. Fixes Kotlin#3066 Fixes Kotlin#3069
pablobaxter
pushed a commit
to pablobaxter/kotlinx.coroutines
that referenced
this pull request
Sep 14, 2022
* The error is more verbose: it lists the active children. * If the test coroutine is already finishing with an error, that error is added to the timeout notification as suppressed. * The timeout is considered more important, so now, if there were any uncaught exceptions, they don't take precedence over it but are suppressed by it. Fixes Kotlin#3066 Fixes Kotlin#3069
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #3066
Fixes #3069