Skip to content

Commit

Permalink
refactor: data scratch file references (#1841)
Browse files Browse the repository at this point in the history
  • Loading branch information
Sloox authored Apr 29, 2021
1 parent 2f21a19 commit c5122e8
Show file tree
Hide file tree
Showing 19 changed files with 113 additions and 58 deletions.
12 changes: 12 additions & 0 deletions test_runner/src/main/kotlin/ftl/adapter/DownloadAsJunitXML.kt
Original file line number Diff line number Diff line change
@@ -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()
}
11 changes: 11 additions & 0 deletions test_runner/src/main/kotlin/ftl/adapter/GcStorageDownload.kt
Original file line number Diff line number Diff line change
@@ -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)
}
Original file line number Diff line number Diff line change
@@ -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)
17 changes: 17 additions & 0 deletions test_runner/src/main/kotlin/ftl/api/FileReference.kt
Original file line number Diff line number Diff line change
@@ -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
}
5 changes: 3 additions & 2 deletions test_runner/src/main/kotlin/ftl/args/ArgsHelper.kt
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@ 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
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
Expand All @@ -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
Expand Down Expand Up @@ -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) }
}
Expand Down
25 changes: 25 additions & 0 deletions test_runner/src/main/kotlin/ftl/client/google/FileReference.kt
Original file line number Diff line number Diff line change
@@ -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)) }
14 changes: 13 additions & 1 deletion test_runner/src/main/kotlin/ftl/client/google/GcStorage.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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() }
Expand Down Expand Up @@ -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" }
Expand Down
13 changes: 0 additions & 13 deletions test_runner/src/main/kotlin/ftl/client/google/GcStorageAdapter.kt

This file was deleted.

8 changes: 4 additions & 4 deletions test_runner/src/main/kotlin/ftl/reports/util/ReportManager.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -63,7 +63,7 @@ private fun InstrumentationTestContext.userShards(customShardingMap: Map<String,
}
?: this

private fun FileReference.hasReference(path: String) = local == path || gcs == path
private fun FileReference.hasReference(path: String) = local == path || remote == path

private val AndroidArgs.useCustomSharding: Boolean
get() = customSharding.isNotEmpty()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import ftl.shard.testCases
internal fun AndroidArgs.createInstrumentationConfig(
testApk: InstrumentationTestContext
) = AndroidTestConfig.Instrumentation(
appApkGcsPath = testApk.app.gcs,
testApkGcsPath = testApk.test.gcs,
appApkGcsPath = testApk.app.remote,
testApkGcsPath = testApk.test.remote,
testRunnerClass = testRunnerClass,
orchestratorOption = "USE_ORCHESTRATOR".takeIf { useOrchestrator },
disableSharding = disableSharding,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@ import ftl.run.model.SanityRoboTestContext
internal fun AndroidArgs.createRoboConfig(
testApk: RoboTestContext
) = AndroidTestConfig.Robo(
appApkGcsPath = testApk.app.gcs,
appApkGcsPath = testApk.app.remote,
flankRoboDirectives = roboDirectives,
roboScriptGcsPath = testApk.roboScript.gcs
roboScriptGcsPath = testApk.roboScript.remote
)

internal fun createSanityRoboConfig(
testApk: SanityRoboTestContext
) = AndroidTestConfig.Robo(
appApkGcsPath = testApk.app.gcs,
appApkGcsPath = testApk.app.remote,
flankRoboDirectives = null,
roboScriptGcsPath = null
)
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,6 @@ private suspend fun List<String>.uploadToGcloudIfNeeded(
resultsBucket: String
) = coroutineScope {
map {
async { it.asFileReference().uploadIfNeeded(resultsBucket, runGcsPath).gcs }
async { it.asFileReference().uploadIfNeeded(resultsBucket, runGcsPath).remote }
}.awaitAll()
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import kotlinx.coroutines.flow.flowOf

internal fun IosArgs.createGameloopTestContexts(): Flow<IosTestContext> {
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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import kotlinx.coroutines.flow.map

internal fun IosArgs.createXcTestContexts(): Flow<IosTestContext> {
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()
Expand Down
32 changes: 7 additions & 25 deletions test_runner/src/main/kotlin/ftl/util/FileReference.kt
Original file line number Diff line number Diff line change
@@ -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(
Expand All @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down

0 comments on commit c5122e8

Please sign in to comment.