Skip to content

Commit 88e7ed0

Browse files
steve-the-edwardssteve-the-edwards
authored
Merge pull request #1454 from square/sedwards/udpate-coroutine-unit-test-dispatch
Update coroutine based tests to use StandardTestDispatcher
2 parents 1c8d88a + e85b98b commit 88e7ed0

7 files changed

Lines changed: 68 additions & 20 deletions

File tree

‎.github/workflows/kotlin.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -564,7 +564,7 @@ jobs:
564564
kmp-ios-tests:
565565
name: iOS Unit Tests for KMP Modules
566566
runs-on: macos-latest
567-
timeout-minutes: 30
567+
timeout-minutes: 45
568568
steps:
569569
- name: Checkout
570570
uses: actions/checkout@v6

‎workflow-core/src/commonTest/kotlin/com/squareup/workflow1/SinkTest.kt‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import kotlinx.coroutines.flow.MutableStateFlow
88
import kotlinx.coroutines.flow.consumeAsFlow
99
import kotlinx.coroutines.launch
1010
import kotlinx.coroutines.test.StandardTestDispatcher
11-
import kotlinx.coroutines.test.UnconfinedTestDispatcher
1211
import kotlinx.coroutines.test.advanceUntilIdle
1312
import kotlinx.coroutines.test.runTest
1413
import kotlinx.coroutines.withContext
@@ -68,35 +67,48 @@ internal class SinkTest {
6867
sentActions += it
6968
}
7069

