JetBrains / compose-multiplatform

Compose Multiplatform, a modern UI framework for Kotlin that makes building performant and beautiful user interfaces easy and enjoyable.
https://jetbrains.com/lp/compose-multiplatform
Apache License 2.0
16.01k stars 1.17k forks source link

Integration tests suddenly not working #1336

Closed codeslubber closed 1 year ago

codeslubber commented 2 years ago

We had them working after reading through this article (and adding some android classes):

But then we ran into a situation where it was complaining that the lateinit window was never initialized. We tried wrapping the content in a Window {} (both the new version and the deprecated one). Did not work. Then we upgraded to the latest. Now it says scene not found.

kotlin.UninitializedPropertyAccessException: lateinit property scene has not been initialized
    at androidx.compose.ui.test.junit4.DesktopComposeTestRule.getScene(DesktopComposeTestRule.desktop.kt:72)
    at androidx.compose.ui.test.junit4.DesktopComposeTestRule$DesktopTestOwner.getRoots(DesktopComposeTestRule.desktop.kt:210)
    at androidx.compose.ui.test.TestContext.getAllSemanticsNodes$ui_test(TestOwner.kt:95)
    at androidx.compose.ui.test.SemanticsNodeInteraction.fetchSemanticsNodes$ui_test(SemanticsNodeInteraction.kt:79)
    at androidx.compose.ui.test.SemanticsNodeInteraction.fetchOneOrDie(SemanticsNodeInteraction.kt:145)
    at androidx.compose.ui.test.SemanticsNodeInteraction.assertExists(SemanticsNodeInteraction.kt:137)
    at com.ontometrics.mobtimer.view.ShiftViewTests.testTimeChange(ShiftViewTests.kt:30)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:566)
    at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:725)
    at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
    at org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:131)
    at org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:149)
    at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:140)
    at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestMethod(TimeoutExtension.java:84)
    at org.junit.jupiter.engine.execution.ExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(ExecutableInvoker.java:115)
    at org.junit.jupiter.engine.execution.ExecutableInvoker.lambda$invoke$0(ExecutableInvoker.java:105)
    at org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:106)
    at org.junit.jupiter.engine.execution.InvocationInterceptorChain.proceed(InvocationInterceptorChain.java:64)
    at org.junit.jupiter.engine.execution.InvocationInterceptorChain.chainAndInvoke(InvocationInterceptorChain.java:45)
    at org.junit.jupiter.engine.execution.InvocationInterceptorChain.invoke(InvocationInterceptorChain.java:37)
    at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:104)
    at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:98)
    at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeTestMethod$7(TestMethodTestDescriptor.java:214)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeTestMethod(TestMethodTestDescriptor.java:210)
    at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:135)
    at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:66)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:151)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
    at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1541)
    at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
    at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
jimgoog commented 2 years ago

But then we ran into a situation

What changed? Did it break when you bumped the Compose version or something else? Do you have a minimized/sharable sample which gets it into this state? It is going to be hard to debug without a simplified/sharable reproducer.

cc @theapache64 Looks like the OP is using your template. Have you upgraded the template to the latest Compose MPP release by chance? Have you ever run into this error?

codeslubber commented 2 years ago

I didn't use the template, just pulled in some of the dependencies I saw in there. I guess it did fail after one of the upgrades, I think we upgraded GP?

I have a repo, I can add someone to it. It's private right now.

theapache64 commented 2 years ago

@jimgoog I've never seen this error before. It is hard to identify the issue without the code. @codeslubber Can you share reproducible code with the kotlinVersion, gradleVersion and composeVersion you're using? Something like this

codeslubber commented 2 years ago

Here's the test:

import androidx.compose.ui.test.junit4.createComposeRule
import androidx.compose.ui.test.onNodeWithText
import androidx.compose.ui.test.printToString
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.runBlocking
import org.hamcrest.MatcherAssert.assertThat
import org.hamcrest.Matchers.equalTo
import org.hamcrest.Matchers.greaterThan
import org.junit.Before
import org.junit.Rule
import org.junit.jupiter.api.Test
import java.time.LocalDateTime

class ShiftViewTests {
    @get:Rule
    val composeRule = createComposeRule()

    private val timerEndCallback:(message:String) -> Unit = {}

