Skip to content

Eagerly enter launch and async blocks with unconfined dispatcher. #3011

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

Merged

Conversation

dkhalanskyjb
Copy link
Collaborator

@qwwdfsad found a convenient migration path for most tests that rely on launch and async blocks being entered eagerly. Implemented it here.

Also, I fixed Dispatchers.Main not delegating Delay methods and discovered that, on JS, Dispatchers.Main gets reset during the test if it is reset in AfterTest.

Also, fix `Dispatchers.Main` not delegating `Delay` methods and
discover that, on JS, `Dispatchers.Main` gets reset during the test
if it is reset in `AfterTest`.
@dkhalanskyjb dkhalanskyjb requested a review from qwwdfsad November 8, 2021 09:46
Copy link
Collaborator

@qwwdfsad qwwdfsad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat!

*
* ```
* @Test
* fun testEagerlyEnteringChildCoroutines() = runTest(UnconfinedTestDispatcher()) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, what do you think about providing runTestUnconfined for a better migration path?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like it because of this confusing behavior:

val ctx = StandardTestDispatcher()

@Test
fun test() = runTestUnconfined(ctx) {
  // not unconfined
}

@dkhalanskyjb
Copy link
Collaborator Author

I added separate FailingTests files that document and check the known problematic behaviors. I haven't seen such things in our codebase before—is there an established way to do this?

@dkhalanskyjb dkhalanskyjb force-pushed the coroutines-test-eagerly-enter branch from 43cecf5 to 61461a3 Compare November 8, 2021 14:23
@dkhalanskyjb dkhalanskyjb requested a review from qwwdfsad November 9, 2021 08:26
Copy link
Collaborator

@qwwdfsad qwwdfsad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

What an elegant idea with @NoPlatform annotation!

@qwwdfsad
Copy link
Collaborator

qwwdfsad commented Nov 9, 2021

FailingTests is fine, we do not have en established way for this

@dkhalanskyjb dkhalanskyjb merged commit 95b98c3 into coroutines-test-virtualtime Nov 10, 2021
@dkhalanskyjb dkhalanskyjb deleted the coroutines-test-eagerly-enter branch November 10, 2021 07:41
dkhalanskyjb added a commit that referenced this pull request Nov 17, 2021
Also, fix `Dispatchers.Main` not delegating `Delay` methods and
discover that, on JS, `Dispatchers.Main` gets reset during the test
if it is reset in `AfterTest`.
dkhalanskyjb added a commit that referenced this pull request Nov 17, 2021
Also, fix `Dispatchers.Main` not delegating `Delay` methods and
discover that, on JS, `Dispatchers.Main` gets reset during the test
if it is reset in `AfterTest`.
dkhalanskyjb added a commit that referenced this pull request Nov 19, 2021
Also, fix `Dispatchers.Main` not delegating `Delay` methods and
discover that, on JS, `Dispatchers.Main` gets reset during the test
if it is reset in `AfterTest`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants