-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Implement new test dispatchers #2986
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 new test dispatchers #2986
Conversation
98102de
to
608783e
Compare
e33ba72
to
8169351
Compare
608783e
to
d694912
Compare
5fd866a
to
d8ae171
Compare
d694912
to
77d524f
Compare
d8ae171
to
80e076f
Compare
77d524f
to
78e5855
Compare
b7f4216
to
015b06c
Compare
15d1dec
to
a8e43d6
Compare
015b06c
to
e821121
Compare
* scheduler in order for the tasks to run. | ||
*/ | ||
@ExperimentalCoroutinesApi | ||
public class UnconfinedTestDispatcher( |
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 do not recall our final decision regarding this, maybe we've discussed it and decided not to do:
Are we going to add runTestUnconfined
for people to gracefully migrate from runBlockingTest
and to have a counterpart for 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.
Also, we can then bulk-replace deprecated runBlockingTest
in our tests
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.
Are we going to add
runTestUnconfined
for people to gracefully migrate fromrunBlockingTest
We didn't decide on this in our discussion. However, checking more closely how Unconfined
behaves, I discovered that it does not, in fact, enter launch
and async
blocks eagerly. The only reliable behavior I got from it is that launching a coroutine with Unconfined
leads to all dispatches to that coroutine from non-unconfined dispatchers to happen eagerly. Launching the whole test under UnconfinedTestDispatcher
leads to haphazard execution. I still decided to keep it given how useful the unconfined dispatcher is when child coroutines are launched with it.
So, it may turn out that we'll need what is now the TestCoroutineDispatcher
after all, just with a different name, and probably with slightly different behavior.
Also, we can then bulk-replace deprecated runBlockingTest in our tests
I don't think we should do that. The only tests that use runBlockingTest
are the legacy ones that ensure that we didn't break a lot of existing code during the transition. I checked the tests that use runBlockingTest
and, for each one that I found useful, I added the corresponding version in terms of the new primitives.
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.
(This is a test to see if I can still write in this discussion; a Github bug could prevent me)
e821121
to
7765d76
Compare
* | ||
* Additionally, [name] can be set to distinguish each dispatcher instance when debugging. | ||
* | ||
* @see StandardTestDispatcher for a more predictable [TestDispatcher] that, however, requires interacting with the |
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.
Maybe it's worth to avoid such caution, especially if we would like to promote standard dispatcher (and, well, runTest
in general) as the default one
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.
Also, it may be good to state that such dispatcher is intended to be used in advanced scenarios and show one with a code example to prevent people from using it in a new code where it is not necessary and to warn that this may be not the best migration path from runBlockingTest
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.
Done.
7765d76
to
6a33516
Compare
Defines two test dispatchers: * StandardTestDispatcher, which, combined with runTest, gives an illusion of an event loop; * UnconfinedTestDispatcher, which is like Dispatchers.Unconfined, but skips delays. By default, StandardTestDispatcher is used due to the somewhat chaotic execution order of Dispatchers.Unconfined. TestCoroutineDispatcher is deprecated. Fixes #1626 Fixes #1742 Fixes #2082 Fixes #2102 Fixes #2405 Fixes #2462
Defines two test dispatchers: * StandardTestDispatcher, which, combined with runTest, gives an illusion of an event loop; * UnconfinedTestDispatcher, which is like Dispatchers.Unconfined, but skips delays. By default, StandardTestDispatcher is used due to the somewhat chaotic execution order of Dispatchers.Unconfined. TestCoroutineDispatcher is deprecated. Fixes #1626 Fixes #1742 Fixes #2082 Fixes #2102 Fixes #2405 Fixes #2462
Defines two test dispatchers: * StandardTestDispatcher, which, combined with runTest, gives an illusion of an event loop; * UnconfinedTestDispatcher, which is like Dispatchers.Unconfined, but skips delays. By default, StandardTestDispatcher is used due to the somewhat chaotic execution order of Dispatchers.Unconfined. TestCoroutineDispatcher is deprecated. Fixes #1626 Fixes #1742 Fixes #2082 Fixes #2102 Fixes #2405 Fixes #2462
Defines two test dispatchers:
StandardTestDispatcher
, which, combined withrunTest
, gives an illusion of an event loop;UnconfinedTestDispatcher
, which is likeDispatchers.Unconfined
, but skips delays.By default,
StandardTestDispatcher
is used due to the somewhat chaotic execution order ofDispatchers.Unconfined
.TestCoroutineDispatcher
is deprecated.Fixes #1626
Fixes #1742
Fixes #2082
Fixes #2102
Fixes #2405
Fixes #2462