-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Add a scope for launching background work in tests #3348
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
Merged
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
9bc3335
WIP
dkhalanskyjb 4e35d15
Fix crashing on Native
dkhalanskyjb 6fa024c
Add some tests, fix a bug
dkhalanskyjb cefc6b9
backgroundWorkScope -> backgroundScope
dkhalanskyjb bffa53a
Improve the backgroundScope documentation
dkhalanskyjb 1865848
Catch more of the errors
dkhalanskyjb 6329202
Fix the API dump
dkhalanskyjb dd4536d
Fixup
dkhalanskyjb ad6134b
Add a TODO
dkhalanskyjb 4fe8ef0
Add a test for coroutine.onJoin
dkhalanskyjb 3a2c287
Fixup
dkhalanskyjb cf2f979
Fixup
dkhalanskyjb e21d65c
Add tests for backgroundScope in runBlockingTest
dkhalanskyjb File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -164,15 +164,19 @@ public fun TestScope.runTest( | |
| ): TestResult = asSpecificImplementation().let { | ||
| it.enter() | ||
| createTestResult { | ||
| runTestCoroutine(it, dispatchTimeoutMs, TestScopeImpl::tryGetCompletionCause, testBody) { it.leave() } | ||
| runTestCoroutine(it, dispatchTimeoutMs, TestScopeImpl::tryGetCompletionCause, testBody) { | ||
| backgroundScope.cancel() | ||
| testScheduler.advanceUntilIdleOr { false } | ||
| it.leave() | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Runs [testProcedure], creating a [TestResult]. | ||
| */ | ||
| @Suppress("NO_ACTUAL_FOR_EXPECT") // actually suppresses `TestResult` | ||
| internal expect fun createTestResult(testProcedure: suspend () -> Unit): TestResult | ||
| internal expect fun createTestResult(testProcedure: suspend CoroutineScope.() -> Unit): TestResult | ||
|
|
||
| /** A coroutine context element indicating that the coroutine is running inside `runTest`. */ | ||
| internal object RunningInRunTest : CoroutineContext.Key<RunningInRunTest>, CoroutineContext.Element { | ||
|
|
@@ -195,7 +199,7 @@ internal const val DEFAULT_DISPATCH_TIMEOUT_MS = 60_000L | |
| * The [cleanup] procedure may either throw [UncompletedCoroutinesError] to denote that child coroutines were leaked, or | ||
| * return a list of uncaught exceptions that should be reported at the end of the test. | ||
| */ | ||
| internal suspend fun <T: AbstractCoroutine<Unit>> runTestCoroutine( | ||
| internal suspend fun <T: AbstractCoroutine<Unit>> CoroutineScope.runTestCoroutine( | ||
| coroutine: T, | ||
| dispatchTimeoutMs: Long, | ||
| tryGetCompletionCause: T.() -> Throwable?, | ||
|
|
@@ -207,6 +211,27 @@ internal suspend fun <T: AbstractCoroutine<Unit>> runTestCoroutine( | |
| coroutine.start(CoroutineStart.UNDISPATCHED, coroutine) { | ||
| testBody() | ||
| } | ||
| /** | ||
| * The general procedure here is as follows: | ||
| * 1. Try running the work that the scheduler knows about, both background and foreground. | ||
| * | ||
| * 2. Wait until we run out of foreground work to do. This could mean one of the following: | ||
| * * The main coroutine is already completed. This is checked separately; then we leave the procedure. | ||
| * * It's switched to another dispatcher that doesn't know about the [TestCoroutineScheduler]. | ||
| * * Generally, it's waiting for something external (like a network request, or just an arbitrary callback). | ||
| * * The test simply hanged. | ||
| * * The main coroutine is waiting for some background work. | ||
| * | ||
| * 3. We await progress from things that are not the code under test: | ||
| * the background work that the scheduler knows about, the external callbacks, | ||
| * the work on dispatchers not linked to the scheduler, etc. | ||
| * | ||
| * When we observe that the code under test can proceed, we go to step 1 again. | ||
| * If there is no activity for [dispatchTimeoutMs] milliseconds, we consider the test to have hanged. | ||
| * | ||
| * The background work is not running on a dedicated thread. | ||
| * Instead, the test thread itself is used, by spawning a separate coroutine. | ||
| */ | ||
| var completed = false | ||
| while (!completed) { | ||
| scheduler.advanceUntilIdle() | ||
|
|
@@ -216,16 +241,29 @@ internal suspend fun <T: AbstractCoroutine<Unit>> runTestCoroutine( | |
| completed = true | ||
| continue | ||
| } | ||
| select<Unit> { | ||
| coroutine.onJoin { | ||
| completed = true | ||
| } | ||
| scheduler.onDispatchEvent { | ||
| // we received knowledge that `scheduler` observed a dispatch event, so we reset the timeout | ||
| // in case progress depends on some background work, we need to keep spinning it. | ||
| val backgroundWorkRunner = launch(CoroutineName("background work runner")) { | ||
| while (true) { | ||
| scheduler.tryRunNextTaskUnless { !isActive } | ||
| // yield so that the `select` below has a chance to check if its conditions are fulfilled | ||
| yield() | ||
| } | ||
| onTimeout(dispatchTimeoutMs) { | ||
| handleTimeout(coroutine, dispatchTimeoutMs, tryGetCompletionCause, cleanup) | ||
| } | ||
| try { | ||
| select<Unit> { | ||
| coroutine.onJoin { | ||
| // observe that someone completed the test coroutine and leave without waiting for the timeout | ||
| completed = true | ||
| } | ||
| scheduler.onDispatchEvent { | ||
| // we received knowledge that `scheduler` observed a dispatch event, so we reset the timeout | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also worth noting that it doesn't receive events from background jobs |
||
| } | ||
| onTimeout(dispatchTimeoutMs) { | ||
| handleTimeout(coroutine, dispatchTimeoutMs, tryGetCompletionCause, cleanup) | ||
| } | ||
| } | ||
| } finally { | ||
| backgroundWorkRunner.cancelAndJoin() | ||
| } | ||
| } | ||
| coroutine.getCompletionExceptionOrNull()?.let { exception -> | ||
|
|
||
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.