From d082fce0e1bd56e224a0003c6ab0ca0059392dff Mon Sep 17 00:00:00 2001 From: Marcel Schnelle Date: Sat, 13 Apr 2024 14:15:32 +0200 Subject: [PATCH] Discard JUnit 5 test runners even after creation, if further filters result in it not contributing any tests (#326) This is interesting for `@Tag`-annotated classes that are skipped completely. If their runner is not prevented from being returned to the instrumentation, Android expects something reported back to it for the class, but in reality JUnit 5 won't be reporting anything, causing a mismatch error --- instrumentation/CHANGELOG.md | 3 +- instrumentation/core/build.gradle.kts | 21 ++++- .../de/mannodermaus/junit5/TaggedTests.kt | 25 +++++ .../junit5/AndroidJUnit5Builder.kt | 16 +--- .../AndroidJUnitPlatformRunnerListener.kt | 13 ++- .../runners/AndroidJUnitPlatformTestTree.kt | 15 ++- .../junit5/internal/runners/DummyJUnit5.kt | 15 ++- .../internal/runners/JUnit5RunnerFactory.kt | 44 ++++++--- .../junit5/AndroidJUnit5BuilderTests.kt | 93 +++++++++++-------- 9 files changed, 161 insertions(+), 84 deletions(-) create mode 100644 instrumentation/core/src/androidTest/java/de/mannodermaus/junit5/TaggedTests.kt diff --git a/instrumentation/CHANGELOG.md b/instrumentation/CHANGELOG.md index 1b929f49..e74604e5 100644 --- a/instrumentation/CHANGELOG.md +++ b/instrumentation/CHANGELOG.md @@ -6,7 +6,8 @@ Change Log - Fix inheritance hierarchy of `ComposeExtension` to avoid false-positive warning regarding `@RegisterExtension` (#318) - Improve parallel test execution for Android instrumentation tests - Fix invalid naming of dynamic tests when executing only a singular test method from the IDE (#317) -- Prevent test methods incorrectly defined as Kotlin top-level functions from messing up Android's internal test counting, causing issues like "Expected N+1 tests, received N" (#316) +- Prevent test methods incorrectly defined as Kotlin top-level functions from messing up Android's internal test counting, causing issues like "Expected N+1 tests, received N" (#316) +- Prevent test classes ignored by a tag from being considered for test execution, causing issues like "Expected N+1 tests, received N" (#298) ## 1.4.0 (2023-11-05) diff --git a/instrumentation/core/build.gradle.kts b/instrumentation/core/build.gradle.kts index ff45c7e8..22aea72d 100644 --- a/instrumentation/core/build.gradle.kts +++ b/instrumentation/core/build.gradle.kts @@ -51,8 +51,10 @@ android { } junitPlatform { - // Using local dependency instead of Maven coordinates - instrumentationTests.enabled = false + filters { + // See TaggedTests.kt for usage of this tag + excludeTags("nope") + } } tasks.withType { @@ -68,6 +70,20 @@ tasks.withType { } } +// Use local project dependencies on android-test instrumentation libraries +// instead of relying on their Maven coordinates for this module +val instrumentationLibraryRegex = Regex("de\\.mannodermaus\\.junit5:android-test-(.+):") + +configurations.all { + if ("debugAndroidTestRuntimeClasspath" in name) { + resolutionStrategy.dependencySubstitution.all { + instrumentationLibraryRegex.find(requested.toString())?.let { result -> + useTarget(project(":${result.groupValues[1]}")) + } + } + } +} + dependencies { implementation(libs.kotlinStdLib) implementation(libs.junitJupiterApi) @@ -87,7 +103,6 @@ dependencies { androidTestImplementation(libs.junitJupiterParams) androidTestImplementation(libs.espressoCore) androidTestRuntimeOnly(project(":runner")) - androidTestRuntimeOnly(libs.junitJupiterEngine) testImplementation(project(":testutil")) } diff --git a/instrumentation/core/src/androidTest/java/de/mannodermaus/junit5/TaggedTests.kt b/instrumentation/core/src/androidTest/java/de/mannodermaus/junit5/TaggedTests.kt new file mode 100644 index 00000000..ea5a7508 --- /dev/null +++ b/instrumentation/core/src/androidTest/java/de/mannodermaus/junit5/TaggedTests.kt @@ -0,0 +1,25 @@ +package de.mannodermaus.junit5 + +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Tag +import org.junit.jupiter.api.Test + +class TaggedTests { + @Test + fun includedTest() { + } + + @Tag("nope") + @Test + fun taggedTestDisabledOnMethodLevel() { + assertEquals(5, 2 + 2) + } +} + +@Tag("nope") +class TaggedTestsDisabledOnClassLevel { + @Test + fun excludedTest() { + assertEquals(5, 2 + 2) + } +} diff --git a/instrumentation/runner/src/main/kotlin/de/mannodermaus/junit5/AndroidJUnit5Builder.kt b/instrumentation/runner/src/main/kotlin/de/mannodermaus/junit5/AndroidJUnit5Builder.kt index 4c2eb1e6..58b27c89 100644 --- a/instrumentation/runner/src/main/kotlin/de/mannodermaus/junit5/AndroidJUnit5Builder.kt +++ b/instrumentation/runner/src/main/kotlin/de/mannodermaus/junit5/AndroidJUnit5Builder.kt @@ -1,9 +1,8 @@ package de.mannodermaus.junit5 import android.util.Log -import de.mannodermaus.junit5.internal.runners.JUnit5RunnerFactory.createJUnit5Runner import de.mannodermaus.junit5.internal.LOG_TAG -import de.mannodermaus.junit5.internal.extensions.jupiterTestMethods +import de.mannodermaus.junit5.internal.runners.tryCreateJUnit5Runner import org.junit.runner.Runner import org.junit.runners.model.RunnerBuilder @@ -60,16 +59,11 @@ public class AndroidJUnit5Builder : RunnerBuilder() { @Throws(Throwable::class) override fun runnerForClass(testClass: Class<*>): Runner? { try { - if (!junit5Available) { - return null + return if (junit5Available) { + tryCreateJUnit5Runner(testClass) + } else { + null } - - if (testClass.jupiterTestMethods().isEmpty()) { - return null - } - - return createJUnit5Runner(testClass) - } catch (e: NoClassDefFoundError) { Log.e(LOG_TAG, "JUnitPlatform not found on runtime classpath") throw IllegalStateException( diff --git a/instrumentation/runner/src/main/kotlin/de/mannodermaus/junit5/internal/runners/AndroidJUnitPlatformRunnerListener.kt b/instrumentation/runner/src/main/kotlin/de/mannodermaus/junit5/internal/runners/AndroidJUnitPlatformRunnerListener.kt index 99140598..831cbefc 100644 --- a/instrumentation/runner/src/main/kotlin/de/mannodermaus/junit5/internal/runners/AndroidJUnitPlatformRunnerListener.kt +++ b/instrumentation/runner/src/main/kotlin/de/mannodermaus/junit5/internal/runners/AndroidJUnitPlatformRunnerListener.kt @@ -63,11 +63,14 @@ internal class AndroidJUnitPlatformRunnerListener( ) { val description = testTree.getDescription(testIdentifier) val status = testExecutionResult.status - if (status == TestExecutionResult.Status.ABORTED) { - notifier.fireTestAssumptionFailed(toFailure(testExecutionResult, description)) - } else if (status == TestExecutionResult.Status.FAILED) { - notifier.fireTestFailure(toFailure(testExecutionResult, description)) - } else if (testIdentifier.isTest) { + + if (testIdentifier.isTest) { + if (status == TestExecutionResult.Status.ABORTED) { + notifier.fireTestAssumptionFailed(toFailure(testExecutionResult, description)) + } else if (status == TestExecutionResult.Status.FAILED) { + notifier.fireTestFailure(toFailure(testExecutionResult, description)) + } + notifier.fireTestFinished(description) } } diff --git a/instrumentation/runner/src/main/kotlin/de/mannodermaus/junit5/internal/runners/AndroidJUnitPlatformTestTree.kt b/instrumentation/runner/src/main/kotlin/de/mannodermaus/junit5/internal/runners/AndroidJUnitPlatformTestTree.kt index 9d9baf9c..a2d53c48 100644 --- a/instrumentation/runner/src/main/kotlin/de/mannodermaus/junit5/internal/runners/AndroidJUnitPlatformTestTree.kt +++ b/instrumentation/runner/src/main/kotlin/de/mannodermaus/junit5/internal/runners/AndroidJUnitPlatformTestTree.kt @@ -131,12 +131,19 @@ internal class AndroidJUnitPlatformTestTree( parent: Description, testPlan: TestPlan ) { - val newDescription = createJUnit4Description(identifier, testPlan) - parent.addChild(newDescription) - descriptions[identifier] = newDescription + val newDescription = createJUnit4Description(identifier, testPlan).also { + descriptions[identifier] = it + } + + val newParent = if (identifier.isTest || identifier.isDynamicTest) { + parent.addChild(newDescription) + newDescription + } else { + parent + } testPlan.getChildren(identifier).forEach { child -> - buildDescription(child, newDescription, testPlan) + buildDescription(child, newParent, testPlan) } } diff --git a/instrumentation/runner/src/main/kotlin/de/mannodermaus/junit5/internal/runners/DummyJUnit5.kt b/instrumentation/runner/src/main/kotlin/de/mannodermaus/junit5/internal/runners/DummyJUnit5.kt index 5b3def5c..c0e98d81 100644 --- a/instrumentation/runner/src/main/kotlin/de/mannodermaus/junit5/internal/runners/DummyJUnit5.kt +++ b/instrumentation/runner/src/main/kotlin/de/mannodermaus/junit5/internal/runners/DummyJUnit5.kt @@ -2,18 +2,19 @@ package de.mannodermaus.junit5.internal.runners import android.util.Log import de.mannodermaus.junit5.internal.LOG_TAG -import de.mannodermaus.junit5.internal.extensions.jupiterTestMethods import org.junit.runner.Description import org.junit.runner.Runner import org.junit.runner.notification.RunNotifier +import java.lang.reflect.Method /** * Fake Runner that marks all JUnit 5 methods as ignored, * used for old devices without Java 8 capabilities. */ -internal class DummyJUnit5(private val testClass: Class<*>) : Runner() { - - private val testMethods = testClass.jupiterTestMethods() +internal class DummyJUnit5( + private val testClass: Class<*>, + private val testMethods: Set, +) : Runner() { override fun run(notifier: RunNotifier) { Log.w( @@ -27,5 +28,9 @@ internal class DummyJUnit5(private val testClass: Class<*>) : Runner() { } } - override fun getDescription(): Description = Description.createSuiteDescription(testClass) + override fun getDescription(): Description = Description.createSuiteDescription(testClass).also { + testMethods.forEach { method -> + it.addChild(Description.createTestDescription(testClass, method.name)) + } + } } diff --git a/instrumentation/runner/src/main/kotlin/de/mannodermaus/junit5/internal/runners/JUnit5RunnerFactory.kt b/instrumentation/runner/src/main/kotlin/de/mannodermaus/junit5/internal/runners/JUnit5RunnerFactory.kt index aaeb8261..0c0dae87 100644 --- a/instrumentation/runner/src/main/kotlin/de/mannodermaus/junit5/internal/runners/JUnit5RunnerFactory.kt +++ b/instrumentation/runner/src/main/kotlin/de/mannodermaus/junit5/internal/runners/JUnit5RunnerFactory.kt @@ -1,20 +1,36 @@ package de.mannodermaus.junit5.internal.runners import android.os.Build +import de.mannodermaus.junit5.internal.extensions.jupiterTestMethods import org.junit.runner.Runner -internal object JUnit5RunnerFactory { - /** - * Since we can't reference AndroidJUnit5 directly, use this factory for instantiation. - * - * On API 26 and above, delegate to the real implementation to drive JUnit 5 tests. - * Below that however, they wouldn't work; for this case, delegate a dummy runner - * which will highlight these tests as ignored. - */ - internal fun createJUnit5Runner(klass: Class<*>): Runner = - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { - AndroidJUnit5(klass) - } else { - DummyJUnit5(klass) - } +/** + * Since we can't reference AndroidJUnit5 directly, use this factory for instantiation. + * + * On API 26 and above, delegate to the real implementation to drive JUnit 5 tests. + * Below that however, they wouldn't work; for this case, delegate a dummy runner + * which will highlight these tests as ignored. + */ +internal fun tryCreateJUnit5Runner(klass: Class<*>): Runner? { + val testMethods = klass.jupiterTestMethods() + + if (testMethods.isEmpty()) { + return null + } + + val runner = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { + AndroidJUnit5(klass) + } else { + DummyJUnit5(klass, testMethods) + } + + // It's still possible for the runner to not be relevant to the test run, + // which is related to how further filters are applied (e.g. via @Tag). + // Only return the runner to the instrumentation if it has any tests to contribute, + // otherwise there would be a mismatch between the number of test classes reported + // to Android, and the number of test classes actually tested with JUnit 5 (ref #298) + return runner.takeIf(Runner::hasExecutableTests) } + +private fun Runner.hasExecutableTests() = + this.description.children.isNotEmpty() diff --git a/instrumentation/runner/src/test/kotlin/de/mannodermaus/junit5/AndroidJUnit5BuilderTests.kt b/instrumentation/runner/src/test/kotlin/de/mannodermaus/junit5/AndroidJUnit5BuilderTests.kt index 9c892215..873bef22 100644 --- a/instrumentation/runner/src/test/kotlin/de/mannodermaus/junit5/AndroidJUnit5BuilderTests.kt +++ b/instrumentation/runner/src/test/kotlin/de/mannodermaus/junit5/AndroidJUnit5BuilderTests.kt @@ -1,60 +1,71 @@ package de.mannodermaus.junit5 +import android.os.Build import com.google.common.truth.Truth.assertThat -import org.junit.jupiter.api.Test -import org.junit.jupiter.params.ParameterizedTest -import org.junit.jupiter.params.provider.ValueSource +import de.mannodermaus.junit5.testutil.AndroidBuildUtils.withApiLevel +import de.mannodermaus.junit5.testutil.AndroidBuildUtils.withMockedInstrumentation +import org.junit.jupiter.api.DynamicContainer.dynamicContainer +import org.junit.jupiter.api.DynamicNode +import org.junit.jupiter.api.DynamicTest.dynamicTest +import org.junit.jupiter.api.TestFactory class AndroidJUnit5BuilderTests { private val builder = AndroidJUnit5Builder() - @Test - fun `no runner is created if class only contains top-level test methods`() { + @TestFactory + fun `no runner is created if class only contains top-level test methods`() = runTest( + expectSuccess = false, // In Kotlin, a 'Kt'-suffixed class of top-level functions cannot be referenced // via the ::class syntax, so construct a reference to the class directly - val cls = Class.forName(javaClass.packageName + ".TestClassesKt") - - // Top-level tests should be discarded, so no runner must be created for this class - runTest(cls, expectSuccess = false) - } + Class.forName(javaClass.packageName + ".TestClassesKt") + ) - @ValueSource( - classes = [ - HasTest::class, - HasRepeatedTest::class, - HasTestFactory::class, - HasTestTemplate::class, - HasParameterizedTest::class, - HasInnerClassWithTest::class, - HasTaggedTest::class, - HasInheritedTestsFromClass::class, - HasInheritedTestsFromInterface::class, - ] + @TestFactory + fun `runner is created correctly for classes with valid jupiter test methods`() = runTest( + expectSuccess = true, + HasTest::class.java, + HasRepeatedTest::class.java, + HasTestFactory::class.java, + HasTestTemplate::class.java, + HasParameterizedTest::class.java, + HasInnerClassWithTest::class.java, + HasTaggedTest::class.java, + HasInheritedTestsFromClass::class.java, + HasInheritedTestsFromInterface::class.java, ) - @ParameterizedTest - fun `runner is created correctly for classes with valid jupiter test methods`(cls: Class<*>) = - runTest(cls, expectSuccess = true) - - @ValueSource( - classes = [ - DoesntHaveTestMethods::class, - HasJUnit4Tests::class, - kotlin.time.Duration::class, - ] + + @TestFactory + fun `no runner is created if class has no jupiter test methods`() = runTest( + expectSuccess = false, + DoesntHaveTestMethods::class.java, + HasJUnit4Tests::class.java, + kotlin.time.Duration::class.java, ) - @ParameterizedTest - fun `no runner is created if class has no jupiter test methods`(cls: Class<*>) = - runTest(cls, expectSuccess = false) /* Private */ - private fun runTest(cls: Class<*>, expectSuccess: Boolean) { - val runner = builder.runnerForClass(cls) - if (expectSuccess) { - assertThat(runner).isNotNull() - } else { - assertThat(runner).isNull() + private fun runTest(expectSuccess: Boolean, vararg classes: Class<*>): List { + // Generate a test container for each given class, + // then create two sub-variants for testing both DummyJUnit5 and AndroidJUnit5 + return classes.map { cls -> + dynamicContainer( + /* displayName = */ cls.name, + /* dynamicNodes = */ setOf(Build.VERSION_CODES.M, Build.VERSION_CODES.TIRAMISU).map { apiLevel -> + dynamicTest("API Level $apiLevel") { + withMockedInstrumentation { + withApiLevel(apiLevel) { + val runner = builder.runnerForClass(cls) + if (expectSuccess) { + assertThat(runner).isNotNull() + } else { + assertThat(runner).isNull() + } + } + } + } + } + ) } } }