Skip to content
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

fix: firebase refresh fails when test zip file doesn't exist (#1052) #1054

Merged
merged 8 commits into from
Sep 1, 2020
Merged
5 changes: 1 addition & 4 deletions test_runner/src/main/kotlin/ftl/args/AndroidArgsCompanion.kt
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,5 @@ open class AndroidArgsCompanion : IArgs.ICompanion {
config = defaultAndroidConfig() +
loadAndroidConfig(reader = yamlReader) +
cli?.config
).apply {
commonArgs.validate()
this.validate()
}
)
}
11 changes: 7 additions & 4 deletions test_runner/src/main/kotlin/ftl/args/ArgsHelper.kt
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,10 @@ object ArgsHelper {
}
}

fun String.processFilePath(name: String): String =
if (startsWith(GCS_PREFIX))
this.also { ArgsHelper.assertGcsFileExists(it) } else
ArgsHelper.evaluateFilePath(this).also { ArgsHelper.assertFileExists(it, name) }
fun String.normalizeFilePath(): String =
if (startsWith(GCS_PREFIX)) this
else try {
ArgsHelper.evaluateFilePath(this)
} catch (e: Throwable) {
this
}
14 changes: 7 additions & 7 deletions test_runner/src/main/kotlin/ftl/args/CreateAndroidArgs.kt
Original file line number Diff line number Diff line change
Expand Up @@ -13,26 +13,26 @@ fun createAndroidArgs(
) = AndroidArgs(
commonArgs = commonArgs,
// gcloud
appApk = gcloud.app?.processFilePath("from app"),
testApk = gcloud.test?.processFilePath("from test"),
appApk = gcloud.app?.normalizeFilePath(),
testApk = gcloud.test?.normalizeFilePath(),
useOrchestrator = gcloud.useOrchestrator!!,
testTargets = gcloud.testTargets!!.filterNotNull(),
testRunnerClass = gcloud.testRunnerClass,
roboDirectives = gcloud.roboDirectives!!.parseRoboDirectives(),
performanceMetrics = gcloud.performanceMetrics!!,
otherFiles = gcloud.otherFiles!!.mapValues { (_, path) -> path.processFilePath("from otherFiles") },
otherFiles = gcloud.otherFiles!!.mapValues { (_, path) -> path.normalizeFilePath() },
numUniformShards = gcloud.numUniformShards,
environmentVariables = gcloud.environmentVariables!!,
directoriesToPull = gcloud.directoriesToPull!!,
autoGoogleLogin = gcloud.autoGoogleLogin!!,
additionalApks = gcloud.additionalApks!!.map { it.processFilePath("from additional-apks") },
roboScript = gcloud.roboScript?.processFilePath("from roboScript"),
additionalApks = gcloud.additionalApks!!.map { it.normalizeFilePath() },
roboScript = gcloud.roboScript?.normalizeFilePath(),

// flank
additionalAppTestApks = flank.additionalAppTestApks?.map { (app, test) ->
AppTestPair(
app = app?.processFilePath("from additional-app-test-apks.app"),
test = test.processFilePath("from additional-app-test-apks.test")
app = app?.normalizeFilePath(),
test = test.normalizeFilePath()
)
} ?: emptyList(),
useLegacyJUnitResult = flank.useLegacyJUnitResult!!
Expand Down
6 changes: 3 additions & 3 deletions test_runner/src/main/kotlin/ftl/args/CreateIosArgs.kt
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ private fun createIosArgs(
commonArgs: CommonArgs
) = IosArgs(
commonArgs = commonArgs.copy(maxTestShards = convertToShardCount(commonArgs.maxTestShards)),
xctestrunZip = gcloud.test?.processFilePath("from test").orEmpty(),
xctestrunFile = gcloud.xctestrunFile?.processFilePath("from xctestrun-file").orEmpty(),
xctestrunZip = gcloud.test?.normalizeFilePath().orEmpty(),
xctestrunFile = gcloud.xctestrunFile?.normalizeFilePath().orEmpty(),
xcodeVersion = gcloud.xcodeVersion,
testTargets = flank.testTargets!!.filterNotNull()
testTargets = flank.testTargets?.filterNotNull().orEmpty()
)

private fun convertToShardCount(inputValue: Int) =
Expand Down
5 changes: 1 addition & 4 deletions test_runner/src/main/kotlin/ftl/args/IosArgsCompanion.kt
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,5 @@ open class IosArgsCompanion : IArgs.ICompanion {
config = defaultIosConfig() +
loadIosConfig(reader = yamlReader) +
cli?.config
).apply {
commonArgs.validate()
this.validate()
}
)
}
78 changes: 56 additions & 22 deletions test_runner/src/main/kotlin/ftl/args/ValidateAndroidArgs.kt
Original file line number Diff line number Diff line change
Expand Up @@ -11,25 +11,16 @@ import ftl.run.exception.FlankConfigurationError
import ftl.run.exception.IncompatibleTestDimensionError
import java.io.File