71-
runTest(UnconfinedTestDispatcher()) {
70+
// With StandardTestDispatcher, coroutines are not eagerly dispatched. The sendJob starts
71+
// UNDISPATCHED so it runs to its first suspension (channel.send("a")). Then advanceUntilIdle
72+
// starts the collectJob which receives "a", sends the action to the sink, and suspends in
73+
// sendAndAwaitApplication. The sendJob then resumes from send("a") and suspends at send("b")
74+
// because the collector is blocked on backpressure. Only after we apply the first action and
75+
// advance again does "b" get processed.
76+
runTest(StandardTestDispatcher()) {
7277
val collectJob = launch {
7378
flow.collectToSink(sink) { action("") { setOutput(it) } }
7479
}
7580

7681
val sendJob = launch(start = UNDISPATCHED) {
7782
assertEquals(0, counter.getAndIncrement())
7883
channel.send("a")
84+
// Resumed after advanceUntilIdle when collectJob receives "a" from channel.
7985
assertEquals(1, counter.getAndIncrement())
8086
channel.send("b")
81-
assertEquals(4, counter.getAndIncrement())
82-
channel.close()
87+
// Resumed after second advanceUntilIdle when collectJob receives "b" from channel.
8388
assertEquals(5, counter.getAndIncrement())
89+
channel.close()
90+
assertEquals(6, counter.getAndIncrement())
8491
}
8592
advanceUntilIdle()
8693
assertEquals(2, counter.getAndIncrement())
8794

8895
sentActions.removeFirst()
8996
.also {
90-
advanceUntilIdle()
9197
// Sender won't resume until we've _applied_ the action.
9298
assertEquals(3, counter.getAndIncrement())
9399
}
94100
.applyTo(Unit, Unit)
95101
.let { (_, result) ->
96-
assertEquals(6, counter.getAndIncrement())
102+
// With StandardTestDispatcher, continuation.resume is dispatched, not immediate.
103+
// So the counter hasn't been incremented by the sendJob yet.
104+
assertEquals(4, counter.getAndIncrement())
97105
assertEquals("a", result.output!!.value)
98106
}
99107

108+
// Advance to let collector resume from sendAndAwaitApplication, receive "b",
109+
// and let sendJob resume from send("b") through close().
110+
advanceUntilIdle()
111+
100112
sentActions.removeFirst()
101113
.applyTo(Unit, Unit)
102114
.let { (_, result) ->

‎workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/WorkflowOperatorsTest.kt‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@ import kotlinx.coroutines.flow.launchIn
99
import kotlinx.coroutines.flow.onEach
1010
import kotlinx.coroutines.flow.onStart
1111
import kotlinx.coroutines.plus
12-
import kotlinx.coroutines.test.UnconfinedTestDispatcher
12+
import kotlinx.coroutines.test.StandardTestDispatcher
13+
import kotlinx.coroutines.test.advanceUntilIdle
1314
import kotlinx.coroutines.test.runTest
1415
import kotlin.test.Test
1516
import kotlin.test.assertEquals
@@ -36,18 +37,22 @@ class WorkflowOperatorsTest {
3637
val childWorkflow = object : StateFlowWorkflow<String>("child", trigger) {}
3738
val mappedWorkflow = childWorkflow.mapRendering { "mapped: $it" }
3839

39-
runTest(UnconfinedTestDispatcher()) {
40+
runTest(StandardTestDispatcher()) {
4041
val renderings = mutableListOf<String>()
4142
val workflowJob = Job(coroutineContext[Job])
4243
renderWorkflowIn(mappedWorkflow, this + workflowJob, MutableStateFlow(Unit)) {}
4344
.onEach { renderings += it.rendering }
4445
.launchIn(this + workflowJob)
46+
// Start the runtime loop and collector coroutines.
47+
advanceUntilIdle()
4548
assertEquals(listOf("mapped: initial"), renderings)
4649

4750
trigger.value = "foo"
51+
advanceUntilIdle()
4852
assertEquals(listOf("mapped: initial", "mapped: foo"), renderings)
4953

5054
trigger.value = "bar"
55+
advanceUntilIdle()
5156
assertEquals(listOf("mapped: initial", "mapped: foo", "mapped: bar"), renderings)
5257

5358
workflowJob.cancel()
@@ -66,12 +71,14 @@ class WorkflowOperatorsTest {
6671
).toString()
6772
}
6873

69-
runTest(UnconfinedTestDispatcher()) {
74+
runTest(StandardTestDispatcher()) {
7075
val renderings = mutableListOf<String>()
7176
val workflowJob = Job(coroutineContext[Job])
7277
renderWorkflowIn(parentWorkflow, this + workflowJob, MutableStateFlow(Unit)) {}
7378
.onEach { renderings += it.rendering }
7479
.launchIn(this + workflowJob)
80+
// Start the runtime loop and collector coroutines.
81+
advanceUntilIdle()
7582
assertEquals(
7683
listOf(
7784
"[rendering1: initial1, rendering2: initial2]"
@@ -80,6 +87,7 @@ class WorkflowOperatorsTest {
8087
)
8188

8289
trigger1.value = "foo"
90+
advanceUntilIdle()
8391
assertEquals(
8492
listOf(
8593
"[rendering1: initial1, rendering2: initial2]",
@@ -89,6 +97,7 @@ class WorkflowOperatorsTest {
8997
)
9098

9199
trigger2.value = "bar"
100+
advanceUntilIdle()
92101
assertEquals(
93102
listOf(
94103
"[rendering1: initial1, rendering2: initial2]",
@@ -114,12 +123,14 @@ class WorkflowOperatorsTest {
114123
).toString()
115124
}
116125

117-
runTest(UnconfinedTestDispatcher()) {
126+
runTest(StandardTestDispatcher()) {
118127
val renderings = mutableListOf<String>()
119128
val workflowJob = Job(coroutineContext[Job])
120129
renderWorkflowIn(parentWorkflow, this + workflowJob, MutableStateFlow(Unit)) {}
121130
.onEach { renderings += it.rendering }
122131
.launchIn(this + workflowJob)
132+
// Start the runtime loop and collector coroutines.
133+
advanceUntilIdle()
123134
assertEquals(
124135
listOf(
125136
"[initial1, rendering2: initial2]"
@@ -128,6 +139,7 @@ class WorkflowOperatorsTest {
128139
)
129140

130141
trigger1.value = "foo"
142+
advanceUntilIdle()
131143
assertEquals(
132144
listOf(
133145
"[initial1, rendering2: initial2]",
@@ -137,6 +149,7 @@ class WorkflowOperatorsTest {
137149
)
138150

139151
trigger2.value = "bar"
152+
advanceUntilIdle()
140153
assertEquals(
141154
listOf(
142155
"[initial1, rendering2: initial2]",
@@ -163,12 +176,14 @@ class WorkflowOperatorsTest {
163176
}
164177
val props = MutableStateFlow(0)
165178

166-
runTest(UnconfinedTestDispatcher()) {
179+
runTest(StandardTestDispatcher()) {
167180
val renderings = mutableListOf<String>()
168181
val workflowJob = Job(coroutineContext[Job])
169182
renderWorkflowIn(parentWorkflow, this + workflowJob, props) {}
170183
.onEach { renderings += it.rendering }
171184
.launchIn(this + workflowJob)
185+
// Start the runtime loop and collector coroutines.
186+
advanceUntilIdle()
172187
assertEquals(
173188
listOf(
174189
"rendering1: initial"
@@ -178,6 +193,7 @@ class WorkflowOperatorsTest {
178193
assertEquals(1, childWorkflow.starts)
179194

180195
trigger.value = "foo"
196+
advanceUntilIdle()
181197
assertEquals(1, childWorkflow.starts)
182198
assertEquals(
183199
listOf(
@@ -188,7 +204,9 @@ class WorkflowOperatorsTest {
188204
)
189205

190206
props.value = 1
207+
advanceUntilIdle()
191208
trigger.value = "bar"
209+
advanceUntilIdle()
192210
assertEquals(1, childWorkflow.starts)
193211
assertEquals(
194212
listOf(

‎workflow-testing/src/test/java/com/squareup/workflow1/WorkflowsLifecycleTests.kt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,8 @@ class WorkflowsLifecycleTests(
203203
@OptIn(ExperimentalCoroutinesApi::class)
204204
@Test
205205
fun childSessionWorkflowStartAndStoppedWhenHandledSynchronously() {
206+
// Unfortunately we have to use the UnconfinedTestDispatcher here otherwise we'd only ever see
207+
// one of the test states when we get around to advancing the test scheduler.
206208
val dispatcher = UnconfinedTestDispatcher()
207209
workflowWithChildSession.renderForTest(
208210
coroutineContext = dispatcher,

‎workflow-tracing/src/test/java/com/squareup/workflow1/tracing/WorkflowRuntimeMonitorTest.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@ import com.squareup.workflow1.tracing.RenderCause.RootCreation
2222
import com.squareup.workflow1.tracing.RenderCause.RootPropsChanged
2323
import kotlinx.coroutines.CoroutineScope
2424
import kotlinx.coroutines.ExperimentalCoroutinesApi
25+
import kotlinx.coroutines.test.StandardTestDispatcher
2526
import kotlinx.coroutines.test.TestScope
26-
import kotlinx.coroutines.test.UnconfinedTestDispatcher
2727
import kotlin.coroutines.CoroutineContext
2828
import kotlin.coroutines.EmptyCoroutineContext
2929
import kotlin.reflect.KType
@@ -86,7 +86,7 @@ internal class WorkflowRuntimeMonitorTest {
8686
workflowRuntimeTracers = listOf(fakeRuntimeTracer)
8787
)
8888
val testWorkflow = TestWorkflow()
89-
val testCoroutineDispatcher = UnconfinedTestDispatcher()
89+
val testCoroutineDispatcher = StandardTestDispatcher()
9090
val rootSession = testWorkflow.createRootSession(testCoroutineDispatcher)
9191
val testScope = TestScope(testCoroutineDispatcher)
9292

‎workflow-ui/compose/src/androidTest/java/com/squareup/workflow1/ui/compose/RenderAsStateTest.kt‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ import kotlinx.coroutines.Job
3131
import kotlinx.coroutines.awaitCancellation
3232
import kotlinx.coroutines.isActive
3333
import kotlinx.coroutines.job
34+
import kotlinx.coroutines.test.StandardTestDispatcher
3435
import kotlinx.coroutines.test.TestScope
35-
import kotlinx.coroutines.test.UnconfinedTestDispatcher
3636
import kotlinx.coroutines.test.advanceUntilIdle
3737
import kotlinx.coroutines.test.runTest
3838
import leakcanary.DetectLeaksAfterTestSuccess
@@ -279,14 +279,15 @@ internal class RenderAsStateTest {
279279
awaitCancellation()
280280
}
281281
}
282-
val scope = TestScope(UnconfinedTestDispatcher())
282+
val scope = TestScope(StandardTestDispatcher())
283283

284284
class CancelCompositionException : RuntimeException()
285285

286286
scope.runTest {
287287
assertFailsWith<CancelCompositionException> {
288288
composeRule.setContent {
289289
workflow.renderAsState(props = Unit, onOutput = {}, scope = scope)
290+
scope.advanceUntilIdle()
290291
throw CancelCompositionException()
291292
}
292293
}

‎workflow-ui/core-android/src/test/java/com/squareup/workflow1/ui/WorkflowLayoutTest.kt‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ import com.squareup.workflow1.ui.navigation.WrappedScreen
1818
import kotlinx.coroutines.ExperimentalCoroutinesApi
1919
import kotlinx.coroutines.flow.MutableSharedFlow
2020
import kotlinx.coroutines.flow.onEach
21-
import kotlinx.coroutines.test.UnconfinedTestDispatcher
21+
import kotlinx.coroutines.test.StandardTestDispatcher
22+
import kotlinx.coroutines.test.advanceUntilIdle
2223
import kotlinx.coroutines.test.runTest
2324
import org.junit.Test
2425
import org.junit.runner.RunWith
@@ -95,7 +96,7 @@ internal class WorkflowLayoutTest {
9596
}
9697

9798
@Test fun takes() {
98-
val lifecycleDispatcher = UnconfinedTestDispatcher()
99+
val lifecycleDispatcher = StandardTestDispatcher()
99100
val testLifecycle = TestLifecycleOwner(
100101
initialState = Lifecycle.State.RESUMED,
101102
coroutineDispatcher = lifecycleDispatcher
@@ -107,14 +108,18 @@ internal class WorkflowLayoutTest {
107108
lifecycle = testLifecycle.lifecycle,
108109
renderings = flow,
109110
)
111+
// Start the collector coroutine so it subscribes to the SharedFlow before we emit.
112+
advanceUntilIdle()
110113
assertThat(workflowLayout[0]).isInstanceOf(WorkflowViewStub::class.java)
114+
111115
flow.emit(WrappedScreen())
116+
advanceUntilIdle()
112117
assertThat(workflowLayout[0]).isNotInstanceOf(WorkflowViewStub::class.java)
113118
}
114119
}
115120

116121
@Test fun canStopTaking() {
117-
val lifecycleDispatcher = UnconfinedTestDispatcher()
122+
val lifecycleDispatcher = StandardTestDispatcher()
118123
val testLifecycle = TestLifecycleOwner(
119124
initialState = Lifecycle.State.RESUMED,
120125
coroutineDispatcher = lifecycleDispatcher
@@ -126,14 +131,21 @@ internal class WorkflowLayoutTest {
126131
lifecycle = testLifecycle.lifecycle,
127132
renderings = flow,
128133
)
134+
// Start the collector, then cancel and process cancellation so it unsubscribes from the
135+
// SharedFlow. Without this, emit() would deadlock waiting for the cancelled subscriber.
136+
advanceUntilIdle()
129137
job.cancel()
138+
advanceUntilIdle()
130139
flow.emit(WrappedScreen())
140+
// Technically not needed since emit returns right away, but included so we know that
141+
// we tried to take anything emitted.
142+
advanceUntilIdle()
131143
assertThat(workflowLayout[0]).isInstanceOf(WorkflowViewStub::class.java)
132144
}
133145
}
134146

135147
@Test fun usesProvidedCoroutineContext() {
136-
val lifecycleDispatcher = UnconfinedTestDispatcher()
148+
val lifecycleDispatcher = StandardTestDispatcher()
137149
val testLifecycle = TestLifecycleOwner(
138150
initialState = Lifecycle.State.RESUMED,
139151
coroutineDispatcher = lifecycleDispatcher
@@ -154,8 +166,11 @@ internal class WorkflowLayoutTest {
154166
renderings = trackedFlow,
155167
collectionContext = testElement
156168
)
169+
// Start the collector coroutine so it subscribes to the SharedFlow before we emit.
170+
advanceUntilIdle()
157171

158172
flow.emit(WrappedScreen())
173+
advanceUntilIdle()
159174

160175
assertThat(testElement.wasUsed).isTrue()
161176
}

0 commit comments

Comments
 (0)