Skip to content

Conversation

@eymar
Copy link
Member

@eymar eymar commented May 1, 2025

The corresponding CL - https://android-review.googlesource.com/c/platform/frameworks/support/+/3509670

Note: API changes here only in the experimental API.

Fixes https://youtrack.jetbrains.com/issue/CMP-7994/

Testing

Added a copy of relevant tests from AOSP.

Release Notes

Features - Multiple Platforms

  • Adopted a change in ComposeUiTest API. The block in runComposeUiTest is suspend now. It allows to call awaitIdle and other suspend functions. It ensures a correct execution of a test on all platforms. See the web specifics in kotlinx.coroutines.test.runTest documentation.

@eymar eymar requested review from igordmn and m-sasha May 1, 2025 10:19
return synchronized(lock) { roots.toSet() }
}

fun <R> withRegistry(block: () -> R): R {
Copy link
Member Author

Choose a reason for hiding this comment

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

Some exaplanation here:

calling withRegistry and passing a block withkotlinx.coroutines.test.runTest call is not correct for web - because kotlinx.coroutines.test.runTest returns immediately.

So I split the logic from this function:

  • setupRegistry is called before the scene is created and before calling kotlinx.coroutines.test.runTest
  • tearDownRegistry is called inside of kotlinx.coroutines.test.runTest block (in finally)

block: suspend SkikoComposeUiTest.() -> Unit
): TestResult {
composeRootRegistry.setupRegistry()
scene = runOnUiThread(::createUi)
Copy link
Member Author

@eymar eymar May 1, 2025

Choose a reason for hiding this comment

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

Some explanation:

Creating the scene manually instead of using withScene {.
We need an effectContext of the recomposer to be passed to the runTestContext, so with the current structure of classes, the scene must be created before we run kotlinx.coroutines.test.runTest(. See combinedCoroutineContext.

This requirement is ensured by this test:

@eymar
Copy link
Member Author

eymar commented May 1, 2025

Note: the failing test on web (androidx.compose.ui.input.TextInputTests.compositeInputWebkit[wasmJs, browser, Chrome135.0.0.0, Linuxx86_64]) doesn't use the ComposeUiTest API.

package androidx.compose.ui.test

@OptionalExpectation
expect annotation class IgnoreWebTest() No newline at end of file
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick: Let's use the same pattern between modules. Let's have separate annotations for each build target (IgnoreJs + IgnoreWasm) and just apply both where required.

But it's minor, so up to you "Web" in name is descriptive enough

@eymar eymar merged commit 7caa6bf into jb-main May 13, 2025
8 of 10 checks passed
@eymar eymar deleted the ok/adopt_new_composeUiTest branch May 13, 2025 08:45
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.

5 participants