From 4928b65c74ef74fc763225ecbbbc320aadea8821 Mon Sep 17 00:00:00 2001 From: "Paul.Craciunas" Date: Fri, 15 Dec 2023 13:21:56 +0000 Subject: [PATCH 1/2] Added custom exceptions for failures in report generation and screenshot pulling and integrated them --- .../main/kotlin/com/malinskiy/marathon/Marathon.kt | 12 ++++++++++-- .../exceptions/FailedToPullScreenshotsException.kt | 3 +++ .../marathon/exceptions/ReportGenerationException.kt | 3 +++ .../listeners/pull/PullScreenshotTestRunListener.kt | 4 +++- .../android/ddmlib/AndroidDeviceTestRunner.kt | 1 + 5 files changed, 20 insertions(+), 3 deletions(-) create mode 100644 core/src/main/kotlin/com/malinskiy/marathon/exceptions/FailedToPullScreenshotsException.kt create mode 100644 core/src/main/kotlin/com/malinskiy/marathon/exceptions/ReportGenerationException.kt diff --git a/core/src/main/kotlin/com/malinskiy/marathon/Marathon.kt b/core/src/main/kotlin/com/malinskiy/marathon/Marathon.kt index ab82d2357..135731740 100644 --- a/core/src/main/kotlin/com/malinskiy/marathon/Marathon.kt +++ b/core/src/main/kotlin/com/malinskiy/marathon/Marathon.kt @@ -9,6 +9,7 @@ import com.malinskiy.marathon.cache.test.TestCacheSaver import com.malinskiy.marathon.config.LogicalConfigurationValidator import com.malinskiy.marathon.device.DeviceProvider import com.malinskiy.marathon.exceptions.NoDevicesException +import com.malinskiy.marathon.exceptions.ReportGenerationException import com.malinskiy.marathon.execution.ComponentInfo import com.malinskiy.marathon.execution.ComponentInfoExtractor import com.malinskiy.marathon.execution.Configuration @@ -174,14 +175,17 @@ class Marathon( try { scheduler.stopAndWaitForCompletion() onFinish(analytics, deviceProvider, attachmentManager) + } catch (up: ReportGenerationException) { + // We don't want to catch this. This should fail the execution + throw up } catch (throwable: Throwable) { log.error("Error occurred while finishing tests run", throwable) } finally { hook.uninstall() stopKoin() - return progressReporter.aggregateResult() } + return progressReporter.aggregateResult() } private fun installShutdownHook(block: suspend () -> Unit): ShutdownHook { @@ -202,7 +206,11 @@ class Marathon( analytics.close() deviceProvider.terminate() attachmentManager.terminate() - tracker.close() + try { + tracker.close() + } catch (e: Throwable) { + throw ReportGenerationException("Failed to generate test run report with exception", e) + } } private fun applyTestFilters(parsedTests: List): List { diff --git a/core/src/main/kotlin/com/malinskiy/marathon/exceptions/FailedToPullScreenshotsException.kt b/core/src/main/kotlin/com/malinskiy/marathon/exceptions/FailedToPullScreenshotsException.kt new file mode 100644 index 000000000..584025f35 --- /dev/null +++ b/core/src/main/kotlin/com/malinskiy/marathon/exceptions/FailedToPullScreenshotsException.kt @@ -0,0 +1,3 @@ +package com.malinskiy.marathon.exceptions + +class FailedToPullScreenshotsException(message: String, cause: Throwable) : RuntimeException(message, cause) diff --git a/core/src/main/kotlin/com/malinskiy/marathon/exceptions/ReportGenerationException.kt b/core/src/main/kotlin/com/malinskiy/marathon/exceptions/ReportGenerationException.kt new file mode 100644 index 000000000..728ad6057 --- /dev/null +++ b/core/src/main/kotlin/com/malinskiy/marathon/exceptions/ReportGenerationException.kt @@ -0,0 +1,3 @@ +package com.malinskiy.marathon.exceptions + +class ReportGenerationException(message: String, cause: Throwable) : RuntimeException(message, cause) diff --git a/vendor/vendor-android/base/src/main/kotlin/com/malinskiy/marathon/android/executor/listeners/pull/PullScreenshotTestRunListener.kt b/vendor/vendor-android/base/src/main/kotlin/com/malinskiy/marathon/android/executor/listeners/pull/PullScreenshotTestRunListener.kt index d937e3dcf..a05081460 100644 --- a/vendor/vendor-android/base/src/main/kotlin/com/malinskiy/marathon/android/executor/listeners/pull/PullScreenshotTestRunListener.kt +++ b/vendor/vendor-android/base/src/main/kotlin/com/malinskiy/marathon/android/executor/listeners/pull/PullScreenshotTestRunListener.kt @@ -5,6 +5,7 @@ import com.malinskiy.marathon.android.AndroidDevice import com.malinskiy.marathon.android.executor.listeners.TestRunListener import com.malinskiy.marathon.device.DevicePoolId import com.malinskiy.marathon.device.toDeviceInfo +import com.malinskiy.marathon.exceptions.FailedToPullScreenshotsException import com.malinskiy.marathon.execution.FilteringConfiguration import com.malinskiy.marathon.execution.matches import com.malinskiy.marathon.log.MarathonLogging @@ -75,7 +76,8 @@ class PullScreenshotTestRunListener( } logger.trace { "Pulling screenshots finished in ${millis}ms from $remoteDir to $outputDir" } } catch (e: Exception) { - logger.error(e) { "Failed to pull screenshots from $remoteDir" } + // It's unlikely we can continue from this point if pulling screenshots fails + throw FailedToPullScreenshotsException("Failed to pull screenshots from $remoteDir", e) } } diff --git a/vendor/vendor-android/ddmlib/src/main/kotlin/com/malinskiy/marathon/android/ddmlib/AndroidDeviceTestRunner.kt b/vendor/vendor-android/ddmlib/src/main/kotlin/com/malinskiy/marathon/android/ddmlib/AndroidDeviceTestRunner.kt index bbd6aa23c..39ee68300 100644 --- a/vendor/vendor-android/ddmlib/src/main/kotlin/com/malinskiy/marathon/android/ddmlib/AndroidDeviceTestRunner.kt +++ b/vendor/vendor-android/ddmlib/src/main/kotlin/com/malinskiy/marathon/android/ddmlib/AndroidDeviceTestRunner.kt @@ -75,6 +75,7 @@ class AndroidDeviceTestRunner(private val device: DdmlibAndroidDevice) { } finally { } + // Do not catch FailedToPullScreenshotsException. If that's thrown, we should fail the whole task } private fun notifyIgnoredTest(ignoredTests: List, listeners: ITestRunListener) { From 45e692582f73b86d85e8a6797bb6dd84d26c4493 Mon Sep 17 00:00:00 2001 From: "Paul.Craciunas" Date: Mon, 18 Dec 2023 10:03:58 +0000 Subject: [PATCH 2/2] Fail the marathon task on any exception thrown, instead of silently swallowing them --- core/src/main/kotlin/com/malinskiy/marathon/Marathon.kt | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/core/src/main/kotlin/com/malinskiy/marathon/Marathon.kt b/core/src/main/kotlin/com/malinskiy/marathon/Marathon.kt index 135731740..3aa42903b 100644 --- a/core/src/main/kotlin/com/malinskiy/marathon/Marathon.kt +++ b/core/src/main/kotlin/com/malinskiy/marathon/Marathon.kt @@ -175,11 +175,10 @@ class Marathon( try { scheduler.stopAndWaitForCompletion() onFinish(analytics, deviceProvider, attachmentManager) - } catch (up: ReportGenerationException) { - // We don't want to catch this. This should fail the execution - throw up } catch (throwable: Throwable) { + // We don't want to catch these. If an exception was thrown, we should fail the execution log.error("Error occurred while finishing tests run", throwable) + throw throwable } finally { hook.uninstall()