Change runOnIdle to execute on the UI thread, and to not waitForIdle afterward.

m-sasha · m-sasha · commit 12e5e54e4f22 · 2024-09-30T15:52:07.000+03:00
diff --git a/compose/foundation/foundation/src/skikoTest/kotlin/androidx/compose/foundation/ToggleableTest.kt b/compose/foundation/foundation/src/skikoTest/kotlin/androidx/compose/foundation/ToggleableTest.kt
@@ -52,9 +52,7 @@ class ToggleableTest {
             ).testTag("toggle"))
         }
 
-        runOnIdle {
-            assertEquals(false, state)
-        }
+        assertEquals(false, state)
 
         onRoot().apply {
             performKeyPress(KeyEvent(Key.Tab, KeyEventType.KeyDown))
@@ -66,17 +64,13 @@ class ToggleableTest {
             performKeyPress(KeyEvent(Key.Spacebar, KeyEventType.KeyUp))
         }
 
-        runOnIdle {
-            assertEquals(true, state)
-        }
+        assertEquals(true, state)
 
         onNodeWithTag("toggle").apply {
             performKeyPress(KeyEvent(Key.Spacebar, KeyEventType.KeyDown))
             performKeyPress(KeyEvent(Key.Spacebar, KeyEventType.KeyUp))
         }
 
-        runOnIdle {
-            assertEquals(false, state)
-        }
+        assertEquals(false, state)
     }
 }
diff --git a/compose/foundation/foundation/src/skikoTest/kotlin/androidx/compose/foundation/copyPasteAndroidTests/ScrollableTest.kt b/compose/foundation/foundation/src/skikoTest/kotlin/androidx/compose/foundation/copyPasteAndroidTests/ScrollableTest.kt
@@ -1701,23 +1701,19 @@ class ScrollableTest {
             }
         }
 
