Adopt IdlingResource move from commonMain to supporting platforms #1822
Conversation
Reasoning: Compose for Web can't support blocking waiting for idling resources. Several methods in `ui-test` require blocking waiting for idling resources (for example waitForIdle). A non-idle resource would block a test using it. Since `IdlingResource` usages can’t be reliably implemented across all Kotlin targets, we choose to exclude it from `commonMain` to prevent its usage in common tests. Test: N/A Change-Id: I3df6e216fa62b7268bfe3aeb7788214eb527d68e
| @@ -126,7 +124,7 @@ fun defaultTestDispatcher() = UnconfinedTestDispatcher() | |||
| */ | |||
| @ExperimentalTestApi | |||
| @OptIn(InternalTestApi::class, InternalComposeUiApi::class) | |||
| class SkikoComposeUiTest @InternalTestApi constructor( | |||
| open class SkikoComposeUiTest @InternalTestApi constructor( | |||
There was a problem hiding this comment.
Why I think it's okay to make it open:
- it's experimental, so such changes are okay. If someone uses/extends it, it's at their own risk.
- SkikoComposeUiTest is not available in common tests (with Android), which I believe is the main place where the tests should be added in Compose Multiplatform case. Those tests will use only
ComposeUiTestinterface. - In the future, I believe we will need to update this API, since the intention is to not expose "skiko". SkikoComposeUiTest exposes
scene, which is convenient, but it's not supposed to be used outside of Compose - InternalComposeUiApi.
| @@ -124,47 +121,6 @@ class TestBasicsTest { | |||
| } | |||
| } | |||
|
|
|||
| @Test | |||
| fun testIdlingResource() = runComposeUiTest { | |||
There was a problem hiding this comment.
It's moved to IdlingResourceTest in deskopTest
There was a problem hiding this comment.
I'd move it to DesktopTestsTest. I don't think it deserves its own file.
(if you do move it, please also fix the typo in the kdoc for that class: "specified" -> "specific").
| fun runDesktopTest(block: DesktopComposeUiTest.() -> Unit) { | ||
| runTest { | ||
| block() | ||
| } | ||
| } |
There was a problem hiding this comment.
This function looks a bit weird here. It's member of DesktopComposeUiTest, but you don't ever want to call it inside the test itself. The same actually applies to SkikoComposeUiTest.runTest too.
We could make it internal, or get rid of it completely, replacing it with
with(DesktopComposeUiTest(width, height, effectContext)) {
runTest { block() }
}
runTest could just be made internal (because SkikoComposeUiTest is experimental).
There was a problem hiding this comment.
-
SkikoComposeUiTest.runTest can't be internal because it's used in DesktopComposeTestRule (ui-test-junit4 module).
-
got rid of
runDesktopTest
| @@ -124,47 +121,6 @@ class TestBasicsTest { | |||
| } | |||
| } | |||
|
|
|||
| @Test | |||
| fun testIdlingResource() = runComposeUiTest { | |||
There was a problem hiding this comment.
I'd move it to DesktopTestsTest. I don't think it deserves its own file.
(if you do move it, please also fix the typo in the kdoc for that class: "specified" -> "specific").
CL: https://android-review.googlesource.com/c/platform/frameworks/support/+/3460196
Main changes:
API changes happend only for Experimtal APIs.