-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Remove TestCoroutineContext.kt completely #3291
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Grepping for the mentions of the package, I also see things to remove in
kotlinx-coroutines-core/README.md
kotlinx-coroutines-core/build.gradle
@@ -77,9 +77,6 @@ public class CoroutinesBlockHoundIntegration : BlockHoundIntegration { | |||
for (method in listOf("clear", "peek", "removeFirstOrNull", "addLast")) { | |||
allowBlockingCallsInside("kotlinx.coroutines.internal.ThreadSafeHeap", method) | |||
} | |||
// [addLastIf] is only used in [EventLoop.common]. Users of [removeFirstIf]: | |||
allowBlockingCallsInside("kotlinx.coroutines.test.TestCoroutineDispatcher", "doActionsUntil") | |||
allowBlockingCallsInside("kotlinx.coroutines.test.TestCoroutineContext", "triggerActions") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not directly related, but looking at this, I don't know how I feel about it being forbidden to do the test module's runCurrent
and advance*
from Dispatchers.Default
when BlockHound is used, even if the operations themselves are completely fine. I don't think this is a realistic concern, but it still feels a bit semantically unclean.
0510294
to
f0260ba
Compare
While we typically avoid breaking experimental API in minor releases, even if we're allowed to, this one seems to be acceptable. It was obsolete since the very 1.0.0 and was never integrated into structured concurrency, and on the other side it prevents using
kotlinx-coroutines-core
along withkotlinx-coroutines-test
in a modularized environment due to split packagekotlinx.coroutines.test
.Fixes #3218