    @Before
    fun setUp() {
        composeRule.setContent {
            TimerView(LocalDateTime.now().plusMinutes(5), timerEndCallback)
        }
    }

    @Test
    fun testTimeChange() { //FIXME test not running
        runBlocking(Dispatchers.Main) {
            val textNode = composeRule.onNodeWithText("Elapsed time: ", substring = true)
                .assertExists("Could not find elapsed time")
            val textString = textNode.printToString()
            val value1 =
                textString.substring(textString.indexOf("[") + 1, textString.indexOf("]")).split(" ").last().toInt()
            Thread.sleep(1000)
            val textString2 = textNode.printToString()
            val value2 =
                textString2.substring(textString.indexOf("[") + 1, textString.indexOf("]")).split(" ").last().toInt()
            assertThat(value2, greaterThan(value1))
            assertThat(1, equalTo(2))
        }

    }

}

Here's the gradle file:

import org.jetbrains.compose.compose
import org.jetbrains.compose.desktop.application.dsl.TargetFormat
import org.jetbrains.kotlin.gradle.tasks.KotlinCompile

plugins {
    kotlin("jvm") version "1.5.31"
    id("org.jetbrains.compose") version "1.0.0-beta5"
}

group = "me.robwilliams"
version = "1.0"

repositories {
    google()
    mavenCentral()
    maven("https://maven.pkg.jetbrains.space/public/p/compose/dev")
}

dependencies {
    implementation(compose.desktop.currentOs)
    testImplementation("androidx.annotation:annotation:1.2.0")
    testImplementation(compose("org.jetbrains.compose.ui:ui-test-junit4-desktop"))
    testImplementation("org.junit.jupiter:junit-jupiter:5.8.1")
    testImplementation(platform("org.junit:junit-bom:5.8.1"))
    testImplementation("org.junit.jupiter:junit-jupiter:5.8.1")
    testImplementation("org.hamcrest:hamcrest-all:1.3")
}

tasks.withType<Test>().configureEach {
    useJUnitPlatform()
}

tasks.withType<KotlinCompile>() {
    kotlinOptions.jvmTarget = "11"
}

compose.desktop {
    application {
        mainClass = "MainKt"
        nativeDistributions {
            targetFormats(TargetFormat.Dmg, TargetFormat.Msi, TargetFormat.Deb)
            packageName = "com.ontometrics.mobtimer"
            packageVersion = "1.0.0"
        }
    }
}
codeslubber commented 2 years ago

I looked at the code for the DesktopComposeTestRule:

lateinit var scene: ComposeScene

private val testOwner = DesktopTestOwner()
private val testContext = createTestContext(testOwner)

override fun apply(base: Statement, description: Description?): Statement {
      return object : Statement() {
          override fun evaluate() {
              scene = runOnUiThread(::createUi)

              try {
                  base.evaluate()
              } finally {
                  runOnUiThread(scene::close)
              }

              coroutineDispatcher.cleanupTestCoroutines()
              uncaughtExceptionHandler.throwUncaught()
          }
      }
}

Well seems pretty clear, no? we never get inside the evaluate() or don't get there before the first assertion is made in the test, which asks for the root and fails.

Sure enough a breakpoint placed at the assignment of scene never fires.

Guess the tests for the tests didn't work, eh?

jimgoog commented 2 years ago

Wow, this is bad. Minimal repro looks like:

import androidx.compose.material.Text
import androidx.compose.ui.test.junit4.createComposeRule
import org.junit.Rule
import org.junit.jupiter.api.Test

class ShiftViewTests {
    @get:Rule
    val composeRule = createComposeRule()

    @Test
    fun testTimeChange() {
        composeRule.setContent {
            Text("Test")
        }
    }
}

How did we miss this? Are we still not running desktop tests in Google CI? Seems should turn that on.

cc @igordmn can you take a look?

igordmn commented 2 years ago

Are we still not running desktop tests in Google CI

We run them on our CI, all tests pass.

Seems should turn that on.

That would be great, but there are a couple of tests which are failing in androidx-main, and fixed in jb-main. We plan to upstream all fixes after CfD 1.0.

lateinit property scene has not been initialized

The issue seems old, as lateinit property + evaluate has been around for about a year.

