Skip to content

Commit a71c2d0

Browse files
mannodermausmannodermaus
authored
Restructure test name formatting to fix intermittent regression of isolated method runs (#341)
Since relying fully on JUnit 4's test discovery mechanism, we need to adhere to its expectations when it comes to method filtering. For dynamic tests with JUnit 5, this means that we have to remove the parameters and brackets from the display name when invoking an individual test through CLI or IDE, otherwise it cannot be found
1 parent 08c30f0 commit a71c2d0

File tree

8 files changed

+371
-43
lines changed

8 files changed

+371
-43
lines changed

‎instrumentation/CHANGELOG.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ Change Log
55

66
- Fix inheritance hierarchy of `ComposeExtension` to avoid false-positive warning regarding `@RegisterExtension` (#318)
77
- Improve parallel test execution for Android instrumentation tests
8-
- Fix invalid naming of dynamic tests when executing only a singular test method from the IDE (#317)
8+
- Fix invalid naming of dynamic tests when executing only a singular test method from the IDE (#317, #339)
99
- 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)
1010
- Prevent test classes ignored by a tag from being considered for test execution, causing issues like "Expected N+1 tests, received N" (#298)
1111

‎instrumentation/runner/src/main/kotlin/de/mannodermaus/junit5/internal/extensions/TestIdentifierExt.kt‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package de.mannodermaus.junit5.internal.extensions
22

3+
import de.mannodermaus.junit5.internal.formatters.TestNameFormatter
34
import org.junit.platform.launcher.TestIdentifier
45

56
private val DYNAMIC_TEST_PREFIXES = listOf(
@@ -29,3 +30,12 @@ internal val TestIdentifier.isDynamicTest: Boolean
2930
val shortId = this.shortId
3031
return DYNAMIC_TEST_PREFIXES.any { shortId.startsWith(it) }
3132
}
33+
34+
/**
35+
* Returns a formatted version of this identifier's name,
36+
* which is compatible with the quirks and limitations
37+
* of the Android Instrumentation, esp. when the [isIsolatedMethodRun]
38+
* flag is enabled.
39+
*/
40+
internal fun TestIdentifier.format(isIsolatedMethodRun: Boolean = false): String =
41+
TestNameFormatter.format(this, isIsolatedMethodRun)

‎instrumentation/runner/src/main/kotlin/de/mannodermaus/junit5/internal/formatters/TestNameFormatter.kt‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
package de.mannodermaus.junit5.internal.formatters
2+
3+
import org.junit.platform.launcher.TestIdentifier
4+
5+
/**
6+
* A class for naming Jupiter test methods in a compatible manner,
7+
* taking into account several limitations imposed by the
8+
* Android instrumentation (e.g. on isolated test runs).
9+
*/
10+
internal object TestNameFormatter {
11+
fun format(identifier: TestIdentifier, isIsolatedMethodRun: Boolean = false): String {
12+
// During isolated executions of a single test method,
13+
// construct a technical version of its name for backwards compatibility
14+
// with the JUnit 4-based instrumentation of Android by stripping the brackets of parameterized tests completely.
15+
// If this didn't happen, running them from the IDE will cause "No tests found" errors.
16+
// See AndroidX's TestRequestBuilder$MethodFilter for where this is cross-referenced in the instrumentation!
17+
//
18+
// History:
19+
// - #199 & #207 (the original unearthing of this behavior)
20+
// - #317 (making an exception for dynamic tests)
21+
// - #339 (retain indices of parameterized methods to avoid premature filtering by JUnit 4's test discovery)
22+
if (isIsolatedMethodRun) {
23+
val reportName = identifier.legacyReportingName
24+
val paramStartIndex = reportName.indexOf('(')
25+
if (paramStartIndex > -1) {
26+
val result = reportName.substring(0, paramStartIndex)
27+
28+
val paramEndIndex = reportName.lastIndexOf('[')
29+
30+
return if (paramEndIndex > -1) {
31+
// Retain suffix of parameterized methods (i.e. "[1]", "[2]" etc)
32+
// so that they won't be filtered out by JUnit 4 on isolated method runs
33+
result + reportName.substring(paramEndIndex)
34+
} else {
35+
result
36+
}
37+
}
38+
}
39+
40+
return identifier.displayName.replace("()", "")
41+
}
42+
}

‎instrumentation/runner/src/main/kotlin/de/mannodermaus/junit5/internal/runners/AndroidJUnitPlatformTestTree.kt‎

Lines changed: 30 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
package de.mannodermaus.junit5.internal.runners
44

55
import android.annotation.SuppressLint
6+
import de.mannodermaus.junit5.internal.extensions.format
67
import de.mannodermaus.junit5.internal.extensions.isDynamicTest
78
import org.junit.platform.commons.util.AnnotationUtils
89
import org.junit.platform.engine.UniqueId
@@ -35,53 +36,40 @@ internal class AndroidJUnitPlatformTestTree(
3536

3637
val suiteDescription = generateSuiteDescription(testPlan, testClass)
3738

38-
fun getTestName(identifier: TestIdentifier): String =
39-
when {
40-
identifier.isContainer -> getTechnicalName(identifier)
41-
42-
identifier.isDynamicTest -> {
43-
// Collect all dynamic tests' IDs from this identifier,
44-
// all the way up to the first non-dynamic test.
45-
// Collect the name of all these into a list, then finally
46-
// compose the final name from this list. Note that, because we
47-
// move upwards the test plan, the elements must be reversed
48-
// before the final name can be composed.
49-
val nameComponents = mutableListOf<String>()
50-
var currentNode: TestIdentifier? = identifier
51-
while (currentNode != null && currentNode.isDynamicTest) {
52-
nameComponents.add(formatTestName(currentNode))
53-
currentNode = modifiedTestPlan.getRealParent(currentNode).orElse(null)
54-
}
55-
nameComponents.reverse()
56-
57-
// Android's Unified Test Platform (AGP 7.0+) is using literal test names
58-
// to create files when capturing Logcat output during execution.
59-
// Ergo, make sure that only legal characters are being used in the test names
60-
// (ref. https://github.com/mannodermaus/android-junit5/issues/263)
61-
nameComponents.joinToString(" - ")
39+
// Order matters here, since all dynamic tests are also containers,
40+
// but not all containers are dynamic tests
41+
fun getTestName(identifier: TestIdentifier): String = when {
42+
identifier.isDynamicTest -> if (isIsolatedMethodRun) {
43+
// In isolated method runs, there is no need to compose
44+
// dynamic test names from multiple pieces, as the
45+
// Android Instrumentation only looks at the raw method name
46+
// anyway and all information about parameter types is lost
47+
identifier.format(true)
48+
} else {
49+
// Collect all dynamic tests' IDs from this identifier,
50+
// all the way up to the first non-dynamic test.
51+
// Collect the name of all these into a list, then finally
52+
// compose the final name from this list. Note that, because we
53+
// move upwards the test plan, the elements must be reversed
54+
// before the final name can be composed.
55+
val nameComponents = mutableListOf<String>()
56+
var currentNode: TestIdentifier? = identifier
57+
while (currentNode != null && currentNode.isDynamicTest) {
58+
nameComponents.add(currentNode.format(false))
59+
currentNode = modifiedTestPlan.getRealParent(currentNode).orElse(null)
6260
}
61+
nameComponents.reverse()
6362

64-
else -> formatTestName(identifier)
63+
// Android's Unified Test Platform (AGP 7.0+) is using literal test names
64+
// to create files when capturing Logcat output during execution.
65+
// Ergo, make sure that only legal characters are being used in the test names
66+
// (ref. https://github.com/mannodermaus/android-junit5/issues/263)
67+
nameComponents.joinToString(" - ")
6568
}
6669

67-
private fun formatTestName(identifier: TestIdentifier): String {
68-
// During isolated executions of non-dynamic @Test methods,
69-
// construct a technical version of the test name for backwards compatibility
70-
// with the JUnit 4-based instrumentation of Android by stripping the brackets and parameters completely.
71-
// If this didn't happen, running them from the IDE will cause "No tests found" errors.
72-
// See AndroidX's TestRequestBuilder$MethodFilter for where this is cross-referenced in the instrumentation!
73-
//
74-
// Ref issues #199 & #207 (the original unearthing of this behavior),
75-
// as well as #317 (making an exception for dynamic tests).
76-
if (isIsolatedMethodRun && !identifier.isDynamicTest) {
77-
val reportName = identifier.legacyReportingName
78-
val bracketIndex = reportName.indexOf('(')
79-
if (bracketIndex > -1) {
80-
return reportName.substring(0, bracketIndex)
81-
}
82-
}
70+
identifier.isContainer -> getTechnicalName(identifier)
8371

84-
return identifier.displayName.replace("()", "")
72+
else -> identifier.format(isIsolatedMethodRun)
8573
}
8674

8775
// Do not expose our custom TestPlan, because JUnit Platform wouldn't like that very much.

‎instrumentation/runner/src/test/kotlin/de/mannodermaus/junit5/TestClasses.kt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ class HasTestTemplate {
4646

4747
private fun context(param: String): TestTemplateInvocationContext =
4848
object : TestTemplateInvocationContext {
49+
override fun getDisplayName(invocationIndex: Int): String = param
50+
4951
override fun getAdditionalExtensions() = listOf(
5052
object : ParameterResolver {
5153
override fun supportsParameter(parameterContext: ParameterContext, extensionContext: ExtensionContext) =

‎instrumentation/runner/src/test/kotlin/de/mannodermaus/junit5/TestHelpers.kt‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
package de.mannodermaus.junit5
2+
3+
import org.junit.platform.engine.discovery.DiscoverySelectors
4+
import org.junit.platform.launcher.Launcher
5+
import org.junit.platform.launcher.TestPlan
6+
import org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder
7+
import org.junit.platform.launcher.core.LauncherFactory
8+
import kotlin.reflect.KClass
9+
10+
/**
11+
* A quick one-liner for executing a Jupiter discover-and-execute pass
12+
* from inside of a Jupiter test. Useful for testing runner code
13+
* that needs to work with the innards of the [TestPlan], such as
14+
* individual test identifiers and such.
15+
*/
16+
fun discoverTests(
17+
cls: KClass<*>,
18+
launcher: Launcher = LauncherFactory.create(),
19+
executeAsWell: Boolean = true,
20+
): TestPlan {
21+
return launcher.discover(
22+
LauncherDiscoveryRequestBuilder.request()
23+
.selectors(DiscoverySelectors.selectClass(cls.java))
24+
.build()
25+
).also { plan ->
26+
if (executeAsWell) {
27+
launcher.execute(plan)
28+
}
29+
}
30+
}

‎instrumentation/runner/src/test/kotlin/de/mannodermaus/junit5/internal/formatters/TestNameFormatterTests.kt‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
package de.mannodermaus.junit5.internal.formatters
2+
3+
import com.google.common.truth.Truth.assertThat
4+
import de.mannodermaus.junit5.HasParameterizedTest
5+
import de.mannodermaus.junit5.HasRepeatedTest
6+
import de.mannodermaus.junit5.HasTest
7+
import de.mannodermaus.junit5.HasTestFactory
8+
import de.mannodermaus.junit5.HasTestTemplate
9+
import de.mannodermaus.junit5.discoverTests
10+
import de.mannodermaus.junit5.internal.extensions.format
11+
import org.junit.jupiter.api.Test
12+
import org.junit.platform.engine.discovery.DiscoverySelectors
13+
import org.junit.platform.launcher.TestIdentifier
14+
import org.junit.platform.launcher.TestPlan
15+
import org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder
16+
import org.junit.platform.launcher.core.LauncherFactory
17+
import kotlin.reflect.KClass
18+
19+
class TestNameFormatterTests {
20+
21+
@Test
22+
fun `normal junit5 test`() = runTestWith(HasTest::class) { identifier ->
23+
assertThat(identifier.format(false)).isEqualTo("method")
24+
assertThat(identifier.format(true)).isEqualTo("method")
25+
}
26+
27+
@Test
28+
fun `repeated test`() = runTestWith(HasRepeatedTest::class) { identifier ->
29+
assertThat(identifier.format(false)).isEqualTo("method(RepetitionInfo)")
30+
assertThat(identifier.format(true)).isEqualTo("method")
31+
32+
// Inspect individual executions, too
33+
assertChildren(identifier, expectedCount = 5) { index, child ->
34+
val number = index + 1
35+
assertThat(child.format(false)).isEqualTo("repetition $number of 5")
36+
assertThat(child.format(true)).isEqualTo("method[$number]")
37+
}
38+
}
39+
40+
@Test
41+
fun `test factory`() = runTestWith(HasTestFactory::class) { identifier ->
42+
assertThat(identifier.format(false)).isEqualTo("method")
43+
assertThat(identifier.format(true)).isEqualTo("method")
44+
45+
// Inspect individual executions, too
46+
assertChildren(identifier, expectedCount = 2) { index, child ->
47+
val number = index + 1
48+
assertThat(child.format(false)).isEqualTo(if (index == 0) "a" else "b")
49+
assertThat(child.format(true)).isEqualTo("method[$number]")
50+
}
51+
}
52+
53+
@Test
54+
fun `test template`() = runTestWith(HasTestTemplate::class) { identifier ->
55+
assertThat(identifier.format(false)).isEqualTo("method(String)")
56+
assertThat(identifier.format(true)).isEqualTo("method")
57+
58+
// Inspect individual executions, too
59+
assertChildren(identifier, expectedCount = 2) { index, child ->
60+
val number = index + 1
61+
assertThat(child.format(false)).isEqualTo("param$number")
62+
assertThat(child.format(true)).isEqualTo("method[$number]")
63+
}
64+
}
65+
66+
@Test
67+
fun `parameterized test`() = runTestWith(HasParameterizedTest::class) { identifier ->
68+
assertThat(identifier.format(false)).isEqualTo("method(String)")
69+
assertThat(identifier.format(true)).isEqualTo("method")
70+
71+
// Inspect individual executions, too
72+
assertChildren(identifier, expectedCount = 2) { index, child ->
73+
val number = index + 1
74+
assertThat(child.format(false)).isEqualTo("[$number] " + if (index == 0) "a" else "b")
75+
assertThat(child.format(true)).isEqualTo("method[$number]")
76+
}
77+
}
78+
79+
/* Private */
80+
81+
private fun runTestWith(cls: KClass<*>, block: TestPlan.(TestIdentifier) -> Unit) {
82+
// Discover and execute the test plan of the given class
83+
// (execution is important to resolve any dynamic tests
84+
// that aren't generated until the test plan actually runs)
85+
val plan = discoverTests(cls, executeAsWell = true)
86+
87+
// Validate common behavior of formatter against class names
88+
val root = plan.roots.first()
89+
val classIdentifier = plan.getChildren(root).first()
90+
assertThat(classIdentifier.format(false)).isEqualTo(cls.simpleName)
91+
assertThat(classIdentifier.format(true)).isEqualTo(cls.simpleName)
92+
93+
// Delegate to the provided block for the test method of the class
94+
val methodIdentifier = plan.getChildren(classIdentifier).first()
95+
plan.block(methodIdentifier)
96+
}
97+
98+
private fun TestPlan.assertChildren(
99+
of: TestIdentifier,
100+
expectedCount: Int,
101+
block: (Int, TestIdentifier) -> Unit
102+
) {
103+
with(getChildren(of)) {
104+
assertThat(size).isEqualTo(expectedCount)
105+
forEachIndexed(block)
106+
}
107+
}
108+
}

0 commit comments

Comments
 (0)