Prevent test methods incorrectly defined as Kotlin top-level functions from being considered for test execution (#325)

This won't work, as JUnit 5 doesn't actually run these methods. However, if a runner is created for the class,
then there is a mismatch between Android's expectations and JUnit 5's deliverables.
This messes up Android's internal test counting, causing issues like "Expected N+1 tests, received N"

Author: mannodermaus

instrumentation/CHANGELOG.md

@@ -6,6 +6,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)
+- 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)  
 
 ## 1.4.0 (2023-11-05)
 

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

@@ -3,6 +3,7 @@ package de.mannodermaus.junit5.internal.extensions
 import android.util.Log
 import de.mannodermaus.junit5.internal.LOG_TAG
 import java.lang.reflect.Method
+import java.lang.reflect.Modifier
 
 private val jupiterTestAnnotations = listOf(
     "org.junit.jupiter.api.Test",
@@ -38,6 +39,12 @@ private fun Class<*>.jupiterTestMethods(includeInherited: Boolean): Set<Method>
 
 private fun Array<Method>.filterAnnotatedByJUnitJupiter(): List<Method> =
     filter { method ->
+        // The method must not be static...
+        if (method.isStatic) return@filter false
+
+        // ...and have at least one of the recognized JUnit 5 annotations
         val names = method.declaredAnnotations.map { it.annotationClass.qualifiedName }
-        jupiterTestAnnotations.any { it in names }
+        jupiterTestAnnotations.any(names::contains)
     }
+
+private val Method.isStatic get() = Modifier.isStatic(modifiers)

instrumentation/runner/src/test/kotlin/de/mannodermaus/junit5/AndroidJUnit5BuilderTests.kt

@@ -0,0 +1,60 @@
+package de.mannodermaus.junit5
+
+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
+
+class AndroidJUnit5BuilderTests {
+
+    private val builder = AndroidJUnit5Builder()
+
+    @Test
+    fun `no runner is created if class only contains top-level test methods`() {
+        // 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)
+    }
+
+    @ValueSource(
+        classes = [
+            HasTest::class,
+            HasRepeatedTest::class,
+            HasTestFactory::class,
+            HasTestTemplate::class,
+            HasParameterizedTest::class,
+            HasInnerClassWithTest::class,
+            HasTaggedTest::class,
+            HasInheritedTestsFromClass::class,
+            HasInheritedTestsFromInterface::class,
+        ]
+    )
+    @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,
+        ]
+    )
+    @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()
+        }
+    }
+}

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

@@ -5,6 +5,7 @@ import org.junit.jupiter.api.DynamicTest.dynamicTest
 import org.junit.jupiter.api.extension.*
 import org.junit.jupiter.params.ParameterizedTest
 import org.junit.jupiter.params.provider.CsvSource
+import org.junit.jupiter.params.provider.ValueSource
 import java.util.stream.Stream
 
 class DoesntHaveTestMethods
@@ -101,3 +102,25 @@ class HasInheritedTestsFromClass : AbstractTestClass() {
 }
 
 class HasInheritedTestsFromInterface : AbstractTestInterface
+
+// These tests should not be acknowledged,
+// as classes with legacy tests & top-level tests
+// are unsupported by JUnit 5
+
+class HasJUnit4Tests {
+  @org.junit.Test
+  fun method() {}
+}
+
+@RepeatedTest(2)
+fun topLevelRepeatedTest(unused: RepetitionInfo) {}
+
+@ValueSource(strings = ["a", "b"])
+@ParameterizedTest
+fun topLevelParameterizedTest(unused: String) {}
+
+@TestTemplate
+fun topLevelTestTemplate() {}
+
+@TestFactory
+fun topLevelTestFactory(): Stream<DynamicNode> = Stream.empty()