fix(core): unify FileManager logic for screenshot file with generic l…

…ogic

core/src/main/kotlin/com/malinskiy/marathon/io/FileManager.kt

@@ -19,32 +19,26 @@ import java.util.UUID
 class FileManager(private val maxPath: Int, private val maxFilename: Int, private val output: File) {
     val log = MarathonLogging.logger("FileManager")
 
-    fun createFile(fileType: FileType, pool: DevicePoolId, device: DeviceInfo, test: Test? = null, testBatchId: String? = null): File {
+    fun createFile(
+        fileType: FileType,
+        pool: DevicePoolId,
+        device: DeviceInfo,
+        test: Test? = null,
+        testBatchId: String? = null,
+        id: String? = null
+    ): File {
         val directory = when {
             test != null || testBatchId != null -> createDirectory(fileType, pool, device)
             else -> createDirectory(fileType, pool)
         }
         val filename = when {
-            test != null -> createTestFilename(test, fileType, testBatchId)
-            testBatchId != null -> createBatchFilename(testBatchId, fileType)
-            else -> createDeviceFilename(device, fileType)
+            test != null -> createTestFilename(test, fileType, testBatchId, id = id)
+            testBatchId != null -> createBatchFilename(testBatchId, fileType, id = id)
+            else -> createDeviceFilename(device, fileType, id = id)
         }
         return createFile(directory, filename)
     }
 
-    fun createScreenshotFile(extension: String, pool: DevicePoolId, device: DeviceInfo, test: Test, testBatchId: String): File {
-        val directory = createDirectory(FileType.SCREENSHOT, pool, device)
-        val filename =
-            createTestFilename(
-                test,
-                FileType.SCREENSHOT,
-                testBatchId = null,
-                extension,
-                UUID.randomUUID().toString()
-            )
-        return createFile(directory, filename)
-    }
-
     fun createFolder(folderType: FolderType, pool: DevicePoolId? = null, device: DeviceInfo? = null): File {
         var path = get(output.absolutePath, folderType.dir)
         if (pool != null) {
@@ -57,8 +51,10 @@ class FileManager(private val maxPath: Int, private val maxFilename: Int, privat
         val maybeTooLongPath = path.toFile()
         path = if (maxPath > 0 && maybeTooLongPath.absolutePath.length > maxPath) {
             val trimmed = maybeTooLongPath.absolutePath.take(maxPath)
-            log.error { "Directory path length cannot exceed $maxPath characters and has been trimmed from $maybeTooLongPath to $trimmed and can create a conflict. " +
-                "This happened because the combination of file path, pool name and device serial is too long." }
+            log.error {
+                "Directory path length cannot exceed $maxPath characters and has been trimmed from $maybeTooLongPath to $trimmed and can create a conflict. " +
+                    "This happened because the combination of file path, pool name and device serial is too long."
+            }
             File(trimmed)
         } else {
             maybeTooLongPath
@@ -98,17 +94,22 @@ class FileManager(private val maxPath: Int, private val maxFilename: Int, privat
         val maybeTooLongPath = File(directory.toFile(), trimmedFilename)
         return if (maxPath > 0 && maybeTooLongPath.absolutePath.length > maxPath) {
             val trimmed = maybeTooLongPath.absolutePath.substring(0 until maxPath)
-            log.error { "File path length cannot exceed $maxPath characters and has been trimmed from $maybeTooLongPath to $trimmed and can create a conflict. " +
-                "This happened because the combination of file path, test class name, and test name is too long." }
+            log.error {
+                "File path length cannot exceed $maxPath characters and has been trimmed from $maybeTooLongPath to $trimmed and can create a conflict. " +
+                    "This happened because the combination of file path, test class name, and test name is too long."
+            }
             File(trimmed)
         } else {
             maybeTooLongPath
         }
     }
 
-    private fun createBatchFilename(testBatchId: String, fileType: FileType): String {
+    private fun createBatchFilename(testBatchId: String, fileType: FileType, id: String? = null): String {
         return StringBuilder().apply {
             append(testBatchId)
+            if (id != null) {
+                append("-$id")
+            }
             if (fileType.suffix.isNotEmpty()) {
                 append(".$testBatchId")
             }
@@ -139,9 +140,12 @@ class FileManager(private val maxPath: Int, private val maxFilename: Int, privat
         return "$testName$testSuffix"
     }
 
-    private fun createDeviceFilename(device: DeviceInfo, fileType: FileType): String {
+    private fun createDeviceFilename(device: DeviceInfo, fileType: FileType, id: String? = null): String {
         return StringBuilder().apply {
             append(device.safeSerialNumber)
+            if (id != null) {
+                append("-$id")
+            }
             if (fileType.suffix.isNotEmpty()) {
                 append(".${fileType.suffix}")
             }

core/src/main/kotlin/com/malinskiy/marathon/io/FileType.kt

@@ -9,6 +9,9 @@ enum class FileType(val dir: String, val suffix: String) {
     VIDEO("video", "mp4"),
     SCREENSHOT("screenshot", "gif"),
     SCREENSHOT_PNG("screenshot", "png"),
+    SCREENSHOT_JPG("screenshot", "jpg"),
+    SCREENSHOT_WEBP("screenshot", "jpg"),
+    SCREENSHOT_GIF("screenshot", "jpg"),
     XCTESTRUN("xctestrun", "xctestrun"),
     BILL("bill", "json"),
 }

core/src/test/kotlin/com/malinskiy/marathon/io/FileManagerTest.kt

@@ -94,6 +94,25 @@ class FileManagerTest {
         file.name shouldBeEqualTo "127.0.0.1-5037-emulator-5554.log"
     }
 
+    @Test
+    fun testScreenshotfile() {
+        val fileManager = FileManager(0, 255, output)
+        val file = fileManager.createFile(
+            FileType.SCREENSHOT_PNG, poolId, DeviceInfo(
+                operatingSystem = OperatingSystem("23"),
+                serialNumber = "127.0.0.1:5037:emulator-5554",
+                model = "Android SDK built for x86",
+                manufacturer = "unknown",
+                networkState = NetworkState.CONNECTED,
+                deviceFeatures = listOf(DeviceFeature.SCREENSHOT, DeviceFeature.VIDEO),
+                healthy = true
+            ),
+            test = shortNameTest,
+            id = "on-device-test",
+        )
+        file.name shouldBeEqualTo "com.example.Clazz#method-on-device-test.png"
+    }
+
     @Test
     fun testTooLongOutputPathUnlimitedFilename() {
         val test = com.malinskiy.marathon.test.Test(
@@ -110,5 +129,7 @@ class FileManagerTest {
         val limitedOutputDirectory = File(tempDir, "x".repeat(additionalPathCharacters))
         val limitedFileManager = FileManager(limitedMaxPath, 0, limitedOutputDirectory)
         val file = limitedFileManager.createFile(FileType.LOG, poolId, deviceInfo, test, batchId)
-    } 
+
+        file.path.length shouldBeEqualTo 255
+    }
 }

vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/executor/listeners/screenshot/AdamScreenCaptureTestRunListener.kt

@@ -8,9 +8,11 @@ import com.malinskiy.marathon.device.toDeviceInfo
 import com.malinskiy.marathon.execution.Attachment
 import com.malinskiy.marathon.execution.AttachmentType
 import com.malinskiy.marathon.io.FileManager
+import com.malinskiy.marathon.io.FileType
 import com.malinskiy.marathon.log.MarathonLogging
 import com.malinskiy.marathon.report.attachment.AttachmentListener
 import com.malinskiy.marathon.report.attachment.AttachmentProvider
+import java.util.UUID
 
 class AdamScreenCaptureTestRunListener(
     private val pool: DevicePoolId,
@@ -27,14 +29,30 @@ class AdamScreenCaptureTestRunListener(
         val screenshots = testMetrics.filterKeys { it == "com.malinskiy.adam.junit4.android.screencapture.AdamScreenCaptureProcessor.v1" }
         screenshots.values.forEach { path ->
             val extension = path.substringAfterLast('.')
-            val attachmentType = when (extension) {
-                "jpeg", "jpg" -> AttachmentType.SCREENSHOT_JPEG
-                "png" -> AttachmentType.SCREENSHOT_PNG
-                "webp" -> AttachmentType.SCREENSHOT_WEBP
-                else -> null
+            var attachmentType: AttachmentType? = null
+            var fileType: FileType? = null
+            when (extension) {
+                "jpeg", "jpg" -> {
+                    attachmentType = AttachmentType.SCREENSHOT_JPEG
+                    fileType = FileType.SCREENSHOT_JPG
+                }
+                "png" -> {
+                    attachmentType = AttachmentType.SCREENSHOT_PNG
+                    fileType = FileType.SCREENSHOT_PNG
+                }
+                "webp" -> {
+                    attachmentType = AttachmentType.SCREENSHOT_WEBP
+                    fileType = FileType.SCREENSHOT_WEBP
+                }
+                "gif" -> {
+                    attachmentType = AttachmentType.SCREENSHOT_GIF
+                    fileType = FileType.SCREENSHOT_GIF
+                }
+                else -> Unit
             }
-            if (attachmentType != null) {
-                val localFile = fileManager.createScreenshotFile(extension, pool, device.toDeviceInfo(), test.toTest(), testBatchId)
+
+            if (attachmentType != null && fileType != null) {
+                val localFile = fileManager.createFile(fileType, pool, device.toDeviceInfo(), test.toTest(), testBatchId = testBatchId, id = UUID.randomUUID().toString())
                 device.safePullFile(path, localFile.absolutePath)
                 logger.debug { "Received screen capture file $path" }
                 attachmentListeners.forEach {