-
-
Notifications
You must be signed in to change notification settings - Fork 122
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
#796 Improved the screen recorder for creating more than 3 minutes long videos #939
Changes from 3 commits
c0f879a
4adcb17
f0474d9
aa07237
194fffc
a94214d
62186d9
f482f12
aac6021
17bc268
5b1cbc7
18ddb75
ba88d7d
2dcdf08
ed82b6f
5316ba3
3afa536
51d7cc0
4ab20e1
9f22612
3431df5
aae7032
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,7 +44,7 @@ class RemoteFileManager(private val device: AndroidDevice) { | |
} | ||
|
||
companion object { | ||
const val MAX_FILENAME = 255 | ||
const val MAX_FILENAME = 254 //we need 1 more char for fileNumber in case of using video attachment > 180 && apiLevel < 34 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This constant is not specific for video files so it should never change. It's a detail about Android filesystem implementation. I suggest changing the logic of the code, not the constants here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
const val TMP_PATH = "/data/local/tmp" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,7 @@ import com.malinskiy.marathon.device.DevicePoolId | |
import com.malinskiy.marathon.device.toDeviceInfo | ||
import com.malinskiy.marathon.execution.Attachment | ||
import com.malinskiy.marathon.execution.AttachmentType | ||
import com.malinskiy.marathon.extension.addFileNumberForVideo | ||
import com.malinskiy.marathon.extension.withTimeoutOrNull | ||
import com.malinskiy.marathon.io.FileManager | ||
import com.malinskiy.marathon.io.FileType | ||
|
@@ -22,6 +23,7 @@ import kotlinx.coroutines.Job | |
import kotlinx.coroutines.SupervisorJob | ||
import kotlinx.coroutines.async | ||
import kotlinx.coroutines.cancelAndJoin | ||
import java.io.File | ||
import java.time.Duration | ||
import kotlin.system.measureTimeMillis | ||
|
||
|
@@ -122,29 +124,56 @@ class ScreenRecorderTestBatchListener( | |
} | ||
|
||
private suspend fun pullTestVideo(test: Test) { | ||
val localVideoFile = fileManager.createFile(FileType.VIDEO, pool, device.toDeviceInfo(), test, testBatchId) | ||
val localVideoFiles = mutableListOf<File>() | ||
val remoteFilePath = device.fileManager.remoteVideoForTest(test, testBatchId) | ||
val millis = measureTimeMillis { | ||
device.safePullFile(remoteFilePath, localVideoFile.toString()) | ||
if(device.apiLevel >= 34 || videoConfiguration.timeLimit <= 180) { | ||
val localVideoFile = fileManager.createFile(FileType.VIDEO, pool, device.toDeviceInfo(), test, testBatchId) | ||
device.safePullFile(remoteFilePath, localVideoFile.toString()) | ||
localVideoFiles.add(localVideoFile) | ||
} else { | ||
for (i in 0 .. (videoConfiguration.timeLimit / 180)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suspect that the actual number of files depends on the time the test took, not the configuration limit, so the loop should take into account only the time passed or/and number of shell invocations with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
val localVideoFile = fileManager.createFile(FileType.VIDEO, pool, device.toDeviceInfo(), test, testBatchId, i.toString()) | ||
device.safePullFile(remoteFilePath.addFileNumberForVideo(i.toString()), localVideoFile.toString()) | ||
localVideoFiles.add(localVideoFile) | ||
} | ||
} | ||
} | ||
logger.trace { "Pulling finished in ${millis}ms $remoteFilePath " } | ||
attachmentListeners.forEach { it.onAttachment(test, Attachment(localVideoFile, AttachmentType.VIDEO)) } | ||
attachmentListeners.forEach { | ||
localVideoFiles.forEach { localVideoFile -> | ||
it.onAttachment(test, Attachment(localVideoFile, AttachmentType.VIDEO)) | ||
} | ||
} | ||
} | ||
|
||
/** | ||
* This can be called both when test times out and device unavailable | ||
*/ | ||
private suspend fun pullLastBatchVideo(remoteFilePath: String) { | ||
val localVideoFile = fileManager.createFile(FileType.VIDEO, pool, device.toDeviceInfo(), testBatchId = testBatchId) | ||
val millis = measureTimeMillis { | ||
device.safePullFile(remoteFilePath, localVideoFile.toString()) | ||
if(device.apiLevel >= 34 || videoConfiguration.timeLimit <= 180) { | ||
val localVideoFile = fileManager.createFile(FileType.VIDEO, pool, device.toDeviceInfo(), testBatchId = testBatchId) | ||
device.safePullFile(remoteFilePath, localVideoFile.toString()) | ||
} else { | ||
for (i in 0 .. (videoConfiguration.timeLimit / 180)) { | ||
val localVideoFile = fileManager.createFile(FileType.VIDEO, pool, device.toDeviceInfo(), testBatchId = testBatchId, id = i.toString()) | ||
device.safePullFile(remoteFilePath.addFileNumberForVideo(i.toString()), localVideoFile.toString()) | ||
} | ||
} | ||
} | ||
logger.trace { "Pulling finished in ${millis}ms $remoteFilePath " } | ||
} | ||
|
||
private suspend fun removeRemoteVideo(remoteFilePath: String) { | ||
val millis = measureTimeMillis { | ||
device.fileManager.removeRemotePath(remoteFilePath) | ||
if(device.apiLevel >= 34 || videoConfiguration.timeLimit <= 180) { | ||
device.fileManager.removeRemotePath(remoteFilePath) | ||
} else { | ||
for (i in 0 .. (videoConfiguration.timeLimit / 180)) { | ||
device.fileManager.removeRemotePath(remoteFilePath.addFileNumberForVideo(i.toString())) | ||
} | ||
} | ||
} | ||
logger.trace { "Removed file in ${millis}ms $remoteFilePath" } | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like very specific thing for stringextensions. I suggest putting this somewhere closer to the video file creation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done