diff --git a/instrumentation/CHANGELOG.md b/instrumentation/CHANGELOG.md index e74604e5..4ba7c679 100644 --- a/instrumentation/CHANGELOG.md +++ b/instrumentation/CHANGELOG.md @@ -5,7 +5,7 @@ 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) +- Fix invalid naming of dynamic tests when executing only a singular test method from the IDE (#317, #339) - 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) diff --git a/instrumentation/runner/src/main/kotlin/de/mannodermaus/junit5/internal/extensions/TestIdentifierExt.kt b/instrumentation/runner/src/main/kotlin/de/mannodermaus/junit5/internal/extensions/TestIdentifierExt.kt index f7169987..ab2cf710 100644 --- a/instrumentation/runner/src/main/kotlin/de/mannodermaus/junit5/internal/extensions/TestIdentifierExt.kt +++ b/instrumentation/runner/src/main/kotlin/de/mannodermaus/junit5/internal/extensions/TestIdentifierExt.kt @@ -1,5 +1,6 @@ package de.mannodermaus.junit5.internal.extensions +import de.mannodermaus.junit5.internal.formatters.TestNameFormatter import org.junit.platform.launcher.TestIdentifier private val DYNAMIC_TEST_PREFIXES = listOf( @@ -29,3 +30,12 @@ internal val TestIdentifier.isDynamicTest: Boolean val shortId = this.shortId return DYNAMIC_TEST_PREFIXES.any { shortId.startsWith(it) } } + +/** + * Returns a formatted version of this identifier's name, + * which is compatible with the quirks and limitations + * of the Android Instrumentation, esp. when the [isIsolatedMethodRun] + * flag is enabled. + */ +internal fun TestIdentifier.format(isIsolatedMethodRun: Boolean = false): String = + TestNameFormatter.format(this, isIsolatedMethodRun) diff --git a/instrumentation/runner/src/main/kotlin/de/mannodermaus/junit5/internal/formatters/TestNameFormatter.kt b/instrumentation/runner/src/main/kotlin/de/mannodermaus/junit5/internal/formatters/TestNameFormatter.kt new file mode 100644 index 00000000..62220a0d --- /dev/null +++ b/instrumentation/runner/src/main/kotlin/de/mannodermaus/junit5/internal/formatters/TestNameFormatter.kt @@ -0,0 +1,42 @@ +package de.mannodermaus.junit5.internal.formatters + +import org.junit.platform.launcher.TestIdentifier + +/** + * A class for naming Jupiter test methods in a compatible manner, + * taking into account several limitations imposed by the + * Android instrumentation (e.g. on isolated test runs). + */ +internal object TestNameFormatter { + fun format(identifier: TestIdentifier, isIsolatedMethodRun: Boolean = false): String { + // During isolated executions of a single test method, + // construct a technical version of its name for backwards compatibility + // with the JUnit 4-based instrumentation of Android by stripping the brackets of parameterized tests completely. + // If this didn't happen, running them from the IDE will cause "No tests found" errors. + // See AndroidX's TestRequestBuilder$MethodFilter for where this is cross-referenced in the instrumentation! + // + // History: + // - #199 & #207 (the original unearthing of this behavior) + // - #317 (making an exception for dynamic tests) + // - #339 (retain indices of parameterized methods to avoid premature filtering by JUnit 4's test discovery) + if (isIsolatedMethodRun) { + val reportName = identifier.legacyReportingName + val paramStartIndex = reportName.indexOf('(') + if (paramStartIndex > -1) { + val result = reportName.substring(0, paramStartIndex) + + val paramEndIndex = reportName.lastIndexOf('[') + + return if (paramEndIndex > -1) { + // Retain suffix of parameterized methods (i.e. "[1]", "[2]" etc) + // so that they won't be filtered out by JUnit 4 on isolated method runs + result + reportName.substring(paramEndIndex) + } else { + result + } + } + } + + return identifier.displayName.replace("()", "") + } +} 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 0719e209..b489baa2 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 @@ -3,6 +3,7 @@ package de.mannodermaus.junit5.internal.runners import android.annotation.SuppressLint +import de.mannodermaus.junit5.internal.extensions.format import de.mannodermaus.junit5.internal.extensions.isDynamicTest import org.junit.platform.commons.util.AnnotationUtils import org.junit.platform.engine.UniqueId @@ -35,53 +36,40 @@ internal class AndroidJUnitPlatformTestTree( val suiteDescription = generateSuiteDescription(testPlan, testClass) - fun getTestName(identifier: TestIdentifier): String = - when { - identifier.isContainer -> getTechnicalName(identifier) - - identifier.isDynamicTest -> { - // Collect all dynamic tests' IDs from this identifier, - // all the way up to the first non-dynamic test. - // Collect the name of all these into a list, then finally - // compose the final name from this list. Note that, because we - // move upwards the test plan, the elements must be reversed - // before the final name can be composed. - val nameComponents = mutableListOf() - var currentNode: TestIdentifier? = identifier - while (currentNode != null && currentNode.isDynamicTest) { - nameComponents.add(formatTestName(currentNode)) - currentNode = modifiedTestPlan.getRealParent(currentNode).orElse(null) - } - nameComponents.reverse() - - // Android's Unified Test Platform (AGP 7.0+) is using literal test names - // to create files when capturing Logcat output during execution. - // Ergo, make sure that only legal characters are being used in the test names - // (ref. https://github.com/mannodermaus/android-junit5/issues/263) - nameComponents.joinToString(" - ") + // Order matters here, since all dynamic tests are also containers, + // but not all containers are dynamic tests + fun getTestName(identifier: TestIdentifier): String = when { + identifier.isDynamicTest -> if (isIsolatedMethodRun) { + // In isolated method runs, there is no need to compose + // dynamic test names from multiple pieces, as the + // Android Instrumentation only looks at the raw method name + // anyway and all information about parameter types is lost + identifier.format(true) + } else { + // Collect all dynamic tests' IDs from this identifier, + // all the way up to the first non-dynamic test. + // Collect the name of all these into a list, then finally + // compose the final name from this list. Note that, because we + // move upwards the test plan, the elements must be reversed + // before the final name can be composed. + val nameComponents = mutableListOf() + var currentNode: TestIdentifier? = identifier + while (currentNode != null && currentNode.isDynamicTest) { + nameComponents.add(currentNode.format(false)) + currentNode = modifiedTestPlan.getRealParent(currentNode).orElse(null) } + nameComponents.reverse() - else -> formatTestName(identifier) + // Android's Unified Test Platform (AGP 7.0+) is using literal test names + // to create files when capturing Logcat output during execution. + // Ergo, make sure that only legal characters are being used in the test names + // (ref. https://github.com/mannodermaus/android-junit5/issues/263) + nameComponents.joinToString(" - ") } - private fun formatTestName(identifier: TestIdentifier): String { - // During isolated executions of non-dynamic @Test methods, - // construct a technical version of the test name for backwards compatibility - // with the JUnit 4-based instrumentation of Android by stripping the brackets and parameters completely. - // If this didn't happen, running them from the IDE will cause "No tests found" errors. - // See AndroidX's TestRequestBuilder$MethodFilter for where this is cross-referenced in the instrumentation! - // - // Ref issues #199 & #207 (the original unearthing of this behavior), - // as well as #317 (making an exception for dynamic tests). - if (isIsolatedMethodRun && !identifier.isDynamicTest) { - val reportName = identifier.legacyReportingName - val bracketIndex = reportName.indexOf('(') - if (bracketIndex > -1) { - return reportName.substring(0, bracketIndex) - } - } + identifier.isContainer -> getTechnicalName(identifier) - return identifier.displayName.replace("()", "") + else -> identifier.format(isIsolatedMethodRun) } // Do not expose our custom TestPlan, because JUnit Platform wouldn't like that very much. diff --git a/instrumentation/runner/src/test/kotlin/de/mannodermaus/junit5/TestClasses.kt b/instrumentation/runner/src/test/kotlin/de/mannodermaus/junit5/TestClasses.kt index f8155ab0..3e75ab2d 100644 --- a/instrumentation/runner/src/test/kotlin/de/mannodermaus/junit5/TestClasses.kt +++ b/instrumentation/runner/src/test/kotlin/de/mannodermaus/junit5/TestClasses.kt @@ -46,6 +46,8 @@ class HasTestTemplate { private fun context(param: String): TestTemplateInvocationContext = object : TestTemplateInvocationContext { + override fun getDisplayName(invocationIndex: Int): String = param + override fun getAdditionalExtensions() = listOf( object : ParameterResolver { override fun supportsParameter(parameterContext: ParameterContext, extensionContext: ExtensionContext) = diff --git a/instrumentation/runner/src/test/kotlin/de/mannodermaus/junit5/TestHelpers.kt b/instrumentation/runner/src/test/kotlin/de/mannodermaus/junit5/TestHelpers.kt new file mode 100644 index 00000000..26be67cb --- /dev/null +++ b/instrumentation/runner/src/test/kotlin/de/mannodermaus/junit5/TestHelpers.kt @@ -0,0 +1,30 @@ +package de.mannodermaus.junit5 + +import org.junit.platform.engine.discovery.DiscoverySelectors +import org.junit.platform.launcher.Launcher +import org.junit.platform.launcher.TestPlan +import org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder +import org.junit.platform.launcher.core.LauncherFactory +import kotlin.reflect.KClass + +/** + * A quick one-liner for executing a Jupiter discover-and-execute pass + * from inside of a Jupiter test. Useful for testing runner code + * that needs to work with the innards of the [TestPlan], such as + * individual test identifiers and such. + */ +fun discoverTests( + cls: KClass<*>, + launcher: Launcher = LauncherFactory.create(), + executeAsWell: Boolean = true, +): TestPlan { + return launcher.discover( + LauncherDiscoveryRequestBuilder.request() + .selectors(DiscoverySelectors.selectClass(cls.java)) + .build() + ).also { plan -> + if (executeAsWell) { + launcher.execute(plan) + } + } +} diff --git a/instrumentation/runner/src/test/kotlin/de/mannodermaus/junit5/internal/formatters/TestNameFormatterTests.kt b/instrumentation/runner/src/test/kotlin/de/mannodermaus/junit5/internal/formatters/TestNameFormatterTests.kt new file mode 100644 index 00000000..7ec92ab1 --- /dev/null +++ b/instrumentation/runner/src/test/kotlin/de/mannodermaus/junit5/internal/formatters/TestNameFormatterTests.kt @@ -0,0 +1,108 @@ +package de.mannodermaus.junit5.internal.formatters + +import com.google.common.truth.Truth.assertThat +import de.mannodermaus.junit5.HasParameterizedTest +import de.mannodermaus.junit5.HasRepeatedTest +import de.mannodermaus.junit5.HasTest +import de.mannodermaus.junit5.HasTestFactory +import de.mannodermaus.junit5.HasTestTemplate +import de.mannodermaus.junit5.discoverTests +import de.mannodermaus.junit5.internal.extensions.format +import org.junit.jupiter.api.Test +import org.junit.platform.engine.discovery.DiscoverySelectors +import org.junit.platform.launcher.TestIdentifier +import org.junit.platform.launcher.TestPlan +import org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder +import org.junit.platform.launcher.core.LauncherFactory +import kotlin.reflect.KClass + +class TestNameFormatterTests { + + @Test + fun `normal junit5 test`() = runTestWith(HasTest::class) { identifier -> + assertThat(identifier.format(false)).isEqualTo("method") + assertThat(identifier.format(true)).isEqualTo("method") + } + + @Test + fun `repeated test`() = runTestWith(HasRepeatedTest::class) { identifier -> + assertThat(identifier.format(false)).isEqualTo("method(RepetitionInfo)") + assertThat(identifier.format(true)).isEqualTo("method") + + // Inspect individual executions, too + assertChildren(identifier, expectedCount = 5) { index, child -> + val number = index + 1 + assertThat(child.format(false)).isEqualTo("repetition $number of 5") + assertThat(child.format(true)).isEqualTo("method[$number]") + } + } + + @Test + fun `test factory`() = runTestWith(HasTestFactory::class) { identifier -> + assertThat(identifier.format(false)).isEqualTo("method") + assertThat(identifier.format(true)).isEqualTo("method") + + // Inspect individual executions, too + assertChildren(identifier, expectedCount = 2) { index, child -> + val number = index + 1 + assertThat(child.format(false)).isEqualTo(if (index == 0) "a" else "b") + assertThat(child.format(true)).isEqualTo("method[$number]") + } + } + + @Test + fun `test template`() = runTestWith(HasTestTemplate::class) { identifier -> + assertThat(identifier.format(false)).isEqualTo("method(String)") + assertThat(identifier.format(true)).isEqualTo("method") + + // Inspect individual executions, too + assertChildren(identifier, expectedCount = 2) { index, child -> + val number = index + 1 + assertThat(child.format(false)).isEqualTo("param$number") + assertThat(child.format(true)).isEqualTo("method[$number]") + } + } + + @Test + fun `parameterized test`() = runTestWith(HasParameterizedTest::class) { identifier -> + assertThat(identifier.format(false)).isEqualTo("method(String)") + assertThat(identifier.format(true)).isEqualTo("method") + + // Inspect individual executions, too + assertChildren(identifier, expectedCount = 2) { index, child -> + val number = index + 1 + assertThat(child.format(false)).isEqualTo("[$number] " + if (index == 0) "a" else "b") + assertThat(child.format(true)).isEqualTo("method[$number]") + } + } + + /* Private */ + + private fun runTestWith(cls: KClass<*>, block: TestPlan.(TestIdentifier) -> Unit) { + // Discover and execute the test plan of the given class + // (execution is important to resolve any dynamic tests + // that aren't generated until the test plan actually runs) + val plan = discoverTests(cls, executeAsWell = true) + + // Validate common behavior of formatter against class names + val root = plan.roots.first() + val classIdentifier = plan.getChildren(root).first() + assertThat(classIdentifier.format(false)).isEqualTo(cls.simpleName) + assertThat(classIdentifier.format(true)).isEqualTo(cls.simpleName) + + // Delegate to the provided block for the test method of the class + val methodIdentifier = plan.getChildren(classIdentifier).first() + plan.block(methodIdentifier) + } + + private fun TestPlan.assertChildren( + of: TestIdentifier, + expectedCount: Int, + block: (Int, TestIdentifier) -> Unit + ) { + with(getChildren(of)) { + assertThat(size).isEqualTo(expectedCount) + forEachIndexed(block) + } + } +} diff --git a/instrumentation/runner/src/test/kotlin/de/mannodermaus/junit5/internal/runners/AndroidJUnitPlatformTestTreeTests.kt b/instrumentation/runner/src/test/kotlin/de/mannodermaus/junit5/internal/runners/AndroidJUnitPlatformTestTreeTests.kt new file mode 100644 index 00000000..a999ac03 --- /dev/null +++ b/instrumentation/runner/src/test/kotlin/de/mannodermaus/junit5/internal/runners/AndroidJUnitPlatformTestTreeTests.kt @@ -0,0 +1,148 @@ +package de.mannodermaus.junit5.internal.runners + +import com.google.common.truth.Truth.assertThat +import de.mannodermaus.junit5.HasParameterizedTest +import de.mannodermaus.junit5.HasRepeatedTest +import de.mannodermaus.junit5.HasTest +import de.mannodermaus.junit5.HasTestFactory +import de.mannodermaus.junit5.HasTestTemplate +import de.mannodermaus.junit5.discoverTests +import org.junit.jupiter.params.ParameterizedTest +import org.junit.jupiter.params.provider.CsvSource +import org.junit.platform.launcher.TestExecutionListener +import org.junit.platform.launcher.TestIdentifier +import org.junit.platform.launcher.core.LauncherFactory +import kotlin.reflect.KClass + +class AndroidJUnitPlatformTestTreeTests { + @CsvSource( + "false, method", + "true, method", + ) + @ParameterizedTest + fun test(isolated: Boolean, expected: String) = + runTestWith(HasTest::class, isolated) { identifier -> + val description = getDescription(identifier) + assertThat(description.methodName).isEqualTo(expected) + } + + @CsvSource( + "false, method(RepetitionInfo) - repetition %d of 5", + "true, method[%d]", + ) + @ParameterizedTest + fun `repeated test`(isolated: Boolean, expected: String) = + runTestWith(HasRepeatedTest::class, isolated) { identifier -> + assertChildren(identifier, expectedCount = 5) { index, child -> + val num = index + 1 + val childDescription = getDescription(child) + assertThat(childDescription.methodName).isEqualTo(expected.format(num)) + } + } + + @CsvSource( + "false, method - %s", + "true, method[%d]", + ) + @ParameterizedTest + fun `test factory`(isolated: Boolean, expected: String) = + runTestWith(HasTestFactory::class, isolated) { identifier -> + val childMethodNames = listOf("a", "b") + + assertChildren(identifier, expectedCount = 2) { index, child -> + val num = index + 1 + val childDescription = getDescription(child) + + if (isolated) { + assertThat(childDescription.methodName).isEqualTo(expected.format(num)) + } else { + assertThat(childDescription.methodName).isEqualTo(expected.format(childMethodNames[index])) + } + } + } + + @CsvSource( + "false, method(String) - %s", + "true, method[%d]", + ) + @ParameterizedTest + fun `test template`(isolated: Boolean, expected: String) = + runTestWith(HasTestTemplate::class, isolated) { identifier -> + val childMethodNames = listOf("param1", "param2") + + assertChildren(identifier, expectedCount = 2) { index, child -> + val num = index + 1 + val childDescription = getDescription(child) + + if (isolated) { + assertThat(childDescription.methodName).isEqualTo(expected.format(num)) + } else { + assertThat(childDescription.methodName).isEqualTo(expected.format(childMethodNames[index])) + } + } + } + + @CsvSource( + "false, method(String) - [%d] %s", + "true, method[%d]", + ) + @ParameterizedTest + fun `parameterized test`(isolated: Boolean, expected: String) = + runTestWith(HasParameterizedTest::class, isolated) { identifier -> + val childMethodNames = listOf("a", "b") + + assertChildren(identifier, expectedCount = 2) { index, child -> + val num = index + 1 + val childDescription = getDescription(child) + + if (isolated) { + assertThat(childDescription.methodName).isEqualTo(expected.format(num)) + } else { + assertThat(childDescription.methodName).isEqualTo(expected.format(num, childMethodNames[index])) + } + } + } + + /* Private */ + + private fun runTestWith( + cls: KClass<*>, + isIsolatedMethodRun: Boolean = false, + block: AndroidJUnitPlatformTestTree.(TestIdentifier) -> Unit, + ) { + // Prepare a test plan to launch + val launcher = LauncherFactory.create() + val plan = discoverTests(cls, launcher, executeAsWell = false) + val tree = AndroidJUnitPlatformTestTree( + testPlan = plan, + testClass = cls.java, + isIsolatedMethodRun = isIsolatedMethodRun, + isParallelExecutionEnabled = false, + ) + + // Execute the test plan, adding dynamic tests with the tree + // as they are registered during execution + launcher.execute(plan, object : TestExecutionListener { + override fun dynamicTestRegistered(testIdentifier: TestIdentifier) { + tree.addDynamicDescription(testIdentifier, testIdentifier.parentId.get()) + } + }) + + // For concrete assertions, delegate to the given block + val root = plan.roots.first() + val classIdentifier = plan.getChildren(root).first() + val methodIdentifier = plan.getChildren(classIdentifier).first() + tree.block(methodIdentifier) + } + + private fun AndroidJUnitPlatformTestTree.assertChildren( + identifier: TestIdentifier, + expectedCount: Int, + block: (Int, TestIdentifier) -> Unit + ) { + with(getChildren(identifier)) { + assertThat(size).isEqualTo(expectedCount) + forEachIndexed(block) + } + } +}