Temporal workaround is calling setContent only in @Test methods, not in @Before methods.

igordmn commented 2 years ago

Temporal workaround is calling setContent only in @Test methods, not in @Before methods.

It seems I was wrong. Before should be calling in evaluate.

I have checked beta5, and tests work, even if we use setContent in @Before.

I used desktop-template and added:

testImplementation(compose("org.jetbrains.compose.ui:ui-test-junit4"))

This and this tests work.

My guess, you mix junit5 and junit4. Try to remove testImplementation("org.junit.jupiter:junit-jupiter:5.8.1") and replace:

org.junit.jupiter.api.Test

by

org.junit.Test
codeslubber commented 2 years ago

The two changes resulted in runner finding no tests. Maybe have to comment the 5 runner too? lemme try that...

codeslubber commented 2 years ago

That worked:

//tasks.withType<Test>().configureEach {
//    useJUnitPlatform()
//}

Thanks!

codeslubber commented 2 years ago

Kind of insane that we can't use JUnit 5 but should maybe add that to the readme.

jimgoog commented 2 years ago

We use junit4 internally and you imported org.jetbrains.compose.ui:ui-test-junit4-desktop as a dependency presumably to reuse the test rules we use instead of writing your own. There is nothing stopping you from writing a junit5 test rule. We can't support every version of every test library, although it does seem likely we would add junit5 at some point in the near-ish future. Certainly I think we would take community contributions to add such support.

In the meantime, the error above is rather surprising / non-obvious. Might be worth adding something that gives a more sensible error message if the user is running a junit4 test with a junit5 engine. It's unfortunate that junit doesn't automatically detect such misconfigurations and give helpful messages.

codeslubber commented 2 years ago

Yeah sorry if my redux was dismissive, I am down on JUnit in general. 5 doesn't really add that much, we only use it to be on a chain that is under active dev, 4 was released 15 years ago.

Is that all this was though? I also wonder why the test was working when we first did it. We put the 5 runner in when we first made the project so there's no way our original runs were with 4.

Totally agree that the error is as the Pauli quote goes: 'not even wrong.'

igordmn commented 2 years ago

add that to the readme.

Yeah, we will add this to readme in https://github.com/JetBrains/compose-jb/issues/368.

There is nothing stopping you from writing a junit5 test rule

It is probably not trivial on the user side. And there are a couple of things that are marked as @InternalTestApi

Might be worth adding something that gives a more sensible error message

We can add an assert to all public methods with the message:

Compose test rule isn't initialized:

1. Check if `@get:Rule` is applied:

@get:Rule
val composeRule = createComposeRule()

2. Check if you use JUnit 4, and not JUnit 5. The import of `@Test` should be `org.junit.Test`, not `org.junit.jupiter.api.Test`

I also wonder why the test was working when we first did it.

JUnit 5 can't use org.junit.Rule. Maybe you had JUnit 5 in your classpath, but marked your tests with org.junit.Test annotation.

serandel commented 2 years ago

We can't support every version of every test library, although it does seem likely we would add junit5 at some point in the near-ish future.

Taking into consideration that JUnit 4 is 16 years old and JUnit 5 already 5 years old, I would say that official support for the latest major version of the industry standard is way overdue. Of course, the sheer majority of the blame falls into Google for their lack of JUnit 5 support for Android testing.

It is probably not trivial on the user side.

It's not really that difficult. I'm sure there will be compromises, but simply copying the two classes from https://github.com/mannodermaus/android-junit5/issues/234#issuecomment-950169571 allowed me to run a JUnit5 UI test in Compose for Desktop.

@ExtendWith(ComposeRuleExtension::class)
@Ignore
class MyUITest {
    @Test
    fun buttonIsShown(runner: ComposeRuleRunner) = runner.run {
        // Start the app
        setContent {
            MaterialTheme {
                App()
            }
        }

        onNodeWithText("Hello", substring = true).performClick()

        onNodeWithText("Hi there", substring = true).assertIsDisplayed()
    }
}
igordmn commented 1 year ago

Closing this in favor to more specific issues:

okushnikov commented 2 months ago

Please check the following ticket on YouTrack for follow-ups to this issue. GitHub issues will be closed in the coming weeks.