From a4ea2c19cfb967f30b8ff8d44689161053dcc94e Mon Sep 17 00:00:00 2001 From: Evgenii Matsiuk Date: Mon, 22 Jan 2024 13:10:00 +0300 Subject: [PATCH] feat(android): remote test parser should gracefully handle invalid test 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 --- buildSrc/src/main/kotlin/Versions.kt | 2 +- .../android/adam/AmInstrumentTestParser.kt | 40 ++++++++++++++++--- .../android/adam/AndroidDeviceTestRunner.kt | 2 + .../listeners/AndroidTestRunListener.kt | 2 + .../listeners/DebugTestRunListener.kt | 4 ++ .../executor/listeners/NoOpTestRunListener.kt | 2 + 6 files changed, 45 insertions(+), 7 deletions(-) diff --git a/buildSrc/src/main/kotlin/Versions.kt b/buildSrc/src/main/kotlin/Versions.kt index 03670334a..2faa8b0b8 100644 --- 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/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/adam/AmInstrumentTestParser.kt b/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/adam/AmInstrumentTestParser.kt index 24630a589..bb44bb0dc 100644 --- a/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/adam/AmInstrumentTestParser.kt +++ b/vendor/vendor-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 { 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 + testBundles: List, + blockListenerArgumentOverride: Boolean, ): List { val androidAppInstaller = AndroidAppInstaller(configuration) androidAppInstaller.prepareInstallation(device) @@ -71,7 +84,12 @@ class AmInstrumentTestParser( val testParserConfiguration = vendorConfiguration.testParserConfiguration val overrides: Map = 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,7 +150,7 @@ 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" } } @@ -132,3 +158,5 @@ class AmInstrumentTestParser( } } } + +private class TestAnnotationProducerNotFoundException : RuntimeException() diff --git a/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/adam/AndroidDeviceTestRunner.kt b/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/adam/AndroidDeviceTestRunner.kt index f2c83e1d5..a4e8c027d 100644 --- a/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/adam/AndroidDeviceTestRunner.kt +++ b/vendor/vendor-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/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/executor/listeners/AndroidTestRunListener.kt b/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/executor/listeners/AndroidTestRunListener.kt index d0b5f8763..1df3bf37b 100644 --- a/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/executor/listeners/AndroidTestRunListener.kt +++ b/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/executor/listeners/AndroidTestRunListener.kt @@ -18,6 +18,8 @@ interface AndroidTestRunListener { suspend fun testEnded(test: TestIdentifier, testMetrics: Map) {} + suspend fun testRunFailing(error: String, stackTrace: String) {} + suspend fun testRunFailed(errorMessage: String) {} suspend fun testRunStopped(elapsedTime: Long) {} diff --git a/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/executor/listeners/DebugTestRunListener.kt b/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/executor/listeners/DebugTestRunListener.kt index 67a6bd5ba..132d874cf 100644 --- a/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/executor/listeners/DebugTestRunListener.kt +++ b/vendor/vendor-android/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/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/executor/listeners/NoOpTestRunListener.kt b/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/executor/listeners/NoOpTestRunListener.kt index fb6be0b2d..0fda946f6 100644 --- a/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/executor/listeners/NoOpTestRunListener.kt +++ b/vendor/vendor-android/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) {} + override suspend fun testRunFailing(error: String, stackTrace: String) {} + override suspend fun testRunFailed(errorMessage: String) {} override suspend fun testRunStopped(elapsedTime: Long) {}