feat(android): remote test parser should gracefully handle invalid te…

…st listener (#877) when users forget to add the correspondent dependency in the project then test parsing should gracefully disable the listener instead of silent failure --------- Co-authored-by: Anton Malinskiy <anton@malinskiy.com>
MarathonLabs · Jan 22, 2024 · a4ea2c1 · a4ea2c1
1 parent 034a996
commit a4ea2c1
Show file tree

Hide file tree

Showing 6 changed files with 45 additions and 7 deletions.
diff --git a/buildSrc/src/main/kotlin/Versions.kt b/buildSrc/src/main/kotlin/Versions.kt
@@ -6,7 +6,7 @@ object Versions {
     val coroutinesTest = coroutines
 
     val androidCommon = "31.1.3"
-    val adam = "0.5.2"
+    val adam = "0.5.3"
     val dexTestParser = "2.3.4"
     val kotlinLogging = "3.0.5"
     val logbackClassic = "1.4.11"

diff --git a/...dor-android/src/main/kotlin/com/malinskiy/marathon/android/adam/AmInstrumentTestParser.kt b/...dor-android/src/main/kotlin/com/malinskiy/marathon/android/adam/AmInstrumentTestParser.kt
@@ -8,6 +8,7 @@ import com.malinskiy.adam.request.testrunner.TestFailed
 import com.malinskiy.adam.request.testrunner.TestIgnored
 import com.malinskiy.adam.request.testrunner.TestRunEnded
 import com.malinskiy.adam.request.testrunner.TestRunFailed
+import com.malinskiy.adam.request.testrunner.TestRunFailing
 import com.malinskiy.adam.request.testrunner.TestRunStartedEvent
 import com.malinskiy.adam.request.testrunner.TestRunStopped
 import com.malinskiy.adam.request.testrunner.TestRunnerRequest
@@ -32,6 +33,9 @@ import kotlinx.coroutines.CancellationException
 import kotlinx.coroutines.NonCancellable.isActive
 import kotlinx.coroutines.withTimeoutOrNull
 
+private const val LISTENER_ARGUMENT = "listener"
+private const val TEST_ANNOTATION_PRODUCER = "com.malinskiy.adam.junit4.android.listener.TestAnnotationProducer"
+
 class AmInstrumentTestParser(
     private val configuration: Configuration,
     private val testBundleIdentifier: AndroidTestBundleIdentifier,
@@ -42,15 +46,23 @@ class AmInstrumentTestParser(
 
     override suspend fun extract(device: Device): List<Test> {
         val testBundles = vendorConfiguration.testBundlesCompat()
+        var blockListenerArgumentOverride = false
         return withRetry(3, 0) {
             try {
                 val device = device as? AdamAndroidDevice ?: throw ConfigurationException("Unexpected device type for remote test parsing")
-                return@withRetry parseTests(device, configuration, vendorConfiguration, testBundles)
+                return@withRetry parseTests(device, configuration, vendorConfiguration, testBundles, blockListenerArgumentOverride)
             } catch (e: CancellationException) {
                 throw e
-            } catch (e: Exception) {
-                logger.debug(e) { "Remote parsing failed. Retrying" }
+            } catch (e: TestAnnotationProducerNotFoundException) {
+                logger.warn {
+                    "Previous parsing attempt failed due to test parser misconfiguration: test annotation producer was not found. See https://docs.marathonlabs.io/runner/android/configure#test-parser. " +
+                        "Next parsing attempt will remove overridden test run listener."
+                }
+                blockListenerArgumentOverride = true
                 throw e
+            } catch (throwable: Throwable) {
+                logger.debug(throwable) { "Remote parsing failed. Retrying" }
+                throw throwable
             }
         }
     }
@@ -59,7 +71,8 @@ class AmInstrumentTestParser(
         device: AdamAndroidDevice,
         configuration: Configuration,
         vendorConfiguration: VendorConfiguration.AndroidConfiguration,
-        testBundles: List<AndroidTestBundle>
+        testBundles: List<AndroidTestBundle>,
+        blockListenerArgumentOverride: Boolean,
     ): List<Test> {
         val androidAppInstaller = AndroidAppInstaller(configuration)
         androidAppInstaller.prepareInstallation(device)
@@ -71,7 +84,12 @@ class AmInstrumentTestParser(
 
             val testParserConfiguration = vendorConfiguration.testParserConfiguration
             val overrides: Map<String, String> = when {
-                testParserConfiguration is TestParserConfiguration.RemoteTestParserConfiguration -> testParserConfiguration.instrumentationArgs
+                testParserConfiguration is TestParserConfiguration.RemoteTestParserConfiguration -> {
+                    if (blockListenerArgumentOverride) testParserConfiguration.instrumentationArgs
+                        .filterNot { it.key == LISTENER_ARGUMENT && it.value == TEST_ANNOTATION_PRODUCER }
+                    else testParserConfiguration.instrumentationArgs
+                }
+
                 else -> emptyMap()
             }
 
@@ -113,9 +131,17 @@ class AmInstrumentTestParser(
                                 testBundleIdentifier.put(test, androidTestBundle)
                             }
 
+                            is TestRunFailing -> {
+                                // Error message is stable, see https://github.com/android/android-test/blame/1ae53b93e02cc363311f6564a35edeea1b075103/runner/android_junit_runner/java/androidx/test/internal/runner/RunnerArgs.java#L624
+                                if (event.error.contains("Could not find extra class $TEST_ANNOTATION_PRODUCER")) {
+                                    throw TestAnnotationProducerNotFoundException()
+                                }
+                            }
+
                             is TestRunFailed -> Unit
                             is TestRunStopped -> Unit
                             is TestRunEnded -> Unit
+
                         }
                     }
                 }
@@ -124,11 +150,13 @@ class AmInstrumentTestParser(
             if (!observedAnnotations) {
                 logger.warn {
                     "Bundle ${bundle.id} did not report any test annotations. If you need test annotations retrieval, remote test parser requires additional setup " +
-                        "see https://docs.marathonlabs.io/android/configure#test-parser"
+                        "see https://docs.marathonlabs.io/runner/android/configure#test-parser"
                 }
             }
 
             tests
         }
     }
 }
+
+private class TestAnnotationProducerNotFoundException : RuntimeException()
diff --git a/...or-android/src/main/kotlin/com/malinskiy/marathon/android/adam/AndroidDeviceTestRunner.kt b/...or-android/src/main/kotlin/com/malinskiy/marathon/android/adam/AndroidDeviceTestRunner.kt
@@ -8,6 +8,7 @@ import com.malinskiy.adam.request.testrunner.TestFailed
 import com.malinskiy.adam.request.testrunner.TestIgnored
 import com.malinskiy.adam.request.testrunner.TestRunEnded
 import com.malinskiy.adam.request.testrunner.TestRunFailed
+import com.malinskiy.adam.request.testrunner.TestRunFailing
 import com.malinskiy.adam.request.testrunner.TestRunStartedEvent
 import com.malinskiy.adam.request.testrunner.TestRunStopped
 import com.malinskiy.adam.request.testrunner.TestRunnerRequest
@@ -116,6 +117,7 @@ class AndroidDeviceTestRunner(private val device: AdamAndroidDevice, private val
                 is TestRunFailed -> listener.testRunFailed(event.error)
                 is TestRunStopped -> listener.testRunStopped(event.elapsedTimeMillis)
                 is TestRunEnded -> listener.testRunEnded(event.elapsedTimeMillis, event.metrics)
+                is TestRunFailing -> listener.testRunFailing(event.error, event.stackTrace)
             }
         }
     }

