From c5122e8f833f50623444420629f2b0fec037849c Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Thu, 29 Apr 2021 16:50:25 +0200 Subject: [PATCH] refactor: data scratch file references (#1841) --- .../kotlin/ftl/adapter/DownloadAsJunitXML.kt | 12 +++++++ .../kotlin/ftl/adapter/GcStorageDownload.kt | 11 +++++++ .../adapter/google/FileReferenceAdapter.kt | 8 +++++ .../src/main/kotlin/ftl/api/FileReference.kt | 17 ++++++++++ .../src/main/kotlin/ftl/args/ArgsHelper.kt | 5 +-- .../kotlin/ftl/client/google/FileReference.kt | 25 +++++++++++++++ .../kotlin/ftl/client/google/GcStorage.kt | 14 +++++++- .../ftl/client/google/GcStorageAdapter.kt | 13 -------- .../kotlin/ftl/reports/util/ReportManager.kt | 8 ++--- .../ftl/run/model/AndroidTestContext.kt | 2 +- .../android/CreateAndroidLoopConfig.kt | 2 +- .../android/CreateAndroidTestContext.kt | 4 +-- .../android/CreateInstrumentationConfig.kt | 4 +-- .../run/platform/android/CreateRoboConfig.kt | 6 ++-- .../ftl/run/platform/android/UploadApks.kt | 2 +- .../platform/ios/CreateGameloopTestContext.kt | 2 +- .../run/platform/ios/CreateXcTestContext.kt | 2 +- .../src/main/kotlin/ftl/util/FileReference.kt | 32 ++++--------------- .../android/CreateAndroidTestContextKtTest.kt | 2 +- 19 files changed, 113 insertions(+), 58 deletions(-) create mode 100644 test_runner/src/main/kotlin/ftl/adapter/DownloadAsJunitXML.kt create mode 100644 test_runner/src/main/kotlin/ftl/adapter/GcStorageDownload.kt create mode 100644 test_runner/src/main/kotlin/ftl/adapter/google/FileReferenceAdapter.kt create mode 100644 test_runner/src/main/kotlin/ftl/api/FileReference.kt create mode 100644 test_runner/src/main/kotlin/ftl/client/google/FileReference.kt delete mode 100644 test_runner/src/main/kotlin/ftl/client/google/GcStorageAdapter.kt diff --git a/test_runner/src/main/kotlin/ftl/adapter/DownloadAsJunitXML.kt b/test_runner/src/main/kotlin/ftl/adapter/DownloadAsJunitXML.kt new file mode 100644 index 0000000000..ea9679a2f7 --- /dev/null +++ b/test_runner/src/main/kotlin/ftl/adapter/DownloadAsJunitXML.kt @@ -0,0 +1,12 @@ +package ftl.adapter + +import ftl.adapter.google.toApiModel +import ftl.api.FileReference +import ftl.client.google.downloadAsJunitXml +import ftl.reports.xml.model.JUnitTestResult + +object DownloadAsJunitXML : + FileReference.DownloadAsXML, + (FileReference) -> JUnitTestResult by { fileReference -> + downloadAsJunitXml(fileReference).toApiModel() + } diff --git a/test_runner/src/main/kotlin/ftl/adapter/GcStorageDownload.kt b/test_runner/src/main/kotlin/ftl/adapter/GcStorageDownload.kt new file mode 100644 index 0000000000..fc1f4bd53e --- /dev/null +++ b/test_runner/src/main/kotlin/ftl/adapter/GcStorageDownload.kt @@ -0,0 +1,11 @@ +package ftl.adapter + +import ftl.adapter.google.toApiModel +import ftl.api.FileReference +import ftl.client.google.fileReferenceDownload + +object GcStorageDownload : + FileReference.Download, + (FileReference, Boolean, Boolean) -> FileReference by { fileReference, ifNeeded, ignoreErrors -> + fileReferenceDownload(fileReference, ifNeeded, ignoreErrors).toApiModel(fileReference) + } diff --git a/test_runner/src/main/kotlin/ftl/adapter/google/FileReferenceAdapter.kt b/test_runner/src/main/kotlin/ftl/adapter/google/FileReferenceAdapter.kt new file mode 100644 index 0000000000..be5701c239 --- /dev/null +++ b/test_runner/src/main/kotlin/ftl/adapter/google/FileReferenceAdapter.kt @@ -0,0 +1,8 @@ +package ftl.adapter.google + +import ftl.api.FileReference +import ftl.reports.xml.model.JUnitTestResult + +internal fun String.toApiModel(fileReference: FileReference) = fileReference.copy(local = this) + +internal fun JUnitTestResult?.toApiModel() = JUnitTestResult(testsuites = this?.testsuites) diff --git a/test_runner/src/main/kotlin/ftl/api/FileReference.kt b/test_runner/src/main/kotlin/ftl/api/FileReference.kt new file mode 100644 index 0000000000..b6216a9c2e --- /dev/null +++ b/test_runner/src/main/kotlin/ftl/api/FileReference.kt @@ -0,0 +1,17 @@ +package ftl.api + +import ftl.adapter.DownloadAsJunitXML +import ftl.adapter.GcStorageDownload +import ftl.reports.xml.model.JUnitTestResult + +val downloadFileReference: FileReference.Download get() = GcStorageDownload +val downloadAsJunitXML: FileReference.DownloadAsXML get() = DownloadAsJunitXML + +data class FileReference( + val local: String = "", + val remote: String = "" +) { + + interface Download : (FileReference, Boolean, Boolean) -> FileReference + interface DownloadAsXML : (FileReference) -> JUnitTestResult +} diff --git a/test_runner/src/main/kotlin/ftl/args/ArgsHelper.kt b/test_runner/src/main/kotlin/ftl/args/ArgsHelper.kt index 2bd98ae1a0..936fbc1cca 100644 --- a/test_runner/src/main/kotlin/ftl/args/ArgsHelper.kt +++ b/test_runner/src/main/kotlin/ftl/args/ArgsHelper.kt @@ -13,6 +13,7 @@ import flank.common.defaultCredentialPath import flank.common.isWindows import flank.common.logLn import ftl.adapter.google.credential +import ftl.api.downloadAsJunitXML import ftl.args.IArgs.Companion.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE import ftl.args.yml.YamlObjectMapper import ftl.client.google.GcStorage @@ -20,7 +21,6 @@ import ftl.config.FtlConstants.GCS_PREFIX import ftl.config.FtlConstants.JSON_FACTORY import ftl.config.FtlConstants.useMock import ftl.gc.GcToolResults -import ftl.reports.xml.model.JUnitTestResult import ftl.run.exception.FlankConfigurationError import ftl.run.exception.FlankGeneralError import ftl.shard.Chunk @@ -30,6 +30,7 @@ import ftl.shard.shardCountByTime import ftl.util.FlankTestMethod import ftl.util.assertNotEmpty import ftl.util.getGACPathOrEmpty +import ftl.util.getSmartFlankGCSPathAsFileReference import java.io.File import java.net.URI import java.nio.charset.StandardCharsets @@ -255,7 +256,7 @@ object ArgsHelper { ) ) } else { - val oldTestResult = GcStorage.downloadJunitXml(args) ?: JUnitTestResult(mutableListOf()) + val oldTestResult = downloadAsJunitXML(args.getSmartFlankGCSPathAsFileReference()) val shardCount = forcedShardCount ?: shardCountByTime(testsToExecute, oldTestResult, args) createShardsByShardCount(testsToExecute, oldTestResult, args, shardCount).map { Chunk(it.testMethods) } } diff --git a/test_runner/src/main/kotlin/ftl/client/google/FileReference.kt b/test_runner/src/main/kotlin/ftl/client/google/FileReference.kt new file mode 100644 index 0000000000..e9a38ac371 --- /dev/null +++ b/test_runner/src/main/kotlin/ftl/client/google/FileReference.kt @@ -0,0 +1,25 @@ +package ftl.client.google + +import ftl.api.FileReference +import ftl.reports.xml.model.JUnitTestResult +import ftl.reports.xml.parseAllSuitesXml +import ftl.run.exception.FlankGeneralError +import java.nio.file.Paths + +internal fun fileReferenceDownload(fileReference: FileReference, ifNeeded: Boolean, ignoreErrors: Boolean): String { + if (!ignoreErrors) { + if (fileReference.local.isBlank() && fileReference.remote.isBlank()) throw FlankGeneralError("Cannot create empty FileReference") + } + return when { + ifNeeded -> { + if (fileReference.local.isNotBlank()) fileReference.local + else gcStorageDownload(fileReference.remote, ignoreErrors) + } + else -> gcStorageDownload(fileReference.remote, ignoreErrors) + } +} + +internal fun downloadAsJunitXml(fileReference: FileReference): JUnitTestResult? = + fileReferenceDownload(fileReference, ifNeeded = false, ignoreErrors = true) + .takeIf { it.isNotEmpty() } + ?.let { parseAllSuitesXml(Paths.get(it)) } diff --git a/test_runner/src/main/kotlin/ftl/client/google/GcStorage.kt b/test_runner/src/main/kotlin/ftl/client/google/GcStorage.kt index 41482a4816..31d53f98cd 100644 --- a/test_runner/src/main/kotlin/ftl/client/google/GcStorage.kt +++ b/test_runner/src/main/kotlin/ftl/client/google/GcStorage.kt @@ -77,7 +77,7 @@ object GcStorage { } // junit xml may not exist. ignore error if it doesn't exist - fun downloadJunitXml( + private fun downloadJunitXml( args: IArgs ): JUnitTestResult? = download(args.smartFlankGcsPath, ignoreError = true) .takeIf { it.isNotEmpty() } @@ -160,6 +160,18 @@ object GcStorage { private fun String.dropLeadingSlash() = drop(1) } +internal fun gcStorageUpload( + filePath: String, + fileBytes: ByteArray, + rootGcsBucket: String, + runGcsPath: String +) = if (filePath.startsWith(GCS_PREFIX)) filePath else GcStorage.upload(filePath, fileBytes, rootGcsBucket, runGcsPath) + +internal fun gcStorageExist(rootGcsBucket: String, runGcsPath: String) = GcStorage.exist(rootGcsBucket, runGcsPath) + +internal fun gcStorageDownload(gcsUriString: String, ignoreError: Boolean = false) = + GcStorage.download(gcsUriString, ignoreError) + object TestStorageProvider { init { require(FtlConstants.useMock) { "Storage provider can be used only during tests" } diff --git a/test_runner/src/main/kotlin/ftl/client/google/GcStorageAdapter.kt b/test_runner/src/main/kotlin/ftl/client/google/GcStorageAdapter.kt deleted file mode 100644 index da7379f9b4..0000000000 --- a/test_runner/src/main/kotlin/ftl/client/google/GcStorageAdapter.kt +++ /dev/null @@ -1,13 +0,0 @@ -package ftl.client.google - -import ftl.config.FtlConstants - -internal fun gcStorageUpload( - filePath: String, - fileBytes: ByteArray, - rootGcsBucket: String, - runGcsPath: String -) = if (filePath.startsWith(FtlConstants.GCS_PREFIX)) filePath -else GcStorage.upload(filePath, fileBytes, rootGcsBucket, runGcsPath) - -internal fun gcStorageExist(rootGcsBucket: String, runGcsPath: String) = GcStorage.exist(rootGcsBucket, runGcsPath) diff --git a/test_runner/src/main/kotlin/ftl/reports/util/ReportManager.kt b/test_runner/src/main/kotlin/ftl/reports/util/ReportManager.kt index ca3f068be5..e365be2bf4 100644 --- a/test_runner/src/main/kotlin/ftl/reports/util/ReportManager.kt +++ b/test_runner/src/main/kotlin/ftl/reports/util/ReportManager.kt @@ -4,6 +4,7 @@ import com.google.common.annotations.VisibleForTesting import com.google.testing.model.TestExecution import flank.common.logLn import ftl.api.RemoteStorage +import ftl.api.downloadAsJunitXML import ftl.api.uploadToRemoteStorage import ftl.args.AndroidArgs import ftl.args.IArgs @@ -31,6 +32,7 @@ import ftl.run.common.getMatrixFilePath import ftl.run.exception.FlankGeneralError import ftl.shard.createTestMethodDurationMap import ftl.util.Artifacts +import ftl.util.getSmartFlankGCSPathAsFileReference import ftl.util.resolveLocalRunPath import java.io.File import java.nio.file.Files @@ -249,15 +251,13 @@ object ReportManager { ) { if (newTestResult == null || newTestResult.testsuites.isNullOrEmpty()) return - val oldTestResult = GcStorage.downloadJunitXml(args) + val oldTestResult = downloadAsJunitXML(args.getSmartFlankGCSPathAsFileReference()) if (args.useLegacyJUnitResult) { newTestResult.mergeTestTimes(oldTestResult) } - if (oldTestResult != null) { - printActual(oldTestResult, newTestResult, args, testShardChunks) - } + printActual(oldTestResult, newTestResult, args, testShardChunks) GcStorage.uploadJunitXml(newTestResult, args) } diff --git a/test_runner/src/main/kotlin/ftl/run/model/AndroidTestContext.kt b/test_runner/src/main/kotlin/ftl/run/model/AndroidTestContext.kt index 56ec0be027..151e10455b 100644 --- a/test_runner/src/main/kotlin/ftl/run/model/AndroidTestContext.kt +++ b/test_runner/src/main/kotlin/ftl/run/model/AndroidTestContext.kt @@ -1,9 +1,9 @@ package ftl.run.model +import ftl.api.FileReference import ftl.args.IgnoredTestCases import ftl.args.ShardChunks import ftl.shard.Chunk -import ftl.util.FileReference sealed class AndroidTestContext diff --git a/test_runner/src/main/kotlin/ftl/run/platform/android/CreateAndroidLoopConfig.kt b/test_runner/src/main/kotlin/ftl/run/platform/android/CreateAndroidLoopConfig.kt index d6c308b08d..e24a498eba 100644 --- a/test_runner/src/main/kotlin/ftl/run/platform/android/CreateAndroidLoopConfig.kt +++ b/test_runner/src/main/kotlin/ftl/run/platform/android/CreateAndroidLoopConfig.kt @@ -6,7 +6,7 @@ import ftl.run.model.GameLoopContext internal fun AndroidArgs.createAndroidLoopConfig( testApk: GameLoopContext ) = AndroidTestConfig.GameLoop( - appApkGcsPath = testApk.app.gcs, + appApkGcsPath = testApk.app.remote, testRunnerClass = testRunnerClass, scenarioLabels = scenarioLabels, scenarioNumbers = scenarioNumbers diff --git a/test_runner/src/main/kotlin/ftl/run/platform/android/CreateAndroidTestContext.kt b/test_runner/src/main/kotlin/ftl/run/platform/android/CreateAndroidTestContext.kt index 67f7d5a65a..3509ac2f15 100644 --- a/test_runner/src/main/kotlin/ftl/run/platform/android/CreateAndroidTestContext.kt +++ b/test_runner/src/main/kotlin/ftl/run/platform/android/CreateAndroidTestContext.kt @@ -11,6 +11,7 @@ import com.linkedin.dex.parser.getClassAnnotationValues import com.linkedin.dex.spec.ClassDefItem import com.linkedin.dex.spec.DexFile import flank.common.logLn +import ftl.api.FileReference import ftl.args.AndroidArgs import ftl.args.ArgsHelper import ftl.args.CalculateShardsResult @@ -22,7 +23,6 @@ import ftl.run.model.AndroidTestShards import ftl.run.model.InstrumentationTestContext import ftl.shard.Chunk import ftl.shard.createShardsByTestForShards -import ftl.util.FileReference import ftl.util.FlankTestMethod import ftl.util.downloadIfNeeded import kotlinx.coroutines.async @@ -63,7 +63,7 @@ private fun InstrumentationTestContext.userShards(customShardingMap: Map.uploadToGcloudIfNeeded( resultsBucket: String ) = coroutineScope { map { - async { it.asFileReference().uploadIfNeeded(resultsBucket, runGcsPath).gcs } + async { it.asFileReference().uploadIfNeeded(resultsBucket, runGcsPath).remote } }.awaitAll() } diff --git a/test_runner/src/main/kotlin/ftl/run/platform/ios/CreateGameloopTestContext.kt b/test_runner/src/main/kotlin/ftl/run/platform/ios/CreateGameloopTestContext.kt index 47e265bfac..406c17dadb 100644 --- a/test_runner/src/main/kotlin/ftl/run/platform/ios/CreateGameloopTestContext.kt +++ b/test_runner/src/main/kotlin/ftl/run/platform/ios/CreateGameloopTestContext.kt @@ -12,7 +12,7 @@ import kotlinx.coroutines.flow.flowOf internal fun IosArgs.createGameloopTestContexts(): Flow { val shardCounter = ShardCounter() - val appGcsPath = uploadIfNeeded(app.asFileReference()).gcs + val appGcsPath = uploadIfNeeded(app.asFileReference()).remote val gcsBucket = resultsBucket val shardName = shardCounter.next() val matrixGcsSuffix = join(resultsDir, shardName) diff --git a/test_runner/src/main/kotlin/ftl/run/platform/ios/CreateXcTestContext.kt b/test_runner/src/main/kotlin/ftl/run/platform/ios/CreateXcTestContext.kt index c435fa081c..cda0712374 100644 --- a/test_runner/src/main/kotlin/ftl/run/platform/ios/CreateXcTestContext.kt +++ b/test_runner/src/main/kotlin/ftl/run/platform/ios/CreateXcTestContext.kt @@ -15,7 +15,7 @@ import kotlinx.coroutines.flow.map internal fun IosArgs.createXcTestContexts(): Flow { val shardCounter = ShardCounter() - val xcTestGcsPath = uploadIfNeeded(xctestrunZip.asFileReference()).gcs + val xcTestGcsPath = uploadIfNeeded(xctestrunZip.asFileReference()).remote val gcsBucket = resultsBucket return xcTestRunFlow().map { xcTestRun -> val shardName = shardCounter.next() diff --git a/test_runner/src/main/kotlin/ftl/util/FileReference.kt b/test_runner/src/main/kotlin/ftl/util/FileReference.kt index 42c7005b8c..84d7b48640 100644 --- a/test_runner/src/main/kotlin/ftl/util/FileReference.kt +++ b/test_runner/src/main/kotlin/ftl/util/FileReference.kt @@ -1,36 +1,20 @@ package ftl.util +import ftl.api.FileReference import ftl.api.RemoteStorage +import ftl.api.downloadFileReference import ftl.api.uploadToRemoteStorage import ftl.args.IArgs -import ftl.client.google.GcStorage import ftl.config.FtlConstants -import ftl.run.exception.FlankGeneralError import java.nio.file.Files import java.nio.file.Paths -data class FileReference( - val local: String = "", - val gcs: String = "" -) { - init { - assertNotEmpty() - } -} - -private fun FileReference.assertNotEmpty() { - if (local.isBlank() && gcs.isBlank()) - throw FlankGeneralError("Cannot create empty FileReference") -} - fun String.asFileReference(): FileReference = - if (startsWith(FtlConstants.GCS_PREFIX)) - FileReference(gcs = this) else - FileReference(local = this) + if (startsWith(FtlConstants.GCS_PREFIX)) FileReference(remote = this) else FileReference(local = this) + +fun IArgs.getSmartFlankGCSPathAsFileReference() = this.smartFlankGcsPath.asFileReference() -fun FileReference.downloadIfNeeded() = - if (local.isNotBlank()) this - else copy(local = GcStorage.download(gcs)) +internal fun FileReference.downloadIfNeeded() = downloadFileReference(this, true, false) fun IArgs.uploadIfNeeded(file: FileReference): FileReference = file.uploadIfNeeded( @@ -41,9 +25,7 @@ fun IArgs.uploadIfNeeded(file: FileReference): FileReference = fun FileReference.uploadIfNeeded( rootGcsBucket: String, runGcsPath: String -): FileReference = - if (gcs.isNotBlank()) this - else copy(gcs = upload(local, rootGcsBucket, runGcsPath)) +): FileReference = if (remote.isNotBlank()) this else copy(remote = upload(local, rootGcsBucket, runGcsPath)) fun upload(file: String, rootGcsBucket: String, runGcsPath: String): String { return uploadToRemoteStorage( diff --git a/test_runner/src/test/kotlin/ftl/run/platform/android/CreateAndroidTestContextKtTest.kt b/test_runner/src/test/kotlin/ftl/run/platform/android/CreateAndroidTestContextKtTest.kt index a773a08970..a3c3841b31 100644 --- a/test_runner/src/test/kotlin/ftl/run/platform/android/CreateAndroidTestContextKtTest.kt +++ b/test_runner/src/test/kotlin/ftl/run/platform/android/CreateAndroidTestContextKtTest.kt @@ -5,6 +5,7 @@ import com.linkedin.dex.parser.DexParser import com.linkedin.dex.parser.DexParser.Companion.findTestMethods import com.linkedin.dex.parser.TestMethod import flank.common.isWindows +import ftl.api.FileReference import ftl.args.AndroidArgs import ftl.args.normalizeFilePath import ftl.filter.TestFilter @@ -16,7 +17,6 @@ import ftl.run.model.InstrumentationTestContext import ftl.run.model.RoboTestContext import ftl.test.util.mixedConfigYaml import ftl.test.util.should -import ftl.util.FileReference import ftl.util.FlankTestMethod import io.mockk.every import io.mockk.mockk