robolectric / robolectric

Android Unit Testing Framework
http://robolectric.org
Other
5.91k stars 1.37k forks source link

ClickableText offset value posted by compose rule test node click is different (end position) in 4.9.1 (and 4.9.2) than in 4.9 (start position) #7900

Open alwa opened 1 year ago

alwa commented 1 year ago

Description

I have a test for some code in Jetpack compose, including a "androidx.compose.foundation.text.ClickableText" My ClickableText has its text attribute set to an AnnotatedString which consists of several parts. My test finds the relevant text (matches the first part of this AnnotatedString) and clicks on it. So I would expect the offset to be 0 (which it is in Robolectric 4.9). But in 4.9.1 / 4.9.2 the offset is instead "82", which happens to be the full length of the annotated string, so basically the end position of the string.

Steps to Reproduce

Prod code:

@Composable
fun TestComposable(onLinkClicked: (AnnotatedString.Range<String>) -> Unit) {
    LinkText(
        modifier = Modifier,
        linkTextData = listOf(
            LinkTextData(
                tag = "not null",
                annotation = "not null",
                text = "License agreement",
                onClick = onLinkClicked,
            ),
            LinkTextData(". "),
            LinkTextData(
                text = "Please review it and give your consent by checking the checkbox",
            )
        )
    )
}

@Composable
private fun LinkText(modifier: Modifier = Modifier, linkTextData: List<LinkTextData>) {
    val annotatedString = createAnnotatedString(linkTextData)
    ClickableText(
        modifier = modifier,
        text = annotatedString,
        onClick = { offset ->
               if (offset != 0) { 
                   throw IllegalStateException("Bug!")
               }
            }
        },
    )
}

data class LinkTextData(
    val text: String,
    val tag: String? = null,
    val annotation: String? = null,
    val onClick: ((str: AnnotatedString.Range<String>) -> Unit)? = null,
)

// This is a hack to be able to make part of a string clickable, https://stackoverflow.com/a/69549929/1859486
@Composable
private fun createAnnotatedString(data: List<LinkTextData>): AnnotatedString {
    return buildAnnotatedString {
        data.forEach { linkTextData ->
            if (linkTextData.tag != null && linkTextData.annotation != null) {
                pushStringAnnotation(
                    tag = linkTextData.tag,
                    annotation = linkTextData.annotation,
                )
                withStyle(
                    style = SpanStyle(
                        color = MaterialTheme.colorScheme.primary,
                        textDecoration = TextDecoration.Underline,
                    ),
                ) {
                    append(linkTextData.text)
                }
                pop()
            } else {
                append(linkTextData.text)
            }
        }
    }
}

Test code:

@Config(sdk = [Build.VERSION_CODES.S])
@RunWith(AndroidJUnit4::class)
@LooperMode(LooperMode.Mode.PAUSED)
class TestClass {

   @get:Rule
    val composeTestRule = createComposeRule()

    @Test
    fun reproducible() {
        var onLinkClickedClicked = false
        composeTestRule.setContent {
            TestComposable(onLinkClicked = { onLinkClickedClicked = true })
        }

        composeTestRule.onAllNodesWithText("License agreement", substring = true)
            .assertCountEquals(1)
        composeTestRule.onNodeWithText("License agreement", substring = true)
            .performClick()

        assertTrue(onLinkClickedClicked)
    }

}

Robolectric & Android Version

libs.versions.toml:

[versions]
androidx-compose-compiler = "1.3.2"
androidx-compose-material3 = "1.0.1"
androidx-compose-ui = "1.3.2"
androidx-compose-runtime = "1.3.2"

[libraries]
androidxComposeCompiler = { module = "androidx.compose.compiler:compiler", version.ref = "androidx-compose-compiler" }
androidxComposeRuntimeLivedata = { module = "androidx.compose.runtime:runtime-livedata", version.ref = "androidx-compose-runtime" }
androidxComposeMaterialMaterial3 = { module = "androidx.compose.material3:material3", version.ref = "androidx-compose-material3" }
androidxComposeUiTest = { module = "androidx.compose.ui:ui-test-junit4", version.ref = "androidx-compose-ui" }
androidxComposeUiTooling = { module = "androidx.compose.ui:ui-tooling", version.ref = "androidx-compose-ui" }
androidxComposeTestManifest = { module = "androidx.compose.ui:ui-test-manifest", version.ref = "androidx-compose-ui" }

Link to a public git repo demonstrating the problem:

utzcoz commented 1 year ago

I found there is only one commit that is related to compose and cherry-picked for 4.9.1 and 4.9.2: https://github.com/robolectric/robolectric/commit/ed2d7d3d600972090de29bcf9ad37d65a4d7ef47.

utzcoz commented 1 year ago

@alwa Could you check the variable that is updated in onClick in tests? Like this one: https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/ClickableTest.kt;l=289-314?q=performClick&ss=androidx&start=11.

alwa commented 1 year ago

I found there is only one commit that is related to compose and cherry-picked for 4.9.1 and 4.9.2: ed2d7d3.

More context, I have this explicit dependency in the project:

// Temporary workaround, https://github.com/robolectric/robolectric/issues/6593#issuecomment-974858115
testImplementation("androidx.test.espresso:espresso-core:3.5.0")

If I remove it and run the test above I get this:

java.util.concurrent.ExecutionException: java.lang.RuntimeException: java.lang.IllegalAccessException: class androidx.test.espresso.base.ThreadPoolExecutorExtractor$2 cannot access a member of class androidx.loader.content.ModernAsyncTask with modifiers "public static final"
java.lang.RuntimeException: java.util.concurrent.ExecutionException: java.lang.RuntimeException: java.lang.IllegalAccessException: class androidx.test.espresso.base.ThreadPoolExecutorExtractor$2 cannot access a member of class androidx.loader.content.ModernAsyncTask with modifiers "public static final"
    at androidx.test.espresso.Espresso.onIdle(Espresso.java:35)
    at androidx.test.espresso.Espresso.onIdle(Espresso.java:21)
    at androidx.compose.ui.test.junit4.EspressoLink_androidKt.runEspressoOnIdle(EspressoLink.android.kt:92)
    at androidx.compose.ui.test.junit4.RobolectricIdlingStrategy$runUntilIdle$1.invoke(RobolectricIdlingStrategy.android.kt:66)
    at androidx.compose.ui.test.junit4.RobolectricIdlingStrategy$runUntilIdle$1.invoke(RobolectricIdlingStrategy.android.kt:48)
    at androidx.compose.ui.test.junit4.AndroidSynchronization_androidKt.runOnUiThread(AndroidSynchronization.android.kt:33)
    at androidx.compose.ui.test.junit4.RobolectricIdlingStrategy.runUntilIdle(RobolectricIdlingStrategy.android.kt:48)
    at androidx.compose.ui.test.AndroidComposeUiTestEnvironment.runTest(ComposeUiTest.android.kt:286)
    at androidx.compose.ui.test.junit4.AndroidComposeTestRule$apply$1.evaluate(AndroidComposeTestRule.android.kt:147)
    at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
    at org.robolectric.RobolectricTestRunner$HelperTestRunner$1.evaluate(RobolectricTestRunner.java:580)
    at org.robolectric.internal.SandboxTestRunner$2.lambda$evaluate$2(SandboxTestRunner.java:287)
    at org.robolectric.internal.bytecode.Sandbox.lambda$runOnMainThread$0(Sandbox.java:99)
    at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
    at java.base/java.lang.Thread.run(Thread.java:829)
utzcoz commented 1 year ago

More context, I have this an explicity dependency in the project:

4.9.1 integrates Espresso 3.5 default. I think it doesn't affect.

alwa commented 1 year ago

@alwa Could you check the variable that is updated in onClick in tests? Like this one: https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/ClickableTest.kt;l=289-314?q=performClick&ss=androidx&start=11.

I had to replace an assertion (Truth) with an equivalent and ran this test:

    @Test
    fun clickableTest_clickOnChildBasicText() {
        var counter = 0
        val onClick: () -> Unit = { ++counter }

        rule.setContent {
            Box(modifier = Modifier.clickable(onClick = onClick)) {
                BasicText("Foo")
                BasicText("Bar")
            }
        }

        rule.onNodeWithText("Foo", substring = true).assertExists()
        rule.onNodeWithText("Bar", substring = true).assertExists()

        rule.onNodeWithText("Foo", substring = true).performClick()

        rule.runOnIdle {
            assertEquals(1,counter)
        }

        rule.onNodeWithText("Bar", substring = true).performClick()

        rule.runOnIdle {
            assertEquals(2,counter)
        }
    }

which passed. Did you want a specific value or is it enough to know that the test passed?

utzcoz commented 1 year ago

More context, I have this an explicity dependency in the project:

4.9.1 integrates Espresso 3.5 default. I think it doesn't affect.