diff --git a/...c/main/kotlin/com/malinskiy/marathon/android/executor/listeners/AndroidTestRunListener.kt b/...c/main/kotlin/com/malinskiy/marathon/android/executor/listeners/AndroidTestRunListener.kt
@@ -18,6 +18,8 @@ interface AndroidTestRunListener {
 
     suspend fun testEnded(test: TestIdentifier, testMetrics: Map<String, String>) {}
 
+    suspend fun testRunFailing(error: String, stackTrace: String) {}
+
     suspend fun testRunFailed(errorMessage: String) {}
 
     suspend fun testRunStopped(elapsedTime: Long) {}

diff --git a/...src/main/kotlin/com/malinskiy/marathon/android/executor/listeners/DebugTestRunListener.kt b/...src/main/kotlin/com/malinskiy/marathon/android/executor/listeners/DebugTestRunListener.kt
@@ -36,6 +36,10 @@ class DebugTestRunListener(private val device: AndroidDevice) : AndroidTestRunLi
         logger.info { "testIgnored ${device.serialNumber} test = $test" }
     }
 
+    override suspend fun testRunFailing(error: String, stackTrace: String) {
+        logger.info { "testRunFailing ${device.serialNumber} errorMessage = $error trace = $stackTrace" }
+    }
+
     override suspend fun testRunFailed(errorMessage: String) {
         logger.info { "testRunFailed ${device.serialNumber} errorMessage = $errorMessage" }
     }

diff --git a/.../src/main/kotlin/com/malinskiy/marathon/android/executor/listeners/NoOpTestRunListener.kt b/.../src/main/kotlin/com/malinskiy/marathon/android/executor/listeners/NoOpTestRunListener.kt
@@ -15,6 +15,8 @@ open class NoOpTestRunListener : AndroidTestRunListener {
 
     override suspend fun testEnded(test: TestIdentifier, testMetrics: Map<String, String>) {}
 
+    override suspend fun testRunFailing(error: String, stackTrace: String) {}
+
     override suspend fun testRunFailed(errorMessage: String) {}
 
     override suspend fun testRunStopped(elapsedTime: Long) {}