-        runOnIdle {
-            assertThat(isOuterInScrollableContainer).isFalse()
-            assertThat(isInnerInScrollableContainer).isTrue()
-        }
+        assertThat(isOuterInScrollableContainer).isFalse()
+        assertThat(isInnerInScrollableContainer).isTrue()
     }
 
     @Test
     fun scrollable_scrollByWorksWithRepeatableAnimations() = runSkikoComposeUiTest {
         mainClock.autoAdvance = false
 
         var total = 0f
-        val controller = ScrollableState(
-            consumeScrollDelta = {
-                total += it
-                it
-            }
-        )
+        val controller = ScrollableState {
+            total += it
+            it
+        }
         setContentAndGetScope {
             Box(
                 modifier = Modifier
@@ -1729,62 +1725,48 @@ class ScrollableTest {
             )
         }
 
-        runOnIdle {
-            scope.launch {
-                controller.animateScrollBy(
-                    100f,
-                    keyframes {
-                        durationMillis = 2500
-                        // emulate a repeatable animation:
-                        0f at 0
-                        100f at 500
-                        100f at 1000
-                        0f at 1500
-                        0f at 2000
-                        100f at 2500
-                    }
-                )
-            }
+        scope.launch {
+            controller.animateScrollBy(
+                value = 100f,
+                animationSpec = keyframes {
+                    durationMillis = 2500
+                    // emulate a repeatable animation:
+                    0f at 0
+                    100f at 500
+                    100f at 1000
+                    0f at 1500
+                    0f at 2000
+                    100f at 2500
+                }
+            )
         }
 
         mainClock.advanceTimeBy(250)
-        runOnIdle {
-            // in the middle of the first animation
-            assertThat(total).isGreaterThan(0f)
-            assertThat(total).isLessThan(100f)
-        }
+        // in the middle of the first animation
+        assertThat(total).isGreaterThan(0f)
+        assertThat(total).isLessThan(100f)
 
         mainClock.advanceTimeBy(500) // 750 ms
-        runOnIdle {
-            // first animation finished
-            assertThat(total).isEqualTo(100f)
-        }
+        // first animation finished
+        assertThat(total).isEqualTo(100f)
 
         mainClock.advanceTimeBy(250) // 1250 ms
-        runOnIdle {
-            // in the middle of the second animation
-            assertThat(total).isGreaterThan(0f)
-            assertThat(total).isLessThan(100f)
-        }
+        // in the middle of the second animation
+        assertThat(total).isGreaterThan(0f)
+        assertThat(total).isLessThan(100f)
 
         mainClock.advanceTimeBy(500) // 1750 ms
-        runOnIdle {
-            // second animation finished
-            assertThat(total).isEqualTo(0f)
-        }
+        // second animation finished
+        assertThat(total).isEqualTo(0f)
 
         mainClock.advanceTimeBy(500) // 2250 ms
-        runOnIdle {
-            // in the middle of the third animation
-            assertThat(total).isGreaterThan(0f)
-            assertThat(total).isLessThan(100f)
-        }
+        // in the middle of the third animation
+        assertThat(total).isGreaterThan(0f)
+        assertThat(total).isLessThan(100f)
 
         mainClock.advanceTimeBy(500) // 2750 ms
-        runOnIdle {
-            // third animation finished
-            assertThat(total).isEqualTo(100f)
-        }
+        // third animation finished
+        assertThat(total).isEqualTo(100f)
     }
 
     @Test
diff --git a/compose/material/material/src/desktopTest/kotlin/androidx/compose/material/DesktopMenuTest.kt b/compose/material/material/src/desktopTest/kotlin/androidx/compose/material/DesktopMenuTest.kt
@@ -223,19 +223,15 @@ class DesktopMenuTest {
             }
         }
 
-        fun performKeyDownAndUp(key: Key) {
-            onNodeWithTag("dropDownMenu").apply {
-                performKeyPress(KeyEvent(key, KeyEventType.KeyDown))
-                performKeyPress(KeyEvent(key, KeyEventType.KeyUp))
-            }
+        fun performKeyDownAndUp(key: Key) = onNodeWithTag("dropDownMenu").apply {
+            performKeyPress(KeyEvent(key, KeyEventType.KeyDown))
+            performKeyPress(KeyEvent(key, KeyEventType.KeyUp))
         }
 
         fun assertClicksCount(i1: Int, i2: Int, i3: Int) {
-            runOnIdle {
-                assertThat(item1Clicked).isEqualTo(i1)
-                assertThat(item2Clicked).isEqualTo(i2)
-                assertThat(item3Clicked).isEqualTo(i3)
-            }
+            assertThat(item1Clicked).isEqualTo(i1)
+            assertThat(item2Clicked).isEqualTo(i2)
+            assertThat(item3Clicked).isEqualTo(i3)
         }
 
         performKeyDownAndUp(Key.DirectionDown)
diff --git a/compose/material3/material3/src/skikoTest/kotlin/androidx/compose/material3/ExposedDropdownMenuTest.kt b/compose/material3/material3/src/skikoTest/kotlin/androidx/compose/material3/ExposedDropdownMenuTest.kt
@@ -28,7 +28,6 @@ import androidx.compose.foundation.lazy.LazyColumn
 import androidx.compose.foundation.rememberScrollState
 import androidx.compose.foundation.verticalScroll
 import androidx.compose.runtime.Composable
-import androidx.compose.runtime.SideEffect
 import androidx.compose.runtime.getValue
 import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.remember
@@ -65,6 +64,7 @@ import androidx.compose.ui.unit.IntSize
 import androidx.compose.ui.unit.dp
 import kotlin.test.Ignore
 import kotlin.test.Test
+import kotlin.test.assertTrue
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.launch
@@ -210,7 +210,7 @@ class ExposedDropdownMenuTest {
                     ) {
                         TextField(
                             modifier = Modifier
-                                .menuAnchor()
+                                .menuAnchor(MenuAnchorType.PrimaryEditable)
                                 .then(
                                     if (index == testIndex) Modifier
                                         .testTag(TFTag)
@@ -282,7 +282,11 @@ class ExposedDropdownMenuTest {
         setContent {
             scrollState = rememberScrollState()
             scope = rememberCoroutineScope()
-            Column(Modifier.verticalScroll(scrollState)) {
+            Column(
+                modifier = Modifier
+                    .height(500.dp)
+                    .verticalScroll(scrollState)
+            ) {
                 Spacer(Modifier.height(300.dp))
 
                 val expanded = false
@@ -291,7 +295,7 @@ class ExposedDropdownMenuTest {
                     onExpandedChange = {},
                 ) {
                     TextField(
-                        modifier = Modifier.menuAnchor(),
+                        modifier = Modifier.menuAnchor(MenuAnchorType.PrimaryNotEditable),
                         readOnly = true,
                         value = "",
                         onValueChange = {},
@@ -302,9 +306,7 @@ class ExposedDropdownMenuTest {
                         onDismissRequest = {},
                         content = {},
                     )
-                    SideEffect {
-                        compositionCount++
-                    }
+                    compositionCount++
                 }
 
                 Spacer(Modifier.height(300.dp))
@@ -313,13 +315,13 @@ class ExposedDropdownMenuTest {
 
         assertThat(compositionCount).isEqualTo(1)
 
-        runOnIdle {
-            scope.launch {
-                scrollState.animateScrollBy(500f)
-            }
+        scope.launch {
+            scrollState.animateScrollBy(500f)
         }
+        mainClock.advanceTimeBy(10_000)
         waitForIdle()
 
+        assertTrue(scrollState.value > 0)  // Make sure we did scroll
         assertThat(compositionCount).isEqualTo(1)
     }
 
@@ -344,9 +346,7 @@ class ExposedDropdownMenuTest {
         onNodeWithTag(TFTag).assertIsDisplayed()
         onNodeWithTag(MenuItemTag).assertIsDisplayed()
 
-        runOnIdle {
-            assertThat(menuBounds.width).isEqualTo(textFieldBounds.width)
-        }
+        assertThat(menuBounds.width).isEqualTo(textFieldBounds.width)
     }
 
     @Test
@@ -382,7 +382,7 @@ class ExposedDropdownMenuTest {
                 ) {
                     scrollState = rememberScrollState()
                     TextField(
-                        modifier = Modifier.menuAnchor(),
+                        modifier = Modifier.menuAnchor(MenuAnchorType.PrimaryNotEditable),
                         value = "",
                         onValueChange = { },
                         label = { Text("Label") },
@@ -403,14 +403,10 @@ class ExposedDropdownMenuTest {
             }
         }
 
-        runOnIdle {
-            CoroutineScope(Dispatchers.Unconfined).launch {
-                scrollState.scrollTo(scrollState.maxValue)
-            }
+        CoroutineScope(Dispatchers.Unconfined).launch {
+            scrollState.scrollTo(scrollState.maxValue)
         }
 
-        waitForIdle()
-
         onNodeWithTag("MenuContent 1").assertIsNotDisplayed()
         onNodeWithTag("MenuContent 100").assertIsDisplayed()
     }
@@ -423,7 +419,7 @@ class ExposedDropdownMenuTest {
                 onExpandedChange = { },
             ) {
                 TextField(
-                    modifier = Modifier.menuAnchor(),
+                    modifier = Modifier.menuAnchor(MenuAnchorType.PrimaryNotEditable),
                     value = "",
                     onValueChange = { },
                     label = { Text("Label") },
@@ -458,7 +454,7 @@ class ExposedDropdownMenuTest {
             ) {
                 TextField(
                     modifier = Modifier
-                        .menuAnchor()
+                        .menuAnchor(MenuAnchorType.PrimaryEditable)
                         .testTag(TFTag)
                         .onGloballyPositioned {
                             onTextFieldBoundsChanged?.invoke(it.boundsInRoot())
diff --git a/compose/ui/ui-test-junit4/src/desktopTest/kotlin/androidx/compose/ui/test/TestBasicsTest.kt b/compose/ui/ui-test-junit4/src/desktopTest/kotlin/androidx/compose/ui/test/TestBasicsTest.kt
@@ -22,6 +22,7 @@ import androidx.compose.foundation.layout.size
 import androidx.compose.material.Text
 import androidx.compose.runtime.LaunchedEffect
 import androidx.compose.runtime.getValue
+import androidx.compose.runtime.mutableIntStateOf
 import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.remember
 import androidx.compose.runtime.setValue
@@ -239,4 +240,45 @@ class TestBasicsTest {
         waitForIdle()
         assertEquals(initialTime, mainClock.currentTime)
     }
+
+    @Test
+    fun runOnIdleExecutesOnUiThread() = runComposeUiTest {
+        lateinit var mainThread: Thread
+        setContent {
+            mainThread = Thread.currentThread()
+        }
+        runOnIdle {
+            assertEquals(Thread.currentThread(), mainThread)
+        }
+    }
+
+    @Test
+    fun runOnIdleExecutesWhenIdle() = runComposeUiTest {
+        var sourceValue by mutableIntStateOf(0)
+        var targetValue = 0
+        setContent {
+            LaunchedEffect(sourceValue) {
+                targetValue = sourceValue
+            }
+        }
+        sourceValue = 1
+        runOnIdle {
+            assertEquals(targetValue, 1)
+        }
+    }
+
+    @Test
+    fun runOnIdleDoesNotWaitForIdleAfterward() = runComposeUiTest {
+        var sourceValue by mutableIntStateOf(0)
+        var targetValue = 0
+        setContent {
+            LaunchedEffect(sourceValue) {
+                targetValue = sourceValue
+            }
+        }
+        runOnIdle {
+            sourceValue = 1
+        }
+        assertEquals(targetValue, 0)
+    }
 }
diff --git a/compose/ui/ui-test/src/skikoMain/kotlin/androidx/compose/ui/test/ComposeUiTest.skikoMain.kt b/compose/ui/ui-test/src/skikoMain/kotlin/androidx/compose/ui/test/ComposeUiTest.skikoMain.kt
@@ -302,11 +302,8 @@ class SkikoComposeUiTest @InternalTestApi constructor(
     }
 
     override fun <T> runOnIdle(action: () -> T): T {
-        // We are waiting for idle before and AFTER `action` to guarantee that changes introduced
-        // in `action` are propagated to components. In Android's version, it's executed in the
-        // Main thread which has similar effects.
         waitForIdle()
-        return action().also { waitForIdle() }
+        return runOnUiThread(action)
     }
 
     override fun waitUntil(

Original file line number	Diff line number	Diff line change
`@@ -52,9 +52,7 @@ class ToggleableTest {`
`52`	`52`	`).testTag("toggle"))`
`53`	`53`	`}`
`54`	`54`
`55`		`- runOnIdle {`
`56`		`- assertEquals(false, state)`
`57`		`- }`
	`55`	`+ assertEquals(false, state)`
`58`	`56`
`59`	`57`	`onRoot().apply {`
`60`	`58`	`performKeyPress(KeyEvent(Key.Tab, KeyEventType.KeyDown))`
`@@ -66,17 +64,13 @@ class ToggleableTest {`
`66`	`64`	`performKeyPress(KeyEvent(Key.Spacebar, KeyEventType.KeyUp))`
`67`	`65`	`}`
`68`	`66`
`69`		`- runOnIdle {`
`70`		`- assertEquals(true, state)`
`71`		`- }`
	`67`	`+ assertEquals(true, state)`
`72`	`68`
`73`	`69`	`onNodeWithTag("toggle").apply {`
`74`	`70`	`performKeyPress(KeyEvent(Key.Spacebar, KeyEventType.KeyDown))`
`75`	`71`	`performKeyPress(KeyEvent(Key.Spacebar, KeyEventType.KeyUp))`
`76`	`72`	`}`
`77`	`73`
`78`		`- runOnIdle {`
`79`		`- assertEquals(false, state)`
`80`		`- }`
	`74`	`+ assertEquals(false, state)`
`81`	`75`	`}`
`82`	`76`	`}`
Original file line number	Diff line number	Diff line change
`@@ -223,19 +223,15 @@ class DesktopMenuTest {`
`223`	`223`	`}`
`224`	`224`	`}`
`225`	`225`
`226`		`- fun performKeyDownAndUp(key: Key) {`
`227`		`- onNodeWithTag("dropDownMenu").apply {`
`228`		`- performKeyPress(KeyEvent(key, KeyEventType.KeyDown))`
`229`		`- performKeyPress(KeyEvent(key, KeyEventType.KeyUp))`
`230`		`- }`
	`226`	`+ fun performKeyDownAndUp(key: Key) = onNodeWithTag("dropDownMenu").apply {`
	`227`	`+ performKeyPress(KeyEvent(key, KeyEventType.KeyDown))`
	`228`	`+ performKeyPress(KeyEvent(key, KeyEventType.KeyUp))`
`231`	`229`	`}`
`232`	`230`
`233`	`231`	`fun assertClicksCount(i1: Int, i2: Int, i3: Int) {`
`234`		`- runOnIdle {`
`235`		`- assertThat(item1Clicked).isEqualTo(i1)`
`236`		`- assertThat(item2Clicked).isEqualTo(i2)`
`237`		`- assertThat(item3Clicked).isEqualTo(i3)`
`238`		`- }`
	`232`	`+ assertThat(item1Clicked).isEqualTo(i1)`
	`233`	`+ assertThat(item2Clicked).isEqualTo(i2)`
	`234`	`+ assertThat(item3Clicked).isEqualTo(i3)`
`239`	`235`	`}`
`240`	`236`
`241`	`237`	`performKeyDownAndUp(Key.DirectionDown)`
Original file line number	Diff line number	Diff line change
`@@ -302,11 +302,8 @@ class SkikoComposeUiTest @InternalTestApi constructor(`
`302`	`302`	`}`
`303`	`303`
`304`	`304`	`override fun <T> runOnIdle(action: () -> T): T {`
`305`		- // We are waiting for idle before and AFTER `action` to guarantee that changes introduced
`306`		- // in `action` are propagated to components. In Android's version, it's executed in the
`307`		`- // Main thread which has similar effects.`
`308`	`305`	`waitForIdle()`
`309`		`- return action().also { waitForIdle() }`
	`306`	`+ return runOnUiThread(action)`
`310`	`307`	`}`
`311`	`308`
`312`	`309`	`override fun waitUntil(`