-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Implement 'runTest' that waits for asynchronous completion #2978
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
Implement 'runTest' that waits for asynchronous completion #2978
Conversation
3e872b4
to
f90ba4c
Compare
e2bbbf6
to
19ed1f0
Compare
f90ba4c
to
5ea966d
Compare
98102de
to
608783e
Compare
5ea966d
to
4cdda36
Compare
608783e
to
d694912
Compare
d694912
to
77d524f
Compare
7d004d6
to
0a60a3b
Compare
0a60a3b
to
2cddb28
Compare
77d524f
to
78e5855
Compare
15d1dec
to
a8e43d6
Compare
* immediately from the test body. See the docs for [TestResult] for details. | ||
*/ | ||
@ExperimentalCoroutinesApi | ||
public fun TestDispatcher.runTest( |
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.
Do we still need this one if #3006 gets merged?
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.
It has its uses. People also create dispatchers in advance to perform DI.
In any case, this method looks fairly benign and doesn't hurt anything, so a more important question is whether we need TestCoroutineScope.runTest
, which presupposes that we provide ways to create TestCoroutineScope
. If we don't, then this convenience method can also go.
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.
Let's make this decision in the final branch along with the migration guide.
I still believe TestCoroutineScope.runTest
may be useful for the examples like this one:
@Before
fun before() {
myScope = TestCoroutineScope(...)
myEntity = MyEntity(..., myScope)
}
@Test
fun scenario1() = myScope.runTest {
myEntity.foo()
testScheduler.advanceUntilIdle()
verifyFoo()
}
@Test
fun scenario2() = myScope.runTest {
myEntity.bar()
testScheduler.advanceUntilIdle()
verifyBar()
}
Maybe TestDispatcher.runTest
has the same uses, I was thinking of removing it for the sake of keeping API compact -- it's easily emulated with runTest(dispatcher)
and only provides one more method to be aware of/to study.
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.
I should note that the scope inside myScope.runTest { ... }
is not the same thing as myScope
, which may reduce the usefulness of the pattern.
Also, this code is missing a
@AfterTest
fun after() {
myScope.cleanupTestCoroutines()
}
I think this is a major point. According to the comments at the bottom of https://youtrack.jetbrains.com/issue/KT-19813, AfterTest
and BeforeTest
don't properly allow returning a Promise
, which means that either
- the cleanup procedure also can't return a
TestResult
, or - we shouldn't promote cleaning up in
AfterTest
, but then we'll also have to avoid promoting initializing scopes inBeforeTest
for symmetry.
…-other-dispatchers
…o coroutines-test-await-other-dispatchers
* The platform difference entails that, in order to use this function correctly in common code, one must always | ||
* immediately return the produced [TestResult] from the test method, without doing anything else afterwards. See | ||
* [TestResult] for details on this. | ||
* |
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.
Mentioning the scope/dispatcher and the ability to wait until all tasks are executed is valuable for this API and worth mentioning:
@Test
fun exampleTest() = runTest {
val myComponent = MyComponent(this) // TODO: can't extract dispatcher here
myComponent.executeAsyncOp()
testScheduler.advanceUntilIdle()
verifyAsyncOpCompleted()
}
I do not insist though, but it seems pretty important, WDYT?
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.
Ok, added a mention of this.
Implement a multiplatform
runTest
as an initial implementation of #1996.The new
runTest
also waits for asynchronous completion, which makes this a fix for a long-standing issue.Fixes #1204
Fixes #1222
Fixes #1395
Fixes #1881
Fixes #1910
Fixes #1365 (though there may be some other use cases that this solution doesn't cover)
Fixes #1772