Oh, Robolectric only adds espresso-idle, it might need you to add espresso-core as dependency if you still have problem.

utzcoz commented 1 year ago

Did you want a specific value or is it enough to know that the test passed?

It depends on your need. The counter in this example is just used to calculate the number that onClick is called. You can customize it based on your need instead of using offset in your previous tests.

alwa commented 1 year ago

Oh okay, I thought this was for troubleshooting. Will look into converting my test :) Thanks

utzcoz commented 1 year ago

I found there is only one commit that is related to compose and cherry-picked for 4.9.1 and 4.9.2: ed2d7d3.

I think this commit has a strong correlation with the offset is not 0 after you upgrade to 4.9.1/4.9.2. I am not familiar with Jetpack Compose, and it needs @hoisie 's help to take a look at this behavior change.

@alwa You can convert your test based on previous examples to test your code behavior with 4.9.1/4.9.2. And I will close this issue.

alwa commented 1 year ago

I'm afraid I haven't been able to rewrite my test so that is passes in 4.9.1 or 4.9.2. I get the same kind of behavior regardless. The problem, I guess, is that the use of offset is in the production implementation (which behaves correctly), but the offset value (incorrect, in my opinion) is posted by the test.

So to me it looks like the following behavior has changed:

In 4.9 clicks on Compose rule nodes would happen on the start position of the matching node whereas in 4.9.1 the click would instead happen on the end position. This is problematic if trying to verify clicks that only cover parts of the node. The start position would match but not the end position.

utzcoz commented 1 year ago

@alwa What is the real behavior on the Emulator or real device?

alwa commented 1 year ago

@alwa What is the real behavior on the Emulator or real device?

Well, the problem here is in the test code as the test posts the offset (implicitly through a "click" action) that is processed by the production code. But I can confirm that my code works on a real device, the expected clickable part is clickable and the expected non-clickable part is non-clickable.

So to be clear: Compose rule nodes is test code, not production code.

I have updated the title of this ticket to reflect my current understanding of the problem

hoisie commented 1 year ago

@alwa can you put up a GitHub repo with a repro of the issue? That would be super helpful. I think this is probably related to ed2d7d3.

alwa commented 1 year ago

@alwa can you put up a GitHub repo with a repro of the issue? That would be super helpful. I think this is probably related to ed2d7d3.

I've started to set up such a project. Some issue with the manifest and I won't have more time today to look into it. But here it is:

https://github.com/alwa/robolectric-issues-7900

hoisie commented 1 year ago

@alwa I cloned that repo and ran ./gradlew testDebugUnitTest but everything passed.

alixwar commented 1 year ago

@alwa I cloned that repo and ran ./gradlew testDebugUnitTest but everything passed.

Hi, I'm not done setting up the project so there could be errors. I had to pause the effort due to vacation. Will try to make a proper reproducible when back.

satvik2131 commented 1 year ago

I want to work on this issue can you please assign me this issue , thank you

realdadfish commented 1 year ago

I see the same issue with 4.9.2 and 4.10-alpha-1, trying to come up with a reproducing example.

realdadfish commented 1 year ago

Here is my try, in this case it's even worse, the clickedOffset is always returned as 0.

import android.app.Application
import android.content.ComponentName
import android.content.pm.ActivityInfo
import androidx.activity.ComponentActivity
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.text.ClickableText
import androidx.compose.ui.test.junit4.createComposeRule
import androidx.compose.ui.test.onNodeWithText
import androidx.compose.ui.test.performClick
import androidx.compose.ui.text.AnnotatedString
import androidx.compose.ui.text.buildAnnotatedString
import androidx.test.core.app.ApplicationProvider
import junit.framework.TestCase.assertEquals
import org.junit.Rule
import org.junit.Test
import org.junit.rules.TestWatcher
import org.junit.runner.Description
import org.junit.runner.RunWith
import org.robolectric.RobolectricTestRunner
import org.robolectric.Shadows.shadowOf

class RegisterComponentActivityRule : TestWatcher() {
    override fun starting(description: Description?) {
        val appContext = ApplicationProvider.getApplicationContext<Application>()
        shadowOf(appContext.packageManager).addOrUpdateActivity(ActivityInfo().apply {
            name = ComponentActivity::class.java.name
            packageName = appContext.packageName
        })
    }

    override fun finished(description: Description?) {
        val appContext = ApplicationProvider.getApplicationContext<Application>()
        shadowOf(appContext.packageManager).removeActivity(
            ComponentName(
                ComponentActivity::class.java.name,
                appContext.packageName
            )
        )
    }
}