fun AndroidArgs.validate() {
assertAdditionalAppTestApks()
fun AndroidArgs.validate() = apply {
commonArgs.validate()
assertDevicesSupported()
assertShards()
assertTestTypes()
assertRoboTest()
assertDirectoriesToPull()
assertMaxTestShardsByDeviceType()
assertParametersConflict()
}

private fun AndroidArgs.assertAdditionalAppTestApks() {
if (appApk == null) additionalAppTestApks
.filter { (app, _) -> app == null }
.map { File(it.test).name }
.run {
if (isNotEmpty())
throw FlankConfigurationError("Cannot resolve app apk pair for $this")
}
assertTestFiles()
assertOtherFiles()
}

private fun AndroidArgs.assertDevicesSupported() = devices
Expand Down Expand Up @@ -57,13 +48,6 @@ private fun AndroidArgs.assertTestTypes() {
)
}

private fun AndroidArgs.assertRoboTest() {
// Using both roboDirectives and roboScript may hang test execution on FTL
if (roboDirectives.isNotEmpty() && roboScript != null) throw FlankConfigurationError(
"Options robo-directives and robo-script are mutually exclusive, use only one of them."
)
}

// Validation is done according to https://cloud.google.com/sdk/gcloud/reference/firebase/test/android/run#--directories-to-pull
private fun AndroidArgs.assertDirectoriesToPull() {
val correctNameRegex = "(/[a-zA-Z0-9_\\-.+]+)+/?".toRegex()
Expand All @@ -73,8 +57,8 @@ private fun AndroidArgs.assertDirectoriesToPull() {
?.also {
throw FlankConfigurationError(
"Invalid value for [directories-to-pull]: Invalid path $it.\n" +
"Path must be absolute paths under /sdcard or /data/local/tmp (for example, --directories-to-pull /sdcard/tempDir1,/data/local/tmp/tempDir2).\n" +
"Path names are restricted to the characters [a-zA-Z0-9_-./+]. "
"Path must be absolute paths under /sdcard or /data/local/tmp (for example, --directories-to-pull /sdcard/tempDir1,/data/local/tmp/tempDir2).\n" +
"Path names are restricted to the characters [a-zA-Z0-9_-./+]. "
)
}
}
Expand Down Expand Up @@ -105,3 +89,53 @@ private fun AndroidArgs.assertParametersConflict() {
if (useLegacyJUnitResult && fullJUnitResult)
throw FlankConfigurationError("Parameters conflict, you cannot set: `--legacy-junit-result` and `--full-junit-result` at the same time.")
}

private fun AndroidArgs.assertTestFiles() {
if (isInstrumentationTest) assertInstrumentationTest()
if (isRoboTest) assertRoboTest()
}

private fun AndroidArgs.assertInstrumentationTest() {
assertAdditionalAppTestApks()
assertApkFilePaths()
}

private fun AndroidArgs.assertAdditionalAppTestApks() {
if (appApk == null) additionalAppTestApks
.filter { (app, _) -> app == null }
.map { File(it.test).name }
.run { if (isNotEmpty()) throw FlankConfigurationError("Cannot resolve app apk pair for $this") }
}

private fun AndroidArgs.assertApkFilePaths() {
appApkPath().forEach { (file, comment) ->
ArgsHelper.assertFileExists(file, comment)
}
}

private fun AndroidArgs.appApkPath(): Map<String, String> =
mapOf(
appApk to "from app",
testApk to "from test"
).filterNotNull() + additionalAppTestApks.fold(emptyMap<String, String>()) { acc, pair ->
acc + mapOf(
pair.app to "from additional-app-test-apks.app",
pair.test to "from additional-app-test-apks.test"
).filterNotNull()
}

private fun Map<String?, String>.filterNotNull() = filter { it.key != null }.mapKeys { it.key!! }

private fun AndroidArgs.assertRoboTest() {
// Using both roboDirectives and roboScript may hang test execution on FTL
if (roboDirectives.isNotEmpty() && roboScript != null) throw FlankConfigurationError(
"Options robo-directives and robo-script are mutually exclusive, use only one of them."
)
if (roboScript != null)
ArgsHelper.assertFileExists(roboScript.toString(), "from roboScript")
ArgsHelper.assertFileExists(appApk.toString(), "from app")
}

private fun AndroidArgs.assertOtherFiles() {
otherFiles.forEach { (_, path) -> ArgsHelper.assertFileExists(path, "from otherFiles") }
}
18 changes: 17 additions & 1 deletion test_runner/src/main/kotlin/ftl/args/ValidateIosArgs.kt
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,23 @@ import ftl.ios.IosCatalog
import ftl.run.exception.FlankConfigurationError
import ftl.run.exception.IncompatibleTestDimensionError

fun IosArgs.validate() {
fun IosArgs.validate() = apply {
commonArgs.validate()
assertXcodeSupported()
assertDevicesSupported()
assertTestTypes()
assertMaxTestShards()
assertTestFiles()
}

fun IosArgs.validateRefresh() = apply {
commonArgs.validate()
assertXcodeSupported()
assertDevicesSupported()
assertTestTypes()
assertMaxTestShards()
}

private fun IosArgs.assertMaxTestShards() { this.maxTestShards
if (
maxTestShards !in IArgs.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE &&
Expand All @@ -32,3 +43,8 @@ private fun IosArgs.assertDevicesSupported() = devices.forEach { device ->
if (!IosCatalog.supportedDevice(device.model, device.version, this.project))
throw IncompatibleTestDimensionError("iOS ${device.version} on ${device.model} is not a supported device")
}

private fun IosArgs.assertTestFiles() {
ArgsHelper.assertFileExists(xctestrunFile, "from test")
ArgsHelper.assertFileExists(xctestrunZip, "from xctestrun-file")
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package ftl.cli.firebase.test.android

import ftl.args.AndroidArgs
import ftl.args.validate
import ftl.cli.firebase.test.CommonRunCommand
import ftl.config.FtlConstants
import ftl.config.emptyAndroidConfig
Expand Down Expand Up @@ -43,7 +44,7 @@ class AndroidRunCommand : CommonRunCommand(), Runnable {
MockServer.start()
}

val config = AndroidArgs.load(Paths.get(configPath), cli = this)
val config = AndroidArgs.load(Paths.get(configPath), cli = this).validate()
runBlocking {
if (dumpShards) dumpShards(args = config, obfuscatedOutput = obfuscate)
else newTestRun(config)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package ftl.cli.firebase.test.ios

import ftl.args.IosArgs
import ftl.args.validate
import ftl.cli.firebase.test.CommonRunCommand
import ftl.config.FtlConstants
import ftl.config.emptyIosConfig
Expand Down Expand Up @@ -43,7 +44,7 @@ class IosRunCommand : CommonRunCommand(), Runnable {
MockServer.start()
}

val config = IosArgs.load(Paths.get(configPath), cli = this)
val config = IosArgs.load(Paths.get(configPath), cli = this).validate()

if (dumpShards) {
dumpShards(args = config, obfuscatedOutput = obfuscate)
Expand Down
6 changes: 4 additions & 2 deletions test_runner/src/main/kotlin/ftl/run/common/GetLastArgs.kt
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package ftl.run.common
import ftl.args.AndroidArgs
import ftl.args.IArgs
import ftl.args.IosArgs
import ftl.args.validate
import ftl.args.validateRefresh
import ftl.config.FtlConstants
import ftl.run.exception.FlankGeneralError
import java.nio.file.Paths
Expand All @@ -15,8 +17,8 @@ internal fun getLastArgs(args: IArgs): IArgs {
val androidConfig = Paths.get(args.localResultDir, lastRun, FtlConstants.defaultAndroidConfig)

return when {
iosConfig.toFile().exists() -> IosArgs.load(iosConfig)
androidConfig.toFile().exists() -> AndroidArgs.load(androidConfig)
iosConfig.toFile().exists() -> IosArgs.load(iosConfig).validateRefresh()
androidConfig.toFile().exists() -> AndroidArgs.load(androidConfig).validate()
else -> throw FlankGeneralError("No config file found in the last run folder: $lastRun")
}
}
Loading