@RunWith(RobolectricTestRunner::class)
class Reproducer {

    @get:Rule(order = 0)
    val registration = RegisterComponentActivityRule()

    @get:Rule(order = 1)
    val compose = createComposeRule()

    @Test
    fun reproducer() {
        var clicked: String? = null
        compose.setContent {
            Box {
                val annotatedString = buildAnnotatedString {
                    append("Link to Android: ")
                    appendLink(
                        "android",
                        "https://www.android.com"
                    )
                    append("\nLink to Google: ")
                    appendLink(
                        "google",
                        "https://www.google.com"
                    )
                    append("\nThat's it.")
                }
                ClickableText(text = annotatedString) { clickedOffset ->
                    clicked = annotatedString.getClickedLink(clickedOffset) ?: "wrong offset: $clickedOffset"
                }
            }
        }

        compose.onNodeWithText("android", substring = true).performClick()
        assertEquals("https://www.android.com", clicked)

        compose.onNodeWithText("google", substring = true).performClick()
        assertEquals("https://www.google.com", clicked)
    }

    private fun AnnotatedString.Builder.appendLink(linkText: String, url: String) {
        pushStringAnnotation(tag = "URL", annotation = url)
        append(linkText)
        pop()
    }

    private fun AnnotatedString.getClickedLink(offset: Int): String? =
        getStringAnnotations(tag = "URL", start = offset, end = offset).firstOrNull()?.item
}
hoisie commented 1 year ago

Have you tried @GraphicsMode(NATIVE) or passing the robolectric.graphicsMode=NATIVE system property?

Currently native graphics is not enabled by default as it's still in the early phases (though it should be very stable).

realdadfish commented 1 year ago

@hoisie Ah, forgot to try that, thanks for the reminder. With 4.10-alpha1 and NATIVE graphics mode the tests fail as well, the first clicked offset is however reported to be 36 (expected would be [17,23)).

hoisie commented 1 year ago

@realdadfish what SDK level is the test running? The best supported SDK level for native graphics is Android S (SDK 31) or S_V2 (SDK 32).

realdadfish commented 1 year ago

We're on SDK 33, will downgrade and see what changes.

hoisie commented 1 year ago

I'll try to repro this issue and see what is happening.

Are you using any custom text layout related shadows? i.e. a shadow for LineBreaker, StaticLayout, MeasuredText, MeasuredParagraph, etc.?

realdadfish commented 1 year ago

No, not at all, in the original production code (and the test above) we output basic text, nothing fancy.

Downgrading to SDK 32 didn't change anything, the index is still reported as 36.

hoisie commented 1 year ago

Thanks for reporting this issue, there is definitely something weird going on here. I can repro it. The next step is to determine the lower level Android sdk calls being made and see what the issue is.

hoisie commented 1 year ago

FYI am planning to take a closer look at this. It's a complex issue that involves a lot of Compose internals, and I have a feeling that the issue is more related to Compose's coordinate logic, but I'll post any findings here.

hoisie commented 1 year ago

@realdadfish I am a little confused about this error, maybe you could provide some more clarification.

When I do compose.onNodeWithText("android", substring = true).captureToBitmap() and then dump the Bitmap in a Robolectric environment with RNG enabled, it looks like this:

image

And then when compose.onNodeWithText("android", substring = true).performClick() is called, looking at the compose code, it defaults to click on the center of the node. However, the center of the code does not look like it's the android link.

If I rejigger the text to have the android link be in the middle: image

The assertion works.

Is there a way in Compose to perform a click at a specific position? I am not too familiar with the Compose API.

realdadfish commented 1 year ago

Hrm... I had a deep dive into the compose-test implementation and I guess the API that I'm looking for just does not exist and apparently others have stumbled upon this as well: https://issuetracker.google.com/issues/237987769

Reason is that compose-test only uses the tagged annotated string as a selector for a specific node, but then looses all the information / context about the node and simply offers to click the whole thing.

So it's entirely not Robolectric's fault - sorry for the noise :-(

realdadfish commented 1 month ago

A new API seems to be present in ui-test 1.8.0-alphas. Usage (per kdoc):

composeTestRule
  .onNodeWithText("YOUR_TEXT_WITH_LINK")
  .performFirstLinkClick {
     (it.item as? LinkAnnotation.Url)?.url == "YOUR_URL"
  }